Skip to content

Commit

Permalink
Use extensionError for all extensions (#6672)
Browse files Browse the repository at this point in the history
  • Loading branch information
@timleslie
timleslie authored Sep 29, 2021
1 parent 8631917 commit 689d8ec
Show file tree
Hide file tree
Showing 10 changed files with 196 additions and 118 deletions.
5 changes: 5 additions & 0 deletions .changeset/little-colts-deliver.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-next/keystone': patch
---

Improved the error messages provided from the GraphQL API when extension code (e.g access control functions, hooks, etc) throw exceptions.
35 changes: 18 additions & 17 deletions packages/keystone/src/lib/core/access-control.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
ListFilterAccessControl,
KeystoneContext,
} from '../../types';
import { accessReturnError } from './graphql-errors';
import { accessReturnError, extensionError } from './graphql-errors';
import { InitialisedList } from './types-for-lists';
import { InputFilter } from './where-inputs';

Expand All @@ -25,8 +25,15 @@ export async function getOperationAccess(
const args = { operation, session: context.session, listKey: list.listKey, context };
// Check the mutation access
const access = list.access.operation[operation];
// @ts-ignore
const result = await access(args);
let result;
try {
// @ts-ignore
result = await access(args);
} catch (error: any) {
throw extensionError('Access control', [
{ error, tag: `${list.listKey}.access.operation.${args.operation}` },
]);
}

const resultType = typeof result;

Expand All @@ -49,20 +56,14 @@ export async function getAccessFilters(
const args = { operation, session: context.session, listKey: list.listKey, context };
// Check the mutation access
const access = list.access.filter[operation];
// @ts-ignore
return typeof access === 'function' ? await access(args) : access;
}

export async function validateFieldAccessControl<
Args extends { listKey: string; fieldKey: string; operation: 'read' | 'create' | 'update' }
>({
access,
args,
}: {
access: ((args: Args) => boolean | Promise<boolean>) | boolean;
args: Args;
}) {
return typeof access === 'function' ? await access(args) : access;
try {
// @ts-ignore
return typeof access === 'function' ? await access(args) : access;
} catch (error: any) {
throw extensionError('Access control', [
{ error, tag: `${args.listKey}.access.filter.${args.operation}` },
]);
}
}

export function parseFieldAccessControl(
Expand Down
22 changes: 14 additions & 8 deletions packages/keystone/src/lib/core/filter-order-access.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { KeystoneContext } from '../../types';
import { accessReturnError, filterAccessError } from './graphql-errors';
import { accessReturnError, extensionError, filterAccessError } from './graphql-errors';
import { InitialisedList } from './types-for-lists';

export async function checkFilterOrderAccess(
things: { fieldKey: string; list: InitialisedList }[],
context: KeystoneContext,
operation: 'filter' | 'orderBy'
) {
const func = operation === 'filter' ? 'isFilterable' : 'isOrderable';
const failures: string[] = [];
const returnTypeErrors: any[] = [];
const accessErrors: any[] = [];
for (const { fieldKey, list } of things) {
const field = list.fields[fieldKey];
const rule = field.graphql.isEnabled[operation];
Expand All @@ -18,24 +20,28 @@ export async function checkFilterOrderAccess(
throw new Error('Assert failed');
} else if (typeof rule === 'function') {
// Apply dynamic rules
const result = await rule({
context,
session: context.session,
listKey: list.listKey,
fieldKey,
});
let result;
try {
result = await rule({ context, session: context.session, listKey: list.listKey, fieldKey });
} catch (error: any) {
accessErrors.push({ error, tag: `${list.listKey}.${fieldKey}.${func}` });
continue;
}
const resultType = typeof result;

// It's important that we don't cast objects to truthy values, as there's a strong chance that the user
// has made a mistake.
if (resultType !== 'boolean') {
const func = operation === 'filter' ? 'isFilterable' : 'isOrderable';
returnTypeErrors.push({ tag: `${list.listKey}.${fieldKey}.${func}`, returned: resultType });
} else if (!result) {
failures.push(`${list.listKey}.${fieldKey}`);
}
}
}
if (accessErrors.length) {
throw extensionError(func, accessErrors);
}

if (returnTypeErrors.length) {
throw accessReturnError(returnTypeErrors);
}
Expand Down
131 changes: 82 additions & 49 deletions packages/keystone/src/lib/core/mutations/access-control.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { KeystoneContext } from '../../../types';
import { validateFieldAccessControl } from '../access-control';
import { accessDeniedError, accessReturnError } from '../graphql-errors';
import { accessDeniedError, accessReturnError, extensionError } from '../graphql-errors';
import { mapUniqueWhereToWhere } from '../queries/resolvers';
import { InitialisedList } from '../types-for-lists';
import { runWithPrisma } from '../utils';
Expand Down Expand Up @@ -52,7 +51,15 @@ export async function getAccessControlledItemForDelete(
const args = { operation, session: context.session, listKey: list.listKey, context, item };

// List level 'item' access control
const result = await access(args);
let result;
try {
result = await access(args);
} catch (error: any) {
throw extensionError('Access control', [
{ error, tag: `${args.listKey}.access.item.${args.operation}` },
]);
}

const resultType = typeof result;

// It's important that we don't cast objects to truthy values, as there's a strong chance that the user
Expand Down Expand Up @@ -102,7 +109,14 @@ export async function getAccessControlledItemForUpdate(
};

// List level 'item' access control
const result = await access(args);
let result;
try {
result = await access(args);
} catch (error: any) {
throw extensionError('Access control', [
{ error, tag: `${args.listKey}.access.item.${args.operation}` },
]);
}
const resultType = typeof result;

// It's important that we don't cast objects to truthy values, as there's a strong chance that the user
Expand All @@ -125,32 +139,37 @@ export async function getAccessControlledItemForUpdate(
}

// Field level 'item' access control
const fieldAccess = await Promise.all(
Object.keys(originalInput!).map(async fieldKey => [
fieldKey,
await validateFieldAccessControl({
access: list.fields[fieldKey].access[operation],
args: { ...args, fieldKey },
}),
])
);

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const nonBooleans = fieldAccess.filter(([_, result]) => typeof result !== 'boolean');
if (nonBooleans.length) {
throw accessReturnError(
nonBooleans.map(([fieldKey, result]) => ({
const nonBooleans = [];
const fieldsDenied = [];
const accessErrors = [];
for (const fieldKey of Object.keys(originalInput)) {
let result;
try {
result =
typeof list.fields[fieldKey].access[operation] === 'function'
? await list.fields[fieldKey].access[operation]({ ...args, fieldKey })
: access;
} catch (error: any) {
accessErrors.push({ error, tag: `${args.listKey}.${fieldKey}.access.${args.operation}` });
continue;
}
if (typeof result !== 'boolean') {
nonBooleans.push({
tag: `${args.listKey}.${fieldKey}.access.${args.operation}`,
returned: typeof result,
}))
);
});
} else if (!result) {
fieldsDenied.push(fieldKey);
}
}

if (accessErrors.length) {
throw extensionError('Access control', accessErrors);
}

const fieldsDenied = fieldAccess
// eslint-disable-next-line @typescript-eslint/no-unused-vars
.filter(([_, result]) => !result)
// eslint-disable-next-line @typescript-eslint/no-unused-vars
.map(([fieldKey, _]) => fieldKey);
if (nonBooleans.length) {
throw accessReturnError(nonBooleans);
}

if (fieldsDenied.length) {
throw accessDeniedError(
Expand Down Expand Up @@ -181,7 +200,15 @@ export async function applyAccessControlForCreate(
};

// List level 'item' access control
const result = await access(args);
let result;
try {
result = await access(args);
} catch (error: any) {
throw extensionError('Access control', [
{ error, tag: `${args.listKey}.access.item.${args.operation}` },
]);
}

const resultType = typeof result;

// It's important that we don't cast objects to truthy values, as there's a strong chance that the user
Expand All @@ -204,32 +231,38 @@ export async function applyAccessControlForCreate(
}

// Field level 'item' access control
const fieldAccess = await Promise.all(
Object.keys(originalInput!).map(async fieldKey => [
fieldKey,
await validateFieldAccessControl({
access: list.fields[fieldKey].access[operation],
args: { ...args, fieldKey },
}),
])
);

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const nonBooleans = fieldAccess.filter(([_, result]) => typeof result !== 'boolean');
if (nonBooleans.length) {
throw accessReturnError(
nonBooleans.map(([fieldKey, result]) => ({
// Field level 'item' access control
const nonBooleans = [];
const fieldsDenied = [];
const accessErrors = [];
for (const fieldKey of Object.keys(originalInput)) {
let result;
try {
result =
typeof list.fields[fieldKey].access[operation] === 'function'
? await list.fields[fieldKey].access[operation]({ ...args, fieldKey })
: access;
} catch (error: any) {
accessErrors.push({ error, tag: `${args.listKey}.${fieldKey}.access.${args.operation}` });
continue;
}
if (typeof result !== 'boolean') {
nonBooleans.push({
tag: `${args.listKey}.${fieldKey}.access.${args.operation}`,
returned: typeof result,
}))
);
});
} else if (!result) {
fieldsDenied.push(fieldKey);
}
}

if (accessErrors.length) {
throw extensionError('Access control', accessErrors);
}

const fieldsDenied = fieldAccess
// eslint-disable-next-line @typescript-eslint/no-unused-vars
.filter(([_, result]) => !result)
// eslint-disable-next-line @typescript-eslint/no-unused-vars
.map(([fieldKey, _]) => fieldKey);
if (nonBooleans.length) {
throw accessReturnError(nonBooleans);
}

if (fieldsDenied.length) {
throw accessDeniedError(
Expand Down
4 changes: 2 additions & 2 deletions packages/keystone/src/lib/core/mutations/create-update.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ async function getResolvedData(
fieldKey,
});
} catch (error: any) {
fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}` });
fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.${hookName}` });
}
}
}
Expand All @@ -342,7 +342,7 @@ async function getResolvedData(
try {
resolvedData = (await list.hooks.resolveInput({ ...hookArgs, resolvedData })) as any;
} catch (error: any) {
throw extensionError(hookName, [{ error, tag: list.listKey }]);
throw extensionError(hookName, [{ error, tag: `${list.listKey}.hooks.${hookName}` }]);
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/keystone/src/lib/core/mutations/hooks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export async function runSideEffectOnlyHook<
try {
await field.hooks[hookName]?.({ fieldKey, ...args });
} catch (error: any) {
fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}` });
fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.${hookName}` });
}
}
}
Expand All @@ -49,6 +49,6 @@ export async function runSideEffectOnlyHook<
try {
await list.hooks[hookName]?.(args);
} catch (error: any) {
throw extensionError(hookName, [{ error, tag: list.listKey }]);
throw extensionError(hookName, [{ error, tag: `${list.listKey}.hooks.${hookName}` }]);
}
}
Loading

0 comments on commit 689d8ec

Please sign in to comment.