diff --git a/app.js b/app.js index c2bf9648d..c5507da2e 100644 --- a/app.js +++ b/app.js @@ -1,5 +1,6 @@ const express = require('express'); -const morgan = require('morgan'); +const trace = require('./middleware/trace'); +const logging = require('./middleware/logging'); const path = require('path'); const merge = require('lodash/merge'); const helmet = require('helmet'); @@ -12,6 +13,10 @@ const { ENABLE_TRACING, APOLLO_ENGINE_KEY, PORT } = require('./config'); const app = express(); +// Add the trace middleware first, it will create a request ID for each request +// downstream. +app.use(trace); + //============================================================================== // PLUGIN PRE APPLICATION MIDDLEWARE //============================================================================== @@ -30,7 +35,7 @@ plugins.get('server', 'app').forEach(({ plugin, app: callback }) => { // Add the logging middleware only if we aren't testing. if (process.env.NODE_ENV !== 'test') { - app.use(morgan('dev')); + app.use(logging.log); } if (ENABLE_TRACING && APOLLO_ENGINE_KEY) { 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/client/coral-admin/src/actions/moderation.js b/client/coral-admin/src/actions/moderation.js index 16c2bd2f7..a46d2dccd 100644 --- a/client/coral-admin/src/actions/moderation.js +++ b/client/coral-admin/src/actions/moderation.js @@ -5,14 +5,7 @@ export const singleView = () => ({ type: actions.SINGLE_VIEW }); // hide shortcuts note export const hideShortcutsNote = () => (dispatch, _, { localStorage }) => { - try { - if (localStorage) { - localStorage.setItem('coral:shortcutsNote', 'hide'); - } - } catch (e) { - // above will fail in Safari private mode - } - + localStorage.setItem('coral:shortcutsNote', 'hide'); dispatch({ type: actions.HIDE_SHORTCUTS_NOTE }); }; diff --git a/client/coral-admin/src/index.js b/client/coral-admin/src/index.js index 0cf0a8cf9..158870dec 100644 --- a/client/coral-admin/src/index.js +++ b/client/coral-admin/src/index.js @@ -15,7 +15,8 @@ import { hideShortcutsNote } from './actions/moderation'; smoothscroll.polyfill(); function init({ store, localStorage }) { - if (localStorage && localStorage.getItem('coral:shortcutsNote') === 'hide') { + const shouldHide = localStorage.getItem('coral:shortcutsNote') === 'hide'; + if (shouldHide) { store.dispatch(hideShortcutsNote()); } } diff --git a/client/coral-docs/src/index.js b/client/coral-docs/src/index.js deleted file mode 100644 index d65c6c29e..000000000 --- a/client/coral-docs/src/index.js +++ /dev/null @@ -1,8 +0,0 @@ -import React from 'react'; -import { render } from 'react-dom'; -import { GraphQLDocs } from 'graphql-docs'; - -import fetcher from './services/fetcher'; - -// Render the application into the DOM -render(, document.querySelector('#root')); diff --git a/client/coral-docs/src/services/fetcher.js b/client/coral-docs/src/services/fetcher.js deleted file mode 100644 index 32b412f67..000000000 --- a/client/coral-docs/src/services/fetcher.js +++ /dev/null @@ -1,10 +0,0 @@ -export default function fetcher(query) { - return fetch(`${window.location.origin}/api/v1/graph/ql`, { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ query }), - }).then(res => res.json()); -} diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index 5390153e6..5e3b0846f 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -15,9 +15,7 @@ export const checkLogin = () => ( rest('/auth') .then(result => { if (!result.user) { - if (localStorage) { - cleanAuthData(localStorage); - } + cleanAuthData(localStorage); dispatch(checkLoginSuccess(null)); return; } @@ -52,10 +50,8 @@ const checkLoginSuccess = user => ({ }); export const setAuthToken = token => (dispatch, _, { localStorage }) => { - if (localStorage) { - localStorage.setItem('exp', jwtDecode(token).exp); - localStorage.setItem('token', token); - } + localStorage.setItem('exp', jwtDecode(token).exp); + localStorage.setItem('token', token); // Dispatch the set auth token action. For some browsers and situations, we // may not be able to persist the auth token any other way. Keep it in redux! @@ -70,11 +66,8 @@ export const handleSuccessfulLogin = (user, token) => ( { client, localStorage, postMessage } ) => { const { exp } = jwtDecode(token); - - if (localStorage) { - localStorage.setItem('exp', exp); - localStorage.setItem('token', token); - } + localStorage.setItem('exp', exp); + localStorage.setItem('token', token); // Send the message via the messages service to the window.opener if it // exists. @@ -105,9 +98,8 @@ export const logout = () => async ( ) => { await rest('/auth', { method: 'DELETE' }); - if (localStorage) { - cleanAuthData(localStorage); - } + // Clear the auth data persisted to localStorage. + cleanAuthData(localStorage); // Reset the websocket. client.resetWebsocket(); diff --git a/client/coral-framework/services/i18n.js b/client/coral-framework/services/i18n.js index 9c65c9c4e..e1c3ab99e 100644 --- a/client/coral-framework/services/i18n.js +++ b/client/coral-framework/services/i18n.js @@ -52,26 +52,41 @@ let lang; let timeagoInstance; function setLocale(storage, locale) { - try { - if (storage) { - storage.setItem('locale', locale); - } - } catch (err) { - console.error(err); - } + storage.setItem('locale', locale); } -function getLocale(storage) { +// detectLanguage will try to get the locale from storage if available, +// otherwise will try to get it from the navigator, otherwise, it will fallback +// to the default language. +function detectLanguage(storage) { try { - return ( - (storage && storage.getItem('locale')) || - navigator.language || - defaultLanguage - ).split('-')[0]; + const lang = storage.getItem('locale') || navigator.language; + if (lang) { + return lang; + } } catch (err) { - console.error(err); - return null; + console.warn( + 'Error while trying to detect language, will fallback to', + err + ); } + + console.warn('Could not detect language, will fallback to', defaultLanguage); + return defaultLanguage; +} + +// getLocale will get the users locale from the local detector and parse it to a +// format we can work with. +function getLocale(storage) { + // Get the language from the local detector. + const lang = detectLanguage(storage); + + // Some language strings come with additional subtags as defined in: + // + // https://www.ietf.org/rfc/bcp/bcp47.txt + // + // So we should strip that off if we find it. + return lang.split('-')[0]; } export function setupTranslations() { diff --git a/client/coral-framework/services/storage.js b/client/coral-framework/services/storage.js index 66e2c96cd..7b7acf655 100644 --- a/client/coral-framework/services/storage.js +++ b/client/coral-framework/services/storage.js @@ -1,40 +1,116 @@ import uuid from 'uuid/v4'; -function getStorage(type) { - let storage; - try { - storage = window[type]; - const x = '__storage_test__'; - storage.setItem(x, x); - storage.removeItem(x); - } catch (e) { - const ignore = - e instanceof DOMException && - // everything except Firefox - (e.code === 22 || - // SecurityError related to having 3rd party cookies disabled. - e.code === 18 || - // Firefox +function testStorageAccess(storage) { + const key = '__storage_test__'; - e.code === 1014 || - // test name field too, because code might not be present + // Create a unique test value. + const expectedValue = String(Date.now()); - // everything except Firefox - e.name === 'QuotaExceededError' || - // Firefox - e.name === 'NS_ERROR_DOM_QUOTA_REACHED'); - if (!ignore) { - console.warn(e); + // Try to set, get, and remove that item. + storage.setItem(key, expectedValue); + const canSetGet = expectedValue === storage.getItem(key); + storage.removeItem(key); + + if (!canSetGet) { + // We can't access the desired storage! + throw new Error('Storage access test failed'); + } +} + +// InMemoryStorage is a dumb implementation of the Storage interface that will +// not persist the data at all. It implements the Storage interface found: +// +// https://developer.mozilla.org/en-US/docs/Web/API/Storage +// +class InMemoryStorage { + constructor() { + this.storage = {}; + } + + get length() { + return Object.keys(this.storage).length; + } + + key(n) { + if (this.length <= n) { + return undefined; } - // When third party cookies are disabled, session storage is readable/ - // writable, but localStorage is not. Try to get the sessionStorage to use. - if (type !== 'sessionStorage') { - return getStorage('sessionStorage'); + return this.storage[Object.keys(this.storage)[n]]; + } + + getItem(key) { + return this.storage[key]; + } + + setItem(key, value) { + this.storage[key] = value; + + try { + // Test sessionStorage. We could have been given access recently. + testStorageAccess(sessionStorage); + + // Test passed! Set the item in sessionStorage. + sessionStorage.setItem(key, value); + console.log( + 'Attempt to persist InMemoryStorage value to sessionStorage succeeded' + ); + } catch (err) { + console.warn( + 'Attempt to persist InMemoryStorage value to sessionStorage failed', + err + ); } } - return storage; + removeItem(key) { + delete this.storage[key]; + + try { + // Test sessionStorage. We could have been given access recently. + testStorageAccess(sessionStorage); + + // Test passed! Remove the item from sessionStorage. + sessionStorage.removeItem(key); + console.log( + 'Attempt to persist InMemoryStorage delete to sessionStorage succeeded' + ); + } catch (err) { + console.warn( + 'Attempt to persist InMemoryStorage delete to sessionStorage failed', + err + ); + } + } +} + +// getStorage will test to see if the requested storage type is available, if it +// is not, it will try sessionStorage, and if that is also not available, it +// will fallback to InMemoryStorage. +function getStorage(type) { + try { + // Get the desired storage from the window and test it out. + const storage = window[type]; + testStorageAccess(storage); + + // Storage test was successful! Return it. + return storage; + } catch (err) { + // When third party cookies are disabled, session storage is readable/ + // writable, but localStorage is not. Try to get the sessionStorage to use. + if (type !== 'sessionStorage') { + console.warn('Could not access', type, 'trying sessionStorage', err); + return getStorage('sessionStorage'); + } + + console.warn( + 'Could not access sessionStorage falling back to InMemoryStorage', + err + ); + } + + // No acceptable storage could be found, returning the InMemoryStorage. + return new InMemoryStorage(); } /** diff --git a/client/coral-ui/components/TextField.js b/client/coral-ui/components/TextField.js index 6cbb7a06b..2ba23e6e8 100644 --- a/client/coral-ui/components/TextField.js +++ b/client/coral-ui/components/TextField.js @@ -27,11 +27,14 @@ const TextField = ({ ); TextField.propTypes = { + id: PropTypes.string, label: PropTypes.string, value: PropTypes.string, onChange: PropTypes.func, errorMsg: PropTypes.string, type: PropTypes.string, + className: PropTypes.string, + showErrors: PropTypes.bool, }; export default TextField; diff --git a/docs/_config.yml b/docs/_config.yml index c61fdeb21..6c0da733b 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -122,6 +122,8 @@ sidebar: url: /commenter-features/ - title: Moderator Features url: /moderator-features/ + - title: User Roles in Talk + url: /roles/ - title: Trust url: /trust/ - title: Toxic Comments diff --git a/docs/source/03-04-user-roles.md b/docs/source/03-04-user-roles.md new file mode 100644 index 000000000..6853183d8 --- /dev/null +++ b/docs/source/03-04-user-roles.md @@ -0,0 +1,35 @@ +--- +title: User Roles in Talk +permalink: /roles/ +--- + +We have four preset roles in Talk: + +**Commenter** +* A standard community member +* Could receive a badge (eg. 'Subscriber') via [a custom newsroom Plugin Recipe](https://docs.coralproject.net/talk/plugin-recipes/#recipe-subscriber) +* No moderation abilities +* No configuration abilities + +**Staff** +* A standard community member +* Receives a Staff badge when they comment +* Comments are automatically approved +* No moderation abilities +* No configuration abilities + +**Moderator** +* A standard community member +* Receives a Staff badge when they comment +* Comments are automatically approved +* Has full moderation privileges +* Can configure individual articles via the Configure tab on the article page +* No site-wide configuration abilities + +**Administrator** +* A standard community member +* Receives a Staff badge when they comment +* Comments are automatically approved +* Has full moderation privileges +* Can configure individual articles via the Configure tab on the article page +* Can configure site settings via the Configure tab in the moderation interface diff --git a/docs/source/03-04-product-guide-trust.md b/docs/source/03-07-product-guide-trust.md similarity index 100% rename from docs/source/03-04-product-guide-trust.md rename to docs/source/03-07-product-guide-trust.md 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/graph/resolvers/comment.js b/graph/resolvers/comment.js index 7005701ec..b174dd5ae 100644 --- a/graph/resolvers/comment.js +++ b/graph/resolvers/comment.js @@ -1,6 +1,14 @@ const { property } = require('lodash'); -const { SEARCH_ACTIONS } = require('../../perms/constants'); -const { decorateWithTags, decorateWithPermissionCheck } = require('./util'); +const { + SEARCH_ACTIONS, + SEARCH_COMMENT_STATUS_HISTORY, + VIEW_BODY_HISTORY, +} = require('../../perms/constants'); +const { + decorateWithTags, + decorateWithPermissionCheck, + checkSelfField, +} = require('./util'); const Comment = { hasParent({ parent_id }) { @@ -60,9 +68,19 @@ const Comment = { // Decorate the Comment type resolver with a tags field. decorateWithTags(Comment); -// Protect direct action access. +// Protect direct action and status history access. decorateWithPermissionCheck(Comment, { actions: [SEARCH_ACTIONS], + status_history: [SEARCH_COMMENT_STATUS_HISTORY], }); +// Protect privileged fields. +decorateWithPermissionCheck( + Comment, + { + body_history: [VIEW_BODY_HISTORY], + }, + checkSelfField('author_id') +); + module.exports = Comment; diff --git a/graph/resolvers/user.js b/graph/resolvers/user.js index 46fe0ac3c..67478785a 100644 --- a/graph/resolvers/user.js +++ b/graph/resolvers/user.js @@ -29,9 +29,9 @@ const User = { return Comments.getByQuery(query); }, - ignoredUsers({ ignoresUsers }, args, { user, loaders: { Users } }) { + ignoredUsers({ ignoresUsers }, args, { loaders: { Users } }) { // Return nothing if there is nothing to query for. - if (!user.ignoresUsers || user.ignoresUsers.length <= 0) { + if (!ignoresUsers || ignoresUsers.length <= 0) { return []; } diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index 2414faf1f..c1e7b9ecb 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -505,8 +505,9 @@ type Comment { # The actual comment data. body: String! - # The body history of the comment. - body_history: [CommentBodyHistory!]! + # The body history of the comment. Requires the `ADMIN` or `MODERATOR` role or + # the author. + body_history: [CommentBodyHistory!] # The tags on the comment tags: [TagLink!] diff --git a/jest.config.js b/jest.config.js index 7150d1066..7c3755bea 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,7 +1,7 @@ const path = require('path'); const { pluginsPath } = require('./plugins'); -const buildTargets = ['coral-admin', 'coral-docs']; +const buildTargets = ['coral-admin']; const buildEmbeds = ['stream']; 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/middleware/logging.js b/middleware/logging.js new file mode 100644 index 000000000..efabbe34f --- /dev/null +++ b/middleware/logging.js @@ -0,0 +1,40 @@ +const { logger } = require('../services/logging'); +const now = require('performance-now'); + +const log = (req, res, next) => { + const startTime = now(); + const end = res.end; + res.end = function(chunk, encoding) { + // Compute the end time. + const responseTime = Math.round(now() - startTime); + + // Get some extra goodies from the request. + const userAgent = req.get('User-Agent'); + + // Reattach the old end, and finish. + res.end = end; + res.end(chunk, encoding); + + // Log this out. + logger.info( + { + traceID: req.id, + url: req.originalUrl || req.url, + method: req.method, + statusCode: res.statusCode, + userAgent, + responseTime, + }, + 'http request' + ); + }; + + next(); +}; + +const error = (err, req, res, next) => { + logger.error({ err }, 'http error'); + next(err); +}; + +module.exports = { log, error }; diff --git a/middleware/trace.js b/middleware/trace.js new file mode 100644 index 000000000..24cbf38f4 --- /dev/null +++ b/middleware/trace.js @@ -0,0 +1,7 @@ +const uuid = require('uuid/v1'); + +// Trace middleware attaches a request id to each incoming request. +module.exports = (req, res, next) => { + req.id = uuid(); + next(); +}; diff --git a/package.json b/package.json index dd84c7faf..02cc45e70 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "talk", - "version": "4.3.1", + "version": "4.3.2", "description": "A better commenting experience from Mozilla, The New York Times, and the Washington Post. https://coralproject.net", "main": "app.js", "private": true, @@ -146,7 +146,6 @@ "minimist": "^1.2.0", "moment": "^2.18.1", "mongoose": "^4.12.3", - "morgan": "^1.9.0", "ms": "^2.0.0", "murmurhash-js": "^1.0.0", "name-all-modules-plugin": "^1.0.1", @@ -158,6 +157,7 @@ "passport": "^0.4.0", "passport-jwt": "^3.0.0", "passport-local": "^1.0.0", + "performance-now": "^2.1.0", "pluralize": "^7.0.0", "postcss-loader": "^1.3.3", "postcss-smart-import": "^0.5.1", @@ -215,6 +215,7 @@ "babel-plugin-dynamic-import-node": "^1.1.0", "babel-plugin-transform-es2015-modules-commonjs": "^6.26.0", "browserstack-local": "^1.3.0", + "bunyan-debug-stream": "^1.0.8", "chai": "^3.5.0", "chai-as-promised": "^6.0.0", "chai-datetime": "^1.5.0", diff --git a/perms/constants/query.js b/perms/constants/query.js index b846b15ce..197c5d9f9 100644 --- a/perms/constants/query.js +++ b/perms/constants/query.js @@ -10,4 +10,5 @@ module.exports = { LIST_OWN_TOKENS: 'LIST_OWN_TOKENS', VIEW_USER_ROLE: 'VIEW_USER_ROLE', VIEW_USER_EMAIL: 'VIEW_USER_EMAIL', + VIEW_BODY_HISTORY: 'VIEW_BODY_HISTORY', }; diff --git a/perms/reducers/query.js b/perms/reducers/query.js index 0852d8e2b..ed507139d 100644 --- a/perms/reducers/query.js +++ b/perms/reducers/query.js @@ -13,6 +13,7 @@ module.exports = (user, perm) => { case types.VIEW_PROTECTED_SETTINGS: case types.VIEW_USER_ROLE: case types.VIEW_USER_EMAIL: + case types.VIEW_BODY_HISTORY: return check(user, ['ADMIN', 'MODERATOR']); case types.LIST_OWN_TOKENS: return check(user, ['ADMIN']); 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-auth/client/login/components/SignUp.js b/plugins/talk-plugin-auth/client/login/components/SignUp.js index cc49e7391..83f81f81d 100644 --- a/plugins/talk-plugin-auth/client/login/components/SignUp.js +++ b/plugins/talk-plugin-auth/client/login/components/SignUp.js @@ -75,6 +75,7 @@ class SignUp extends React.Component { showErrors={!!emailError} errorMsg={emailError} onChange={this.handleEmailChange} + autocomplete="off" /> {passwordError && ( @@ -113,6 +117,7 @@ class SignUp extends React.Component { errorMsg={passwordRepeatError} onChange={this.handlePasswordRepeatChange} minLength="8" + autocomplete="off" /> { @@ -23,6 +23,9 @@ const handle = async (ctx, comment) => { id user { id + ignoredUsers { + id + } notificationSettings { onReply } @@ -53,13 +56,23 @@ const handle = async (ctx, comment) => { return; } + // Pull out the author of the new comment. + const authorID = get(comment, 'author_id'); + // Check to see if this is yourself replying to yourself, if that's the case // don't send a notification. - if (userID === get(comment, 'author_id')) { + if (userID === authorID) { ctx.log.info('user id of parent comment is the same as the new comment'); return; } + // Check to see if this user is ignoring the user who replied to their + // comment. + if (map(get(comment, 'user.ignoredUsers', []), 'id').indexOf(authorID)) { + ctx.log.info('parent user has ignored the author of the new comment'); + return; + } + // The user does have notifications for replied comments enabled, queue the // notification to be sent. return { userID, date: comment.created_at, context: comment.id }; 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..b6e46791e 100644 --- a/routes/index.js +++ b/routes/index.js @@ -1,8 +1,8 @@ const SetupService = require('../services/setup'); const authentication = require('../middleware/authentication'); +const logging = require('../middleware/logging'); 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,16 @@ router.use(require('./plugins')); // Catch 404 and forward to error handler. router.use((req, res, next) => { - next(errors.ErrNotFound); + next(new ErrNotFound()); }); +// Add logging for errors. +router.use(logging.error); + // 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 (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 +169,7 @@ router.use('/api', (err, req, res, next) => { }); router.use('/', (err, req, res, next) => { - if (err !== errors.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/logging.js b/services/logging.js index 47ef93af8..3e2a6d731 100644 --- a/services/logging.js +++ b/services/logging.js @@ -1,18 +1,46 @@ const { version } = require('../package.json'); -const Logger = require('bunyan'); +const path = require('path'); +const { createLogger: createBunyanLogger, stdSerializers } = require('bunyan'); const { LOGGING_LEVEL, REVISION_HASH } = require('../config'); -const logger = new Logger({ + +// Streams enables the ability for development logs to be readable to a human, +// but will send JSON logs in production that's parsable by a system like ELK. +const streams = (() => { + // In development, use the debug stream printer. + if (process.env.NODE_ENV === 'development') { + const debug = require('bunyan-debug-stream'); + return [ + { + level: 'debug', + type: 'raw', + stream: debug({ + basepath: path.resolve(__dirname, '..'), + forceColor: true, + }), + }, + ]; + } + + // In production, emit JSON. + return [{ stream: process.stdout, level: 'info' }]; +})(); + +// logger is the base logger used by all logging systems in Talk. +const logger = createBunyanLogger({ src: true, name: 'talk', version, revision: REVISION_HASH, level: LOGGING_LEVEL, - serializers: Logger.stdSerializers, + streams, + serializers: stdSerializers, }); -// Create the logging instance that all logger's are branched from. -function createLogger(name, traceID) { - return logger.child({ origin: name, traceID }); -} +/** + * + * @param {String} origin the origin name used by the logger + * @param {String} traceID the id of the request being made + */ +const createLogger = (origin, traceID) => logger.child({ origin, traceID }); module.exports = { logger, createLogger }; 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/mongoose.js b/services/mongoose.js index 20765293a..5e498cee8 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -1,48 +1,40 @@ -const { MONGO_URL, WEBPACK, CREATE_MONGO_INDEXES } = require('../config'); - +const { + MONGO_URL, + WEBPACK, + CREATE_MONGO_INDEXES, + LOGGING_LEVEL, +} = require('../config'); +const { logger } = require('./logging'); const mongoose = require('mongoose'); -const debug = require('debug')('talk:db'); -const enabled = require('debug').enabled; -const queryDebugger = require('debug')('talk:db:query'); - -// Loading the formatter from Mongoose: -// -// https://github.com/Automattic/mongoose/blob/1a93d1f4d12e441e17ddf451e96fbc5f6e8f54b8/lib/drivers/node-mongodb-native/collection.js#L182 -// -// so we can wrap parameters. -const formatter = require('mongoose').Collection.prototype.$format; // Provide a newly wrapped debugQuery function which wraps the `debug` package. -function debugQuery(name, i, ...args) { - let functionCall = ['db', name, i].join('.'); - let _args = []; - for (let j = args.length - 1; j >= 0; --j) { - if (formatter(args[j]) || _args.length) { - _args.unshift(formatter(args[j])); - } - } - - let params = `(${_args.join(', ')})`; - - queryDebugger(functionCall + params); +function debugQuery(name, operation, ...args) { + logger.debug( + { + query: `db.${name}.${operation}(${args + .map(arg => JSON.stringify(arg)) + .join(', ')})`, + }, + 'mongodb query' + ); } // Use native promises mongoose.Promise = global.Promise; -// Check if debugging is enabled on the talk:db prefix. -if (enabled('talk:db:query')) { +// Check if verbose logging is enabled. +if (['debug', 'trace'].includes(LOGGING_LEVEL)) { // Enable the mongoose debugger, here we wrap the similar print function // provided by setting the debug parameter. mongoose.set('debug', debugQuery); } if (WEBPACK) { - debug('Not connecting to mongodb during webpack build'); + logger.debug('Not connecting to mongodb during webpack build'); - // @wyattjoh: We didn't call connect, but because we include mongoose, it will hold the socket ready, - // preventing node from exiting. Calling disconnect here just ensures that the application - // can quit correctly. + // @wyattjoh: We didn't call connect, but because we include mongoose, it will + // hold the socket ready, preventing node from exiting. Calling disconnect + // here just ensures that the application can quit correctly. mongoose.disconnect(); } else { // Connect to the Mongo instance. @@ -54,7 +46,7 @@ if (WEBPACK) { }, }) .then(() => { - debug('connection established'); + logger.debug('mongodb connection established'); }) .catch(err => { console.error(err); @@ -66,10 +58,13 @@ module.exports = mongoose; // Here we include all the models that mongoose is used for, this ensures that // when we import mongoose that we also start up all the indexing operations -// here. -require('../models/action'); -require('../models/asset'); -require('../models/comment'); -require('../models/setting'); -require('../models/user'); -require('./migration'); +// here. No point also in importing this if we're not actually doing any +// indexing now. +if (CREATE_MONGO_INDEXES) { + require('../models/action'); + require('../models/asset'); + require('../models/comment'); + require('../models/setting'); + require('../models/user'); + require('./migration'); +} 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/redis.js b/services/redis.js index 2576d2c13..0a25b34d6 100644 --- a/services/redis.js +++ b/services/redis.js @@ -1,7 +1,5 @@ const Redis = require('ioredis'); const merge = require('lodash/merge'); -const debug = require('debug')('talk:services:redis'); -const enabled = require('debug').enabled('talk:services:redis'); const { REDIS_URL, REDIS_RECONNECTION_BACKOFF_FACTOR, @@ -9,29 +7,32 @@ const { REDIS_CLIENT_CONFIG, REDIS_CLUSTER_MODE, REDIS_CLUSTER_CONFIGURATION, + LOGGING_LEVEL, } = require('../config'); +const { createLogger } = require('./logging'); +const logger = createLogger('redis'); const attachMonitors = client => { - debug('client created'); + logger.debug('client created'); // Debug events. - if (enabled) { - client.on('connect', () => debug('client connected')); - client.on('ready', () => debug('client ready')); - client.on('close', () => debug('client closed the connection')); + if (['debug', 'trace'].includes(LOGGING_LEVEL)) { + client.on('connect', () => logger.info('client connected')); + client.on('ready', () => logger.debug('client ready')); + client.on('close', () => logger.debug('client closed the connection')); client.on('reconnecting', () => - debug('client connection lost, attempting to reconnect') + logger.debug('client connection lost, attempting to reconnect') ); - client.on('end', () => debug('client ended')); + client.on('end', () => logger.debug('client ended')); } // Error events. client.on('error', err => { if (err) { - console.error('Error connecting to redis:', err); + logger.error({ err }, 'cannot connect to redis'); } }); - client.on('node error', err => debug('node error', err)); + client.on('node error', err => logger.error({ err }, 'node error')); }; function retryStrategy(times) { @@ -40,7 +41,7 @@ function retryStrategy(times) { REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME ); - debug(`retry strategy: try to reconnect ${delay} ms from now`); + logger.debug(`retry strategy: try to reconnect ${delay} ms from now`); return delay; } 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', () => { diff --git a/yarn.lock b/yarn.lock index f97cf762f..b5d130ab0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1684,6 +1684,13 @@ builtin-status-codes@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/builtin-status-codes/-/builtin-status-codes-3.0.0.tgz#85982878e21b98e1c66425e03d0174788f569ee8" +bunyan-debug-stream@^1.0.8: + version "1.0.8" + resolved "https://registry.yarnpkg.com/bunyan-debug-stream/-/bunyan-debug-stream-1.0.8.tgz#df612852d5d0b6d6df3f30214d8a7e4ee925106d" + dependencies: + colors "^1.0.3" + exception-formatter "^1.0.4" + bunyan@^1.8.12: version "1.8.12" resolved "https://registry.yarnpkg.com/bunyan/-/bunyan-1.8.12.tgz#f150f0f6748abdd72aeae84f04403be2ef113797" @@ -2174,6 +2181,10 @@ colors@1.0.3, colors@1.0.x: version "1.0.3" resolved "https://registry.yarnpkg.com/colors/-/colors-1.0.3.tgz#0433f44d809680fdeb60ed260f1b0c262e82a40b" +colors@^1.0.3: + version "1.2.1" + resolved "https://registry.yarnpkg.com/colors/-/colors-1.2.1.tgz#f4a3d302976aaf042356ba1ade3b1a2c62d9d794" + colors@^1.1.2, colors@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/colors/-/colors-1.1.2.tgz#168a4701756b6a7f51a12ce0c97bfa28c084ed63" @@ -3577,6 +3588,12 @@ evp_bytestokey@^1.0.0, evp_bytestokey@^1.0.3: md5.js "^1.3.4" safe-buffer "^5.1.1" +exception-formatter@^1.0.4: + version "1.0.5" + resolved "https://registry.yarnpkg.com/exception-formatter/-/exception-formatter-1.0.5.tgz#bda957319789cbabdf36848fb5288c59634b73a5" + dependencies: + colors "^1.0.3" + exec-sh@^0.2.0: version "0.2.1" resolved "https://registry.yarnpkg.com/exec-sh/-/exec-sh-0.2.1.tgz#163b98a6e89e6b65b47c2a28d215bc1f63989c38" @@ -7480,7 +7497,7 @@ moo-server@*, moo-server@1.3.x: version "1.3.0" resolved "https://registry.yarnpkg.com/moo-server/-/moo-server-1.3.0.tgz#5dc79569565a10d6efed5439491e69d2392e58f1" -morgan@^1.6.1, morgan@^1.9.0: +morgan@^1.6.1: version "1.9.0" resolved "https://registry.yarnpkg.com/morgan/-/morgan-1.9.0.tgz#d01fa6c65859b76fcf31b3cb53a3821a311d8051" dependencies: