Skip to content

Commit

Permalink
fix: post-update rule for id field is not effective if id is updated (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ymc9 authored Apr 11, 2024
1 parent e3fb73a commit 5fe85ff
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ export default function createRouter<Config extends BaseConfig>(
.input($Schema.PostInputSchema.aggregate)
.query(({ ctx, input }) => checkRead(db(ctx).post.aggregate(input as any))),

createMany: procedure
.input($Schema.PostInputSchema.createMany)
.mutation(async ({ ctx, input }) => checkMutate(db(ctx).post.createMany(input as any))),

create: procedure
.input($Schema.PostInputSchema.create)
.mutation(async ({ ctx, input }) => checkMutate(db(ctx).post.create(input as any))),
Expand Down Expand Up @@ -88,6 +92,29 @@ export interface ClientType<AppRouter extends AnyRouter, Context = AppRouter['_d
opts?: UseTRPCInfiniteQueryOptions<string, T, Prisma.GetPostAggregateType<T>, Error>,
) => UseTRPCInfiniteQueryResult<Prisma.GetPostAggregateType<T>, TRPCClientErrorLike<AppRouter>>;
};
createMany: {
useMutation: <T extends Prisma.PostCreateManyArgs>(
opts?: UseTRPCMutationOptions<
Prisma.PostCreateManyArgs,
TRPCClientErrorLike<AppRouter>,
Prisma.BatchPayload,
Context
>,
) => Omit<
UseTRPCMutationResult<
Prisma.BatchPayload,
TRPCClientErrorLike<AppRouter>,
Prisma.SelectSubset<T, Prisma.PostCreateManyArgs>,
Context
>,
'mutateAsync'
> & {
mutateAsync: <T extends Prisma.PostCreateManyArgs>(
variables: T,
opts?: UseTRPCMutationOptions<T, TRPCClientErrorLike<AppRouter>, Prisma.BatchPayload, Context>,
) => Promise<Prisma.BatchPayload>;
};
};
create: {
useMutation: <T extends Prisma.PostCreateArgs>(
opts?: UseTRPCMutationOptions<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ export default function createRouter<Config extends BaseConfig>(
.input($Schema.UserInputSchema.aggregate)
.query(({ ctx, input }) => checkRead(db(ctx).user.aggregate(input as any))),

createMany: procedure
.input($Schema.UserInputSchema.createMany)
.mutation(async ({ ctx, input }) => checkMutate(db(ctx).user.createMany(input as any))),

create: procedure
.input($Schema.UserInputSchema.create)
.mutation(async ({ ctx, input }) => checkMutate(db(ctx).user.create(input as any))),
Expand Down Expand Up @@ -88,6 +92,29 @@ export interface ClientType<AppRouter extends AnyRouter, Context = AppRouter['_d
opts?: UseTRPCInfiniteQueryOptions<string, T, Prisma.GetUserAggregateType<T>, Error>,
) => UseTRPCInfiniteQueryResult<Prisma.GetUserAggregateType<T>, TRPCClientErrorLike<AppRouter>>;
};
createMany: {
useMutation: <T extends Prisma.UserCreateManyArgs>(
opts?: UseTRPCMutationOptions<
Prisma.UserCreateManyArgs,
TRPCClientErrorLike<AppRouter>,
Prisma.BatchPayload,
Context
>,
) => Omit<
UseTRPCMutationResult<
Prisma.BatchPayload,
TRPCClientErrorLike<AppRouter>,
Prisma.SelectSubset<T, Prisma.UserCreateManyArgs>,
Context
>,
'mutateAsync'
> & {
mutateAsync: <T extends Prisma.UserCreateManyArgs>(
variables: T,
opts?: UseTRPCMutationOptions<T, TRPCClientErrorLike<AppRouter>, Prisma.BatchPayload, Context>,
) => Promise<Prisma.BatchPayload>;
};
};
create: {
useMutation: <T extends Prisma.UserCreateArgs>(
opts?: UseTRPCMutationOptions<
Expand Down
25 changes: 17 additions & 8 deletions packages/runtime/src/enhancements/policy/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -690,16 +690,25 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
const postWriteChecks: PostWriteCheckRecord[] = [];

// registers a post-update check task
const _registerPostUpdateCheck = async (model: string, uniqueFilter: any) => {
const _registerPostUpdateCheck = async (
model: string,
preUpdateLookupFilter: any,
postUpdateLookupFilter: any
) => {
// both "post-update" rules and Zod schemas require a post-update check
if (this.utils.hasAuthGuard(model, 'postUpdate') || this.utils.getZodSchema(model)) {
// select pre-update field values
let preValue: any;
const preValueSelect = this.utils.getPreValueSelect(model);
if (preValueSelect && Object.keys(preValueSelect).length > 0) {
preValue = await db[model].findFirst({ where: uniqueFilter, select: preValueSelect });
preValue = await db[model].findFirst({ where: preUpdateLookupFilter, select: preValueSelect });
}
postWriteChecks.push({ model, operation: 'postUpdate', uniqueFilter, preValue });
postWriteChecks.push({
model,
operation: 'postUpdate',
uniqueFilter: postUpdateLookupFilter,
preValue,
});
}
};

Expand Down Expand Up @@ -826,7 +835,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
await this.utils.checkPolicyForUnique(model, args, 'update', db, checkArgs);

// register post-update check
await _registerPostUpdateCheck(model, args);
await _registerPostUpdateCheck(model, args, args);
}
}
};
Expand Down Expand Up @@ -873,20 +882,20 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
await this.utils.checkPolicyForUnique(model, uniqueFilter, 'update', db, args);

// handles the case where id fields are updated
const ids = this.utils.clone(existing);
const postUpdateIds = this.utils.clone(existing);
for (const key of Object.keys(existing)) {
const updateValue = (args as any).data ? (args as any).data[key] : (args as any)[key];
if (
typeof updateValue === 'string' ||
typeof updateValue === 'number' ||
typeof updateValue === 'bigint'
) {
ids[key] = updateValue;
postUpdateIds[key] = updateValue;
}
}

// register post-update check
await _registerPostUpdateCheck(model, ids);
await _registerPostUpdateCheck(model, existing, postUpdateIds);
}
},

Expand Down Expand Up @@ -978,7 +987,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
await this.utils.checkPolicyForUnique(model, uniqueFilter, 'update', db, args);

// register post-update check
await _registerPostUpdateCheck(model, uniqueFilter);
await _registerPostUpdateCheck(model, uniqueFilter, uniqueFilter);

// convert upsert to update
const convertedUpdate = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ export class ExpressionWriter {
this.plainExprBuilder = new TypeScriptExpressionTransformer({
context: ExpressionContext.AccessPolicy,
isPostGuard: this.isPostGuard,
// in post-guard context, `this` references pre-update value
thisExprContext: this.isPostGuard ? 'context.preValue' : undefined,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
Enum,
Expression,
Model,
isBinaryExpr,
isDataModel,
isDataModelField,
isEnum,
Expand All @@ -15,7 +14,6 @@ import {
isMemberAccessExpr,
isReferenceExpr,
isThisExpr,
isUnaryExpr,
} from '@zenstackhq/language/ast';
import {
FIELD_LEVEL_OVERRIDE_READ_GUARD_PREFIX,
Expand Down Expand Up @@ -281,30 +279,6 @@ export default class PolicyGenerator {
}
}

private visitPolicyExpression(expr: Expression, postUpdate: boolean): Expression | undefined {
if (isBinaryExpr(expr) && (expr.operator === '&&' || expr.operator === '||')) {
const left = this.visitPolicyExpression(expr.left, postUpdate);
const right = this.visitPolicyExpression(expr.right, postUpdate);
if (!left) return right;
if (!right) return left;
return { ...expr, left, right };
}

if (isUnaryExpr(expr) && expr.operator === '!') {
const operand = this.visitPolicyExpression(expr.operand, postUpdate);
if (!operand) return undefined;
return { ...expr, operand };
}

if (postUpdate && !this.hasFutureReference(expr)) {
return undefined;
} else if (!postUpdate && this.hasFutureReference(expr)) {
return undefined;
}

return expr;
}

private hasFutureReference(expr: Expression) {
for (const node of streamAst(expr)) {
if (isInvocationExpr(node) && node.function.ref?.name === 'future' && isFromStdlib(node.function.ref)) {
Expand Down Expand Up @@ -599,13 +573,19 @@ export default class PolicyGenerator {
// visit a reference or member access expression to build a
// selection path
const visit = (node: Expression): string[] | undefined => {
if (isThisExpr(node)) {
return [];
}

if (isReferenceExpr(node)) {
const target = resolved(node.target);
if (isDataModelField(target)) {
// a field selection, it's a terminal
return [target.name];
}
} else if (isMemberAccessExpr(node)) {
}

if (isMemberAccessExpr(node)) {
if (forAuthContext && isAuthInvocation(node.operand)) {
return [node.member.$refText];
}
Expand All @@ -621,6 +601,7 @@ export default class PolicyGenerator {
return [...inner, node.member.$refText];
}
}

return undefined;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ export class TypeScriptExpressionTransformer {
throw new TypeScriptExpressionTransformerError(`Unresolved MemberAccessExpr`);
}

if (isThisExpr(expr.operand)) {
return expr.member.ref.name;
} else if (isFutureExpr(expr.operand)) {
if (isFutureExpr(expr.operand)) {
if (this.options?.isPostGuard !== true) {
throw new TypeScriptExpressionTransformerError(`future() is only supported in postUpdate rules`);
}
Expand Down
35 changes: 35 additions & 0 deletions tests/integration/tests/regression/issue-1235.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { loadSchema } from '@zenstackhq/testtools';

describe('issue 1235', () => {
it('regression1', async () => {
const { enhance } = await loadSchema(
`
model Post {
id Int @id @default(autoincrement())
@@deny("update", future().id != id)
@@allow("all", true)
}
`
);

const db = enhance();
const post = await db.post.create({ data: {} });
await expect(db.post.update({ data: { id: post.id + 1 }, where: { id: post.id } })).toBeRejectedByPolicy();
});

it('regression2', async () => {
const { enhance } = await loadSchema(
`
model Post {
id Int @id @default(autoincrement())
@@deny("update", future().id != this.id)
@@allow("all", true)
}
`
);

const db = enhance();
const post = await db.post.create({ data: {} });
await expect(db.post.update({ data: { id: post.id + 1 }, where: { id: post.id } })).toBeRejectedByPolicy();
});
});

0 comments on commit 5fe85ff

Please sign in to comment.