Skip to content

fix: '@password' attribute doesn't work well with data validation #1547

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.

Sign up for GitHub

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
Jun 30, 2024
Merged

fix: '@password' attribute doesn't work well with data validation #1547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 7 additions & 5 deletions packages/runtime/src/enhancements/create-enhancement.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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