From 26a5a7736b92811fb1a3b6f6f1a7c5a08dfef831 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Mon, 26 Aug 2024 16:25:31 -0700 Subject: [PATCH] fix: field-level policy should filter out records when the field used for filtering is not allowed to read Fixes #1644 --- .../src/enhancements/policy/policy-utils.ts | 35 +++++++++++-------- .../create-many-and-return.test.ts | 21 ++++++----- tests/regression/tests/issue-1644.test.ts | 23 ++++++++++++ 3 files changed, 55 insertions(+), 24 deletions(-) create mode 100644 tests/regression/tests/issue-1644.test.ts diff --git a/packages/runtime/src/enhancements/policy/policy-utils.ts b/packages/runtime/src/enhancements/policy/policy-utils.ts index 2df2ea2e2..b676d84ba 100644 --- a/packages/runtime/src/enhancements/policy/policy-utils.ts +++ b/packages/runtime/src/enhancements/policy/policy-utils.ts @@ -473,10 +473,11 @@ export class PolicyUtil extends QueryUtils { let mergedGuard = guard; if (args.where) { - // inject into relation fields: + // inject into fields: // to-many: some/none/every // to-one: direct-conditions/is/isNot - mergedGuard = this.injectReadGuardForRelationFields(db, model, args.where, guard); + // regular fields + mergedGuard = this.buildReadGuardForFields(db, model, args.where, guard); } args.where = this.and(args.where, mergedGuard); @@ -485,7 +486,7 @@ export class PolicyUtil extends QueryUtils { // Injects guard for relation fields nested in `payload`. The `modelGuard` parameter represents the model-level guard for `model`. // The function returns a modified copy of `modelGuard` with field-level policies combined. - private injectReadGuardForRelationFields(db: CrudContract, model: string, payload: any, modelGuard: any) { + private buildReadGuardForFields(db: CrudContract, model: string, payload: any, modelGuard: any) { if (!payload || typeof payload !== 'object' || Object.keys(payload).length === 0) { return modelGuard; } @@ -530,12 +531,12 @@ export class PolicyUtil extends QueryUtils { ) { const guard = this.getAuthGuard(db, fieldInfo.type, 'read'); if (payload.some) { - const mergedGuard = this.injectReadGuardForRelationFields(db, fieldInfo.type, payload.some, guard); + const mergedGuard = this.buildReadGuardForFields(db, fieldInfo.type, payload.some, guard); // turn "some" into: { some: { AND: [guard, payload.some] } } payload.some = this.and(payload.some, mergedGuard); } if (payload.none) { - const mergedGuard = this.injectReadGuardForRelationFields(db, fieldInfo.type, payload.none, guard); + const mergedGuard = this.buildReadGuardForFields(db, fieldInfo.type, payload.none, guard); // turn none into: { none: { AND: [guard, payload.none] } } payload.none = this.and(payload.none, mergedGuard); } @@ -545,7 +546,7 @@ export class PolicyUtil extends QueryUtils { // ignore empty every clause Object.keys(payload.every).length > 0 ) { - const mergedGuard = this.injectReadGuardForRelationFields(db, fieldInfo.type, payload.every, guard); + const mergedGuard = this.buildReadGuardForFields(db, fieldInfo.type, payload.every, guard); // turn "every" into: { none: { AND: [guard, { NOT: payload.every }] } } if (!payload.none) { @@ -569,18 +570,18 @@ export class PolicyUtil extends QueryUtils { if (payload.is !== undefined || payload.isNot !== undefined) { if (payload.is) { - const mergedGuard = this.injectReadGuardForRelationFields(db, fieldInfo.type, payload.is, guard); + const mergedGuard = this.buildReadGuardForFields(db, fieldInfo.type, payload.is, guard); // merge guard with existing "is": { is: { AND: [originalIs, guard] } } payload.is = this.and(payload.is, mergedGuard); } if (payload.isNot) { - const mergedGuard = this.injectReadGuardForRelationFields(db, fieldInfo.type, payload.isNot, guard); + const mergedGuard = this.buildReadGuardForFields(db, fieldInfo.type, payload.isNot, guard); // merge guard with existing "isNot": { isNot: { AND: [originalIsNot, guard] } } payload.isNot = this.and(payload.isNot, mergedGuard); } } else { - const mergedGuard = this.injectReadGuardForRelationFields(db, fieldInfo.type, payload, guard); + const mergedGuard = this.buildReadGuardForFields(db, fieldInfo.type, payload, guard); // turn direct conditions into: { is: { AND: [ originalConditions, guard ] } } const combined = this.and(clone(payload), mergedGuard); Object.keys(payload).forEach((key) => delete payload[key]); @@ -600,18 +601,22 @@ export class PolicyUtil extends QueryUtils { } if (args.where) { - // inject into relation fields: + // inject into fields: // to-many: some/none/every // to-one: direct-conditions/is/isNot - this.injectReadGuardForRelationFields(db, model, args.where, {}); + // regular fields + const mergedGuard = this.buildReadGuardForFields(db, model, args.where, {}); + this.mergeWhereClause(args.where, mergedGuard); } - if (injected.where && Object.keys(injected.where).length > 0 && !this.isTrue(injected.where)) { - if (!args.where) { - args.where = injected.where; - } else { + if (args.where) { + if (injected.where && Object.keys(injected.where).length > 0) { + // merge injected guard with the user-provided where clause this.mergeWhereClause(args.where, injected.where); } + } else if (injected.where) { + // no user-provided where clause, use the injected one + args.where = injected.where; } // recursively inject read guard conditions into nested select, include, and _count diff --git a/tests/integration/tests/enhancements/with-policy/create-many-and-return.test.ts b/tests/integration/tests/enhancements/with-policy/create-many-and-return.test.ts index c96f16256..e03cd096b 100644 --- a/tests/integration/tests/enhancements/with-policy/create-many-and-return.test.ts +++ b/tests/integration/tests/enhancements/with-policy/create-many-and-return.test.ts @@ -92,14 +92,17 @@ describe('Test API createManyAndReturn', () => { const db = enhance(); - const r = await db.post.createManyAndReturn({ - data: [ - { title: 'post1', published: true }, - { title: 'post2', published: false }, - ], - }); - expect(r).toHaveLength(2); - expect(r[0].title).toBe('post1'); - expect(r[1].title).toBeUndefined(); + // create should succeed but one result can't be read back + await expect( + db.post.createManyAndReturn({ + data: [ + { title: 'post1', published: true }, + { title: 'post2', published: false }, + ], + }) + ).toBeRejectedByPolicy(); + + // check posts are created + await expect(prisma.post.findMany()).resolves.toHaveLength(2); }); }); diff --git a/tests/regression/tests/issue-1644.test.ts b/tests/regression/tests/issue-1644.test.ts new file mode 100644 index 000000000..212cf5c99 --- /dev/null +++ b/tests/regression/tests/issue-1644.test.ts @@ -0,0 +1,23 @@ +import { loadSchema } from '@zenstackhq/testtools'; +describe('issue 1644', () => { + it('regression', async () => { + const { prisma, enhance } = await loadSchema( + ` + model User { + id Int @id @default(autoincrement()) + email String @unique @email @length(6, 32) @allow('read', auth() == this) + + // full access to all + @@allow('all', true) + } + ` + ); + + await prisma.user.create({ data: { id: 1, email: 'a@example.com' } }); + await prisma.user.create({ data: { id: 2, email: 'b@example.com' } }); + + const db = enhance({ id: 1 }); + await expect(db.user.count({ where: { email: { contains: 'example.com' } } })).resolves.toBe(1); + await expect(db.user.findMany({ where: { email: { contains: 'example.com' } } })).resolves.toHaveLength(1); + }); +});