Преглед на файлове

Merge pull request #28402 from overleaf/jel-link-logged-in-async-local-storage

[web] Extend `AsyncLocalStorage` to SAML linking in route, clear `AsyncLocalStorage` on email updates, await analytics helper on email updates,

GitOrigin-RevId: 86a51e6800a4b954ff81a2d977edf1401064dda4
Jessica Lawshe преди 11 месеца
родител
ревизия
a9a28a13f5

+ 36 - 22
services/web/app/src/Features/User/UserUpdater.js

@@ -20,6 +20,7 @@ const _ = require('lodash')
 const Modules = require('../../infrastructure/Modules')
 const UserSessionsManager = require('./UserSessionsManager')
 const ThirdPartyIdentityManager = require('./ThirdPartyIdentityManager')
+const AsyncLocalStorage = require('../../infrastructure/AsyncLocalStorage')
 
 async function _sendSecurityAlertPrimaryEmailChanged(
   userId,
@@ -81,6 +82,7 @@ async function _sendSecurityAlertPrimaryEmailChanged(
  * or any other user
  */
 async function addEmailAddress(userId, newEmail, affiliationOptions, auditLog) {
+  AsyncLocalStorage.removeItem('userFullEmails')
   newEmail = EmailHelper.parseEmail(newEmail)
   if (!newEmail) {
     throw new Error('invalid email')
@@ -134,15 +136,17 @@ async function addEmailAddress(userId, newEmail, affiliationOptions, auditLog) {
     return
   }
 
-  EmailChangeHelper.registerEmailCreation(userId, newEmail, {
-    createdAt: new Date(),
-    emailCreatedAt: createdAt,
-  }).catch(error => {
+  try {
+    await EmailChangeHelper.registerEmailCreation(userId, newEmail, {
+      createdAt: new Date(),
+      emailCreatedAt: createdAt,
+    })
+  } catch (error) {
     logger.warn(
       { error, userId, newEmail },
       'Error registering email creation with analytics'
     )
-  })
+  }
 }
 
 async function clearSAMLData(userId, auditLog, sendEmail) {
@@ -215,6 +219,7 @@ async function setDefaultEmailAddress(
   sendSecurityAlert,
   deleteOldEmail = false
 ) {
+  AsyncLocalStorage.removeItem('userFullEmails')
   email = EmailHelper.parseEmail(email)
   if (email == null) {
     throw new Error('invalid email')
@@ -262,16 +267,18 @@ async function setDefaultEmailAddress(
     'primary-email-address-updated'
   )
 
-  EmailChangeHelper.registerEmailUpdate(userId, email, {
-    isPrimary: true,
-    action: 'updated',
-    createdAt: new Date(),
-  }).catch(err => {
+  try {
+    await EmailChangeHelper.registerEmailUpdate(userId, email, {
+      isPrimary: true,
+      action: 'updated',
+      createdAt: new Date(),
+    })
+  } catch (err) {
     logger.warn(
       { err, userId, email },
       'Error registering email change with analytics'
     )
-  })
+  }
 
   if (sendSecurityAlert) {
     // no need to wait, errors are logged and not passed back
@@ -366,6 +373,7 @@ async function migrateDefaultEmailAddress(
 }
 
 async function confirmEmail(userId, email, affiliationOptions) {
+  AsyncLocalStorage.removeItem('userFullEmails')
   // used for initial email confirmation (non-SSO and SSO)
   // also used for reconfirmation of non-SSO emails
   const confirmedAt = new Date()
@@ -415,16 +423,18 @@ async function confirmEmail(userId, email, affiliationOptions) {
   }
   await FeaturesUpdater.promises.refreshFeatures(userId, 'confirm-email')
 
-  EmailChangeHelper.registerEmailUpdate(userId, email, {
-    emailConfirmedAt: confirmedAt,
-    action: 'updated',
-    isPrimary: false,
-  }).catch(error =>
+  try {
+    await EmailChangeHelper.registerEmailUpdate(userId, email, {
+      emailConfirmedAt: confirmedAt,
+      action: 'updated',
+      isPrimary: false,
+    })
+  } catch (error) {
     logger.warn(
       { error, userId, email },
       'Error registering email confirmation with analytics'
     )
-  )
+  }
 
   try {
     await maybeCreateRedundantSubscriptionNotification(userId, email)
@@ -467,6 +477,7 @@ async function removeEmailAddress(
   auditLog,
   skipParseEmail = false
 ) {
+  AsyncLocalStorage.removeItem('userFullEmails')
   // remove one of the user's email addresses. The email cannot be the user's
   // default email address
   if (!skipParseEmail) {
@@ -520,15 +531,17 @@ async function removeEmailAddress(
     throw new Error('Cannot remove email')
   }
 
-  EmailChangeHelper.registerEmailDeletion(userId, email, {
-    isPrimary: false,
-    emailDeletedAt: new Date(),
-  }).catch(error =>
+  try {
+    await EmailChangeHelper.registerEmailDeletion(userId, email, {
+      isPrimary: false,
+      emailDeletedAt: new Date(),
+    })
+  } catch (error) {
     logger.warn(
       { error, userId, email },
       'Error registering email deletion with analytics'
     )
-  )
+  }
 
   await FeaturesUpdater.promises.refreshFeatures(userId, 'remove-email')
 }
@@ -538,6 +551,7 @@ async function addAffiliationForNewUser(
   email,
   affiliationOptions = {}
 ) {
+  AsyncLocalStorage.removeItem('userFullEmails')
   await InstitutionsAPI.promises.addAffiliation(
     userId,
     email,

+ 18 - 1
services/web/app/src/infrastructure/AsyncLocalStorage.js

@@ -18,4 +18,21 @@ function middleware(req, res, next) {
   asyncLocalStorage.run({}, next)
 }
 
-module.exports = { middleware, storage: asyncLocalStorage }
+/**
+ * Remove a key from the AsyncLocalStorage cache
+ *
+ * @param {string} key
+ */
+function removeItem(key) {
+  const store = asyncLocalStorage.getStore()
+
+  if (store?.[key]) {
+    delete store[key]
+  }
+}
+
+module.exports = {
+  middleware,
+  storage: asyncLocalStorage,
+  removeItem,
+}

+ 58 - 0
services/web/test/unit/src/User/UserUpdaterTests.js

@@ -116,6 +116,10 @@ describe('UserUpdater', function () {
       },
     }
 
+    this.AsyncLocalStorage = {
+      removeItem: sinon.stub(),
+    }
+
     this.UserUpdater = SandboxedModule.require(MODULE_PATH, {
       requires: {
         '../Helpers/Mongo': { normalizeQuery },
@@ -136,6 +140,7 @@ describe('UserUpdater', function () {
         '../../infrastructure/Modules': this.Modules,
         './UserSessionsManager': this.UserSessionsManager,
         './ThirdPartyIdentityManager': this.ThirdPartyIdentityManager,
+        '../../infrastructure/AsyncLocalStorage': this.AsyncLocalStorage,
       },
     })
 
@@ -178,6 +183,16 @@ describe('UserUpdater', function () {
         this.newEmail
       )
     })
+
+    it('calls to remove userFullEmails from AsyncLocalStorage', async function () {
+      await this.UserUpdater.promises.addAffiliationForNewUser(
+        this.user._id,
+        this.newEmail
+      )
+      expect(this.AsyncLocalStorage.removeItem).to.have.been.calledWith(
+        'userFullEmails'
+      )
+    })
   })
 
   describe('changeEmailAddress', function () {
@@ -385,6 +400,18 @@ describe('UserUpdater', function () {
         })
       })
     })
+
+    it('calls to remove userFullEmails from AsyncLocalStorage', async function () {
+      await this.UserUpdater.promises.addEmailAddress(
+        this.user._id,
+        this.newEmail,
+        {},
+        { initiatorId: this.user._id, ipAddress: '127:0:0:0' }
+      )
+      expect(this.AsyncLocalStorage.removeItem).to.have.been.calledWith(
+        'userFullEmails'
+      )
+    })
   })
 
   describe('removeEmailAddress', function () {
@@ -562,6 +589,17 @@ describe('UserUpdater', function () {
         }
       )
     })
+
+    it('calls to remove userFullEmails from AsyncLocalStorage', async function () {
+      await this.UserUpdater.promises.removeEmailAddress(
+        this.user._id,
+        this.newEmail,
+        this.auditLog
+      )
+      expect(this.AsyncLocalStorage.removeItem).to.have.been.calledWith(
+        'userFullEmails'
+      )
+    })
   })
 
   describe('setDefaultEmailAddress', function () {
@@ -691,6 +729,18 @@ describe('UserUpdater', function () {
       expect(this.db.users.updateOne).to.not.have.been.called
     })
 
+    it('calls to remove userFullEmails from AsyncLocalStorage', async function () {
+      await this.UserUpdater.promises.setDefaultEmailAddress(
+        this.user._id,
+        this.newEmail,
+        false,
+        this.auditLog
+      )
+      expect(this.AsyncLocalStorage.removeItem).to.have.been.calledWith(
+        'userFullEmails'
+      )
+    })
+
     describe('when email not confirmed', function () {
       beforeEach(function () {
         setUserEmails(this, [
@@ -1024,6 +1074,14 @@ describe('UserUpdater', function () {
       )
     })
 
+    it('calls to remove userFullEmails from AsyncLocalStorage', async function () {
+      await this.UserUpdater.promises.confirmEmail(this.user._id, this.newEmail)
+      expect(this.AsyncLocalStorage.removeItem).to.have.been.called
+      expect(this.AsyncLocalStorage.removeItem).to.have.been.calledWith(
+        'userFullEmails'
+      )
+    })
+
     describe('with institution licence and subscription', function () {
       beforeEach(async function () {
         this.affiliation = {