From e8a4a88e874031d373614de13b8577a9a8f25345 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Wed, 12 Jan 2022 10:19:30 -0600 Subject: [PATCH] Merge pull request #6238 from overleaf/jel-script-remove-email [web] Script to remove email GitOrigin-RevId: c8f1a69259904b08ef39181b8b7e9c3150ea59f0 --- .../web/app/src/Features/User/UserUpdater.js | 75 ++++++++------ services/web/scripts/remove_email.js | 56 +++++++++++ .../test/unit/src/User/UserUpdaterTests.js | 99 ++++++++++++++++--- 3 files changed, 188 insertions(+), 42 deletions(-) create mode 100644 services/web/scripts/remove_email.js diff --git a/services/web/app/src/Features/User/UserUpdater.js b/services/web/app/src/Features/User/UserUpdater.js index 9008e8ffa4..aad4f46b4e 100644 --- a/services/web/app/src/Features/User/UserUpdater.js +++ b/services/web/app/src/Features/User/UserUpdater.js @@ -8,7 +8,6 @@ const { callbackify, promisify } = require('util') const UserGetter = require('./UserGetter') const { addAffiliation, - removeAffiliation, promises: InstitutionsAPIPromises, } = require('../Institutions/InstitutionsAPI') const Features = require('../../infrastructure/Features') @@ -232,6 +231,51 @@ async function confirmEmail(userId, email) { await FeaturesUpdater.promises.refreshFeatures(userId, 'confirm-email') } +async function removeEmailAddress(userId, email, skipParseEmail = false) { + // remove one of the user's email addresses. The email cannot be the user's + // default email address + if (!skipParseEmail) { + email = EmailHelper.parseEmail(email) + } else if (skipParseEmail && typeof email !== 'string') { + throw new Error('email must be a string') + } + + if (!email) { + throw new Error('invalid email') + } + + const isMainEmail = await UserGetter.promises.getUserByMainEmail(email, { + _id: 1, + }) + if (isMainEmail) { + throw new Error('cannot remove primary email') + } + + try { + await InstitutionsAPIPromises.removeAffiliation(userId, email) + } catch (error) { + OError.tag(error, 'problem removing affiliation') + throw error + } + + const query = { _id: userId, email: { $ne: email } } + const update = { $pull: { emails: { email } } } + + let res + try { + res = await UserUpdater.promises.updateUser(query, update) + } catch (error) { + OError.tag(error, 'problem removing users email') + throw error + } + + if (res.matchedCount !== 1) { + throw new Error('Cannot remove email') + } + + await FeaturesUpdater.promises.refreshFeatures(userId, 'remove-email') +} + const UserUpdater = { addAffiliationForNewUser(userId, email, affiliationOptions, callback) { if (callback == null) { @@ -321,33 +365,7 @@ const UserUpdater = { // or any other user addEmailAddress: callbackify(addEmailAddress), - // remove one of the user's email addresses. The email cannot be the user's - // default email address - removeEmailAddress(userId, email, callback) { - email = EmailHelper.parseEmail(email) - if (email == null) { - return callback(new Error('invalid email')) - } - removeAffiliation(userId, email, error => { - if (error != null) { - OError.tag(error, 'problem removing affiliation') - return callback(error) - } - - const query = { _id: userId, email: { $ne: email } } - const update = { $pull: { emails: { email } } } - UserUpdater.updateUser(query, update, (error, res) => { - if (error != null) { - OError.tag(error, 'problem removing users email') - return callback(error) - } - if (res.matchedCount !== 1) { - return callback(new Error('Cannot remove email')) - } - FeaturesUpdater.refreshFeatures(userId, 'remove-email', callback) - }) - }) - }, + removeEmailAddress: callbackify(removeEmailAddress), clearSAMLData: callbackify(clearSAMLData), @@ -384,6 +402,7 @@ const promises = { confirmEmail, setDefaultEmailAddress, updateUser: promisify(UserUpdater.updateUser), + removeEmailAddress, removeReconfirmFlag: promisify(UserUpdater.removeReconfirmFlag), } diff --git a/services/web/scripts/remove_email.js b/services/web/scripts/remove_email.js new file mode 100644 index 0000000000..c3e45aee01 --- /dev/null +++ b/services/web/scripts/remove_email.js @@ -0,0 +1,56 @@ +// Run all the mongo queries on secondaries +process.env.MONGO_CONNECTION_STRING = + process.env.READ_ONLY_MONGO_CONNECTION_STRING + +const { ObjectId, waitForDb } = require('../app/src/infrastructure/mongodb') +const UserUpdater = require('../app/src/Features/User/UserUpdater') +const UserGetter = require('../app/src/Features/User/UserGetter') + +waitForDb() + .then(removeEmail) + .catch(error => { + console.error(error) + process.exit(1) + }) + .then(() => { + console.log('Done.') + process.exit() + }) + +async function removeEmail() { + const userId = process.argv[2] + let email = process.argv[3] + + if (!ObjectId.isValid(userId)) { + throw new Error(`user ID ${userId} is not valid`) + } + + if (!email) { + throw new Error('no email provided') + } + + // email arg can be within double quotes for arg so that we can handle + // malformed emails with spaces + email = email.replace(/"/g, '') + + console.log( + `\nBegin request to remove email "${email}" from user "${userId}"\n` + ) + + const userWithEmail = await UserGetter.promises.getUserByAnyEmail(email, { + _id: 1, + }) + + if (!userWithEmail) { + throw new Error(`no user found with email "${email}"`) + } + + if (userWithEmail._id.toString() !== userId) { + throw new Error( + `email does not belong to user. Belongs to ${userWithEmail._id}` + ) + } + + const skipParseEmail = true + await UserUpdater.promises.removeEmailAddress(userId, email, skipParseEmail) +} diff --git a/services/web/test/unit/src/User/UserUpdaterTests.js b/services/web/test/unit/src/User/UserUpdaterTests.js index c5f0128bdd..81cda51c7d 100644 --- a/services/web/test/unit/src/User/UserUpdaterTests.js +++ b/services/web/test/unit/src/User/UserUpdaterTests.js @@ -24,6 +24,7 @@ describe('UserUpdater', function () { promises: { ensureUniqueEmailAddress: sinon.stub(), getUser: sinon.stub(), + getUserByMainEmail: sinon.stub(), }, } this.addAffiliation = sinon.stub().yields() @@ -52,6 +53,7 @@ describe('UserUpdater', function () { removeAffiliation: this.removeAffiliation, promises: { addAffiliation: sinon.stub(), + removeAffiliation: sinon.stub(), }, }), '../Email/EmailHandler': (this.EmailHandler = { @@ -347,9 +349,9 @@ describe('UserUpdater', function () { describe('removeEmailAddress', function () { beforeEach(function () { - this.UserUpdater.updateUser = sinon + this.UserUpdater.promises.updateUser = sinon .stub() - .yields(null, { matchedCount: 1 }) + .returns({ matchedCount: 1 }) }) it('remove email', function (done) { @@ -358,7 +360,7 @@ describe('UserUpdater', function () { this.newEmail, err => { expect(err).not.to.exist - this.UserUpdater.updateUser + this.UserUpdater.promises.updateUser .calledWith( { _id: this.stubbedUser._id, email: { $ne: this.newEmail } }, { $pull: { emails: { email: this.newEmail } } } @@ -375,8 +377,12 @@ describe('UserUpdater', function () { this.newEmail, err => { expect(err).not.to.exist - this.removeAffiliation.calledOnce.should.equal(true) - const { args } = this.removeAffiliation.lastCall + this.InstitutionsAPI.promises.removeAffiliation.calledOnce.should.equal( + true + ) + const { + args, + } = this.InstitutionsAPI.promises.removeAffiliation.lastCall args[0].should.equal(this.stubbedUser._id) args[1].should.equal(this.newEmail) done() @@ -390,50 +396,87 @@ describe('UserUpdater', function () { this.newEmail, err => { expect(err).not.to.exist - sinon.assert.calledWith(this.refreshFeatures, this.stubbedUser._id) + sinon.assert.calledWith( + this.FeaturesUpdater.promises.refreshFeatures, + this.stubbedUser._id + ) done() } ) }) - it('handle error', function (done) { - this.UserUpdater.updateUser = sinon - .stub() - .callsArgWith(2, new Error('nope')) + it('handle error from updateUser', function (done) { + const anError = new Error('nope') + this.UserUpdater.promises.updateUser.rejects(anError) this.UserUpdater.removeEmailAddress( this.stubbedUser._id, this.newEmail, err => { expect(err).to.exist + expect(err).to.deep.equal(anError) + expect(err._oErrorTags[0].message).to.equal( + 'problem removing users email' + ) + expect( + this.FeaturesUpdater.promises.refreshFeatures.callCount + ).to.equal(0) done() } ) }) it('handle missed update', function (done) { - this.UserUpdater.updateUser = sinon + this.UserUpdater.promises.updateUser = sinon .stub() - .yields(null, { matchedCount: 0 }) + .returns({ matchedCount: 0 }) this.UserUpdater.removeEmailAddress( this.stubbedUser._id, this.newEmail, err => { expect(err).to.exist + expect(err.message).to.equal('Cannot remove email') + expect( + this.FeaturesUpdater.promises.refreshFeatures.callCount + ).to.equal(0) done() } ) }) it('handle affiliation error', function (done) { - this.removeAffiliation.callsArgWith(2, new Error('nope')) + const anError = new Error('nope') + this.InstitutionsAPI.promises.removeAffiliation.rejects(anError) this.UserUpdater.removeEmailAddress( this.stubbedUser._id, this.newEmail, err => { expect(err).to.exist - this.UserUpdater.updateUser.called.should.equal(false) + expect(err).to.deep.equal(anError) + this.UserUpdater.promises.updateUser.called.should.equal(false) + expect( + this.FeaturesUpdater.promises.refreshFeatures.callCount + ).to.equal(0) + done() + } + ) + }) + + it('returns error when removing primary email', function (done) { + this.UserGetter.promises.getUserByMainEmail = sinon + .stub() + .returns({ _id: '123abc' }) + this.UserUpdater.removeEmailAddress( + this.stubbedUser._id, + this.newEmail, + err => { + expect(err).to.exist + expect(err.message).to.deep.equal('cannot remove primary email') + expect(this.UserUpdater.promises.updateUser.callCount).to.equal(0) + expect( + this.FeaturesUpdater.promises.refreshFeatures.callCount + ).to.equal(0) done() } ) @@ -442,9 +485,37 @@ describe('UserUpdater', function () { it('validates email', function (done) { this.UserUpdater.removeEmailAddress(this.stubbedUser._id, 'baz', err => { expect(err).to.exist + expect(err.message).to.equal('invalid email') done() }) }) + + it('skip email validation when skipParseEmail included', function (done) { + const skipParseEmail = true + this.UserUpdater.removeEmailAddress( + this.stubbedUser._id, + 'baz', + skipParseEmail, + err => { + expect(err).to.not.exist + done() + } + ) + }) + + it('returns an error when skipParseEmail included but email is not a string', function (done) { + const skipParseEmail = true + this.UserUpdater.removeEmailAddress( + this.stubbedUser._id, + 1, + skipParseEmail, + err => { + expect(err).to.exist + expect(err.message).to.equal('email must be a string') + done() + } + ) + }) }) describe('setDefaultEmailAddress', function () {