From 7ff114bbef6a579a2434740af7484f33b090972e Mon Sep 17 00:00:00 2001 From: Jessica Lawshe <5312836+lawshe@users.noreply.github.com> Date: Thu, 23 Apr 2026 09:04:41 -0500 Subject: [PATCH] Merge pull request #32396 from overleaf/jel-unlink-sso [web] Add button so user can unlink Commons SSO GitOrigin-RevId: 46e0607549341a98beca3873ea63bf091a883e85 --- .../src/Features/User/SAMLIdentityManager.mjs | 30 ++-- .../web/frontend/extracted-translations.json | 7 + .../settings/components/emails/row.tsx | 61 ++++++-- .../emails/unlink-commons-sso-modal.tsx | 98 +++++++++++++ services/web/locales/en.json | 7 + .../components/emails/emails-row.test.tsx | 11 +- .../emails/unlink-commons-sso-modal.test.tsx | 137 ++++++++++++++++++ .../src/User/SAMLIdentityManager.test.mjs | 26 ++-- 8 files changed, 339 insertions(+), 38 deletions(-) create mode 100644 services/web/frontend/js/features/settings/components/emails/unlink-commons-sso-modal.tsx create mode 100644 services/web/test/frontend/features/settings/components/emails/unlink-commons-sso-modal.test.tsx diff --git a/services/web/app/src/Features/User/SAMLIdentityManager.mjs b/services/web/app/src/Features/User/SAMLIdentityManager.mjs index 983b5b4d76..77433f2fc5 100644 --- a/services/web/app/src/Features/User/SAMLIdentityManager.mjs +++ b/services/web/app/src/Features/User/SAMLIdentityManager.mjs @@ -244,6 +244,7 @@ async function redundantSubscription(userId, providerId, providerName) { } async function linkAccounts(userId, samlData, auditLog) { + // Only for Commons SSO const { externalUserId, institutionEmail, @@ -303,19 +304,30 @@ async function unlinkAccounts( } async function _removeIdentifier(userId, providerId) { + // Called either when a user deletes a linked email + // or unlinks their institution SSO. + // The identifier is stored both in samlIdentifiers array + // and in the emails array as samlProviderId. + // When a user deletes a linked email, the email is removed + // before this function is called, so we do expect no match + // in that scenario + providerId = providerId.toString() - const query = { - _id: userId, - } - const update = { - $pull: { - samlIdentifiers: { - providerId, + await User.updateOne( + { _id: userId }, + { + $pull: { + samlIdentifiers: { providerId }, + }, + $unset: { + 'emails.$[email].samlProviderId': 1, }, }, - } - await User.updateOne(query, update).exec() + { + arrayFilters: [{ 'email.samlProviderId': providerId }], + } + ).exec() } async function updateEntitlement( diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index ca1a2a873a..315ccd80d3 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -2134,6 +2134,12 @@ "unlink": "", "unlink_all_users": "", "unlink_all_users_explanation": "", + "unlink_commons_sso_confirm": "", + "unlink_commons_sso_error": "", + "unlink_commons_sso_lose_licence": "", + "unlink_commons_sso_no_login": "", + "unlink_commons_sso_title": "", + "unlink_commons_sso_troubleshoot": "", "unlink_dropbox_folder": "", "unlink_dropbox_warning": "", "unlink_from_sso": "", @@ -2144,6 +2150,7 @@ "unlink_provider_account_title": "", "unlink_provider_account_warning": "", "unlink_reference": "", + "unlink_sso": "", "unlink_the_project_from_the_current_github_repo": "", "unlink_user_explanation": "", "unlink_users": "", diff --git a/services/web/frontend/js/features/settings/components/emails/row.tsx b/services/web/frontend/js/features/settings/components/emails/row.tsx index c3c982d533..0c34c99989 100644 --- a/services/web/frontend/js/features/settings/components/emails/row.tsx +++ b/services/web/frontend/js/features/settings/components/emails/row.tsx @@ -14,6 +14,7 @@ import { useLocation } from '../../../../shared/hooks/use-location' import OLRow from '@/shared/components/ol/ol-row' import OLCol from '@/shared/components/ol/ol-col' import OLButton from '@/shared/components/ol/ol-button' +import UnlinkCommonsSSOModal from './unlink-commons-sso-modal' type EmailsRowProps = { userEmailData: UserEmailData @@ -65,6 +66,7 @@ function SSOAffiliationInfo({ userEmailData }: SSOAffiliationInfoProps) { const { t } = useTranslation() const { state } = useUserEmailsContext() const location = useLocation() + const [showUnlinkSSOModal, setShowUnlinkSSOModal] = useState(false) const [linkAccountsButtonDisabled, setLinkAccountsButtonDisabled] = useState(false) @@ -89,22 +91,49 @@ function SSOAffiliationInfo({ userEmailData }: SSOAffiliationInfoProps) { return ( - -

- ] - } - values={{ - institutionName: userEmailData.affiliation?.institution.name, - }} - shouldUnescape - tOptions={{ interpolation: { escapeValue: true } }} - /> -

- +
+ + + +

+ ] + } + values={{ + institutionName: + userEmailData.affiliation?.institution.name, + }} + shouldUnescape + tOptions={{ interpolation: { escapeValue: true } }} + /> +

+ + + + + setShowUnlinkSSOModal(true)} + > + {t('unlink_sso')} + + + setShowUnlinkSSOModal(false)} + institutionName={ + userEmailData.affiliation?.institution.name || '' + } + institutionEmail={userEmailData.email} + hasLicence={userEmailData.emailHasInstitutionLicence || false} + /> + + + ) diff --git a/services/web/frontend/js/features/settings/components/emails/unlink-commons-sso-modal.tsx b/services/web/frontend/js/features/settings/components/emails/unlink-commons-sso-modal.tsx new file mode 100644 index 0000000000..a03e047dff --- /dev/null +++ b/services/web/frontend/js/features/settings/components/emails/unlink-commons-sso-modal.tsx @@ -0,0 +1,98 @@ +import { + OLModal, + OLModalBody, + OLModalFooter, + OLModalHeader, + OLModalTitle, +} from '@/shared/components/ol/ol-modal' +import OLNotification from '@/shared/components/ol/ol-notification' +import OLButton from '@/shared/components/ol/ol-button' +import { postJSON } from '@/infrastructure/fetch-json' +import { debugConsole } from '@/utils/debugging' +import { useLocation } from '@/shared/hooks/use-location' +import { useTranslation } from 'react-i18next' +import getMeta from '../../../../utils/meta' +import { useAsync } from '@/shared/hooks/use-async-with-cancel' + +export default function UnlinkCommonsSSOModal({ + show, + onClose, + institutionName, + institutionEmail, + hasLicence, +}: { + show: boolean + onClose: () => void + institutionName: string + institutionEmail: string + hasLicence: boolean +}) { + const { t } = useTranslation() + const location = useLocation() + const hasPassword = getMeta('ol-hasPassword') + const { isLoading, isError, runAsync, reset } = useAsync() + + function handleClose() { + reset() + onClose() + } + + function handleUnlinkSSO() { + runAsync(signal => + postJSON('/saml/unlink-commons', { + body: { institutionEmail }, + signal, + }) + ) + .then(() => { + handleClose() + location.reload() + }) + .catch(error => { + debugConsole.error('Error unlinking SSO account', error) + }) + } + + return ( + + + {t('unlink_commons_sso_title')} + + +

{t('unlink_commons_sso_confirm', { institutionName })}

+

{t('unlink_commons_sso_troubleshoot')}

+ {hasLicence &&

{t('unlink_commons_sso_lose_licence')}

} + {!hasPassword &&

{t('unlink_commons_sso_no_login')}

} + {isError && ( + + )} + + + + {t('cancel')} + + + {t('unlink')} + + + + ) +} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 0c43b12a4d..f4b38b1151 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -2767,6 +2767,12 @@ "unlink": "Unlink", "unlink_all_users": "Unlink all users", "unlink_all_users_explanation": "You’re about to remove the SSO login option for all users in your group. If SSO is enabled, this will force users to reauthenticate their Overleaf accounts with your IdP. They’ll receive an email asking them to do this.", + "unlink_commons_sso_confirm": "Are you sure you want to remove your __institutionName__ login?", + "unlink_commons_sso_error": "Unlinking failed. Please try again.", + "unlink_commons_sso_lose_licence": "Your account will lose your institutional premium features. You can rejoin your institutional subscription if you are entitled by adding your institutional login option back to your account.", + "unlink_commons_sso_no_login": "If you do not add the institutional login back, you can still access your work by setting up an Overleaf-specific password using the password reset page.", + "unlink_commons_sso_title": "Unlink institutional login", + "unlink_commons_sso_troubleshoot": "Removing and re-adding your institutional login can often fix login problems.", "unlink_dropbox_folder": "Unlink Dropbox Account", "unlink_dropbox_warning": "Any projects that you have synced with Dropbox will be disconnected and no longer kept in sync with Dropbox. Are you sure you want to unlink your Dropbox account?", "unlink_from_sso": "Unlink from SSO", @@ -2777,6 +2783,7 @@ "unlink_provider_account_title": "Unlink __provider__ Account", "unlink_provider_account_warning": "Warning: When you unlink your account from __provider__ you will not be able to sign in using __provider__ anymore.", "unlink_reference": "Unlink References Provider", + "unlink_sso": "Unlink SSO", "unlink_the_project_from_the_current_github_repo": "Unlink the project from the current GitHub repository and create a connection to a repository you own. (You need an active __appName__ subscription to set up a GitHub Sync).", "unlink_user_explanation": "You’re about to remove the SSO login option for <0>__email__. This will force them to reauthenticate their Overleaf account with your IdP. They’ll receive an email asking them to do this.", "unlink_users": "Unlink users", diff --git a/services/web/test/frontend/features/settings/components/emails/emails-row.test.tsx b/services/web/test/frontend/features/settings/components/emails/emails-row.test.tsx index 3d47a4ddc7..9fcd48bebf 100644 --- a/services/web/test/frontend/features/settings/components/emails/emails-row.test.tsx +++ b/services/web/test/frontend/features/settings/components/emails/emails-row.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react' +import { render, screen, fireEvent } from '@testing-library/react' import { expect } from 'chai' import fetchMock from 'fetch-mock' import { cloneDeep } from 'lodash' @@ -116,6 +116,15 @@ describe('', function () { expect(screen.queryByRole('button', { name: 'Link accounts' })).to.be .null }) + + it('shows unlink button and opens unlink modal', function () { + renderEmailsRow(affiliatedEmail) + + fireEvent.click(screen.getByRole('button', { name: 'Unlink SSO' })) + + screen.getByRole('dialog') + screen.getByText('Unlink institutional login') + }) }) describe('and domain capture is also on for group and Commons SSO also enabled', function () { diff --git a/services/web/test/frontend/features/settings/components/emails/unlink-commons-sso-modal.test.tsx b/services/web/test/frontend/features/settings/components/emails/unlink-commons-sso-modal.test.tsx new file mode 100644 index 0000000000..4e52c3050b --- /dev/null +++ b/services/web/test/frontend/features/settings/components/emails/unlink-commons-sso-modal.test.tsx @@ -0,0 +1,137 @@ +import { expect } from 'chai' +import sinon from 'sinon' +import { + fireEvent, + render, + screen, + waitFor, + within, +} from '@testing-library/react' +import fetchMock from 'fetch-mock' +import UnlinkCommonsSSOModal from '../../../../../../frontend/js/features/settings/components/emails/unlink-commons-sso-modal' +import { location } from '../../../../../../frontend/js/shared/components/location' + +describe('', function () { + beforeEach(function () { + fetchMock.removeRoutes().clearHistory() + this.locationSandbox = sinon.createSandbox() + this.locationStub = this.locationSandbox.stub(location) + window.metaAttributesCache.set('ol-hasPassword', false) + }) + + afterEach(function () { + fetchMock.removeRoutes().clearHistory() + this.locationSandbox.restore() + window.metaAttributesCache.delete('ol-hasPassword') + }) + + function renderModal(props = {}) { + const onClose = sinon.stub() + + render( + + ) + + return { onClose } + } + + it('renders modal content including licence warning', function () { + renderModal() + + screen.getByRole('dialog') + screen.getByText('Unlink institutional login') + screen.getByText(/remove your Overleaf University login/i) + screen.getByText(/Removing and re-adding your institutional login/i) + screen.getByText(/will lose your institutional premium features/i) + screen.getByText(/can still access your work by setting up/i) + }) + + it('does not render licence warning when hasLicence is false', function () { + renderModal({ hasLicence: false }) + + expect(screen.queryByText(/will lose your institutional premium features/i)) + .to.be.null + }) + + it('does not render password reset hint when user already has a password', function () { + window.metaAttributesCache.set('ol-hasPassword', true) + renderModal() + + expect(screen.queryByText(/can still access your work by setting up/i)).to + .be.null + }) + + it('posts to unlink endpoint with correct body, closes modal, and reloads page', async function () { + const { onClose } = renderModal() + fetchMock.post('/saml/unlink-commons', 204) + + const modal = screen.getByRole('dialog') + fireEvent.click(within(modal).getByRole('button', { name: 'Unlink' })) + await fetchMock.callHistory.flush(true) + + expect(fetchMock.callHistory.called('/saml/unlink-commons')).to.be.true + const call = fetchMock.callHistory.calls('/saml/unlink-commons')[0] + expect(JSON.parse(call.options!.body as string)).to.deep.equal({ + institutionEmail: 'user@overleaf.com', + }) + await waitFor(() => { + expect(onClose).to.have.been.calledOnce + expect(this.locationStub.reload).to.have.been.calledOnce + }) + }) + + it('disables buttons and shows loading label while request is in flight', async function () { + renderModal() + // Use a deferred promise so the request stays pending + let resolveRequest!: () => void + fetchMock.post( + '/saml/unlink-commons', + new Promise(resolve => { + resolveRequest = resolve + }).then(() => 204) + ) + + const modal = screen.getByRole('dialog') + const unlinkButton = within(modal).getByRole('button', { name: 'Unlink' }) + const cancelButton = within(modal).getByRole('button', { name: 'Cancel' }) + + fireEvent.click(unlinkButton) + + await screen.findByText('Unlinking') + expect(unlinkButton).to.have.property('disabled', true) + expect(cancelButton).to.have.property('disabled', true) + + resolveRequest() + await fetchMock.callHistory.flush(true) + }) + + it('shows an error notification when unlink fails', async function () { + renderModal() + fetchMock.post('/saml/unlink-commons', 500) + + const modal = screen.getByRole('dialog') + fireEvent.click(within(modal).getByRole('button', { name: 'Unlink' })) + + await screen.findByText('Unlinking failed. Please try again.') + }) + + it('clears error state when modal is closed via Cancel after a failure', async function () { + const { onClose } = renderModal() + fetchMock.post('/saml/unlink-commons', 500) + + const modal = screen.getByRole('dialog') + fireEvent.click(within(modal).getByRole('button', { name: 'Unlink' })) + await screen.findByText('Unlinking failed. Please try again.') + + // Cancel should call handleClose (which calls reset()) not just onClose + fireEvent.click(within(modal).getByRole('button', { name: 'Cancel' })) + expect(onClose).to.have.been.calledOnce + }) +}) diff --git a/services/web/test/unit/src/User/SAMLIdentityManager.test.mjs b/services/web/test/unit/src/User/SAMLIdentityManager.test.mjs index 0409f8f166..127dccedc0 100644 --- a/services/web/test/unit/src/User/SAMLIdentityManager.test.mjs +++ b/services/web/test/unit/src/User/SAMLIdentityManager.test.mjs @@ -566,7 +566,7 @@ describe('SAMLIdentityManager', function () { } ) }) - it('should remove the identifier', async function (ctx) { + it('should remove the identifier with a single update', async function (ctx) { await ctx.SAMLIdentityManager.unlinkAccounts( ctx.user._id, linkedEmail, @@ -575,19 +575,21 @@ describe('SAMLIdentityManager', function () { 'Overleaf University', ctx.auditLog ) - const query = { - _id: ctx.user._id, - } - const update = { - $pull: { - samlIdentifiers: { - providerId: '1', + + expect(ctx.User.updateOne).to.have.been.calledOnce + expect(ctx.User.updateOne.getCall(0)).to.have.been.calledWithMatch( + { _id: ctx.user._id }, + { + $pull: { + samlIdentifiers: { providerId: '1' }, + }, + $unset: { + 'emails.$[email].samlProviderId': 1, }, }, - } - expect(ctx.User.updateOne).to.have.been.calledOnce.and.calledWithMatch( - query, - update + { + arrayFilters: [{ 'email.samlProviderId': '1' }], + } ) }) it('should send an email notification', async function (ctx) {