Merge pull request #6509 from overleaf/jel-migrate-sso-id

[v1 and web] Migrate institution SSO external user ID

GitOrigin-RevId: f31cd50fbada9a2704df1c837d695f2ff547420d

package-lock.json

@@ -35052,6 +35052,7 @@
         "multer": "https://github.com/overleaf/multer/archive/7a2928d7ea2da02dd92888ea1c9ba5704e07aeeb.tar.gz",
         "nocache": "^2.1.0",
         "nock": "^13.1.3",
+        "node-fetch": "^2.6.7",
         "nodemailer": "^6.7.0",
         "nodemailer-mandrill-transport": "^1.2.0",
         "nodemailer-ses-transport": "^1.5.1",
@@ -35176,7 +35177,6 @@
         "mocha": "^8.4.0",
         "mock-fs": "^5.1.2",
         "nock": "^13.1.1",
-        "node-fetch": "^2.6.7",
         "pirates": "^4.0.1",
         "postcss-loader": "^6.2.1",
         "requirejs": "^2.3.6",

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

@@ -10,26 +10,15 @@ const UserGetter = require('../User/UserGetter')
 const UserUpdater = require('../User/UserUpdater')
 const logger = require('@overleaf/logger')
 const { User } = require('../../models/User')
+const { promiseMapWithLimit } = require('../../util/promises')
 
-async function _addAuditLogEntry(
+async function _addAuditLogEntry(operation, userId, auditLog, extraInfo) {
-  link,
-  userId,
-  auditLog,
-  institutionEmail,
-  providerId,
-  providerName
-) {
-  const operation = link ? 'link-institution-sso' : 'unlink-institution-sso'
   await UserAuditLogHandler.promises.addEntry(
     userId,
     operation,
     auditLog.initiatorId,
     auditLog.ipAddress,
-    {
+    extraInfo
-      institutionEmail,
-      providerId,
-      providerName,
-    }
   )
 }
 
@@ -95,19 +84,25 @@ async function _addIdentifier(
   hasEntitlement,
   institutionEmail,
   providerName,
-  auditLog
+  auditLog,
+  userIdAttribute
 ) {
   providerId = providerId.toString()
 
   await _ensureCanAddIdentifier(userId, institutionEmail, providerId)
 
-  await _addAuditLogEntry(
+  const auditLogInfo = {
-    true,
-    userId,
-    auditLog,
     institutionEmail,
     providerId,
-    providerName
+    providerName,
+    userIdAttribute,
+  }
+
+  await _addAuditLogEntry(
+    'link-institution-sso',
+    userId,
+    auditLog,
+    auditLogInfo
   )
 
   hasEntitlement = !!hasEntitlement
@@ -117,12 +112,14 @@ async function _addIdentifier(
       $ne: providerId,
     },
   }
+
   const update = {
     $push: {
       samlIdentifiers: {
         hasEntitlement,
         externalUserId,
         providerId,
+        userIdAttribute,
       },
     },
   }
@@ -239,15 +236,16 @@ async function redundantSubscription(userId, providerId, providerName) {
   }
 }
 
-async function linkAccounts(
+async function linkAccounts(userId, samlData, auditLog) {
-  userId,
+  const {
     externalUserId,
     institutionEmail,
-  providerId,
+    universityId: providerId,
-  providerName,
+    universityName: providerName,
     hasEntitlement,
-  auditLog
+    userIdAttribute,
-) {
+  } = samlData
+
   await _addIdentifier(
     userId,
     externalUserId,
@@ -255,7 +253,8 @@ async function linkAccounts(
     hasEntitlement,
     institutionEmail,
     providerName,
-    auditLog
+    auditLog,
+    userIdAttribute
   )
   try {
     await _addInstitutionEmail(userId, institutionEmail, providerId, auditLog)
@@ -288,14 +287,11 @@ async function unlinkAccounts(
 ) {
   providerId = providerId.toString()
 
-  await _addAuditLogEntry(
+  await _addAuditLogEntry('unlink-institution-sso', userId, auditLog, {
-    false,
-    userId,
-    auditLog,
     institutionEmail,
     providerId,
-    providerName
+    providerName,
-  )
+  })
   // update v2 user
   await _removeIdentifier(userId, providerId)
   // update v1 affiliations record
@@ -379,12 +375,96 @@ function userHasEntitlement(user, providerId) {
   return false
 }
 
+async function migrateIdentifier(
+  userId,
+  externalUserId,
+  providerId,
+  hasEntitlement,
+  institutionEmail,
+  providerName,
+  auditLog,
+  userIdAttribute
+) {
+  providerId = providerId.toString()
+
+  const query = {
+    _id: userId,
+    'samlIdentifiers.providerId': providerId,
+  }
+
+  const update = {
+    $set: {
+      'samlIdentifiers.$.externalUserId': externalUserId,
+      'samlIdentifiers.$.userIdAttribute': userIdAttribute,
+    },
+  }
+  await User.updateOne(query, update).exec()
+
+  const auditLogInfo = {
+    institutionEmail,
+    providerId,
+    providerName,
+    userIdAttribute,
+  }
+
+  await _addAuditLogEntry(
+    'migrate-institution-sso-id',
+    userId,
+    auditLog,
+    auditLogInfo
+  )
+}
+
+async function unlinkNotMigrated(userId, providerId, providerName, auditLog) {
+  providerId = providerId.toString()
+
+  const query = {
+    _id: userId,
+    'emails.samlProviderId': providerId,
+  }
+  const update = {
+    $pull: {
+      samlIdentifiers: {
+        providerId,
+      },
+    },
+    $unset: {
+      'emails.$.samlProviderId': 1,
+    },
+  }
+
+  const originalDoc = await User.findOneAndUpdate(query, update).exec()
+
+  // should only be 1
+  const linkedEmails = originalDoc.emails.filter(email => {
+    return email.samlProviderId === providerId
+  })
+
+  const auditLogInfo = {
+    providerId,
+    providerName,
+  }
+
+  await _addAuditLogEntry(
+    'unlink-institution-sso-not-migrated',
+    userId,
+    auditLog,
+    auditLogInfo
+  )
+
+  await promiseMapWithLimit(10, linkedEmails, async emailData => {
+    await InstitutionsAPI.promises.removeEntitlement(userId, emailData.email)
+  })
+}
+
 const SAMLIdentityManager = {
   entitlementAttributeMatches,
   getUser,
   linkAccounts,
+  migrateIdentifier,
   redundantSubscription,
   unlinkAccounts,
+  unlinkNotMigrated,
   updateEntitlement,
   userHasEntitlement,
 }

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

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

services/web/package.json

@@ -145,6 +145,7 @@
     "multer": "https://github.com/overleaf/multer/archive/7a2928d7ea2da02dd92888ea1c9ba5704e07aeeb.tar.gz",
     "nocache": "^2.1.0",
     "nock": "^13.1.3",
+    "node-fetch": "^2.6.7",
     "nodemailer": "^6.7.0",
     "nodemailer-mandrill-transport": "^1.2.0",
     "nodemailer-ses-transport": "^1.5.1",
@@ -269,7 +270,6 @@
     "mocha": "^8.4.0",
     "mock-fs": "^5.1.2",
     "nock": "^13.1.1",
-    "node-fetch": "^2.6.7",
     "pirates": "^4.0.1",
     "postcss-loader": "^6.2.1",
     "requirejs": "^2.3.6",

services/web/scripts/backfill_users_sso_attribute.js

@@ -0,0 +1,38 @@
+const SAMLUserIdAttributeBatchHandler = require('../modules/overleaf-integration/app/src/SAML/SAMLUserIdAttributeBatchHandler')
+
+const COMMIT = process.argv.includes('--commit')
+const startInstitutionId = parseInt(process.argv[2])
+const endInstitutionId = parseInt(process.argv[3])
+
+process.env.LOG_LEVEL = 'info'
+
+let method = 'check'
+if (COMMIT) {
+  method = 'run'
+  console.log('Setting attribute for linked users')
+} else {
+  process.env.MONGO_CONNECTION_STRING =
+    process.env.READ_ONLY_MONGO_CONNECTION_STRING
+  console.log('Doing a dry run without --commit')
+  console.log('Checking users at institutions')
+}
+
+console.log(
+  'Start institution ID:',
+  startInstitutionId ||
+    'none provided, will start at beginning of ordered list.'
+)
+console.log(
+  'End institution ID:',
+  endInstitutionId || 'none provided, will go to end of ordered list.'
+)
+
+SAMLUserIdAttributeBatchHandler[method](startInstitutionId, endInstitutionId)
+  .then(result => {
+    console.log(result)
+    process.exit(0)
+  })
+  .catch(error => {
+    console.error(error)
+    process.exit(1)
+  })

services/web/scripts/sso_id_migration_check.js

@@ -0,0 +1,36 @@
+process.env.MONGO_CONNECTION_STRING =
+  process.env.READ_ONLY_MONGO_CONNECTION_STRING
+
+const { waitForDb } = require('../app/src/infrastructure/mongodb')
+const SAMLUserIdMigrationHandler = require('../modules/overleaf-integration/app/src/SAML/SAMLUserIdMigrationHandler')
+
+const institutionId = parseInt(process.argv[2])
+if (isNaN(institutionId)) throw new Error('No institution id')
+const emitUsers = process.argv.includes('--emit-users')
+
+console.log('Checking SSO user ID migration for institution:', institutionId)
+
+waitForDb()
+  .then(main)
+  .catch(error => {
+    console.error(error)
+    process.exit(1)
+  })
+
+async function main() {
+  const result = await SAMLUserIdMigrationHandler.promises.checkMigration(
+    institutionId
+  )
+
+  if (emitUsers) {
+    console.log(
+      `\nMigrated: ${result.migrated}\nNot migrated: ${result.notMigrated}\nMultiple Identifers: ${result.multipleIdentifiers}`
+    )
+  }
+
+  console.log(
+    `\nMigrated: ${result.migrated.length}\nNot migrated: ${result.notMigrated.length}\nMultiple Identifers: ${result.multipleIdentifiers.length}`
+  )
+
+  process.exit()
+}

services/web/scripts/sso_id_remove_not_migrated.js

@@ -0,0 +1,36 @@
+const { waitForDb } = require('../app/src/infrastructure/mongodb')
+const SAMLUserIdMigrationHandler = require('../modules/overleaf-integration/app/src/SAML/SAMLUserIdMigrationHandler')
+
+const institutionId = parseInt(process.argv[2])
+if (isNaN(institutionId)) throw new Error('No institution id')
+const emitUsers = process.argv.includes('--emit-users')
+
+console.log(
+  'Remove SSO linking for users not migrated at institution:',
+  institutionId
+)
+
+waitForDb()
+  .then(main)
+  .catch(error => {
+    console.error(error)
+    process.exit(1)
+  })
+
+async function main() {
+  const result = await SAMLUserIdMigrationHandler.promises.removeNotMigrated(
+    institutionId
+  )
+
+  if (emitUsers) {
+    console.log(
+      `\nRemoved: ${result.success}\nFailed to remove: ${result.failed}`
+    )
+  }
+
+  console.log(
+    `\nRemoved: ${result.success.length}\nFailed to remove: ${result.failed.length}`
+  )
+
+  process.exit()
+}

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

@@ -121,13 +121,7 @@ describe('SAMLIdentityManager', function () {
       it('should throw an error if missing data', async function () {
         let error
         try {
-          await this.SAMLIdentityManager.linkAccounts(
+          await this.SAMLIdentityManager.linkAccounts(null, null, null)
-            null,
-            null,
-            null,
-            null,
-            null
-          )
         } catch (e) {
           error = e
         } finally {
@@ -144,14 +138,17 @@ describe('SAMLIdentityManager', function () {
 
         it('should throw an EmailExistsError error', async function () {
           let error
+
           try {
             await this.SAMLIdentityManager.linkAccounts(
               '6005c75b12cbcaf771f4a105',
-              'not-linked-id',
+              {
-              'exists@overleaf.com',
+                externalUserId: 'not-linked-id',
-              'provider-id',
+                institutionEmail: 'exists@overleaf.com',
-              'provider-name',
+                universityId: 'provider-id',
-              true,
+                universityName: 'provider-name',
+                hasEntitlement: true,
+              },
               {
                 intiatorId: '6005c75b12cbcaf771f4a105',
                 ip: '0:0:0:0',
@@ -181,11 +178,13 @@ describe('SAMLIdentityManager', function () {
           try {
             await this.SAMLIdentityManager.linkAccounts(
               '6005c75b12cbcaf771f4a105',
-              'not-linked-id',
+              {
-              'not-affiliated@overleaf.com',
+                externalUserId: 'not-linked-id',
-              'provider-id',
+                institutionEmail: 'not-affiliated@overleaf.com',
-              'provider-name',
+                universityId: 'provider-id',
-              true,
+                universityName: 'provider-name',
+                hasEntitlement: true,
+              },
               {
                 intiatorId: 'user-id-1',
                 ip: '0:0:0:0',
@@ -216,11 +215,13 @@ describe('SAMLIdentityManager', function () {
           try {
             await this.SAMLIdentityManager.linkAccounts(
               '6005c75b12cbcaf771f4a105',
-              'not-linked-id',
+              {
-              'affiliated@overleaf.com',
+                externalUserId: 'not-linked-id',
-              'provider-id',
+                institutionEmail: 'affiliated@overleaf.com',
-              'provider-name',
+                universityId: 'provider-id',
-              true,
+                universityName: 'provider-name',
+                hasEntitlement: true,
+              },
               {
                 intiatorId: 'user-id-1',
                 ip: '0:0:0:0',
@@ -249,11 +250,13 @@ describe('SAMLIdentityManager', function () {
           try {
             await this.SAMLIdentityManager.linkAccounts(
               '6005c75b12cbcaf771f4a105',
-              'already-linked-id',
+              {
-              'linked@overleaf.com',
+                externalUserId: 'already-linked-id',
-              'provider-id',
+                institutionEmail: 'linked@overleaf.com',
-              'provider-name',
+                universityId: 'provider-id',
-              true,
+                universityName: 'provider-name',
+                hasEntitlement: true,
+              },
               {
                 intiatorId: '6005c75b12cbcaf771f4a105',
                 ip: '0:0:0:0',
@@ -279,11 +282,13 @@ describe('SAMLIdentityManager', function () {
           try {
             await this.SAMLIdentityManager.linkAccounts(
               '6005c75b12cbcaf771f4a105',
-              'already-linked-id',
+              {
-              'linked@overleaf.com',
+                externalUserId: 'already-linked-id',
-              123456,
+                institutionEmail: 'linked@overleaf.com',
-              'provider-name',
+                universityId: 123456,
-              true,
+                universityName: 'provider-name',
+                hasEntitlement: true,
+              },
               {
                 intiatorId: '6005c75b12cbcaf771f4a105',
                 ip: '0:0:0:0',
@@ -311,11 +316,13 @@ describe('SAMLIdentityManager', function () {
         try {
           await this.SAMLIdentityManager.linkAccounts(
             this.user._id,
-            'externalUserId',
+            {
-            this.user.email,
+              externalUserId: 'externalUserId',
-            '1',
+              institutionEmail: this.user.email,
-            'Overleaf University',
+              universityId: '1',
-            undefined,
+              universityName: 'Overleaf University',
+              hasEntitlement: false,
+            },
             {
               intiatorId: '6005c75b12cbcaf771f4a105',
               ipAddress: '0:0:0:0',
@@ -346,11 +353,14 @@ describe('SAMLIdentityManager', function () {
         }
         await this.SAMLIdentityManager.linkAccounts(
           this.user._id,
-          'externalUserId',
+          {
-          this.user.email,
+            externalUserId: 'externalUserId',
-          '1',
+            institutionEmail: this.user.email,
-          'Overleaf University',
+            universityId: '1',
-          undefined,
+            universityName: 'Overleaf University',
+            hasEntitlement: false,
+            userIdAttribute: 'uniqueId',
+          },
           auditLog
         )
 
@@ -365,6 +375,7 @@ describe('SAMLIdentityManager', function () {
             institutionEmail: this.user.email,
             providerId: '1',
             providerName: 'Overleaf University',
+            userIdAttribute: 'uniqueId',
           }
         )
       })
@@ -372,11 +383,13 @@ describe('SAMLIdentityManager', function () {
       it('should send an email notification', async function () {
         await this.SAMLIdentityManager.linkAccounts(
           this.user._id,
-          'externalUserId',
+          {
-          this.user.email,
+            externalUserId: 'externalUserId',
-          '1',
+            institutionEmail: this.user.email,
-          'Overleaf University',
+            universityId: '1',
-          undefined,
+            universityName: 'Overleaf University',
+            hasEntitlement: false,
+          },
           {
             intiatorId: '6005c75b12cbcaf771f4a105',
             ipAddress: '0:0:0:0',
@@ -602,4 +615,123 @@ describe('SAMLIdentityManager', function () {
       })
     })
   })
+
+  describe('migrateIdentifier', function () {
+    const userId = '5efb8b6e9b647b0027e4c0b0'
+    const externalUserId = '987zyx'
+    const providerId = 123
+    const hasEntitlement = false
+    const institutionEmail = 'someone@email.com'
+    const providerName = 'Example University'
+    const auditLog = {
+      initiatorId: userId,
+      ipAddress: '0.0.0.0',
+      migration: {
+        from: 'uniqueId',
+        to: 'newUniqueId',
+      },
+    }
+    const userIdAttribute = 'newUniqueId'
+
+    it('should remove the old identifier and add the new identifier', async function () {
+      this.UserGetter.promises.getUser.resolves()
+      this.UserGetter.promises.getUserByAnyEmail
+        .withArgs(institutionEmail)
+        .resolves({ _id: userId, emails: [{ email: institutionEmail }] })
+      this.UserGetter.promises.getUserFullEmails.withArgs(userId).resolves([
+        {
+          email: institutionEmail,
+          affiliation: { institution: { id: providerId } },
+        },
+      ])
+      await this.SAMLIdentityManager.migrateIdentifier(
+        userId,
+        externalUserId,
+        providerId,
+        hasEntitlement,
+        institutionEmail,
+        providerName,
+        auditLog,
+        userIdAttribute
+      )
+
+      expect(this.User.updateOne).to.have.been.calledOnce
+      const query = {
+        _id: userId,
+        'samlIdentifiers.providerId': providerId.toString(),
+      }
+
+      const update = {
+        $set: {
+          'samlIdentifiers.$.externalUserId': externalUserId,
+          'samlIdentifiers.$.userIdAttribute': userIdAttribute,
+        },
+      }
+
+      expect(this.User.updateOne.lastCall.args).to.deep.equal([query, update])
+    })
+  })
+
+  describe('unlinkNotMigrated', function () {
+    const userId = '5efb8b6e9b647b0027e4c0b0'
+    const providerId = '123'
+    const institutionEmail = 'someone@email.com'
+    const providerName = 'Example University'
+    const auditLog = {
+      ipAddress: 'N/A',
+    }
+
+    it('should remove the identifier om samlIdentifiers and samlProviderId on the email', async function () {
+      this.User.findOneAndUpdate = sinon.stub().returns({
+        exec: sinon.stub().resolves({
+          _id: userId,
+          emails: [{ email: institutionEmail, samlProviderId: providerId }],
+        }),
+      })
+
+      await this.SAMLIdentityManager.unlinkNotMigrated(
+        userId,
+        providerId,
+        providerName,
+        auditLog
+      )
+
+      expect(this.User.findOneAndUpdate).to.have.been.calledOnce
+      const query = {
+        _id: userId,
+        'emails.samlProviderId': providerId,
+      }
+      const update = {
+        $pull: {
+          samlIdentifiers: {
+            providerId,
+          },
+        },
+        $unset: {
+          'emails.$.samlProviderId': 1,
+        },
+      }
+      expect(this.User.findOneAndUpdate.lastCall.args).to.deep.equal([
+        query,
+        update,
+      ])
+
+      expect(this.UserAuditLogHandler.promises.addEntry).to.have.been.calledOnce
+      expect(
+        this.UserAuditLogHandler.promises.addEntry.lastCall.args
+      ).to.deep.equal([
+        userId,
+        'unlink-institution-sso-not-migrated',
+        undefined,
+        'N/A',
+        { providerId, providerName },
+      ])
+
+      expect(this.InstitutionsAPI.promises.removeEntitlement).to.have.been
+        .calledOnce
+      expect(
+        this.InstitutionsAPI.promises.removeEntitlement.lastCall.args
+      ).to.deep.equal([userId, institutionEmail])
+    })
+  })
 })