Skip to content

Commit

Permalink
fix: '@password' attribute doesn't work well with data validation
Browse files Browse the repository at this point in the history 
Fixes #1502
  • Loading branch information
@ymc9
ymc9 committed Jun 30, 2024
1 parent ba2a113 commit e1e6e6b
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 67 deletions.
12 changes: 7 additions & 5 deletions packages/runtime/src/enhancements/create-enhancement.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ export function createEnhancement<DbClient extends object>(
}
}

// password enhancement must be applied prior to policy because it changes then length of the field
// and can break validation rules like `@length`
if (hasPassword && kinds.includes('password')) {
// @password proxy
result = withPassword(result, options);
}

// 'policy' and 'validation' enhancements are both enabled by `withPolicy`
if (kinds.includes('policy') || kinds.includes('validation')) {
result = withPolicy(result, options, context);
Expand All @@ -157,11 +164,6 @@ export function createEnhancement<DbClient extends object>(
}
}

if (hasPassword && kinds.includes('password')) {
// @password proxy
result = withPassword(result, options);
}

if (hasOmit && kinds.includes('omit')) {
// @omit proxy
result = withOmit(result, options);
Expand Down
72 changes: 33 additions & 39 deletions packages/runtime/src/enhancements/policy/handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,22 +410,19 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr

// Validates the given create payload against Zod schema if any
private validateCreateInputSchema(model: string, data: any) {
const schema = this.policyUtils.getZodSchema(model, 'create');
if (schema && data) {
const parseResult = schema.safeParse(data);
if (!parseResult.success) {
throw this.policyUtils.deniedByPolicy(
model,
'create',
`input failed validation: ${fromZodError(parseResult.error)}`,
CrudFailureReason.DATA_VALIDATION_VIOLATION,
parseResult.error
);
}
return parseResult.data;
} else {
if (!data) {
return data;
}

return this.policyUtils.validateZodSchema(model, 'create', data, false, (err) => {
throw this.policyUtils.deniedByPolicy(
model,
'create',
`input failed validation: ${fromZodError(err)}`,
CrudFailureReason.DATA_VALIDATION_VIOLATION,
err
);
});
}

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

// Validates the given update payload against Zod schema if any
private validateUpdateInputSchema(model: string, data: any) {
const schema = this.policyUtils.getZodSchema(model, 'update');
if (schema && data) {
// update payload can contain non-literal fields, like:
// { x: { increment: 1 } }
// we should only validate literal fields

const literalData = Object.entries(data).reduce<any>(
(acc, [k, v]) => ({ ...acc, ...(typeof v !== 'object' ? { [k]: v } : {}) }),
{}
);

const parseResult = schema.safeParse(literalData);
if (!parseResult.success) {
throw this.policyUtils.deniedByPolicy(
model,
'update',
`input failed validation: ${fromZodError(parseResult.error)}`,
CrudFailureReason.DATA_VALIDATION_VIOLATION,
parseResult.error
);
}

// schema may have transformed field values, use it to overwrite the original data
return { ...data, ...parseResult.data };
} else {
if (!data) {
return data;
}

// update payload can contain non-literal fields, like:
// { x: { increment: 1 } }
// we should only validate literal fields
const literalData = Object.entries(data).reduce<any>(
(acc, [k, v]) => ({ ...acc, ...(typeof v !== 'object' ? { [k]: v } : {}) }),
{}
);

const validatedData = this.policyUtils.validateZodSchema(model, 'update', literalData, false, (err) => {
throw this.policyUtils.deniedByPolicy(
model,
'update',
`input failed validation: ${fromZodError(err)}`,
CrudFailureReason.DATA_VALIDATION_VIOLATION,
err
);
});

// schema may have transformed field values, use it to overwrite the original data
return { ...data, ...validatedData };
}

updateMany(args: any) {
Expand Down
94 changes: 79 additions & 15 deletions packages/runtime/src/enhancements/policy/policy-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,19 @@
import deepmerge from 'deepmerge';
import { lowerCaseFirst } from 'lower-case-first';
import { upperCaseFirst } from 'upper-case-first';
import { ZodError } from 'zod';
import { z, type ZodError, type ZodObject, type ZodSchema } from 'zod';
import { fromZodError } from 'zod-validation-error';
import { CrudFailureReason, PrismaErrorCode } from '../../constants';
import { enumerate, getFields, getModelFields, resolveField, zip, type FieldInfo, type ModelMeta } from '../../cross';
import { clone } from '../../cross';
import {
clone,
enumerate,
getFields,
getModelFields,
resolveField,
zip,
type FieldInfo,
type ModelMeta,
} from '../../cross';
import {
AuthUser,
CrudContract,
Expand Down Expand Up @@ -843,20 +851,15 @@ export class PolicyUtil extends QueryUtils {

if (schema) {
// TODO: push down schema check to the database
const parseResult = schema.safeParse(result);
if (!parseResult.success) {
const error = fromZodError(parseResult.error);
if (this.logger.enabled('info')) {
this.logger.info(`entity ${model} failed validation for operation ${operation}: ${error}`);
}
this.validateZodSchema(model, undefined, result, true, (err) => {
throw this.deniedByPolicy(
model,
operation,
`entities ${formatObject(uniqueFilter, false)} failed validation: [${error}]`,
`entity ${formatObject(uniqueFilter, false)} failed validation: [${fromZodError(err)}]`,
CrudFailureReason.DATA_VALIDATION_VIOLATION,
parseResult.error
err
);
}
});
}
}

Expand Down Expand Up @@ -1262,14 +1265,75 @@ export class PolicyUtil extends QueryUtils {
/**
* Gets Zod schema for the given model and access kind.
*
* @param kind If undefined, returns the full schema.
* @param kind kind of Zod schema to get for. If undefined, returns the full schema.
*/
getZodSchema(model: string, kind: 'create' | 'update' | undefined = undefined) {
getZodSchema(
model: string,
excludePasswordFields: boolean = true,
kind: 'create' | 'update' | undefined = undefined
) {
if (!this.hasFieldValidation(model)) {
return undefined;
}
const schemaKey = `${upperCaseFirst(model)}${kind ? 'Prisma' + upperCaseFirst(kind) : ''}Schema`;
return this.zodSchemas?.models?.[schemaKey];
let result = this.zodSchemas?.models?.[schemaKey] as ZodObject<any> | undefined;

if (result && excludePasswordFields) {
// fields with `@password` attribute changes at runtime, so we cannot directly use the generated
// zod schema to validate it, instead, the validation happens when checking the input of "create"
// and "update" operations
const modelFields = this.modelMeta.models[lowerCaseFirst(model)]?.fields;
if (modelFields) {
for (const [key, field] of Object.entries(modelFields)) {
if (field.attributes?.some((attr) => attr.name === '@password')) {
// override `@password` field schema with a string schema
let pwFieldSchema: ZodSchema = z.string();
if (field.isOptional) {
pwFieldSchema = pwFieldSchema.nullish();
}
result = result?.merge(z.object({ [key]: pwFieldSchema }));
}
}
}
}

return result;
}

/**
* Validates the given data against the Zod schema for the given model and kind.
*
* @param model model
* @param kind validation kind. Pass undefined to validate against the full schema.
* @param data input data
* @param excludePasswordFields whether exclude schema validation for `@password` fields
* @param onError error callback
* @returns Zod-validated data
*/
validateZodSchema(
model: string,
kind: 'create' | 'update' | undefined,
data: object,
excludePasswordFields: boolean,
onError: (error: ZodError) => void
) {
const schema = this.getZodSchema(model, excludePasswordFields, kind);
if (!schema) {
return data;
}

const parseResult = schema.safeParse(data);
if (!parseResult.success) {
if (this.logger.enabled('info')) {
this.logger.info(
`entity ${model} failed validation for operation ${kind}: ${fromZodError(parseResult.error)}`
);
}
onError(parseResult.error);
return undefined;
}

return parseResult.data;
}

/**
Expand Down
62 changes: 57 additions & 5 deletions tests/integration/tests/enhancements/with-password/with-password.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,14 @@ describe('Password test', () => {
process.chdir(origDir);
});

const model = `
it('password tests', async () => {
const { enhance } = await loadSchema(`
model User {
id String @id @default(cuid())
password String @password(saltLength: 16)
@@allow('all', true)
}`;

it('password tests', async () => {
const { enhance } = await loadSchema(model);
}`);

const db = enhance();
const r = await db.user.create({
Expand All @@ -41,4 +39,58 @@ describe('Password test', () => {
});
expect(compareSync('abc456', r1.password)).toBeTruthy();
});

it('length tests', async () => {
const { enhance } = await loadSchema(`
model User {
id String @id @default(cuid())
password String @password(saltLength: 16) @length(1, 8) @startsWith('abc')
@@allow('all', true)
}`);

const db = enhance();
let r = await db.user.create({
data: {
id: '1',
password: 'abc123',
},
});
expect(compareSync('abc123', r.password)).toBeTruthy();

r = await db.user.update({
where: { id: '1' },
data: {
password: 'abc456',
},
});
expect(compareSync('abc456', r.password)).toBeTruthy();

await expect(
db.user.update({
where: { id: '1' },
data: {
password: 'abc456789',
},
})
).toBeRejectedByPolicy(['String must contain at most 8 character(s) at "password"']);

await expect(
db.user.create({
data: {
id: '2',
password: 'abc456789',
},
})
).toBeRejectedByPolicy(['String must contain at most 8 character(s) at "password"']);

await expect(
db.user.create({
data: {
id: '2',
password: '123456',
},
})
).toBeRejectedByPolicy(['must start with "abc" at "password"']);
});
});
2 changes: 1 addition & 1 deletion tests/integration/tests/enhancements/with-policy/field-validation.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('Field validation', () => {
`
model User {
id String @id @default(cuid())
password String @length(8, 16)
password String @password @length(8, 16)
email String? @email @endsWith("@myorg.com")
profileImage String? @url
handle String? @regex("^[0-9a-zA-Z]{4,16}$")
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/tests/enhancements/with-policy/postgres.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ describe('With Policy: with postgres', () => {

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

Expand Down
4 changes: 3 additions & 1 deletion tests/integration/tests/enhancements/with-policy/todo-sample.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ describe('Todo Policy Tests', () => {

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

Expand Down

0 comments on commit e1e6e6b

Please sign in to comment.