Skip to content

fix(delegate): policies inherited from delegate base models are not injected into proper hierarchy #1776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions packages/runtime/src/enhancements/node/delegate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 11 additions & 1 deletion packages/schema/src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,17 @@ function cloneAst<T extends InheritableNode>(
): Mutable<T> {
const clone = copyAstNode(node, buildReference) as Mutable<T>;
clone.$container = newContainer;
clone.$inheritedFrom = node.$inheritedFrom ?? getContainerOfType(node, isDataModel);

if (isDataModel(newContainer) && isDataModelField(node)) {
// 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);
}

return clone;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
49 changes: 49 additions & 0 deletions tests/regression/tests/issue-1770.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
Loading