fix(databases): Ensure that query sanitization is not necessary when …

…using query schemas (#3022)

packages/adapter-commons/src/index.ts

@@ -3,7 +3,7 @@ import { Params } from '@feathersjs/feathers'
 
 export * from './declarations'
 export * from './service'
-export { filterQuery, FILTERS, OPERATORS } from './query'
+export * from './query'
 export * from './sort'
 
 // Return a function that filters a result object or array

packages/adapter-commons/src/query.ts

@@ -1,7 +1,7 @@
 import { _ } from '@feathersjs/commons'
 import { BadRequest } from '@feathersjs/errors'
 import { Query } from '@feathersjs/feathers'
-import { FilterQueryOptions, FilterSettings } from './declarations'
+import { FilterQueryOptions, FilterSettings, PaginationParams } from './declarations'
 
 const parse = (value: any) => (typeof value !== 'undefined' ? parseInt(value, 10) : value)
 
@@ -64,6 +64,26 @@ const getQuery = (query: Query, settings: FilterQueryOptions) => {
   }, {} as Query)
 }
 
+/**
+ * Returns the converted `$limit` value based on the `paginate` configuration.
+ * @param _limit The limit value
+ * @param paginate The pagination options
+ * @returns The converted $limit value
+ */
+export const getLimit = (_limit: any, paginate?: PaginationParams) => {
+  const limit = parse(_limit)
+
+  if (paginate && (paginate.default || paginate.max)) {
+    const base = paginate.default || 0
+    const lower = typeof limit === 'number' && !isNaN(limit) && limit >= 0 ? limit : base
+    const upper = typeof paginate.max === 'number' ? paginate.max : Number.MAX_VALUE
+
+    return Math.min(lower, upper)
+  }
+
+  return limit
+}
+
 export const OPERATORS = ['$in', '$nin', '$lt', '$lte', '$gt', '$gte', '$ne', '$or']
 
 export const FILTERS: FilterSettings = {
@@ -79,19 +99,7 @@ export const FILTERS: FilterSettings = {
       return result
     }, {} as { [key: string]: number })
   },
-  $limit: (_limit: any, { paginate }: FilterQueryOptions) => {
-    const limit = parse(_limit)
-
-    if (paginate && (paginate.default || paginate.max)) {
-      const base = paginate.default || 0
-      const lower = typeof limit === 'number' && !isNaN(limit) && limit >= 0 ? limit : base
-      const upper = typeof paginate.max === 'number' ? paginate.max : Number.MAX_VALUE
-
-      return Math.min(lower, upper)
-    }
-
-    return limit
-  },
+  $limit: (_limit: any, { paginate }: FilterQueryOptions) => getLimit(_limit, paginate),
   $select: (select: any) => {
     if (Array.isArray(select)) {
       return select.map((current) => `${current}`)

packages/knex/package.json

@@ -61,6 +61,7 @@
   },
   "devDependencies": {
     "@feathersjs/adapter-tests": "^5.0.0-pre.35",
+    "@feathersjs/schema": "^5.0.0-pre.35",
     "@types/mocha": "^10.0.1",
     "@types/node": "^18.11.18",
     "knex": "^2.3.0",

packages/knex/src/adapter.ts

@@ -1,6 +1,6 @@
 import { Id, NullableId, Paginated, Query } from '@feathersjs/feathers'
 import { _ } from '@feathersjs/commons'
-import { AdapterBase, PaginationOptions, filterQuery } from '@feathersjs/adapter-commons'
+import { AdapterBase, PaginationOptions, AdapterQuery, getLimit } from '@feathersjs/adapter-commons'
 import { BadRequest, MethodNotAllowed, NotFound } from '@feathersjs/errors'
 import { Knex } from 'knex'
 
@@ -50,7 +50,7 @@ export class KnexAdapter<
         ...options.filters,
         $and: (value: any) => value
       },
-      operators: [...(options.operators || []), '$like', '$notlike', '$ilike', '$and', '$or']
+      operators: [...(options.operators || []), '$like', '$notlike', '$ilike']
     })
   }
 
@@ -150,9 +150,14 @@ export class KnexAdapter<
 
   filterQuery(params: ServiceParams) {
     const options = this.getOptions(params)
-    const { filters, query } = filterQuery(params?.query || {}, options)
+    const { $select, $sort, $limit: _limit, $skip = 0, ...query } = (params.query || {}) as AdapterQuery
+    const $limit = getLimit(_limit, options.paginate)
 
-    return { filters, query, paginate: options.paginate }
+    return {
+      paginate: options.paginate,
+      filters: { $select, $sort, $limit, $skip },
+      query
+    }
   }
 
   async _find(params?: ServiceParams & { paginate?: PaginationOptions }): Promise<Paginated<Result>>

packages/knex/test/index.test.ts

@@ -3,6 +3,7 @@ import assert from 'assert'
 import { feathers, HookContext, Service } from '@feathersjs/feathers'
 import adapterTests from '@feathersjs/adapter-tests'
 import { errors } from '@feathersjs/errors'
+import { Ajv, getValidator, querySyntax, hooks } from '@feathersjs/schema'
 
 import connection from './connection'
 import { ERROR, KnexAdapterParams, KnexService, transaction } from '../src/index'
@@ -126,6 +127,38 @@ const clean = async () => {
   })
 }
 
+const personSchema = {
+  $id: 'Person',
+  type: 'object',
+  additionalProperties: false,
+  required: ['_id', 'name', 'age'],
+  properties: {
+    id: { type: 'number' },
+    name: { type: 'string' },
+    age: { type: ['number', 'null'] },
+    time: { type: 'string' },
+    create: { type: 'boolean' }
+  }
+} as const
+const personQuery = {
+  $id: 'PersonQuery',
+  type: 'object',
+  additionalProperties: false,
+  properties: {
+    ...querySyntax(personSchema.properties, {
+      name: {
+        $like: { type: 'string' },
+        $ilike: { type: 'string' },
+        $notlike: { type: 'string' }
+      }
+    })
+  }
+} as const
+const validator = new Ajv({
+  coerceTypes: true
+})
+const personQueryValidator = getValidator(personQuery, validator)
+
 type Person = {
   id: number
   name: string
@@ -195,6 +228,11 @@ describe('Feathers Knex Service', () => {
     .use('todos', todos)
   const peopleService = app.service('people')
 
+  peopleService.hooks({
+    before: {
+      find: [hooks.validateQuery(personQueryValidator)]
+    }
+  })
   before(() => {
     if (TYPE === 'sqlite') {
       // Attach the public database to mimic a "schema"

packages/mongodb/package.json

@@ -61,6 +61,7 @@
   },
   "devDependencies": {
     "@feathersjs/adapter-tests": "^5.0.0-pre.35",
+    "@feathersjs/schema": "^5.0.0-pre.35",
     "@types/mocha": "^10.0.1",
     "@types/node": "^18.11.18",
     "mocha": "^10.2.0",

packages/mongodb/src/adapter.ts

@@ -17,7 +17,8 @@ import {
   AdapterParams,
   AdapterServiceOptions,
   PaginationOptions,
-  AdapterQuery
+  AdapterQuery,
+  getLimit
 } from '@feathersjs/adapter-commons'
 import { Id, Paginated } from '@feathersjs/feathers'
 import { errorHandler } from './error-handler'
@@ -75,8 +76,9 @@ export class MongoDbAdapter<
   }
 
   filterQuery(id: NullableAdapterId, params: ServiceParams) {
-    const { $select, $sort, $limit, $skip, ...query } = (params.query || {}) as AdapterQuery
-
+    const options = this.getOptions(params)
+    const { $select, $sort, $limit: _limit, $skip = 0, ...query } = (params.query || {}) as AdapterQuery
+    const $limit = getLimit(_limit, options.paginate)
     if (id !== null) {
       query.$and = (query.$and || []).concat({
         [this.id]: this.getObjectId(id)
@@ -226,7 +228,6 @@ export class MongoDbAdapter<
     const { paginate, useEstimatedDocumentCount } = this.getOptions(params)
     const { filters, query } = this.filterQuery(null, params)
     const useAggregation = !params.mongodb && filters.$limit !== 0
-
     const countDocuments = async () => {
       if (paginate && paginate.default) {
         const model = await this.getModel(params)

packages/mongodb/test/index.test.ts

@@ -2,10 +2,10 @@ import { Db, MongoClient, ObjectId } from 'mongodb'
 import adapterTests from '@feathersjs/adapter-tests'
 import assert from 'assert'
 import { MongoMemoryServer } from 'mongodb-memory-server'
-
+import { Ajv, FromSchema, getValidator, hooks, querySyntax } from '@feathersjs/schema'
 import { feathers } from '@feathersjs/feathers'
 import errors from '@feathersjs/errors'
-import { MongoDBService } from '../src'
+import { MongoDBService, AdapterId } from '../src'
 
 const testSuite = adapterTests([
   '.options',
@@ -82,16 +82,43 @@ const testSuite = adapterTests([
 ])
 
 describe('Feathers MongoDB Service', () => {
-  type Person = {
-    _id: string
-    name: string
-    age: number
-    friends?: string[]
-    team: string
-    $push: {
-      friends: string
+  const personSchema = {
+    $id: 'Person',
+    type: 'object',
+    additionalProperties: false,
+    required: ['_id', 'name', 'age'],
+    properties: {
+      _id: { oneOf: [{ type: 'string' }, { type: 'object' }] },
+      name: { type: 'string' },
+      age: { type: 'number' },
+      friends: { type: 'array', items: { type: 'string' } },
+      team: { type: 'string' },
+      $push: {
+        type: 'object',
+        properties: {
+          friends: { type: 'string' }
+        }
+      }
     }
-  }
+  } as const
+  const personQuery = {
+    $id: 'PersonQuery',
+    type: 'object',
+    additionalProperties: false,
+    properties: {
+      ...querySyntax(personSchema.properties, {
+        name: {
+          $regex: { type: 'string' }
+        }
+      })
+    }
+  } as const
+  const validator = new Ajv({
+    coerceTypes: true
+  })
+  const personQueryValidator = getValidator(personQuery, validator)
+
+  type Person = Omit<FromSchema<typeof personSchema>, '_id'> & { _id: AdapterId }
   type Todo = {
     _id: string
     name: string
@@ -135,6 +162,12 @@ describe('Feathers MongoDB Service', () => {
       })
     )
 
+    app.service('people').hooks({
+      before: {
+        find: [hooks.validateQuery(personQueryValidator)]
+      }
+    })
+
     db.collection('people-customid').deleteMany({})
     db.collection('people').deleteMany({})
     db.collection('todos').deleteMany({})
@@ -441,6 +474,22 @@ describe('Feathers MongoDB Service', () => {
     })
   })
 
+  describe('query validation', () => {
+    it('validated queries are not sanitized', async () => {
+      const dave = await app.service('people').create({ name: 'Dave' })
+      const result = await app.service('people').find({
+        query: {
+          name: {
+            $regex: 'Da.*'
+          }
+        }
+      })
+      assert.deepStrictEqual(result, [dave])
+
+      app.service('people').remove(dave._id)
+    })
+  })
+
   testSuite(app, errors, 'people', '_id')
   testSuite(app, errors, 'people-customid', 'customid')
 })

packages/schema/src/json-schema.ts

@@ -172,6 +172,25 @@ export const querySyntax = <
 ) => {
   const keys = Object.keys(definition)
   const props = queryProperties(definition, extensions)
+  const $or = {
+    type: 'array',
+    items: {
+      type: 'object',
+      additionalProperties: false,
+      properties: props
+    }
+  } as const
+  const $and = {
+    type: 'array',
+    items: {
+      type: 'object',
+      additionalProperties: false,
+      properties: {
+        ...props,
+        $or
+      }
+    }
+  } as const
 
   return {
     $limit: {
@@ -203,14 +222,8 @@ export const querySyntax = <
         ...(keys.length > 0 ? { enum: keys as any as (keyof T)[] } : {})
       }
     },
-    $or: {
-      type: 'array',
-      items: {
-        type: 'object',
-        additionalProperties: false,
-        properties: props
-      }
-    },
+    $or,
+    $and,
     ...props
   } as const
 }