diff --git a/docs/api/databases/knex.md b/docs/api/databases/knex.md index fc57770c75..510695538b 100644 --- a/docs/api/databases/knex.md +++ b/docs/api/databases/knex.md @@ -113,19 +113,21 @@ In addition to the [common querying mechanism](./querying.md), this adapter also ```ts const messageQuerySchema = Type.Intersect( [ - // This will additionally allow querying for `{ name: { $ilike: 'Dav%' } }` querySyntax(messageQueryProperties, { name: { - $ilike: Type.String() + $like: Type.String(), + $notlike: Type.String(), + $ilike: Type.String() // PostgreSQL } }), - // Add additional query properties here - Type.Object({}) + Type.Object({}, { additionalProperties: false }) ], { additionalProperties: false } ) ``` +More extension examples are in [querySyntax](../schema/typebox.md#querysyntax). `{ age: null }` and `{ age: { $ne: null } }` work when the query property type includes `null` (see the `age` field in the adapter tests). + ### $like Find all records where the value matches the given string pattern. The following query retrieves all messages that start with `Hello`: diff --git a/docs/api/databases/mongodb.md b/docs/api/databases/mongodb.md index 785a77b05f..fc868299de 100644 --- a/docs/api/databases/mongodb.md +++ b/docs/api/databases/mongodb.md @@ -215,13 +215,13 @@ new MongoDBService({
-Note that in a normal application all MongoDB specific operators have to explicitly be added to the [TypeBox query schema](../schema/typebox.md#query-schemas) or [JSON query schema](../schema/schema.md#querysyntax). +Note that in a normal application all MongoDB specific operators have to explicitly be added to the [TypeBox query schema](../schema/typebox.md#querysyntax) or [JSON query schema](../schema/schema.md#querysyntax).There are two ways to perform search queries with MongoDB: -- Perform basic Regular Expression matches using the `$regex` filter. +- Perform basic Regular Expression matches using the `$regex` operator. - Perform full-text search using the `$search` filter. ### Basic Regex Search @@ -234,6 +234,19 @@ You can perform basic search using regular expressions with the `$regex` operato } ``` +Allow those operators on the properties that need them: + +```ts +querySyntax(messageQueryProperties, { + text: { + $regex: Type.String(), + $options: Type.String() + } +}) +``` + +If you also use [`validateQuery(schema, { skipSanitize: false })`](../schema/validators.md#keeping-adapter-sanitization), list them on the service as well: `operators: ['$regex', '$options']`. + ### Full-Text Search See the MongoDB documentation for instructions on performing full-text search using the `$search` operator: @@ -456,7 +469,7 @@ validator.addKeyword(keywordObjectId) ### ObjectIdSchema -Both, `@feathersjs/typebox` and `@feathersjs/schema` export an `ObjectIdSchema` helper that creates a schema which can be both, a MongoDB ObjectId or a string that will be converted with the `objectid` keyword: +Both, `@feathersjs/typebox` and `@feathersjs/schema` export an `ObjectIdSchema` helper that creates a schema which can be a MongoDB ObjectId instance or a string that will be converted with the `objectid` keyword. Arbitrary objects — including query operator documents like `{ $ne: null }` or `{ $where: '…' }` — are not valid ObjectIds. ```ts import { ObjectIdSchema } from '@feathersjs/typebox' // or '@feathersjs/schema' diff --git a/docs/api/schema/schema.md b/docs/api/schema/schema.md index f0615acfb2..65d2ae8801 100644 --- a/docs/api/schema/schema.md +++ b/docs/api/schema/schema.md @@ -171,7 +171,7 @@ const userQuery: UserQuery = { } ``` -Additional special query properties [that are not already included in the query syntax](../databases/querying.md) like `$ilike` can be added like this: +Additional operators that are [not already in the common query syntax](../databases/querying.md) (`$like`, `$regex`, …) are added per property. Only add operators your adapter supports. See [TypeBox querySyntax](./typebox.md#querysyntax) for more examples. ```ts import { querySyntax } from '@feathersjs/schema' @@ -184,9 +184,7 @@ export const userQuerySchema = { properties: { ...querySyntax(userSchema.properties, { email: { - $ilike: { - type: 'string' - } + $ilike: { type: 'string' } } } as const) } diff --git a/docs/api/schema/typebox.md b/docs/api/schema/typebox.md index 0bc80386ee..be2daf3ab4 100644 --- a/docs/api/schema/typebox.md +++ b/docs/api/schema/typebox.md @@ -101,30 +101,38 @@ const messageQuerySchema = querySyntax(messageQueryProperties) type MessageQuery = Static
diff --git a/docs/guides/cli/service.schemas.md b/docs/guides/cli/service.schemas.md index 788303c053..140d5d723c 100644 --- a/docs/guides/cli/service.schemas.md +++ b/docs/guides/cli/service.schemas.md @@ -105,7 +105,7 @@ export const messageQueryValidator = getValidator(messageQuerySchema, queryValid export const messageQueryResolver = resolve({}) ``` -To add additional operators like `$like` see the [querySyntax](../../api/schema/typebox.md#querysyntax) documentation. You can also add your own query parameters in the `Type.Object({}, { additionalProperties: false })` definition. +To add additional operators like `$like` or `$regex`, see [querySyntax](../../api/schema/typebox.md#querysyntax). You can also add your own query parameters in the `Type.Object({}, { additionalProperties: false })` definition. diff --git a/packages/adapter-commons/src/query.ts b/packages/adapter-commons/src/query.ts index 1dc31ffdec..38fd2605c8 100644 --- a/packages/adapter-commons/src/query.ts +++ b/packages/adapter-commons/src/query.ts @@ -8,6 +8,10 @@ const parse = (value: any) => (typeof value !== 'undefined' ? parseInt(value, 10 const isPlainObject = (value: any) => _.isObject(value) && value.constructor === {}.constructor const validateQueryProperty = (query: any, operators: string[] = []): Query => { + if (Array.isArray(query)) { + return query.map((value) => validateQueryProperty(value, operators)) + } + if (!isPlainObject(query)) { return query } @@ -17,11 +21,7 @@ const validateQueryProperty = (query: any, operators: string[] = []): Query => { throw new BadRequest(`Invalid query parameter ${key}`, query) } - const value = query[key] - - if (isPlainObject(value)) { - query[key] = validateQueryProperty(value, operators) - } + query[key] = validateQueryProperty(query[key], operators) } return { @@ -91,41 +91,44 @@ export const OPERATORS = ['$in', '$nin', '$lt', '$lte', '$gt', '$gte', '$ne', '$ export const FILTERS: FilterSettings = { $skip: (value: any) => parse(value), - $sort: (sort: any): { [key: string]: number } => { + $sort: (sort: any, { operators }: FilterQueryOptions): { [key: string]: any } => { if (typeof sort !== 'object' || Array.isArray(sort)) { return sort } return Object.keys(sort).reduce( (result, key) => { - result[key] = typeof sort[key] === 'object' ? sort[key] : parse(sort[key]) + result[key] = + typeof sort[key] === 'object' && sort[key] !== null + ? validateQueryProperty(sort[key], operators) + : parse(sort[key]) return result }, - {} as { [key: string]: number } + {} as { [key: string]: any } ) }, $limit: (_limit: any, { paginate }: FilterQueryOptions) => getLimit(_limit, paginate), - $select: (select: any) => { + $select: (select: any, { operators }: FilterQueryOptions) => { if (Array.isArray(select)) { return select.map((current) => `${current}`) } - return select + return validateQueryProperty(select, operators) }, $or: (or: any, { operators }: FilterQueryOptions) => { if (Array.isArray(or)) { return or.map((current) => validateQueryProperty(current, operators)) } - return or + return validateQueryProperty(or, operators) }, $and: (and: any, { operators }: FilterQueryOptions) => { if (Array.isArray(and)) { return and.map((current) => validateQueryProperty(current, operators)) } - return and + return validateQueryProperty(and, operators) } } diff --git a/packages/adapter-commons/test/query.test.ts b/packages/adapter-commons/test/query.test.ts index 805704c97e..d40f4d7964 100644 --- a/packages/adapter-commons/test/query.test.ts +++ b/packages/adapter-commons/test/query.test.ts @@ -245,6 +245,169 @@ describe('@feathersjs/adapter-commons/filterQuery', () => { $or: [{ value: { $gte: 10 } }] }) }) + + it('rejects unknown operators nested one array level under $or', () => { + assert.throws( + () => { + filterQuery({ + $or: [[{ $where: '1==1' }]] + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $where' + } + ) + }) + + it('rejects unknown operators nested one array level under $and', () => { + assert.throws( + () => { + filterQuery({ + $and: [[{ $where: '1==1' }]] + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $where' + } + ) + }) + + it('rejects unknown operators in a property value array', () => { + assert.throws( + () => { + filterQuery({ + name: [{ $where: '1==1' }] + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $where' + } + ) + }) + + it('rejects unknown operators nested inside an allowed operator array', () => { + assert.throws( + () => { + filterQuery({ + name: { $in: [{ $where: '1==1' }] } + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $where' + } + ) + }) + + it('rejects unknown operators in deeply nested arrays', () => { + assert.throws( + () => { + filterQuery({ + $or: [[[{ $exists: false }]]] + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $exists' + } + ) + }) + + it('allows primitive arrays and valid nested objects', () => { + const { query, filters } = filterQuery({ + tags: ['a', 'b'], + name: { $in: ['dave', 'alice'] }, + $or: [{ value: { $gte: 10 } }, { name: 'dave' }] + }) + + assert.deepStrictEqual(query, { + tags: ['a', 'b'], + name: { $in: ['dave', 'alice'] } + }) + assert.deepStrictEqual(filters, { + $or: [{ value: { $gte: 10 } }, { name: 'dave' }] + }) + }) + + it('rejects unknown operators in a non-array $or object', () => { + assert.throws( + () => { + filterQuery({ + $or: { $where: '1==1' } + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $where' + } + ) + }) + + it('rejects unknown operators in a non-array $and object', () => { + assert.throws( + () => { + filterQuery({ + $and: { $where: '1==1' } + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $where' + } + ) + }) + + it('rejects unknown operators nested in an object $select', () => { + assert.throws( + () => { + filterQuery({ + $select: { + owned: { $function: { body: 'return 1', lang: 'js', args: [] } } + } + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $function' + } + ) + }) + + it('allows MongoDB inclusion-style object $select', () => { + const { filters } = filterQuery({ + $select: { name: 1, age: 1 } + }) + + assert.deepStrictEqual(filters.$select, { name: 1, age: 1 }) + }) + + it('allows extra operators in object $select when listed on operators', () => { + const { filters } = filterQuery( + { + $select: { score: { $meta: 'textScore' } } + }, + { operators: ['$meta'] } + ) + + assert.deepStrictEqual(filters.$select, { score: { $meta: 'textScore' } }) + }) + + it('rejects unknown operators nested in a $sort value', () => { + assert.throws( + () => { + filterQuery({ + $sort: { score: { $function: { body: 'return 1', lang: 'js', args: [] } } } + }) + }, + { + name: 'BadRequest', + message: 'Invalid query parameter $function' + } + ) + }) }) describe('additional filters', () => { @@ -286,6 +449,42 @@ describe('@feathersjs/adapter-commons/filterQuery', () => { }) }) + describe('configured operators', () => { + it('allows $exists when listed on operators', () => { + const { query } = filterQuery({ name: { $exists: true } }, { operators: ['$exists'] }) + + assert.deepStrictEqual(query, { name: { $exists: true } }) + }) + + it('allows $regex and $options when listed on operators', () => { + const { query } = filterQuery( + { name: { $regex: 'Dav', $options: 'i' } }, + { operators: ['$regex', '$options'] } + ) + + assert.deepStrictEqual(query, { name: { $regex: 'Dav', $options: 'i' } }) + }) + + it('allows $like when listed on operators', () => { + const { query } = filterQuery({ name: { $like: 'D%' } }, { operators: ['$like'] }) + + assert.deepStrictEqual(query, { name: { $like: 'D%' } }) + }) + + it('allows $meta in $select and $sort when listed on operators', () => { + const { filters } = filterQuery( + { + $select: { score: { $meta: 'textScore' } }, + $sort: { score: { $meta: 'textScore' } } + }, + { operators: ['$meta'] } + ) + + assert.deepStrictEqual(filters.$select, { score: { $meta: 'textScore' } }) + assert.deepStrictEqual(filters.$sort, { score: { $meta: 'textScore' } }) + }) + }) + describe('additional operators', () => { it('returns query with default and known additional operators', () => { const { query } = filterQuery( diff --git a/packages/adapter-commons/test/service.test.ts b/packages/adapter-commons/test/service.test.ts index ed5cc4a4c6..dce497b077 100644 --- a/packages/adapter-commons/test/service.test.ts +++ b/packages/adapter-commons/test/service.test.ts @@ -103,6 +103,16 @@ describe('@feathersjs/adapter-commons/service', () => { } ) + await assert.rejects( + () => + service.sanitizeQuery({ + query: { $or: [[{ $where: '1==1' }]] } + }), + { + message: 'Invalid query parameter $where' + } + ) + assert.deepStrictEqual( await service.sanitizeQuery({ adapter: { diff --git a/packages/mongodb/src/converters.ts b/packages/mongodb/src/converters.ts index e38a677def..4e97ccdc5d 100644 --- a/packages/mongodb/src/converters.ts +++ b/packages/mongodb/src/converters.ts @@ -44,14 +44,39 @@ export async function resolveQueryObjectId(value: ObjectIdParam | IdQueryObject< return convertedObject } +const isObjectId = (value: any): value is ObjectId => { + if (value == null || typeof value !== 'object') { + return false + } + + if (value instanceof ObjectId) { + return true + } + + // Another mongodb/bson copy of ObjectId (instanceof fails across duplicates). + // Reject plain JSON such as `{ _bsontype: 'ObjectId' }`. + return ( + value._bsontype === 'ObjectId' && + value.constructor !== Object && + typeof value.toHexString === 'function' + ) +} + export const keywordObjectId = { keyword: 'objectid', - type: 'string', modifying: true, compile(schemaVal: boolean) { if (!schemaVal) return () => true - return function (value: string, obj: any) { + return function (value: any, obj: any) { + if (isObjectId(value)) { + return true + } + + if (typeof value !== 'string') { + return false + } + const { parentData, parentDataProperty } = obj try { parentData[parentDataProperty] = new ObjectId(value) diff --git a/packages/mongodb/test/converters.test.ts b/packages/mongodb/test/converters.test.ts index 910ba58fa5..e85f100059 100644 --- a/packages/mongodb/test/converters.test.ts +++ b/packages/mongodb/test/converters.test.ts @@ -108,6 +108,39 @@ describe('objectid keyword', () => { assert.equal(validate.errors?.[0].keyword, 'objectid') }) + it('accepts ObjectId instances', async () => { + const schema = { + type: 'object', + properties: { + _id: { type: 'object', objectid: true } + }, + additionalProperties: false + } + const validate = validator.compile(schema) + const data = { _id: new ObjectId() } + + assert.equal(validate(data), true) + assert.ok(data._id instanceof ObjectId) + }) + + it('rejects operator objects that are not ObjectId instances', async () => { + const schema = { + type: 'object', + properties: { + _id: { type: 'object', objectid: true } + }, + additionalProperties: false + } + const validate = validator.compile(schema) + + assert.equal(validate({ _id: { $where: '1==1' } }), false) + assert.equal(validate.errors?.[0].keyword, 'objectid') + assert.equal(validate({ _id: { $ne: null } }), false) + assert.equal(validate({ _id: { $regex: '.*' } }), false) + assert.equal(validate({ _id: { _bsontype: 'ObjectId' } }), false) + assert.equal(validate({ _id: { _bsontype: 'ObjectId', $where: '1==1' } }), false) + }) + it('continues validating nullable unions when an objectid branch fails', async () => { const nullableValidator = new Ajv({ coerceTypes: true, useDefaults: true }) nullableValidator.addKeyword(keywordObjectId) diff --git a/packages/mongodb/test/index.test.ts b/packages/mongodb/test/index.test.ts index 561b75f336..bd5944c741 100644 --- a/packages/mongodb/test/index.test.ts +++ b/packages/mongodb/test/index.test.ts @@ -747,21 +747,11 @@ describe('Feathers MongoDB Service', () => { describe('query validation', () => { it('validated queries are not sanitized', async () => { const people = app.service('people') - // Isolate from earlier tests that mutate shared service options - const previous = { - multi: people.options.multi, - disableObjectify: people.options.disableObjectify, - paginate: people.options.paginate - } - people.options.multi = false - people.options.disableObjectify = false - people.options.paginate = false + const name = `Dave-${Date.now()}` + const inserted = await db.collection('people').insertOne({ name }) + const dave = { _id: inserted.insertedId, name } try { - const name = `Dave-${Date.now()}` - const dave = await people.create({ name }) - assert.ok(dave && dave._id, 'create should return the created person') - // $regex is not in the default operator allowlist; validateQuery marks the // query as validated so sanitizeQuery skips and $regex reaches MongoDB. const result = await people.find({ @@ -773,12 +763,8 @@ describe('Feathers MongoDB Service', () => { } }) assert.deepStrictEqual(result, [dave]) - - await people.remove(dave._id) } finally { - people.options.multi = previous.multi - people.options.disableObjectify = previous.disableObjectify - people.options.paginate = previous.paginate + await db.collection('people').deleteOne({ _id: inserted.insertedId }) } }) }) diff --git a/packages/schema/src/json-schema.ts b/packages/schema/src/json-schema.ts index 0d0739b49b..9d9f55dbac 100644 --- a/packages/schema/src/json-schema.ts +++ b/packages/schema/src/json-schema.ts @@ -239,6 +239,6 @@ export const ObjectIdSchema = () => ({ anyOf: [ { type: 'string', objectid: true }, - { type: 'object', properties: {}, additionalProperties: true } + { type: 'object', objectid: true } ] }) as const diff --git a/packages/schema/test/json-schema.test.ts b/packages/schema/test/json-schema.test.ts index 85a8234104..255b193b0f 100644 --- a/packages/schema/test/json-schema.test.ts +++ b/packages/schema/test/json-schema.test.ts @@ -1,6 +1,7 @@ import Ajv from 'ajv' import assert from 'assert' import { ObjectId as MongoObjectId } from 'mongodb' +import { keywordObjectId } from '@feathersjs/mongodb' import { FromSchema } from '../src/' import { querySyntax, ObjectIdSchema } from '../src/json-schema' @@ -57,6 +58,38 @@ describe('@feathersjs/schema/json-schema', () => { assert.ok(validator(q)) }) + it('can extend common operators people typically add', async () => { + const ajv = new Ajv({ strict: false }) + ajv.addKeyword(keywordObjectId) + + const querySchema = { + type: 'object', + additionalProperties: false, + properties: querySyntax( + { + _id: { anyOf: [ObjectIdSchema(), { type: 'null' }] }, + name: { type: 'string' } + }, + { + name: { + $regex: { type: 'string' }, + $options: { type: 'string' }, + $like: { type: 'string' }, + $exists: { type: 'boolean' } + } + } + ) + } + const validator = ajv.compile(querySchema) + + assert.equal(validator({ name: { $regex: 'Dav', $options: 'i' } }), true) + assert.equal(validator({ name: { $like: 'D%' } }), true) + assert.equal(validator({ name: { $exists: true } }), true) + assert.equal(validator({ _id: null }), true) + assert.equal(validator({ _id: { $ne: null } }), true) + assert.equal(validator({ name: { $where: '1==1' } }), false) + }) + it('$in and $nin works with array definitions', async () => { const schema = { things: { @@ -106,4 +139,43 @@ describe('@feathersjs/schema/json-schema', () => { }) assert.ok(validated2) }) + + it('ObjectIdSchema rejects operator objects when the objectid keyword is registered', async () => { + const ajv = new Ajv({ strict: false }) + ajv.addKeyword(keywordObjectId) + + const schema = { + type: 'object', + properties: { + _id: ObjectIdSchema() + } + } + const validator = ajv.compile(schema) + + assert.equal(validator({ _id: '507f191e810c19729de860ea' }), true) + assert.equal(validator({ _id: new MongoObjectId() }), true) + assert.equal(validator({ _id: { $where: '1==1' } }), false) + assert.equal(validator({ _id: { $regex: '.*' } }), false) + assert.equal(validator({ _id: { $ne: null } }), false) + }) + + it('querySyntax with ObjectIdSchema does not treat operator objects as ids', async () => { + const ajv = new Ajv({ strict: false }) + ajv.addKeyword(keywordObjectId) + + const querySchema = { + type: 'object', + additionalProperties: false, + properties: querySyntax({ + _id: ObjectIdSchema(), + text: { type: 'string' } + }) + } + const validator = ajv.compile(querySchema) + + assert.equal(validator({ _id: '507f191e810c19729de860ea' }), true) + assert.equal(validator({ _id: { $ne: '507f191e810c19729de860ea' } }), true) + assert.equal(validator({ _id: { $where: '1==1' } }), false) + assert.equal(validator({ $or: [{ _id: { $where: '1==1' } }] }), false) + }) }) diff --git a/packages/typebox/src/index.ts b/packages/typebox/src/index.ts index 098cf22a90..9ef726ee8c 100644 --- a/packages/typebox/src/index.ts +++ b/packages/typebox/src/index.ts @@ -198,4 +198,4 @@ export const querySyntax = < } export const ObjectIdSchema = () => - Type.Union([Type.String({ objectid: true }), Type.Object({}, { additionalProperties: true })]) + Type.Union([Type.String({ objectid: true }), Type.Unsafe