diff --git a/errors.js b/errors.js index ab1667e07..93ebf34b9 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,31 +196,45 @@ 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, +}); + module.exports = { - ExtendableError, APIError, ErrAlreadyExists, - ErrPasswordTooShort, - ErrSettingsNotInit, + ErrAssetCommentingClosed, + ErrAssetURLAlreadyExists, + ErrAuthentication, + 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 + ErrSettingsNotInit, + ErrSpecialChars, + ErrUsernameTaken, + ExtendableError, }; diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 189d0ea3e..d076288e6 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'); @@ -109,16 +110,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 @@ -138,7 +138,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) { @@ -166,25 +166,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(); } }); @@ -208,7 +228,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..7acc78f0c --- /dev/null +++ b/services/limit.js @@ -0,0 +1,47 @@ +const ms = require('ms'); +const errors = require('../errors'); +const {createClientFactory} = require('./redis'); +const client = createClientFactory(); + +class Limit { + constructor(prefix, max, duration) { + this.ttl = ms(duration) / 1000; + this.prefix = prefix; + this.max = max; + } + + key(value) { + return `limit[${this.prefix}][${value}]`; + } + + async get(value) { + const key = this.key(value); + + return client().get(key); + } + + 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 78708b40a..4e9d52901 100644 --- a/services/users.js +++ b/services/users.js @@ -17,7 +17,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'); @@ -34,9 +34,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. @@ -70,23 +69,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; } } @@ -97,9 +87,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; } @@ -717,7 +705,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'); } @@ -731,20 +719,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 251618187..09a8396c4 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;