Merge pull request #2643 from overleaf/jel-affiliations-cron-job

Ensure affiliations cron job

GitOrigin-RevId: 4ac6f8b29b1e1460d627a86172fcdf1fa27a59a8

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

@@ -19,7 +19,7 @@ const EmailHandler = require('../Email/EmailHandler')
 const UrlHelper = require('../Helpers/UrlHelper')
 const { promisify } = require('util')
 
-async function _ensureAffiliations(userId, emailData) {
+async function _ensureAffiliation(userId, emailData) {
   if (emailData.samlProviderId) {
     await UserUpdater.promises.confirmEmail(userId, emailData.email)
   } else {
@@ -27,14 +27,10 @@ async function _ensureAffiliations(userId, emailData) {
   }
 }
 
-async function ensureAffiliations(userId) {
+async function ensureAffiliation(user) {
   if (!Features.hasFeature('affiliations')) {
     return
   }
-  const user = await UserGetter.promises.getUser(userId)
-  if (!user) {
-    return new Errors.UserNotFoundError({ info: { userId } })
-  }
 
   const flaggedEmails = user.emails.filter(email => email.affiliationUnchecked)
   if (flaggedEmails.length === 0) {
@@ -43,21 +39,27 @@ async function ensureAffiliations(userId) {
 
   if (flaggedEmails.length > 1) {
     logger.error(
-      { userId },
+      { userId: user._id },
       `Unexpected number of flagged emails: ${flaggedEmails.length}`
     )
   }
 
-  await _ensureAffiliations(userId, flaggedEmails[0])
+  await _ensureAffiliation(user._id, flaggedEmails[0])
 }
 
-async function ensureAffiliationsMiddleware(req, res, next) {
-  if (!Features.hasFeature('affiliations') || !req.query.ensureAffiliations) {
+async function ensureAffiliationMiddleware(req, res, next) {
+  let user
+  if (!Features.hasFeature('affiliations') || !req.query.ensureAffiliation) {
     return next()
   }
   const userId = AuthenticationController.getLoggedInUserId(req)
   try {
-    await ensureAffiliations(userId)
+    user = await UserGetter.promises.getUser(userId)
+  } catch (error) {
+    return new Errors.UserNotFoundError({ info: { userId } })
+  }
+  try {
+    await ensureAffiliation(user)
   } catch (error) {
     return next(error)
   }
@@ -446,8 +448,8 @@ const UserController = {
 
 UserController.promises = {
   doLogout: promisify(UserController.doLogout),
-  ensureAffiliations,
-  ensureAffiliationsMiddleware
+  ensureAffiliation,
+  ensureAffiliationMiddleware
 }
 
 module.exports = UserController

services/web/app/src/router.js

@@ -139,7 +139,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   webRouter.get(
     '/user/emails',
     AuthenticationController.requireLogin(),
-    UserController.promises.ensureAffiliationsMiddleware,
+    UserController.promises.ensureAffiliationMiddleware,
     UserEmailsController.list
   )
   webRouter.get('/user/emails/confirm', UserEmailsController.showConfirm)

services/web/frontend/js/main/affiliations/controllers/UserAffiliationsController.js

@@ -295,7 +295,7 @@ define(['base'], App =>
       _resetMakingRequestType()
       $scope.ui.isLoadingEmails = true
       _monitorRequest(
-        UserAffiliationsDataService.getUserEmailsEnsureAffiliations()
+        UserAffiliationsDataService.getUserEmailsEnsureAffiliation()
       )
         .then(emails => {
           $scope.userEmails = emails.map(email => {

services/web/frontend/js/main/affiliations/factories/UserAffiliationsDataService.js

@@ -410,9 +410,9 @@ define(['base'], function(App) {
     const getUserEmails = () =>
       $http.get('/user/emails').then(response => response.data)
 
-    const getUserEmailsEnsureAffiliations = () =>
+    const getUserEmailsEnsureAffiliation = () =>
       $http
-        .get('/user/emails?ensureAffiliations=true')
+        .get('/user/emails?ensureAffiliation=true')
         .then(response => response.data)
 
     const getUserDefaultEmail = () =>
@@ -532,7 +532,7 @@ define(['base'], function(App) {
       getDefaultRoleHints,
       getDefaultDepartmentHints,
       getUserEmails,
-      getUserEmailsEnsureAffiliations,
+      getUserEmailsEnsureAffiliation,
       getUserDefaultEmail,
       getUniversitiesFromCountry,
       getUniversityDomainFromPartialDomainInput,

services/web/scripts/backfill_sso_affiliations.js

@@ -1,53 +0,0 @@
-const { User } = require('../app/src/models/User')
-const UserUpdater = require('../app/src/Features/User/UserUpdater')
-require('logger-sharelatex').logger.level('error')
-const pLimit = require('p-limit')
-const CONCURRENCY = 10
-const unexpectedUserStates = []
-
-console.log('Starting SSO affiliation backfill')
-
-const query = {
-  emails: {
-    $elemMatch: {
-      samlProviderId: { $exists: true },
-      confirmedAt: { $exists: false }
-    }
-  }
-}
-
-async function backfillAffiliation(user) {
-  const ssoEmail = user.emails.filter(
-    emailData => !emailData.confirmedAt && emailData.samlProviderId
-  )
-  if (ssoEmail.length > 1 || ssoEmail.length === 0) {
-    unexpectedUserStates.push(user._id)
-  }
-  const { email } = ssoEmail[0]
-  await UserUpdater.promises.confirmEmail(user._id, email)
-}
-
-async function getUsers() {
-  return User.find(query, { emails: 1 }).exec()
-}
-
-async function run() {
-  const limit = pLimit(CONCURRENCY)
-  const users = await getUsers()
-  console.log(`Found ${users.length} users`)
-  await Promise.all(users.map(user => limit(() => backfillAffiliation(user))))
-  console.log('Finished')
-  console.log(
-    `Found ${unexpectedUserStates.length} in unexpected states`,
-    unexpectedUserStates
-  )
-}
-
-run()
-  .then(() => {
-    process.exit()
-  })
-  .catch(error => {
-    console.log(error)
-    process.exit(1)
-  })

services/web/scripts/ensure_affiliations.js

@@ -0,0 +1,51 @@
+const { User } = require('../app/src/models/User')
+const UserController = require('../app/src/Features/User/UserController')
+require('logger-sharelatex').logger.level('error')
+const pLimit = require('p-limit')
+const CONCURRENCY = 10
+const failure = []
+const success = []
+console.log('Starting ensure affiliations')
+
+const query = {
+  'emails.affiliationUnchecked': true
+}
+
+async function _handleEnsureAffiliation(user) {
+  try {
+    await UserController.promises.ensureAffiliation(user)
+    console.log(`✔ ${user._id}`)
+    success.push(user._id)
+  } catch (error) {
+    failure.push(user._id)
+    console.log(`ERROR: ${user._id}`, error)
+  }
+}
+
+async function getUsers() {
+  return User.find(query, { emails: 1 }).exec()
+}
+
+async function run() {
+  const limit = pLimit(CONCURRENCY)
+  const users = await getUsers()
+  console.log(`Found ${users.length} users`)
+  await Promise.all(
+    users.map(user => limit(() => _handleEnsureAffiliation(user)))
+  )
+
+  console.log(`${success.length} successes`)
+  console.log(`${failure.length} failures`)
+  if (failure.length > 0) {
+    console.log('Failed to update:', failure)
+  }
+}
+
+run()
+  .then(() => {
+    process.exit()
+  })
+  .catch(error => {
+    console.log(error)
+    process.exit(1)
+  })

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

@@ -604,10 +604,10 @@ describe('UserController', function() {
     })
   })
 
-  describe('ensureAffiliationsMiddleware', function() {
+  describe('ensureAffiliationMiddleware', function() {
     describe('without affiliations feature', function() {
       beforeEach(async function() {
-        await this.UserController.promises.ensureAffiliationsMiddleware(
+        await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next
@@ -623,10 +623,10 @@ describe('UserController', function() {
         expect(this.next).to.be.calledWith()
       })
     })
-    describe('without ensureAffiliations query parameter', function() {
+    describe('without ensureAffiliation query parameter', function() {
       beforeEach(async function() {
         this.Features.hasFeature.withArgs('affiliations').returns(true)
-        await this.UserController.promises.ensureAffiliationsMiddleware(
+        await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next
@@ -652,8 +652,8 @@ describe('UserController', function() {
           }
         ]
         this.Features.hasFeature.withArgs('affiliations').returns(true)
-        this.req.query.ensureAffiliations = true
-        await this.UserController.promises.ensureAffiliationsMiddleware(
+        this.req.query.ensureAffiliation = true
+        await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next
@@ -684,8 +684,8 @@ describe('UserController', function() {
           }
         ]
         this.Features.hasFeature.withArgs('affiliations').returns(true)
-        this.req.query.ensureAffiliations = true
-        await this.UserController.promises.ensureAffiliationsMiddleware(
+        this.req.query.ensureAffiliation = true
+        await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next
@@ -716,8 +716,8 @@ describe('UserController', function() {
           }
         ]
         this.Features.hasFeature.withArgs('affiliations').returns(true)
-        this.req.query.ensureAffiliations = true
-        await this.UserController.promises.ensureAffiliationsMiddleware(
+        this.req.query.ensureAffiliation = true
+        await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next
@@ -748,8 +748,8 @@ describe('UserController', function() {
           }
         ]
         this.Features.hasFeature.withArgs('affiliations').returns(true)
-        this.req.query.ensureAffiliations = true
-        await this.UserController.promises.ensureAffiliationsMiddleware(
+        this.req.query.ensureAffiliation = true
+        await this.UserController.promises.ensureAffiliationMiddleware(
           this.req,
           this.res,
           this.next