From 0322e984e0f559973064b227615a1497601ffd1c Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 3 Feb 2022 15:41:40 -0500 Subject: [PATCH 1/5] fix: #10236, don't check email:uid, instead verify an email confirmation is active --- src/controllers/write/users.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/controllers/write/users.js b/src/controllers/write/users.js index 3ab37025ca..140cf6c171 100644 --- a/src/controllers/write/users.js +++ b/src/controllers/write/users.js @@ -265,8 +265,8 @@ Users.getEmail = async (req, res) => { }; Users.confirmEmail = async (req, res) => { - const [exists, canManage] = await Promise.all([ - db.isSortedSetMember('email:uid', req.params.email.toLowerCase()), + const [pending, canManage] = await Promise.all([ + user.email.isValidationPending(req.params.uid, req.params.email), privileges.admin.can('admin:users', req.uid), ]); @@ -274,8 +274,9 @@ Users.confirmEmail = async (req, res) => { helpers.notAllowed(req, res); } - if (exists) { - await user.email.confirmByUid(req.params.uid); + if (pending) { + const code = await db.get(`confirm:byUid:${req.params.uid}`); + await user.email.confirmByCode(code, req.session.id); helpers.formatApiResponse(200, res); } else { helpers.formatApiResponse(404, res); From 936562c3cb04ae22edbb83c4bbe9e25af16aa643 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 3 Feb 2022 15:49:13 -0500 Subject: [PATCH 2/5] fix: handle case where email is explicitly passed into user.create, and thus is set in user hash, but confirmation request may have expired --- src/controllers/write/users.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/controllers/write/users.js b/src/controllers/write/users.js index 140cf6c171..026e5db13d 100644 --- a/src/controllers/write/users.js +++ b/src/controllers/write/users.js @@ -265,8 +265,9 @@ Users.getEmail = async (req, res) => { }; Users.confirmEmail = async (req, res) => { - const [pending, canManage] = await Promise.all([ + const [pending, current, canManage] = await Promise.all([ user.email.isValidationPending(req.params.uid, req.params.email), + user.getUserField(req.params.uid, 'email'), privileges.admin.can('admin:users', req.uid), ]); @@ -274,10 +275,13 @@ Users.confirmEmail = async (req, res) => { helpers.notAllowed(req, res); } - if (pending) { + if (pending) { // has active confirmation request const code = await db.get(`confirm:byUid:${req.params.uid}`); await user.email.confirmByCode(code, req.session.id); helpers.formatApiResponse(200, res); + } else if (current && current === req.params.email) { // email in user hash (i.e. email passed into user.create) + await user.email.confirmByUid(req.params.uid); + helpers.formatApiResponse(200, res); } else { helpers.formatApiResponse(404, res); } From ad635175112eda19010f7ac023ea7036f4c76685 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 3 Feb 2022 15:49:35 -0500 Subject: [PATCH 3/5] fix: missing early return --- src/controllers/write/users.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/write/users.js b/src/controllers/write/users.js index 026e5db13d..4cea3af1da 100644 --- a/src/controllers/write/users.js +++ b/src/controllers/write/users.js @@ -272,7 +272,7 @@ Users.confirmEmail = async (req, res) => { ]); if (!canManage) { - helpers.notAllowed(req, res); + return helpers.notAllowed(req, res); } if (pending) { // has active confirmation request From d1b1f50bb247c5d7fe07835d2d40387f25375891 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 3 Feb 2022 15:55:52 -0500 Subject: [PATCH 4/5] test: stricter isValidationPending check --- test/user.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/user.js b/test/user.js index c18b5bfc1f..e1a7149c46 100644 --- a/test/user.js +++ b/test/user.js @@ -983,7 +983,7 @@ describe('User', () => { await User.email.expireValidation(uid); await apiUser.update({ uid: uid }, { uid: uid, email: 'updatedAgain@me.com', password: '123456' }); - assert.strictEqual(await User.email.isValidationPending(uid), true); + assert.strictEqual(await User.email.isValidationPending(uid, 'updatedAgain@me.com'.toLowerCase()), true); }); it('should update cover image', (done) => { From aa8914a1535064171b760db3f5ad21287e50d4c7 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 3 Feb 2022 16:49:41 -0500 Subject: [PATCH 5/5] feat: v3 user email tests --- test/user.js | 15 +++++++ test/user/emails.js | 107 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 test/user/emails.js diff --git a/test/user.js b/test/user.js index e1a7149c46..63f200f198 100644 --- a/test/user.js +++ b/test/user.js @@ -20,6 +20,7 @@ const groups = require('../src/groups'); const messaging = require('../src/messaging'); const helpers = require('./helpers'); const meta = require('../src/meta'); +const file = require('../src/file'); const socketUser = require('../src/socket.io/user'); const apiUser = require('../src/api/users'); @@ -2849,4 +2850,18 @@ describe('User', () => { }); }); }); + + describe('User\'s', async () => { + let files; + + before(async () => { + files = await file.walk(path.resolve(__dirname, './user')); + }); + + it('subfolder tests', () => { + files.forEach((filePath) => { + require(filePath); + }); + }); + }); }); diff --git a/test/user/emails.js b/test/user/emails.js new file mode 100644 index 0000000000..c99f47ff88 --- /dev/null +++ b/test/user/emails.js @@ -0,0 +1,107 @@ +'use strict'; + +const assert = require('assert'); +const nconf = require('nconf'); +const util = require('util'); + +const db = require('../mocks/databasemock'); + +const helpers = require('../helpers'); + +const user = require('../../src/user'); +const groups = require('../../src/groups'); + +describe('email confirmation (v3 api)', () => { + let userObj; + let jar; + const register = data => new Promise((resolve, reject) => { + helpers.registerUser(data, (err, jar, response, body) => { + if (err) { + return reject(err); + } + + resolve({ jar, response, body }); + }); + }); + const login = util.promisify(helpers.loginUser); + + before(async () => { + // If you're running this file directly, uncomment these lines + await register({ + username: 'fake-user', + password: 'derpioansdosa', + email: 'b@c.com', + gdpr_consent: true, + }); + + ({ body: userObj, jar } = await register({ + username: 'email-test', + password: 'abcdef', + email: 'test@example.org', + gdpr_consent: true, + })); + }); + + it('should have a pending validation', async () => { + const code = await db.get(`confirm:byUid:${userObj.uid}`); + assert.strictEqual(await user.email.isValidationPending(userObj.uid, 'test@example.org'), true); + }); + + it('should not list their email', async () => { + const { res, body } = await helpers.request('get', `/api/v3/users/${userObj.uid}/emails`, { + jar, + json: true, + }); + + assert.strictEqual(res.statusCode, 200); + assert.deepStrictEqual(body, JSON.parse('{"status":{"code":"ok","message":"OK"},"response":{"emails":[]}}')); + }); + + it('should not allow confirmation if they are not an admin', async () => { + const { res } = await helpers.request('post', `/api/v3/users/${userObj.uid}/emails/${encodeURIComponent('test@example.org')}/confirm`, { + jar, + json: true, + }); + + assert.strictEqual(res.statusCode, 403); + }); + + it('should not confirm an email that is not pending or set', async () => { + await groups.join('administrators', userObj.uid); + const { res, body } = await helpers.request('post', `/api/v3/users/${userObj.uid}/emails/${encodeURIComponent('fake@example.org')}/confirm`, { + jar, + json: true, + }); + + assert.strictEqual(res.statusCode, 404); + await groups.leave('administrators', userObj.uid); + }); + + it('should confirm their email (using the pending validation)', async () => { + await groups.join('administrators', userObj.uid); + const { res, body } = await helpers.request('post', `/api/v3/users/${userObj.uid}/emails/${encodeURIComponent('test@example.org')}/confirm`, { + jar, + json: true, + }); + + assert.strictEqual(res.statusCode, 200); + assert.deepStrictEqual(body, JSON.parse('{"status":{"code":"ok","message":"OK"},"response":{}}')); + await groups.leave('administrators', userObj.uid); + }); + + it('should still confirm the email (as email is set in user hash)', async () => { + await user.email.remove(userObj.uid); + await user.setUserField(userObj.uid, 'email', 'test@example.org'); + ({ jar } = await login('email-test', 'abcdef')); // email removal logs out everybody + await groups.join('administrators', userObj.uid); + + const { res, body } = await helpers.request('post', `/api/v3/users/${userObj.uid}/emails/${encodeURIComponent('test@example.org')}/confirm`, { + jar, + json: true, + }); + + assert.strictEqual(res.statusCode, 200); + assert.deepStrictEqual(body, JSON.parse('{"status":{"code":"ok","message":"OK"},"response":{}}')); + await groups.leave('administrators', userObj.uid); + }); +});