From 6f5d7a831b56e2f79d20dc3242458343d264fcea 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: Thu, 22 Dec 2022 01:42:02 +0100 Subject: [PATCH] refactor: Refactored middlewares to use RequestError Refactored middlewares to use RequestError rather than using `try {...}catch (err){...}`. With this change it's possible to manage all error details within one place. Added: - Added Sentry.captureException to Error Handler --- backend/src/app.ts | 6 ++ backend/src/helpers/integration.ts | 2 + backend/src/helpers/membership.ts | 1 + backend/src/integrations/refresh.ts | 1 + backend/src/middleware/requireAuth.ts | 39 +++++----- backend/src/middleware/requireBotAuth.ts | 39 +++++----- .../src/middleware/requireIntegrationAuth.ts | 71 +++++++++---------- .../requireIntegrationAuthorizationAuth.ts | 53 ++++++-------- .../src/middleware/requireOrganizationAuth.ts | 47 ++++++------ .../src/middleware/requireServiceTokenAuth.ts | 41 +++++------ backend/src/middleware/requireSignupAuth.ts | 39 +++++----- .../src/middleware/requireWorkspaceAuth.ts | 7 +- backend/src/middleware/validateRequest.ts | 9 +-- 13 files changed, 156 insertions(+), 199 deletions(-) diff --git a/backend/src/app.ts b/backend/src/app.ts index e22cc41be..c043eb344 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -4,6 +4,7 @@ import helmet from 'helmet'; import cors from 'cors'; import cookieParser from 'cookie-parser'; import dotenv from 'dotenv'; +import * as Sentry from '@sentry/node'; dotenv.config(); import { PORT, NODE_ENV, SITE_URL } from './config'; @@ -60,6 +61,11 @@ app.use((error: RequestError|Error, req: Request, res: Response, next: NextFunct error = InternalServerError({context: {exception: error.message}, stack: error.stack}) getLogger('backend-main').log((error).levelName.toLowerCase(), (error).message) } + //* Sentry Error Capture + if(req.user !== undefined || req.user !== null) + Sentry.setUser({ email: req.user.email }) + Sentry.captureException(error) + res.status((error).statusCode).json((error).format(req)) next() }) diff --git a/backend/src/helpers/integration.ts b/backend/src/helpers/integration.ts index ccfd72a53..a2d95d56a 100644 --- a/backend/src/helpers/integration.ts +++ b/backend/src/helpers/integration.ts @@ -176,6 +176,7 @@ 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) @@ -209,6 +210,7 @@ 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) diff --git a/backend/src/helpers/membership.ts b/backend/src/helpers/membership.ts index 1ff542f2a..b06460cde 100644 --- a/backend/src/helpers/membership.ts +++ b/backend/src/helpers/membership.ts @@ -21,6 +21,7 @@ const validateMembership = async ({ }) => { let membership; + //TODO: Refactor code to take advantage of using RequestError. It's possible to create new types of errors for more detailed errors try { membership = await Membership.findOne({ user: userId, diff --git a/backend/src/integrations/refresh.ts b/backend/src/integrations/refresh.ts index 16870944d..bf636c4fc 100644 --- a/backend/src/integrations/refresh.ts +++ b/backend/src/integrations/refresh.ts @@ -53,6 +53,7 @@ const exchangeRefreshHeroku = async ({ refreshToken: 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 res = await axios.post( INTEGRATION_HEROKU_TOKEN_URL, diff --git a/backend/src/middleware/requireAuth.ts b/backend/src/middleware/requireAuth.ts index a6bd79073..b55ba1486 100644 --- a/backend/src/middleware/requireAuth.ts +++ b/backend/src/middleware/requireAuth.ts @@ -1,8 +1,8 @@ import jwt from 'jsonwebtoken'; import { Request, Response, NextFunction } from 'express'; -import * as Sentry from '@sentry/node'; import { User } from '../models'; import { JWT_AUTH_SECRET } from '../config'; +import { BadRequestError, UnauthorizedRequestError } from '../utils/errors'; declare module 'jsonwebtoken' { export interface UserIDJwtPayload extends jwt.JwtPayload { @@ -20,32 +20,25 @@ declare module 'jsonwebtoken' { */ const requireAuth = async (req: Request, res: Response, next: NextFunction) => { // JWT authentication middleware - try { - if (!req.headers?.authorization) - throw new Error('Failed to locate authorization header'); + 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_VALUE === null) return next(BadRequestError({message: 'Missing Authorization Body in the request header'})) - const token = req.headers.authorization.split(' ')[1]; - const decodedToken = ( - jwt.verify(token, JWT_AUTH_SECRET) - ); + const decodedToken = ( + jwt.verify(AUTH_TOKEN_VALUE, JWT_AUTH_SECRET) + ); - const user = await User.findOne({ - _id: decodedToken.userId - }).select('+publicKey'); + const user = await User.findOne({ + _id: decodedToken.userId + }).select('+publicKey'); - if (!user) throw new Error('Failed to authenticate unfound user'); - if (!user?.publicKey) - throw new Error('Failed to authenticate not fully set up account'); + if (!user) return next(UnauthorizedRequestError({message: 'Failed to locate User account'})) + if (!user?.publicKey) + return next(UnauthorizedRequestError({message: 'Unable to authenticate due to partially set up account'})) - req.user = user; - return next(); - } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: 'Failed to authenticate user. Try logging in' - }); - } + req.user = user; + return next(); }; export default requireAuth; diff --git a/backend/src/middleware/requireBotAuth.ts b/backend/src/middleware/requireBotAuth.ts index 6c5a3820a..a3ef2e715 100644 --- a/backend/src/middleware/requireBotAuth.ts +++ b/backend/src/middleware/requireBotAuth.ts @@ -2,6 +2,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'; type req = 'params' | 'body' | 'query'; @@ -15,30 +16,22 @@ const requireBotAuth = ({ location?: req; }) => { return async (req: Request, res: Response, next: NextFunction) => { - try { - const bot = await Bot.findOne({ _id: req[location].botId }); - - if (!bot) { - throw new Error('Failed to find bot'); - } - - await validateMembership({ - userId: req.user._id.toString(), - workspaceId: bot.workspace.toString(), - acceptedRoles, - acceptedStatuses - }); - - req.bot = bot; - - next(); - } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: 'Failed bot authorization' - }); + const bot = await Bot.findOne({ _id: req[location].botId }); + + if (!bot) { + return next(UnauthorizedRequestError({message: 'Failed to locate Bot account'})) } + + await validateMembership({ + userId: req.user._id.toString(), + workspaceId: bot.workspace.toString(), + acceptedRoles, + acceptedStatuses + }); + + req.bot = bot; + + next(); } } diff --git a/backend/src/middleware/requireIntegrationAuth.ts b/backend/src/middleware/requireIntegrationAuth.ts index fe653dbc0..c7e76af4d 100644 --- a/backend/src/middleware/requireIntegrationAuth.ts +++ b/backend/src/middleware/requireIntegrationAuth.ts @@ -3,6 +3,7 @@ import { Request, Response, NextFunction } from 'express'; import { Bot, Integration, IntegrationAuth, Membership } from '../models'; import { IntegrationService } from '../services'; import { validateMembership } from '../helpers/membership'; +import { UnauthorizedRequestError } from '../utils/errors'; /** * Validate if user on request is a member of workspace with proper roles associated @@ -21,48 +22,40 @@ const requireIntegrationAuth = ({ return async (req: Request, res: Response, next: NextFunction) => { // integration authorization middleware - try { - const { integrationId } = req.params; + const { integrationId } = req.params; - // validate integration accessibility - const integration = await Integration.findOne({ - _id: integrationId - }); + // validate integration accessibility + const integration = await Integration.findOne({ + _id: integrationId + }); - if (!integration) { - throw new Error('Failed to find integration'); - } - - await validateMembership({ - userId: req.user._id.toString(), - workspaceId: integration.workspace.toString(), - acceptedRoles, - acceptedStatuses - }); - - const integrationAuth = await IntegrationAuth.findOne({ - _id: integration.integrationAuth - }).select( - '+refreshCiphertext +refreshIV +refreshTag +accessCiphertext +accessIV +accessTag +accessExpiresAt' - ); - - if (!integrationAuth) { - throw new Error('Failed to find integration authorization'); - } - - req.integration = integration; - req.accessToken = await IntegrationService.getIntegrationAuthAccess({ - integrationAuthId: integrationAuth._id.toString() - }); - - return next(); - } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: 'Failed integration authorization' - }); + if (!integration) { + return next(UnauthorizedRequestError({message: 'Failed to locate Integration'})) } + + await validateMembership({ + userId: req.user._id.toString(), + workspaceId: integration.workspace.toString(), + acceptedRoles, + acceptedStatuses + }); + + const integrationAuth = await IntegrationAuth.findOne({ + _id: integration.integrationAuth + }).select( + '+refreshCiphertext +refreshIV +refreshTag +accessCiphertext +accessIV +accessTag +accessExpiresAt' + ); + + if (!integrationAuth) { + return next(UnauthorizedRequestError({message: 'Failed to locate Integration Authentication credentials'})) + } + + req.integration = integration; + req.accessToken = await IntegrationService.getIntegrationAuthAccess({ + integrationAuthId: integrationAuth._id.toString() + }); + + return next(); }; }; diff --git a/backend/src/middleware/requireIntegrationAuthorizationAuth.ts b/backend/src/middleware/requireIntegrationAuthorizationAuth.ts index ed44ffec5..278716e60 100644 --- a/backend/src/middleware/requireIntegrationAuthorizationAuth.ts +++ b/backend/src/middleware/requireIntegrationAuthorizationAuth.ts @@ -3,6 +3,7 @@ import { Request, Response, NextFunction } from 'express'; import { IntegrationAuth } from '../models'; import { IntegrationService } from '../services'; import { validateMembership } from '../helpers/membership'; +import { UnauthorizedRequestError } from '../utils/errors'; /** * Validate if user on request is a member of workspace with proper roles associated @@ -22,41 +23,33 @@ const requireIntegrationAuthorizationAuth = ({ attachAccessToken?: boolean; }) => { return async (req: Request, res: Response, next: NextFunction) => { - try { - const { integrationAuthId } = req.params; + const { integrationAuthId } = req.params; - const integrationAuth = await IntegrationAuth.findOne({ - _id: integrationAuthId - }).select( - '+refreshCiphertext +refreshIV +refreshTag +accessCiphertext +accessIV +accessTag +accessExpiresAt' - ); + const integrationAuth = await IntegrationAuth.findOne({ + _id: integrationAuthId + }).select( + '+refreshCiphertext +refreshIV +refreshTag +accessCiphertext +accessIV +accessTag +accessExpiresAt' + ); - if (!integrationAuth) { - throw new Error('Failed to find integration authorization'); - } - - await validateMembership({ - userId: req.user._id.toString(), - workspaceId: integrationAuth.workspace.toString(), - acceptedRoles, - acceptedStatuses - }); + if (!integrationAuth) { + return next(UnauthorizedRequestError({message: 'Failed to locate Integration Authorization credentials'})) + } + + await validateMembership({ + userId: req.user._id.toString(), + workspaceId: integrationAuth.workspace.toString(), + acceptedRoles, + acceptedStatuses + }); - req.integrationAuth = integrationAuth; - if (attachAccessToken) { - req.accessToken = await IntegrationService.getIntegrationAuthAccess({ - integrationAuthId: integrationAuth._id.toString() - }); - } - - return next(); - } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: 'Failed (authorization) integration authorizationt' + req.integrationAuth = integrationAuth; + if (attachAccessToken) { + req.accessToken = await IntegrationService.getIntegrationAuthAccess({ + integrationAuthId: integrationAuth._id.toString() }); } + + return next(); }; }; diff --git a/backend/src/middleware/requireOrganizationAuth.ts b/backend/src/middleware/requireOrganizationAuth.ts index c77bd312e..0583f0f84 100644 --- a/backend/src/middleware/requireOrganizationAuth.ts +++ b/backend/src/middleware/requireOrganizationAuth.ts @@ -1,6 +1,7 @@ import * as Sentry from '@sentry/node'; import { Request, Response, NextFunction } from 'express'; import { IOrganization, MembershipOrg } from '../models'; +import { UnauthorizedRequestError } from '../utils/errors'; /** * Validate if user on request is a member with proper roles for organization @@ -19,35 +20,27 @@ const requireOrganizationAuth = ({ return async (req: Request, res: Response, next: NextFunction) => { // organization authorization middleware - try { - // validate organization membership - const membershipOrg = await MembershipOrg.findOne({ - user: req.user._id, - organization: req.params.organizationId - }).populate<{ organization: IOrganization }>('organization'); + // validate organization membership + const membershipOrg = await MembershipOrg.findOne({ + user: req.user._id, + organization: req.params.organizationId + }).populate<{ organization: IOrganization }>('organization'); - if (!membershipOrg) { - throw new Error('Failed to find organization membership'); - } - - if (!acceptedRoles.includes(membershipOrg.role)) { - throw new Error('Failed to validate organization membership role'); - } - - if (!acceptedStatuses.includes(membershipOrg.status)) { - throw new Error('Failed to validate organization membership status'); - } - - req.membershipOrg = membershipOrg; - - return next(); - } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: 'Failed organization authorization' - }); + if (!membershipOrg) { + return next(UnauthorizedRequestError({message: 'Failed to locate Organization Membership'})) } + + if (!acceptedRoles.includes(membershipOrg.role)) { + return next(UnauthorizedRequestError({message: 'Failed to validate Organization Membership Role'})) + } + + if (!acceptedStatuses.includes(membershipOrg.status)) { + return next(UnauthorizedRequestError({message: 'Failed to validate Organization Membership Status'})) + } + + req.membershipOrg = membershipOrg; + + return next(); }; }; diff --git a/backend/src/middleware/requireServiceTokenAuth.ts b/backend/src/middleware/requireServiceTokenAuth.ts index d403e8fe5..4313d4d04 100644 --- a/backend/src/middleware/requireServiceTokenAuth.ts +++ b/backend/src/middleware/requireServiceTokenAuth.ts @@ -3,6 +3,7 @@ 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'; declare module 'jsonwebtoken' { export interface UserIDJwtPayload extends jwt.JwtPayload { @@ -24,33 +25,27 @@ const requireServiceTokenAuth = async ( next: NextFunction ) => { // JWT service token middleware - try { - if (!req.headers?.authorization) - throw new Error('Failed to locate authorization header'); + + 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.`})) + //TODO: Determine what is the actual Token Type for Service Token Authentication (ex. Bearer) + //if(AUTH_TOKEN_TYPE.toLowerCase() !== 'bearer') return next(UnauthorizedRequestError({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 token = req.headers.authorization.split(' ')[1]; + const decodedToken = ( + jwt.verify(AUTH_TOKEN_VALUE, JWT_SERVICE_SECRET) + ); - const decodedToken = ( - jwt.verify(token, JWT_SERVICE_SECRET) - ); + const serviceToken = await ServiceToken.findOne({ + _id: decodedToken.serviceTokenId + }) + .populate('user', '+publicKey') + .select('+encryptedKey +publicKey +nonce'); - const serviceToken = await ServiceToken.findOne({ - _id: decodedToken.serviceTokenId - }) - .populate('user', '+publicKey') - .select('+encryptedKey +publicKey +nonce'); + if (!serviceToken) return next(UnauthorizedRequestError({message: 'Failed to locate Service Token'})) - if (!serviceToken) throw new Error('Failed to find service token'); - - req.serviceToken = serviceToken; - return next(); - } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: 'Failed to authenticate service token' - }); - } + req.serviceToken = serviceToken; + return next(); }; export default requireServiceTokenAuth; diff --git a/backend/src/middleware/requireSignupAuth.ts b/backend/src/middleware/requireSignupAuth.ts index 9384387ee..8ef71b1e8 100644 --- a/backend/src/middleware/requireSignupAuth.ts +++ b/backend/src/middleware/requireSignupAuth.ts @@ -3,6 +3,7 @@ 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'; declare module 'jsonwebtoken' { export interface UserIDJwtPayload extends jwt.JwtPayload { @@ -21,32 +22,24 @@ const requireSignupAuth = async ( ) => { // JWT (temporary) authentication middleware for complete signup - try { - if (!req.headers?.authorization) - throw new Error('Failed to locate authorization header'); + 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_VALUE === null) return next(BadRequestError({message: 'Missing Authorization Body in the request header'})) + + const decodedToken = ( + jwt.verify(AUTH_TOKEN_VALUE, JWT_SIGNUP_SECRET) + ); - const token = req.headers.authorization.split(' ')[1]; - const decodedToken = ( - jwt.verify(token, JWT_SIGNUP_SECRET) - ); + const user = await User.findOne({ + _id: decodedToken.userId + }).select('+publicKey'); - const user = await User.findOne({ - _id: decodedToken.userId - }).select('+publicKey'); + if (!user) + return next(UnauthorizedRequestError({message: 'Unable to authenticate for User account completion. Try logging in again'})) - if (!user) - throw new Error('Failed to temporarily authenticate unfound user'); - - req.user = user; - return next(); - } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: - 'Failed to temporarily authenticate user for complete account. Try logging in' - }); - } + req.user = user; + return next(); }; export default requireSignupAuth; diff --git a/backend/src/middleware/requireWorkspaceAuth.ts b/backend/src/middleware/requireWorkspaceAuth.ts index a27611f2d..fab980508 100644 --- a/backend/src/middleware/requireWorkspaceAuth.ts +++ b/backend/src/middleware/requireWorkspaceAuth.ts @@ -1,6 +1,7 @@ import * as Sentry from '@sentry/node'; import { Request, Response, NextFunction } from 'express'; import { validateMembership } from '../helpers/membership'; +import { UnauthorizedRequestError } from '../utils/errors'; type req = 'params' | 'body' | 'query'; @@ -36,11 +37,7 @@ const requireWorkspaceAuth = ({ return next(); } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: 'Failed workspace authorization' - }); + return next(UnauthorizedRequestError({message: 'Unable to authenticate workspace'})) } }; }; diff --git a/backend/src/middleware/validateRequest.ts b/backend/src/middleware/validateRequest.ts index 4dcafc13b..22ccac92d 100644 --- a/backend/src/middleware/validateRequest.ts +++ b/backend/src/middleware/validateRequest.ts @@ -1,6 +1,7 @@ import { Request, Response, NextFunction } from 'express'; import * as Sentry from '@sentry/node'; import { validationResult } from 'express-validator'; +import { BadRequestError, UnauthorizedRequestError } from '../utils/errors'; /** * Validate intended inputs on [req] via express-validator @@ -15,16 +16,12 @@ const validate = (req: Request, res: Response, next: NextFunction) => { try { const errors = validationResult(req); if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); + return next(BadRequestError({context: {errors: errors.array}})) } return next(); } catch (err) { - Sentry.setUser(null); - Sentry.captureException(err); - return res.status(401).send({ - error: "Looks like you're unauthenticated . Try logging in" - }); + return next(UnauthorizedRequestError({message: 'Unauthenticated requests are not allowed. Try logging in'})) } };