Skip to content

Commit

Permalink
fix(emails): broken test for api/user/email/:email
Browse files Browse the repository at this point in the history
+ fixed broken tests due to unexpected behaviour for email confirmation
  • Loading branch information
julianlam committed Jul 30, 2021
1 parent c4e3362 commit 81611ae
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 49 deletions.
4 changes: 3 additions & 1 deletion public/openapi/read/user/email/email.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/controllers/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
5 changes: 1 addition & 4 deletions src/user/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions test/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 37 additions & 13 deletions test/controllers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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,
});
});

Expand Down
4 changes: 2 additions & 2 deletions test/flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand Down
36 changes: 8 additions & 28 deletions test/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down

0 comments on commit 81611ae

Please sign in to comment.