Skip to content

Commit 9294f04

Browse files
ymc9ymc9
authored
fix: '@password' attribute doesn't work well with data validation (#1547)
1 parent ba2a113 commit 9294f04

File tree

7 files changed

+183
-67
lines changed

7 files changed

+183
-67
lines changed

‎packages/runtime/src/enhancements/create-enhancement.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,13 @@ export function createEnhancement<DbClient extends object>(
148148
}
149149
}
150150

151+
// password enhancement must be applied prior to policy because it changes then length of the field
152+
// and can break validation rules like `@length`
153+
if (hasPassword && kinds.includes('password')) {
154+
// @password proxy
155+
result = withPassword(result, options);
156+
}
157+
151158
// 'policy' and 'validation' enhancements are both enabled by `withPolicy`
152159
if (kinds.includes('policy') || kinds.includes('validation')) {
153160
result = withPolicy(result, options, context);
@@ -157,11 +164,6 @@ export function createEnhancement<DbClient extends object>(
157164
}
158165
}
159166

160-
if (hasPassword && kinds.includes('password')) {
161-
// @password proxy
162-
result = withPassword(result, options);
163-
}
164-
165167
if (hasOmit && kinds.includes('omit')) {
166168
// @omit proxy
167169
result = withOmit(result, options);

‎packages/runtime/src/enhancements/policy/handler.ts‎

Lines changed: 33 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -410,22 +410,19 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
410410

411411
// Validates the given create payload against Zod schema if any
412412
private validateCreateInputSchema(model: string, data: any) {
413-
const schema = this.policyUtils.getZodSchema(model, 'create');
414-
if (schema && data) {
415-
const parseResult = schema.safeParse(data);
416-
if (!parseResult.success) {
417-
throw this.policyUtils.deniedByPolicy(
418-
model,
419-
'create',
420-
`input failed validation: ${fromZodError(parseResult.error)}`,
421-
CrudFailureReason.DATA_VALIDATION_VIOLATION,
422-
parseResult.error
423-
);
424-
}
425-
return parseResult.data;
426-
} else {
413+
if (!data) {
427414
return data;
428415
}
416+
417+
return this.policyUtils.validateZodSchema(model, 'create', data, false, (err) => {
418+
throw this.policyUtils.deniedByPolicy(
419+
model,
420+
'create',
421+
`input failed validation: ${fromZodError(err)}`,
422+
CrudFailureReason.DATA_VALIDATION_VIOLATION,
423+
err
424+
);
425+
});
429426
}
430427

431428
createMany(args: { data: any; skipDuplicates?: boolean }) {
@@ -1195,33 +1192,30 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
11951192

11961193
// Validates the given update payload against Zod schema if any
11971194
private validateUpdateInputSchema(model: string, data: any) {
1198-
const schema = this.policyUtils.getZodSchema(model, 'update');
1199-
if (schema && data) {
1200-
// update payload can contain non-literal fields, like:
1201-
// { x: { increment: 1 } }
1202-
// we should only validate literal fields
1203-
1204-
const literalData = Object.entries(data).reduce<any>(
1205-
(acc, [k, v]) => ({ ...acc, ...(typeof v !== 'object' ? { [k]: v } : {}) }),
1206-
{}
1207-
);
1208-
1209-
const parseResult = schema.safeParse(literalData);
1210-
if (!parseResult.success) {
1211-
throw this.policyUtils.deniedByPolicy(
1212-
model,
1213-
'update',
1214-
`input failed validation: ${fromZodError(parseResult.error)}`,
1215-
CrudFailureReason.DATA_VALIDATION_VIOLATION,
1216-
parseResult.error
1217-
);
1218-
}
1219-
1220-
// schema may have transformed field values, use it to overwrite the original data
1221-
return { ...data, ...parseResult.data };
1222-
} else {
1195+
if (!data) {
12231196
return data;
12241197
}
1198+
1199+
// update payload can contain non-literal fields, like:
1200+
// { x: { increment: 1 } }
1201+
// we should only validate literal fields
1202+
const literalData = Object.entries(data).reduce<any>(
1203+
(acc, [k, v]) => ({ ...acc, ...(typeof v !== 'object' ? { [k]: v } : {}) }),
1204+
{}
1205+
);
1206+
1207+
const validatedData = this.policyUtils.validateZodSchema(model, 'update', literalData, false, (err) => {
1208+
throw this.policyUtils.deniedByPolicy(
1209+
model,
1210+
'update',
1211+
`input failed validation: ${fromZodError(err)}`,
1212+
CrudFailureReason.DATA_VALIDATION_VIOLATION,
1213+
err
1214+
);
1215+
});
1216+
1217+
// schema may have transformed field values, use it to overwrite the original data
1218+
return { ...data, ...validatedData };
12251219
}
12261220

12271221
updateMany(args: any) {

‎packages/runtime/src/enhancements/policy/policy-utils.ts‎

Lines changed: 79 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,19 @@
33
import deepmerge from 'deepmerge';
44
import { lowerCaseFirst } from 'lower-case-first';
55
import { upperCaseFirst } from 'upper-case-first';
6-
import { ZodError } from 'zod';
6+
import { z, type ZodError, type ZodObject, type ZodSchema } from 'zod';
77
import { fromZodError } from 'zod-validation-error';
88
import { CrudFailureReason, PrismaErrorCode } from '../../constants';
9-
import { enumerate, getFields, getModelFields, resolveField, zip, type FieldInfo, type ModelMeta } from '../../cross';
10-
import { clone } from '../../cross';
9+
import {
10+
clone,
11+
enumerate,
12+
getFields,
13+
getModelFields,
14+
resolveField,
15+
zip,
16+
type FieldInfo,
17+
type ModelMeta,
18+
} from '../../cross';
1119
import {
1220
AuthUser,
1321
CrudContract,
@@ -843,20 +851,15 @@ export class PolicyUtil extends QueryUtils {
843851

844852
if (schema) {
845853
// TODO: push down schema check to the database
846-
const parseResult = schema.safeParse(result);
847-
if (!parseResult.success) {
848-
const error = fromZodError(parseResult.error);
849-
if (this.logger.enabled('info')) {
850-
this.logger.info(`entity ${model} failed validation for operation ${operation}: ${error}`);
851-
}
854+
this.validateZodSchema(model, undefined, result, true, (err) => {
852855
throw this.deniedByPolicy(
853856
model,
854857
operation,
855-
`entities ${formatObject(uniqueFilter, false)} failed validation: [${error}]`,
858+
`entity ${formatObject(uniqueFilter, false)} failed validation: [${fromZodError(err)}]`,
856859
CrudFailureReason.DATA_VALIDATION_VIOLATION,
857-
parseResult.error
860+
err
858861
);
859-
}
862+
});
860863
}
861864
}
862865

@@ -1262,14 +1265,75 @@ export class PolicyUtil extends QueryUtils {
12621265
/**
12631266
* Gets Zod schema for the given model and access kind.
12641267
*
1265-
* @param kind If undefined, returns the full schema.
1268+
* @param kind kind of Zod schema to get for. If undefined, returns the full schema.
12661269
*/
1267-
getZodSchema(model: string, kind: 'create' | 'update' | undefined = undefined) {
1270+
getZodSchema(
1271+
model: string,
1272+
excludePasswordFields: boolean = true,
1273+
kind: 'create' | 'update' | undefined = undefined
1274+
) {
12681275
if (!this.hasFieldValidation(model)) {
12691276
return undefined;
12701277
}
12711278
const schemaKey = `${upperCaseFirst(model)}${kind ? 'Prisma' + upperCaseFirst(kind) : ''}Schema`;
1272-
return this.zodSchemas?.models?.[schemaKey];
1279+
let result = this.zodSchemas?.models?.[schemaKey] as ZodObject<any> | undefined;
1280+
1281+
if (result && excludePasswordFields) {
1282+
// fields with `@password` attribute changes at runtime, so we cannot directly use the generated
1283+
// zod schema to validate it, instead, the validation happens when checking the input of "create"
1284+
// and "update" operations
1285+
const modelFields = this.modelMeta.models[lowerCaseFirst(model)]?.fields;
1286+
if (modelFields) {
1287+
for (const [key, field] of Object.entries(modelFields)) {
1288+
if (field.attributes?.some((attr) => attr.name === '@password')) {
1289+
// override `@password` field schema with a string schema
1290+
let pwFieldSchema: ZodSchema = z.string();
1291+
if (field.isOptional) {
1292+
pwFieldSchema = pwFieldSchema.nullish();
1293+
}
1294+
result = result?.merge(z.object({ [key]: pwFieldSchema }));
1295+
}
1296+
}
1297+
}
1298+
}
1299+
1300+
return result;
1301+
}
1302+
1303+
/**
1304+
* Validates the given data against the Zod schema for the given model and kind.
1305+
*
1306+
* @param model model
1307+
* @param kind validation kind. Pass undefined to validate against the full schema.
1308+
* @param data input data
1309+
* @param excludePasswordFields whether exclude schema validation for `@password` fields
1310+
* @param onError error callback
1311+
* @returns Zod-validated data
1312+
*/
1313+
validateZodSchema(
1314+
model: string,
1315+
kind: 'create' | 'update' | undefined,
1316+
data: object,
1317+
excludePasswordFields: boolean,
1318+
onError: (error: ZodError) => void
1319+
) {
1320+
const schema = this.getZodSchema(model, excludePasswordFields, kind);
1321+
if (!schema) {
1322+
return data;
1323+
}
1324+
1325+
const parseResult = schema.safeParse(data);
1326+
if (!parseResult.success) {
1327+
if (this.logger.enabled('info')) {
1328+
this.logger.info(
1329+
`entity ${model} failed validation for operation ${kind}: ${fromZodError(parseResult.error)}`
1330+
);
1331+
}
1332+
onError(parseResult.error);
1333+
return undefined;
1334+
}
1335+
1336+
return parseResult.data;
12731337
}
12741338

12751339
/**

‎tests/integration/tests/enhancements/with-password/with-password.test.ts‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,14 @@ describe('Password test', () => {
1313
process.chdir(origDir);
1414
});
1515

16-
const model = `
16+
it('password tests', async () => {
17+
const { enhance } = await loadSchema(`
1718
model User {
1819
id String @id @default(cuid())
1920
password String @password(saltLength: 16)
2021
2122
@@allow('all', true)
22-
}`;
23-
24-
it('password tests', async () => {
25-
const { enhance } = await loadSchema(model);
23+
}`);
2624

2725
const db = enhance();
2826
const r = await db.user.create({
@@ -41,4 +39,58 @@ describe('Password test', () => {
4139
});
4240
expect(compareSync('abc456', r1.password)).toBeTruthy();
4341
});
42+
43+
it('length tests', async () => {
44+
const { enhance } = await loadSchema(`
45+
model User {
46+
id String @id @default(cuid())
47+
password String @password(saltLength: 16) @length(1, 8) @startsWith('abc')
48+
49+
@@allow('all', true)
50+
}`);
51+
52+
const db = enhance();
53+
let r = await db.user.create({
54+
data: {
55+
id: '1',
56+
password: 'abc123',
57+
},
58+
});
59+
expect(compareSync('abc123', r.password)).toBeTruthy();
60+
61+
r = await db.user.update({
62+
where: { id: '1' },
63+
data: {
64+
password: 'abc456',
65+
},
66+
});
67+
expect(compareSync('abc456', r.password)).toBeTruthy();
68+
69+
await expect(
70+
db.user.update({
71+
where: { id: '1' },
72+
data: {
73+
password: 'abc456789',
74+
},
75+
})
76+
).toBeRejectedByPolicy(['String must contain at most 8 character(s) at "password"']);
77+
78+
await expect(
79+
db.user.create({
80+
data: {
81+
id: '2',
82+
password: 'abc456789',
83+
},
84+
})
85+
).toBeRejectedByPolicy(['String must contain at most 8 character(s) at "password"']);
86+
87+
await expect(
88+
db.user.create({
89+
data: {
90+
id: '2',
91+
password: '123456',
92+
},
93+
})
94+
).toBeRejectedByPolicy(['must start with "abc" at "password"']);
95+
});
4496
});

‎tests/integration/tests/enhancements/with-policy/field-validation.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe('Field validation', () => {
99
`
1010
model User {
1111
id String @id @default(cuid())
12-
password String @length(8, 16)
12+
password String @password @length(8, 16)
1313
email String? @email @endsWith("@myorg.com")
1414
profileImage String? @url
1515
handle String? @regex("^[0-9a-zA-Z]{4,16}$")

‎tests/integration/tests/enhancements/with-policy/postgres.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ describe('With Policy: with postgres', () => {
5353

5454
// create user1
5555
// create should succeed but result can be read back anonymously
56-
await expect(anonDb.user.create({ data: user1 })).toBeRejectedByPolicy();
56+
await expect(anonDb.user.create({ data: user1 })).toBeRejectedByPolicy([
57+
'result is not allowed to be read back',
58+
]);
5759
await expect(user1Db.user.findUnique({ where: { id: user1.id } })).toResolveTruthy();
5860
await expect(user2Db.user.findUnique({ where: { id: user1.id } })).toResolveNull();
5961

‎tests/integration/tests/enhancements/with-policy/todo-sample.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ describe('Todo Policy Tests', () => {
3838

3939
// create user1
4040
// create should succeed but result can be read back anonymously
41-
await expect(anonDb.user.create({ data: user1 })).toBeRejectedByPolicy();
41+
await expect(anonDb.user.create({ data: user1 })).toBeRejectedByPolicy([
42+
'result is not allowed to be read back',
43+
]);
4244
await expect(user1Db.user.findUnique({ where: { id: user1.id } })).toResolveTruthy();
4345
await expect(user2Db.user.findUnique({ where: { id: user1.id } })).toResolveNull();
4446

0 commit comments

Comments
 (0)