From 28818db757b35cb4396e52fce131c75abb19276b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=BCseyin=20Berke=20B=C3=BCt=C3=BCn?= <8071263+Zamion101@users.noreply.github.com> Date: Sat, 24 Dec 2022 20:52:26 +0100 Subject: [PATCH] refactor: Prefer use of RequestError and next() function on middlewares Added: - New error types such as `IntegrationNotFoundError`, `WorkspaceNotFoundError`, `AccountNotFoundError' and more. Refactored: - Refactored most of the middlewares and very little number of helper functions to use RequestError - Deleted unused imports Changed: - Some of the error types in middlewares changed to more related error types. - Environment variable of 'VERBOSE_ERROR_OUTPUT' changed to more reliable validation method in `config/index.ts` as per @dangtony98 requested. --- backend/environment.d.ts | 2 +- backend/src/config/index.ts | 2 +- backend/src/helpers/integration.ts | 24 +++++---- backend/src/middleware/requireAuth.ts | 6 +-- backend/src/middleware/requireBotAuth.ts | 5 +- .../src/middleware/requireIntegrationAuth.ts | 7 ++- .../src/middleware/requireOrganizationAuth.ts | 14 ++--- .../src/middleware/requireServiceTokenAuth.ts | 3 +- backend/src/middleware/requireSignupAuth.ts | 3 +- .../src/middleware/requireWorkspaceAuth.ts | 1 - backend/src/middleware/validateRequest.ts | 1 - backend/src/utils/errors.ts | 54 ++++++++++++++++++- 12 files changed, 87 insertions(+), 35 deletions(-) diff --git a/backend/environment.d.ts b/backend/environment.d.ts index e39b4280c..a11cfc4c6 100644 --- a/backend/environment.d.ts +++ b/backend/environment.d.ts @@ -14,7 +14,7 @@ declare global { JWT_SIGNUP_SECRET: string; MONGO_URL: string; NODE_ENV: 'development' | 'staging' | 'testing' | 'production'; - VERBOSE_ERROR_OUTPUT: boolean; + VERBOSE_ERROR_OUTPUT: string; LOKI_HOST: string; CLIENT_ID_HEROKU: string; CLIENT_ID_VERCEL: string; diff --git a/backend/src/config/index.ts b/backend/src/config/index.ts index c8fe63c9a..121a0aad0 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -10,7 +10,7 @@ const JWT_SIGNUP_LIFETIME = process.env.JWT_SIGNUP_LIFETIME! || '15m'; const JWT_SIGNUP_SECRET = process.env.JWT_SIGNUP_SECRET!; const MONGO_URL = process.env.MONGO_URL!; const NODE_ENV = process.env.NODE_ENV! || 'production'; -const VERBOSE_ERROR_OUTPUT = process.env.VERBOSE_ERROR_OUTPUT || false; +const VERBOSE_ERROR_OUTPUT = process.env.VERBOSE_ERROR_OUTPUT! !== 'true' && true; const LOKI_HOST = process.env.LOKI_HOST || undefined; const CLIENT_SECRET_HEROKU = process.env.CLIENT_SECRET_HEROKU!; const CLIENT_ID_HEROKU = process.env.CLIENT_ID_HEROKU!; diff --git a/backend/src/helpers/integration.ts b/backend/src/helpers/integration.ts index a2d95d56a..d92156ece 100644 --- a/backend/src/helpers/integration.ts +++ b/backend/src/helpers/integration.ts @@ -2,18 +2,18 @@ import * as Sentry from '@sentry/node'; import { Bot, Integration, - IIntegration, IntegrationAuth, - IIntegrationAuth } from '../models'; import { exchangeCode, exchangeRefresh, syncSecrets } from '../integrations'; -import { BotService, IntegrationService } from '../services'; +import { BotService } from '../services'; import { ENV_DEV, EVENT_PUSH_SECRETS, INTEGRATION_VERCEL, INTEGRATION_NETLIFY } from '../variables'; +import { UnauthorizedRequestError } from '../utils/errors'; +import RequestError from '../utils/requestError'; interface Update { workspace: string; @@ -176,13 +176,13 @@ const syncIntegrationsHelper = async ({ */ const getIntegrationAuthRefreshHelper = async ({ integrationAuthId }: { integrationAuthId: string }) => { let refreshToken; - //TODO: Refactor code to take advantage of using RequestError. It's possible to create new types of errors for more detailed errors + try { const integrationAuth = await IntegrationAuth .findById(integrationAuthId) .select('+refreshCiphertext +refreshIV +refreshTag'); - if (!integrationAuth) throw new Error('Failed to find integration auth'); + if (!integrationAuth) throw UnauthorizedRequestError({message: 'Failed to locate Integration Authentication credentials'}); refreshToken = await BotService.decryptSymmetric({ workspaceId: integrationAuth.workspace.toString(), @@ -194,7 +194,10 @@ const syncIntegrationsHelper = async ({ } catch (err) { Sentry.setUser(null); Sentry.captureException(err); - throw new Error('Failed to get integration refresh token'); + if(err instanceof RequestError) + throw err + else + throw new Error('Failed to get integration refresh token'); } return refreshToken; @@ -210,13 +213,13 @@ const syncIntegrationsHelper = async ({ */ const getIntegrationAuthAccessHelper = async ({ integrationAuthId }: { integrationAuthId: string }) => { let accessToken; - //TODO: Refactor code to take advantage of using RequestError. It's possible to create new types of errors for more detailed errors + try { const integrationAuth = await IntegrationAuth .findById(integrationAuthId) .select('workspace integration +accessCiphertext +accessIV +accessTag +accessExpiresAt + refreshCiphertext'); - if (!integrationAuth) throw new Error('Failed to find integration auth'); + if (!integrationAuth) throw UnauthorizedRequestError({message: 'Failed to locate Integration Authentication credentials'}); accessToken = await BotService.decryptSymmetric({ workspaceId: integrationAuth.workspace.toString(), @@ -242,7 +245,10 @@ const getIntegrationAuthAccessHelper = async ({ integrationAuthId }: { integrati } catch (err) { Sentry.setUser(null); Sentry.captureException(err); - throw new Error('Failed to get integration access token'); + if(err instanceof RequestError) + throw err + else + throw new Error('Failed to get integration access token'); } return accessToken; diff --git a/backend/src/middleware/requireAuth.ts b/backend/src/middleware/requireAuth.ts index b55ba1486..d917d362a 100644 --- a/backend/src/middleware/requireAuth.ts +++ b/backend/src/middleware/requireAuth.ts @@ -2,7 +2,7 @@ import jwt from 'jsonwebtoken'; import { Request, Response, NextFunction } from 'express'; import { User } from '../models'; import { JWT_AUTH_SECRET } from '../config'; -import { BadRequestError, UnauthorizedRequestError } from '../utils/errors'; +import { AccountNotFoundError, BadRequestError, UnauthorizedRequestError } from '../utils/errors'; declare module 'jsonwebtoken' { export interface UserIDJwtPayload extends jwt.JwtPayload { @@ -22,7 +22,7 @@ const requireAuth = async (req: Request, res: Response, next: NextFunction) => { // JWT authentication middleware const [ AUTH_TOKEN_TYPE, AUTH_TOKEN_VALUE ] = <[string, string]>req.headers['authorization']?.split(' ', 2) ?? [null, null] if(AUTH_TOKEN_TYPE === null) return next(BadRequestError({message: `Missing Authorization Header in the request header.`})) - if(AUTH_TOKEN_TYPE.toLowerCase() !== 'bearer') return next(UnauthorizedRequestError({message: `The provided authentication type '${AUTH_TOKEN_TYPE}' is not supported.`})) + if(AUTH_TOKEN_TYPE.toLowerCase() !== 'bearer') return next(BadRequestError({message: `The provided authentication type '${AUTH_TOKEN_TYPE}' is not supported.`})) if(AUTH_TOKEN_VALUE === null) return next(BadRequestError({message: 'Missing Authorization Body in the request header'})) const decodedToken = ( @@ -33,7 +33,7 @@ const requireAuth = async (req: Request, res: Response, next: NextFunction) => { _id: decodedToken.userId }).select('+publicKey'); - if (!user) return next(UnauthorizedRequestError({message: 'Failed to locate User account'})) + if (!user) return next(AccountNotFoundError({message: 'Failed to locate User account'})) if (!user?.publicKey) return next(UnauthorizedRequestError({message: 'Unable to authenticate due to partially set up account'})) diff --git a/backend/src/middleware/requireBotAuth.ts b/backend/src/middleware/requireBotAuth.ts index a3ef2e715..e39f0d1b5 100644 --- a/backend/src/middleware/requireBotAuth.ts +++ b/backend/src/middleware/requireBotAuth.ts @@ -1,8 +1,7 @@ -import * as Sentry from '@sentry/node'; import { Request, Response, NextFunction } from 'express'; import { Bot } from '../models'; import { validateMembership } from '../helpers/membership'; -import { UnauthorizedRequestError } from '../utils/errors'; +import { AccountNotFoundError } from '../utils/errors'; type req = 'params' | 'body' | 'query'; @@ -19,7 +18,7 @@ const requireBotAuth = ({ const bot = await Bot.findOne({ _id: req[location].botId }); if (!bot) { - return next(UnauthorizedRequestError({message: 'Failed to locate Bot account'})) + return next(AccountNotFoundError({message: 'Failed to locate Bot account'})) } await validateMembership({ diff --git a/backend/src/middleware/requireIntegrationAuth.ts b/backend/src/middleware/requireIntegrationAuth.ts index c7e76af4d..4389028ab 100644 --- a/backend/src/middleware/requireIntegrationAuth.ts +++ b/backend/src/middleware/requireIntegrationAuth.ts @@ -1,9 +1,8 @@ -import * as Sentry from '@sentry/node'; import { Request, Response, NextFunction } from 'express'; -import { Bot, Integration, IntegrationAuth, Membership } from '../models'; +import { Integration, IntegrationAuth } from '../models'; import { IntegrationService } from '../services'; import { validateMembership } from '../helpers/membership'; -import { UnauthorizedRequestError } from '../utils/errors'; +import { IntegrationNotFoundError, UnauthorizedRequestError } from '../utils/errors'; /** * Validate if user on request is a member of workspace with proper roles associated @@ -30,7 +29,7 @@ const requireIntegrationAuth = ({ }); if (!integration) { - return next(UnauthorizedRequestError({message: 'Failed to locate Integration'})) + return next(IntegrationNotFoundError({message: 'Failed to locate Integration'})) } await validateMembership({ diff --git a/backend/src/middleware/requireOrganizationAuth.ts b/backend/src/middleware/requireOrganizationAuth.ts index 0583f0f84..04542b429 100644 --- a/backend/src/middleware/requireOrganizationAuth.ts +++ b/backend/src/middleware/requireOrganizationAuth.ts @@ -1,7 +1,6 @@ -import * as Sentry from '@sentry/node'; import { Request, Response, NextFunction } from 'express'; import { IOrganization, MembershipOrg } from '../models'; -import { UnauthorizedRequestError } from '../utils/errors'; +import { UnauthorizedRequestError, ValidationError } from '../utils/errors'; /** * Validate if user on request is a member with proper roles for organization @@ -26,16 +25,17 @@ const requireOrganizationAuth = ({ organization: req.params.organizationId }).populate<{ organization: IOrganization }>('organization'); - if (!membershipOrg) { - return next(UnauthorizedRequestError({message: 'Failed to locate Organization Membership'})) - } + if (!membershipOrg) { + return next(UnauthorizedRequestError({message: "You're not a member of this Organization."})) + } + //TODO is this important to validate? I mean is it possible to save wrong role to database or get wrong role from databse? - Zamion101 if (!acceptedRoles.includes(membershipOrg.role)) { - return next(UnauthorizedRequestError({message: 'Failed to validate Organization Membership Role'})) + return next(ValidationError({message: 'Failed to validate Organization Membership Role'})) } if (!acceptedStatuses.includes(membershipOrg.status)) { - return next(UnauthorizedRequestError({message: 'Failed to validate Organization Membership Status'})) + return next(ValidationError({message: 'Failed to validate Organization Membership Status'})) } req.membershipOrg = membershipOrg; diff --git a/backend/src/middleware/requireServiceTokenAuth.ts b/backend/src/middleware/requireServiceTokenAuth.ts index 4313d4d04..94e8363ff 100644 --- a/backend/src/middleware/requireServiceTokenAuth.ts +++ b/backend/src/middleware/requireServiceTokenAuth.ts @@ -1,6 +1,5 @@ import jwt from 'jsonwebtoken'; import { Request, Response, NextFunction } from 'express'; -import * as Sentry from '@sentry/node'; import { ServiceToken } from '../models'; import { JWT_SERVICE_SECRET } from '../config'; import { BadRequestError, UnauthorizedRequestError } from '../utils/errors'; @@ -42,7 +41,7 @@ const requireServiceTokenAuth = async ( .populate('user', '+publicKey') .select('+encryptedKey +publicKey +nonce'); - if (!serviceToken) return next(UnauthorizedRequestError({message: 'Failed to locate Service Token'})) + if (!serviceToken) return next(UnauthorizedRequestError({message: 'The service token does not match the record in the database'})) req.serviceToken = serviceToken; return next(); diff --git a/backend/src/middleware/requireSignupAuth.ts b/backend/src/middleware/requireSignupAuth.ts index 8ef71b1e8..3318bd8d3 100644 --- a/backend/src/middleware/requireSignupAuth.ts +++ b/backend/src/middleware/requireSignupAuth.ts @@ -1,6 +1,5 @@ import jwt from 'jsonwebtoken'; import { Request, Response, NextFunction } from 'express'; -import * as Sentry from '@sentry/node'; import { User } from '../models'; import { JWT_SIGNUP_SECRET } from '../config'; import { BadRequestError, UnauthorizedRequestError } from '../utils/errors'; @@ -24,7 +23,7 @@ const requireSignupAuth = async ( const [ AUTH_TOKEN_TYPE, AUTH_TOKEN_VALUE ] = <[string, string]>req.headers['authorization']?.split(' ', 2) ?? [null, null] if(AUTH_TOKEN_TYPE === null) return next(BadRequestError({message: `Missing Authorization Header in the request header.`})) - if(AUTH_TOKEN_TYPE.toLowerCase() !== 'bearer') return next(UnauthorizedRequestError({message: `The provided authentication type '${AUTH_TOKEN_TYPE}' is not supported.`})) + if(AUTH_TOKEN_TYPE.toLowerCase() !== 'bearer') return next(BadRequestError({message: `The provided authentication type '${AUTH_TOKEN_TYPE}' is not supported.`})) if(AUTH_TOKEN_VALUE === null) return next(BadRequestError({message: 'Missing Authorization Body in the request header'})) const decodedToken = ( diff --git a/backend/src/middleware/requireWorkspaceAuth.ts b/backend/src/middleware/requireWorkspaceAuth.ts index fab980508..e5b8898f3 100644 --- a/backend/src/middleware/requireWorkspaceAuth.ts +++ b/backend/src/middleware/requireWorkspaceAuth.ts @@ -1,4 +1,3 @@ -import * as Sentry from '@sentry/node'; import { Request, Response, NextFunction } from 'express'; import { validateMembership } from '../helpers/membership'; import { UnauthorizedRequestError } from '../utils/errors'; diff --git a/backend/src/middleware/validateRequest.ts b/backend/src/middleware/validateRequest.ts index 22ccac92d..484b02cab 100644 --- a/backend/src/middleware/validateRequest.ts +++ b/backend/src/middleware/validateRequest.ts @@ -1,5 +1,4 @@ import { Request, Response, NextFunction } from 'express'; -import * as Sentry from '@sentry/node'; import { validationResult } from 'express-validator'; import { BadRequestError, UnauthorizedRequestError } from '../utils/errors'; diff --git a/backend/src/utils/errors.ts b/backend/src/utils/errors.ts index d9227213a..40c467131 100644 --- a/backend/src/utils/errors.ts +++ b/backend/src/utils/errors.ts @@ -1,5 +1,6 @@ import RequestError, { LogLevel, RequestErrorContext } from "./requestError" +//* ----->[GENERAL HTTP ERRORS]<----- export const RouteNotFoundError = (error?: Partial) => new RequestError({ logLevel: error?.logLevel ?? LogLevel.INFO, statusCode: error?.statusCode ?? 404, @@ -61,4 +62,55 @@ export const ServiceUnavailableError = (error?: Partial) => message: error?.message ?? 'The service is currently unavailable. Please try again later.', context: error?.context, stack: error?.stack -}) \ No newline at end of file +}) + +export const ValidationError = (error?: Partial) => new RequestError({ + logLevel: error?.logLevel ?? LogLevel.ERROR, + statusCode: error?.statusCode ?? 400, + type: error?.type ?? 'validation_error', + message: error?.message ?? 'The request failed validation', + context: error?.context, + stack: error?.stack +}) + +//* ----->[INTEGRATION ERRORS]<----- +export const IntegrationNotFoundError = (error?: Partial) => new RequestError({ + logLevel: error?.logLevel ?? LogLevel.ERROR, + statusCode: error?.statusCode ?? 404, + type: error?.type ?? 'integration_not_found_error', + message: error?.message ?? 'The requested integration was not found', + context: error?.context, + stack: error?.stack +}) + +//* ----->[WORKSPACE ERRORS]<----- +export const WorkspaceNotFoundError = (error?: Partial) => new RequestError({ + logLevel: error?.logLevel ?? LogLevel.ERROR, + statusCode: error?.statusCode ?? 404, + type: error?.type ?? 'workspace_not_found_error', + message: error?.message ?? 'The requested workspace was not found', + context: error?.context, + stack: error?.stack +}) + +//* ----->[ORGANIZATION ERRORS]<----- +export const OrganizationNotFoundError = (error?: Partial) => new RequestError({ + logLevel: error?.logLevel ?? LogLevel.ERROR, + statusCode: error?.statusCode ?? 404, + type: error?.type ?? 'organization_not_found_error', + message: error?.message ?? 'The requested organization was not found', + context: error?.context, + stack: error?.stack +}) + +//* ----->[ACCOUNT ERRORS]<----- +export const AccountNotFoundError = (error?: Partial) => new RequestError({ + logLevel: error?.logLevel ?? LogLevel.ERROR, + statusCode: error?.statusCode ?? 404, + type: error?.type ?? 'account_not_found_error', + message: error?.message ?? 'The requested account was not found', + context: error?.context, + stack: error?.stack +}) + +//* ----->[MISC ERRORS]<-----