Skip to content

Commit

Permalink
refactor(emails): more work in update email interstitial, interstitia…
Browse files Browse the repository at this point in the history
…l skipping, email change on confirmation, deprecation of requireEmailConfirmation
  • Loading branch information
julianlam committed Jul 30, 2021
1 parent f365bc4 commit 69c96dd
Show file tree
Hide file tree
Showing 17 changed files with 24 additions and 30 deletions.
1 change: 0 additions & 1 deletion install/data/defaults.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
"profileImageDimension": 200,
"profile:convertProfileImageToPNG": 0,
"profile:keepAllUserImages": 0,
"requireEmailConfirmation": 0,
"gdpr_enabled": 1,
"allowProfileImageUploads": 1,
"teaserPost": "last-reply",
Expand Down
2 changes: 0 additions & 2 deletions public/openapi/read/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ get:
description: A user identifier
cache-buster:
type: string
requireEmailConfirmation:
type: boolean
topicPostSort:
type: string
categoryTopicSort:
Expand Down
2 changes: 1 addition & 1 deletion public/src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ app.cacheBuster = null;
};

app.showEmailConfirmWarning = function (err) {
if (!config.requireEmailConfirmation || !app.user.uid) {
if (!app.user.uid) {
return;
}
var msg = {
Expand Down
4 changes: 0 additions & 4 deletions src/api/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ usersAPI.update = async function (caller, data) {
await events.log(eventData);
}

if (userData.email !== oldUserData.email) {
await log('email-change', { oldEmail: oldUserData.email, newEmail: userData.email });
}

if (userData.username !== oldUserData.username) {
await log('username-change', { oldUsername: oldUserData.username, newUsername: userData.username });
}
Expand Down
1 change: 0 additions & 1 deletion src/controllers/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ apiController.loadConfig = async function (req) {
loggedIn: !!req.user,
uid: req.uid,
'cache-buster': meta.config['cache-buster'] || '',
requireEmailConfirmation: meta.config.requireEmailConfirmation === 1,
topicPostSort: meta.config.topicPostSort || 'oldest_to_newest',
categoryTopicSort: meta.config.categoryTopicSort || 'newest_to_oldest',
csrf_token: req.uid >= 0 && req.csrfToken && req.csrfToken(),
Expand Down
7 changes: 5 additions & 2 deletions src/controllers/authentication.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ const sockets = require('../socket.io');
const authenticationController = module.exports;

async function registerAndLoginUser(req, res, userData) {
if (!userData.email) {
userData.updateEmail = true;
}

const data = await plugins.hooks.fire('filter:register.interstitial', {
userData: userData,
userData,
interstitials: [],
});

Expand All @@ -33,7 +37,6 @@ async function registerAndLoginUser(req, res, userData) {

if (deferRegistration) {
userData.register = true;
userData.updateEmail = true;
req.session.registration = userData;

if (req.body.noscript === 'true') {
Expand Down
2 changes: 1 addition & 1 deletion src/emailer.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ Emailer.send = async (template, uid, params) => {
}

const allowedTpls = ['verify_email', 'welcome', 'registration_accepted'];
if (meta.config.requireEmailConfirmation && !userData['email:confirmed'] && !allowedTpls.includes(template)) {
if (!userData['email:confirmed'] && !allowedTpls.includes(template)) {
if (process.env.NODE_ENV === 'development') {
winston.warn(`uid : ${uid} (${userData.email}) has not confirmed email, not sending "${template}" email.`);
}
Expand Down
1 change: 1 addition & 0 deletions src/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ events.types = [
'user-deleteAccount',
'user-deleteContent',
'password-change',
'email-confirmation-sent',
'email-change',
'username-change',
'ip-blacklist-save',
Expand Down
2 changes: 1 addition & 1 deletion src/middleware/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ middleware.renderHeader = async function renderHeader(req, res, data) {
isModerator: user.isModeratorOfAnyCategory(req.uid),
privileges: privileges.global.get(req.uid),
user: user.getUserData(req.uid),
isEmailConfirmSent: (!meta.config.requireEmailConfirmation || req.uid <= 0) ? false : await db.get(`uid:${req.uid}:confirm:email:sent`),
isEmailConfirmSent: req.uid <= 0 ? false : await db.get(`uid:${req.uid}:confirm:email:sent`),
languageDirection: translator.translate('[[language:dir]]', res.locals.config.userLang),
timeagoCode: languages.userTimeagoCode(res.locals.config.userLang),
browserTitle: translator.translate(controllers.helpers.buildTitle(translator.unescape(data.title))),
Expand Down
2 changes: 2 additions & 0 deletions src/middleware/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ module.exports = function (middleware) {
return setImmediate(next);
}

console.log('middleware hit', req.session);

const path = req.path.startsWith('/api/') ? req.path.replace('/api', '') : req.path;
const { allowed } = await plugins.hooks.fire('filter:middleware.registrationComplete', {
allowed: ['/register/complete'],
Expand Down
5 changes: 0 additions & 5 deletions src/socket.io/admin/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const api = require('../../api');
const groups = require('../../groups');
const user = require('../../user');
const events = require('../../events');
const meta = require('../../meta');
const translator = require('../../translator');
const sockets = require('..');

Expand Down Expand Up @@ -81,10 +80,6 @@ User.sendValidationEmail = async function (socket, uids) {
throw new Error('[[error:invalid-data]]');
}

if (!meta.config.requireEmailConfirmation) {
throw new Error('[[error:email-confirmations-are-disabled]]');
}

const failed = [];
let errorLogged = false;
await async.eachLimit(uids, 50, async (uid) => {
Expand Down
4 changes: 0 additions & 4 deletions src/socket.io/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ SocketUser.emailConfirm = async function (socket) {
throw new Error('[[error:no-privileges]]');
}

if (!meta.config.requireEmailConfirmation) {
throw new Error('[[error:email-confirmations-are-disabled]]');
}

return await user.email.sendValidationEmail(socket.uid);
};

Expand Down
2 changes: 2 additions & 0 deletions src/upgrades/1.15.0/verified_users_group.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ async function updatePrivilges() {
// if email confirmation is required
// give chat, posting privs to "verified-users" group
// remove chat, posting privs from "registered-users" group

// This config property has been removed from v1.18.0+, but is still present in old datasets
if (meta.config.requireEmailConfirmation) {
const cids = await db.getSortedSetRevRange('categories:cid', 0, -1);
const canChat = await privileges.global.canGroup('chat', 'registered-users');
Expand Down
2 changes: 1 addition & 1 deletion src/user/digest.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ Digest.send = async function (data) {
let errorLogged = false;
await batch.processArray(data.subscribers, async (uids) => {
let userData = await user.getUsersFields(uids, ['uid', 'email', 'email:confirmed', 'username', 'userslug', 'lastonline']);
userData = userData.filter(u => u && u.email && (!meta.config.requireEmailConfirmation || u['email:confirmed']));
userData = userData.filter(u => u && u.email && (meta.config.includeUnverifiedEmails || u['email:confirmed']));
if (!userData.length) {
return;
}
Expand Down
9 changes: 9 additions & 0 deletions src/user/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const db = require('../database');
const meta = require('../meta');
const emailer = require('../emailer');
const groups = require('../groups');
const events = require('../events');

const UserEmail = module.exports;

Expand Down Expand Up @@ -69,6 +70,13 @@ UserEmail.sendValidationEmail = async function (uid, options) {
await db.expireAt(`confirm:${confirm_code}`, Math.floor((Date.now() / 1000) + (60 * 60 * 24)));
const username = await user.getUserField(uid, 'username');

events.log({
type: 'email-confirmation-sent',
uid,
confirm_code,
...options,
});

const data = {
username: username,
confirm_link: confirm_link,
Expand Down Expand Up @@ -104,6 +112,7 @@ UserEmail.confirmByCode = async function (code) {
await db.sortedSetRemove('email:uid', oldEmail.toLowerCase());
await db.sortedSetRemove('email:sorted', `${oldEmail.toLowerCase()}:${confirmObj.uid}`);
await user.auth.revokeAllSessions(confirmObj.uid);
await events.log('email-change', { oldEmail, newEmail: confirmObj.email });
}

await Promise.all([
Expand Down
1 change: 1 addition & 0 deletions src/user/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ module.exports = function (User) {
}

if (newEmail) {
await db.delete(`uid:${uid}:confirm:email:sent`);
await User.email.sendValidationEmail(uid, {
email: newEmail,
subject: '[[email:email.verify-your-email.subject]]',
Expand Down
7 changes: 0 additions & 7 deletions src/views/admin/settings/user.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,6 @@
<div class="col-sm-2 col-xs-12 settings-header">[[admin/settings/user:authentication]]</div>
<div class="col-sm-10 col-xs-12">
<form role="form">
<div class="checkbox">
<label class="mdl-switch mdl-js-switch mdl-js-ripple-effect">
<input class="mdl-switch__input" type="checkbox" data-field="requireEmailConfirmation">
<span class="mdl-switch__label"><strong>[[admin/settings/user:require-email-confirmation]]</strong></span>
</label>
</div>

<div class="form-group form-inline">
<label for="emailConfirmInterval">[[admin/settings/user:email-confirm-interval]]</label>
<input class="form-control" data-field="emailConfirmInterval" type="number" id="emailConfirmInterval" placeholder="Default: 10"
Expand Down

0 comments on commit 69c96dd

Please sign in to comment.