diff --git a/src/upgrades/2.8.7/fix-email-sorted-sets.js b/src/upgrades/2.8.7/fix-email-sorted-sets.js new file mode 100644 index 0000000000..fcab69a8f4 --- /dev/null +++ b/src/upgrades/2.8.7/fix-email-sorted-sets.js @@ -0,0 +1,46 @@ +'use strict'; + + +const db = require('../../database'); +const batch = require('../../batch'); + + +module.exports = { + name: 'Fix user email sorted sets', + timestamp: Date.UTC(2023, 1, 4), + method: async function () { + const { progress } = this; + const bulkRemove = []; + await batch.processSortedSet('email:uid', async (data) => { + progress.incr(data.length); + const usersData = await db.getObjects(data.map(d => `user:${d.score}`)); + data.forEach((emailData, index) => { + const { score: uid, value: email } = emailData; + const userData = usersData[index]; + // user no longer exists or doesn't have email set in user hash + // remove the email/uid pair from email:uid, email:sorted + if (!userData || !userData.email) { + bulkRemove.push(['email:uid', email]); + bulkRemove.push(['email:sorted', `${email.toLowerCase()}:${uid}`]); + return; + } + + // user has email but doesn't match whats stored in user hash, gh#11259 + if (userData.email && userData.email.toLowerCase() !== email.toLowerCase()) { + bulkRemove.push(['email:uid', email]); + bulkRemove.push(['email:sorted', `${email.toLowerCase()}:${uid}`]); + } + }); + }, { + batch: 500, + withScores: true, + progress: progress, + }); + + await batch.processArray(bulkRemove, async (bulk) => { + await db.sortedSetRemoveBulk(bulk); + }, { + batch: 500, + }); + }, +}; diff --git a/src/user/email.js b/src/user/email.js index 1ea8bd551e..c6fc3274d4 100644 --- a/src/user/email.js +++ b/src/user/email.js @@ -39,7 +39,7 @@ UserEmail.remove = async function (uid, sessionId) { db.sortedSetRemove('email:uid', email.toLowerCase()), db.sortedSetRemove('email:sorted', `${email.toLowerCase()}:${uid}`), user.email.expireValidation(uid), - user.auth.revokeAllSessions(uid, sessionId), + sessionId ? user.auth.revokeAllSessions(uid, sessionId) : Promise.resolve(), events.log({ type: 'email-change', email, newEmail: '' }), ]); }; @@ -69,7 +69,7 @@ UserEmail.expireValidation = async (uid) => { }; UserEmail.canSendValidation = async (uid, email) => { - const pending = UserEmail.isValidationPending(uid, email); + const pending = await UserEmail.isValidationPending(uid, email); if (!pending) { return true; } @@ -196,6 +196,20 @@ UserEmail.confirmByUid = async function (uid) { throw new Error('[[error:invalid-email]]'); } + // If another uid has the same email throw error + const oldUid = await db.sortedSetScore('email:uid', currentEmail.toLowerCase()); + if (oldUid && oldUid !== parseInt(uid, 10)) { + throw new Error('[[error:email-taken]]'); + } + + const confirmedEmails = await db.getSortedSetRangeByScore(`email:uid`, 0, -1, uid, uid); + if (confirmedEmails.length) { + // remove old email of user by uid + await db.sortedSetsRemoveRangeByScore([`email:uid`], uid, uid); + await db.sortedSetRemoveBulk( + confirmedEmails.map(email => [`email:sorted`, `${email.toLowerCase()}:${uid}`]) + ); + } await Promise.all([ db.sortedSetAddBulk([ ['email:uid', uid, currentEmail.toLowerCase()], diff --git a/src/user/interstitials.js b/src/user/interstitials.js index 2a662785f9..aa70e8098f 100644 --- a/src/user/interstitials.js +++ b/src/user/interstitials.js @@ -42,6 +42,7 @@ Interstitials.email = async (data) => { callback: async (userData, formData) => { // Validate and send email confirmation if (userData.uid) { + const isSelf = parseInt(userData.uid, 10) === parseInt(data.req.uid, 10); const [isPasswordCorrect, canEdit, { email: current, 'email:confirmed': confirmed }, { allowed, error }] = await Promise.all([ user.isPasswordCorrect(userData.uid, formData.password, data.req.ip), privileges.users.canEdit(data.req.uid, userData.uid), @@ -68,13 +69,17 @@ Interstitials.email = async (data) => { if (formData.email === current) { if (confirmed) { throw new Error('[[error:email-nochange]]'); - } else if (await user.email.canSendValidation(userData.uid, current)) { + } else if (!await user.email.canSendValidation(userData.uid, current)) { throw new Error(`[[error:confirm-email-already-sent, ${meta.config.emailConfirmInterval}]]`); } } // Admins editing will auto-confirm, unless editing their own email if (isAdminOrGlobalMod && userData.uid !== data.req.uid) { + if (!await user.email.available(formData.email)) { + throw new Error('[[error:email-taken]]'); + } + await user.email.remove(userData.uid); await user.setUserField(userData.uid, 'email', formData.email); await user.email.confirmByUid(userData.uid); } else if (canEdit) { @@ -99,8 +104,8 @@ Interstitials.email = async (data) => { } if (current.length && (!hasPassword || (hasPassword && isPasswordCorrect) || isAdminOrGlobalMod)) { - // User explicitly clearing their email - await user.email.remove(userData.uid, data.req.session.id); + // User or admin explicitly clearing their email + await user.email.remove(userData.uid, isSelf ? data.req.session.id : null); } } } else { diff --git a/test/user/emails.js b/test/user/emails.js index f413a51283..47d3fcb6d0 100644 --- a/test/user/emails.js +++ b/test/user/emails.js @@ -120,7 +120,7 @@ describe('email confirmation (library methods)', () => { await user.email.sendValidationEmail(uid, { email, }); - const ok = await user.email.canSendValidation(uid, 'test@example.com'); + const ok = await user.email.canSendValidation(uid, email); assert.strictEqual(ok, false); }); @@ -131,7 +131,7 @@ describe('email confirmation (library methods)', () => { email, }); await db.pexpire(`confirm:byUid:${uid}`, 1000); - const ok = await user.email.canSendValidation(uid, 'test@example.com'); + const ok = await user.email.canSendValidation(uid, email); assert(ok); });