diff --git a/services/web/app/src/Features/Errors/Errors.js b/services/web/app/src/Features/Errors/Errors.js
index 1469b5dacb..5b2ff97d6c 100644
--- a/services/web/app/src/Features/Errors/Errors.js
+++ b/services/web/app/src/Features/Errors/Errors.js
@@ -66,13 +66,27 @@ class NotInV2Error extends BackwardCompatibleError {}
class SLInV2Error extends BackwardCompatibleError {}
-class SAMLIdentityExistsError extends BackwardCompatibleError {
- constructor(arg) {
- super(arg)
- if (!this.message) {
- this.message =
- 'provider and external id already linked to another account'
- }
+class SAMLIdentityExistsError extends OError {
+ get i18nKey() {
+ return 'institution_account_tried_to_add_already_registered'
+ }
+}
+
+class SAMLAlreadyLinkedError extends OError {
+ get i18nKey() {
+ return 'institution_account_tried_to_add_already_linked'
+ }
+}
+
+class SAMLEmailNotAffiliatedError extends OError {
+ get i18nKey() {
+ return 'institution_account_tried_to_add_not_affiliated'
+ }
+}
+
+class SAMLEmailAffiliatedWithAnotherInstitutionError extends OError {
+ get i18nKey() {
+ return 'institution_account_tried_to_add_affiliated_with_another_institution'
}
}
@@ -170,6 +184,12 @@ class InvalidQueryError extends OErrorV2CompatibleError {
class AffiliationError extends OError {}
+class InvalidInstitutionalEmailError extends OError {
+ get i18nKey() {
+ return 'invalid_institutional_email'
+ }
+}
+
module.exports = {
OError,
BackwardCompatibleError,
@@ -189,6 +209,9 @@ module.exports = {
InvalidError,
NotInV2Error,
SAMLIdentityExistsError,
+ SAMLAlreadyLinkedError,
+ SAMLEmailNotAffiliatedError,
+ SAMLEmailAffiliatedWithAnotherInstitutionError,
SAMLSessionDataMissing,
SLInV2Error,
ThirdPartyIdentityExistsError,
@@ -199,5 +222,6 @@ module.exports = {
UserNotCollaboratorError,
DocHasRangesError,
InvalidQueryError,
- AffiliationError
+ AffiliationError,
+ InvalidInstitutionalEmailError
}
diff --git a/services/web/app/src/Features/Institutions/InstitutionsAPI.js b/services/web/app/src/Features/Institutions/InstitutionsAPI.js
index fce48d9401..fa2e7547ec 100644
--- a/services/web/app/src/Features/Institutions/InstitutionsAPI.js
+++ b/services/web/app/src/Features/Institutions/InstitutionsAPI.js
@@ -1,10 +1,14 @@
+const OError = require('@overleaf/o-error')
const logger = require('logger-sharelatex')
const metrics = require('@overleaf/metrics')
const settings = require('settings-sharelatex')
const request = require('request')
const { promisifyAll } = require('../../util/promises')
const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
-const { V1ConnectionError } = require('../Errors/Errors')
+const {
+ V1ConnectionError,
+ InvalidInstitutionalEmailError
+} = require('../Errors/Errors')
const InstitutionsAPI = {
getInstitutionAffiliations(institutionId, callback) {
@@ -65,18 +69,32 @@ const InstitutionsAPI = {
department,
role,
confirmedAt,
- entitlement
+ entitlement,
+ rejectIfBlocklisted
} = affiliationOptions
makeAffiliationRequest(
{
method: 'POST',
path: `/api/v2/users/${userId.toString()}/affiliations`,
- body: { email, university, department, role, confirmedAt, entitlement },
+ body: {
+ email,
+ university,
+ department,
+ role,
+ confirmedAt,
+ entitlement,
+ rejectIfBlocklisted
+ },
defaultErrorMessage: "Couldn't create affiliation"
},
function(error, body) {
if (error) {
- return callback(error, body)
+ if (error.info.statusCode === 422) {
+ return callback(
+ new InvalidInstitutionalEmailError(error.message).withCause(error)
+ )
+ }
+ return callback(error)
}
if (!university) {
return callback(null, body)
@@ -214,7 +232,9 @@ var makeAffiliationRequest = function(requestOptions, callback) {
{ path: requestOptions.path, body: requestOptions.body },
errorMessage
)
- return callback(new Error(errorMessage))
+ return callback(
+ new OError(errorMessage, { statusCode: response.statusCode })
+ )
}
callback(null, body)
diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js
index e576c7b024..77982ab0a5 100644
--- a/services/web/app/src/Features/Project/ProjectController.js
+++ b/services/web/app/src/Features/Project/ProjectController.js
@@ -380,7 +380,6 @@ const ProjectController = {
const timer = new metrics.Timer('project-list')
const userId = AuthenticationController.getLoggedInUserId(req)
const currentUser = AuthenticationController.getSessionUser(req)
- let institutionLinkingError
async.parallel(
{
tags(cb) {
@@ -517,7 +516,7 @@ const ProjectController = {
if (
samlSession.requestedEmail &&
samlSession.emailNonCanonical &&
- !samlSession.linkedToAnother
+ !samlSession.error
) {
notificationsInstitution.push({
institutionEmail: samlSession.emailNonCanonical,
@@ -533,7 +532,7 @@ const ProjectController = {
if (
samlSession.registerIntercept &&
samlSession.institutionEmail &&
- !samlSession.linkedToAnother
+ !samlSession.error
) {
notificationsInstitution.push({
email: samlSession.institutionEmail,
@@ -541,20 +540,11 @@ const ProjectController = {
})
}
- // Notification: Already linked to another account
- if (samlSession.linkedToAnother) {
- notificationsInstitution.push({
- templateKey: 'notification_institution_sso_linked_by_another'
- })
- }
-
// Notification: When there is a session error
if (samlSession.error) {
- institutionLinkingError = samlSession.error
notificationsInstitution.push({
- message: samlSession.error.message,
templateKey: 'notification_institution_sso_error',
- tryAgain: samlSession.error.tryAgain
+ error: samlSession.error
})
}
}
@@ -591,7 +581,6 @@ const ProjectController = {
userAffiliations,
userEmails,
hasSubscription: results.hasSubscription,
- institutionLinkingError,
zipFileSizeLimit: Settings.maxUploadSize
}
diff --git a/services/web/app/src/Features/User/SAMLIdentityManager.js b/services/web/app/src/Features/User/SAMLIdentityManager.js
index c13615e020..a4575b75bb 100644
--- a/services/web/app/src/Features/User/SAMLIdentityManager.js
+++ b/services/web/app/src/Features/User/SAMLIdentityManager.js
@@ -1,3 +1,4 @@
+const { ObjectId } = require('mongodb')
const EmailHandler = require('../Email/EmailHandler')
const Errors = require('../Errors/Errors')
const InstitutionsAPI = require('../Institutions/InstitutionsAPI')
@@ -32,6 +33,61 @@ async function _addAuditLogEntry(
)
}
+async function _ensureCanAddIdentifier(userId, institutionEmail, providerId) {
+ const userWithProvider = await UserGetter.promises.getUser(
+ { _id: ObjectId(userId), 'samlIdentifiers.providerId': providerId },
+ { _id: 1 }
+ )
+
+ if (userWithProvider) {
+ throw new Errors.SAMLAlreadyLinkedError()
+ }
+
+ const userWithEmail = await UserGetter.promises.getUserByAnyEmail(
+ institutionEmail
+ )
+
+ if (!userWithEmail) {
+ // email doesn't exist; all good
+ return
+ }
+
+ const emailBelongToUser = userWithEmail._id.toString() === userId.toString()
+ const existingEmailData = userWithEmail.emails.find(
+ emailData => emailData.email === institutionEmail
+ )
+
+ if (!emailBelongToUser && existingEmailData.samlProviderId) {
+ // email exists and institution link.
+ // Return back to requesting page with error
+ throw new Errors.SAMLIdentityExistsError()
+ }
+
+ if (!emailBelongToUser) {
+ // email exists but not linked, so redirect to linking page
+ // which will tell this user to log out to link
+ throw new Errors.EmailExistsError()
+ }
+
+ // email belongs to user. Make sure it's already affiliated with the provider
+ const fullEmails = await UserGetter.promises.getUserFullEmails(
+ userWithEmail._id
+ )
+ const existingFullEmailData = fullEmails.find(
+ emailData => emailData.email === institutionEmail
+ )
+
+ if (!existingFullEmailData.affiliation) {
+ throw new Errors.SAMLEmailNotAffiliatedError()
+ }
+
+ if (
+ existingFullEmailData.affiliation.institution.id.toString() !== providerId
+ ) {
+ throw new Errors.SAMLEmailAffiliatedWithAnotherInstitutionError()
+ }
+}
+
async function _addIdentifier(
userId,
externalUserId,
@@ -41,26 +97,10 @@ async function _addIdentifier(
providerName,
auditLog
) {
- // first check if institutionEmail linked to another account
- // before adding the identifier for the email
- const user = await UserGetter.promises.getUserByAnyEmail(institutionEmail)
- if (user && user._id.toString() !== userId.toString()) {
- const existingEmailData = user.emails.find(
- emailData => emailData.email === institutionEmail
- )
- if (existingEmailData && existingEmailData.samlProviderId) {
- // email exists and institution link.
- // Return back to requesting page with error
- throw new Errors.SAMLIdentityExistsError()
- } else {
- // Only email exists but not linked, so redirect to linking page
- // which will tell this user to log out to link
- throw new Errors.EmailExistsError()
- }
- }
-
providerId = providerId.toString()
+ await _ensureCanAddIdentifier(userId, institutionEmail, providerId)
+
await _addAuditLogEntry(
true,
userId,
@@ -86,11 +126,15 @@ async function _addIdentifier(
}
}
}
+
try {
// update v2 user record
- const updatedUser = User.findOneAndUpdate(query, update, {
+ const updatedUser = await User.findOneAndUpdate(query, update, {
new: true
}).exec()
+ if (!updatedUser) {
+ throw new OError('No update while linking user')
+ }
return updatedUser
} catch (err) {
if (err.code === 11000) {
@@ -121,7 +165,12 @@ async function _addInstitutionEmail(userId, email, providerId, auditLog) {
} else if (emailAlreadyAssociated) {
await UserUpdater.promises.updateUser(query, update)
} else {
- await UserUpdater.promises.addEmailAddress(user._id, email, {}, auditLog)
+ await UserUpdater.promises.addEmailAddress(
+ user._id,
+ email,
+ { university: { id: providerId }, rejectIfBlocklisted: true },
+ auditLog
+ )
await UserUpdater.promises.updateUser(query, update)
}
}
@@ -209,7 +258,12 @@ async function linkAccounts(
providerName,
auditLog
)
- await _addInstitutionEmail(userId, institutionEmail, providerId, auditLog)
+ try {
+ await _addInstitutionEmail(userId, institutionEmail, providerId, auditLog)
+ } catch (error) {
+ await _removeIdentifier(userId, providerId)
+ throw error
+ }
await UserUpdater.promises.confirmEmail(userId, institutionEmail) // will set confirmedAt if not set, and will always update reconfirmedAt
await _sendLinkedEmail(userId, providerName, institutionEmail)
// update v1 affiliations record
@@ -234,9 +288,6 @@ async function unlinkAccounts(
auditLog
) {
providerId = providerId.toString()
- const query = {
- _id: userId
- }
await _addAuditLogEntry(
false,
@@ -246,7 +297,20 @@ async function unlinkAccounts(
providerId,
providerName
)
+ // update v2 user
+ await _removeIdentifier(userId, providerId)
+ // update v1 affiliations record
+ await InstitutionsAPI.promises.removeEntitlement(userId, institutionEmail)
+ // send email
+ _sendUnlinkedEmail(primaryEmail, providerName, institutionEmail)
+}
+async function _removeIdentifier(userId, providerId) {
+ providerId = providerId.toString()
+
+ const query = {
+ _id: userId
+ }
const update = {
$pull: {
samlIdentifiers: {
@@ -254,12 +318,7 @@ async function unlinkAccounts(
}
}
}
- // update v2 user
await User.updateOne(query, update).exec()
- // update v1 affiliations record
- await InstitutionsAPI.promises.removeEntitlement(userId, institutionEmail)
- // send email
- _sendUnlinkedEmail(primaryEmail, providerName, institutionEmail)
}
async function updateEntitlement(
diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js
index 35252f2c0b..4e8f9f5b7a 100644
--- a/services/web/app/src/Features/User/UserPagesController.js
+++ b/services/web/app/src/Features/User/UserPagesController.js
@@ -82,10 +82,7 @@ const UserPagesController = {
institutionLinked
)
}
- const institutionLinkedToAnother = _.get(req.session, [
- 'saml',
- 'linkedToAnother'
- ])
+ const samlError = _.get(req.session, ['saml', 'error'])
const institutionEmailNonCanonical = _.get(req.session, [
'saml',
'emailNonCanonical'
@@ -94,7 +91,6 @@ const UserPagesController = {
'saml',
'requestedEmail'
])
- const institutionLinkingError = _.get(req.session, ['saml', 'error'])
delete req.session.saml
let shouldAllowEditingDetails = true
if (Settings.ldap && Settings.ldap.updateUserDetailsOnLogin) {
@@ -122,12 +118,11 @@ const UserPagesController = {
),
oauthUseV2: Settings.oauthUseV2 || false,
institutionLinked,
- institutionLinkedToAnother,
+ samlError,
institutionEmailNonCanonical:
institutionEmailNonCanonical && institutionRequestedEmail
? institutionEmailNonCanonical
: undefined,
- institutionLinkingError,
samlBeta: req.session.samlBeta,
ssoError: ssoError,
thirdPartyIds: UserPagesController._restructureThirdPartyIds(user)
diff --git a/services/web/app/views/_mixins/saml.pug b/services/web/app/views/_mixins/saml.pug
deleted file mode 100644
index f87a3a8b61..0000000000
--- a/services/web/app/views/_mixins/saml.pug
+++ /dev/null
@@ -1,9 +0,0 @@
-mixin samlErrorLoggedIn(error)
- i.fa.fa-exclamation-triangle(aria-hidden="true")
- | #{translate("generic_something_went_wrong")}.
- if error.message
- br
- | #{institutionLinkingError.message}
- if error.tryAgain
- br
- | #{translate("try_again")}.
diff --git a/services/web/app/views/project/list/notifications.pug b/services/web/app/views/project/list/notifications.pug
index 04c03bf037..c83c0ed721 100644
--- a/services/web/app/views/project/list/notifications.pug
+++ b/services/web/app/views/project/list/notifications.pug
@@ -1,5 +1,3 @@
-include ../../_mixins/saml
-
.user-notifications(ng-controller="NotificationsController")
include ./unsupported-browser
@@ -179,28 +177,20 @@ include ../../_mixins/saml
span(aria-hidden="true") ×
span.sr-only #{translate("close")}
- .alert.alert-danger(
- ng-switch-when="notification_institution_sso_linked_by_another"
- )
+ .alert.alert-danger(ng-switch-when="notification_institution_sso_error")
.notification-body
div
i.fa.fa-fw.fa-exclamation-triangle(aria-hidden="true")
- | !{translate("institution_account_tried_to_add_already_registered")}
+ | #{translate("generic_something_went_wrong")}.
+ div(ng-if="notification.error.translatedMessage" ng-bind-html="notification.error.translatedMessage")
+ div(ng-else="notification.error.message") {{ notification.error.message}}
+ div(ng-if="notification.error.tryAgain") #{translate("try_again")}.
+
.notification-close
button(ng-click="dismiss(notification)").close.pull-right
span(aria-hidden="true") ×
span.sr-only #{translate("close")}
- if institutionLinkingError
- .alert.alert-danger(ng-switch-when="notification_institution_sso_error")
- .notification-body
- div
- +samlErrorLoggedIn(institutionLinkingError)
- .notification-close
- button(ng-click="dismiss(notification)").close.pull-right
- span(aria-hidden="true") ×
- span.sr-only #{translate("close")}
-
ul.list-unstyled(
ng-controller="EmailNotificationController",
ng-cloak
diff --git a/services/web/app/views/user/settings/user-affiliations.pug b/services/web/app/views/user/settings/user-affiliations.pug
index 51b33b2ba7..6d3901e39a 100644
--- a/services/web/app/views/user/settings/user-affiliations.pug
+++ b/services/web/app/views/user/settings/user-affiliations.pug
@@ -1,5 +1,3 @@
-include ../../_mixins/saml
-
mixin aboutInstitutionLink()
a(href="/learn/how-to/Institutional_Login") #{translate("find_out_more_about_institution_login")}.
@@ -267,21 +265,8 @@ form.row(
i.fa.fa-exclamation-triangle(aria-hidden="true")
|
| !{translate("in_order_to_match_institutional_metadata", {email: institutionEmailNonCanonical})}
- if institutionLinkedToAnother
- tr.affiliations-table-error-row(ng-if="!hideInstitutionNotifications.error")
- td.text-center(aria-live="assertive" colspan="3")
- 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")}
- if institutionLinkingError
+
+ if samlError
tr.affiliations-table-error-row(ng-if="!hideInstitutionNotifications.linkError")
td.text-center(aria-live="assertive" colspan="3")
button.close(
@@ -292,8 +277,16 @@ form.row(
)
span(aria-hidden="true") ×
.small
- +samlErrorLoggedIn(institutionLinkingError)
- hr
+ i.fa.fa-fw.fa-exclamation-triangle(aria-hidden="true")
+ | #{translate("generic_something_went_wrong")}.
+ br
+ if samlError.translatedMessage
+ | !{samlError.translatedMessage}
+ else if samlError.message
+ | #{samlError.message}
+ if samlError.tryAgain
+ br
+ | #{translate("try_again")}.
script(type="text/ng-template", id="affiliationFormTpl")
.affiliations-form-group(
diff --git a/services/web/locales/en.json b/services/web/locales/en.json
index 5e417fb27f..f2de760170 100644
--- a/services/web/locales/en.json
+++ b/services/web/locales/en.json
@@ -164,6 +164,9 @@
"in_order_to_match_institutional_metadata": "In order to match your institutional metadata, we've linked your account using __email__.",
"in_order_to_match_institutional_metadata_associated": "In order to match your institutional metadata, your account is associated with the email __email__.",
"institution_account_tried_to_add_already_registered": "The email/institution account you tried to add is already registered with __appName__.",
+ "institution_account_tried_to_add_already_linked": "This institution is already linked with your account via another email address.",
+ "institution_account_tried_to_add_not_affiliated": "This email is already associated with your account but not affiliated with this institution.",
+ "institution_account_tried_to_add_affiliated_with_another_institution": "This email is already associated with your account but affiliated with another institution.",
"institution_acct_successfully_linked": "Your __appName__ account was successfully linked to your __institutionName__ institutional account.",
"institution_email_new_to_app": "Your __institutionName__ email (__email__) is new to __appName__.",
"institutional": "Institutional",
diff --git a/services/web/test/unit/src/Institutions/InstitutionsAPITests.js b/services/web/test/unit/src/Institutions/InstitutionsAPITests.js
index a90962a6b7..2fb3ce64fa 100644
--- a/services/web/test/unit/src/Institutions/InstitutionsAPITests.js
+++ b/services/web/test/unit/src/Institutions/InstitutionsAPITests.js
@@ -199,7 +199,7 @@ describe('InstitutionsAPI', function() {
requestOptions.method.should.equal('POST')
const { body } = requestOptions
- Object.keys(body).length.should.equal(6)
+ Object.keys(body).length.should.equal(7)
body.email.should.equal(this.newEmail)
body.university.should.equal(affiliationOptions.university)
body.department.should.equal(affiliationOptions.department)
diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js
index b82b3de39e..cda5e7015c 100644
--- a/services/web/test/unit/src/Project/ProjectControllerTests.js
+++ b/services/web/test/unit/src/Project/ProjectControllerTests.js
@@ -3,6 +3,7 @@ const path = require('path')
const sinon = require('sinon')
const { expect } = require('chai')
const { ObjectId } = require('mongodb')
+const Errors = require('../../../../app/src/Features/Errors/Errors')
const MODULE_PATH = path.join(
__dirname,
@@ -605,6 +606,7 @@ describe('ProjectController', function() {
}
this.ProjectController.projectListPage(this.req, this.res)
})
+
it('should show a notification when intent was to register via SSO but account existed', function() {
this.res.render = (pageName, opts) => {
expect(opts.notificationsInstitution).to.deep.include({
@@ -625,6 +627,7 @@ describe('ProjectController', function() {
}
this.ProjectController.projectListPage(this.req, this.res)
})
+
it('should not show a register notification if the flow was abandoned', function() {
// could initially start to register with an SSO email and then
// abandon flow and login with an existing non-institution SSO email
@@ -642,35 +645,24 @@ describe('ProjectController', function() {
}
this.ProjectController.projectListPage(this.req, this.res)
})
- it('should show institution account linked to another account', function() {
+
+ it('should show error notification', function() {
this.res.render = (pageName, opts) => {
- expect(opts.notificationsInstitution).to.deep.include({
- templateKey: 'notification_institution_sso_linked_by_another'
- })
- // Also check other notifications are not shown
- expect(opts.notificationsInstitution).to.not.deep.include({
- email: this.institutionEmail,
- templateKey: 'notification_institution_sso_already_registered'
- })
- expect(opts.notificationsInstitution).to.not.deep.include({
- institutionEmail: this.institutionEmail,
- requestedEmail: 'requested@overleaf.com',
- templateKey: 'notification_institution_sso_non_canonical'
- })
- expect(opts.notificationsInstitution).to.not.deep.include({
- email: this.institutionEmail,
- institutionName: this.institutionName,
- templateKey: 'notification_institution_sso_linked'
- })
+ expect(opts.notificationsInstitution.length).to.equal(1)
+ expect(opts.notificationsInstitution[0].templateKey).to.equal(
+ 'notification_institution_sso_error'
+ )
+ expect(opts.notificationsInstitution[0].error).to.be.instanceof(
+ Errors.SAMLAlreadyLinkedError
+ )
}
this.req.session.saml = {
- emailNonCanonical: this.institutionEmail,
institutionEmail: this.institutionEmail,
- requestedEmail: 'requested@overleaf.com',
- linkedToAnother: true
+ error: new Errors.SAMLAlreadyLinkedError()
}
this.ProjectController.projectListPage(this.req, this.res)
})
+
describe('for an unconfirmed domain for an SSO institution', function() {
beforeEach(function(done) {
this.UserGetter.getUserFullEmails.yields(null, [
diff --git a/services/web/test/unit/src/User/SAMLIdentityManagerTests.js b/services/web/test/unit/src/User/SAMLIdentityManagerTests.js
index 59433e2a25..39294b8578 100644
--- a/services/web/test/unit/src/User/SAMLIdentityManagerTests.js
+++ b/services/web/test/unit/src/User/SAMLIdentityManagerTests.js
@@ -1,3 +1,4 @@
+const { ObjectId } = require('mongodb')
const sinon = require('sinon')
const chai = require('chai')
const { expect } = chai
@@ -9,7 +10,7 @@ describe('SAMLIdentityManager', function() {
const linkedEmail = 'another@example.com'
beforeEach(function() {
- this.userId = 'user-id-1'
+ this.userId = '6005c75b12cbcaf771f4a105'
this.user = {
_id: this.userId,
email: 'not-linked@overleaf.com',
@@ -21,13 +22,13 @@ describe('SAMLIdentityManager', function() {
ipAddress: '0:0:0:0'
}
this.userAlreadyLinked = {
- _id: 'user-id-2',
+ _id: '6005c7a012cbcaf771f4a106',
email: 'linked@overleaf.com',
emails: [{ email: 'linked@overleaf.com', samlProviderId: '1' }],
samlIdentifiers: [{ externalUserId: 'linked-id', providerId: '1' }]
}
this.userEmailExists = {
- _id: 'user-id-3',
+ _id: '6005c7a012cbcaf771f4a107',
email: 'exists@overleaf.com',
emails: [{ email: 'exists@overleaf.com' }],
samlIdentifiers: []
@@ -69,7 +70,7 @@ describe('SAMLIdentityManager', function() {
'../../models/User': {
User: (this.User = {
findOneAndUpdate: sinon.stub().returns({
- exec: sinon.stub().resolves()
+ exec: sinon.stub().resolves(this.user)
}),
findOne: sinon.stub().returns({
exec: sinon.stub().resolves()
@@ -88,7 +89,8 @@ describe('SAMLIdentityManager', function() {
getUser: sinon.stub(),
promises: {
getUser: sinon.stub().resolves(this.user),
- getUserByAnyEmail: sinon.stub().resolves()
+ getUserByAnyEmail: sinon.stub().resolves(),
+ getUserFullEmails: sinon.stub().resolves()
}
}),
'../User/UserUpdater': (this.UserUpdater = {
@@ -120,6 +122,12 @@ describe('SAMLIdentityManager', function() {
describe('linkAccounts', function() {
describe('errors', function() {
+ beforeEach(function() {
+ // first call is to get userWithProvider; should be falsy
+ this.UserGetter.promises.getUser.onFirstCall().resolves()
+ this.UserGetter.promises.getUser.onSecondCall().resolves(this.user)
+ })
+
it('should throw an error if missing data', async function() {
let error
try {
@@ -148,12 +156,46 @@ describe('SAMLIdentityManager', function() {
let error
try {
await this.SAMLIdentityManager.linkAccounts(
- 'user-id-1',
+ '6005c75b12cbcaf771f4a105',
'not-linked-id',
'exists@overleaf.com',
'provider-id',
'provider-name',
true,
+ {
+ intiatorId: '6005c75b12cbcaf771f4a105',
+ ip: '0:0:0:0'
+ }
+ )
+ } catch (e) {
+ error = e
+ } finally {
+ expect(error).to.be.instanceof(Errors.EmailExistsError)
+ expect(this.User.findOneAndUpdate).to.not.have.been.called
+ }
+ })
+ })
+
+ describe('when email is not affiliated', function() {
+ beforeEach(function() {
+ this.UserGetter.promises.getUserByAnyEmail.resolves(this.user)
+ this.UserGetter.promises.getUserFullEmails.resolves([
+ {
+ email: 'not-affiliated@overleaf.com'
+ }
+ ])
+ })
+
+ it('should throw SAMLEmailNotAffiliatedError', async function() {
+ let error
+ try {
+ await this.SAMLIdentityManager.linkAccounts(
+ '6005c75b12cbcaf771f4a105',
+ 'not-linked-id',
+ 'not-affiliated@overleaf.com',
+ 'provider-id',
+ 'provider-name',
+ true,
{
intiatorId: 'user-id-1',
ip: '0:0:0:0'
@@ -162,7 +204,44 @@ describe('SAMLIdentityManager', function() {
} catch (e) {
error = e
} finally {
- expect(error).to.be.instanceof(Errors.EmailExistsError)
+ expect(error).to.be.instanceof(Errors.SAMLEmailNotAffiliatedError)
+ expect(this.User.findOneAndUpdate).to.not.have.been.called
+ }
+ })
+ })
+
+ describe('when email is affiliated with another institution', function() {
+ beforeEach(function() {
+ this.UserGetter.promises.getUserByAnyEmail.resolves(this.user)
+ this.UserGetter.promises.getUserFullEmails.resolves([
+ {
+ email: 'affiliated@overleaf.com',
+ affiliation: { institution: { id: '987' } }
+ }
+ ])
+ })
+
+ it('should throw SAMLEmailAffiliatedWithAnotherInstitutionError', async function() {
+ let error
+ try {
+ await this.SAMLIdentityManager.linkAccounts(
+ '6005c75b12cbcaf771f4a105',
+ 'not-linked-id',
+ 'affiliated@overleaf.com',
+ 'provider-id',
+ 'provider-name',
+ true,
+ {
+ intiatorId: 'user-id-1',
+ ip: '0:0:0:0'
+ }
+ )
+ } catch (e) {
+ error = e
+ } finally {
+ expect(error).to.be.instanceof(
+ Errors.SAMLEmailAffiliatedWithAnotherInstitutionError
+ )
expect(this.User.findOneAndUpdate).to.not.have.been.called
}
})
@@ -179,14 +258,14 @@ describe('SAMLIdentityManager', function() {
let error
try {
await this.SAMLIdentityManager.linkAccounts(
- 'user-id-1',
+ '6005c75b12cbcaf771f4a105',
'already-linked-id',
'linked@overleaf.com',
'provider-id',
'provider-name',
true,
{
- intiatorId: 'user-id-1',
+ intiatorId: '6005c75b12cbcaf771f4a105',
ip: '0:0:0:0'
}
)
@@ -199,6 +278,42 @@ describe('SAMLIdentityManager', function() {
})
})
+ describe('when institution provider is already associated with the user', function() {
+ beforeEach(function() {
+ // first call is to get userWithProvider; resolves with any user
+ this.UserGetter.promises.getUser.onFirstCall().resolves(this.user)
+ })
+
+ it('should throw an SAMLAlreadyLinkedError error', async function() {
+ let error
+ try {
+ await this.SAMLIdentityManager.linkAccounts(
+ '6005c75b12cbcaf771f4a105',
+ 'already-linked-id',
+ 'linked@overleaf.com',
+ 123456,
+ 'provider-name',
+ true,
+ {
+ intiatorId: '6005c75b12cbcaf771f4a105',
+ ip: '0:0:0:0'
+ }
+ )
+ } catch (e) {
+ error = e
+ } finally {
+ expect(
+ this.UserGetter.promises.getUser
+ ).to.have.been.calledWithMatch({
+ _id: ObjectId('6005c75b12cbcaf771f4a105'),
+ 'samlIdentifiers.providerId': '123456'
+ })
+ expect(error).to.be.instanceof(Errors.SAMLAlreadyLinkedError)
+ expect(this.User.findOneAndUpdate).to.not.have.been.called
+ }
+ })
+ })
+
it('should pass back errors via UserAuditLogHandler', async function() {
let error
const anError = new Error('oops')
@@ -212,7 +327,7 @@ describe('SAMLIdentityManager', function() {
'Overleaf University',
undefined,
{
- intiatorId: 'user-id-1',
+ intiatorId: '6005c75b12cbcaf771f4a105',
ipAddress: '0:0:0:0'
}
)
@@ -228,9 +343,15 @@ describe('SAMLIdentityManager', function() {
})
describe('success', function() {
+ beforeEach(function() {
+ // first call is to get userWithProvider; should be falsy
+ this.UserGetter.promises.getUser.onFirstCall().resolves()
+ this.UserGetter.promises.getUser.onSecondCall().resolves(this.user)
+ })
+
it('should update the user audit log', function() {
const auditLog = {
- intiatorId: 'user-id-1',
+ intiatorId: '6005c75b12cbcaf771f4a105',
ip: '0:0:0:0'
}
this.SAMLIdentityManager.linkAccounts(
@@ -258,6 +379,7 @@ describe('SAMLIdentityManager', function() {
}
)
})
+
it('should send an email notification', function() {
this.SAMLIdentityManager.linkAccounts(
this.user._id,
@@ -267,7 +389,7 @@ describe('SAMLIdentityManager', function() {
'Overleaf University',
undefined,
{
- intiatorId: 'user-id-1',
+ intiatorId: '6005c75b12cbcaf771f4a105',
ipAddress: '0:0:0:0'
},
() => {