From ff86ce0059ac87f18538da1b01ad9ef52685ff25 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Tue, 31 Dec 2024 22:32:23 +0800 Subject: [PATCH 1/5] fix(delegate): delegate model's guards are not properly including concrete models fixes #1930 --- .../src/plugins/enhancer/enhance/index.ts | 17 +--- .../enhancer/policy/expression-writer.ts | 24 ++--- .../enhancer/policy/policy-guard-generator.ts | 63 +++++++------ .../src/plugins/enhancer/policy/utils.ts | 72 ++++++++++++++- .../src/plugins/prisma/schema-generator.ts | 5 +- packages/schema/src/utils/ast-utils.ts | 28 +++++- .../with-delegate/policy-interaction.test.ts | 78 ++++++++++++++++ tests/regression/tests/issue-1930.test.ts | 91 +++++++++++++++++++ 8 files changed, 319 insertions(+), 59 deletions(-) create mode 100644 tests/regression/tests/issue-1930.test.ts diff --git a/packages/schema/src/plugins/enhancer/enhance/index.ts b/packages/schema/src/plugins/enhancer/enhance/index.ts index ba8c50feb..689ddaf2c 100644 --- a/packages/schema/src/plugins/enhancer/enhance/index.ts +++ b/packages/schema/src/plugins/enhancer/enhance/index.ts @@ -24,7 +24,6 @@ import { isArrayExpr, isDataModel, isGeneratorDecl, - isReferenceExpr, isTypeDef, type Model, } from '@zenstackhq/sdk/ast'; @@ -45,6 +44,7 @@ import { } from 'ts-morph'; import { upperCaseFirst } from 'upper-case-first'; import { name } from '..'; +import { getConcreteModels, getDiscriminatorField } from '../../../utils/ast-utils'; import { execPackage } from '../../../utils/exec-utils'; import { CorePlugins, getPluginCustomOutputFolder } from '../../plugin-utils'; import { trackPrismaSchemaError } from '../../prisma'; @@ -407,9 +407,7 @@ export function enhance(prisma: any, context?: EnhancementContext<${authTypePara this.model.declarations .filter((d): d is DataModel => isDelegateModel(d)) .forEach((dm) => { - const concreteModels = this.model.declarations.filter( - (d): d is DataModel => isDataModel(d) && d.superTypes.some((s) => s.ref === dm) - ); + const concreteModels = getConcreteModels(dm); if (concreteModels.length > 0) { delegateInfo.push([dm, concreteModels]); } @@ -579,7 +577,7 @@ export function enhance(prisma: any, context?: EnhancementContext<${authTypePara const typeName = typeAlias.getName(); const payloadRecord = delegateInfo.find(([delegate]) => `$${delegate.name}Payload` === typeName); if (payloadRecord) { - const discriminatorDecl = this.getDiscriminatorField(payloadRecord[0]); + const discriminatorDecl = getDiscriminatorField(payloadRecord[0]); if (discriminatorDecl) { source = `${payloadRecord[1] .map( @@ -826,15 +824,6 @@ export function enhance(prisma: any, context?: EnhancementContext<${authTypePara .filter((n) => n.getName().startsWith(DELEGATE_AUX_RELATION_PREFIX)); } - private getDiscriminatorField(delegate: DataModel) { - const delegateAttr = getAttribute(delegate, '@@delegate'); - if (!delegateAttr) { - return undefined; - } - const arg = delegateAttr.args[0]?.value; - return isReferenceExpr(arg) ? (arg.target.ref as DataModelField) : undefined; - } - private saveSourceFile(sf: SourceFile) { if (this.options.preserveTsFiles) { saveSourceFile(sf); diff --git a/packages/schema/src/plugins/enhancer/policy/expression-writer.ts b/packages/schema/src/plugins/enhancer/policy/expression-writer.ts index 645e02cd1..0d792bdc1 100644 --- a/packages/schema/src/plugins/enhancer/policy/expression-writer.ts +++ b/packages/schema/src/plugins/enhancer/policy/expression-writer.ts @@ -839,16 +839,18 @@ export class ExpressionWriter { operation = this.options.operationContext; } - this.block(() => { - if (operation === 'postUpdate') { - // 'postUpdate' policies are not delegated to relations, just use constant `false` here - // e.g.: - // @@allow('all', check(author)) should not delegate "postUpdate" to author - this.writer.write(`${fieldRef.target.$refText}: ${FALSE}`); - } else { - const targetGuardFunc = getQueryGuardFunctionName(targetModel, undefined, false, operation); - this.writer.write(`${fieldRef.target.$refText}: ${targetGuardFunc}(context, db)`); - } - }); + this.block(() => + this.writeFieldCondition(fieldRef, () => { + if (operation === 'postUpdate') { + // 'postUpdate' policies are not delegated to relations, just use constant `false` here + // e.g.: + // @@allow('all', check(author)) should not delegate "postUpdate" to author + this.writer.write(FALSE); + } else { + const targetGuardFunc = getQueryGuardFunctionName(targetModel, undefined, false, operation); + this.writer.write(`${targetGuardFunc}(context, db)`); + } + }) + ); } } diff --git a/packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts b/packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts index 8206f797b..80a6e1e78 100644 --- a/packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts +++ b/packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts @@ -25,6 +25,7 @@ import { hasAttribute, hasValidationAttributes, isAuthInvocation, + isDelegateModel, isForeignKeyField, saveSourceFile, } from '@zenstackhq/sdk'; @@ -454,36 +455,44 @@ export class PolicyGenerator { writer: CodeBlockWriter, sourceFile: SourceFile ) { - if (kind === 'update' && allows.length === 0) { - // no allow rule for 'update', policy is constant based on if there's - // post-update counterpart - let func: FunctionDeclaration; - if (getPolicyExpressions(model, 'allow', 'postUpdate').length === 0) { - func = generateConstantQueryGuardFunction(sourceFile, model, kind, false); - } else { - func = generateConstantQueryGuardFunction(sourceFile, model, kind, true); + const isDelegate = isDelegateModel(model); + + if (!isDelegate) { + // handle cases where a constant function can be used + // note that this doesn't apply to delegate models because + // all concrete models inheriting it need to be considered + + if (kind === 'update' && allows.length === 0) { + // no allow rule for 'update', policy is constant based on if there's + // post-update counterpart + let func: FunctionDeclaration; + if (getPolicyExpressions(model, 'allow', 'postUpdate').length === 0) { + func = generateConstantQueryGuardFunction(sourceFile, model, kind, false); + } else { + func = generateConstantQueryGuardFunction(sourceFile, model, kind, true); + } + writer.write(`guard: ${func.getName()!},`); + return; } - writer.write(`guard: ${func.getName()!},`); - return; - } - if (kind === 'postUpdate' && allows.length === 0 && denies.length === 0) { - // no 'postUpdate' rule, always allow - const func = generateConstantQueryGuardFunction(sourceFile, model, kind, true); - writer.write(`guard: ${func.getName()},`); - return; - } + if (kind === 'postUpdate' && allows.length === 0 && denies.length === 0) { + // no 'postUpdate' rule, always allow + const func = generateConstantQueryGuardFunction(sourceFile, model, kind, true); + writer.write(`guard: ${func.getName()},`); + return; + } - if (kind in policies && typeof policies[kind as keyof typeof policies] === 'boolean') { - // constant policy - const func = generateConstantQueryGuardFunction( - sourceFile, - model, - kind, - policies[kind as keyof typeof policies] as boolean - ); - writer.write(`guard: ${func.getName()!},`); - return; + if (kind in policies && typeof policies[kind as keyof typeof policies] === 'boolean') { + // constant policy + const func = generateConstantQueryGuardFunction( + sourceFile, + model, + kind, + policies[kind as keyof typeof policies] as boolean + ); + writer.write(`guard: ${func.getName()!},`); + return; + } } // generate a policy function that evaluates a partial prisma query diff --git a/packages/schema/src/plugins/enhancer/policy/utils.ts b/packages/schema/src/plugins/enhancer/policy/utils.ts index fee0cc15a..79d67f3f5 100644 --- a/packages/schema/src/plugins/enhancer/policy/utils.ts +++ b/packages/schema/src/plugins/enhancer/policy/utils.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import type { PolicyKind, PolicyOperationKind } from '@zenstackhq/runtime'; +import { DELEGATE_AUX_RELATION_PREFIX, type PolicyKind, type PolicyOperationKind } from '@zenstackhq/runtime'; import { ExpressionContext, PluginError, @@ -15,6 +15,7 @@ import { getQueryGuardFunctionName, isAuthInvocation, isDataModelFieldReference, + isDelegateModel, isEnumFieldReference, isFromStdlib, isFutureExpr, @@ -39,9 +40,16 @@ import { } from '@zenstackhq/sdk/ast'; import deepmerge from 'deepmerge'; import { getContainerOfType, streamAllContents, streamAst, streamContents } from 'langium'; +import { lowerCaseFirst } from 'lower-case-first'; import { SourceFile, WriterFunction } from 'ts-morph'; import { name } from '..'; -import { isCheckInvocation, isCollectionPredicate, isFutureInvocation } from '../../../utils/ast-utils'; +import { + getConcreteModels, + getDiscriminatorField, + isCheckInvocation, + isCollectionPredicate, + isFutureInvocation, +} from '../../../utils/ast-utils'; import { ExpressionWriter, FALSE, TRUE } from './expression-writer'; /** @@ -303,8 +311,11 @@ export function generateQueryGuardFunction( forField?: DataModelField, fieldOverride = false ) { - const statements: (string | WriterFunction)[] = []; + if (isDelegateModel(model) && !forField) { + return generateDelegateQueryGuardFunction(sourceFile, model, kind); + } + const statements: (string | WriterFunction)[] = []; const allowRules = allows.filter((rule) => !hasCrossModelComparison(rule)); const denyRules = denies.filter((rule) => !hasCrossModelComparison(rule)); @@ -438,6 +449,61 @@ export function generateQueryGuardFunction( return func; } +function generateDelegateQueryGuardFunction(sourceFile: SourceFile, model: DataModel, kind: PolicyOperationKind) { + const concreteModels = getConcreteModels(model); + + const discriminator = getDiscriminatorField(model); + if (!discriminator) { + throw new PluginError(name, `Model '${model.name}' does not have a discriminator field`); + } + + const func = sourceFile.addFunction({ + name: getQueryGuardFunctionName(model, undefined, false, kind), + returnType: 'any', + parameters: [ + { + name: 'context', + type: 'QueryContext', + }, + { + // for generating field references used by field comparison in the same model + name: 'db', + type: 'CrudContract', + }, + ], + statements: (writer) => { + writer.write('return '); + if (concreteModels.length === 0) { + writer.write(TRUE); + } else { + writer.block(() => { + // union all concrete model's guards + writer.writeLine('OR: ['); + concreteModels.forEach((concrete) => { + writer.block(() => { + writer.write('AND: ['); + // discriminator condition + writer.write(`{ ${discriminator.name}: '${concrete.name}' },`); + // concrete model guard + writer.write( + `{ ${DELEGATE_AUX_RELATION_PREFIX}_${lowerCaseFirst( + concrete.name + )}: ${getQueryGuardFunctionName(concrete, undefined, false, kind)}(context, db) }` + ); + writer.writeLine(']'); + }); + writer.write(','); + }); + writer.writeLine(']'); + }); + } + writer.write(';'); + }, + }); + + return func; +} + export function generateEntityCheckerFunction( sourceFile: SourceFile, model: DataModel, diff --git a/packages/schema/src/plugins/prisma/schema-generator.ts b/packages/schema/src/plugins/prisma/schema-generator.ts index 96a3b15f5..a0bde1769 100644 --- a/packages/schema/src/plugins/prisma/schema-generator.ts +++ b/packages/schema/src/plugins/prisma/schema-generator.ts @@ -57,6 +57,7 @@ import path from 'path'; import semver from 'semver'; import { name } from '.'; import { getStringLiteral } from '../../language-server/validator/utils'; +import { getConcreteModels } from '../../utils/ast-utils'; import { execPackage } from '../../utils/exec-utils'; import { isDefaultWithAuth } from '../enhancer/enhancer-utils'; import { @@ -320,9 +321,7 @@ export class PrismaSchemaGenerator { } // collect concrete models inheriting this model - const concreteModels = decl.$container.declarations.filter( - (d) => isDataModel(d) && d !== decl && d.superTypes.some((base) => base.ref === decl) - ); + const concreteModels = getConcreteModels(decl); // generate an optional relation field in delegate base model to each concrete model concreteModels.forEach((concrete) => { diff --git a/packages/schema/src/utils/ast-utils.ts b/packages/schema/src/utils/ast-utils.ts index a6fab7ea5..206aa2300 100644 --- a/packages/schema/src/utils/ast-utils.ts +++ b/packages/schema/src/utils/ast-utils.ts @@ -2,6 +2,7 @@ import { BinaryExpr, DataModel, DataModelAttribute, + DataModelField, Expression, InheritableNode, isBinaryExpr, @@ -9,12 +10,13 @@ import { isDataModelField, isInvocationExpr, isModel, + isReferenceExpr, isTypeDef, Model, ModelImport, TypeDef, } from '@zenstackhq/language/ast'; -import { getInheritanceChain, getRecursiveBases, isDelegateModel, isFromStdlib } from '@zenstackhq/sdk'; +import { getAttribute, getInheritanceChain, getRecursiveBases, isDelegateModel, isFromStdlib } from '@zenstackhq/sdk'; import { AstNode, copyAstNode, @@ -310,3 +312,27 @@ export function findUpInheritance(start: DataModel, target: DataModel): DataMode } return undefined; } + +/** + * Gets all concrete models that inherit from the given delegate model + */ +export function getConcreteModels(dataModel: DataModel): DataModel[] { + if (!isDelegateModel(dataModel)) { + return []; + } + return dataModel.$container.declarations.filter( + (d): d is DataModel => isDataModel(d) && d !== dataModel && d.superTypes.some((base) => base.ref === dataModel) + ); +} + +/** + * Gets the discriminator field for the given delegate model + */ +export function getDiscriminatorField(delegate: DataModel) { + const delegateAttr = getAttribute(delegate, '@@delegate'); + if (!delegateAttr) { + return undefined; + } + const arg = delegateAttr.args[0]?.value; + return isReferenceExpr(arg) ? (arg.target.ref as DataModelField) : undefined; +} 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 d149a6392..c52b021b5 100644 --- a/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts +++ b/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts @@ -571,4 +571,82 @@ describe('Polymorphic Policy Test', () => { expect(foundPost2.foo).toBeUndefined(); expect(foundPost2.bar).toBeUndefined(); }); + + it('respects concrete policies when read as base optional relation', async () => { + const { enhance } = await loadSchema( + ` + model User { + id Int @id @default(autoincrement()) + asset Asset? + @@allow('all', true) + } + + model Asset { + id Int @id @default(autoincrement()) + user User @relation(fields: [userId], references: [id]) + userId Int @unique + type String + + @@delegate(type) + } + + model Post extends Asset { + title String + private Boolean + @@allow('create', true) + @@allow('read', !private) + } + ` + ); + + const fullDb = enhance(undefined, { kinds: ['delegate'] }); + await fullDb.user.create({ data: { id: 1 } }); + await fullDb.post.create({ data: { title: 'Post1', private: true, user: { connect: { id: 1 } } } }); + await expect(fullDb.user.findUnique({ where: { id: 1 }, include: { asset: true } })).resolves.toMatchObject({ + asset: expect.objectContaining({ type: 'Post' }), + }); + + const db = enhance(); + await expect(db.user.findUnique({ where: { id: 1 }, include: { asset: true } })).resolves.toMatchObject({ + asset: null, + }); + }); + + it('respects concrete policies when read as base required relation', async () => { + const { enhance } = await loadSchema( + ` + model User { + id Int @id @default(autoincrement()) + asset Asset @relation(fields: [assetId], references: [id]) + assetId Int @unique + @@allow('all', true) + } + + model Asset { + id Int @id @default(autoincrement()) + user User? + type String + + @@delegate(type) + @@allow('all', true) + } + + model Post extends Asset { + title String + private Boolean + @@deny('read', private) + } + `, + { logPrismaQuery: true } + ); + + const fullDb = enhance(undefined, { kinds: ['delegate'] }); + await fullDb.post.create({ data: { id: 1, title: 'Post1', private: true, user: { create: { id: 1 } } } }); + await expect(fullDb.user.findUnique({ where: { id: 1 }, include: { asset: true } })).resolves.toMatchObject({ + asset: expect.objectContaining({ type: 'Post' }), + }); + + const db = enhance(); + await expect(db.user.findUnique({ where: { id: 1 }, include: { asset: true } })).toResolveNull(); + }); }); diff --git a/tests/regression/tests/issue-1930.test.ts b/tests/regression/tests/issue-1930.test.ts new file mode 100644 index 000000000..95d3a0862 --- /dev/null +++ b/tests/regression/tests/issue-1930.test.ts @@ -0,0 +1,91 @@ +import { loadSchema } from '@zenstackhq/testtools'; + +describe('issue 1930', () => { + it('regression', async () => { + const { enhance } = await loadSchema( + ` +model Organization { + id String @id @default(cuid()) + entities Entity[] + + @@allow('all', true) +} + +model Entity { + id String @id @default(cuid()) + org Organization? @relation(fields: [orgId], references: [id]) + orgId String? + contents EntityContent[] + entityType String + isDeleted Boolean @default(false) + + @@delegate(entityType) + + @@allow('all', !isDeleted) +} + +model EntityContent { + id String @id @default(cuid()) + entity Entity @relation(fields: [entityId], references: [id]) + entityId String + + entityContentType String + + @@delegate(entityContentType) + + @@allow('create', true) + @@allow('read', check(entity)) +} + +model Article extends Entity { + private Boolean @default(false) + @@deny('all', private) +} + +model ArticleContent extends EntityContent { + body String? +} + +model OtherContent extends EntityContent { + data Int +} + ` + ); + + const fullDb = enhance(undefined, { kinds: ['delegate'] }); + const org = await fullDb.organization.create({ data: {} }); + const article = await fullDb.article.create({ + data: { org: { connect: { id: org.id } } }, + }); + + const db = enhance(); + + // normal create/read + await expect( + db.articleContent.create({ + data: { body: 'abc', entity: { connect: { id: article.id } } }, + }) + ).toResolveTruthy(); + await expect(db.article.findFirst({ include: { contents: true } })).resolves.toMatchObject({ + contents: expect.arrayContaining([expect.objectContaining({ body: 'abc' })]), + }); + + // deleted article's contents are not readable + const deletedArticle = await fullDb.article.create({ + data: { org: { connect: { id: org.id } }, isDeleted: true }, + }); + const content1 = await fullDb.articleContent.create({ + data: { body: 'bcd', entity: { connect: { id: deletedArticle.id } } }, + }); + await expect(db.articleContent.findUnique({ where: { id: content1.id } })).toResolveNull(); + + // private article's contents are not readable + const privateArticle = await fullDb.article.create({ + data: { org: { connect: { id: org.id } }, private: true }, + }); + const content2 = await fullDb.articleContent.create({ + data: { body: 'cde', entity: { connect: { id: privateArticle.id } } }, + }); + await expect(db.articleContent.findUnique({ where: { id: content2.id } })).toResolveNull(); + }); +}); From d6618c9c8a147d2ff2105a106772dd48dd44f186 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Thu, 2 Jan 2025 16:02:39 +0800 Subject: [PATCH 2/5] more fixes --- .../enhancer/policy/policy-guard-generator.ts | 65 ++++++++--------- .../src/plugins/enhancer/policy/utils.ts | 71 +------------------ .../with-delegate/policy-interaction.test.ts | 24 ++++--- tests/regression/tests/issue-1930.test.ts | 11 --- 4 files changed, 46 insertions(+), 125 deletions(-) diff --git a/packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts b/packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts index 80a6e1e78..9ffe41dcb 100644 --- a/packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts +++ b/packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts @@ -25,7 +25,6 @@ import { hasAttribute, hasValidationAttributes, isAuthInvocation, - isDelegateModel, isForeignKeyField, saveSourceFile, } from '@zenstackhq/sdk'; @@ -455,44 +454,38 @@ export class PolicyGenerator { writer: CodeBlockWriter, sourceFile: SourceFile ) { - const isDelegate = isDelegateModel(model); - - if (!isDelegate) { - // handle cases where a constant function can be used - // note that this doesn't apply to delegate models because - // all concrete models inheriting it need to be considered - - if (kind === 'update' && allows.length === 0) { - // no allow rule for 'update', policy is constant based on if there's - // post-update counterpart - let func: FunctionDeclaration; - if (getPolicyExpressions(model, 'allow', 'postUpdate').length === 0) { - func = generateConstantQueryGuardFunction(sourceFile, model, kind, false); - } else { - func = generateConstantQueryGuardFunction(sourceFile, model, kind, true); - } - writer.write(`guard: ${func.getName()!},`); - return; - } + // first handle several cases where a constant function can be used - if (kind === 'postUpdate' && allows.length === 0 && denies.length === 0) { - // no 'postUpdate' rule, always allow - const func = generateConstantQueryGuardFunction(sourceFile, model, kind, true); - writer.write(`guard: ${func.getName()},`); - return; + if (kind === 'update' && allows.length === 0) { + // no allow rule for 'update', policy is constant based on if there's + // post-update counterpart + let func: FunctionDeclaration; + if (getPolicyExpressions(model, 'allow', 'postUpdate').length === 0) { + func = generateConstantQueryGuardFunction(sourceFile, model, kind, false); + } else { + func = generateConstantQueryGuardFunction(sourceFile, model, kind, true); } + writer.write(`guard: ${func.getName()!},`); + return; + } - if (kind in policies && typeof policies[kind as keyof typeof policies] === 'boolean') { - // constant policy - const func = generateConstantQueryGuardFunction( - sourceFile, - model, - kind, - policies[kind as keyof typeof policies] as boolean - ); - writer.write(`guard: ${func.getName()!},`); - return; - } + if (kind === 'postUpdate' && allows.length === 0 && denies.length === 0) { + // no 'postUpdate' rule, always allow + const func = generateConstantQueryGuardFunction(sourceFile, model, kind, true); + writer.write(`guard: ${func.getName()},`); + return; + } + + if (kind in policies && typeof policies[kind as keyof typeof policies] === 'boolean') { + // constant policy + const func = generateConstantQueryGuardFunction( + sourceFile, + model, + kind, + policies[kind as keyof typeof policies] as boolean + ); + writer.write(`guard: ${func.getName()!},`); + return; } // generate a policy function that evaluates a partial prisma query diff --git a/packages/schema/src/plugins/enhancer/policy/utils.ts b/packages/schema/src/plugins/enhancer/policy/utils.ts index 79d67f3f5..4dd5e7050 100644 --- a/packages/schema/src/plugins/enhancer/policy/utils.ts +++ b/packages/schema/src/plugins/enhancer/policy/utils.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { DELEGATE_AUX_RELATION_PREFIX, type PolicyKind, type PolicyOperationKind } from '@zenstackhq/runtime'; +import { type PolicyKind, type PolicyOperationKind } from '@zenstackhq/runtime'; import { ExpressionContext, PluginError, @@ -15,7 +15,6 @@ import { getQueryGuardFunctionName, isAuthInvocation, isDataModelFieldReference, - isDelegateModel, isEnumFieldReference, isFromStdlib, isFutureExpr, @@ -40,16 +39,9 @@ import { } from '@zenstackhq/sdk/ast'; import deepmerge from 'deepmerge'; import { getContainerOfType, streamAllContents, streamAst, streamContents } from 'langium'; -import { lowerCaseFirst } from 'lower-case-first'; import { SourceFile, WriterFunction } from 'ts-morph'; import { name } from '..'; -import { - getConcreteModels, - getDiscriminatorField, - isCheckInvocation, - isCollectionPredicate, - isFutureInvocation, -} from '../../../utils/ast-utils'; +import { isCheckInvocation, isCollectionPredicate, isFutureInvocation } from '../../../utils/ast-utils'; import { ExpressionWriter, FALSE, TRUE } from './expression-writer'; /** @@ -311,10 +303,6 @@ export function generateQueryGuardFunction( forField?: DataModelField, fieldOverride = false ) { - if (isDelegateModel(model) && !forField) { - return generateDelegateQueryGuardFunction(sourceFile, model, kind); - } - const statements: (string | WriterFunction)[] = []; const allowRules = allows.filter((rule) => !hasCrossModelComparison(rule)); const denyRules = denies.filter((rule) => !hasCrossModelComparison(rule)); @@ -449,61 +437,6 @@ export function generateQueryGuardFunction( return func; } -function generateDelegateQueryGuardFunction(sourceFile: SourceFile, model: DataModel, kind: PolicyOperationKind) { - const concreteModels = getConcreteModels(model); - - const discriminator = getDiscriminatorField(model); - if (!discriminator) { - throw new PluginError(name, `Model '${model.name}' does not have a discriminator field`); - } - - const func = sourceFile.addFunction({ - name: getQueryGuardFunctionName(model, undefined, false, kind), - returnType: 'any', - parameters: [ - { - name: 'context', - type: 'QueryContext', - }, - { - // for generating field references used by field comparison in the same model - name: 'db', - type: 'CrudContract', - }, - ], - statements: (writer) => { - writer.write('return '); - if (concreteModels.length === 0) { - writer.write(TRUE); - } else { - writer.block(() => { - // union all concrete model's guards - writer.writeLine('OR: ['); - concreteModels.forEach((concrete) => { - writer.block(() => { - writer.write('AND: ['); - // discriminator condition - writer.write(`{ ${discriminator.name}: '${concrete.name}' },`); - // concrete model guard - writer.write( - `{ ${DELEGATE_AUX_RELATION_PREFIX}_${lowerCaseFirst( - concrete.name - )}: ${getQueryGuardFunctionName(concrete, undefined, false, kind)}(context, db) }` - ); - writer.writeLine(']'); - }); - writer.write(','); - }); - writer.writeLine(']'); - }); - } - writer.write(';'); - }, - }); - - return func; -} - export function generateEntityCheckerFunction( sourceFile: SourceFile, model: DataModel, 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 c52b021b5..d9f71c38b 100644 --- a/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts +++ b/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts @@ -87,12 +87,16 @@ describe('Polymorphic Policy Test', () => { `; for (const schema of [booleanCondition, booleanExpression]) { - const { enhanceRaw: enhance, prisma } = await loadSchema(schema); + const { enhanceRaw: enhance, prisma } = await loadSchema(schema, { logPrismaQuery: true }); const fullDb = enhance(prisma, undefined, { kinds: ['delegate'] }); const user = await fullDb.user.create({ data: { id: 1 } }); - const userDb = enhance(prisma, { user: { id: user.id } }, { kinds: ['delegate', 'policy'] }); + const userDb = enhance( + prisma, + { user: { id: user.id } }, + { kinds: ['delegate', 'policy'], logPrismaQuery: true } + ); // violating Asset create await expect( @@ -588,13 +592,14 @@ describe('Polymorphic Policy Test', () => { type String @@delegate(type) + @@allow('all', true) } model Post extends Asset { title String private Boolean @@allow('create', true) - @@allow('read', !private) + @@deny('read', private) } ` ); @@ -607,9 +612,9 @@ describe('Polymorphic Policy Test', () => { }); const db = enhance(); - await expect(db.user.findUnique({ where: { id: 1 }, include: { asset: true } })).resolves.toMatchObject({ - asset: null, - }); + const read = await db.user.findUnique({ where: { id: 1 }, include: { asset: true } }); + expect(read.asset).toBeTruthy(); + expect(read.asset.title).toBeUndefined(); }); it('respects concrete policies when read as base required relation', async () => { @@ -636,8 +641,7 @@ describe('Polymorphic Policy Test', () => { private Boolean @@deny('read', private) } - `, - { logPrismaQuery: true } + ` ); const fullDb = enhance(undefined, { kinds: ['delegate'] }); @@ -647,6 +651,8 @@ describe('Polymorphic Policy Test', () => { }); const db = enhance(); - await expect(db.user.findUnique({ where: { id: 1 }, include: { asset: true } })).toResolveNull(); + const read = await db.user.findUnique({ where: { id: 1 }, include: { asset: true } }); + expect(read).toBeTruthy(); + expect(read.asset.title).toBeUndefined(); }); }); diff --git a/tests/regression/tests/issue-1930.test.ts b/tests/regression/tests/issue-1930.test.ts index 95d3a0862..762369321 100644 --- a/tests/regression/tests/issue-1930.test.ts +++ b/tests/regression/tests/issue-1930.test.ts @@ -38,8 +38,6 @@ model EntityContent { } model Article extends Entity { - private Boolean @default(false) - @@deny('all', private) } model ArticleContent extends EntityContent { @@ -78,14 +76,5 @@ model OtherContent extends EntityContent { data: { body: 'bcd', entity: { connect: { id: deletedArticle.id } } }, }); await expect(db.articleContent.findUnique({ where: { id: content1.id } })).toResolveNull(); - - // private article's contents are not readable - const privateArticle = await fullDb.article.create({ - data: { org: { connect: { id: org.id } }, private: true }, - }); - const content2 = await fullDb.articleContent.create({ - data: { body: 'cde', entity: { connect: { id: privateArticle.id } } }, - }); - await expect(db.articleContent.findUnique({ where: { id: content2.id } })).toResolveNull(); }); }); From 8a6127ebc5ddd288e59a3969afe6681f03240378 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Thu, 2 Jan 2025 16:08:01 +0800 Subject: [PATCH 3/5] update --- packages/schema/src/plugins/enhancer/policy/utils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/schema/src/plugins/enhancer/policy/utils.ts b/packages/schema/src/plugins/enhancer/policy/utils.ts index 4dd5e7050..fee0cc15a 100644 --- a/packages/schema/src/plugins/enhancer/policy/utils.ts +++ b/packages/schema/src/plugins/enhancer/policy/utils.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { type PolicyKind, type PolicyOperationKind } from '@zenstackhq/runtime'; +import type { PolicyKind, PolicyOperationKind } from '@zenstackhq/runtime'; import { ExpressionContext, PluginError, @@ -304,6 +304,7 @@ export function generateQueryGuardFunction( fieldOverride = false ) { const statements: (string | WriterFunction)[] = []; + const allowRules = allows.filter((rule) => !hasCrossModelComparison(rule)); const denyRules = denies.filter((rule) => !hasCrossModelComparison(rule)); From c0a457947863f7df55f17d0ed6fcff1a9e580be8 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Thu, 2 Jan 2025 16:12:06 +0800 Subject: [PATCH 4/5] update --- packages/schema/src/utils/ast-utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/schema/src/utils/ast-utils.ts b/packages/schema/src/utils/ast-utils.ts index 206aa2300..0e462547f 100644 --- a/packages/schema/src/utils/ast-utils.ts +++ b/packages/schema/src/utils/ast-utils.ts @@ -328,8 +328,8 @@ export function getConcreteModels(dataModel: DataModel): DataModel[] { /** * Gets the discriminator field for the given delegate model */ -export function getDiscriminatorField(delegate: DataModel) { - const delegateAttr = getAttribute(delegate, '@@delegate'); +export function getDiscriminatorField(dataModel: DataModel) { + const delegateAttr = getAttribute(dataModel, '@@delegate'); if (!delegateAttr) { return undefined; } From bbd7cb50e0962b3440d4453a627745eaa326d015 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Thu, 2 Jan 2025 16:12:38 +0800 Subject: [PATCH 5/5] update --- .../enhancements/with-delegate/policy-interaction.test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) 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 d9f71c38b..67fc456af 100644 --- a/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts +++ b/tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts @@ -87,16 +87,12 @@ describe('Polymorphic Policy Test', () => { `; for (const schema of [booleanCondition, booleanExpression]) { - const { enhanceRaw: enhance, prisma } = await loadSchema(schema, { logPrismaQuery: true }); + const { enhanceRaw: enhance, prisma } = await loadSchema(schema); const fullDb = enhance(prisma, undefined, { kinds: ['delegate'] }); const user = await fullDb.user.create({ data: { id: 1 } }); - const userDb = enhance( - prisma, - { user: { id: user.id } }, - { kinds: ['delegate', 'policy'], logPrismaQuery: true } - ); + const userDb = enhance(prisma, { user: { id: user.id } }, { kinds: ['delegate', 'policy'] }); // violating Asset create await expect(