diff --git a/public/openapi/read/user/email/email.yaml b/public/openapi/read/user/email/email.yaml index 48af02c8f5..131a37fb20 100644 --- a/public/openapi/read/user/email/email.yaml +++ b/public/openapi/read/user/email/email.yaml @@ -2,7 +2,9 @@ get: tags: - users summary: Get user by email - description: This route retrieves a user's public profile data. If the calling user is the same as the profile, then it will also return data the user elected to hide (e.g. email/fullname) + description: | + This route retrieves a user's public profile data. If the calling user is the same as the profile, then it will also return data the user elected to hide (e.g. email/fullname). + Additionally, this route will only return data if the calling user is an admin or global moderator, or if the end user has elected to make their email public. Otherwise, it will simply return a `404 Not Found`. parameters: - name: email in: path diff --git a/src/controllers/user.js b/src/controllers/user.js index 1692c618fb..9e038ea2c6 100644 --- a/src/controllers/user.js +++ b/src/controllers/user.js @@ -47,8 +47,9 @@ userController.getUserDataByField = async function (callerUid, field, fieldValue } else if (field === 'email') { uid = await user.getUidByEmail(fieldValue); if (uid) { + const isPrivileged = await user.isAdminOrGlobalMod(callerUid); const settings = await user.getSettings(uid); - if (settings && !settings.showemail) { + if (!isPrivileged && (settings && !settings.showemail)) { uid = 0; } } diff --git a/src/user/index.js b/src/user/index.js index c7151c49de..a36de7bf38 100644 --- a/src/user/index.js +++ b/src/user/index.js @@ -263,10 +263,7 @@ User.addInterstitials = function (callback) { User.isAdminOrGlobalMod(data.req.uid), privileges.users.canEdit(data.req.uid, userData.uid), ]); - if (isAdminOrGlobalMod) { - await User.setUserField(userData.uid, 'email', formData.email); - await User.email.confirmByUid(userData.uid); - } else if (canEdit) { + if (isAdminOrGlobalMod || canEdit) { await User.email.sendValidationEmail(userData.uid, { email: formData.email, force: true, diff --git a/test/api.js b/test/api.js index 38d16e45cc..fa59ffeda7 100644 --- a/test/api.js +++ b/test/api.js @@ -207,6 +207,7 @@ describe('API', async () => { method: dummyEmailerHook, }); + // All tests run as admin user jar = await helpers.loginUser('admin', '123456'); // Retrieve CSRF token using cookie, to test Write API diff --git a/test/controllers.js b/test/controllers.js index bd842a3e60..5696ea7073 100644 --- a/test/controllers.js +++ b/test/controllers.js @@ -4,6 +4,7 @@ const async = require('async'); const assert = require('assert'); const nconf = require('nconf'); const request = require('request'); +const requestAsync = require('request-promise-native'); const fs = require('fs'); const path = require('path'); @@ -35,8 +36,11 @@ describe('Controllers', () => { description: 'Test category created by testing script', }, next); }, - user: function (next) { - user.create({ username: 'foo', password: 'barbar', email: 'foo@test.com' }, next); + user: async () => { + const uid = await user.create({ username: 'foo', password: 'barbar', gdpr_consent: true }); + await user.setUserField(uid, 'email', 'foo@test.com'); + await user.email.confirmByUid(uid); + return uid; }, navigation: function (next) { const navigation = require('../src/navigation/admin'); @@ -1342,13 +1346,23 @@ describe('Controllers', () => { }); }); - it('should load user by email', (done) => { - request(`${nconf.get('url')}/api/user/email/foo@test.com`, (err, res, body) => { - assert.ifError(err); - assert.equal(res.statusCode, 200); - assert(body); - done(); + it('should NOT load user by email (by default)', async () => { + const res = await requestAsync(`${nconf.get('url')}/api/user/email/foo@test.com`, { + resolveWithFullResponse: true, + simple: false, + }); + + assert.strictEqual(res.statusCode, 404); + }); + + it('should load user by email if user has elected to show their email', async () => { + await user.setSetting(fooUid, 'showemail', 1); + const res = await requestAsync(`${nconf.get('url')}/api/user/email/foo@test.com`, { + resolveWithFullResponse: true, }); + assert.strictEqual(res.statusCode, 200); + assert(res.body); + await user.setSetting(fooUid, 'showemail', 0); }); it('should return 401 if user does not have view:users privilege', (done) => { @@ -1551,11 +1565,21 @@ describe('Controllers', () => { }); }); - it('should render edit/email', (done) => { - request(`${nconf.get('url')}/api/user/foo/edit/email`, { jar: jar, json: true }, (err, res, body) => { - assert.ifError(err); - assert.equal(res.statusCode, 200); - done(); + it('should render edit/email', async () => { + const res = await requestAsync(`${nconf.get('url')}/api/user/foo/edit/email`, { + jar, + json: true, + resolveWithFullResponse: true, + }); + + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.body, '/register/complete'); + + await requestAsync({ + uri: `${nconf.get('url')}/register/abort`, + method: 'post', + jar, + simple: false, }); }); diff --git a/test/flags.js b/test/flags.js index 3a7fa5acd2..d812a9d939 100644 --- a/test/flags.js +++ b/test/flags.js @@ -685,8 +685,8 @@ describe('Flags', () => { throw err; } - // 1 for the new event appended, 2 for username and email change - assert.strictEqual(entries + 3, history.length); + // 1 for the new event appended, 1 for username change (email not changed immediately) + assert.strictEqual(entries + 2, history.length); done(); }); }); diff --git a/test/user.js b/test/user.js index e5e4c898b3..f2e4288227 100644 --- a/test/user.js +++ b/test/user.js @@ -69,8 +69,11 @@ describe('User', () => { describe('.create(), when created', () => { it('should be created properly', async () => { - testUid = await User.create({ username: userData.username, password: userData.password, email: userData.email }); + testUid = await User.create({ username: userData.username, password: userData.password }); assert.ok(testUid); + + await User.setUserField(testUid, 'email', userData.email); + await User.email.confirmByUid(testUid); }); it('should be created properly', async () => { @@ -559,12 +562,10 @@ describe('User', () => { describe('passwordReset', () => { let uid; let code; - before((done) => { - User.create({ username: 'resetuser', password: '123456', email: 'reset@me.com' }, (err, newUid) => { - assert.ifError(err); - uid = newUid; - done(); - }); + before(async () => { + uid = await User.create({ username: 'resetuser', password: '123456' }); + await User.setUserField(uid, 'email', 'reset@me.com'); + await User.email.confirmByUid(uid); }); it('.generate() should generate a new reset code', (done) => { @@ -1013,27 +1014,6 @@ describe('User', () => { assert.strictEqual(await User.email.isValidationPending(uid), true); }); - it('should error if email is identical', async () => { - await User.create({ - username: 'trimtest1', - email: 'trim1@trim.com', - }); - const uid2 = await User.create({ - username: 'trimtest2', - email: 'trim2@trim.com', - }); - let err; - try { - await socketUser.changeUsernameEmail({ uid: uid2 }, { - uid: uid2, - email: ' trim1@trim.com', - }); - } catch (_err) { - err = _err; - } - assert.strictEqual(err.message, '[[error:email-taken]]'); - }); - it('should update cover image', (done) => { const position = '50.0301% 19.2464%'; socketUser.updateCover({ uid: uid }, { uid: uid, imageData: goodImage, position: position }, (err, result) => {