diff --git a/routes/api/account/index.js b/routes/api/account/index.js index 99cb9a468..85558d33c 100644 --- a/routes/api/account/index.js +++ b/routes/api/account/index.js @@ -50,22 +50,18 @@ router.post('/password/reset', async (req, res, next) => { try { let token = await UsersService.createPasswordResetToken(email, loc); - if (!token) { - res.status(204).end(); - return; + if (token) { + await mailer.sendSimple({ + template: 'password-reset', + locals: { + token, + rootURL: ROOT_URL + }, + subject: 'Password Reset', + to: email + }); } - // Send the password reset email. - await mailer.sendSimple({ - template: 'password-reset', // needed to know which template to render! - locals: { // specifies the template locals. - token, - rootURL: ROOT_URL - }, - subject: 'Password Reset', - to: email - }); - res.status(204).end(); } catch (e) { return next(e); diff --git a/services/domainlist.js b/services/domainlist.js index 29673e2b6..a75d62dca 100644 --- a/services/domainlist.js +++ b/services/domainlist.js @@ -2,6 +2,8 @@ const debug = require('debug')('talk:services:domainlist'); const _ = require('lodash'); const SettingsService = require('./settings'); +const {ROOT_URL} = require('../config'); + /** * The root domainlist object. * @type {Object} @@ -17,31 +19,24 @@ class Domainlist { /** * Loads domains white list in from the database */ - load() { - return SettingsService - .retrieve() - .then((settings) => { - - // Insert the settings domains whitelist. - this.upsert(settings.domains); - }); + async load() { + const {domains} = await SettingsService.retrieve(); + this.upsert(domains); } /** * Inserts the domains whitelist data * @param {Array} list list of domains to be set to the whitelist */ - upsert(lists) { + async upsert(lists) { // Add the domains to this array and also be sure are all unique domains if (!('whitelist' in lists)) { return; } - this.lists['whitelist'] = Domainlist.parseList(lists['whitelist']); - debug(`Added ${lists['whitelist'].length} domains to the whitelist.`); - - return Promise.resolve(this); + this.lists.whitelist = Domainlist.parseList(lists.whitelist); + debug(`Added ${lists.whitelist.length} domains to the whitelist.`); } /** @@ -51,19 +46,22 @@ class Domainlist { */ match(list, url) { + // Parse the url that we're matching with. const domainToMatch = Domainlist.parseURL(url); // This will return true in the event that at least one blockword is found // in the phrase. - for (let i = 0; i < list.length; i++) { - if (list[i] === domainToMatch) { - return true; - } - } + return list.indexOf(domainToMatch) >= 0; + } - // We've walked over all the whitelisted domains, and haven't had a - // mismatch... It is not an allowed domain! - return false; + /** + * Checks to see if the passed url matches the domain of the root path. + * + * @param {String} url + * @returns {Boolean} true if the domains match + */ + static matchMount(url) { + return Domainlist.parseURL(url) === Domainlist.parseURL(ROOT_URL); } /** @@ -84,7 +82,7 @@ class Domainlist { let domain; // removes protocol and get domain - if (url.indexOf('://') > -1) { + if (url.indexOf('//') > -1) { domain = url.split('/')[2]; } else { domain = url.split('/')[0]; @@ -96,13 +94,14 @@ class Domainlist { return domain.toLowerCase(); } - static urlCheck(url) { + static async urlCheck(url) { const dl = new Domainlist(); - return dl.load() - .then(() => { - return dl.match(dl.lists['whitelist'], url); - }); + // Load the domain list. + await dl.load(); + + // Perform a match. + return dl.match(dl.lists.whitelist, url); } } diff --git a/services/users.js b/services/users.js index 35810ef7f..faea67d75 100644 --- a/services/users.js +++ b/services/users.js @@ -1,8 +1,6 @@ const assert = require('assert'); const uuid = require('uuid'); const bcrypt = require('bcryptjs'); -const url = require('url'); -const Wordlist = require('./wordlist'); const errors = require('../errors'); const { @@ -22,9 +20,10 @@ const USER_ROLES = require('../models/enum/user_roles'); const RECAPTCHA_WINDOW_SECONDS = 60 * 10; // 10 minutes. const RECAPTCHA_INCORRECT_TRIGGER = 5; // after 3 incorrect attempts, recaptcha will be required. -const SettingsService = require('./settings'); const ActionsService = require('./actions'); const MailerService = require('./mailer'); +const Wordlist = require('./wordlist'); +const Domainlist = require('./domainlist'); const EMAIL_CONFIRM_JWT_SUBJECT = 'email_confirm'; const PASSWORD_RESET_JWT_SUBJECT = 'password_reset'; @@ -557,11 +556,10 @@ module.exports = class UsersService { email = email.toLowerCase(); - const [user, settings] = await Promise.all([ + const [user, domainValidated] = await Promise.all([ UserModel.findOne({profiles: {$elemMatch: {id: email}}}), - SettingsService.retrieve(), + Domainlist.urlCheck(loc), ]); - if (!user) { // Since we don't want to reveal that the email does/doesn't exist @@ -569,19 +567,11 @@ module.exports = class UsersService { // endpoint. return; } - let redirectDomain; - try { - const {hostname, port} = url.parse(loc); - redirectDomain = hostname; - if (port) { - redirectDomain += `:${port}`; - } - } catch (e) { - throw new Error('redirect location is invalid'); - } - if (settings.domains.whitelist.indexOf(redirectDomain) === -1) { - throw new Error('redirect location is not on the list of acceptable domains'); + // If the domain didn't match any of the whitelisted domains and if it + // didn't match the mount domain, then throw an error. + if (!domainValidated && !Domainlist.matchMount(loc)) { + throw new Error('user supplied location exists on non-permitted domain'); } const payload = { @@ -619,16 +609,14 @@ module.exports = class UsersService { * Verifies a jwt and returns the associated user. * @param {String} token the JSON Web Token to verify */ - static verifyPasswordResetToken(token) { - return UsersService - .verifyToken(token, { - subject: PASSWORD_RESET_JWT_SUBJECT - }) + static async verifyPasswordResetToken(token) { + const {userId, loc} = await UsersService.verifyToken(token, { + subject: PASSWORD_RESET_JWT_SUBJECT + }); - // TODO: add search by __v as well - .then((decoded) => { - return Promise.all([UsersService.findById(decoded.userId), decoded.loc]); - }); + const user = await UsersService.findById(userId); + + return [user, loc]; } /** diff --git a/test/server/services/domainlist.js b/test/server/services/domainlist.js index 9dafa438a..33afdda87 100644 --- a/test/server/services/domainlist.js +++ b/test/server/services/domainlist.js @@ -26,13 +26,84 @@ describe('services.Domainlist', () => { }); + describe('#parseURL', () => { + it('parses the domain correctly', () => { + [ + ['http://google.ca/test', 'google.ca'], + ['http://google.ca:80/test', 'google.ca'], + ['https://google.ca/test', 'google.ca'], + ['https://google.ca:443/test', 'google.ca'], + ['//google.ca/test', 'google.ca'], + ['//google.ca:80/test', 'google.ca'], + ['//google.ca:443/test', 'google.ca'], + ['google.ca/test', 'google.ca'], + ['google.ca:80/test', 'google.ca'], + ['google.ca:443/test', 'google.ca'], + ['http://google.ca/', 'google.ca'], + ['http://google.ca:80/', 'google.ca'], + ['https://google.ca/', 'google.ca'], + ['https://google.ca:443/', 'google.ca'], + ['//google.ca/', 'google.ca'], + ['//google.ca:80/', 'google.ca'], + ['//google.ca:443/', 'google.ca'], + ['google.ca/', 'google.ca'], + ['google.ca:80/', 'google.ca'], + ['google.ca:443/', 'google.ca'], + ['google.ca', 'google.ca'], + ['http://google.ca', 'google.ca'], + ['http://google.ca:80', 'google.ca'], + ['https://google.ca', 'google.ca'], + ['https://google.ca:443', 'google.ca'], + ['//google.ca', 'google.ca'], + ['//google.ca:80', 'google.ca'], + ['//google.ca:443', 'google.ca'], + ['google.ca', 'google.ca'], + ['google.ca:80', 'google.ca'], + ['google.ca:443', 'google.ca'], + ['http://google.Ca/test', 'google.ca'], + ['http://google.ca:80/test', 'google.ca'], + ['https://google.Ca/test', 'google.ca'], + ['https://google.ca:443/test', 'google.ca'], + ['//google.Ca/test', 'google.ca'], + ['//google.Ca:80/test', 'google.ca'], + ['//google.Ca:443/test', 'google.ca'], + ['google.Ca/test', 'google.ca'], + ['google.ca:80/test', 'google.ca'], + ['google.ca:443/test', 'google.ca'], + ['http://Google.ca/', 'google.ca'], + ['http://google.Ca:80/', 'google.ca'], + ['https://Google.ca/', 'google.ca'], + ['https://google.Ca:443/', 'google.ca'], + ['//Google.ca/', 'google.ca'], + ['//google.Ca:80/', 'google.ca'], + ['//google.Ca:443/', 'google.ca'], + ['Google.ca/', 'google.ca'], + ['google.Ca:80/', 'google.ca'], + ['google.Ca:443/', 'google.ca'], + ['Google.ca', 'google.ca'], + ['http://Google.ca', 'google.ca'], + ['http://google.Ca:80', 'google.ca'], + ['https://Google.ca', 'google.ca'], + ['https://google.Ca:443', 'google.ca'], + ['//Google.ca', 'google.ca'], + ['//google.Ca:80', 'google.ca'], + ['//google.Ca:443', 'google.ca'], + ['Google.ca', 'google.ca'], + ['google.Ca:80', 'google.ca'], + ['google.Ca:443', 'google.ca'], + ].forEach(([domain, hostname]) => { + expect(Domainlist.parseURL(domain), `domain ${domain} should be parsed as ${hostname}`).to.equal(hostname); + }); + }); + }); + describe('#match', () => { const whiteList = Domainlist.parseList(domainlists['whitelist']); it('does match on an included domain', () => { [ - 'wapo.com', + 'http://wapo.com', 'nytimes.com' ].forEach((domain) => { expect(domainlist.match(whiteList, domain)).to.be.true;