Merge pull request #6238 from overleaf/jel-script-remove-email

[web] Script to remove email

GitOrigin-RevId: c8f1a69259904b08ef39181b8b7e9c3150ea59f0

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
+  removeEmailAddress: callbackify(removeEmailAddress),
-  // 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)
-      })
-    })
-  },
 
   clearSAMLData: callbackify(clearSAMLData),
 
@@ -384,6 +402,7 @@ const promises = {
   confirmEmail,
   setDefaultEmailAddress,
   updateUser: promisify(UserUpdater.updateUser),
+  removeEmailAddress,
   removeReconfirmFlag: promisify(UserUpdater.removeReconfirmFlag),
 }
 

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)
+}

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)
+          this.InstitutionsAPI.promises.removeAffiliation.calledOnce.should.equal(
-          const { args } = this.removeAffiliation.lastCall
+            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) {
+    it('handle error from updateUser', function (done) {
-      this.UserUpdater.updateUser = sinon
+      const anError = new Error('nope')
-        .stub()
+      this.UserUpdater.promises.updateUser.rejects(anError)
-        .callsArgWith(2, new Error('nope'))
 
       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 () {