From e4129f796bfa2c7e14e30d1abce69e6fab75688c Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Mon, 14 Oct 2024 16:09:43 -0700 Subject: [PATCH 1/3] fix(delegate): policies inherited from delegate base models are not injected into proper hierarchy Fixes #1770 --- .../runtime/src/enhancements/node/delegate.ts | 10 ++-- packages/schema/src/utils/ast-utils.ts | 10 +++- .../with-delegate/policy-interaction.test.ts | 47 ++++++++++++++++++ tests/regression/tests/issue-1770.test.ts | 49 +++++++++++++++++++ 4 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 tests/regression/tests/issue-1770.test.ts diff --git a/packages/runtime/src/enhancements/node/delegate.ts b/packages/runtime/src/enhancements/node/delegate.ts index 5a9ff162c..8a7d613a1 100644 --- a/packages/runtime/src/enhancements/node/delegate.ts +++ b/packages/runtime/src/enhancements/node/delegate.ts @@ -365,12 +365,14 @@ export class DelegateProxyHandler extends DefaultPrismaProxyHandler { if (this.options.processIncludeRelationPayload) { // use the callback in options to process the include payload, so enhancements // like 'policy' can do extra work (e.g., inject policy rules) + + // TODO: this causes both delegate base's policy rules and concrete model's rules to be injected, + // which is not wrong but redundant + await this.options.processIncludeRelationPayload(this.prisma, model, result, this.options, this.context); - // the callback may directly reference fields from polymorphic bases, we need to fix it - // into a proper hierarchy by moving base field references to the base layer relations - const properHierarchy = await this.buildSelectIncludeHierarchy(model, result, false); - result = { ...result, ...properHierarchy }; + const properSelectIncludeHierarchy = await this.buildSelectIncludeHierarchy(model, result, false); + result = { ...result, ...properSelectIncludeHierarchy }; } return result; diff --git a/packages/schema/src/utils/ast-utils.ts b/packages/schema/src/utils/ast-utils.ts index bf935ce20..d4ebead87 100644 --- a/packages/schema/src/utils/ast-utils.ts +++ b/packages/schema/src/utils/ast-utils.ts @@ -116,7 +116,15 @@ function cloneAst( ): Mutable { const clone = copyAstNode(node, buildReference) as Mutable; clone.$container = newContainer; - clone.$inheritedFrom = node.$inheritedFrom ?? getContainerOfType(node, isDataModel); + + if (isDataModel(newContainer) && isDataModelField(node)) { + // walk up the hierarchy to find the model where the field is defined (delegate or abstract base) + const allBases = getRecursiveBases(newContainer); + clone.$inheritedFrom = allBases.find((base) => base.fields.some((f) => f.name === node.name)); + } else { + clone.$inheritedFrom = node.$inheritedFrom ?? getContainerOfType(node, isDataModel); + } + return clone; } diff --git a/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts b/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts index ff791beb1..d149a6392 100644 --- a/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts +++ b/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts @@ -484,6 +484,53 @@ describe('Polymorphic Policy Test', () => { await expect(prisma.post.findUnique({ where: { id: post.id } })).toResolveFalsy(); }); + it('respects sub model policies when queried from a base: case 3', async () => { + const { enhance } = await loadSchema( + ` + model User { + id Int @id @default(autoincrement()) + assets Asset[] + @@allow('all', true) + } + + model Asset { + id Int @id @default(autoincrement()) + user User @relation(fields: [userId], references: [id]) + userId Int + value Int @default(0) + type String + @@delegate(type) + @@allow('all', value > 0) + } + + model Post extends Asset { + title String + deleted Boolean @default(false) + @@deny('read', deleted) + } + ` + ); + + const db = enhance(); + const user = await db.user.create({ data: { id: 1 } }); + + // can't create + await expect( + db.post.create({ data: { id: 1, title: 'Post1', userId: user.id, value: 0 } }) + ).toBeRejectedByPolicy(); + + // can't read back + await expect( + db.post.create({ data: { id: 1, title: 'Post1', userId: user.id, value: 1, deleted: true } }) + ).toBeRejectedByPolicy(); + + await expect( + db.post.create({ data: { id: 2, title: 'Post1', userId: user.id, value: 1, deleted: false } }) + ).toResolveTruthy(); + + await expect(db.asset.findMany()).resolves.toHaveLength(2); + }); + it('respects field-level policies', async () => { const { enhance } = await loadSchema(` model User { diff --git a/tests/regression/tests/issue-1770.test.ts b/tests/regression/tests/issue-1770.test.ts new file mode 100644 index 000000000..03af5cd19 --- /dev/null +++ b/tests/regression/tests/issue-1770.test.ts @@ -0,0 +1,49 @@ +import { loadSchema } from '@zenstackhq/testtools'; + +describe('issue 1770', () => { + it('regression', async () => { + const { enhance } = await loadSchema( + ` + model User { + id String @id @default(cuid()) + orgs OrgUser[] + } + + model OrgUser { + id String @id @default(cuid()) + user User @relation(fields: [userId], references: [id]) + userId String + org Organization @relation(fields: [orgId], references: [id]) + orgId String + } + + model Organization { + id String @id @default(uuid()) + users OrgUser[] + resources Resource[] @relation("organization") + } + + abstract model BaseAuth { + id String @id @default(uuid()) + organizationId String? + organization Organization? @relation(fields: [organizationId], references: [id], name: "organization") + + @@allow('all', organization.users?[user == auth()]) + } + + model Resource extends BaseAuth { + name String? + type String? + + @@delegate(type) + } + + model Personnel extends Resource { + } + ` + ); + + const db = enhance(); + await expect(db.resource.findMany()).toResolveTruthy(); + }); +}); From 55ae94ef9bd3c3b0aae707c8f64179dca6c85bed Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Mon, 14 Oct 2024 22:37:36 -0700 Subject: [PATCH 2/3] fix broken zod generation --- packages/sdk/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/src/utils.ts b/packages/sdk/src/utils.ts index d6c4c0fd5..8b968d0ca 100644 --- a/packages/sdk/src/utils.ts +++ b/packages/sdk/src/utils.ts @@ -460,7 +460,7 @@ export function isDelegateModel(node: AstNode) { } export function isDiscriminatorField(field: DataModelField) { - const model = field.$inheritedFrom ?? field.$container; + const model = getInheritedFromDelegate(field) ?? field.$container; const delegateAttr = getAttribute(model, '@@delegate'); if (!delegateAttr) { return false; From 686538ec9dca2928cf1f172daf3b348e4ee049fe Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Tue, 15 Oct 2024 00:21:47 -0700 Subject: [PATCH 3/3] new fix --- packages/schema/src/utils/ast-utils.ts | 10 ++++++---- packages/sdk/src/utils.ts | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/schema/src/utils/ast-utils.ts b/packages/schema/src/utils/ast-utils.ts index d4ebead87..e7f24b72a 100644 --- a/packages/schema/src/utils/ast-utils.ts +++ b/packages/schema/src/utils/ast-utils.ts @@ -118,10 +118,12 @@ function cloneAst( clone.$container = newContainer; if (isDataModel(newContainer) && isDataModelField(node)) { - // walk up the hierarchy to find the model where the field is defined (delegate or abstract base) - const allBases = getRecursiveBases(newContainer); - clone.$inheritedFrom = allBases.find((base) => base.fields.some((f) => f.name === node.name)); - } else { + // walk up the hierarchy to find the upper-most delegate ancestor that defines the field + const delegateBases = getRecursiveBases(newContainer).filter(isDelegateModel); + clone.$inheritedFrom = delegateBases.findLast((base) => base.fields.some((f) => f.name === node.name)); + } + + if (!clone.$inheritedFrom) { clone.$inheritedFrom = node.$inheritedFrom ?? getContainerOfType(node, isDataModel); } diff --git a/packages/sdk/src/utils.ts b/packages/sdk/src/utils.ts index 8b968d0ca..d6c4c0fd5 100644 --- a/packages/sdk/src/utils.ts +++ b/packages/sdk/src/utils.ts @@ -460,7 +460,7 @@ export function isDelegateModel(node: AstNode) { } export function isDiscriminatorField(field: DataModelField) { - const model = getInheritedFromDelegate(field) ?? field.$container; + const model = field.$inheritedFrom ?? field.$container; const delegateAttr = getAttribute(model, '@@delegate'); if (!delegateAttr) { return false;