diff --git a/services/web/app/src/Features/User/SAMLIdentityManager.js b/services/web/app/src/Features/User/SAMLIdentityManager.js index c4896dd5e5..9e89f132e4 100644 --- a/services/web/app/src/Features/User/SAMLIdentityManager.js +++ b/services/web/app/src/Features/User/SAMLIdentityManager.js @@ -205,15 +205,16 @@ function _sendUnlinkedEmail(primaryEmail, providerName, institutionEmail) { }) } -async function getUser(providerId, externalUserId) { - if (!providerId || !externalUserId) { +async function getUser(providerId, externalUserId, userIdAttribute) { + if (!providerId || !externalUserId || !userIdAttribute) { throw new Error( - `invalid arguments: providerId: ${providerId}, externalUserId: ${externalUserId}` + `invalid arguments: providerId: ${providerId}, externalUserId: ${externalUserId}, userIdAttribute: ${userIdAttribute}` ) } const user = await User.findOne({ 'samlIdentifiers.externalUserId': externalUserId.toString(), 'samlIdentifiers.providerId': providerId.toString(), + 'samlIdentifiers.userIdAttribute': userIdAttribute.toString(), }).exec() return user @@ -246,6 +247,12 @@ async function linkAccounts(userId, samlData, auditLog) { userIdAttribute, } = samlData + if (!externalUserId || !institutionEmail || !providerId || !userIdAttribute) { + throw new Error( + `missing data when linking institution SSO: ${JSON.stringify(samlData)}` + ) + } + await _addIdentifier( userId, externalUserId, diff --git a/services/web/test/unit/src/User/SAMLIdentityManagerTests.js b/services/web/test/unit/src/User/SAMLIdentityManagerTests.js index ce8005e3bc..7f56b19b1c 100644 --- a/services/web/test/unit/src/User/SAMLIdentityManagerTests.js +++ b/services/web/test/unit/src/User/SAMLIdentityManagerTests.js @@ -98,14 +98,53 @@ describe('SAMLIdentityManager', function () { }) describe('getUser', function () { - it('should throw an error if missing provider ID and/or external user ID', async function () { + it('should throw an error if missing all of: provider ID, external user ID, attribute', async function () { let error try { - await this.SAMLIdentityManager.getUser(null, null) + await this.SAMLIdentityManager.getUser(undefined, undefined, undefined) } catch (e) { error = e } finally { expect(error).to.exist + expect(error.message).to.equal( + 'invalid arguments: providerId: undefined, externalUserId: undefined, userIdAttribute: undefined' + ) + } + }) + it('should throw an error if missing provider ID', async function () { + let error + try { + await this.SAMLIdentityManager.getUser(undefined, 'id123', 'someAttr') + } catch (e) { + error = e + } finally { + expect(error).to.exist + expect(error.message).to.equal( + 'invalid arguments: providerId: undefined, externalUserId: id123, userIdAttribute: someAttr' + ) + } + }) + it('should throw an error if missing external user ID', async function () { + let error + try { + await this.SAMLIdentityManager.getUser('123', null, 'someAttr') + } catch (e) { + error = e + } finally { + expect(error).to.exist + } + }) + it('should throw an error if missing attribute', async function () { + let error + try { + await this.SAMLIdentityManager.getUser('123', 'id123', undefined) + } catch (e) { + error = e + } finally { + expect(error).to.exist + expect(error.message).to.equal( + 'invalid arguments: providerId: 123, externalUserId: id123, userIdAttribute: undefined' + ) } }) }) @@ -118,7 +157,7 @@ describe('SAMLIdentityManager', function () { this.UserGetter.promises.getUser.onSecondCall().resolves(this.user) }) - it('should throw an error if missing data', async function () { + it('should throw an error if missing all data', async function () { let error try { await this.SAMLIdentityManager.linkAccounts(null, null, null) @@ -129,6 +168,33 @@ describe('SAMLIdentityManager', function () { } }) + describe('linking data', function () { + const requiredData = { + externalUserId: 'someUniqueId', + institutionEmail: 'user@example.com', + providerId: '123', + userIdAttribute: 'attribute', + } + for (const [data] of Object.entries(requiredData)) { + const testData = { ...requiredData } + delete testData[data] + let error + it(`should throw an error when missing ${data}`, async function () { + try { + await this.SAMLIdentityManager.linkAccounts('123', testData, {}) + } catch (e) { + error = e + } finally { + expect(error).to.exist + console.log(error) + expect(error.message).to.contain( + 'missing data when linking institution SSO' + ) + } + }) + } + }) + describe('when email is already associated with another Overleaf account', function () { beforeEach(function () { this.UserGetter.promises.getUserByAnyEmail.resolves( @@ -148,6 +214,7 @@ describe('SAMLIdentityManager', function () { universityId: 'provider-id', universityName: 'provider-name', hasEntitlement: true, + userIdAttribute: 'someAttribute', }, { intiatorId: '6005c75b12cbcaf771f4a105', @@ -184,6 +251,7 @@ describe('SAMLIdentityManager', function () { universityId: 'provider-id', universityName: 'provider-name', hasEntitlement: true, + userIdAttribute: 'someAttribute', }, { intiatorId: 'user-id-1', @@ -221,6 +289,7 @@ describe('SAMLIdentityManager', function () { universityId: 'provider-id', universityName: 'provider-name', hasEntitlement: true, + userIdAttribute: 'someAttribute', }, { intiatorId: 'user-id-1', @@ -256,6 +325,7 @@ describe('SAMLIdentityManager', function () { universityId: 'provider-id', universityName: 'provider-name', hasEntitlement: true, + userIdAttribute: 'someAttribute', }, { intiatorId: '6005c75b12cbcaf771f4a105', @@ -288,6 +358,7 @@ describe('SAMLIdentityManager', function () { universityId: 123456, universityName: 'provider-name', hasEntitlement: true, + userIdAttribute: 'someAttribute', }, { intiatorId: '6005c75b12cbcaf771f4a105', @@ -322,6 +393,7 @@ describe('SAMLIdentityManager', function () { universityId: '1', universityName: 'Overleaf University', hasEntitlement: false, + userIdAttribute: 'someAttribute', }, { intiatorId: '6005c75b12cbcaf771f4a105', @@ -389,6 +461,7 @@ describe('SAMLIdentityManager', function () { universityId: '1', universityName: 'Overleaf University', hasEntitlement: false, + userIdAttribute: 'someAttribute', }, { intiatorId: '6005c75b12cbcaf771f4a105',