OIDC: Redirect to delegated auth provider when signing out (#11432)

* util for account url

* test cases

* disable multi session selection on device list

* remove sign out all from context menus when oidc-aware

* comment

* remove unused param

* redirect to auth provider when signing out

* open auth provider in new tab, refresh sessions on return

* correct comment

* fix bad copy paste

* try to make sonar happy

* Update for latest revision of MSCs

* Update SessionManagerTab-test.tsx

* Make InteractiveAuthCallback async and await it

---------

Co-authored-by: Hugh Nimmo-Smith <hughns@matrix.org>
Co-authored-by: Hugh Nimmo-Smith <hughns@users.noreply.github.com>
Co-authored-by: Andy Balaam <andy.balaam@matrix.org>
This commit is contained in:
Kerry 2023-08-22 23:15:35 +12:00 committed by GitHub
parent 5c1b62cf99
commit 23196d49e1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 199 additions and 44 deletions

View file

@ -35,8 +35,8 @@ type InteractiveAuthCallbackSuccess<T> = (
success: true, success: true,
response: T, response: T,
extra?: { emailSid?: string; clientSecret?: string }, extra?: { emailSid?: string; clientSecret?: string },
) => void; ) => Promise<void>;
type InteractiveAuthCallbackFailure = (success: false, response: IAuthData | Error) => void; type InteractiveAuthCallbackFailure = (success: false, response: IAuthData | Error) => Promise<void>;
export type InteractiveAuthCallback<T> = InteractiveAuthCallbackSuccess<T> & InteractiveAuthCallbackFailure; export type InteractiveAuthCallback<T> = InteractiveAuthCallbackSuccess<T> & InteractiveAuthCallbackFailure;
export interface InteractiveAuthProps<T> { export interface InteractiveAuthProps<T> {
@ -141,15 +141,15 @@ export default class InteractiveAuthComponent<T> extends React.Component<Interac
public componentDidMount(): void { public componentDidMount(): void {
this.authLogic this.authLogic
.attemptAuth() .attemptAuth()
.then((result) => { .then(async (result) => {
const extra = { const extra = {
emailSid: this.authLogic.getEmailSid(), emailSid: this.authLogic.getEmailSid(),
clientSecret: this.authLogic.getClientSecret(), clientSecret: this.authLogic.getClientSecret(),
}; };
this.props.onAuthFinished(true, result, extra); await this.props.onAuthFinished(true, result, extra);
}) })
.catch((error) => { .catch(async (error) => {
this.props.onAuthFinished(false, error); await this.props.onAuthFinished(false, error);
logger.error("Error during user-interactive auth:", error); logger.error("Error during user-interactive auth:", error);
if (this.unmounted) { if (this.unmounted) {
return; return;
@ -251,12 +251,12 @@ export default class InteractiveAuthComponent<T> extends React.Component<Interac
this.props.onStagePhaseChange?.(this.state.authStage ?? null, newPhase || 0); this.props.onStagePhaseChange?.(this.state.authStage ?? null, newPhase || 0);
}; };
private onStageCancel = (): void => { private onStageCancel = async (): Promise<void> => {
this.props.onAuthFinished(false, ERROR_USER_CANCELLED); await this.props.onAuthFinished(false, ERROR_USER_CANCELLED);
}; };
private onAuthStageFailed = (e: Error): void => { private onAuthStageFailed = async (e: Error): Promise<void> => {
this.props.onAuthFinished(false, e); await this.props.onAuthFinished(false, e);
}; };
private setEmailSid = (sid: string): void => { private setEmailSid = (sid: string): void => {

View file

@ -110,7 +110,7 @@ export default class DeactivateAccountDialog extends React.Component<IProps, ISt
this.setState({ bodyText, continueText, continueKind }); this.setState({ bodyText, continueText, continueKind });
}; };
private onUIAuthFinished: InteractiveAuthCallback<Awaited<ReturnType<MatrixClient["deactivateAccount"]>>> = ( private onUIAuthFinished: InteractiveAuthCallback<Awaited<ReturnType<MatrixClient["deactivateAccount"]>>> = async (
success, success,
result, result,
) => { ) => {

View file

@ -116,7 +116,7 @@ export default class InteractiveAuthDialog<T> extends React.Component<Interactiv
}; };
} }
private onAuthFinished: InteractiveAuthCallback<T> = (success, result): void => { private onAuthFinished: InteractiveAuthCallback<T> = async (success, result): Promise<void> => {
if (success) { if (success) {
this.props.onFinished(true, result); this.props.onFinished(true, result);
} else { } else {

View file

@ -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<OidcLogoutDialogProps> = ({
delegatedAuthAccountUrl,
deviceId,
onFinished,
}) => {
const [hasOpenedLogoutLink, setHasOpenedLogoutLink] = useState(false);
const logoutUrl = getOidcLogoutUrl(delegatedAuthAccountUrl, deviceId);
return (
<BaseDialog onFinished={onFinished} title={_t("Sign out")} contentId="mx_Dialog_content">
<div className="mx_Dialog_content" id="mx_Dialog_content">
{_t("You will be redirected to your server's authentication provider to complete sign out.")}
</div>
<div className="mx_Dialog_buttons">
{hasOpenedLogoutLink ? (
<AccessibleButton kind="primary" onClick={() => onFinished(true)}>
{_t("Close")}
</AccessibleButton>
) : (
<>
<AccessibleButton kind="secondary" onClick={() => onFinished(false)}>
{_t("Cancel")}
</AccessibleButton>
<AccessibleButton
element="a"
onClick={() => setHasOpenedLogoutLink(true)}
kind="primary"
href={logoutUrl}
target="_blank"
>
{_t("Continue")}
</AccessibleButton>
</>
)}
</div>
</BaseDialog>
);
};

View file

@ -40,7 +40,7 @@ export const deleteDevicesWithInteractiveAuth = async (
try { try {
await makeDeleteRequest(matrixClient, deviceIds)(null); await makeDeleteRequest(matrixClient, deviceIds)(null);
// no interactive auth needed // no interactive auth needed
onFinished(true, undefined); await onFinished(true, undefined);
} catch (error) { } catch (error) {
if (!(error instanceof MatrixError) || error.httpStatus !== 401 || !error.data?.flows) { if (!(error instanceof MatrixError) || error.httpStatus !== 401 || !error.data?.flows) {
// doesn't look like an interactive-auth failure // doesn't look like an interactive-auth failure

View file

@ -40,6 +40,7 @@ import { FilterVariation } from "../../devices/filter";
import { OtherSessionsSectionHeading } from "../../devices/OtherSessionsSectionHeading"; import { OtherSessionsSectionHeading } from "../../devices/OtherSessionsSectionHeading";
import { SettingsSection } from "../../shared/SettingsSection"; import { SettingsSection } from "../../shared/SettingsSection";
import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl"; import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl";
import { OidcLogoutDialog } from "../../../dialogs/oidc/OidcLogoutDialog";
const confirmSignOut = async (sessionsToSignOutCount: number): Promise<boolean> => { const confirmSignOut = async (sessionsToSignOutCount: number): Promise<boolean> => {
const { finished } = Modal.createDialog(QuestionDialog, { const { finished } = Modal.createDialog(QuestionDialog, {
@ -61,9 +62,20 @@ const confirmSignOut = async (sessionsToSignOutCount: number): Promise<boolean>
return !!confirmed; return !!confirmed;
}; };
const confirmDelegatedAuthSignOut = async (delegatedAuthAccountUrl: string, deviceId: string): Promise<boolean> => {
const { finished } = Modal.createDialog(OidcLogoutDialog, {
deviceId,
delegatedAuthAccountUrl,
});
const [confirmed] = await finished;
return !!confirmed;
};
const useSignOut = ( const useSignOut = (
matrixClient: MatrixClient, matrixClient: MatrixClient,
onSignoutResolvedCallback: () => Promise<void>, onSignoutResolvedCallback: () => Promise<void>,
delegatedAuthAccountUrl?: string,
): { ): {
onSignOutCurrentDevice: () => void; onSignOutCurrentDevice: () => void;
onSignOutOtherDevices: (deviceIds: ExtendedDevice["device_id"][]) => Promise<void>; onSignOutOtherDevices: (deviceIds: ExtendedDevice["device_id"][]) => Promise<void>;
@ -85,19 +97,44 @@ const useSignOut = (
if (!deviceIds.length) { if (!deviceIds.length) {
return; return;
} }
// 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); const userConfirmedSignout = await confirmSignOut(deviceIds.length);
if (!userConfirmedSignout) { if (!userConfirmedSignout) {
return; return;
} }
}
try { try {
setSigningOutDeviceIds([...signingOutDeviceIds, ...deviceIds]); setSigningOutDeviceIds([...signingOutDeviceIds, ...deviceIds]);
await deleteDevicesWithInteractiveAuth(matrixClient, deviceIds, async (success): Promise<void> => {
const onSignOutFinished = async (success: boolean): Promise<void> => {
if (success) { if (success) {
await onSignoutResolvedCallback(); await onSignoutResolvedCallback();
} }
setSigningOutDeviceIds(signingOutDeviceIds.filter((deviceId) => !deviceIds.includes(deviceId))); 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) { } catch (error) {
logger.error("Error deleting sessions", error); logger.error("Error deleting sessions", error);
setSigningOutDeviceIds(signingOutDeviceIds.filter((deviceId) => !deviceIds.includes(deviceId))); setSigningOutDeviceIds(signingOutDeviceIds.filter((deviceId) => !deviceIds.includes(deviceId)));
@ -200,6 +237,7 @@ const SessionManagerTab: React.FC = () => {
const { onSignOutCurrentDevice, onSignOutOtherDevices, signingOutDeviceIds } = useSignOut( const { onSignOutCurrentDevice, onSignOutOtherDevices, signingOutDeviceIds } = useSignOut(
matrixClient, matrixClient,
onSignoutResolvedCallback, onSignoutResolvedCallback,
delegatedAuthAccountUrl,
); );
useEffect( useEffect(

View file

@ -3285,6 +3285,7 @@
"Not a valid Security Key": "Not a valid Security Key", "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.", "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 <button>set up new recovery options</button>": "If you've forgotten your Security Key you can <button>set up new recovery options</button>", "If you've forgotten your Security Key you can <button>set up new recovery options</button>": "If you've forgotten your Security Key you can <button>set up new recovery options</button>",
"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 account data event": "Send custom account data event",
"Send custom room account data event": "Send custom room account data event", "Send custom room account data event": "Send custom room account data event",
"Event Type": "Event Type", "Event Type": "Event Type",

View file

@ -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();
};

View file

@ -55,6 +55,9 @@ import { getClientInformationEventType } from "../../../../../../src/utils/devic
mockPlatformPeg(); mockPlatformPeg();
// to restore later
const realWindowLocation = window.location;
describe("<SessionManagerTab />", () => { describe("<SessionManagerTab />", () => {
const aliceId = "@alice:server.org"; const aliceId = "@alice:server.org";
const deviceId = "alices_device"; const deviceId = "alices_device";
@ -166,7 +169,7 @@ describe("<SessionManagerTab />", () => {
confirm = true, confirm = true,
): Promise<void> => { ): Promise<void> => {
// modal has sleeps in rendering process :( // modal has sleeps in rendering process :(
await sleep(100); await screen.findByRole("dialog");
const buttonId = confirm ? "dialog-primary-button" : "dialog-cancel-button"; const buttonId = confirm ? "dialog-primary-button" : "dialog-cancel-button";
fireEvent.click(getByTestId(buttonId)); fireEvent.click(getByTestId(buttonId));
@ -227,11 +230,23 @@ describe("<SessionManagerTab />", () => {
} }
}); });
// @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 // sometimes a verification modal is in modal state when these tests run
// make sure the coast is clear // make sure the coast is clear
await clearAllModals(); await clearAllModals();
}); });
afterAll(() => {
window.location = realWindowLocation;
});
it("renders spinner while devices load", () => { it("renders spinner while devices load", () => {
const { container } = render(getComponent()); const { container } = render(getComponent());
expect(container.getElementsByClassName("mx_Spinner").length).toBeTruthy(); expect(container.getElementsByClassName("mx_Spinner").length).toBeTruthy();
@ -858,7 +873,7 @@ describe("<SessionManagerTab />", () => {
await flushPromises(); await flushPromises();
// modal rendering has some weird sleeps // modal rendering has some weird sleeps
await sleep(100); await screen.findByRole("dialog");
expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith( expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith(
[alicesMobileDevice.device_id], [alicesMobileDevice.device_id],
@ -1082,18 +1097,9 @@ describe("<SessionManagerTab />", () => {
}); });
describe("other devices", () => { describe("other devices", () => {
// signing out a single device still works it("opens delegated auth provider to sign out a single device", async () => {
// this test will be updated once redirect to MAS is added mockClient.getDevices.mockResolvedValue({
// 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], devices: [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice],
})
// pretend it was really deleted on refresh
.mockResolvedValueOnce({
devices: [alicesDevice, alicesOlderMobileDevice],
}); });
const { getByTestId } = render(getComponent()); const { getByTestId } = render(getComponent());
@ -1102,6 +1108,9 @@ describe("<SessionManagerTab />", () => {
await flushPromises(); await flushPromises();
}); });
// reset call count
mockClient.getDevices.mockClear();
toggleDeviceDetails(getByTestId, alicesMobileDevice.device_id); toggleDeviceDetails(getByTestId, alicesMobileDevice.device_id);
const deviceDetails = getByTestId(`device-detail-${alicesMobileDevice.device_id}`); const deviceDetails = getByTestId(`device-detail-${alicesMobileDevice.device_id}`);
@ -1110,23 +1119,28 @@ describe("<SessionManagerTab />", () => {
) as Element; ) as Element;
fireEvent.click(signOutButton); fireEvent.click(signOutButton);
await confirmSignout(getByTestId); await screen.findByRole("dialog");
// sign out button is disabled with spinner
expect( expect(
( screen.getByText(
deviceDetails.querySelector('[data-testid="device-detail-sign-out-cta"]') as Element "You will be redirected to your server's authentication provider to complete sign out.",
).getAttribute("aria-disabled"), ),
).toEqual("true"); ).toBeInTheDocument();
// delete called // correct link to auth provider
expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith( expect(screen.getByText("Continue")).toHaveAttribute(
[alicesMobileDevice.device_id], "href",
undefined, `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(); await flushPromises();
// devices refreshed // devices were refreshed
expect(mockClient.getDevices).toHaveBeenCalled(); expect(mockClient.getDevices).toHaveBeenCalled();
}); });