diff --git a/bin/cli-users b/bin/cli-users index c6555ab28..89f53a67d 100755 --- a/bin/cli-users +++ b/bin/cli-users @@ -241,13 +241,21 @@ function listUsers() { }); users.forEach((user) => { + let state = user.disabled ? 'Disabled' : 'Enabled'; + const profile = user.profiles.find(({provider}) => provider === 'local'); + if (profile && profile.metadata && profile.metadata.confirmed_at) { + state += ', Verified'; + } else { + state += ', Unverified'; + } + table.push([ user.id, user.username, user.profiles.map((p) => p.provider).join(', '), user.roles.join(', '), user.status, - user.disabled ? 'Disabled' : 'Enabled' + state ]); }); @@ -396,6 +404,23 @@ function enableUser(userID) { }); } +/** + * Verifies an email address for a user. + * + * @param userID the user's id + * @param email the user's email address to be verified + */ +async function verify(userID, email) { + try { + await UsersService.confirmEmail(userID, email); + console.log(`User ${userID} had their email ${email} verified.`); + util.shutdown(); + } catch (err) { + console.error(err); + util.shutdown(1); + } +} + //============================================================================== // Setting up the program command line arguments. //============================================================================== @@ -467,6 +492,11 @@ program .description('enable a given user from logging in') .action(enableUser); +program + .command('verify ') + .description('verifies the given user\'s email address') + .action(verify); + program.parse(process.argv); // If there is no command listed, output help. diff --git a/client/coral-embed-stream/src/actions/auth.js b/client/coral-embed-stream/src/actions/auth.js index b96d6f459..22d46a768 100644 --- a/client/coral-embed-stream/src/actions/auth.js +++ b/client/coral-embed-stream/src/actions/auth.js @@ -23,6 +23,10 @@ export const hideSignInDialog = () => (dispatch) => { dispatch({type: actions.HIDE_SIGNIN_DIALOG}); }; +export const resetSignInDialog = () => (dispatch) => { + dispatch({type: actions.HIDE_SIGNIN_DIALOG}); +}; + export const focusSignInDialog = () => ({ type: actions.FOCUS_SIGNIN_DIALOG, }); @@ -94,8 +98,9 @@ export const cleanState = () => ({ // Sign In Actions -const signInRequest = () => ({ - type: actions.FETCH_SIGNIN_REQUEST +const signInRequest = (email) => ({ + type: actions.FETCH_SIGNIN_REQUEST, + email, }); const signInFailure = (error) => ({ @@ -122,7 +127,7 @@ export const handleAuthToken = (token) => (dispatch, _, {storage}) => { export const fetchSignIn = (formData) => { return (dispatch, _, {rest}) => { - dispatch(signInRequest()); + dispatch(signInRequest(formData.email)); return rest('/auth/local', {method: 'POST', body: formData}) .then(({token}) => { @@ -144,8 +149,7 @@ export const fetchSignIn = (formData) => { // invalid credentials dispatch(signInFailure(t('error.email_password'), error.metadata)); } else { - const str = error.translation_key ? t(`error.${error.translation_key}`) : error.toString(); - dispatch(signInFailure(str)); + dispatch(signInFailure(error)); } }); }; @@ -349,8 +353,9 @@ const verifyEmailSuccess = () => ({ type: actions.VERIFY_EMAIL_SUCCESS }); -const verifyEmailFailure = () => ({ - type: actions.VERIFY_EMAIL_FAILURE +const verifyEmailFailure = (error) => ({ + type: actions.VERIFY_EMAIL_FAILURE, + error, }); export const requestConfirmEmail = (email) => (dispatch, getState, {rest}) => { @@ -366,8 +371,8 @@ export const requestConfirmEmail = (email) => (dispatch, getState, {rest}) => { }) .catch((error) => { console.error(error); - const errorMessage = error.translation_key ? t(`error.${error.translation_key}`) : error.toString(); - dispatch(verifyEmailFailure(errorMessage)); + dispatch(verifyEmailFailure(error)); + throw error; }); }; diff --git a/client/coral-embed-stream/src/constants/auth.js b/client/coral-embed-stream/src/constants/auth.js index 324f75f44..bb2ea6c15 100644 --- a/client/coral-embed-stream/src/constants/auth.js +++ b/client/coral-embed-stream/src/constants/auth.js @@ -53,3 +53,4 @@ export const UPDATE_USERNAME = 'UPDATE_USERNAME'; export const SET_REQUIRE_EMAIL_VERIFICATION = 'SET_REQUIRE_EMAIL_VERIFICATION'; export const SET_REDIRECT_URI = 'SET_REDIRECT_URI'; +export const RESET_SIGNIN_DIALOG = 'RESET_SIGNIN_DIALOG'; diff --git a/client/coral-embed-stream/src/reducers/auth.js b/client/coral-embed-stream/src/reducers/auth.js index 1e18ddffa..679e8d858 100644 --- a/client/coral-embed-stream/src/reducers/auth.js +++ b/client/coral-embed-stream/src/reducers/auth.js @@ -10,7 +10,7 @@ const initialState = { showCreateUsernameDialog: false, checkedInitialLogin: false, view: 'SIGNIN', - error: '', + error: null, passwordRequestSuccess: null, passwordRequestFailure: null, emailVerificationFailure: false, @@ -45,14 +45,15 @@ export default function auth (state = initialState, action) { showSignInDialog: true, signInDialogFocus: true, }; - case actions.HIDE_SIGNIN_DIALOG : + case actions.RESET_SIGNIN_DIALOG: + case actions.HIDE_SIGNIN_DIALOG: return { ...state, isLoading: false, showSignInDialog: false, signInDialogFocus: false, view: 'SIGNIN', - error: '', + error: null, passwordRequestFailure: null, passwordRequestSuccess: null, emailVerificationFailure: false, @@ -74,7 +75,7 @@ export default function auth (state = initialState, action) { return { ...state, showCreateUsernameDialog: false, - error: '', + error: null, }; case actions.CREATE_USERNAME_FAILURE: return { @@ -92,6 +93,7 @@ export default function auth (state = initialState, action) { case actions.FETCH_SIGNIN_REQUEST: return { ...state, + email: action.email, isLoading: true, }; case actions.CHECK_LOGIN_FAILURE: @@ -120,6 +122,7 @@ export default function auth (state = initialState, action) { isLoading: false, error: action.error, user: null, + view: action.error.translation_key === 'EMAIL_NOT_VERIFIED' ? 'RESEND_VERIFICATION' : state.view, }; case actions.FETCH_SIGNUP_FACEBOOK_REQUEST: return { @@ -175,7 +178,7 @@ export default function auth (state = initialState, action) { case actions.VALID_FORM: return { ...state, - error: '', + error: null, }; case actions.FETCH_FORGOT_PASSWORD_SUCCESS: return { @@ -200,7 +203,7 @@ export default function auth (state = initialState, action) { case actions.VERIFY_EMAIL_FAILURE: return { ...state, - emailVerificationFailure: true, + emailVerificationFailure: action.error, emailVerificationLoading: false, }; case actions.VERIFY_EMAIL_REQUEST: diff --git a/errors.js b/errors.js index dca3f0511..3d145d197 100644 --- a/errors.js +++ b/errors.js @@ -97,7 +97,8 @@ class ErrAssetCommentingClosed extends APIError { class ErrAuthentication extends APIError { constructor(message = null) { super('authentication error occured', { - status: 401 + status: 401, + translation_key: 'AUTHENTICATION' }, { message }); @@ -195,38 +196,52 @@ const ErrAssetURLAlreadyExists = new APIError('Asset URL already exists, cannot 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, +}); + +const ErrMaxRateLimit = new APIError('Rate limit exeeded', { + translation_key: 'RATE_LIMIT_EXCEEDED', + status: 429, +}); + // 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 + status: 400, }); module.exports = { - ExtendableError, APIError, ErrAlreadyExists, - ErrPasswordTooShort, - ErrSettingsNotInit, + ErrAssetCommentingClosed, + ErrAssetURLAlreadyExists, + ErrAuthentication, + ErrCannotIgnoreStaff, + ErrCommentTooShort, + ErrContainsProfanity, + ErrEditWindowHasEnded, + ErrEmailTaken, + ErrInstallLock, + ErrInvalidAssetURL, + ErrLoginAttemptMaximumExceeded, + ErrMaxRateLimit, ErrMissingEmail, ErrMissingPassword, ErrMissingToken, - ErrEmailTaken, - ErrSpecialChars, ErrMissingUsername, - ErrContainsProfanity, - ErrUsernameTaken, - ErrAssetCommentingClosed, - ErrNotFound, - ErrInvalidAssetURL, - ErrAuthentication, ErrNotAuthorized, + ErrNotFound, + ErrNotVerified, + ErrPasswordTooShort, ErrPermissionUpdateUsername, ErrSameUsernameProvided, ErrSettingsInit, - ErrInstallLock, - ErrLoginAttemptMaximumExceeded, - ErrEditWindowHasEnded, - ErrCommentTooShort, - ErrAssetURLAlreadyExists, - ErrCannotIgnoreStaff -}; + ErrSettingsNotInit, + ErrSpecialChars, + ErrUsernameTaken, + ExtendableError, +}; \ No newline at end of file diff --git a/locales/en.yml b/locales/en.yml index 2d560fedc..76a434dfc 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -194,8 +194,10 @@ en: NO_SPECIAL_CHARACTERS: "Usernames can contain letters numbers and _ only" PASSWORD_LENGTH: "Password is too short" PROFANITY_ERROR: "Usernames must not contain profanity. Please contact the administrator if you believe this to be in error." + RATE_LIMIT_EXCEEDED: "Rate limit exceeded" USERNAME_IN_USE: "Username already in use" USERNAME_REQUIRED: "Must input a username" + EMAIL_NOT_VERIFIED: "E-mail address not verified" EDIT_WINDOW_ENDED: "You can no longer edit this comment. The time window to do so has expired." EDIT_USERNAME_NOT_AUTHORIZED: "You do not have permission to update your username." SAME_USERNAME_PROVIDED: "You must submit a different username." diff --git a/plugins/talk-plugin-auth/client/components/ResendVerification.css b/plugins/talk-plugin-auth/client/components/ResendVerification.css new file mode 100644 index 000000000..dc59a9ff1 --- /dev/null +++ b/plugins/talk-plugin-auth/client/components/ResendVerification.css @@ -0,0 +1,15 @@ +.header { + margin-bottom: 20px; + text-align: center; + font-size: 1.2em; +} + +.notVerified { + padding: 10px; + margin-bottom: 20px; + border-radius: 2px; + border: solid 1px #dddd00; + background: #FFFF9C; + color: #777700; +} + diff --git a/plugins/talk-plugin-auth/client/components/ResendVerification.js b/plugins/talk-plugin-auth/client/components/ResendVerification.js new file mode 100644 index 000000000..b345577a9 --- /dev/null +++ b/plugins/talk-plugin-auth/client/components/ResendVerification.js @@ -0,0 +1,43 @@ +import React from 'react'; +import {Button, Spinner, Success, Alert} from 'plugin-api/beta/client/components/ui'; +import PropTypes from 'prop-types'; +import styles from './ResendVerification.css'; +import t from 'coral-framework/services/i18n'; + +class ResendVerification extends React.Component { + render() { + const {resendVerification, error, loading, success, email} = this.props; + return ( +
+

+ {t('sign_in.email_verify_cta')} +

+ + {error && + + {error.translation_key ? t(`error.${error.translation_key}`) : error.toString()} + } +
+ {t('error.email_not_verified', email)} +
+
+ + {loading && } + {success && } +
+
+ ); + } +} + +ResendVerification.propTypes = { + resendVerification: PropTypes.bool.isRequired, + error: PropTypes.object, + loading: PropTypes.bool, + success: PropTypes.bool, + email: PropTypes.string.isRequired, +}; + +export default ResendVerification; diff --git a/plugins/talk-plugin-auth/client/components/SignDialog.js b/plugins/talk-plugin-auth/client/components/SignDialog.js index 542f53f98..652148d75 100644 --- a/plugins/talk-plugin-auth/client/components/SignDialog.js +++ b/plugins/talk-plugin-auth/client/components/SignDialog.js @@ -5,13 +5,22 @@ import styles from './styles.css'; import SignInContent from './SignInContent'; import SignUpContent from './SignUpContent'; import ForgotContent from './ForgotContent'; +import ResendVerification from './ResendVerification'; -const SignDialog = ({open, view, hideSignInDialog, ...props}) => ( +const SignDialog = ({open, view, resetSignInDialog, ...props}) => ( - × + {view !== 'SIGNIN' && ×} {view === 'SIGNIN' && } {view === 'SIGNUP' && } {view === 'FORGOT' && } + {view === 'RESEND_VERIFICATION' && + } ); diff --git a/plugins/talk-plugin-auth/client/components/SignInContainer.js b/plugins/talk-plugin-auth/client/components/SignInContainer.js index 011ce45f1..c22ad72c4 100644 --- a/plugins/talk-plugin-auth/client/components/SignInContainer.js +++ b/plugins/talk-plugin-auth/client/components/SignInContainer.js @@ -15,6 +15,7 @@ import { fetchSignUpFacebook, fetchForgotPassword, requestConfirmEmail, + resetSignInDialog, facebookCallback, invalidForm, validForm, @@ -31,7 +32,6 @@ class SignInContainer extends React.Component { password: '', confirmPassword: '' }, - emailToBeResent: '', errors: {}, showErrors: false }; @@ -80,26 +80,15 @@ class SignInContainer extends React.Component { ); }; - handleChangeEmail = (e) => { - const {value} = e.target; - this.setState({emailToBeResent: value}); - }; - - handleResendVerification = (e) => { - e.preventDefault(); + resendVerification = () => { this.props - .requestConfirmEmail( - this.state.emailToBeResent, - ) + .requestConfirmEmail(this.props.auth.email) .then(() => { setTimeout(() => { // allow success UI to be shown for a second, and then close the modal - this.props.hideSignInDialog(); + this.props.resetSignInDialog(); }, 2500); - }) - .catch((err) => { - console.error(err); }); }; @@ -194,6 +183,7 @@ const mapDispatchToProps = (dispatch) => requestConfirmEmail, changeView, hideSignInDialog, + resetSignInDialog, invalidForm, validForm }, diff --git a/plugins/talk-plugin-auth/client/components/SignInContent.js b/plugins/talk-plugin-auth/client/components/SignInContent.js index d1b4caec1..956176b32 100644 --- a/plugins/talk-plugin-auth/client/components/SignInContent.js +++ b/plugins/talk-plugin-auth/client/components/SignInContent.js @@ -1,16 +1,11 @@ import React from 'react'; import PropTypes from 'prop-types'; -import {Button, TextField, Spinner, Success, Alert} from 'plugin-api/beta/client/components/ui'; +import {Button, TextField, Spinner, Alert} from 'plugin-api/beta/client/components/ui'; import styles from './styles.css'; import t from 'coral-framework/services/i18n'; const SignInContent = ({ handleChange, - handleChangeEmail, - emailToBeResent, - handleResendVerification, - emailVerificationLoading, - emailVerificationSuccess, formData, changeView, handleSignIn, @@ -21,71 +16,56 @@ const SignInContent = ({

- {auth.emailVerificationFailure - ? t('sign_in.email_verify_cta') - : t('sign_in.sign_in_to_join')} + {t('sign_in.sign_in_to_join')}

- {auth.error && {auth.error}} - {auth.emailVerificationFailure - ?
-

{t('sign_in.request_new_verify_email')}

+ {auth.error && + + {auth.error.translation_key ? t(`error.${auth.error.translation_key}`) : auth.error.toString()} + } +
+
+ +
+
+

+ {t('sign_in.or')} +

+
+ - - {emailVerificationLoading && } - {emailVerificationSuccess && } + +
+ {!auth.isLoading + ? + : } +
- :
-
- -
-
-

- {t('sign_in.or')} -

-
-
- - -
- {!auth.isLoading - ? - : } -
- -
} +
changeView('FORGOT')}> @@ -111,12 +91,12 @@ SignInContent.propTypes = { }).isRequired, fetchSignInFacebook: PropTypes.func.isRequired, handleSignIn: PropTypes.func.isRequired, + handleChange: PropTypes.func.isRequired, changeView: PropTypes.func.isRequired, emailVerificationLoading: PropTypes.bool.isRequired, emailVerificationSuccess: PropTypes.bool.isRequired, - handleResendVerification: PropTypes.func.isRequired, - handleChangeEmail: PropTypes.func.isRequired, - emailToBeResent: PropTypes.string.isRequired + resendVerification: PropTypes.func.isRequired, + formData: PropTypes.object, }; export default SignInContent; diff --git a/plugins/talk-plugin-auth/client/translations.yml b/plugins/talk-plugin-auth/client/translations.yml index 189c3d9da..f410ec2d3 100644 --- a/plugins/talk-plugin-auth/client/translations.yml +++ b/plugins/talk-plugin-auth/client/translations.yml @@ -1,7 +1,7 @@ en: sign_in: email_verify_cta: "Please verify your email address." - request_new_verify_email: "Request another email:" + request_new_verify_email: "Request another email" verify_email: "Thank you for creating an account! We sent an email to the address you provided to verify your account." verify_email2: "You must verify your account before engaging with the community." not_you: "Not you?" @@ -45,7 +45,7 @@ en: es: sign_in: email_verify_cta: "Por favor confirme su correo." - request_new_verify_email: "Enviar otro correo:" + request_new_verify_email: "Enviar otro correo" verify_email: "¡Gracias por crear una cuenta! Le enviamos un correo a la direcció\ n que dio para confirmar su cuenta." verify_email2: "Debe confirmarla antes de poder involucrarse en la comunidad." @@ -92,7 +92,7 @@ es: fr: sign_in: email_verify_cta: "Merci de vérifier votre adresse e-mail." - request_new_verify_email: "Demander un nouvel envoi d'e-mail :" + request_new_verify_email: "Demander un nouvel envoi d'e-mail" verify_email: "Merci d'avoir créé un compte ! Nous avons envoyé un courrier électronique à l'adresse que vous avez fournie pour vérifier votre adresse e-mail." verify_email2: "Vous devez vérifier votre adresse e-mail avant de vous engager auprès de la communauté." not_you: "Pas vous ?" diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 98cdcdead..3199c25ab 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -5,6 +5,7 @@ const mailer = require('../../../services/mailer'); const errors = require('../../../errors'); const authorization = require('../../../middleware/authorization'); const i18n = require('../../../services/i18n'); +const Limit = require('../../../services/limit'); const { ROOT_URL } = require('../../../config'); @@ -115,16 +116,15 @@ router.post('/:user_id/email', authorization.needed('ADMIN', 'MODERATOR'), async /** * SendEmailConfirmation sends a confirmation email to the user. - * @param {ExpressApp} app the instance of the express app * @param {String} userID the id for the user to send the email to * @param {String} email the email for the user to send the email to */ -const SendEmailConfirmation = async (app, userID, email, referer) => { - let token = await UsersService.createEmailConfirmToken(userID, email, referer); +const SendEmailConfirmation = async (user, email, referer) => { + let token = await UsersService.createEmailConfirmToken(user, email, referer); return mailer.sendSimple({ - template: 'email-confirm', // needed to know which template to render! - locals: { // specifies the template locals. + template: 'email-confirm', + locals: { token, rootURL: ROOT_URL, email @@ -144,7 +144,7 @@ router.post('/', async (req, res, next) => { // Send an email confirmation. The Front end will know about the // requireEmailConfirmation as it's included in the settings get endpoint. - await SendEmailConfirmation(req.app, user.id, email, redirectUri); + await SendEmailConfirmation(user, email, redirectUri); res.status(201).json(user); } catch (e) { @@ -172,25 +172,45 @@ router.post('/:user_id/actions', authorization.needed(), async (req, res, next) } }); +// This will allow 1 try every minute. +const resendRateLimiter = new Limit('/api/v1/users/resend-verify', 1, '1m'); + // trigger an email confirmation re-send by a new user router.post('/resend-verify', async (req, res, next) => { - const {email} = req.body; const redirectUri = req.header('X-Pym-Url') || req.header('Referer'); + let {email = ''} = req.body; - if (!email) { + // Clean up and validate the email. + email = email.toLowerCase().trim(); + if (email.length < 5) { return next(errors.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; + } - // find user by email. - // if the local profile is verified, return an error code? - // send a 204 after the email is re-sent - await SendEmailConfirmation(req.app, null, email, redirectUri); + await resendRateLimiter.test(email); + } catch (err) { + return next(err); + } + + try { + const user = await UsersService.findLocalUser(email); + if (!user) { + throw errors.ErrNotFound; + } + + await SendEmailConfirmation(user, email, redirectUri); res.status(204).end(); } catch (e) { - return next(e); + console.trace(e); + res.status(204).end(); } }); @@ -214,7 +234,7 @@ router.post('/:user_id/email/confirm', authorization.needed('ADMIN', 'MODERATOR' } // Send the email to the first local profile that was found. - await SendEmailConfirmation(req.app, user.id, localProfile.id); + await SendEmailConfirmation(user, localProfile.id); res.status(204).end(); } catch (e) { diff --git a/services/limit.js b/services/limit.js new file mode 100644 index 000000000..9d1101643 --- /dev/null +++ b/services/limit.js @@ -0,0 +1,71 @@ +const ms = require('ms'); +const errors = require('../errors'); +const {createClientFactory} = require('./redis'); +const client = createClientFactory(); + +/** + * Limit is designed to support rate limiting a resource. + */ +class Limit { + constructor(prefix, max, duration) { + this.ttl = ms(duration) / 1000; + this.prefix = prefix; + this.max = max; + } + + /** + * key will compose the redis key used to store the rate limit information. + * + * @param {String} value the string to use that is being limited + * @returns {String} the redis key to set + */ + key(value) { + return `limit[${this.prefix}][${value}]`; + } + + /** + * get will fetch the current number of attempts within the given window + * duration. + * + * @param {String} value the value to limit with + * @returns {Integer} the number of tries within the current window + */ + async get(value) { + const key = this.key(value); + + return client().get(key); + } + + /** + * test will increment the number of tries, reset the window length and + * will throw an error if the number of tries exceed the maximum for the + * window duration. + * + * @param {String} value the value to limit with + * @returns {Promise} resolves to the number of tries, or throws an error + */ + async test(value) { + const key = this.key(value); + + const [[, tries], [, expiry]] = await client() + .multi() + .incr(key) + .expire(key, this.ttl) + .exec(); + + // if this is new or has no expiry + if (tries === 1 || expiry === -1) { + + // then expire it after the timeout + client().expire(key, this.ttl); + } + + if (tries > this.max) { + throw errors.ErrMaxRateLimit; + } + + return tries; + } +} + +module.exports = Limit; diff --git a/services/passport.js b/services/passport.js index 83635fa85..0920c1c39 100644 --- a/services/passport.js +++ b/services/passport.js @@ -145,7 +145,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 (!profile.metadata || !profile.metadata.confirmed_at || profile.metadata.confirmed_at === null) { - return done(new errors.ErrAuthentication(loginProfile.id)); + return done(errors.ErrNotVerified); } } diff --git a/services/users.js b/services/users.js index 4d974f4a4..9906ef6c0 100644 --- a/services/users.js +++ b/services/users.js @@ -18,7 +18,7 @@ const UserModel = require('../models/user'); const USER_STATUS = require('../models/enum/user_status'); const USER_ROLES = require('../models/enum/user_roles'); -const RECAPTCHA_WINDOW_SECONDS = 60 * 10; // 10 minutes. +const RECAPTCHA_WINDOW = '10m'; // 10 minutes. const RECAPTCHA_INCORRECT_TRIGGER = 5; // after 3 incorrect attempts, recaptcha will be required. const ActionsService = require('./actions'); @@ -35,9 +35,8 @@ const PASSWORD_RESET_JWT_SUBJECT = 'password_reset'; const SALT_ROUNDS = 10; // Create a redis client to use for authentication. -const {createClientFactory} = require('./redis'); - -const client = createClientFactory(); +const Limit = require('./limit'); +const loginRateLimiter = new Limit('loginAttempts', RECAPTCHA_INCORRECT_TRIGGER, RECAPTCHA_WINDOW); // UsersService is the interface for the application to interact with the // UserModel through. @@ -71,23 +70,14 @@ module.exports = class UsersService { * where future login attempts must be made with the recaptcha flag. */ static async recordLoginAttempt(email) { - const rdskey = `la[${email.toLowerCase().trim()}]`; + try { + await loginRateLimiter.test(email.toLowerCase().trim()); + } catch (err) { + if (err === errors.ErrMaxRateLimit) { + throw errors.ErrLoginAttemptMaximumExceeded; + } - const replies = await client() - .multi() - .incr(rdskey) - .expire(rdskey, RECAPTCHA_WINDOW_SECONDS) - .exec(); - - // if this is new or has no expiry - if (replies[0] === 1 || replies[1] === -1) { - - // then expire it after the timeout - client().expire(rdskey, RECAPTCHA_WINDOW_SECONDS); - } - - if (replies[0] >= RECAPTCHA_INCORRECT_TRIGGER) { - throw errors.ErrLoginAttemptMaximumExceeded; + throw err; } } @@ -98,9 +88,7 @@ module.exports = class UsersService { * errors.ErrLoginAttemptMaximumExceeded */ static async checkLoginAttempts(email) { - const rdskey = `la[${email.toLowerCase().trim()}]`; - - const attempts = await client().get(rdskey); + const attempts = await loginRateLimiter.get(email.toLowerCase().trim()); if (!attempts) { return; } @@ -718,7 +706,7 @@ module.exports = class UsersService { * @param {String} email The email that we are needing to get confirmed. * @return {Promise} */ - static async createEmailConfirmToken(userID = null, email, referer = ROOT_URL) { + static async createEmailConfirmToken(user, email, referer = ROOT_URL) { if (!email || typeof email !== 'string') { throw new Error('email is required when creating a JWT for resetting passord'); } @@ -732,20 +720,6 @@ module.exports = class UsersService { subject: EMAIL_CONFIRM_JWT_SUBJECT }; - let user; - if (!userID) { - - // If there is no userID, we're coming from the endpoint where a new user - // is re-requesting a confirmation email and we don't know the userID. - user = await UserModel.findOne({profiles: {$elemMatch: {id: email, provider: 'local'}}}); - } else { - user = await UsersService.findById(userID); - } - - if (!user) { - throw errors.ErrNotFound; - } - // Get the profile representing the local account. let profile = user.profiles.find((profile) => profile.id === email && profile.provider === 'local'); diff --git a/test/server/routes/api/auth/index.js b/test/server/routes/api/auth/index.js index 2f3c4116a..463e27711 100644 --- a/test/server/routes/api/auth/index.js +++ b/test/server/routes/api/auth/index.js @@ -74,10 +74,8 @@ describe('/api/v1/auth/local', () => { .catch((err) => { expect(err).to.have.status(401); err.response.body.should.have.property('error'); - err.response.body.error.should.have.property('metadata'); - err.response.body.error.metadata.should.have.property('message', 'maria@gmail.com'); - return UsersService.createEmailConfirmToken(mockUser.id, mockUser.profiles[0].id); + return UsersService.createEmailConfirmToken(mockUser, mockUser.profiles[0].id); }) .then(UsersService.verifyEmailConfirmation) .then(() => { diff --git a/test/server/services/users.js b/test/server/services/users.js index 6ac56947a..dddd5eda8 100644 --- a/test/server/services/users.js +++ b/test/server/services/users.js @@ -88,17 +88,19 @@ describe('services.UsersService', () => { describe('#createEmailConfirmToken', () => { it('should create a token for a valid user', async () => { - const token = await UsersService.createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id); + const token = await UsersService.createEmailConfirmToken(mockUsers[0], mockUsers[0].profiles[0].id); expect(token).to.not.be.null; }); it('should not create a token for a user already verified', async () => { - const token = await UsersService.createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id); + const token = await UsersService.createEmailConfirmToken(mockUsers[0], mockUsers[0].profiles[0].id); expect(token).to.not.be.null; await UsersService.verifyEmailConfirmation(token); - return expect(UsersService.createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id)).to.eventually.be.rejected; + const user = await UsersService.findById(mockUsers[0].id); + + return expect(UsersService.createEmailConfirmToken(user, mockUsers[0].profiles[0].id)).to.eventually.be.rejected; }); }); @@ -106,7 +108,7 @@ describe('services.UsersService', () => { describe('#verifyEmailConfirmation', () => { it('should correctly validate a valid token', async () => { - const token = await UsersService.createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id); + const token = await UsersService.createEmailConfirmToken(mockUsers[0], mockUsers[0].profiles[0].id); expect(token).to.not.be.null; return expect(UsersService.verifyEmailConfirmation(token)).to.eventually.not.be.rejected; @@ -122,7 +124,7 @@ describe('services.UsersService', () => { it('should update the user model when verification is complete', () => { return UsersService - .createEmailConfirmToken(mockUsers[0].id, mockUsers[0].profiles[0].id) + .createEmailConfirmToken(mockUsers[0], mockUsers[0].profiles[0].id) .then((token) => { expect(token).to.not.be.null;