From 179634ebd6f2ebac750ccb80eb9dd0a0e1f093fc Mon Sep 17 00:00:00 2001 From: Yiming Date: Sun, 4 Aug 2024 07:51:48 -0700 Subject: [PATCH] fix(runtime): improved query reduction to workaround Prisma issue prisma/prisma#21856 (#1634) --- packages/runtime/jest.config.ts | 1 + .../src/enhancements/policy/policy-utils.ts | 126 +++++++++++------- .../runtime/tests/policy/reduction.test.ts | 67 ++++++++++ tests/regression/tests/issue-1627.test.ts | 50 +++++++ 4 files changed, 193 insertions(+), 51 deletions(-) create mode 120000 packages/runtime/jest.config.ts create mode 100644 packages/runtime/tests/policy/reduction.test.ts create mode 100644 tests/regression/tests/issue-1627.test.ts diff --git a/packages/runtime/jest.config.ts b/packages/runtime/jest.config.ts new file mode 120000 index 000000000..a12d812fd --- /dev/null +++ b/packages/runtime/jest.config.ts @@ -0,0 +1 @@ +../../jest.config.ts \ No newline at end of file diff --git a/packages/runtime/src/enhancements/policy/policy-utils.ts b/packages/runtime/src/enhancements/policy/policy-utils.ts index 91e460783..79c50b6c9 100644 --- a/packages/runtime/src/enhancements/policy/policy-utils.ts +++ b/packages/runtime/src/enhancements/policy/policy-utils.ts @@ -30,6 +30,7 @@ import { Logger } from '../logger'; import { QueryUtils } from '../query-utils'; import type { EntityChecker, ModelPolicyDef, PermissionCheckerFunc, PolicyDef, PolicyFunc, ZodSchemas } from '../types'; import { formatObject, prismaClientKnownRequestError } from '../utils'; +import { isPlainObject } from 'is-plain-object'; /** * Access policy enforcement utilities @@ -107,23 +108,63 @@ export class PolicyUtil extends QueryUtils { // Static True/False conditions // https://www.prisma.io/docs/concepts/components/prisma-client/null-and-undefined#the-effect-of-null-and-undefined-on-conditionals - public isTrue(condition: object) { - if (condition === null || condition === undefined) { + private singleKey(obj: object | null | undefined, key: string): obj is { [key: string]: unknown } { + if (!obj) { return false; } else { - return ( - (typeof condition === 'object' && Object.keys(condition).length === 0) || - ('AND' in condition && Array.isArray(condition.AND) && condition.AND.length === 0) - ); + return Object.keys(obj).length === 1 && Object.keys(obj)[0] === key; } } - public isFalse(condition: object) { - if (condition === null || condition === undefined) { + public isTrue(condition: object | null | undefined) { + if (condition === null || condition === undefined || !isPlainObject(condition)) { return false; - } else { - return 'OR' in condition && Array.isArray(condition.OR) && condition.OR.length === 0; } + + // {} is true + if (Object.keys(condition).length === 0) { + return true; + } + + // { OR: TRUE } is true + if (this.singleKey(condition, 'OR') && typeof condition.OR === 'object' && this.isTrue(condition.OR)) { + return true; + } + + // { NOT: FALSE } is true + if (this.singleKey(condition, 'NOT') && typeof condition.NOT === 'object' && this.isFalse(condition.NOT)) { + return true; + } + + // { AND: [] } is true + if (this.singleKey(condition, 'AND') && Array.isArray(condition.AND) && condition.AND.length === 0) { + return true; + } + + return false; + } + + public isFalse(condition: object | null | undefined) { + if (condition === null || condition === undefined || !isPlainObject(condition)) { + return false; + } + + // { AND: FALSE } is false + if (this.singleKey(condition, 'AND') && typeof condition.AND === 'object' && this.isFalse(condition.AND)) { + return true; + } + + // { NOT: TRUE } is false + if (this.singleKey(condition, 'NOT') && typeof condition.NOT === 'object' && this.isTrue(condition.NOT)) { + return true; + } + + // { OR: [] } is false + if (this.singleKey(condition, 'OR') && Array.isArray(condition.OR) && condition.OR.length === 0) { + return true; + } + + return false; } private makeTrue() { @@ -149,11 +190,6 @@ export class PolicyUtil extends QueryUtils { const result: any = {}; for (const [key, value] of Object.entries(condition)) { - if (this.isFalse(result)) { - // already false, no need to continue - break; - } - if (value === null || value === undefined) { result[key] = value; continue; @@ -165,14 +201,13 @@ export class PolicyUtil extends QueryUtils { .map((c: any) => this.reduce(c)) .filter((c) => c !== undefined && !this.isTrue(c)); if (children.length === 0) { - result[key] = []; // true + // { ..., AND: [] } + result[key] = []; } else if (children.some((c) => this.isFalse(c))) { - result['OR'] = []; // false + // { ..., AND: { OR: [] } } + result[key] = this.makeFalse(); } else { - if (!this.isTrue({ AND: result[key] })) { - // use AND only if it's not already true - result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children; - } + result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children; } break; } @@ -182,54 +217,43 @@ export class PolicyUtil extends QueryUtils { .map((c: any) => this.reduce(c)) .filter((c) => c !== undefined && !this.isFalse(c)); if (children.length === 0) { - result[key] = []; // false + // { ..., OR: [] } + result[key] = []; } else if (children.some((c) => this.isTrue(c))) { - result['AND'] = []; // true + // { ..., OR: { AND: [] } } + result[key] = this.makeTrue(); } else { - if (!this.isFalse({ OR: result[key] })) { - // use OR only if it's not already false - result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children; - } + result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children; } break; } case 'NOT': { - const children = enumerate(value) - .map((c: any) => this.reduce(c)) - .filter((c) => c !== undefined && !this.isFalse(c)); - if (children.length === 0) { - // all clauses are false, result is a constant true, - // thus eliminated (not adding into result) - } else if (children.some((c) => this.isTrue(c))) { - // some clauses are true, result is a constant false, - // eliminate all other keys and set entire condition to false - Object.keys(result).forEach((k) => delete result[k]); - result['OR'] = []; // this will cause the outer loop to exit too - } else { - result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children; - } + const children = enumerate(value).map((c: any) => this.reduce(c)); + result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children; break; } default: { - const booleanKeys = ['AND', 'OR', 'NOT', 'is', 'isNot', 'none', 'every', 'some']; - if ( - typeof value === 'object' && - value && - // recurse only if the value has at least one boolean key - Object.keys(value).some((k) => booleanKeys.includes(k)) - ) { - result[key] = this.reduce(value); - } else { + if (!isPlainObject(value)) { + // don't visit into non-plain object values - could be Date, array, etc. result[key] = value; + } else { + result[key] = this.reduce(value); } break; } } } - return result; + // finally normalize constant true/false conditions + if (this.isTrue(result)) { + return this.makeTrue(); + } else if (this.isFalse(result)) { + return this.makeFalse(); + } else { + return result; + } } //#endregion diff --git a/packages/runtime/tests/policy/reduction.test.ts b/packages/runtime/tests/policy/reduction.test.ts new file mode 100644 index 000000000..95a2e6f2f --- /dev/null +++ b/packages/runtime/tests/policy/reduction.test.ts @@ -0,0 +1,67 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { PolicyUtil } from '../../src/enhancements/policy/policy-utils'; + +// eslint-disable-next-line jest/no-disabled-tests +describe.skip('Prisma query reduction tests', () => { + function reduce(query: any) { + const util = new PolicyUtil({} as any, {} as any); + return util['reduce'](query); + } + + const TRUE = { AND: [] }; + const FALSE = { OR: [] }; + + it('should keep regular queries unchanged', () => { + expect(reduce(null)).toEqual(null); + expect(reduce({ x: 1, y: 'hello' })).toEqual({ x: 1, y: 'hello' }); + const d = new Date(); + expect(reduce({ x: d })).toEqual({ x: d }); + }); + + it('should keep regular logical queries unchanged', () => { + expect(reduce({ AND: [{ x: 1 }, { y: 'hello' }] })).toEqual({ AND: [{ x: 1 }, { y: 'hello' }] }); + expect(reduce({ OR: [{ x: 1 }, { y: 'hello' }] })).toEqual({ OR: [{ x: 1 }, { y: 'hello' }] }); + expect(reduce({ NOT: [{ x: 1 }, { y: 'hello' }] })).toEqual({ NOT: [{ x: 1 }, { y: 'hello' }] }); + expect(reduce({ AND: { x: 1 }, OR: { y: 'hello' }, NOT: { z: 2 } })).toEqual({ + AND: { x: 1 }, + OR: { y: 'hello' }, + NOT: { z: 2 }, + }); + expect(reduce({ AND: { x: 1, OR: { y: 'hello', NOT: [{ z: 2 }] } } })).toEqual({ + AND: { x: 1, OR: { y: 'hello', NOT: [{ z: 2 }] } }, + }); + }); + + it('should handle constant true false', () => { + expect(reduce(undefined)).toEqual(TRUE); + expect(reduce({})).toEqual(TRUE); + expect(reduce(TRUE)).toEqual(TRUE); + expect(reduce(FALSE)).toEqual(FALSE); + }); + + it('should reduce simple true false', () => { + expect(reduce({ AND: TRUE })).toEqual(TRUE); + expect(reduce({ AND: FALSE })).toEqual(FALSE); + expect(reduce({ OR: TRUE })).toEqual(TRUE); + expect(reduce({ OR: FALSE })).toEqual(FALSE); + expect(reduce({ NOT: TRUE })).toEqual(FALSE); + expect(reduce({ NOT: FALSE })).toEqual(TRUE); + }); + + it('should reduce AND queries', () => { + expect(reduce({ AND: [{ x: 1 }, TRUE, { y: 2 }] })).toEqual({ AND: [{ x: 1 }, { y: 2 }] }); + expect(reduce({ AND: [{ x: 1 }, FALSE, { y: 2 }] })).toEqual(FALSE); + expect(reduce({ AND: [{ x: 1 }, TRUE, FALSE, { y: 2 }] })).toEqual(FALSE); + }); + + it('should reduce OR queries', () => { + expect(reduce({ OR: [{ x: 1 }, TRUE, { y: 2 }] })).toEqual(TRUE); + expect(reduce({ OR: [{ x: 1 }, FALSE, { y: 2 }] })).toEqual({ OR: [{ x: 1 }, { y: 2 }] }); + expect(reduce({ OR: [{ x: 1 }, TRUE, FALSE, { y: 2 }] })).toEqual(TRUE); + }); + + it('should reduce NOT queries', () => { + expect(reduce({ NOT: { AND: [FALSE, { x: 1 }] } })).toEqual(TRUE); + expect(reduce({ NOT: { OR: [TRUE, { x: 1 }] } })).toEqual(FALSE); + }); +}); diff --git a/tests/regression/tests/issue-1627.test.ts b/tests/regression/tests/issue-1627.test.ts new file mode 100644 index 000000000..cc3c9fe00 --- /dev/null +++ b/tests/regression/tests/issue-1627.test.ts @@ -0,0 +1,50 @@ +import { loadSchema } from '@zenstackhq/testtools'; +describe('issue 1627', () => { + it('regression', async () => { + const { prisma, enhance } = await loadSchema( + ` +model User { + id String @id + memberships GymUser[] +} + +model Gym { + id String @id + members GymUser[] + + @@allow('all', true) +} + +model GymUser { + id String @id + userID String + user User @relation(fields: [userID], references: [id]) + gymID String? + gym Gym? @relation(fields: [gymID], references: [id]) + role String + + @@allow('read',gym.members?[user == auth() && (role == "ADMIN" || role == "TRAINER")]) + @@unique([userID, gymID]) +} + ` + ); + + await prisma.user.create({ data: { id: '1' } }); + + await prisma.gym.create({ + data: { + id: '1', + members: { + create: { + id: '1', + user: { connect: { id: '1' } }, + role: 'ADMIN', + }, + }, + }, + }); + + const db = enhance(); + await expect(db.gymUser.findMany()).resolves.toHaveLength(0); + }); +});