Merge pull request #2167 from overleaf/jel-saml-account-settings-layout

Account settings layout for institution SSO

GitOrigin-RevId: d9c9e5eeb3b4a215456b0f5294139c1b8d4968c3

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

@@ -6,6 +6,7 @@ const UserGetter = require('../User/UserGetter')
 const UserUpdater = require('../User/UserUpdater')
 
 function _addIdentifier(userId, externalUserId, providerId) {
+  providerId = providerId.toString()
   const query = {
     _id: userId,
     'samlIdentifiers.providerId': {
@@ -36,71 +37,52 @@ function _addIdentifier(userId, externalUserId, providerId) {
   return updatedUser
 }
 
-async function _addInstitutionEmail(userId, email) {
+function _getUserQuery(providerId, externalUserId) {
+  externalUserId = externalUserId.toString()
+  providerId = providerId.toString()
+  const query = {
+    'samlIdentifiers.externalUserId': externalUserId,
+    'samlIdentifiers.providerId': providerId
+  }
+  return query
+}
+
+async function _addInstitutionEmail(userId, email, providerId) {
   const user = await UserGetter.promises.getUser(userId)
+  const query = {
+    _id: userId,
+    'emails.email': email
+  }
+  const update = {
+    $set: {
+      'emails.$.samlProviderId': providerId.toString()
+    }
+  }
   if (user == null) {
     logger.log(userId, 'could not find user for institution SAML linking')
     throw new Errors.NotFoundError('user not found')
   }
   const emailAlreadyAssociated = user.emails.find(e => e.email === email)
   if (emailAlreadyAssociated && emailAlreadyAssociated.confirmedAt) {
-    // nothing to do, email is already added and confirmed
+    await UserUpdater.promises.updateUser(query, update)
   } else if (emailAlreadyAssociated) {
     // add and confirm email
-    await _confirmEmail(user._id, email)
+    await UserUpdater.promises.confirmEmail(user._id, email)
+    await UserUpdater.promises.updateUser(query, update)
   } else {
     // add and confirm email
-    await _addEmail(user._id, email)
-    await _confirmEmail(user._id, email)
+    await UserUpdater.promises.addEmailAddress(user._id, email)
+    await UserUpdater.promises.confirmEmail(user._id, email)
+    await UserUpdater.promises.updateUser(query, update)
   }
 }
 
-function _addEmail(userId, institutionEmail) {
-  return new Promise((resolve, reject) => {
-    UserUpdater.addEmailAddress(userId, institutionEmail, function(
-      error,
-      addEmailResult
-    ) {
-      if (error) {
-        logger.log(
-          error,
-          userId,
-          'could not add institution email after SAML linking'
-        )
-        reject(error)
-      } else {
-        resolve()
-      }
-    })
-  })
-}
-
-function _confirmEmail(userId, institutionEmail) {
-  return new Promise((resolve, reject) => {
-    UserUpdater.confirmEmail(userId, institutionEmail, function(
-      error,
-      confirmedResult
-    ) {
-      if (error) {
-        logger.log(
-          error,
-          userId,
-          'could not confirm institution email after SAML linking'
-        )
-        reject(error)
-      } else {
-        resolve()
-      }
-    })
-  })
-}
-
 async function getUser(providerId, externalUserId) {
   if (providerId == null || externalUserId == null) {
     throw new Error('invalid arguments')
   }
   try {
-    const query = SAMLIdentityManager._getUserQuery(providerId, externalUserId)
+    const query = _getUserQuery(providerId, externalUserId)
     let user = await User.findOne(query).exec()
     if (!user) {
       throw new Errors.SAMLUserNotFoundError()
@@ -117,20 +99,15 @@ async function linkAccounts(
   institutionEmail,
   providerId
 ) {
-  await _addIdentifier(userId, externalUserId, providerId)
-  await _addInstitutionEmail(userId, institutionEmail)
+  try {
+    await _addIdentifier(userId, externalUserId, providerId)
+    await _addInstitutionEmail(userId, institutionEmail, providerId)
+  } catch (error) {
+    throw error
+  }
 }
 
 const SAMLIdentityManager = {
-  _getUserQuery(providerId, externalUserId) {
-    externalUserId = externalUserId.toString()
-    providerId = providerId.toString()
-    const query = {
-      'samlIdentifiers.externalUserId': externalUserId,
-      'samlIdentifiers.providerId': providerId
-    }
-    return query
-  },
   getUser,
   linkAccounts
 }

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

@@ -4,6 +4,7 @@ const ErrorController = require('../Errors/ErrorController')
 const logger = require('logger-sharelatex')
 const Settings = require('settings-sharelatex')
 const AuthenticationController = require('../Authentication/AuthenticationController')
+const _ = require('lodash')
 
 const UserPagesController = {
   registerPage(req, res) {
@@ -106,10 +107,15 @@ const UserPagesController = {
 
   settingsPage(req, res, next) {
     const userId = AuthenticationController.getLoggedInUserId(req)
+    // SSO
     const ssoError = req.session.ssoError
     if (ssoError) {
       delete req.session.ssoError
     }
+    // Institution SSO
+    const institutionLinked = _.get(req.session, ['saml', 'linked'])
+    const institutionNotLinked = _.get(req.session, ['saml', 'notLinked'])
+    delete req.session.saml
     logger.log({ user: userId }, 'loading settings page')
     let shouldAllowEditingDetails = true
     if (Settings.ldap && Settings.ldap.updateUserDetailsOnLogin) {
@@ -136,9 +142,11 @@ const UserPagesController = {
           req
         ),
         oauthUseV2: Settings.oauthUseV2 || false,
+        samlInitPath: _.get(Settings, ['saml', 'ukamf', 'initPath']),
+        institutionLinked,
+        institutionNotLinked,
         ssoError: ssoError,
-        thirdPartyIds: UserPagesController._restructureThirdPartyIds(user),
-        previewOauth: req.query.prvw != null
+        thirdPartyIds: UserPagesController._restructureThirdPartyIds(user)
       })
     })
   },

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

@@ -4,6 +4,7 @@ const metrics = require('metrics-sharelatex')
 const { db } = mongojs
 const async = require('async')
 const { ObjectId } = mongojs
+const { promisify } = require('util')
 const UserGetter = require('./UserGetter')
 const {
   addAffiliation,
@@ -249,4 +250,12 @@ const UserUpdater = {
   metrics.timeAsyncMethod(UserUpdater, method, 'mongo.UserUpdater', logger)
 )
 
+const promises = {
+  addEmailAddress: promisify(UserUpdater.addEmailAddress),
+  confirmEmail: promisify(UserUpdater.confirmEmail),
+  updateUser: promisify(UserUpdater.updateUser)
+}
+
+UserUpdater.promises = promises
+
 module.exports = UserUpdater

services/web/app/views/user/settings.pug

@@ -174,7 +174,7 @@ block content
 							div
 								a(id="sessions-link", href="/user/sessions") #{translate("manage_sessions")}
 
-							if hasFeature('oauth') || previewOauth
+							if hasFeature('oauth')
 								hr
 								include settings/user-oauth
 
@@ -292,3 +292,4 @@ block content
 	
 	script(type='text/javascript').
 		window.passwordStrengthOptions = !{StringHelper.stringifyJsonForScript(settings.passwordStrengthOptions || {})}
+		window.samlInitPath = '#{samlInitPath}';

services/web/app/views/user/settings/user-affiliations.pug

@@ -13,18 +13,18 @@ form.row(
 					th.affiliations-table-inline-actions
 			tbody
 				tr(
-					ng-repeat="userEmail in userEmails"
+					ng-repeat-start="userEmail in userEmails"
 				)
 					td
 						| {{ userEmail.email + (userEmail.default ? ' (primary)' : '') }}
 						div(ng-if="!userEmail.confirmedAt").small
 							strong #{translate('unconfirmed')}.
-							|
-							| #{translate('please_check_your_inbox')}.
+								span(ng-if="!userEmail.affiliation.institution.ssoEnabled") #{translate('please_check_your_inbox')}.
 							br
 							a(
 								href,
-								ng-click="resendConfirmationEmail(userEmail)"
+								ng-click="resendConfirmationEmail(userEmail)",
+								ng-if="!userEmail.affiliation.institution.ssoEnabled"
 							) #{translate('resend_confirmation_email')}
 						div(ng-if="userEmail.confirmedAt && userEmail.affiliation.institution && userEmail.affiliation.institution.confirmed && userEmail.affiliation.institution.licence != 'free'").small
 							span.label.label-primary #{translate("professional")}
@@ -72,7 +72,7 @@ form.row(
 						div.affiliations-table-inline-action-disabled-wrapper(
 							tooltip=translate("please_confirm_your_email_before_making_it_default")
 							tooltip-enable="!ui.isMakingRequest"
-							ng-if="!userEmail.default && (!userEmail.confirmedAt || ui.isMakingRequest)"
+							ng-if="!userEmail.default && (!userEmail.confirmedAt || ui.isMakingRequest) && !institutionAlreadyLinked(userEmail)"
 						)
 							button.btn.btn-sm.btn-success.affiliations-table-inline-action(
 								disabled
@@ -91,6 +91,24 @@ form.row(
 						)
 							i.fa.fa-fw.fa-trash(aria-hidden="true")
 							span.sr-only #{translate("remove")}
+				tr.affiliations-table-saml-row(ng-repeat-end ng-if="userEmail.affiliation && userEmail.affiliation && userEmail.affiliation.institution.ssoEnabled")
+					if hasFeature('saml')
+						td
+						td(ng-attr-colspan="{{userEmail.samlProviderId ? '2' : '1'}}" ng-class="institutionAlreadyLinked(userEmail) ? '' : 'with-border'")
+							p.small(ng-if="userEmail.samlProviderId")
+								| !{translate("acct_linked_to_institution_acct", {institutionName: '{{userEmail.affiliation.institution.name}}'})}
+							div(ng-if="!userEmail.samlProviderId && !institutionAlreadyLinked(userEmail)")
+								//- this email is not linked to institution login but
+								//- cannot have multiple emails at same institution linked for "institution login"
+								//- so check if institution is already linked
+								p.small
+									| !{translate("can_link_your_institution_acct", {institutionName: '{{userEmail.affiliation.institution.name}}'})}
+								p.small
+									| !{translate("doing_this_allow_log_in_through_institution")} 
+									a(href="/") #{translate("find_out_more_about_institution_login")}
+						td.with-border.affiliations-table-inline-actions(ng-if="!userEmail.samlProviderId && !institutionAlreadyLinked(userEmail)")
+							button.btn-sm.btn.btn-info(ng-click="linkInstitutionAcct(userEmail.email, userEmail.affiliation.institution.id)" ng-disabled="ui.isMakingRequest")
+								| #{translate("link_accounts")}
 				tr.affiliations-table-highlighted-row(
 					ng-if="!ui.showAddEmailUI && !ui.isMakingRequest"
 				)
@@ -160,7 +178,10 @@ form.row(
 					td.text-center(colspan="3", ng-if="ui.isResendingConfirmation") 
 						i.fa.fa-fw.fa-spin.fa-refresh(aria-hidden="true")
 						|  #{translate("sending")}...
-					td.text-center(colspan="3", ng-if="!ui.isLoadingEmails && !ui.isResendingConfirmation") 
+					td.text-center(colspan="3", ng-if="ui.isLinkingInstitution") 
+						i.fa.fa-fw.fa-spin.fa-refresh(aria-hidden="true")
+						|  #{translate("processing")}...
+					td.text-center(colspan="3", ng-if="!ui.isLoadingEmails && !ui.isResendingConfirmations && !ui.isLinkingInstitution") 
 						i.fa.fa-fw.fa-spin.fa-refresh(aria-hidden="true")
 						|  #{translate("saving")}...
 				tr.affiliations-table-error-row(
@@ -171,7 +192,47 @@ form.row(
 							i.fa.fa-fw.fa-exclamation-triangle(aria-hidden="true")
 							span(ng-if="!ui.errorMessage")  #{translate("error_performing_request")}
 							span(ng-if="ui.errorMessage")  {{ui.errorMessage}}
-				
+				if institutionLinked
+					tr.affiliations-table-info-row(ng-if="!hideInstitutionNotifications.info")
+						td(colspan="3").text-center(aria-live="assertive")
+							button.close(
+								type="button"
+								data-dismiss="modal"
+								ng-click="closeInstitutionNotification('info')"
+								aria-label=translate("close")
+							)
+								span(aria-hidden="true") ×
+							.small !{translate("institution_acct_successfully_linked", {institutionName: institutionLinked.universityName})}
+							if institutionLinked.hasEntitlement
+								.small !{translate("this_grants_access_to_features", {featureType: translate("professional")})}
+					if institutionLinked.emailViaInstitution
+						tr.affiliations-table-warning-row(ng-if="!hideInstitutionNotifications.warning")
+							td(colspan="3").text-center(aria-live="assertive")
+								button.close(
+									type="button"
+									data-dismiss="modal"
+									ng-click="closeInstitutionNotification('warning')"
+									aria-label=translate("close")
+								)
+									span(aria-hidden="true") ×
+								.small
+									i.fa.fa-exclamation-triangle(aria-hidden="true")
+									|  
+									| !{translate("in_order_to_match_institutional_metadata", {email: institutionLinked.emailViaInstitution})}
+				if institutionNotLinked
+					tr.affiliations-table-error-row(ng-if="!hideInstitutionNotifications.error")
+						td(colspan="3").text-center(aria-live="assertive")
+							button.close(
+								type="button"
+								data-dismiss="modal"
+								ng-click="closeInstitutionNotification('error')"
+								aria-label=translate("close")
+							)
+								span(aria-hidden="true") ×
+							.small
+								i.fa.fa-exclamation-triangle(aria-hidden="true")
+								|  
+								| !{translate("institution_account_tried_to_add_already_registered")}
 		hr
 
 script(type="text/ng-template", id="affiliationFormTpl")

services/web/public/src/main/affiliations/controllers/UserAffiliationsController.js

@@ -19,9 +19,15 @@ define(['base'], App =>
     $scope,
     UserAffiliationsDataService,
     $q,
+    $window,
     _
   ) {
     $scope.userEmails = []
+    $scope.linkedInstitutionIds = []
+    $scope.hideInstitutionNotifications = {}
+    $scope.closeInstitutionNotification = type => {
+      $scope.hideInstitutionNotifications[type] = true
+    }
 
     const LOCAL_AND_DOMAIN_REGEX = /([^@]+)@(.+)/
     const EMAIL_REGEX = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\ ".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA -Z\-0-9]+\.)+[a-zA-Z]{2,}))$/
@@ -80,6 +86,14 @@ define(['base'], App =>
       }
     }
 
+    $scope.linkInstitutionAcct = function(email, institutionId) {
+      $scope.ui.isMakingRequest = true
+      $scope.ui.isLinkingInstitution = true
+      $window.location.href = `${
+        window.samlInitPath
+      }?university_id=${institutionId}&auto=true&email=${email}`
+    }
+
     $scope.selectUniversityManually = function() {
       $scope.newAffiliation.university = null
       $scope.newAffiliation.department = null
@@ -224,6 +238,7 @@ define(['base'], App =>
         errorMessage: '',
         showChangeAffiliationUI: false,
         isMakingRequest: false,
+        isLinkingInstitution: false,
         isLoadingEmails: false,
         isAddingNewEmail: false,
         isResendingConfirmation: false
@@ -249,11 +264,31 @@ define(['base'], App =>
       return promise
     }
 
+    $scope.institutionAlreadyLinked = function(emailData) {
+      const institutionId =
+        emailData.affiliation &&
+        emailData.affiliation.institution &&
+        emailData.affiliation.institution &&
+        emailData.affiliation.institution.id
+          ? emailData.affiliation.institution.id.toString()
+          : undefined
+      return $scope.linkedInstitutionIds.indexOf(institutionId) !== -1
+    }
+
     // Populates the emails table
     var _getUserEmails = function() {
       $scope.ui.isLoadingEmails = true
       return _monitorRequest(UserAffiliationsDataService.getUserEmails())
-        .then(emails => ($scope.userEmails = emails))
+        .then(emails => {
+          $scope.userEmails = emails
+          $scope.linkedInstitutionIds = emails
+            .filter(email => {
+              if (email.samlProviderId) {
+                return email.samlProviderId
+              }
+            })
+            .map(email => email.samlProviderId)
+        })
         .finally(() => ($scope.ui.isLoadingEmails = false))
     }
     return _getUserEmails()

services/web/public/stylesheets/app/account-settings.less

@@ -1,65 +1,98 @@
 .account-settings {
-	.alert {
-		margin-bottom: 0;
-	}
-	h3 {
-		margin-top: 0;
-	}
+  .alert {
+    margin-bottom: 0;
+  }
+  h3 {
+    margin-top: 0;
+  }
 }
 
 #delete-account-modal {
-	.alert {
-		margin-top: 25px;
-		margin-bottom: 4px;
-	}
-	.confirmation-checkbox-wrapper {
-		padding-top: 8px;
-		input {
-			margin-right: 6px;
-		}
-	}
+  .alert {
+    margin-top: 25px;
+    margin-bottom: 4px;
+  }
+  .confirmation-checkbox-wrapper {
+    padding-top: 8px;
+    input {
+      margin-right: 6px;
+    }
+  }
 }
 
 .affiliations-table {
-	table-layout: fixed;
+  table-layout: fixed;
+}
+.affiliations-table-email {
+  width: 40%;
+}
+.affiliations-table-institution {
+  width: 40%;
+}
+.affiliations-table-inline-actions {
+  padding: 0 !important;
+  text-align: right;
+  word-wrap: break-word;
+  button {
+    margin: @table-cell-padding 0;
+  }
+}
+.affiliations-table-inline-action {
+  text-transform: capitalize;
+}
+.affiliations-table-inline-action-disabled-wrapper {
+  display: inline-block;
+}
+.affiliations-table-highlighted-row {
+  background-color: tint(@content-alt-bg-color, 6%);
+}
+.affiliations-table-error-row {
+  background-color: @alert-danger-bg;
+  color: @alert-danger-text;
+  .btn {
+    margin-top: @table-cell-padding;
+    .button-variant(
+      @btn-danger-color; darken(@btn-danger-bg, 8%) ; @btn-danger-border
+    );
+  }
+  .small {
+    color: @alert-danger-text;
+  }
+}
+.affiliations-table-info-row {
+  background-color: @alert-info-bg;
+  color: @alert-info-text;
+  .small {
+    color: @alert-info-text;
+  }
+}
+.affiliations-table-warning-row {
+  background-color: @alert-warning-bg;
+  color: @alert-warning-text;
+  .small {
+    color: @alert-warning-text;
+  }
+}
+tbody > tr.affiliations-table-saml-row > td:not(.with-border) {
+  border: 0;
+}
+tbody > tr.affiliations-table-info-row > td {
+  border: 0;
+}
+tbody > tr.affiliations-table-warning-row > td {
+  border: 0;
+}
+.affiliations-form-group {
+  margin-top: @table-cell-padding;
+  &:first-child {
+    margin-top: 0;
+  }
+}
+.affiliation-change-container,
+.affiliation-change-actions {
+  margin-top: @table-cell-padding;
 }
-	.affiliations-table-email {
-		width: 40%;
-	}
-	.affiliations-table-institution {
-		width: 40%;
-	}
-	.affiliations-table-inline-actions {
-		text-align: right;
-	}
-		.affiliations-table-inline-action {
-			text-transform: capitalize;
-		}
-		.affiliations-table-inline-action-disabled-wrapper {
-			display: inline-block;
-		}
-	.affiliations-table-highlighted-row {
-		background-color: tint(@content-alt-bg-color, 6%);
-	}
-	.affiliations-table-error-row {
-		background-color: @alert-danger-bg;
-		color: @alert-danger-text;
-		.btn {
-			margin-top: @table-cell-padding;
-			.button-variant(@btn-danger-color; darken(@btn-danger-bg, 8%); @btn-danger-border);
-		}
-	}
-	.affiliations-form-group {
-		margin-top: @table-cell-padding;
-		&:first-child {
-			margin-top: 0;
-		}
-	}
-	.affiliation-change-container,
-	.affiliation-change-actions {
-		margin-top: @table-cell-padding;
-	}
 
-	.affiliations-table-label {
-		padding-top: 4px;
-	}
+.affiliations-table-label {
+  padding-top: 4px;
+}

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

@@ -0,0 +1,61 @@
+const sinon = require('sinon')
+const chai = require('chai')
+const { expect } = chai
+const SandboxedModule = require('sandboxed-module')
+const modulePath = '../../../../app/src/Features/User/SAMLIdentityManager.js'
+
+describe('SAMLIdentityManager', function() {
+  beforeEach(function() {
+    this.Errors = {
+      NotFoundError: sinon.stub(),
+      SAMLIdentityExistsError: sinon.stub(),
+      SAMLUserNotFoundError: sinon.stub()
+    }
+    this.user = {
+      _id: 'user-id'
+    }
+    this.SAMLIdentityManager = SandboxedModule.require(modulePath, {
+      requires: {
+        '../Errors/Errors': this.Errors,
+        '../../models/User': {
+          User: (this.User = {
+            findOne: sinon.stub()
+          })
+        },
+        '../User/UserGetter': (this.UserGetter = {
+          getUser: sinon.stub(),
+          promises: {
+            getUser: sinon.stub().resolves()
+          }
+        }),
+        '../User/UserUpdater': (this.UserUpdater = {
+          addEmailAddress: sinon.stub()
+        })
+      }
+    })
+  })
+  describe('getUser', function() {
+    it('should throw an error if missing provider ID and/or external user ID', async function() {
+      let error
+      try {
+        await this.SAMLIdentityManager.getUser(null, null)
+      } catch (e) {
+        error = e
+      } finally {
+        expect(error).to.exist
+      }
+    })
+  })
+  describe('linkAccounts', function() {
+    it('should throw an error if missing data', async function() {
+      let error
+      try {
+        await this.SAMLIdentityManager.linkAccounts(null, null, null, null)
+      } catch (e) {
+        error = e
+      } finally {
+        expect(error).to.exist
+      }
+    })
+  })
+})