Skip to content
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.

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

fix: nullify field instead of reject when an optional relation field is not readable #588

Merged
merged 4 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions packages/plugins/swr/tests/swr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ plugin swr {

${sharedModel}
`,
true,
false,
[`${origDir}/dist`, 'react', '@types/react', 'swr'],
true
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, 'react', '@types/react', 'swr'],
compile: true,
}
);
});
});
23 changes: 15 additions & 8 deletions packages/plugins/tanstack-query/tests/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,16 @@ plugin tanstack {

${sharedModel}
`,
true,
false,
[`${origDir}/dist`, 'react@18.2.0', '@types/react@18.2.0', '@tanstack/react-query@4.29.7'],
true
{
pushDb: false,
extraDependencies: [
`${origDir}/dist`,
'react@18.2.0',
'@types/react@18.2.0',
'@tanstack/react-query@4.29.7',
],
compile: true,
}
);
});

Expand All @@ -69,10 +75,11 @@ plugin tanstack {

${sharedModel}
`,
true,
false,
[`${origDir}/dist`, 'svelte@^3.0.0', '@tanstack/svelte-query@4.29.7'],
true
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, 'svelte@^3.0.0', '@tanstack/svelte-query@4.29.7'],
compile: true,
}
);
});
});
76 changes: 45 additions & 31 deletions packages/plugins/trpc/tests/trpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,12 @@ model Foo {
@@ignore
}
`,
true,
false,
[`${origDir}/dist`, '@trpc/client', '@trpc/server'],
true
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, '@trpc/client', '@trpc/server'],
compile: true,
fullZod: true,
}
);
});

Expand Down Expand Up @@ -88,10 +90,12 @@ model Foo {
@@ignore
}
`,
true,
false,
[`${origDir}/dist`, '@trpc/client', '@trpc/server'],
true
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, '@trpc/client', '@trpc/server'],
compile: true,
fullZod: true,
}
);
expect(fs.existsSync(path.join(projectDir, 'trpc'))).toBe(true);
});
Expand All @@ -116,11 +120,13 @@ model Post {
authorId String?
}
`,
true,
false,
[`${origDir}/dist`, '@trpc/client', '@trpc/server'],
true,
'zenstack/schema.zmodel'
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, '@trpc/client', '@trpc/server'],
compile: true,
fullZod: true,
customSchemaFilePath: 'zenstack/schema.zmodel',
}
);
expect(fs.existsSync(path.join(projectDir, 'zenstack/trpc'))).toBe(true);
});
Expand All @@ -139,11 +145,13 @@ model Post {
title String
}
`,
true,
false,
[`${origDir}/dist`, '@trpc/client', '@trpc/server'],
true,
'zenstack/schema.zmodel'
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, '@trpc/client', '@trpc/server'],
compile: true,
fullZod: true,
customSchemaFilePath: 'zenstack/schema.zmodel',
}
);
const content = fs.readFileSync(path.join(projectDir, 'zenstack/trpc/routers/Post.router.ts'), 'utf-8');
expect(content).toContain('findMany:');
Expand All @@ -167,11 +175,13 @@ model Post {
title String
}
`,
true,
false,
[`${origDir}/dist`, '@trpc/client', '@trpc/server'],
true,
'zenstack/schema.zmodel'
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, '@trpc/client', '@trpc/server'],
compile: true,
fullZod: true,
customSchemaFilePath: 'zenstack/schema.zmodel',
}
);
const content = fs.readFileSync(path.join(projectDir, 'zenstack/trpc/routers/Post.router.ts'), 'utf-8');
expect(content).toContain('findMany:');
Expand Down Expand Up @@ -211,10 +221,12 @@ model Post {

${BLOG_BASE_SCHEMA}
`,
true,
false,
[`${origDir}/dist`, '@trpc/client', '@trpc/server', '@trpc/react-query'],
true
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, '@trpc/client', '@trpc/server', '@trpc/react-query'],
compile: true,
fullZod: true,
}
);
});

Expand All @@ -229,10 +241,12 @@ model Post {

${BLOG_BASE_SCHEMA}
`,
true,
false,
[`${origDir}/dist`, '@trpc/client', '@trpc/server', '@trpc/next'],
true
{
pushDb: false,
extraDependencies: [`${origDir}/dist`, '@trpc/client', '@trpc/server', '@trpc/next'],
compile: true,
fullZod: true,
}
);
});
});
14 changes: 13 additions & 1 deletion packages/runtime/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,23 @@ export const GUARD_FIELD_NAME = 'zenstack_guard';
export const AUXILIARY_FIELDS = [TRANSACTION_FIELD_NAME, GUARD_FIELD_NAME];

/**
* Reasons for a CRUD operation to fail.
* Reasons for a CRUD operation to fail
*/
export enum CrudFailureReason {
/**
* CRUD suceeded but the result was not readable.
*/
RESULT_NOT_READABLE = 'RESULT_NOT_READABLE',

/**
* CRUD failed because of a data validation rule violation.
*/
DATA_VALIDATION_VIOLATION = 'DATA_VALIDATION_VIOLATION',
}

/**
* Prisma error codes used
*/
export enum PrismaErrorCode {
CONSTRAINED_FAILED = 'P2004',
}
39 changes: 35 additions & 4 deletions packages/runtime/src/enhancements/policy/policy-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@ import { lowerCaseFirst } from 'lower-case-first';
import pluralize from 'pluralize';
import { upperCaseFirst } from 'upper-case-first';
import { fromZodError } from 'zod-validation-error';
import { AUXILIARY_FIELDS, CrudFailureReason, GUARD_FIELD_NAME, TRANSACTION_FIELD_NAME } from '../../constants';
import {
AUXILIARY_FIELDS,
CrudFailureReason,
GUARD_FIELD_NAME,
PrismaErrorCode,
TRANSACTION_FIELD_NAME,
} from '../../constants';
import { isPrismaClientKnownRequestError } from '../../error';
import {
AuthUser,
DbClientContract,
Expand Down Expand Up @@ -402,7 +409,26 @@ export class PolicyUtil {
// `Validating read of to-one relation: ${fieldInfo.type}#${formatObject(ids)}`
// );
// }
await this.checkPolicyForFilter(fieldInfo.type, ids, operation, this.db);
try {
await this.checkPolicyForFilter(fieldInfo.type, ids, operation, this.db);
} catch (err) {
if (
isPrismaClientKnownRequestError(err) &&
err.code === PrismaErrorCode.CONSTRAINED_FAILED
) {
// denied by policy
if (fieldInfo.isOptional) {
// if the relation is optional, just nullify it
entityData[field] = null;
} else {
// otherwise reject
throw err;
}
} else {
// unknown error
throw err;
}
}
}
}

Expand Down Expand Up @@ -772,7 +798,7 @@ export class PolicyUtil {
return prismaClientKnownRequestError(
this.db,
`denied by policy: ${model} entities failed '${operation}' check${extra ? ', ' + extra : ''}`,
{ clientVersion: getVersion(), code: 'P2004', meta: { reason } }
{ clientVersion: getVersion(), code: PrismaErrorCode.CONSTRAINED_FAILED, meta: { reason } }
);
}

Expand Down Expand Up @@ -864,7 +890,12 @@ export class PolicyUtil {
if (this.logger.enabled('info')) {
this.logger.info(`entity ${model} failed schema check for operation ${operation}: ${error}`);
}
throw this.deniedByPolicy(model, operation, `entities failed schema check: [${error}]`);
throw this.deniedByPolicy(
model,
operation,
`entities failed schema check: [${error}]`,
CrudFailureReason.DATA_VALIDATION_VIOLATION
);
}
} else {
// count entities with policy injected and see if any of them are filtered out
Expand Down
33 changes: 23 additions & 10 deletions packages/schema/src/cli/plugin-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,38 +99,49 @@ export class PluginRunner {
}

// make sure prerequisites are included
const corePlugins = ['@core/prisma', '@core/model-meta', '@core/access-policy'];
const corePlugins: Array<{ provider: string; options?: Record<string, unknown> }> = [
{ provider: '@core/prisma' },
{ provider: '@core/model-meta' },
{ provider: '@core/access-policy' },
];

if (getDataModels(context.schema).some((model) => hasValidationAttributes(model))) {
// '@core/zod' plugin is auto-enabled if there're validation rules
corePlugins.push('@core/zod');
corePlugins.push({ provider: '@core/zod', options: { modelOnly: true } });
}

// core dependencies introduced by dependencies
// core plugins introduced by dependencies
plugins
.flatMap((p) => p.dependencies)
.forEach((dep) => {
if (dep.startsWith('@core/') && !corePlugins.includes(dep)) {
corePlugins.push(dep);
if (dep.startsWith('@core/')) {
const existing = corePlugins.find((p) => p.provider === dep);
if (existing) {
// reset options to default
existing.options = undefined;
} else {
// add core dependency
corePlugins.push({ provider: dep });
}
}
});

for (const corePlugin of corePlugins.reverse()) {
const existingIdx = plugins.findIndex((p) => p.provider === corePlugin);
const existingIdx = plugins.findIndex((p) => p.provider === corePlugin.provider);
if (existingIdx >= 0) {
// shift the plugin to the front
const existing = plugins[existingIdx];
plugins.splice(existingIdx, 1);
plugins.unshift(existing);
} else {
// synthesize a plugin and insert front
const pluginModule = require(this.getPluginModulePath(corePlugin));
const pluginName = this.getPluginName(pluginModule, corePlugin);
const pluginModule = require(this.getPluginModulePath(corePlugin.provider));
const pluginName = this.getPluginName(pluginModule, corePlugin.provider);
plugins.unshift({
name: pluginName,
provider: corePlugin,
provider: corePlugin.provider,
dependencies: [],
options: { schemaPath: context.schemaPath, name: pluginName },
options: { schemaPath: context.schemaPath, name: pluginName, ...corePlugin.options },
run: pluginModule.default,
module: pluginModule,
});
Expand All @@ -154,7 +165,9 @@ export class PluginRunner {

let dmmf: DMMF.Document | undefined = undefined;
for (const { name, provider, run, options } of plugins) {
// const start = Date.now();
await this.runPlugin(name, run, context, options, dmmf, warnings);
// console.log(`✅ Plugin ${colors.bold(name)} (${provider}) completed in ${Date.now() - start}ms`);
if (provider === '@core/prisma') {
// load prisma DMMF
dmmf = await getDMMF({
Expand Down
2 changes: 1 addition & 1 deletion packages/schema/src/plugins/prisma/schema-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
import {
analyzePolicies,
getDataModels,
getDMMF,
getLiteral,
getLiteralArray,
getPrismaVersion,
Expand All @@ -32,7 +33,6 @@ import {
resolved,
resolvePath,
TRANSACTION_FIELD_NAME,
getDMMF
} from '@zenstackhq/sdk';
import fs from 'fs';
import { writeFile } from 'fs/promises';
Expand Down
Loading