diff --git a/src/components/structures/InteractiveAuth.tsx b/src/components/structures/InteractiveAuth.tsx index 67c765760d..d35d403e5f 100644 --- a/src/components/structures/InteractiveAuth.tsx +++ b/src/components/structures/InteractiveAuth.tsx @@ -35,8 +35,8 @@ type InteractiveAuthCallbackSuccess = ( success: true, response: T, extra?: { emailSid?: string; clientSecret?: string }, -) => void; -type InteractiveAuthCallbackFailure = (success: false, response: IAuthData | Error) => void; +) => Promise; +type InteractiveAuthCallbackFailure = (success: false, response: IAuthData | Error) => Promise; export type InteractiveAuthCallback = InteractiveAuthCallbackSuccess & InteractiveAuthCallbackFailure; export interface InteractiveAuthProps { @@ -141,15 +141,15 @@ export default class InteractiveAuthComponent extends React.Component { + .then(async (result) => { const extra = { emailSid: this.authLogic.getEmailSid(), clientSecret: this.authLogic.getClientSecret(), }; - this.props.onAuthFinished(true, result, extra); + await this.props.onAuthFinished(true, result, extra); }) - .catch((error) => { - this.props.onAuthFinished(false, error); + .catch(async (error) => { + await this.props.onAuthFinished(false, error); logger.error("Error during user-interactive auth:", error); if (this.unmounted) { return; @@ -251,12 +251,12 @@ export default class InteractiveAuthComponent extends React.Component { - this.props.onAuthFinished(false, ERROR_USER_CANCELLED); + private onStageCancel = async (): Promise => { + await this.props.onAuthFinished(false, ERROR_USER_CANCELLED); }; - private onAuthStageFailed = (e: Error): void => { - this.props.onAuthFinished(false, e); + private onAuthStageFailed = async (e: Error): Promise => { + await this.props.onAuthFinished(false, e); }; private setEmailSid = (sid: string): void => { diff --git a/src/components/views/dialogs/DeactivateAccountDialog.tsx b/src/components/views/dialogs/DeactivateAccountDialog.tsx index c53ff765f0..aa92869165 100644 --- a/src/components/views/dialogs/DeactivateAccountDialog.tsx +++ b/src/components/views/dialogs/DeactivateAccountDialog.tsx @@ -110,7 +110,7 @@ export default class DeactivateAccountDialog extends React.Component>> = ( + private onUIAuthFinished: InteractiveAuthCallback>> = async ( success, result, ) => { diff --git a/src/components/views/dialogs/InteractiveAuthDialog.tsx b/src/components/views/dialogs/InteractiveAuthDialog.tsx index 807ca383f0..4faa674637 100644 --- a/src/components/views/dialogs/InteractiveAuthDialog.tsx +++ b/src/components/views/dialogs/InteractiveAuthDialog.tsx @@ -116,7 +116,7 @@ export default class InteractiveAuthDialog extends React.Component = (success, result): void => { + private onAuthFinished: InteractiveAuthCallback = async (success, result): Promise => { if (success) { this.props.onFinished(true, result); } else { diff --git a/src/components/views/dialogs/oidc/OidcLogoutDialog.tsx b/src/components/views/dialogs/oidc/OidcLogoutDialog.tsx new file mode 100644 index 0000000000..856b0089d8 --- /dev/null +++ b/src/components/views/dialogs/oidc/OidcLogoutDialog.tsx @@ -0,0 +1,74 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React, { useState } from "react"; + +import { _t } from "../../../../languageHandler"; +import BaseDialog from "../BaseDialog"; +import { getOidcLogoutUrl } from "../../../../utils/oidc/getOidcLogoutUrl"; +import AccessibleButton from "../../elements/AccessibleButton"; + +export interface OidcLogoutDialogProps { + delegatedAuthAccountUrl: string; + deviceId: string; + onFinished(ok?: boolean): void; +} + +/** + * Handle logout of OIDC sessions other than the current session + * - ask for user confirmation to open the delegated auth provider + * - open the auth provider in a new tab + * - wait for the user to return and close the modal, we assume the user has completed sign out of the session in auth provider UI + * and trigger a refresh of the session list + */ +export const OidcLogoutDialog: React.FC = ({ + delegatedAuthAccountUrl, + deviceId, + onFinished, +}) => { + const [hasOpenedLogoutLink, setHasOpenedLogoutLink] = useState(false); + const logoutUrl = getOidcLogoutUrl(delegatedAuthAccountUrl, deviceId); + + return ( + +
+ {_t("You will be redirected to your server's authentication provider to complete sign out.")} +
+
+ {hasOpenedLogoutLink ? ( + onFinished(true)}> + {_t("Close")} + + ) : ( + <> + onFinished(false)}> + {_t("Cancel")} + + setHasOpenedLogoutLink(true)} + kind="primary" + href={logoutUrl} + target="_blank" + > + {_t("Continue")} + + + )} +
+ + ); +}; diff --git a/src/components/views/settings/devices/deleteDevices.tsx b/src/components/views/settings/devices/deleteDevices.tsx index b1f527187d..7d38448bda 100644 --- a/src/components/views/settings/devices/deleteDevices.tsx +++ b/src/components/views/settings/devices/deleteDevices.tsx @@ -40,7 +40,7 @@ export const deleteDevicesWithInteractiveAuth = async ( try { await makeDeleteRequest(matrixClient, deviceIds)(null); // no interactive auth needed - onFinished(true, undefined); + await onFinished(true, undefined); } catch (error) { if (!(error instanceof MatrixError) || error.httpStatus !== 401 || !error.data?.flows) { // doesn't look like an interactive-auth failure diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx index 8041f6b8a5..ff20814f6c 100644 --- a/src/components/views/settings/tabs/user/SessionManagerTab.tsx +++ b/src/components/views/settings/tabs/user/SessionManagerTab.tsx @@ -40,6 +40,7 @@ import { FilterVariation } from "../../devices/filter"; import { OtherSessionsSectionHeading } from "../../devices/OtherSessionsSectionHeading"; import { SettingsSection } from "../../shared/SettingsSection"; import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl"; +import { OidcLogoutDialog } from "../../../dialogs/oidc/OidcLogoutDialog"; const confirmSignOut = async (sessionsToSignOutCount: number): Promise => { const { finished } = Modal.createDialog(QuestionDialog, { @@ -61,9 +62,20 @@ const confirmSignOut = async (sessionsToSignOutCount: number): Promise return !!confirmed; }; +const confirmDelegatedAuthSignOut = async (delegatedAuthAccountUrl: string, deviceId: string): Promise => { + const { finished } = Modal.createDialog(OidcLogoutDialog, { + deviceId, + delegatedAuthAccountUrl, + }); + const [confirmed] = await finished; + + return !!confirmed; +}; + const useSignOut = ( matrixClient: MatrixClient, onSignoutResolvedCallback: () => Promise, + delegatedAuthAccountUrl?: string, ): { onSignOutCurrentDevice: () => void; onSignOutOtherDevices: (deviceIds: ExtendedDevice["device_id"][]) => Promise; @@ -85,19 +97,44 @@ const useSignOut = ( if (!deviceIds.length) { return; } - const userConfirmedSignout = await confirmSignOut(deviceIds.length); - if (!userConfirmedSignout) { + // we can only sign out exactly one OIDC-aware device at a time + // we should not encounter this + if (delegatedAuthAccountUrl && deviceIds.length !== 1) { + logger.warn("Unexpectedly tried to sign out multiple OIDC-aware devices."); return; } + // delegated auth logout flow confirms and signs out together + // so only confirm if we are NOT doing a delegated auth sign out + if (!delegatedAuthAccountUrl) { + const userConfirmedSignout = await confirmSignOut(deviceIds.length); + if (!userConfirmedSignout) { + return; + } + } + try { setSigningOutDeviceIds([...signingOutDeviceIds, ...deviceIds]); - await deleteDevicesWithInteractiveAuth(matrixClient, deviceIds, async (success): Promise => { + + const onSignOutFinished = async (success: boolean): Promise => { if (success) { await onSignoutResolvedCallback(); } setSigningOutDeviceIds(signingOutDeviceIds.filter((deviceId) => !deviceIds.includes(deviceId))); - }); + }; + + if (delegatedAuthAccountUrl) { + const [deviceId] = deviceIds; + try { + setSigningOutDeviceIds([...signingOutDeviceIds, deviceId]); + const success = await confirmDelegatedAuthSignOut(delegatedAuthAccountUrl, deviceId); + await onSignOutFinished(success); + } catch (error) { + logger.error("Error deleting OIDC-aware sessions", error); + } + } else { + await deleteDevicesWithInteractiveAuth(matrixClient, deviceIds, onSignOutFinished); + } } catch (error) { logger.error("Error deleting sessions", error); setSigningOutDeviceIds(signingOutDeviceIds.filter((deviceId) => !deviceIds.includes(deviceId))); @@ -200,6 +237,7 @@ const SessionManagerTab: React.FC = () => { const { onSignOutCurrentDevice, onSignOutOtherDevices, signingOutDeviceIds } = useSignOut( matrixClient, onSignoutResolvedCallback, + delegatedAuthAccountUrl, ); useEffect( diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 170feb0e64..15f9e4c35c 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3285,6 +3285,7 @@ "Not a valid Security Key": "Not a valid Security Key", "Access your secure message history and set up secure messaging by entering your Security Key.": "Access your secure message history and set up secure messaging by entering your Security Key.", "If you've forgotten your Security Key you can ": "If you've forgotten your Security Key you can ", + "You will be redirected to your server's authentication provider to complete sign out.": "You will be redirected to your server's authentication provider to complete sign out.", "Send custom account data event": "Send custom account data event", "Send custom room account data event": "Send custom room account data event", "Event Type": "Event Type", diff --git a/src/utils/oidc/getOidcLogoutUrl.ts b/src/utils/oidc/getOidcLogoutUrl.ts new file mode 100644 index 0000000000..a18e5b2b6c --- /dev/null +++ b/src/utils/oidc/getOidcLogoutUrl.ts @@ -0,0 +1,28 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/** + * Create a delegated auth account management URL with logout params as per MSC3824 and MSC2965 + * https://github.com/matrix-org/matrix-spec-proposals/blob/hughns/sso-redirect-action/proposals/3824-oidc-aware-clients.md#definition-of-oidc-aware + * https://github.com/sandhose/matrix-doc/blob/msc/sandhose/oidc-discovery/proposals/2965-oidc-discovery.md#account-management-url-parameters + */ +export const getOidcLogoutUrl = (delegatedAuthAccountUrl: string, deviceId: string): string => { + const logoutUrl = new URL(delegatedAuthAccountUrl); + logoutUrl.searchParams.set("action", "session_end"); + logoutUrl.searchParams.set("device_id", deviceId); + + return logoutUrl.toString(); +}; diff --git a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx index 45fc665c6b..1c5c5c7c9a 100644 --- a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx +++ b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx @@ -55,6 +55,9 @@ import { getClientInformationEventType } from "../../../../../../src/utils/devic mockPlatformPeg(); +// to restore later +const realWindowLocation = window.location; + describe("", () => { const aliceId = "@alice:server.org"; const deviceId = "alices_device"; @@ -166,7 +169,7 @@ describe("", () => { confirm = true, ): Promise => { // modal has sleeps in rendering process :( - await sleep(100); + await screen.findByRole("dialog"); const buttonId = confirm ? "dialog-primary-button" : "dialog-cancel-button"; fireEvent.click(getByTestId(buttonId)); @@ -227,11 +230,23 @@ describe("", () => { } }); + // @ts-ignore allow delete of non-optional prop + delete window.location; + // @ts-ignore ugly mocking + window.location = { + href: "https://localhost/", + origin: "https://localhost/", + }; + // sometimes a verification modal is in modal state when these tests run // make sure the coast is clear await clearAllModals(); }); + afterAll(() => { + window.location = realWindowLocation; + }); + it("renders spinner while devices load", () => { const { container } = render(getComponent()); expect(container.getElementsByClassName("mx_Spinner").length).toBeTruthy(); @@ -858,7 +873,7 @@ describe("", () => { await flushPromises(); // modal rendering has some weird sleeps - await sleep(100); + await screen.findByRole("dialog"); expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith( [alicesMobileDevice.device_id], @@ -1082,19 +1097,10 @@ describe("", () => { }); describe("other devices", () => { - // signing out a single device still works - // this test will be updated once redirect to MAS is added - // https://github.com/vector-im/element-web/issues/26000 - it("deletes a device when interactive auth is not required", async () => { - mockClient.deleteMultipleDevices.mockResolvedValue({}); - mockClient.getDevices - .mockResolvedValueOnce({ - devices: [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice], - }) - // pretend it was really deleted on refresh - .mockResolvedValueOnce({ - devices: [alicesDevice, alicesOlderMobileDevice], - }); + it("opens delegated auth provider to sign out a single device", async () => { + mockClient.getDevices.mockResolvedValue({ + devices: [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice], + }); const { getByTestId } = render(getComponent()); @@ -1102,6 +1108,9 @@ describe("", () => { await flushPromises(); }); + // reset call count + mockClient.getDevices.mockClear(); + toggleDeviceDetails(getByTestId, alicesMobileDevice.device_id); const deviceDetails = getByTestId(`device-detail-${alicesMobileDevice.device_id}`); @@ -1110,23 +1119,28 @@ describe("", () => { ) as Element; fireEvent.click(signOutButton); - await confirmSignout(getByTestId); - - // sign out button is disabled with spinner + await screen.findByRole("dialog"); expect( - ( - deviceDetails.querySelector('[data-testid="device-detail-sign-out-cta"]') as Element - ).getAttribute("aria-disabled"), - ).toEqual("true"); - // delete called - expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith( - [alicesMobileDevice.device_id], - undefined, + screen.getByText( + "You will be redirected to your server's authentication provider to complete sign out.", + ), + ).toBeInTheDocument(); + // correct link to auth provider + expect(screen.getByText("Continue")).toHaveAttribute( + "href", + `https://issuer.org/account?action=session_end&device_id=${alicesMobileDevice.device_id}`, ); + // go to the link + fireEvent.click(screen.getByText("Continue")); + await flushPromises(); + + // come back from the link and close the modal + fireEvent.click(screen.getByText("Close")); + await flushPromises(); - // devices refreshed + // devices were refreshed expect(mockClient.getDevices).toHaveBeenCalled(); });