From cdcadb968f1a5cd43ef870cf293285f3037ab327 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Mon, 14 Feb 2022 11:50:47 +0800 Subject: [PATCH] refactor(core): refactor error handler logic (#220) * refactor(core): refactor error handler logic add oidc and slonik custom error handler * fix(core): fix typo fix type * refactor: align core errors align some old core error definitions --- packages/core/src/app/init.ts | 5 ++ .../core/src/errors/OIDCRequestError/index.ts | 65 -------------- .../core/src/errors/RequestError/index.ts | 3 +- packages/core/src/errors/SlonikError/index.ts | 8 +- packages/core/src/middleware/koa-auth.ts | 2 +- .../core/src/middleware/koa-error-handler.ts | 22 ----- .../src/middleware/koa-oidc-error-handler.ts | 88 +++++++++++++++++++ .../middleware/koa-slonik-error-handler.ts | 50 +++++++++++ packages/core/src/queries/application.ts | 2 +- packages/core/src/queries/passcode.ts | 5 +- packages/core/src/queries/resource.ts | 2 +- packages/core/src/queries/scope.ts | 2 +- packages/core/src/queries/user.ts | 2 +- packages/core/src/routes/session.ts | 4 +- packages/phrases/src/locales/en.ts | 2 +- packages/phrases/src/locales/zh-cn.ts | 2 +- packages/schemas/src/api/error.ts | 1 + 17 files changed, 165 insertions(+), 100 deletions(-) delete mode 100644 packages/core/src/errors/OIDCRequestError/index.ts create mode 100644 packages/core/src/middleware/koa-oidc-error-handler.ts create mode 100644 packages/core/src/middleware/koa-slonik-error-handler.ts diff --git a/packages/core/src/app/init.ts b/packages/core/src/app/init.ts index 989b9655d..d389b7ecc 100644 --- a/packages/core/src/app/init.ts +++ b/packages/core/src/app/init.ts @@ -7,6 +7,8 @@ import koaLogger from 'koa-logger'; import { port } from '@/env/consts'; import koaErrorHandler from '@/middleware/koa-error-handler'; import koaI18next from '@/middleware/koa-i18next'; +import koaOIDCErrorHandler from '@/middleware/koa-oidc-error-handler'; +import koaSlonikErrorHandler from '@/middleware/koa-slonik-error-handler'; import koaUIProxy from '@/middleware/koa-ui-proxy'; import koaUserLog from '@/middleware/koa-user-log'; import initOidc from '@/oidc/init'; @@ -14,6 +16,9 @@ import initRouter from '@/routes/init'; export default async function initApp(app: Koa): Promise { app.use(koaErrorHandler()); + app.use(koaOIDCErrorHandler()); + app.use(koaSlonikErrorHandler()); + // TODO move to specific router (LOG-454) app.use(koaUserLog()); app.use(koaLogger()); diff --git a/packages/core/src/errors/OIDCRequestError/index.ts b/packages/core/src/errors/OIDCRequestError/index.ts deleted file mode 100644 index 2558e70b7..000000000 --- a/packages/core/src/errors/OIDCRequestError/index.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { LogtoErrorCode, LogtoErrorI18nKey } from '@logto/phrases'; -import { RequestErrorBody } from '@logto/schemas'; -import decamelize from 'decamelize'; -import i18next from 'i18next'; -import pick from 'lodash.pick'; -import { errors } from 'oidc-provider'; - -export default class OIDCRequestError extends Error { - code: LogtoErrorCode; - status: number; - expose: boolean; - data: unknown; - - constructor(error: errors.OIDCProviderError) { - const { - status = 400, - message, - error_description, - error_detail, - name, - expose = true, - constructor, - ...interpolation - } = error; - - super(message); - - switch (constructor) { - case errors.InvalidScope: - case errors.InvalidTarget: - case errors.InvalidToken: - case errors.InvalidClientMetadata: - case errors.InvalidGrant: - this.code = `oidc.${decamelize(name)}` as LogtoErrorCode; - this.message = i18next.t(`errors:${this.code}`, interpolation); - break; - case errors.SessionNotFound: - this.code = 'session.not_found'; - this.message = i18next.t(`errors:${this.code}`, interpolation); - break; - case errors.InsufficientScope: - this.code = 'oidc.insufficient_scope'; - this.message = i18next.t(`errors:${this.code}`, { - scopes: error_detail, - }); - break; - default: - this.code = 'oidc.provider_error'; - this.message = i18next.t(`errors:${this.code}`, { - message: this.message, - }); - break; - } - - this.status = status; - this.expose = expose; - - // Original OIDCProvider Error description and details are provided in the data field - this.data = { error_description, error_detail }; - } - - get body(): RequestErrorBody { - return pick(this, 'message', 'code', 'data'); - } -} diff --git a/packages/core/src/errors/RequestError/index.ts b/packages/core/src/errors/RequestError/index.ts index 1a88856f6..b16f837e2 100644 --- a/packages/core/src/errors/RequestError/index.ts +++ b/packages/core/src/errors/RequestError/index.ts @@ -13,13 +13,14 @@ export default class RequestError extends Error { const { code, status = 400, + expose = true, ...interpolation } = typeof input === 'string' ? { code: input } : input; const message = i18next.t(`errors:${code}`, interpolation); super(message); - this.expose = true; + this.expose = expose; this.code = code; this.status = status; this.data = data; diff --git a/packages/core/src/errors/SlonikError/index.ts b/packages/core/src/errors/SlonikError/index.ts index 6c70c334d..20585f937 100644 --- a/packages/core/src/errors/SlonikError/index.ts +++ b/packages/core/src/errors/SlonikError/index.ts @@ -1,7 +1,13 @@ import { SlonikError } from 'slonik'; export class DeletionError extends SlonikError { - public constructor() { + table?: string; + id?: string; + + public constructor(table?: string, id?: string) { super('Resource not found.'); + + this.table = table; + this.id = id; } } diff --git a/packages/core/src/middleware/koa-auth.ts b/packages/core/src/middleware/koa-auth.ts index c163e0893..d683c1078 100644 --- a/packages/core/src/middleware/koa-auth.ts +++ b/packages/core/src/middleware/koa-auth.ts @@ -24,7 +24,7 @@ const extractBearerTokenFromHeaders = ({ authorization }: IncomingHttpHeaders) = assertThat( authorization.startsWith(bearerTokenIdentifier), new RequestError( - { code: 'auth.authorization_type_not_supported', status: 401 }, + { code: 'auth.authorization_token_type_not_supported', status: 401 }, { supportedTypes: [bearerTokenIdentifier] } ) ); diff --git a/packages/core/src/middleware/koa-error-handler.ts b/packages/core/src/middleware/koa-error-handler.ts index 48e740aa0..ea09d1768 100644 --- a/packages/core/src/middleware/koa-error-handler.ts +++ b/packages/core/src/middleware/koa-error-handler.ts @@ -1,9 +1,6 @@ import { RequestErrorBody } from '@logto/schemas'; import { Middleware } from 'koa'; -import { errors } from 'oidc-provider'; -import { NotFoundError } from 'slonik'; -import OIDCRequestError from '@/errors/OIDCRequestError'; import RequestError from '@/errors/RequestError'; export default function koaErrorHandler(): Middleware< @@ -22,25 +19,6 @@ export default function koaErrorHandler(): Middleware< return; } - if (error instanceof errors.OIDCProviderError) { - const oidcError = new OIDCRequestError(error); - ctx.status = oidcError.status; - ctx.body = oidcError.body; - - return; - } - - // TODO: Slonik Error - if (error instanceof NotFoundError) { - const error = new RequestError({ code: 'entity.not_found', status: 404 }); - ctx.status = error.status; - ctx.body = error.body; - - return; - } - - // TODO: Zod Error - throw error; } }; diff --git a/packages/core/src/middleware/koa-oidc-error-handler.ts b/packages/core/src/middleware/koa-oidc-error-handler.ts new file mode 100644 index 000000000..4d000cd55 --- /dev/null +++ b/packages/core/src/middleware/koa-oidc-error-handler.ts @@ -0,0 +1,88 @@ +import { LogtoErrorCode } from '@logto/phrases'; +import decamelize from 'decamelize'; +import { Middleware } from 'koa'; +import { errors } from 'oidc-provider'; + +import RequestError from '@/errors/RequestError'; + +/** + * OIDC Provider Error Definiation: https://github.com/panva/node-oidc-provider/blob/main/lib/helpers/errors.js + */ + +export default function koaOIDCErrorHandler(): Middleware { + return async (ctx, next) => { + try { + await next(); + } catch (error: unknown) { + if (!(error instanceof errors.OIDCProviderError)) { + throw error; + } + + const { + status = 400, + message, + error_description, + error_detail, + name, + expose, + constructor, + ...interpolation + } = error; + + // Original OIDCProvider Error description and details are provided in the data field + const data = { error_description, error_detail }; + + switch (constructor) { + case errors.InvalidScope: + case errors.InvalidTarget: + case errors.InvalidToken: + case errors.InvalidClientMetadata: + case errors.InvalidRedirectUri: + case errors.AccessDenied: + case errors.UnsupportedGrantType: + case errors.UnsupportedResponseMode: + case errors.UnsupportedResponseType: + case errors.InvalidGrant: + throw new RequestError( + { + code: `oidc.${decamelize(name)}` as LogtoErrorCode, + status, + expose, + ...interpolation, + }, + data + ); + case errors.SessionNotFound: + throw new RequestError( + { + code: 'session.not_found', + status, + expose, + ...interpolation, + }, + data + ); + case errors.InsufficientScope: + throw new RequestError( + { + code: 'oidc.insufficient_scope', + status, + expose, + scopes: error_detail, + }, + data + ); + default: + throw new RequestError( + { + code: 'oidc.provider_error', + status, + expose, + message, + }, + data + ); + } + } + }; +} diff --git a/packages/core/src/middleware/koa-slonik-error-handler.ts b/packages/core/src/middleware/koa-slonik-error-handler.ts new file mode 100644 index 000000000..36a7386ef --- /dev/null +++ b/packages/core/src/middleware/koa-slonik-error-handler.ts @@ -0,0 +1,50 @@ +/** + * Slonik Error Types: + * + * BackendTerminatedError, + * CheckIntegrityConstraintViolationError, + * ConnectionError, + * DataIntegrityError, + * ForeignKeyIntegrityConstraintViolationError, + * IntegrityConstraintViolationError, + * InvalidConfigurationError, + * InvalidInputError, + * NotFoundError, + * NotNullIntegrityConstraintViolationError, + * StatementCancelledError, + * StatementTimeoutError, + * UnexpectedStateError, + * UniqueIntegrityConstraintViolationError, + * TupleMovedToAnotherPartitionError + * + * (reference)[https://github.com/gajus/slonik#error-handling] + */ + +import { Middleware } from 'koa'; +import { SlonikError, NotFoundError } from 'slonik'; + +import RequestError from '@/errors/RequestError'; +import { DeletionError } from '@/errors/SlonikError'; + +export default function koaSlonikErrorHandler(): Middleware { + return async (ctx, next) => { + try { + await next(); + } catch (error: unknown) { + if (!(error instanceof SlonikError)) { + throw error; + } + + switch (error.constructor) { + case DeletionError: + case NotFoundError: + throw new RequestError({ + code: 'entity.not_found', + status: 404, + }); + default: + throw error; + } + } + }; +} diff --git a/packages/core/src/queries/application.ts b/packages/core/src/queries/application.ts index ac825aa8f..6c3ec83ff 100644 --- a/packages/core/src/queries/application.ts +++ b/packages/core/src/queries/application.ts @@ -50,6 +50,6 @@ export const deleteApplicationById = async (id: string) => { `); if (rowCount < 1) { - throw new DeletionError(); + throw new DeletionError(Applications.table, id); } }; diff --git a/packages/core/src/queries/passcode.ts b/packages/core/src/queries/passcode.ts index 8d77e2952..0980b1126 100644 --- a/packages/core/src/queries/passcode.ts +++ b/packages/core/src/queries/passcode.ts @@ -36,7 +36,7 @@ export const deletePasscodeById = async (id: string) => { `); if (rowCount < 1) { - throw new DeletionError(); + throw new DeletionError(Passcodes.table, id); } }; @@ -47,6 +47,7 @@ export const deletePasscodesByIds = async (ids: string[]) => { `); if (rowCount !== ids.length) { - throw new DeletionError(); + // TODO: need to track the failed ids + throw new DeletionError(Passcodes.table, `${ids.join(',')}`); } }; diff --git a/packages/core/src/queries/resource.ts b/packages/core/src/queries/resource.ts index 3b92e9669..911deaeaa 100644 --- a/packages/core/src/queries/resource.ts +++ b/packages/core/src/queries/resource.ts @@ -49,6 +49,6 @@ export const deleteResourceById = async (id: string) => { `); if (rowCount < 1) { - throw new DeletionError(); + throw new DeletionError(Resources.table, id); } }; diff --git a/packages/core/src/queries/scope.ts b/packages/core/src/queries/scope.ts index 7bf55e629..cce48f48d 100644 --- a/packages/core/src/queries/scope.ts +++ b/packages/core/src/queries/scope.ts @@ -30,6 +30,6 @@ export const deleteScopeById = async (id: string) => { `); if (rowCount < 1) { - throw new DeletionError(); + throw new DeletionError(ResourceScopes.table, id); } }; diff --git a/packages/core/src/queries/user.ts b/packages/core/src/queries/user.ts index 234354edb..297ed6b7c 100644 --- a/packages/core/src/queries/user.ts +++ b/packages/core/src/queries/user.ts @@ -103,6 +103,6 @@ export const deleteUserById = async (id: string) => { `); if (rowCount < 1) { - throw new DeletionError(); + throw new DeletionError(Users.table, id); } }; diff --git a/packages/core/src/routes/session.ts b/packages/core/src/routes/session.ts index faf3bac3b..aec993aaa 100644 --- a/packages/core/src/routes/session.ts +++ b/packages/core/src/routes/session.ts @@ -1,6 +1,6 @@ import { LogtoErrorCode } from '@logto/phrases'; import { conditional } from '@silverhand/essentials'; -import { Provider } from 'oidc-provider'; +import { Provider, errors } from 'oidc-provider'; import { object, string } from 'zod'; import RequestError from '@/errors/RequestError'; @@ -80,7 +80,7 @@ export default function sessionRoutes(router: T, prov return next(); } - throw new Error(`Prompt not supported: ${name}`); + throw new errors.InvalidRequest(`Prompt not supported: ${name}`); } ); diff --git a/packages/phrases/src/locales/en.ts b/packages/phrases/src/locales/en.ts index 8801fc87e..ef47c5b17 100644 --- a/packages/phrases/src/locales/en.ts +++ b/packages/phrases/src/locales/en.ts @@ -17,7 +17,7 @@ const translation = { const errors = { auth: { authorization_header_missing: 'Authorization header is missing.', - authorization_type_not_supported: 'Authorization type is not supported.', + authorization_token_type_not_supported: 'Authorization type is not supported.', unauthorized: 'Unauthorized. Please check credentils and its scope.', jwt_sub_missing: 'Missing `sub` in JWT.', }, diff --git a/packages/phrases/src/locales/zh-cn.ts b/packages/phrases/src/locales/zh-cn.ts index 1009df765..a9540ee52 100644 --- a/packages/phrases/src/locales/zh-cn.ts +++ b/packages/phrases/src/locales/zh-cn.ts @@ -19,7 +19,7 @@ const translation = { const errors = { auth: { authorization_header_missing: 'Authorization 请求 header 遗漏。', - authorization_type_not_supported: '不支持的 authorization 类型。', + authorization_token_type_not_supported: '不支持的 authorization 类型。', unauthorized: '未授权。请检查相关 credentials 和 scope。', jwt_sub_missing: 'JWT 中找不到 `sub`。', }, diff --git a/packages/schemas/src/api/error.ts b/packages/schemas/src/api/error.ts index 130083c2f..d8ddc0228 100644 --- a/packages/schemas/src/api/error.ts +++ b/packages/schemas/src/api/error.ts @@ -3,6 +3,7 @@ import { LogtoErrorCode } from '@logto/phrases'; export type RequestErrorMetadata = Record & { code: LogtoErrorCode; status?: number; + expose?: boolean; }; export type RequestErrorBody = { message: string; data: unknown; code: LogtoErrorCode };