From 03aaee84a38aeaf66091a9ebd420e03e0e7eab44 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe <5312836+lawshe@users.noreply.github.com> Date: Wed, 7 Feb 2024 10:28:05 -0600 Subject: [PATCH] Merge pull request #16945 from overleaf/ab-fix-sso-managed-users-enrollment [web] Fix managed users enrollment clearing out SSO linking status GitOrigin-RevId: b2083b48df1782c426794f16e2cdd767b217256c --- .../Features/Subscription/GroupSSOHandler.js | 84 ----- .../acceptance/src/helpers/Subscription.js | 17 + .../sso/group-settings-sso.spec.tsx | 300 ------------------ .../src/Subscription/GroupSSOHandlerTests.js | 209 ------------ 4 files changed, 17 insertions(+), 593 deletions(-) delete mode 100644 services/web/app/src/Features/Subscription/GroupSSOHandler.js delete mode 100644 services/web/test/frontend/features/group-management/components/sso/group-settings-sso.spec.tsx delete mode 100644 services/web/test/unit/src/Subscription/GroupSSOHandlerTests.js diff --git a/services/web/app/src/Features/Subscription/GroupSSOHandler.js b/services/web/app/src/Features/Subscription/GroupSSOHandler.js deleted file mode 100644 index 45fe595882..0000000000 --- a/services/web/app/src/Features/Subscription/GroupSSOHandler.js +++ /dev/null @@ -1,84 +0,0 @@ -const { SSOConfig } = require('../../models/SSOConfig') -const UserAuditLogHandler = require('../User/UserAuditLogHandler') -const UserUpdater = require('../User/UserUpdater') -const SAMLIdentityManager = require('../User/SAMLIdentityManager') -const { User } = require('../../models/User') -const Errors = require('../Errors/Errors') -const GroupUtils = require('./GroupUtils') - -async function checkUserCanEnrollInSubscription(userId, subscription) { - const ssoConfig = await SSOConfig.findById(subscription?.ssoConfig).exec() - if (!ssoConfig?.enabled) { - throw new Errors.SAMLGroupSSODisabledError() - } - - const userIsMember = subscription.member_ids.some( - memberId => memberId.toString() === userId.toString() - ) - if (!userIsMember) { - throw new Errors.SAMLGroupSSOLoginIdentityNotFoundError() - } - - const user = await User.findOne({ _id: userId }, { enrollment: 1 }).exec() - - const userIsEnrolled = user.enrollment?.sso?.some( - enrollment => enrollment.groupId.toString() === subscription._id.toString() - ) - if (userIsEnrolled) { - throw new Errors.SAMLIdentityExistsError() - } -} - -async function enrollInSubscription( - userId, - subscription, - externalUserId, - userIdAttribute, - auditLog -) { - await checkUserCanEnrollInSubscription(userId, subscription) - - const providerId = GroupUtils.getProviderId(subscription._id) - - const userBySamlIdentifier = await SAMLIdentityManager.getUser( - providerId, - externalUserId, - userIdAttribute - ) - - if (userBySamlIdentifier) { - throw new Errors.SAMLIdentityExistsError() - } - - const samlIdentifiers = { - externalUserId, - userIdAttribute, - providerId, - } - - await UserUpdater.promises.updateUser(userId, { - $push: { - samlIdentifiers, - 'enrollment.sso': { - groupId: subscription._id, - linkedAt: new Date(), - primary: true, - }, - }, - }) - - await UserAuditLogHandler.promises.addEntry( - userId, - 'group-sso-link', - auditLog.initiatorId, - auditLog.ipAddress, - samlIdentifiers - ) -} - -module.exports = { - promises: { - checkUserCanEnrollInSubscription, - enrollInSubscription, - }, -} diff --git a/services/web/test/acceptance/src/helpers/Subscription.js b/services/web/test/acceptance/src/helpers/Subscription.js index c44d77c418..3b4d6019cf 100644 --- a/services/web/test/acceptance/src/helpers/Subscription.js +++ b/services/web/test/acceptance/src/helpers/Subscription.js @@ -138,6 +138,23 @@ class Subscription { }) } + linkGroupSSO(user, externalUserId, userIdAttribute, auditLog, callback) { + SubscriptionModel.findById(this._id).exec((error, subscription) => { + if (error) { + return callback(error) + } + Modules.hooks.fire( + 'linkUserToGroupSSO', + user._id, + subscription, + externalUserId, + userIdAttribute, + auditLog, + callback + ) + }) + } + expectDeleted(deleterData, callback) { DeletedSubscriptionModel.find( { 'subscription._id': this._id }, diff --git a/services/web/test/frontend/features/group-management/components/sso/group-settings-sso.spec.tsx b/services/web/test/frontend/features/group-management/components/sso/group-settings-sso.spec.tsx deleted file mode 100644 index ea9e5c4188..0000000000 --- a/services/web/test/frontend/features/group-management/components/sso/group-settings-sso.spec.tsx +++ /dev/null @@ -1,300 +0,0 @@ -import GroupSettingsSSORoot from '../../../../../../modules/group-settings/frontend/js/components/sso/group-settings-sso-root' -import { SSOConfigurationProvider } from '../../../../../../modules/group-settings/frontend/js/context/sso-configuration-context' -import { singleLineCertificates } from '../../../../../../modules/group-settings/test/data/certificates' - -function GroupSettingsSSOComponent() { - return ( -
- - - -
- ) -} - -const GROUP_ID = '123abc' - -describe('GroupSettingsSSO', function () { - beforeEach(function () { - cy.window().then(win => { - win.metaAttributesCache = new Map() - win.metaAttributesCache.set('ol-groupId', GROUP_ID) - }) - }) - - it('renders sso settings in group management', function () { - cy.mount() - - cy.get('.group-settings-sso').within(() => { - cy.contains('Single Sign-On (SSO)') - cy.contains('Enable SSO') - }) - }) - - describe('GroupSettingsSSOEnable', function () { - it('renders without sso configuration', function () { - cy.mount() - - cy.contains('Enable SSO') - cy.contains( - 'Set up single sign-on for your group. This sign in method will be optional for group members unless Managed Users is enabled.' - ) - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').should('not.be.checked') - cy.get('.invisible-input').should('be.disabled') - }) - }) - - it('renders with sso configuration not validated', function () { - cy.intercept('GET', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: [ - { - value: singleLineCertificates[0], - }, - { value: singleLineCertificates[1] }, - ], - userIdAttribute: 'email', - enabled: false, - validated: false, - }, - }).as('sso') - - cy.mount() - - cy.wait('@sso') - - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').should('not.be.checked') - cy.get('.invisible-input').should('be.disabled') - }) - }) - - it('renders with sso configuration validated and not enabled', function () { - cy.intercept('GET', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: [ - { value: singleLineCertificates[0] }, - { value: singleLineCertificates[1] }, - ], - userIdAttribute: 'email', - validated: true, - enabled: false, - }, - }).as('sso') - - cy.mount() - - cy.wait('@sso') - - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').should('not.be.checked') - cy.get('.invisible-input').should('not.be.disabled') - }) - }) - - it('renders with sso configuration validated and enabled', function () { - cy.intercept('GET', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: [ - { value: singleLineCertificates[0] }, - { value: singleLineCertificates[1] }, - ], - userIdAttribute: 'email', - validated: true, - enabled: true, - }, - }).as('sso') - - cy.mount() - - cy.wait('@sso') - - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').should('be.checked') - cy.get('.invisible-input').should('not.be.disabled') - }) - }) - - it('updates the configuration, and checks the draft configuration message', function () { - cy.intercept('GET', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: [{ value: singleLineCertificates[0] }], - userIdAttribute: 'email', - validated: true, - enabled: false, - }, - }).as('sso') - - cy.intercept('POST', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: [{ value: singleLineCertificates[1] }], - userIdAttribute: 'email', - validated: false, - enabled: false, - }, - }).as('ssoUpdated') - - cy.mount() - - cy.wait('@sso') - - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').should('not.be.checked') - cy.get('.invisible-input').should('not.be.disabled') - }) - - cy.findByRole('button', { name: 'View configuration' }).click() - cy.findByRole('button', { name: 'Edit' }).click() - cy.findByRole('button', { name: 'Next' }).click() - cy.wait('@ssoUpdated') - cy.findByText('Your configuration has not been finalized.') - }) - - describe('sso enable modal', function () { - beforeEach(function () { - cy.intercept('GET', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: [{ value: singleLineCertificates[0] }], - userIdAttribute: 'email', - enabled: false, - }, - }).as('sso') - - cy.mount() - - cy.wait('@sso') - - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').click({ force: true }) - }) - }) - - it('render enable modal correctly', function () { - // enable modal - cy.get('.modal-dialog').within(() => { - cy.contains('Enable single sign-on') - cy.contains('What happens when SSO is enabled?') - }) - }) - - it('close enable modal if Cancel button is clicked', function () { - cy.get('.modal-dialog').within(() => { - cy.findByRole('button', { name: 'Cancel' }).click() - }) - - cy.get('.modal-dialog').should('not.exist') - }) - - it('enables SSO if Enable SSO button is clicked and shows success banner', function () { - cy.intercept('POST', `/manage/groups/${GROUP_ID}/settings/enableSSO`, { - statusCode: 200, - }).as('enableSSO') - - cy.intercept('GET', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: [{ value: singleLineCertificates[0] }], - userIdAttribute: 'email', - validated: true, - enabled: true, - }, - }).as('sso') - - cy.get('.modal-dialog').within(() => { - cy.findByRole('button', { name: 'Enable SSO' }).click() - }) - cy.get('.modal-dialog').should('not.exist') - - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').should('be.checked') - cy.get('.invisible-input').should('not.be.disabled') - }) - - cy.findByText('SSO is enabled') - }) - }) - - describe('SSO disable modal', function () { - beforeEach(function () { - cy.intercept('GET', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: [{ value: singleLineCertificates[0] }], - userIdAttribute: 'email', - validated: true, - enabled: true, - }, - }).as('sso') - - cy.mount() - - cy.wait('@sso') - - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').click({ force: true }) - }) - }) - - it('render disable modal correctly', function () { - // disable modal - cy.get('.modal-dialog').within(() => { - cy.contains('Disable single sign-on') - cy.contains( - 'You’re about to disable single sign-on for all group members.' - ) - }) - }) - - it('close disable modal if Cancel button is clicked', function () { - cy.get('.modal-dialog').within(() => { - cy.findByRole('button', { name: 'Cancel' }).click() - }) - - cy.get('.modal-dialog').should('not.exist') - }) - - it('disables SSO if Disable SSO button is clicked and shows success banner', function () { - cy.intercept('POST', `/manage/groups/${GROUP_ID}/settings/disableSSO`, { - statusCode: 200, - }).as('disableSSO') - - cy.intercept('GET', `/manage/groups/${GROUP_ID}/settings/sso`, { - statusCode: 200, - body: { - entryPoint: 'entrypoint', - certificates: ['cert'], - userIdAttribute: 'email', - validated: true, - enabled: false, - }, - }).as('sso') - - cy.get('.modal-dialog').within(() => { - cy.findByRole('button', { name: 'Disable SSO' }).click() - }) - cy.get('.modal-dialog').should('not.exist') - - cy.get('.switch-input').within(() => { - cy.get('.invisible-input').should('not.be.checked') - }) - - cy.findByText('SSO is disabled') - }) - }) - }) -}) diff --git a/services/web/test/unit/src/Subscription/GroupSSOHandlerTests.js b/services/web/test/unit/src/Subscription/GroupSSOHandlerTests.js deleted file mode 100644 index efab09a9ca..0000000000 --- a/services/web/test/unit/src/Subscription/GroupSSOHandlerTests.js +++ /dev/null @@ -1,209 +0,0 @@ -const SandboxedModule = require('sandboxed-module') -const sinon = require('sinon') -const chai = require('chai') -const { expect } = chai -const { ObjectId } = require('mongodb') -const Errors = require('../../../../app/src/Features/Errors/Errors') - -const MODULE_PATH = '../../../../app/src/Features/Subscription/GroupSSOHandler' - -describe('GroupSSOHandler', function () { - beforeEach(function () { - this.user = { _id: new ObjectId(), enrollment: { sso: [] } } - this.subscription = { - _id: new ObjectId().toString(), - admin_id: new ObjectId(), - member_ids: [this.user._id], - } - this.samlIdentifier = { - externalUserId: 'user@external.com', - userIdAttribute: 'email', - providerId: `ol-group-subscription-id:${this.subscription._id.toString()}`, - } - this.auditLog = { - initiatorId: 'test-initiator-id', - ipAddress: '127.0.0.1', - } - this.ssoConfigData = { - entryPoint: 'https://example.com/saml', - certificates: ['abc'], - userIdAttribute: 'nameId', - enabled: true, - } - - this.SSOConfig = { - findById: sinon.stub().returns({ - exec: sinon.stub().resolves(this.ssoConfigData), - }), - } - this.UserAuditLogHandler = { - promises: { - addEntry: sinon.stub().resolves(), - }, - } - this.UserUpdater = { - promises: { - updateUser: sinon.stub().resolves(), - }, - } - this.SAMLIdentityManager = { - getUser: sinon.stub().resolves(), - } - this.User = { - findOne: sinon.stub().returns({ - exec: sinon.stub().resolves(this.user), - }), - } - this.GroupSSOHandler = SandboxedModule.require(MODULE_PATH, { - requires: { - '../../models/SSOConfig': { SSOConfig: this.SSOConfig }, - '../User/UserAuditLogHandler': this.UserAuditLogHandler, - '../User/UserUpdater': this.UserUpdater, - '../User/SAMLIdentityManager': this.SAMLIdentityManager, - '../../models/User': { - User: this.User, - }, - }, - }) - }) - - describe('checkUserCanEnrollInSubscription', function () { - it('should throw an error if the subscription is not found', async function () { - this.SSOConfig.findById.returns({ - exec: sinon.stub().resolves(undefined), - }) - await expect( - this.GroupSSOHandler.promises.checkUserCanEnrollInSubscription( - this.user._id, - this.subscription - ) - ).to.be.rejectedWith(Errors.SAMLGroupSSODisabledError) - }) - - it('should throw an error if SSO is not enabled for the group', async function () { - const disabledSSOConfig = { ...this.ssoConfig, enabled: false } - this.SSOConfig.findById.returns({ - exec: sinon.stub().resolves(disabledSSOConfig), - }) - await expect( - this.GroupSSOHandler.promises.checkUserCanEnrollInSubscription( - this.user._id, - this.subscription - ) - ).to.be.rejectedWith(Errors.SAMLGroupSSODisabledError) - }) - - it('should throw an error if the user is not a member of the group', async function () { - const testSubscription = { - ...this.subscription, - member_ids: [], - } - await expect( - this.GroupSSOHandler.promises.checkUserCanEnrollInSubscription( - this.user._id, - testSubscription - ) - ).to.be.rejectedWith(Errors.SAMLGroupSSOLoginIdentityNotFoundError) - }) - - it('should throw an error if the user is already enrolled to the group', async function () { - const testUser = { - ...this.user, - enrollment: { - sso: [{ groupId: this.subscription._id }], - }, - } - this.User.findOne.returns({ - exec: sinon.stub().resolves(testUser), - }) - await expect( - this.GroupSSOHandler.promises.checkUserCanEnrollInSubscription( - this.user._id, - this.subscription - ) - ).to.be.rejectedWith(Errors.SAMLIdentityExistsError) - }) - - it('should resolve if the user can be enrolled to the group', async function () { - await expect( - this.GroupSSOHandler.promises.checkUserCanEnrollInSubscription( - this.user._id, - this.subscription - ) - ).to.be.fulfilled - }) - }) - - describe('enrollInSubscription', function () { - it('should throw an error if the user cannot be enrolled', async function () { - const disabledSSOConfig = { ...this.ssoConfig, enabled: false } - this.SSOConfig.findById.returns({ - exec: sinon.stub().resolves(disabledSSOConfig), - }) - await expect( - this.GroupSSOHandler.promises.enrollInSubscription( - this.user._id, - this.subscription, - this.samlIdentifier.externalUserId, - this.samlIdentifier.userIdAttribute, - this.auditLog - ) - ).to.be.rejectedWith(Errors.SAMLGroupSSODisabledError) - }) - - it('should throw an error if an identical SAML identity for the subscription/user already exists', async function () { - this.SAMLIdentityManager.getUser.resolves(this.user) - await expect( - this.GroupSSOHandler.promises.enrollInSubscription( - this.user._id, - this.subscription, - this.samlIdentifier.externalUserId, - this.samlIdentifier.userIdAttribute, - this.auditLog - ) - ).to.be.rejectedWith(Errors.SAMLIdentityExistsError) - }) - - it("should add an entry the user's SSO enrollment and samlIdentifiers lists", async function () { - await this.GroupSSOHandler.promises.enrollInSubscription( - this.user._id, - this.subscription, - this.samlIdentifier.externalUserId, - this.samlIdentifier.userIdAttribute, - this.auditLog - ) - expect(this.UserUpdater.promises.updateUser).to.have.been.calledWith( - this.user._id, - { - $push: { - samlIdentifiers: this.samlIdentifier, - 'enrollment.sso': { - groupId: this.subscription._id, - linkedAt: sinon.match.instanceOf(Date), - primary: true, - }, - }, - } - ) - }) - - it('should add an entry to the user audit log', async function () { - await this.GroupSSOHandler.promises.enrollInSubscription( - this.user._id, - this.subscription, - this.samlIdentifier.externalUserId, - this.samlIdentifier.userIdAttribute, - this.auditLog - ) - expect( - this.UserAuditLogHandler.promises.addEntry - ).to.have.been.calledWith( - this.user._id, - 'group-sso-link', - this.auditLog.initiatorId, - this.auditLog.ipAddress, - this.samlIdentifier - ) - }) - }) -})