From eb1a89f3c13d4e80516cc372cef3dc505ef864f3 Mon Sep 17 00:00:00 2001 From: Mitchell Hamilton Date: Wed, 29 Sep 2021 15:41:05 +1000 Subject: [PATCH] Aff filter coercion and validation back to filter access control (#6678) --- .changeset/few-bats-watch.md | 5 ++ .../keystone/src/lib/core/access-control.ts | 15 ++++- .../filter-coercion-and-validation.test.ts | 63 +++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 .changeset/few-bats-watch.md create mode 100644 tests/api-tests/access-control/filter-coercion-and-validation.test.ts diff --git a/.changeset/few-bats-watch.md b/.changeset/few-bats-watch.md new file mode 100644 index 00000000000..3e46e6e8055 --- /dev/null +++ b/.changeset/few-bats-watch.md @@ -0,0 +1,5 @@ +--- +'@keystone-next/keystone': patch +--- + +Fixed returning filters like `{ NOT: [{ name: { equals: 'blah' } }] }` from filter access control and improve error messages when returning bad filters from filter access control diff --git a/packages/keystone/src/lib/core/access-control.ts b/packages/keystone/src/lib/core/access-control.ts index 66c0dbedcb8..8b7710a01ac 100644 --- a/packages/keystone/src/lib/core/access-control.ts +++ b/packages/keystone/src/lib/core/access-control.ts @@ -1,3 +1,4 @@ +import { assertInputObjectType } from 'graphql'; import { BaseGeneratedListTypes, CreateListItemAccessControl, @@ -12,7 +13,9 @@ import { ListOperationAccessControl, ListFilterAccessControl, KeystoneContext, + getGqlNames, } from '../../types'; +import { coerceAndValidateForGraphQLInput } from '../coerceAndValidateForGraphQLInput'; import { accessReturnError, extensionError } from './graphql-errors'; import { InitialisedList } from './types-for-lists'; import { InputFilter } from './where-inputs'; @@ -58,7 +61,17 @@ export async function getAccessFilters( const access = list.access.filter[operation]; try { // @ts-ignore - return typeof access === 'function' ? await access(args) : access; + let filters = typeof access === 'function' ? await access(args) : access; + if (typeof filters === 'boolean') { + return filters; + } + const schema = context.sudo().graphql.schema; + const whereInput = assertInputObjectType(schema.getType(getGqlNames(list).whereInputName)); + const result = coerceAndValidateForGraphQLInput(schema, whereInput, filters); + if (result.kind === 'valid') { + return result.value; + } + throw result.error; } catch (error: any) { throw extensionError('Access control', [ { error, tag: `${args.listKey}.access.filter.${args.operation}` }, diff --git a/tests/api-tests/access-control/filter-coercion-and-validation.test.ts b/tests/api-tests/access-control/filter-coercion-and-validation.test.ts new file mode 100644 index 00000000000..75f0dd7f03b --- /dev/null +++ b/tests/api-tests/access-control/filter-coercion-and-validation.test.ts @@ -0,0 +1,63 @@ +import { text } from '@keystone-next/keystone/fields'; +import { list } from '@keystone-next/keystone'; +import { setupTestRunner } from '@keystone-next/keystone/testing'; +import { apiTestConfig } from '../utils'; + +const runner = setupTestRunner({ + config: apiTestConfig({ + lists: { + BadAccess: list({ + fields: { name: text({ isFilterable: true, isOrderable: true }) }, + access: { + filter: { + query: () => { + return { + name: 'blah', + }; + }, + }, + }, + }), + Coercion: list({ + fields: { name: text({ isFilterable: true, isOrderable: true }) }, + access: { + filter: { + query: () => { + return { NOT: { name: { equals: 'blah' } } }; + }, + }, + }, + }), + }, + }), +}); + +describe('Access control - Filter', () => { + test( + 'findMany - Bad function return value', + runner(async ({ graphQLRequest }) => { + // Valid name + const { body } = await graphQLRequest({ + query: `query { badAccesses { id } }`, + }); + + // Returns null and throws an error + expect(body.data).toEqual({ badAccesses: null }); + expect(body.errors).toHaveLength(1); + expect(body.errors[0].path).toEqual(['badAccesses']); + expect(body.errors[0].message).toMatchInlineSnapshot(` +"An error occured while running \\"Access control\\". + - BadAccess.access.filter.query: Variable \\"$where\\" got invalid value \\"blah\\" at \\"where.name\\"; Expected type \\"StringFilter\\" to be an object." +`); + }) + ); + test( + 'findMany - Coercion', + runner(async ({ context }) => { + await context.query.Coercion.createMany({ data: [{ name: 'something' }, { name: 'blah' }] }); + expect(await context.query.Coercion.findMany({ query: 'name' })).toEqual([ + { name: 'something' }, + ]); + }) + ); +});