Skip to content

Commit

Permalink
fix: handle foreign key field-level access check during relation upda…
Browse files Browse the repository at this point in the history
  • Loading branch information
ymc9 authored Nov 24, 2023
1 parent 03315cc commit 3c8cba7
Show file tree
Hide file tree
Showing 6 changed files with 349 additions and 20 deletions.
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

0 comments on commit 3c8cba7

Please sign in to comment.