Merge pull request #2220 from overleaf/jel-email-confirmation

Email confirmation only for non-institution SSO emails

GitOrigin-RevId: 95bd0ce077031c11b9d60d2f736a1abe7431a265

services/web/app/src/Features/Institutions/InstitutionsAPI.js

@@ -17,6 +17,24 @@ const settings = require('settings-sharelatex')
 const request = require('request')
 const { promisifyAll } = require('../../util/promises')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
+const V1Api = require('../V1/V1Api')
+
+function getInstitutionViaDomain(domain) {
+  return new Promise(function(resolve, reject) {
+    V1Api.request(
+      {
+        timeout: 20 * 1000,
+        uri: `api/v1/sharelatex/university_saml?hostname=${domain}`
+      },
+      function(error, response, body) {
+        if (error) {
+          reject(error)
+        }
+        resolve(body)
+      }
+    )
+  })
+}
 
 const InstitutionsAPI = {
   getInstitutionAffiliations(institutionId, callback) {
@@ -33,6 +51,8 @@ const InstitutionsAPI = {
     )
   },
 
+  getInstitutionViaDomain,
+
   getInstitutionLicences(institutionId, startDate, endDate, lag, callback) {
     if (callback == null) {
       callback = function(error, body) {}

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

@@ -1,5 +1,7 @@
 let UserEmailsController
 const AuthenticationController = require('../Authentication/AuthenticationController')
+const Features = require('../../infrastructure/Features')
+const InstitutionsAPI = require('../Institutions/InstitutionsAPI')
 const UserGetter = require('./UserGetter')
 const UserUpdater = require('./UserUpdater')
 const EmailHelper = require('../Helpers/EmailHelper')
@@ -38,6 +40,52 @@ function add(req, res, next) {
   })
 }
 
+async function resendConfirmation(req, res, next) {
+  const userId = AuthenticationController.getLoggedInUserId(req)
+  const email = EmailHelper.parseEmail(req.body.email)
+  if (!email) {
+    return res.sendStatus(422)
+  }
+  UserGetter.getUserByAnyEmail(email, { _id: 1 }, async function(error, user) {
+    if (error) {
+      return next(error)
+    }
+    if (!user || user._id.toString() !== userId) {
+      logger.log(
+        { userId, email, foundUserId: user && user._id },
+        "email doesn't match logged in user"
+      )
+      return res.sendStatus(422)
+    }
+    if (Features.hasFeature('saml') || req.session.samlBeta) {
+      // institution SSO emails cannot be confirmed by email,
+      // confirmation happens by linking to the institution
+      let institution
+      try {
+        institution = await InstitutionsAPI.getInstitutionViaDomain(
+          email.split('@').pop()
+        )
+      } catch (error) {
+        if (!(error instanceof Errors.NotFoundError)) {
+          throw error
+        }
+      }
+      if (institution && institution.sso_enabled) {
+        return res.sendStatus(422)
+      }
+    }
+    logger.log({ userId, email }, 'resending email confirmation token')
+    UserEmailsConfirmationHandler.sendConfirmationEmail(userId, email, function(
+      error
+    ) {
+      if (error) {
+        return next(error)
+      }
+      res.sendStatus(200)
+    })
+  })
+}
+
 module.exports = UserEmailsController = {
   list(req, res, next) {
     const userId = AuthenticationController.getLoggedInUserId(req)
@@ -102,36 +150,7 @@ module.exports = UserEmailsController = {
     )
   },
 
-  resendConfirmation(req, res, next) {
-    const userId = AuthenticationController.getLoggedInUserId(req)
-    const email = EmailHelper.parseEmail(req.body.email)
-    if (!email) {
-      return res.sendStatus(422)
-    }
-    UserGetter.getUserByAnyEmail(email, { _id: 1 }, function(error, user) {
-      if (error) {
-        return next(error)
-      }
-      if (!user || user._id.toString() !== userId) {
-        logger.log(
-          { userId, email, foundUserId: user && user._id },
-          "email doesn't match logged in user"
-        )
-        return res.sendStatus(422)
-      }
-      logger.log({ userId, email }, 'resending email confirmation token')
-      UserEmailsConfirmationHandler.sendConfirmationEmail(
-        userId,
-        email,
-        function(error) {
-          if (error) {
-            return next(error)
-          }
-          res.sendStatus(200)
-        }
-      )
-    })
-  },
+  resendConfirmation,
 
   showConfirm(req, res, next) {
     res.render('user/confirm_email', {

services/web/app/views/project/list/notifications.pug

@@ -55,7 +55,7 @@ span(ng-controller="NotificationsController").userNotifications
 	)
 		li.notification_entry(
 			ng-repeat="userEmail in userEmails",
-			ng-if="!userEmail.confirmedAt && !userEmail.hide"
+			ng-if="showConfirmEmail(userEmail)"
 		)
 			.row
 				.col-xs-12

services/web/public/src/main/project-list/notifications-controller.js

@@ -99,6 +99,19 @@ define(['base'], function(App) {
     UserAffiliationsDataService
   ) {
     $scope.userEmails = []
+    $scope.showConfirmEmail = email => {
+      if (!email.confirmedAt && !email.hide) {
+        if (
+          email.affiliation &&
+          email.affiliation.institution &&
+          email.affiliation.institution.ssoEnabled &&
+          (ExposedSettings.hasSamlBeta || ExposedSettings.hasSamlFeature)
+        ) {
+          return false
+        }
+        return true
+      }
+    }
     for (let userEmail of Array.from($scope.userEmails)) {
       userEmail.hide = false
     }

services/web/test/unit/src/Institutions/InstitutionsAPITests.js

@@ -43,6 +43,9 @@ describe('InstitutionsAPI', function() {
         request: this.request,
         '../Notifications/NotificationsBuilder': {
           ipMatcherAffiliation: sinon.stub().returns(this.ipMatcherNotification)
+        },
+        '../../../../../app/src/Features/V1/V1Api': {
+          request: sinon.stub()
         }
       }
     })

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

@@ -32,6 +32,9 @@ describe('UserEmailsController', function() {
       getLoggedInUserId: sinon.stub().returns(this.user._id),
       setInSessionUser: sinon.stub()
     }
+    this.Features = {
+      hasFeature: sinon.stub()
+    }
     this.UserUpdater = {
       addEmailAddress: sinon.stub(),
       removeEmailAddress: sinon.stub(),
@@ -40,6 +43,15 @@ describe('UserEmailsController', function() {
     }
     this.EmailHelper = { parseEmail: sinon.stub() }
     this.endorseAffiliation = sinon.stub().yields()
+    this.InstitutionsAPI = {
+      endorseAffiliation: this.endorseAffiliation,
+      getInstitutionViaDomain: sinon
+        .stub()
+        .withArgs('overleaf.com')
+        .resolves({ sso_enabled: true })
+        .withArgs('example.com')
+        .resolves({ sso_enabled: false })
+    }
     return (this.UserEmailsController = SandboxedModule.require(modulePath, {
       globals: {
         console: console
@@ -47,13 +59,12 @@ describe('UserEmailsController', function() {
       requires: {
         '../Authentication/AuthenticationController': this
           .AuthenticationController,
+        '../../infrastructure/Features': this.Features,
         './UserGetter': this.UserGetter,
         './UserUpdater': this.UserUpdater,
         '../Helpers/EmailHelper': this.EmailHelper,
         './UserEmailsConfirmationHandler': (this.UserEmailsConfirmationHandler = {}),
-        '../Institutions/InstitutionsAPI': {
-          endorseAffiliation: this.endorseAffiliation
-        },
+        '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
         '../Errors/Errors': Errors,
         'logger-sharelatex': {
           log() {
@@ -315,4 +326,88 @@ describe('UserEmailsController', function() {
       })
     })
   })
+  describe('resendConfirmation', function() {
+    beforeEach(function() {
+      this.req = {
+        body: {}
+      }
+      this.res = {
+        sendStatus: sinon.stub()
+      }
+      this.next = sinon.stub()
+      this.UserEmailsConfirmationHandler.sendConfirmationEmail = sinon
+        .stub()
+        .yields()
+    })
+    describe('when institution SSO is released', function() {
+      beforeEach(function() {
+        this.Features.hasFeature.withArgs('saml').returns(true)
+      })
+      describe('for an institution SSO email', function() {
+        beforeEach(function() {
+          this.req.body.email = 'with-sso@overleaf.com'
+        })
+        it('should not send the email', function() {
+          this.UserEmailsController.resendConfirmation(
+            this.req,
+            this.res,
+            () => {
+              this.UserEmailsConfirmationHandler.sendConfirmationEmail.should
+                .not.have.been.called.once
+            }
+          )
+        })
+      })
+      describe('for a non-institution SSO email', function() {
+        beforeEach(function() {
+          this.req.body.email = 'without-sso@example.com'
+        })
+        it('should send the email', function() {
+          this.UserEmailsController.resendConfirmation(
+            this.req,
+            this.res,
+            () => {
+              this.UserEmailsConfirmationHandler.sendConfirmationEmail.should
+                .have.been.called.once
+            }
+          )
+        })
+      })
+    })
+    describe('when institution SSO is not released', function() {
+      beforeEach(function() {
+        this.Features.hasFeature.withArgs('saml').returns(false)
+      })
+      describe('for an institution SSO email', function() {
+        beforeEach(function() {
+          this.req.body.email = 'with-sso@overleaf.com'
+        })
+        it('should send the email', function() {
+          this.UserEmailsController.resendConfirmation(
+            this.req,
+            this.res,
+            () => {
+              this.UserEmailsConfirmationHandler.sendConfirmationEmail.should
+                .have.been.called.once
+            }
+          )
+        })
+      })
+      describe('for a non-institution SSO email', function() {
+        beforeEach(function() {
+          this.req.body.email = 'without-sso@example.com'
+        })
+        it('should send the email', function() {
+          this.UserEmailsController.resendConfirmation(
+            this.req,
+            this.res,
+            () => {
+              this.UserEmailsConfirmationHandler.sendConfirmationEmail.should
+                .have.been.called.once
+            }
+          )
+        })
+      })
+    })
+  })
 })