Skip to content

fix: handle foreign key field-level access check during relation update #847

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 1 commit into from
Nov 24, 2023
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
14 changes: 12 additions & 2 deletions packages/runtime/src/enhancements/policy/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -680,8 +680,18 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
if (context.field?.backLink) {
const backLinkField = this.utils.getModelField(model, context.field.backLink);
if (backLinkField.isRelationOwner) {
// update happens on the related model, require updatable
await this.utils.checkPolicyForUnique(model, args, 'update', db, args);
// update happens on the related model, require updatable,
// translate args to foreign keys so field-level policies can be checked
const checkArgs: any = {};
if (args && typeof args === 'object' && backLinkField.foreignKeyMapping) {
for (const key of Object.keys(args)) {
const fk = backLinkField.foreignKeyMapping[key];
if (fk) {
checkArgs[fk] = args[key];
}
}
}
await this.utils.checkPolicyForUnique(model, args, 'update', db, checkArgs);

// register post-update check
await _registerPostUpdateCheck(model, args);
Expand Down
26 changes: 22 additions & 4 deletions packages/runtime/src/enhancements/policy/policy-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -763,11 +763,29 @@ export class PolicyUtil {
if (typeof v === 'undefined') {
continue;
}
const fieldGuard = this.getFieldUpdateAuthGuard(db, model, k);
if (this.isFalse(fieldGuard)) {
return { guard: allFieldGuards, rejectedByField: k };

const field = resolveField(this.modelMeta, model, k);

if (field?.isDataModel) {
// relation field update should be treated as foreign key update,
// fetch and merge all foreign key guards
if (field.isRelationOwner && field.foreignKeyMapping) {
const foreignKeys = Object.values<string>(field.foreignKeyMapping);
for (const fk of foreignKeys) {
const fieldGuard = this.getFieldUpdateAuthGuard(db, model, fk);
if (this.isFalse(fieldGuard)) {
return { guard: allFieldGuards, rejectedByField: fk };
}
allFieldGuards.push(fieldGuard);
}
}
} else {
const fieldGuard = this.getFieldUpdateAuthGuard(db, model, k);
if (this.isFalse(fieldGuard)) {
return { guard: allFieldGuards, rejectedByField: k };
}
allFieldGuards.push(fieldGuard);
}
allFieldGuards.push(fieldGuard);
}
return { guard: this.and(...allFieldGuards), rejectedByField: undefined };
}
Expand Down
18 changes: 9 additions & 9 deletions packages/runtime/src/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ export function getVersion() {
* "prisma".
*/
export function getPrismaVersion(): string | undefined {
if (process.env.ZENSTACK_TEST === '1') {
// test environment
try {
return require(path.resolve('./node_modules/@prisma/client/package.json')).version;
} catch {
return undefined;
}
}

try {
// eslint-disable-next-line @typescript-eslint/no-var-requires
return require('@prisma/client/package.json').version;
Expand All @@ -27,15 +36,6 @@ export function getPrismaVersion(): string | undefined {
// eslint-disable-next-line @typescript-eslint/no-var-requires
return require('prisma/package.json').version;
} catch {
if (process.env.ZENSTACK_TEST === '1') {
// test environment
try {
return require(path.resolve('./node_modules/@prisma/client/package.json')).version;
} catch {
return undefined;
}
}

return undefined;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
isEnum,
isReferenceExpr,
} from '@zenstackhq/language/ast';
import { isFutureExpr, resolved } from '@zenstackhq/sdk';
import { isFutureExpr, isRelationshipField, resolved } from '@zenstackhq/sdk';
import { ValidationAcceptor, streamAst } from 'langium';
import pluralize from 'pluralize';
import { AstValidator } from '../types';
Expand Down Expand Up @@ -131,12 +131,24 @@ export default class AttributeApplicationValidator implements AstValidator<Attri
accept('error', `expects a string literal`, { node: attr.args[0] });
return;
}
this.validatePolicyKinds(kind, ['read', 'update', 'all'], attr, accept);
const kindItems = this.validatePolicyKinds(kind, ['read', 'update', 'all'], attr, accept);

const expr = attr.args[1].value;
if (streamAst(expr).some((node) => isFutureExpr(node))) {
accept('error', `"future()" is not allowed in field-level policy rules`, { node: expr });
}

// 'update' rules are not allowed for relation fields
if (kindItems.includes('update') || kindItems.includes('all')) {
const field = attr.$container as DataModelField;
if (isRelationshipField(field)) {
accept(
'error',
`Field-level policy rules with "update" or "all" kind are not allowed for relation fields. Put rules on foreign-key fields instead.`,
{ node: attr }
);
}
}
}

private validatePolicyKinds(
Expand All @@ -155,6 +167,7 @@ export default class AttributeApplicationValidator implements AstValidator<Attri
);
}
});
return items;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1186,5 +1186,43 @@ describe('Attribute tests', () => {
}
`)
).toContain('"future()" is not allowed in field-level policy rules');

expect(
await loadModelWithError(`
${prelude}
model M {
id String @id
n N @allow('update', n.x > 0)
}

model N {
id String @id
x Int
m M? @relation(fields: [mId], references: [id])
mId String
}
`)
).toContain(
'Field-level policy rules with "update" or "all" kind are not allowed for relation fields. Put rules on foreign-key fields instead.'
);

expect(
await loadModelWithError(`
${prelude}
model M {
id String @id
n N[] @allow('update', n.x > 0)
}

model N {
id String @id
x Int
m M? @relation(fields: [mId], references: [id])
mId String
}
`)
).toContain(
'Field-level policy rules with "update" or "all" kind are not allowed for relation fields. Put rules on foreign-key fields instead.'
);
});
});
Loading