[web] Add auditing of email removals (#8904)

* [web] Add auditing of email removals

* [web] Improve auditing of email removal from script

GitOrigin-RevId: ccb948f01616a0bcb2d8f718d6b9e69585e8bb89

services/web/app/src/Features/User/UserAuditLogHandler.js

@@ -9,6 +9,7 @@ function _canHaveNoInitiatorId(operation, info) {
   if (operation === 'unlink-sso' && info.providerId === 'collabratec')
     return true
   if (operation === 'unlink-institution-sso-not-migrated') return true
+  if (operation === 'remove-email' && info.script) return true
 }
 
 /**

services/web/app/src/Features/User/UserEmailsController.js

@@ -164,8 +164,11 @@ const UserEmailsController = {
     if (!email) {
       return res.sendStatus(422)
     }
-
-    UserUpdater.removeEmailAddress(userId, email, function (error) {
+    const auditLog = {
+      initiatorId: userId,
+      ipAddress: req.ip,
+    }
+    UserUpdater.removeEmailAddress(userId, email, auditLog, function (error) {
       if (error) {
         return next(error)
       }

services/web/app/src/Features/User/UserUpdater.js

@@ -310,7 +310,12 @@ async function maybeCreateRedundantSubscriptionNotification(userId, email) {
     .create()
 }
 
-async function removeEmailAddress(userId, email, skipParseEmail = false) {
+async function removeEmailAddress(
+  userId,
+  email,
+  auditLog,
+  skipParseEmail = false
+) {
   // remove one of the user's email addresses. The email cannot be the user's
   // default email address
   if (!skipParseEmail) {
@@ -330,6 +335,18 @@ async function removeEmailAddress(userId, email, skipParseEmail = false) {
     throw new Error('cannot remove primary email')
   }
 
+  await UserAuditLogHandler.promises.addEntry(
+    userId,
+    'remove-email',
+    auditLog.initiatorId,
+    auditLog.ipAddress,
+    {
+      removedEmail: email,
+      // Add optional extra info
+      ...(auditLog.extraInfo || {}),
+    }
+  )
+
   try {
     await InstitutionsAPI.promises.removeAffiliation(userId, email)
   } catch (error) {
@@ -406,7 +423,7 @@ async function changeEmailAddress(userId, newEmail, auditLog) {
   const oldEmail = await UserGetter.promises.getUserEmail(userId)
   await addEmailAddress(userId, newEmail, {}, auditLog)
   await setDefaultEmailAddress(userId, newEmail, true, auditLog, true)
-  await removeEmailAddress(userId, oldEmail)
+  await removeEmailAddress(userId, oldEmail, auditLog)
 }
 
 async function removeReconfirmFlag(userId) {

services/web/scripts/remove_email.js

@@ -47,6 +47,19 @@ async function removeEmail() {
     )
   }
 
+  const auditLog = {
+    initiatorId: undefined,
+    ipAddress: '0.0.0.0',
+    extraInfo: {
+      script: true,
+    },
+  }
+
   const skipParseEmail = true
-  await UserUpdater.promises.removeEmailAddress(userId, email, skipParseEmail)
+  await UserUpdater.promises.removeEmailAddress(
+    userId,
+    email,
+    auditLog,
+    skipParseEmail
+  )
 }

services/web/test/unit/src/User/UserAuditLogHandlerTests.js

@@ -65,6 +65,22 @@ describe('UserAuditLogHandler', function () {
         )
         this.UserMock.verify()
       })
+
+      it('updates the log for a email removal via script', async function () {
+        await expect(
+          this.UserAuditLogHandler.promises.addEntry(
+            this.userId,
+            'remove-email',
+            undefined,
+            this.action.ip,
+            {
+              removedEmail: 'foo',
+              script: true,
+            }
+          )
+        )
+        this.UserMock.verify()
+      })
     })
 
     describe('errors', function () {
@@ -124,6 +140,20 @@ describe('UserAuditLogHandler', function () {
             )
           ).to.be.rejected
         })
+
+        it('throws an error when remove-email is not from a script, but has no initiatorId', async function () {
+          await expect(
+            this.UserAuditLogHandler.promises.addEntry(
+              this.userId,
+              'remove-email',
+              undefined,
+              this.action.ip,
+              {
+                removedEmail: 'foo',
+              }
+            )
+          ).to.be.rejected
+        })
       })
     })
   })

services/web/test/unit/src/User/UserEmailsControllerTests.js

@@ -244,7 +244,11 @@ describe('UserEmailsController', function () {
     })
 
     it('removes email', function (done) {
-      this.UserUpdater.removeEmailAddress.callsArgWith(2, null)
+      const auditLog = {
+        initiatorId: this.user._id,
+        ipAddress: this.req.ip,
+      }
+      this.UserUpdater.removeEmailAddress.callsArgWith(3, null)
 
       this.UserEmailsController.remove(this.req, {
         sendStatus: code => {
@@ -253,7 +257,8 @@ describe('UserEmailsController', function () {
           assertCalledWith(
             this.UserUpdater.removeEmailAddress,
             this.user._id,
-            this.email
+            this.email,
+            auditLog
           )
           done()
         },

services/web/test/unit/src/User/UserUpdaterTests.js

@@ -376,10 +376,14 @@ describe('UserUpdater', function () {
   })
 
   describe('removeEmailAddress', function () {
+    this.beforeEach(function () {
+      this.auditLog = { initiatorId: this.user._id, ipAddress: '127:0:0:0' }
+    })
     it('removes the email', async function () {
       await this.UserUpdater.promises.removeEmailAddress(
         this.user._id,
-        this.newEmail
+        this.newEmail,
+        this.auditLog
       )
       expect(this.db.users.updateOne).to.have.been.calledWith(
         { _id: this.user._id, email: { $ne: this.newEmail } },
@@ -390,7 +394,8 @@ describe('UserUpdater', function () {
     it('removes the affiliation', async function () {
       await this.UserUpdater.promises.removeEmailAddress(
         this.user._id,
-        this.newEmail
+        this.newEmail,
+        this.auditLog
       )
       expect(this.InstitutionsAPI.promises.removeAffiliation).to.have.been
         .calledOnce
@@ -402,7 +407,8 @@ describe('UserUpdater', function () {
     it('refreshes features', async function () {
       await this.UserUpdater.promises.removeEmailAddress(
         this.user._id,
-        this.newEmail
+        this.newEmail,
+        this.auditLog
       )
       sinon.assert.calledWith(
         this.FeaturesUpdater.promises.refreshFeatures,
@@ -417,7 +423,8 @@ describe('UserUpdater', function () {
       await expect(
         this.UserUpdater.promises.removeEmailAddress(
           this.user._id,
-          this.newEmail
+          this.newEmail,
+          this.auditLog
         )
       ).to.be.rejected
       expect(this.FeaturesUpdater.promises.refreshFeatures).not.to.have.been
@@ -430,7 +437,8 @@ describe('UserUpdater', function () {
       await expect(
         this.UserUpdater.promises.removeEmailAddress(
           this.user._id,
-          this.newEmail
+          this.newEmail,
+          this.auditLog
         )
       ).to.be.rejectedWith('Cannot remove email')
       expect(this.FeaturesUpdater.promises.refreshFeatures).not.to.have.been
@@ -443,7 +451,8 @@ describe('UserUpdater', function () {
       await expect(
         this.UserUpdater.promises.removeEmailAddress(
           this.user._id,
-          this.newEmail
+          this.newEmail,
+          this.auditLog
         )
       ).to.be.rejected
       expect(this.db.users.updateOne).not.to.have.been.called
@@ -455,7 +464,8 @@ describe('UserUpdater', function () {
       await expect(
         this.UserUpdater.promises.removeEmailAddress(
           this.user._id,
-          this.user.email
+          this.user.email,
+          this.auditLog
         )
       ).to.be.rejectedWith('cannot remove primary email')
       expect(this.db.users.updateOne).not.to.have.been.called
@@ -465,7 +475,11 @@ describe('UserUpdater', function () {
 
     it('validates the email', function () {
       expect(
-        this.UserUpdater.promises.removeEmailAddress(this.user._id, 'baz')
+        this.UserUpdater.promises.removeEmailAddress(
+          this.user._id,
+          'baz',
+          this.auditLog
+        )
       ).to.be.rejectedWith('invalid email')
     })
 
@@ -474,6 +488,7 @@ describe('UserUpdater', function () {
       await this.UserUpdater.promises.removeEmailAddress(
         this.user._id,
         'baz',
+        this.auditLog,
         skipParseEmail
       )
     })
@@ -484,10 +499,57 @@ describe('UserUpdater', function () {
         this.UserUpdater.promises.removeEmailAddress(
           this.user._id,
           1,
+          this.auditLog,
           skipParseEmail
         )
       ).to.be.rejectedWith('email must be a string')
     })
+
+    it('logs the removal to the audit log', async function () {
+      await this.UserUpdater.promises.removeEmailAddress(
+        this.user._id,
+        this.newEmail,
+        this.auditLog
+      )
+      expect(
+        this.UserAuditLogHandler.promises.addEntry
+      ).to.have.been.calledWith(
+        this.user._id,
+        'remove-email',
+        this.auditLog.initiatorId,
+        this.auditLog.ipAddress,
+        {
+          removedEmail: this.newEmail,
+        }
+      )
+    })
+
+    it('logs the removal from script to the audit log', async function () {
+      this.auditLog = {
+        initiatorId: undefined,
+        ipAddress: '0.0.0.0',
+        extraInfo: {
+          script: true,
+        },
+      }
+      await this.UserUpdater.promises.removeEmailAddress(
+        this.user._id,
+        this.newEmail,
+        this.auditLog
+      )
+      expect(
+        this.UserAuditLogHandler.promises.addEntry
+      ).to.have.been.calledWith(
+        this.user._id,
+        'remove-email',
+        this.auditLog.initiatorId,
+        this.auditLog.ipAddress,
+        {
+          removedEmail: this.newEmail,
+          script: true,
+        }
+      )
+    })
   })
 
   describe('setDefaultEmailAddress', function () {