openshiftio / openshift.io

Red Hat OpenShift.io is an end-to-end development environment for planning, building and deploying modern applications.
https://openshift.io
97 stars 66 forks source link

Endpoints accept SQL LIKE syntax for text query #4206

Open kpiwko opened 6 years ago

kpiwko commented 6 years ago
Issue Overview

Endpoint /api/search/spaces provides parameter q. However, this parameter is very confusing as _ is a reserved character in LIKE parameter and escape rules are needed to be provided by user.

Expected Behaviour

User can provide text in human readable / expected format, e.g. a search for space named 'OpenShift_io'

Current Behaviour

User need to escape accordingly to PostgreSQL rules https://www.postgresql.org/docs/7.3/static/functions-matching.html which is an implementation detail.

Steps To Reproduce
  1. curl -sX GET 'https://api.openshift.io/api/search/spaces?q=OpenShift_io' | json | grep -A5 -B5 Agile finds nothing
  2. curl -sX GET 'https://api.openshift.io/api/search/spaces?q=OpenShift\_io' | json | grep -A5 -B5 Agile finds the right space
Additional Information

There is a mixed usage of LIKE and ILIKE in the code base. Looks like usage can be standardized

grep -IR 'LIKE ' .
./space/space.go:       db = db.Where("LOWER(name) LIKE ?", "%"+strings.ToLower(*q)+"%")
./space/space.go:       db = db.Or("LOWER(description) LIKE ?", "%"+strings.ToLower(*q)+"%")
./workitem/workitemtype_repository_bench_test.go://         db := r.DB.Select("id").Where("space_id = ? AND path::text LIKE '"+path.ConvertToLtree(fxt.WorkItemTypes[0].ID)+".%'", fxt.WorkItemTypes[0].SpaceID.String())
./workitem/expression_compiler.go:          return Column(WorkItemStorage{}.TableName(), "fields") + `->>'` + left.FieldName + `' ILIKE ?`
./workitem/expression_compiler_blackbox_test.go:        assert.Equal(t, workitem.Column(wiTbl, "fields")+`->>'system.title' ILIKE ?`, where)
./workitem/expression_compiler_blackbox_test.go:        assert.Equal(t, workitem.Column(wiTbl, "fields")+`->>'system.title;DELETE FROM work_items' ILIKE ?`, where)
./workitem/workitemtype_repository.go:  db := r.db.Select("id").Where("space_template_id = ? AND path::text LIKE '"+path.ConvertToLtree(SystemPlannerItem)+".%'", spaceTemplateID.String()).Order("created_at")

Gorm is preventing SQL injection if user input is provided as a parameter, that's good.

'*' as possible 'search all' option is not mentioned anywhere in the doc.

GeorgeActon commented 6 years ago

@michaelkleinhenz added team planner. Please correct if wrong.

joshuawilson commented 6 years ago

I'm not sure this is a Planner issue. It might be in the Work Item Tracker but I'm not sure who owns spaces on the backend.

Does anyone think we should attempt to catch/prevent this in the UI? I know it needs to be dealt with in the backend for security reasons.

@aslakknutsen @bartoszmajsak