From f1b8d71cba854847290f2caddf257ac0cbf14c5b Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 10 Apr 2018 16:51:07 -0600 Subject: [PATCH] Refactored Errors --- bin/cli-setup | 12 +- errors.js | 369 +++++++++++------- graph/errorHandler.js | 4 +- graph/loaders/assets.js | 6 +- graph/mutators/action.js | 10 +- graph/mutators/asset.js | 10 +- graph/mutators/comment.js | 8 +- graph/mutators/settings.js | 4 +- graph/mutators/tag.js | 6 +- graph/mutators/token.js | 6 +- graph/mutators/user.js | 22 +- jobs/mailer.js | 4 +- middleware/authorization.js | 2 +- plugin-api/beta/server/getReactionConfig.js | 6 +- plugins/talk-plugin-akismet/errors.js | 14 +- plugins/talk-plugin-akismet/index.js | 2 +- .../server/mutators.js | 5 +- .../server/errors.js | 14 +- .../server/hooks.js | 2 +- routes/api/v1/users.js | 15 +- routes/index.js | 12 +- serve.js | 24 +- services/assets.js | 15 +- services/comments.js | 23 +- services/limit.js | 4 +- services/moderation/index.js | 6 +- services/moderation/phases/commentLength.js | 2 +- services/passport.js | 21 +- services/settings.js | 4 +- services/setup.js | 29 +- services/tags.js | 8 +- services/users.js | 69 ++-- services/wordlist.js | 8 +- test/server/graph/context.js | 4 +- test/server/services/wordlist.js | 5 +- 35 files changed, 436 insertions(+), 319 deletions(-) diff --git a/bin/cli-setup b/bin/cli-setup index 572e8c7fc..11da478c2 100755 --- a/bin/cli-setup +++ b/bin/cli-setup @@ -14,7 +14,7 @@ const SettingsService = require('../services/settings'); const SetupService = require('../services/setup'); const UsersService = require('../services/users'); const MigrationService = require('../services/migration'); -const errors = require('../errors'); +const { ErrSettingsInit, ErrSettingsNotInit } = require('../errors'); const Context = require('../graph/context'); // Register the shutdown criteria. @@ -41,13 +41,15 @@ const performSetup = async () => { // We should NOT have gotten a settings object, this means that the // application is already setup. Error out here. - throw errors.ErrSettingsInit; - } catch (e) { + throw new ErrSettingsInit(); + } catch (err) { // If the error is `not init`, then we're good, otherwise, it's something // else. - if (e !== errors.ErrSettingsNotInit) { - throw e; + if (err instanceof ErrSettingsNotInit) { + return; } + + throw err; } if (program.defaults) { diff --git a/errors.js b/errors.js index 53e8bf351..37f42f4ff 100644 --- a/errors.js +++ b/errors.js @@ -10,21 +10,21 @@ class ExtendableError { } /** - * APIError is the base error that all application issued errors originate, they - * are composed of data used by the front end and backend to handle errors + * TalkError is the base error that all application issued errors originate, + * they are composed of data used by the front end and backend to handle errors * consistently. */ -class APIError extends ExtendableError { +class TalkError extends ExtendableError { constructor( message, - { status = 500, translation_key = null }, + { status = 500, translation_key = null } = {}, metadata = {} ) { super(message); - this.status = status; - this.translation_key = translation_key; - this.metadata = metadata; + this.status = status || 500; + this.translation_key = translation_key || null; + this.metadata = metadata || {}; } toJSON() { @@ -38,85 +38,114 @@ class APIError extends ExtendableError { } // ErrPasswordTooShort is returned when the password length is too short. -const ErrPasswordTooShort = new APIError( - 'password must be at least 8 characters', - { - status: 400, - translation_key: 'PASSWORD_LENGTH', +class ErrPasswordTooShort extends TalkError { + constructor() { + super('password must be at least 8 characters', { + status: 400, + translation_key: 'PASSWORD_LENGTH', + }); } -); +} -const ErrMissingEmail = new APIError('email is required', { - translation_key: 'EMAIL_REQUIRED', - status: 400, -}); - -const ErrMissingPassword = new APIError('password is required', { - translation_key: 'PASSWORD_REQUIRED', - status: 400, -}); - -const ErrEmailTaken = new APIError('Email address already in use', { - translation_key: 'EMAIL_IN_USE', - status: 400, -}); - -const ErrUsernameTaken = new APIError('Username already in use', { - translation_key: 'USERNAME_IN_USE', - status: 400, -}); - -const ErrSameUsernameProvided = new APIError( - 'Username provided for change is the same as current', - { - translation_key: 'SAME_USERNAME_PROVIDED', - status: 400, +class ErrMissingEmail extends TalkError { + constructor() { + super('email is required', { + translation_key: 'EMAIL_REQUIRED', + status: 400, + }); } -); +} -const ErrSpecialChars = new APIError( - 'No special characters are allowed in a username', - { - translation_key: 'NO_SPECIAL_CHARACTERS', - status: 400, +class ErrMissingPassword extends TalkError { + constructor() { + super('password is required', { + translation_key: 'PASSWORD_REQUIRED', + status: 400, + }); } -); +} -const ErrMissingUsername = new APIError( - 'A username is required to create a user', - { - translation_key: 'USERNAME_REQUIRED', - status: 400, +class ErrEmailTaken extends TalkError { + constructor() { + super('Email address already in use', { + translation_key: 'EMAIL_IN_USE', + status: 400, + }); } -); +} + +class ErrUsernameTaken extends TalkError { + constructor() { + super('Username already in use', { + translation_key: 'USERNAME_IN_USE', + status: 400, + }); + } +} + +class ErrSameUsernameProvided extends TalkError { + constructor() { + super('Username provided for change is the same as current', { + translation_key: 'SAME_USERNAME_PROVIDED', + status: 400, + }); + } +} + +class ErrSpecialChars extends TalkError { + constructor() { + super('No special characters are allowed in a username', { + translation_key: 'NO_SPECIAL_CHARACTERS', + status: 400, + }); + } +} + +class ErrMissingUsername extends TalkError { + constructor() { + super('A username is required to create a user', { + translation_key: 'USERNAME_REQUIRED', + status: 400, + }); + } +} // ErrEmailVerificationToken is returned in the event that the password reset is requested // without a token. -const ErrEmailVerificationToken = new APIError('token is required', { - translation_key: 'EMAIL_VERIFICATION_TOKEN_INVALID', - status: 400, -}); +class ErrEmailVerificationToken extends TalkError { + constructor() { + super('token is required', { + translation_key: 'EMAIL_VERIFICATION_TOKEN_INVALID', + status: 400, + }); + } +} // ErrEmailAlreadyVerified is returned when the user tries to verify an email // address that has already been verified. -const ErrEmailAlreadyVerified = new APIError( - 'email address is already verified', - { - translation_key: 'EMAIL_ALREADY_VERIFIED', - status: 409, +class ErrEmailAlreadyVerified extends TalkError { + constructor() { + super('email address is already verified', { + translation_key: 'EMAIL_ALREADY_VERIFIED', + status: 409, + }); } -); +} // ErrPasswordResetToken is returned in the event that the password reset is requested // without a token. -const ErrPasswordResetToken = new APIError('token is required', { - translation_key: 'PASSWORD_RESET_TOKEN_INVALID', - status: 400, -}); +class ErrPasswordResetToken extends TalkError { + constructor() { + super('token is required', { + translation_key: 'PASSWORD_RESET_TOKEN_INVALID', + status: 400, + }); + } +} // ErrAssetCommentingClosed is returned when a comment or action is attempted on // a stream where commenting has been closed. -class ErrAssetCommentingClosed extends APIError { +class ErrAssetCommentingClosed extends TalkError { constructor(closedMessage = null) { super( 'asset commenting is closed', @@ -136,7 +165,7 @@ class ErrAssetCommentingClosed extends APIError { * ErrAuthentication is returned when there is an error authenticating and the * message is provided. */ -class ErrAuthentication extends APIError { +class ErrAuthentication extends TalkError { constructor(message = null) { super( 'authentication error occurred', @@ -154,7 +183,7 @@ class ErrAuthentication extends APIError { /** * ErrAlreadyExists is returned when an attempt to create a resource failed due to an existing one. */ -class ErrAlreadyExists extends APIError { +class ErrAlreadyExists extends TalkError { constructor(existing = null) { super( 'resource already exists', @@ -171,121 +200,179 @@ class ErrAlreadyExists extends APIError { // ErrContainsProfanity is returned in the event that the middleware detects // profanity/banned/suspect words in the payload. -const ErrContainsProfanity = new APIError( - 'This username contains elements which are not permitted in our community. If you think this is in error, please contact us or try again.', - { - translation_key: 'PROFANITY_ERROR', - status: 400, +class ErrContainsProfanity extends TalkError { + constructor(phrase) { + super( + 'This username contains elements which are not permitted in our community. If you think this is in error, please contact us or try again.', + { + translation_key: 'PROFANITY_ERROR', + status: 400, + }, + { phrase } + ); } -); +} -const ErrNotFound = new APIError('not found', { - translation_key: 'NOT_FOUND', - status: 404, -}); +class ErrNotFound extends TalkError { + constructor() { + super('not found', { + translation_key: 'NOT_FOUND', + status: 404, + }); + } +} -const ErrInvalidAssetURL = new APIError('asset_url is invalid', { - translation_key: 'INVALID_ASSET_URL', - status: 400, -}); +class ErrInvalidAssetURL extends TalkError { + constructor() { + super('asset_url is invalid', { + translation_key: 'INVALID_ASSET_URL', + status: 400, + }); + } +} // ErrNotAuthorized is an error that is returned in the event an operation is // deemed not authorized. -const ErrNotAuthorized = new APIError('not authorized', { - translation_key: 'NOT_AUTHORIZED', - status: 401, -}); +class ErrNotAuthorized extends TalkError { + constructor() { + super('not authorized', { + translation_key: 'NOT_AUTHORIZED', + status: 401, + }); + } +} // ErrSettingsNotInit is returned when the settings are required but not // initialized. -const ErrSettingsNotInit = new Error( - 'Talk is currently not setup. Please proceed to our web installer at $ROOT_URL/admin/install or run ./bin/cli-setup. Visit https://docs.coralproject.net/talk/ for more information on installation and configuration instructions' -); +class ErrSettingsNotInit extends TalkError { + constructor() { + super( + 'Talk is currently not setup. Please proceed to our web installer at $ROOT_URL/admin/install or run ./bin/cli-setup. Visit https://docs.coralproject.net/talk/ for more information on installation and configuration instructions' + ); + } +} // ErrSettingsInit is returned when the setup endpoint is hit and we are already // initialized. -const ErrSettingsInit = new APIError('settings are already initialized', { - status: 500, -}); +class ErrSettingsInit extends TalkError { + constructor() { + super('settings are already initialized', { + status: 500, + }); + } +} // ErrInstallLock is returned when the setup endpoint is hit and the install // lock is present. -const ErrInstallLock = new APIError('install lock active', { - status: 500, -}); +class ErrInstallLock extends TalkError { + constructor() { + super('install lock active', { + status: 500, + }); + } +} // ErrPermissionUpdateUsername is returned when the user does not have permission to update their username. -const ErrPermissionUpdateUsername = new APIError( - 'You do not have permission to update your username.', - { - translation_key: 'EDIT_USERNAME_NOT_AUTHORIZED', - status: 403, +class ErrPermissionUpdateUsername extends TalkError { + constructor() { + super('You do not have permission to update your username.', { + translation_key: 'EDIT_USERNAME_NOT_AUTHORIZED', + status: 403, + }); } -); +} // ErrLoginAttemptMaximumExceeded is returned when the login maximum is exceeded. -const ErrLoginAttemptMaximumExceeded = new APIError( - 'You have made too many incorrect password attempts.', - { - translation_key: 'LOGIN_MAXIMUM_EXCEEDED', - status: 429, +class ErrLoginAttemptMaximumExceeded extends TalkError { + constructor() { + super('You have made too many incorrect password attempts.', { + translation_key: 'LOGIN_MAXIMUM_EXCEEDED', + status: 429, + }); } -); +} // ErrEditWindowHasEnded is returned when the edit window has expired. -const ErrEditWindowHasEnded = new APIError('Edit window is over', { - translation_key: 'EDIT_WINDOW_ENDED', - status: 403, -}); +class ErrEditWindowHasEnded extends TalkError { + constructor() { + super('Edit window is over', { + translation_key: 'EDIT_WINDOW_ENDED', + status: 403, + }); + } +} // ErrCommentTooShort is returned when the comment is too short. -const ErrCommentTooShort = new APIError('Comment was too short', { - translation_key: 'COMMENT_TOO_SHORT', - status: 400, -}); +class ErrCommentTooShort extends TalkError { + constructor(length) { + super( + 'Comment was too short', + { + translation_key: 'COMMENT_TOO_SHORT', + status: 400, + }, + { length } + ); + } +} // ErrAssetURLAlreadyExists is returned when a rename operation is requested // but an asset already exists with the new url. -const ErrAssetURLAlreadyExists = new APIError( - 'Asset URL already exists, cannot rename', - { - translation_key: 'ASSET_URL_ALREADY_EXISTS', - status: 409, +class ErrAssetURLAlreadyExists extends TalkError { + constructor() { + super('Asset URL already exists, cannot rename', { + translation_key: 'ASSET_URL_ALREADY_EXISTS', + status: 409, + }); } -); +} // ErrNotVerified is returned when a user tries to login with valid credentials // but their email address is not yet verified. -const ErrNotVerified = new APIError( - 'User does not have a verified email address', - { - translation_key: 'EMAIL_NOT_VERIFIED', - status: 401, +class ErrNotVerified extends TalkError { + constructor() { + super('User does not have a verified email address', { + translation_key: 'EMAIL_NOT_VERIFIED', + status: 401, + }); } -); +} -const ErrMaxRateLimit = new APIError('Rate limit exceeded', { - translation_key: 'RATE_LIMIT_EXCEEDED', - status: 429, -}); +class ErrMaxRateLimit extends TalkError { + constructor(max, tries) { + super( + 'Rate limit exceeded', + { + translation_key: 'RATE_LIMIT_EXCEEDED', + status: 429, + }, + { tries, max } + ); + } +} // ErrCannotIgnoreStaff is returned when a user tries to ignore a staff member. -const ErrCannotIgnoreStaff = new APIError('Cannot ignore staff members.', { - translation_key: 'CANNOT_IGNORE_STAFF', - status: 400, -}); +class ErrCannotIgnoreStaff extends TalkError { + constructor() { + super('Cannot ignore staff members.', { + translation_key: 'CANNOT_IGNORE_STAFF', + status: 400, + }); + } +} // ErrParentDoesNotVisible is returned when the user tries to reply to a comment // that isn't visible. -const ErrParentDoesNotVisible = new APIError( - 'Cannot reply to a comment that is not visible', - { - translation_key: 'COMMENT_PARENT_NOT_VISIBLE', +class ErrParentDoesNotVisible extends TalkError { + constructor() { + super('Cannot reply to a comment that is not visible', { + translation_key: 'COMMENT_PARENT_NOT_VISIBLE', + }); } -); +} module.exports = { - APIError, + TalkError, ErrAlreadyExists, ErrAssetCommentingClosed, ErrAssetURLAlreadyExists, diff --git a/graph/errorHandler.js b/graph/errorHandler.js index 89cd5d8a2..9e8d98f74 100644 --- a/graph/errorHandler.js +++ b/graph/errorHandler.js @@ -1,6 +1,6 @@ const { forEachField } = require('./utils'); const { maskErrors } = require('graphql-errors'); -const errors = require('../errors'); +const { TalkError } = require('../errors'); const { Error: { ValidationError } } = require('mongoose'); // If an APIError happens in a mutation, then respond with `{errors: Array}` @@ -11,7 +11,7 @@ const decorateWithMutationErrorHandler = field => { try { return await fieldResolver(obj, args, ctx, info); } catch (err) { - if (err instanceof errors.APIError) { + if (err instanceof TalkError) { return { errors: [err], }; diff --git a/graph/loaders/assets.js b/graph/loaders/assets.js index 2f0595e81..40305760c 100644 --- a/graph/loaders/assets.js +++ b/graph/loaders/assets.js @@ -57,7 +57,7 @@ const findOrCreateAssetByURL = async (ctx, url) => { try { new URL(url); } catch (err) { - throw ErrInvalidAssetURL; + throw new ErrInvalidAssetURL(url); } // Try the easy lookup first. @@ -76,7 +76,7 @@ const findOrCreateAssetByURL = async (ctx, url) => { // If the domain wasn't whitelisted, then we shouldn't create this asset! if (!whitelisted) { - throw ErrInvalidAssetURL; + throw new ErrInvalidAssetURL(url); } // Construct the update operator that we'll use to create the asset. @@ -135,7 +135,7 @@ const findByUrl = async ( try { new URL(asset_url); } catch (err) { - throw errors.ErrInvalidAssetURL; + throw new errors.ErrInvalidAssetURL(asset_url); } return Assets.findByUrl(asset_url); diff --git a/graph/mutators/action.js b/graph/mutators/action.js index 20f9db4fe..d759449b0 100644 --- a/graph/mutators/action.js +++ b/graph/mutators/action.js @@ -1,4 +1,4 @@ -const errors = require('../../errors'); +const { ErrNotFound, ErrNotAuthorized } = require('../../errors'); const { CREATE_ACTION, DELETE_ACTION } = require('../../perms/constants'); const { IGNORE_FLAGS_AGAINST_STAFF } = require('../../config'); @@ -40,7 +40,7 @@ const createAction = async ( // Gets the item referenced by the action. const item = await getActionItem(ctx, { item_id, item_type }); if (!item || item === null) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } // If we are ignoring flags against staff, ensure that the target isn't a @@ -59,7 +59,7 @@ const createAction = async ( // The item is a user, and this is a flag. Check to see if they are staff, // if they are, don't permit the flag. if (item.isStaff()) { - throw errors.ErrNotAuthorized; + throw new ErrNotAuthorized(); } } @@ -108,8 +108,8 @@ const deleteAction = (ctx, { id }) => { module.exports = ctx => { let mutators = { Action: { - create: () => Promise.reject(errors.ErrNotAuthorized), - delete: () => Promise.reject(errors.ErrNotAuthorized), + create: () => Promise.reject(new ErrNotAuthorized()), + delete: () => Promise.reject(new ErrNotAuthorized()), }, }; diff --git a/graph/mutators/asset.js b/graph/mutators/asset.js index 2997e7cd4..d5b72102d 100644 --- a/graph/mutators/asset.js +++ b/graph/mutators/asset.js @@ -1,4 +1,4 @@ -const errors = require('../../errors'); +const { ErrNotAuthorized } = require('../../errors'); const { UPDATE_ASSET_SETTINGS, UPDATE_ASSET_STATUS, @@ -71,10 +71,10 @@ const scrapeAsset = async (ctx, id) => { module.exports = ctx => { let mutators = { Asset: { - updateSettings: () => Promise.reject(errors.ErrNotAuthorized), - updateStatus: () => Promise.reject(errors.ErrNotAuthorized), - closeNow: () => Promise.reject(errors.ErrNotAuthorized), - scrape: () => Promise.reject(errors.ErrNotAuthorized), + updateSettings: () => Promise.reject(new ErrNotAuthorized()), + updateStatus: () => Promise.reject(new ErrNotAuthorized()), + closeNow: () => Promise.reject(new ErrNotAuthorized()), + scrape: () => Promise.reject(new ErrNotAuthorized()), }, }; diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index f63951812..b57aedeb3 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -1,4 +1,4 @@ -const errors = require('../../errors'); +const { ErrNotAuthorized } = require('../../errors'); const ActionModel = require('../../models/action'); const ActionsService = require('../../services/actions'); const TagsService = require('../../services/tags'); @@ -312,9 +312,9 @@ const editComment = async ( module.exports = ctx => { let mutators = { Comment: { - create: () => Promise.reject(errors.ErrNotAuthorized), - setStatus: () => Promise.reject(errors.ErrNotAuthorized), - edit: () => Promise.reject(errors.ErrNotAuthorized), + create: () => Promise.reject(new ErrNotAuthorized()), + setStatus: () => Promise.reject(new ErrNotAuthorized()), + edit: () => Promise.reject(new ErrNotAuthorized()), }, }; diff --git a/graph/mutators/settings.js b/graph/mutators/settings.js index 639b0619b..d9c04cea0 100644 --- a/graph/mutators/settings.js +++ b/graph/mutators/settings.js @@ -1,4 +1,4 @@ -const errors = require('../../errors'); +const { ErrNotAuthorized } = require('../../errors'); const { UPDATE_SETTINGS } = require('../../perms/constants'); @@ -9,7 +9,7 @@ const update = async (ctx, settings) => SettingsService.update(settings); module.exports = ctx => { let mutators = { Settings: { - update: () => Promise.reject(errors.ErrNotAuthorized), + update: () => Promise.reject(new ErrNotAuthorized()), }, }; diff --git a/graph/mutators/tag.js b/graph/mutators/tag.js index c6d8d4c40..d78b020d9 100644 --- a/graph/mutators/tag.js +++ b/graph/mutators/tag.js @@ -1,5 +1,5 @@ const TagsService = require('../../services/tags'); -const errors = require('../../errors'); +const { ErrNotAuthorized } = require('../../errors'); const { ADD_COMMENT_TAG, REMOVE_COMMENT_TAG, @@ -31,8 +31,8 @@ const modify = async ( module.exports = context => { let mutators = { Tag: { - add: () => Promise.reject(errors.ErrNotAuthorized), - remove: () => Promise.reject(errors.ErrNotAuthorized), + add: () => Promise.reject(new ErrNotAuthorized()), + remove: () => Promise.reject(new ErrNotAuthorized()), }, }; diff --git a/graph/mutators/token.js b/graph/mutators/token.js index 5883a2f09..c4d9a1121 100644 --- a/graph/mutators/token.js +++ b/graph/mutators/token.js @@ -1,4 +1,4 @@ -const errors = require('../../errors'); +const { ErrNotAuthorized } = require('../../errors'); const TokensService = require('../../services/tokens'); const { CREATE_TOKEN, REVOKE_TOKEN } = require('../../perms/constants'); @@ -21,8 +21,8 @@ const revokeToken = async ({ user }, { id }) => { module.exports = context => { let mutators = { Token: { - create: () => Promise.reject(errors.ErrNotAuthorized), - revoke: () => Promise.reject(errors.ErrNotAuthorized), + create: () => Promise.reject(new ErrNotAuthorized()), + revoke: () => Promise.reject(new ErrNotAuthorized()), }, }; diff --git a/graph/mutators/user.js b/graph/mutators/user.js index 9d31e6dbd..e0312533f 100644 --- a/graph/mutators/user.js +++ b/graph/mutators/user.js @@ -1,4 +1,4 @@ -const errors = require('../../errors'); +const { ErrNotFound, ErrNotAuthorized } = require('../../errors'); const UsersService = require('../../services/users'); const migrationHelpers = require('../../services/migration/helpers'); const { @@ -92,7 +92,7 @@ const delUser = async (ctx, id) => { // Find the user we're removing. const user = await User.findOne({ id }); if (!user) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } // Get the query transformer we'll use to help batch process the user @@ -156,15 +156,15 @@ const delUser = async (ctx, id) => { module.exports = ctx => { let mutators = { User: { - changeUsername: () => Promise.reject(errors.ErrNotAuthorized), - ignoreUser: () => Promise.reject(errors.ErrNotAuthorized), - setRole: () => Promise.reject(errors.ErrNotAuthorized), - setUserBanStatus: () => Promise.reject(errors.ErrNotAuthorized), - setUserSuspensionStatus: () => Promise.reject(errors.ErrNotAuthorized), - setUserUsernameStatus: () => Promise.reject(errors.ErrNotAuthorized), - setUsername: () => Promise.reject(errors.ErrNotAuthorized), - stopIgnoringUser: () => Promise.reject(errors.ErrNotAuthorized), - del: () => Promise.reject(errors.ErrNotAuthorized), + changeUsername: () => Promise.reject(new ErrNotAuthorized()), + ignoreUser: () => Promise.reject(new ErrNotAuthorized()), + setRole: () => Promise.reject(new ErrNotAuthorized()), + setUserBanStatus: () => Promise.reject(new ErrNotAuthorized()), + setUserSuspensionStatus: () => Promise.reject(new ErrNotAuthorized()), + setUserUsernameStatus: () => Promise.reject(new ErrNotAuthorized()), + setUsername: () => Promise.reject(new ErrNotAuthorized()), + stopIgnoringUser: () => Promise.reject(new ErrNotAuthorized()), + del: () => Promise.reject(new ErrNotAuthorized()), }, }; diff --git a/jobs/mailer.js b/jobs/mailer.js index aa3afa585..a3b21f565 100644 --- a/jobs/mailer.js +++ b/jobs/mailer.js @@ -4,7 +4,6 @@ const { createLogger } = require('../services/logging'); const logger = createLogger('jobs:mailer'); const Context = require('../graph/context'); const { get } = require('lodash'); - const { SMTP_HOST, SMTP_USERNAME, @@ -12,6 +11,7 @@ const { SMTP_PASSWORD, SMTP_FROM_ADDRESS, } = require('../config'); +const { ErrMissingEmail } = require('../errors'); // parseSMTPPort will return the port for SMTP. const parseSMTPPort = () => { @@ -99,7 +99,7 @@ const getEmailAddress = async ({ email, user }) => { const email = get(data, 'user.email'); if (!email) { - throw errors.ErrMissingEmail; + throw new ErrMissingEmail(); } return email; diff --git a/middleware/authorization.js b/middleware/authorization.js index 97003d970..77376f08b 100644 --- a/middleware/authorization.js +++ b/middleware/authorization.js @@ -7,7 +7,7 @@ const authorization = (module.exports = { }); const debug = require('debug')('talk:middleware:authorization'); -const ErrNotAuthorized = require('../errors').ErrNotAuthorized; +const { ErrNotAuthorized } = require('../errors'); /** * has returns true if the user has at least one of the roles specified, diff --git a/plugin-api/beta/server/getReactionConfig.js b/plugin-api/beta/server/getReactionConfig.js index 3baf65806..ffb1a3c07 100644 --- a/plugin-api/beta/server/getReactionConfig.js +++ b/plugin-api/beta/server/getReactionConfig.js @@ -1,5 +1,5 @@ const { SEARCH_OTHER_USERS } = require('../../../perms/constants'); -const errors = require('../../../errors'); +const { ErrNotFound, ErrAlreadyExists } = require('../../../errors'); const pluralize = require('pluralize'); const sc = require('snake-case'); const CommentModel = require('../../../models/comment'); @@ -192,7 +192,7 @@ function getReactionConfig(reaction) { ) => { const comment = await Comments.get.load(item_id); if (!comment) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } try { @@ -211,7 +211,7 @@ function getReactionConfig(reaction) { [reaction]: action, }; } catch (err) { - if (err instanceof errors.ErrAlreadyExists) { + if (err instanceof ErrAlreadyExists) { return err.metadata.existing; } diff --git a/plugins/talk-plugin-akismet/errors.js b/plugins/talk-plugin-akismet/errors.js index b93d178b9..458242ca5 100644 --- a/plugins/talk-plugin-akismet/errors.js +++ b/plugins/talk-plugin-akismet/errors.js @@ -1,12 +1,16 @@ -const { APIError } = require('errors'); +const { TalkError } = require('errors'); // ErrSpam is sent during a `CreateComment` mutation where // `input.checkSpam` is set to true and the comment contains // detected spam as determined by the akismet service. -const ErrSpam = new APIError('Comment is spam', { - status: 400, - translation_key: 'COMMENT_IS_SPAM', -}); +class ErrSpam extends TalkError { + constructor() { + super('Comment is spam', { + status: 400, + translation_key: 'COMMENT_IS_SPAM', + }); + } +} module.exports = { ErrSpam, diff --git a/plugins/talk-plugin-akismet/index.js b/plugins/talk-plugin-akismet/index.js index 4b0e7f997..c57f5da5d 100644 --- a/plugins/talk-plugin-akismet/index.js +++ b/plugins/talk-plugin-akismet/index.js @@ -100,7 +100,7 @@ module.exports = { if (spam) { if (input.checkSpam) { - throw ErrSpam; + throw new ErrSpam(); } // Attach reason information for the flag being added. diff --git a/plugins/talk-plugin-notifications/server/mutators.js b/plugins/talk-plugin-notifications/server/mutators.js index 5c0db9c8d..46954faa5 100644 --- a/plugins/talk-plugin-notifications/server/mutators.js +++ b/plugins/talk-plugin-notifications/server/mutators.js @@ -29,10 +29,11 @@ async function updateNotificationSettings(ctx, settings) { } module.exports = ctx => { + const { connectors: { errors: ErrNotAuthorized } } = ctx; + let mutators = { User: { - updateNotificationSettings: () => - Promise.reject(ctx.connectors.errors.ErrNotAuthorized), + updateNotificationSettings: () => Promise.reject(new ErrNotAuthorized()), }, }; diff --git a/plugins/talk-plugin-toxic-comments/server/errors.js b/plugins/talk-plugin-toxic-comments/server/errors.js index 60135a8f8..a60bd549b 100644 --- a/plugins/talk-plugin-toxic-comments/server/errors.js +++ b/plugins/talk-plugin-toxic-comments/server/errors.js @@ -1,12 +1,16 @@ -const { APIError } = require('errors'); +const { TalkError } = require('errors'); // ErrToxic is sent during a `CreateComment` mutation where // `input.checkToxicity` is set to true and the comment contains // toxic language as determined by the perspective service. -const ErrToxic = new APIError('Comment is toxic', { - status: 400, - translation_key: 'COMMENT_IS_TOXIC', -}); +class ErrToxic extends TalkError { + constructor() { + super('Comment is toxic', { + status: 400, + translation_key: 'COMMENT_IS_TOXIC', + }); + } +} module.exports = { ErrToxic, diff --git a/plugins/talk-plugin-toxic-comments/server/hooks.js b/plugins/talk-plugin-toxic-comments/server/hooks.js index 7b9c93dad..0be363ea1 100644 --- a/plugins/talk-plugin-toxic-comments/server/hooks.js +++ b/plugins/talk-plugin-toxic-comments/server/hooks.js @@ -27,7 +27,7 @@ module.exports = { if (isToxic(scores)) { if (input.checkToxicity) { - throw ErrToxic; + throw new ErrToxic(); } input.status = 'SYSTEM_WITHHELD'; diff --git a/routes/api/v1/users.js b/routes/api/v1/users.js index e0de3b5db..481e5650c 100644 --- a/routes/api/v1/users.js +++ b/routes/api/v1/users.js @@ -1,7 +1,7 @@ const express = require('express'); const router = express.Router(); const UsersService = require('../../../services/users'); -const errors = require('../../../errors'); +const { ErrMissingEmail, ErrNotFound } = require('../../../errors'); const authorization = require('../../../middleware/authorization'); const Limit = require('../../../services/limit'); @@ -40,17 +40,12 @@ router.post('/resend-verify', async (req, res, next) => { // Clean up and validate the email. email = email.toLowerCase().trim(); if (email.length < 5) { - return next(errors.ErrMissingEmail); + return next(new ErrMissingEmail()); } // Check if we're past the rate limit, if we are, stop now. Otherwise, record // this as an attempt to send a verification email. try { - const tries = await resendRateLimiter.get(email); - if (tries > 0) { - throw errors.ErrMaxRateLimit; - } - await resendRateLimiter.test(email); } catch (err) { return next(err); @@ -59,7 +54,7 @@ router.post('/resend-verify', async (req, res, next) => { try { const user = await UsersService.findLocalUser(email); if (!user) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } await UsersService.sendEmailConfirmation(user, email, redirectUri); @@ -81,13 +76,13 @@ router.post( try { let user = await UsersService.findById(user_id); if (!user) { - return next(errors.ErrNotFound); + return next(new ErrNotFound()); } // Find the first local profile. const email = user.firstEmail; if (!email) { - return next(errors.ErrMissingEmail); + return next(new ErrMissingEmail()); } // Send the email to the first local profile that was found. diff --git a/routes/index.js b/routes/index.js index 489a5e8b5..bbdeb8928 100644 --- a/routes/index.js +++ b/routes/index.js @@ -2,7 +2,7 @@ const SetupService = require('../services/setup'); const authentication = require('../middleware/authentication'); const cookieParser = require('cookie-parser'); const enabled = require('debug').enabled; -const errors = require('../errors'); +const { TalkError, ErrNotFound } = require('../errors'); const express = require('express'); const i18n = require('../middleware/i18n'); const path = require('path'); @@ -149,19 +149,19 @@ router.use(require('./plugins')); // Catch 404 and forward to error handler. router.use((req, res, next) => { - next(errors.ErrNotFound); + next(new ErrNotFound()); }); // General API error handler. Respond with the message and error if we have it // while returning a status code that makes sense. router.use('/api', (err, req, res, next) => { - if (err !== errors.ErrNotFound) { + if (!(err instanceof ErrNotFound)) { if (process.env.NODE_ENV !== 'test' || enabled('talk:errors')) { console.error(err); } } - if (err instanceof errors.APIError) { + if (err instanceof TalkError) { res.status(err.status).json({ message: res.locals.t(`error.${err.translation_key}`), error: err, @@ -172,11 +172,11 @@ router.use('/api', (err, req, res, next) => { }); router.use('/', (err, req, res, next) => { - if (err !== errors.ErrNotFound) { + if (!(err instanceof ErrNotFound)) { console.error(err); } - if (err instanceof errors.APIError) { + if (err instanceof TalkError) { res.status(err.status); res.render('error', { message: res.locals.t(`error.${err.translation_key}`), diff --git a/serve.js b/serve.js index ea06a6342..8b4d4556c 100644 --- a/serve.js +++ b/serve.js @@ -1,5 +1,5 @@ const app = require('./app'); -const errors = require('./errors'); +const { ErrSettingsInit, ErrInstallLock } = require('./errors'); const { createServer } = require('http'); const jobs = require('./jobs'); const MigrationService = require('./services/migration'); @@ -95,20 +95,16 @@ async function serve({ await SetupService.isAvailable(); logger.info('Setup is currently available, migrations not being checked'); - } catch (e) { + } catch (err) { // Check the error. - switch (e) { - case errors.ErrInstallLock: - case errors.ErrSettingsInit: - logger.info( - 'Setup is not currently available, migrations now being checked' - ); - - // The error was expected, just continue. - break; - default: - // The error was not expected, throw the error! - throw e; + if (err instanceof ErrInstallLock || err instanceof ErrSettingsInit) { + // The error was expected, just continue. + logger.info( + 'Setup is not currently available, migrations now being checked' + ); + } else { + // The error was not expected, throw the error! + throw err; } // Now try and check the migration status. diff --git a/services/assets.js b/services/assets.js index f229bb353..fbed22287 100644 --- a/services/assets.js +++ b/services/assets.js @@ -2,9 +2,12 @@ const CommentModel = require('../models/comment'); const AssetModel = require('../models/asset'); const SettingsService = require('./settings'); const DomainList = require('./domain_list'); -const errors = require('../errors'); -const merge = require('lodash/merge'); -const isEmpty = require('lodash/isEmpty'); +const { + ErrAssetURLAlreadyExists, + ErrNotFound, + ErrInvalidAssetURL, +} = require('../errors'); +const { merge, isEmpty } = require('lodash'); const { dotize } = require('./utils'); module.exports = class AssetsService { @@ -73,7 +76,7 @@ module.exports = class AssetsService { } if (!whitelisted) { - return Promise.reject(errors.ErrInvalidAssetURL); + throw new ErrInvalidAssetURL(url); } else { return AssetModel.findOneAndUpdate({ url }, update, { // Ensure that if it's new, we return the new object created. @@ -211,7 +214,7 @@ module.exports = class AssetsService { // Try to see if an asset already exists with the given url. let asset = await AssetsService.findByUrl(url); if (asset !== null) { - throw errors.ErrAssetURLAlreadyExists; + throw new ErrAssetURLAlreadyExists(); } // Seems that there was no other asset with the same url, try and perform @@ -227,7 +230,7 @@ module.exports = class AssetsService { dstAssetID, ]); if (!srcAsset || !dstAsset) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } // Resolve the merge operation, this invloves moving all resources attached diff --git a/services/comments.js b/services/comments.js index a1f59c1e1..b9d73d91c 100644 --- a/services/comments.js +++ b/services/comments.js @@ -2,10 +2,13 @@ const CommentModel = require('../models/comment'); const { dotize } = require('./utils'); const debug = require('debug')('talk:services:comments'); const SettingsService = require('./settings'); - -const cloneDeep = require('lodash/cloneDeep'); -const errors = require('../errors'); -const merge = require('lodash/merge'); +const { merge, cloneDeep } = require('lodash'); +const { + ErrParentDoesNotVisible, + ErrNotFound, + ErrNotAuthorized, + ErrEditWindowHasEnded, +} = require('../errors'); const incrReplyCount = async (comment, value) => { try { @@ -40,7 +43,7 @@ module.exports = { if (parent_id !== null) { const parent = await CommentModel.findOne({ id: parent_id }); if (parent === null || !parent.visible) { - throw errors.ErrParentDoesNotVisible; + throw new ErrParentDoesNotVisible(); } } @@ -126,7 +129,7 @@ module.exports = { const comment = await CommentModel.findOne({ id }); if (comment == null) { debug('rejecting comment edit because comment was not found'); - throw errors.ErrNotFound; + throw new ErrNotFound(); } // Check to see if the user was't allowed to edit it. @@ -134,7 +137,7 @@ module.exports = { debug( 'rejecting comment edit because author id does not match editing user' ); - throw errors.ErrNotAuthorized; + throw new ErrNotAuthorized(); } // Check to see if the comment had a status that was editable. @@ -142,13 +145,13 @@ module.exports = { debug( 'rejecting comment edit because original comment has a non-editable status' ); - throw errors.ErrNotAuthorized; + throw new ErrNotAuthorized(); } // Check to see if the edit window expired. if (comment.created_at <= lastEditableCommentCreatedAt) { debug('rejecting comment edit because outside edit time window'); - throw errors.ErrEditWindowHasEnded; + throw new ErrEditWindowHasEnded(); } throw new Error('comment edit failed for an unexpected reason'); @@ -198,7 +201,7 @@ module.exports = { ); if (originalComment == null) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } const editedComment = new CommentModel(originalComment.toObject()); diff --git a/services/limit.js b/services/limit.js index 6d46f3715..573d7a815 100644 --- a/services/limit.js +++ b/services/limit.js @@ -1,5 +1,5 @@ const ms = require('ms'); -const errors = require('../errors'); +const { ErrMaxRateLimit } = require('../errors'); const { createClientFactory } = require('./redis'); const client = createClientFactory(); @@ -60,7 +60,7 @@ class Limit { } if (tries > this.max) { - throw errors.ErrMaxRateLimit; + throw new ErrMaxRateLimit(this.max, tries); } return tries; diff --git a/services/moderation/index.js b/services/moderation/index.js index 4d87e190f..530d6f2a6 100644 --- a/services/moderation/index.js +++ b/services/moderation/index.js @@ -1,4 +1,4 @@ -const errors = require('../../errors'); +const { ErrNotFound } = require('../../errors'); const get = require('lodash/get'); // Load in the phases to use. @@ -92,14 +92,14 @@ const fetchOptions = async (ctx, comment) => { const assetID = get(comment, 'asset_id', null); if (assetID === null) { // And leave now if this asset wasn't found. - throw errors.ErrNotFound; + throw new ErrNotFound(); } // Load the asset. const asset = await Assets.getByID.load(assetID); if (!asset) { // And leave now if this asset wasn't found. - throw errors.ErrNotFound; + throw new ErrNotFound(); } // Combine the asset and the settings to get the asset settings. diff --git a/services/moderation/phases/commentLength.js b/services/moderation/phases/commentLength.js index 925115326..e19198ef1 100644 --- a/services/moderation/phases/commentLength.js +++ b/services/moderation/phases/commentLength.js @@ -8,7 +8,7 @@ module.exports = ( ) => { // Check to see if the body is too short, if it is, then complain about it! if (comment.body.length < 2) { - throw ErrCommentTooShort; + throw new ErrCommentTooShort(comment.body.length); } // Reject if the comment is too long diff --git a/services/passport.js b/services/passport.js index 5748c911b..0ae06afb8 100644 --- a/services/passport.js +++ b/services/passport.js @@ -6,7 +6,12 @@ const TokensService = require('./tokens'); const fetch = require('node-fetch'); const FormData = require('form-data'); const LocalStrategy = require('passport-local').Strategy; -const errors = require('../errors'); +const { + ErrLoginAttemptMaximumExceeded, + ErrNotAuthorized, + ErrAuthentication, + ErrNotVerified, +} = require('../errors'); const uuid = require('uuid'); const debug = require('debug')('talk:services:passport'); const bowser = require('bowser'); @@ -75,7 +80,7 @@ const HandleGenerateCredentials = (req, res, next) => (err, user) => { } if (!user) { - return next(errors.ErrNotAuthorized); + return next(new ErrNotAuthorized()); } // Generate the token to re-issue to the frontend. @@ -117,7 +122,7 @@ const HandleAuthPopupCallback = (req, res, next) => (err, user) => { if (!user) { return res.render('auth-callback', { - auth: { err: errors.ErrNotAuthorized, data: null }, + auth: { err: new ErrNotAuthorized(), data: null }, }); } @@ -143,7 +148,7 @@ async function ValidateUserLogin(loginProfile, user, done) { } if (user.disabled) { - return done(new errors.ErrAuthentication('Account disabled')); + return done(new ErrAuthentication('Account disabled')); } // If the user isn't a local user (i.e., a social user). @@ -169,7 +174,7 @@ async function ValidateUserLogin(loginProfile, user, done) { // If the profile doesn't have a metadata field, or it does not have a // confirmed_at field, or that field is null, then send them back. if (_.get(profile, 'metadata.confirmed_at', null) === null) { - return done(errors.ErrNotVerified); + return done(new ErrNotVerified()); } } @@ -209,7 +214,7 @@ const checkGeneralTokenBlacklist = jwt => .get(`jtir[${jwt.jti}]`) .then(expiry => { if (expiry != null) { - throw new errors.ErrAuthentication('token was revoked'); + throw new ErrAuthentication('token was revoked'); } }); @@ -392,7 +397,7 @@ const HandleFailedAttempt = async (email, userNeedsRecaptcha) => { await UsersService.recordLoginAttempt(email); } catch (err) { if ( - err === errors.ErrLoginAttemptMaximumExceeded && + err instanceof ErrLoginAttemptMaximumExceeded && !userNeedsRecaptcha && RECAPTCHA_ENABLED ) { @@ -448,7 +453,7 @@ passport.use( try { await UsersService.checkLoginAttempts(email); } catch (err) { - if (err === errors.ErrLoginAttemptMaximumExceeded) { + if (err instanceof ErrLoginAttemptMaximumExceeded) { // This says, we didn't have a recaptcha, yet we needed one.. Reject // here. diff --git a/services/settings.js b/services/settings.js index f918f9d01..ee5125183 100644 --- a/services/settings.js +++ b/services/settings.js @@ -1,6 +1,6 @@ const SettingModel = require('../models/setting'); const cache = require('./cache'); -const errors = require('../errors'); +const { ErrSettingsNotInit } = require('../errors'); const { dotize } = require('./utils'); const { SETTINGS_CACHE_TIME } = require('../config'); @@ -17,7 +17,7 @@ const retrieve = async fields => { settings = await SettingModel.findOne(selector); } if (!settings) { - throw errors.ErrSettingsNotInit; + throw new ErrSettingsNotInit(); } return settings; diff --git a/services/setup.js b/services/setup.js index 84f915ff2..2517d376b 100644 --- a/services/setup.js +++ b/services/setup.js @@ -2,7 +2,12 @@ const UsersService = require('./users'); const SettingsService = require('./settings'); const MigrationService = require('./migration'); const SettingsModel = require('../models/setting'); -const errors = require('../errors'); +const { + ErrMissingEmail, + ErrInstallLock, + ErrSettingsInit, + ErrSettingsNotInit, +} = require('../errors'); const { INSTALL_LOCK } = require('../config'); /** @@ -16,25 +21,25 @@ module.exports = class SetupService { static async isAvailable() { // Check if we have an install lock present. if (INSTALL_LOCK) { - throw errors.ErrInstallLock; + throw new ErrInstallLock(); } try { - // Get the current settings, we are expecing an error here. + // Get the current settings, we are expecting an error here. await SettingsService.retrieve(); // We should NOT have gotten a settings object, this means that the // application is already setup. Error out here. - throw errors.ErrSettingsInit; - } catch (e) { - // If the error is `not init`, then we're good, otherwise, it's something - // else. - if (e !== errors.ErrSettingsNotInit) { - throw e; + throw new ErrSettingsInit(); + } catch (err) { + // Allow the request to keep going here. + if (err instanceof ErrSettingsNotInit) { + return; } - // Allow the request to keep going here. - return; + // If the error is `not init`, then we're good, otherwise, it's something + // else. + throw err; } } @@ -44,7 +49,7 @@ module.exports = class SetupService { static validate({ settings, user: { email, username, password } }) { // Verify the email address of the user. if (!email) { - return Promise.reject(errors.ErrMissingEmail); + throw new ErrMissingEmail(); } // Create a settings model to use for validation. diff --git a/services/tags.js b/services/tags.js index 8183d0165..cc8934e0a 100644 --- a/services/tags.js +++ b/services/tags.js @@ -1,12 +1,10 @@ const CommentModel = require('../models/comment'); const AssetModel = require('../models/asset'); const UserModel = require('../models/user'); - const AssetsService = require('./assets'); const SettingsService = require('./settings'); const { ADD_COMMENT_TAG } = require('../perms/constants'); - -const errors = require('../errors'); +const { ErrNotAuthorized } = require('../errors'); const updateModel = async (item_type, query, update) => { // Get the model to update with. @@ -120,13 +118,13 @@ class TagsService { return { tagLink, ownership: true }; } - throw errors.ErrNotAuthorized; + throw new ErrNotAuthorized(); } // Only admin/moderators can modify unique tags, these are tags that are not // in the global list. if (!user.can(ADD_COMMENT_TAG)) { - throw errors.ErrNotAuthorized; + throw new ErrNotAuthorized(); } // Generate the tag in the event now that we have to create the tag for this diff --git a/services/users.js b/services/users.js index 0617d5158..d2dd60b0b 100644 --- a/services/users.js +++ b/services/users.js @@ -1,6 +1,21 @@ const uuid = require('uuid'); const bcrypt = require('bcryptjs'); -const errors = require('../errors'); +const { + ErrMaxRateLimit, + ErrLoginAttemptMaximumExceeded, + ErrNotFound, + ErrPermissionUpdateUsername, + ErrSameUsernameProvided, + ErrUsernameTaken, + ErrMissingUsername, + ErrSpecialChars, + ErrMissingPassword, + ErrPasswordTooShort, + ErrMissingEmail, + ErrEmailTaken, + ErrEmailAlreadyVerified, + ErrCannotIgnoreStaff, +} = require('../errors'); const { difference, sample, some, merge, random } = require('lodash'); const { ROOT_URL } = require('../config'); const { jwt: JWT_SECRET } = require('../secrets'); @@ -59,8 +74,8 @@ class UsersService { try { await loginRateLimiter.test(email.toLowerCase().trim()); } catch (err) { - if (err === errors.ErrMaxRateLimit) { - throw errors.ErrLoginAttemptMaximumExceeded; + if (err instanceof ErrMaxRateLimit) { + throw new ErrLoginAttemptMaximumExceeded(); } throw err; @@ -91,7 +106,7 @@ class UsersService { if (user === null) { user = await UserModel.findOne({ id }); if (user === null) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } // Date comparisons are difficult when using MongoDB. Javascript will @@ -150,10 +165,10 @@ class UsersService { runValidators: true, } ); - if (user === null) { + if (!user) { user = await UserModel.findOne({ id }); - if (user === null) { - throw errors.ErrNotFound; + if (!user) { + throw new ErrNotFound(); } if (user.status.banned.status === status) { @@ -204,7 +219,7 @@ class UsersService { if (user === null) { user = await UserModel.findOne({ id }); if (user === null) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } if (user.status.username.status === status) { @@ -259,15 +274,15 @@ class UsersService { if (!user) { user = await UsersService.findById(id); if (user === null) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } if (user.status.username.status !== fromStatus) { - throw errors.ErrPermissionUpdateUsername; + throw new ErrPermissionUpdateUsername(); } if (!resetAllowed && user.username === username) { - throw errors.ErrSameUsernameProvided; + throw new ErrSameUsernameProvided(); } throw new Error('edit username failed for an unexpected reason'); @@ -276,7 +291,7 @@ class UsersService { return user; } catch (err) { if (err.code === 11000) { - throw errors.ErrUsernameTaken; + throw new ErrUsernameTaken(); } throw err; @@ -317,7 +332,7 @@ class UsersService { } if (attempts >= RECAPTCHA_INCORRECT_TRIGGER) { - throw errors.ErrLoginAttemptMaximumExceeded; + throw new ErrLoginAttemptMaximumExceeded(); } } @@ -515,11 +530,11 @@ class UsersService { const onlyLettersNumbersUnderscore = /^[A-Za-z0-9_]+$/; if (!username) { - throw errors.ErrMissingUsername; + throw new ErrMissingUsername(); } if (!onlyLettersNumbersUnderscore.test(username)) { - throw errors.ErrSpecialChars; + throw new ErrSpecialChars(); } if (checkAgainstWordlist) { @@ -539,11 +554,11 @@ class UsersService { */ static isValidPassword(password) { if (!password) { - throw errors.ErrMissingPassword; + throw new ErrMissingPassword(); } if (password.length < 8) { - throw errors.ErrPasswordTooShort; + throw new ErrPasswordTooShort(); } return password; @@ -558,7 +573,7 @@ class UsersService { */ static async createLocalUser(ctx, email, password, username) { if (!email) { - throw errors.ErrMissingEmail; + throw new ErrMissingEmail(); } email = email.toLowerCase().trim(); @@ -596,9 +611,9 @@ class UsersService { } catch (err) { if (err.code === 11000) { if (err.message.match('Username')) { - throw errors.ErrUsernameTaken; + throw new ErrUsernameTaken(); } - throw errors.ErrEmailTaken; + throw new ErrEmailTaken(); } throw err; } @@ -678,9 +693,7 @@ class UsersService { */ static async createPasswordResetToken(email, loc) { if (!email || typeof email !== 'string') { - throw new Error( - 'email is required when creating a JWT for resetting passord' - ); + throw new ErrMissingEmail(); } email = email.toLowerCase(); @@ -837,7 +850,7 @@ class UsersService { // Ensure that the user email hasn't already been verified. if (profile && profile.metadata && profile.metadata.confirmed_at) { - throw errors.ErrEmailAlreadyVerified; + throw new ErrEmailAlreadyVerified(); } return JWT_SECRET.sign( @@ -875,16 +888,16 @@ class UsersService { }, }); if (!user) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } const profile = user.profiles.find(({ id }) => id === decoded.email); if (!profile) { - throw errors.ErrNotFound; + throw new ErrNotFound(); } if (profile.metadata && profile.metadata.confirmed_at !== null) { - throw errors.ErrEmailAlreadyVerified; + throw new ErrEmailAlreadyVerified(); } return decoded; @@ -943,7 +956,7 @@ class UsersService { const users = await UsersService.findByIdArray(usersToIgnore); if (some(users, user => user.isStaff())) { - throw errors.ErrCannotIgnoreStaff; + throw new ErrCannotIgnoreStaff(); } return UserModel.update( diff --git a/services/wordlist.js b/services/wordlist.js index 04012adcb..9e7e1581e 100644 --- a/services/wordlist.js +++ b/services/wordlist.js @@ -1,7 +1,7 @@ const debug = require('debug')('talk:services:wordlist'); const _ = require('lodash'); const SettingsService = require('./settings'); -const Errors = require('../errors'); +const { ErrContainsProfanity } = require('../errors'); const memoize = require('lodash/memoize'); const { escapeRegExp } = require('./regex'); @@ -96,7 +96,7 @@ class Wordlist { `the field "${fieldName}" contained a phrase "${phrase}" which contained a banned word/phrase` ); - errors.banned = Errors.ErrContainsProfanity; + errors.banned = new ErrContainsProfanity(phrase); // Stop looping through the fields now, we discovered the worst possible // situation (a banned word). @@ -109,7 +109,7 @@ class Wordlist { `the field "${fieldName}" contained a phrase "${phrase}" which contained a suspected word/phrase` ); - errors.suspect = Errors.ErrContainsProfanity; + errors.suspect = new ErrContainsProfanity(phrase); // Continue looping through the fields now, we discovered a possible bad // word (suspect). @@ -167,7 +167,7 @@ class Wordlist { return wl.load().then(() => { if (wl.regexp.banned.test(username)) { - return Errors.ErrContainsProfanity; + throw new ErrContainsProfanity(username); } }); } diff --git a/test/server/graph/context.js b/test/server/graph/context.js index a788f509b..b3319d5c4 100644 --- a/test/server/graph/context.js +++ b/test/server/graph/context.js @@ -1,6 +1,6 @@ const User = require('../../../models/user'); const Context = require('../../../graph/context'); -const errors = require('../../../errors'); +const { ErrNotAuthorized } = require('../../../errors'); const SettingsService = require('../../../services/settings'); const { expect } = require('chai'); @@ -54,7 +54,7 @@ describe('graph.Context', () => { throw new Error('should not reach this point'); }) .catch(err => { - expect(err).to.be.equal(errors.ErrNotAuthorized); + expect(err).to.be.an.instanceof(ErrNotAuthorized); }); }); }); diff --git a/test/server/services/wordlist.js b/test/server/services/wordlist.js index 7a11dbf80..313dde49f 100644 --- a/test/server/services/wordlist.js +++ b/test/server/services/wordlist.js @@ -1,4 +1,4 @@ -const Errors = require('../../../errors'); +const { ErrContainsProfanity } = require('../../../errors'); const Wordlist = require('../../../services/wordlist'); const SettingsService = require('../../../services/settings'); @@ -103,7 +103,8 @@ describe('services.Wordlist', () => { 'content' ); - expect(errors).to.have.property('banned', Errors.ErrContainsProfanity); + expect(errors).to.have.property('banned'); + expect(errors.banned).to.be.an.instanceof(ErrContainsProfanity); }); it('does not match on bodies not containing bad words', () => {