OIDC: disable multi session signout for OIDC-aware servers in session manager (#11431)

* 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

* typo
This commit is contained in:
Kerry 2023-08-22 14:25:34 +12:00 committed by GitHub
parent 3c52ba0c92
commit dfded8d4d3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 362 additions and 61 deletions

View file

@ -31,6 +31,7 @@ import { DevicesState } from "./useOwnDevices";
import FilteredDeviceListHeader from "./FilteredDeviceListHeader";
import Spinner from "../../elements/Spinner";
import { DeviceSecurityLearnMore } from "./DeviceSecurityLearnMore";
import DeviceTile from "./DeviceTile";
interface Props {
devices: DevicesDictionary;
@ -48,6 +49,11 @@ interface Props {
setPushNotifications: (deviceId: string, enabled: boolean) => Promise<void>;
setSelectedDeviceIds: (deviceIds: ExtendedDevice["device_id"][]) => void;
supportsMSC3881?: boolean | undefined;
/**
* Only allow sessions to be signed out individually
* Removes checkboxes and multi selection header
*/
disableMultipleSignout?: boolean;
}
const isDeviceSelected = (
@ -178,6 +184,7 @@ const DeviceListItem: React.FC<{
toggleSelected: () => void;
setPushNotifications: (deviceId: string, enabled: boolean) => Promise<void>;
supportsMSC3881?: boolean | undefined;
isSelectDisabled?: boolean;
}> = ({
device,
pusher,
@ -192,17 +199,30 @@ const DeviceListItem: React.FC<{
setPushNotifications,
toggleSelected,
supportsMSC3881,
}) => (
isSelectDisabled,
}) => {
const tileContent = (
<>
{isSigningOut && <Spinner w={16} h={16} />}
<DeviceExpandDetailsButton isExpanded={isExpanded} onClick={onDeviceExpandToggle} />
</>
);
return (
<li className="mx_FilteredDeviceList_listItem">
{isSelectDisabled ? (
<DeviceTile device={device} onClick={onDeviceExpandToggle}>
{tileContent}
</DeviceTile>
) : (
<SelectableDeviceTile
isSelected={isSelected}
onSelect={toggleSelected}
onClick={onDeviceExpandToggle}
device={device}
>
{isSigningOut && <Spinner w={16} h={16} />}
<DeviceExpandDetailsButton isExpanded={isExpanded} onClick={onDeviceExpandToggle} />
{tileContent}
</SelectableDeviceTile>
)}
{isExpanded && (
<DeviceDetails
device={device}
@ -219,6 +239,7 @@ const DeviceListItem: React.FC<{
)}
</li>
);
};
/**
* Filtered list of devices
@ -242,6 +263,7 @@ export const FilteredDeviceList = forwardRef(
setPushNotifications,
setSelectedDeviceIds,
supportsMSC3881,
disableMultipleSignout,
}: Props,
ref: ForwardedRef<HTMLDivElement>,
) => {
@ -302,6 +324,7 @@ export const FilteredDeviceList = forwardRef(
selectedDeviceCount={selectedDeviceIds.length}
isAllSelected={isAllSelected}
toggleSelectAll={toggleSelectAll}
isSelectDisabled={disableMultipleSignout}
>
{selectedDeviceIds.length ? (
<>
@ -351,6 +374,7 @@ export const FilteredDeviceList = forwardRef(
isExpanded={expandedDeviceIds.includes(device.device_id)}
isSigningOut={signingOutDeviceIds.includes(device.device_id)}
isSelected={isDeviceSelected(device.device_id, selectedDeviceIds)}
isSelectDisabled={disableMultipleSignout}
onDeviceExpandToggle={() => onDeviceExpandToggle(device.device_id)}
onSignOutDevice={() => onSignOutDevices([device.device_id])}
saveDeviceName={(deviceName: string) => saveDeviceName(device.device_id, deviceName)}

View file

@ -24,6 +24,7 @@ import TooltipTarget from "../../elements/TooltipTarget";
interface Props extends Omit<HTMLProps<HTMLDivElement>, "className"> {
selectedDeviceCount: number;
isAllSelected: boolean;
isSelectDisabled?: boolean;
toggleSelectAll: () => void;
children?: React.ReactNode;
}
@ -31,6 +32,7 @@ interface Props extends Omit<HTMLProps<HTMLDivElement>, "className"> {
const FilteredDeviceListHeader: React.FC<Props> = ({
selectedDeviceCount,
isAllSelected,
isSelectDisabled,
toggleSelectAll,
children,
...rest
@ -38,6 +40,7 @@ const FilteredDeviceListHeader: React.FC<Props> = ({
const checkboxLabel = isAllSelected ? _t("Deselect all") : _t("Select all");
return (
<div className="mx_FilteredDeviceListHeader" {...rest}>
{!isSelectDisabled && (
<TooltipTarget label={checkboxLabel} alignment={Alignment.Top}>
<StyledCheckbox
kind={CheckboxStyle.Solid}
@ -48,6 +51,7 @@ const FilteredDeviceListHeader: React.FC<Props> = ({
aria-label={checkboxLabel}
/>
</TooltipTarget>
)}
<span className="mx_FilteredDeviceListHeader_label">
{selectedDeviceCount > 0
? _t("%(count)s sessions selected", { count: selectedDeviceCount })

View file

@ -20,6 +20,7 @@ import { _t } from "../../../../languageHandler";
import { KebabContextMenu } from "../../context_menus/KebabContextMenu";
import { SettingsSubsectionHeading } from "../shared/SettingsSubsectionHeading";
import { IconizedContextMenuOption } from "../../context_menus/IconizedContextMenu";
import { filterBoolean } from "../../../../utils/arrays";
interface Props {
// total count of other sessions
@ -27,7 +28,8 @@ interface Props {
// not affected by filters
otherSessionsCount: number;
disabled?: boolean;
signOutAllOtherSessions: () => void;
// not provided when sign out all other sessions is not available
signOutAllOtherSessions?: () => void;
}
export const OtherSessionsSectionHeading: React.FC<Props> = ({
@ -35,22 +37,26 @@ export const OtherSessionsSectionHeading: React.FC<Props> = ({
disabled,
signOutAllOtherSessions,
}) => {
const menuOptions = [
const menuOptions = filterBoolean([
signOutAllOtherSessions ? (
<IconizedContextMenuOption
key="sign-out-all-others"
label={_t("Sign out of %(count)s sessions", { count: otherSessionsCount })}
onClick={signOutAllOtherSessions}
isDestructive
/>,
];
/>
) : null,
]);
return (
<SettingsSubsectionHeading heading={_t("Other sessions")}>
{!!menuOptions.length && (
<KebabContextMenu
disabled={disabled}
title={_t("Options")}
options={menuOptions}
data-testid="other-sessions-menu"
/>
)}
</SettingsSubsectionHeading>
);
};

View file

@ -17,7 +17,7 @@ limitations under the License.
*/
import React, { ReactNode } from "react";
import { SERVICE_TYPES, IDelegatedAuthConfig, M_AUTHENTICATION, HTTPError } from "matrix-js-sdk/src/matrix";
import { SERVICE_TYPES, HTTPError } from "matrix-js-sdk/src/matrix";
import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids";
import { logger } from "matrix-js-sdk/src/logger";
@ -59,6 +59,7 @@ import Heading from "../../../typography/Heading";
import InlineSpinner from "../../../elements/InlineSpinner";
import MatrixClientContext from "../../../../../contexts/MatrixClientContext";
import { ThirdPartyIdentifier } from "../../../../../AddThreepid";
import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl";
interface IProps {
closeSettingsFn: () => void;
@ -172,8 +173,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
// the enabled flag value.
const canChangePassword = !changePasswordCap || changePasswordCap["enabled"] !== false;
const delegatedAuthConfig = M_AUTHENTICATION.findIn<IDelegatedAuthConfig | undefined>(cli.getClientWellKnown());
const externalAccountManagementUrl = delegatedAuthConfig?.account;
const externalAccountManagementUrl = getDelegatedAuthAccountUrl(cli.getClientWellKnown());
this.setState({ canChangePassword, externalAccountManagementUrl });
}

View file

@ -39,6 +39,7 @@ import QuestionDialog from "../../../dialogs/QuestionDialog";
import { FilterVariation } from "../../devices/filter";
import { OtherSessionsSectionHeading } from "../../devices/OtherSessionsSectionHeading";
import { SettingsSection } from "../../shared/SettingsSection";
import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl";
const confirmSignOut = async (sessionsToSignOutCount: number): Promise<boolean> => {
const { finished } = Modal.createDialog(QuestionDialog, {
@ -130,6 +131,14 @@ const SessionManagerTab: React.FC = () => {
const scrollIntoViewTimeoutRef = useRef<number>();
const matrixClient = useContext(MatrixClientContext);
/**
* If we have a delegated auth account management URL, all sessions but the current session need to be managed in the
* delegated auth provider.
* See https://github.com/matrix-org/matrix-spec-proposals/pull/3824
*/
const delegatedAuthAccountUrl = getDelegatedAuthAccountUrl(matrixClient.getClientWellKnown());
const disableMultipleSignout = !!delegatedAuthAccountUrl;
const userId = matrixClient?.getUserId();
const currentUserMember = (userId && matrixClient?.getUser(userId)) || undefined;
const clientVersions = useAsyncMemo(() => matrixClient.getVersions(), [matrixClient]);
@ -205,7 +214,8 @@ const SessionManagerTab: React.FC = () => {
setSelectedDeviceIds([]);
}, [filter, setSelectedDeviceIds]);
const signOutAllOtherSessions = shouldShowOtherSessions
const signOutAllOtherSessions =
shouldShowOtherSessions && !disableMultipleSignout
? () => {
onSignOutOtherDevices(Object.keys(otherDevices));
}
@ -250,7 +260,7 @@ const SessionManagerTab: React.FC = () => {
heading={
<OtherSessionsSectionHeading
otherSessionsCount={otherSessionsCount}
signOutAllOtherSessions={signOutAllOtherSessions!}
signOutAllOtherSessions={signOutAllOtherSessions}
disabled={!!signingOutDeviceIds.length}
/>
}
@ -280,6 +290,7 @@ const SessionManagerTab: React.FC = () => {
setPushNotifications={setPushNotifications}
ref={filteredDeviceListRef}
supportsMSC3881={supportsMSC3881}
disableMultipleSignout={disableMultipleSignout}
/>
</SettingsSubsection>
)}

View file

@ -0,0 +1,27 @@
/*
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 { IClientWellKnown, IDelegatedAuthConfig, M_AUTHENTICATION } from "matrix-js-sdk/src/matrix";
/**
* Get the delegated auth account management url if configured
* @param clientWellKnown from MatrixClient.getClientWellKnown
* @returns the account management url, or undefined
*/
export const getDelegatedAuthAccountUrl = (clientWellKnown: IClientWellKnown | undefined): string | undefined => {
const delegatedAuthConfig = M_AUTHENTICATION.findIn<IDelegatedAuthConfig | undefined>(clientWellKnown);
return delegatedAuthConfig?.account;
};

View file

@ -15,7 +15,7 @@ limitations under the License.
*/
import React from "react";
import { act, fireEvent, render, RenderResult } from "@testing-library/react";
import { act, fireEvent, render, RenderResult, screen } from "@testing-library/react";
import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo";
import { logger } from "matrix-js-sdk/src/logger";
import { VerificationRequest } from "matrix-js-sdk/src/crypto-api";
@ -32,6 +32,7 @@ import {
CryptoApi,
DeviceVerificationStatus,
MatrixError,
M_AUTHENTICATION,
} from "matrix-js-sdk/src/matrix";
import { mocked } from "jest-mock";
@ -87,7 +88,7 @@ describe("<SessionManagerTab />", () => {
requestDeviceVerification: jest.fn().mockResolvedValue(mockVerificationRequest),
} as unknown as CryptoApi);
const mockClient = getMockClientWithEventEmitter({
let mockClient = getMockClientWithEventEmitter({
...mockClientMethodsUser(aliceId),
getCrypto: jest.fn().mockReturnValue(mockCrypto),
getDevices: jest.fn(),
@ -104,6 +105,7 @@ describe("<SessionManagerTab />", () => {
setLocalNotificationSettings: jest.fn(),
getVersions: jest.fn().mockResolvedValue({}),
getCapabilities: jest.fn().mockResolvedValue({}),
getClientWellKnown: jest.fn().mockReturnValue({}),
});
const defaultProps = {};
@ -173,6 +175,25 @@ describe("<SessionManagerTab />", () => {
};
beforeEach(async () => {
mockClient = getMockClientWithEventEmitter({
...mockClientMethodsUser(aliceId),
getCrypto: jest.fn().mockReturnValue(mockCrypto),
getDevices: jest.fn(),
getStoredDevice: jest.fn(),
getDeviceId: jest.fn().mockReturnValue(deviceId),
deleteMultipleDevices: jest.fn(),
generateClientSecret: jest.fn(),
setDeviceDetails: jest.fn(),
getAccountData: jest.fn(),
deleteAccountData: jest.fn(),
doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(true),
getPushers: jest.fn(),
setPusher: jest.fn(),
setLocalNotificationSettings: jest.fn(),
getVersions: jest.fn().mockResolvedValue({}),
getCapabilities: jest.fn().mockResolvedValue({}),
getClientWellKnown: jest.fn().mockReturnValue({}),
});
jest.clearAllMocks();
jest.spyOn(logger, "error").mockRestore();
mockClient.getStoredDevice.mockImplementation((_userId, id) => {
@ -1012,6 +1033,138 @@ describe("<SessionManagerTab />", () => {
);
});
});
describe("for an OIDC-aware server", () => {
beforeEach(() => {
mockClient.getClientWellKnown.mockReturnValue({
[M_AUTHENTICATION.name]: {
issuer: "https://issuer.org",
account: "https://issuer.org/account",
},
});
});
// signing out the current device works as usual
it("Signs out of current device", async () => {
const modalSpy = jest.spyOn(Modal, "createDialog");
mockClient.getDevices.mockResolvedValue({
devices: [alicesDevice],
});
const { getByTestId } = render(getComponent());
await act(async () => {
await flushPromises();
});
toggleDeviceDetails(getByTestId, alicesDevice.device_id);
const signOutButton = getByTestId("device-detail-sign-out-cta");
expect(signOutButton).toMatchSnapshot();
fireEvent.click(signOutButton);
// logout dialog opened
expect(modalSpy).toHaveBeenCalledWith(LogoutDialog, {}, undefined, false, true);
});
it("does not allow signing out of all other devices from current session context menu", async () => {
mockClient.getDevices.mockResolvedValue({
devices: [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice],
});
const { getByTestId } = render(getComponent());
await act(async () => {
await flushPromises();
});
fireEvent.click(getByTestId("current-session-menu"));
expect(screen.queryByLabelText("Sign out of all other sessions (2)")).not.toBeInTheDocument();
});
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],
});
const { getByTestId } = render(getComponent());
await act(async () => {
await flushPromises();
});
toggleDeviceDetails(getByTestId, alicesMobileDevice.device_id);
const deviceDetails = getByTestId(`device-detail-${alicesMobileDevice.device_id}`);
const signOutButton = deviceDetails.querySelector(
'[data-testid="device-detail-sign-out-cta"]',
) as Element;
fireEvent.click(signOutButton);
await confirmSignout(getByTestId);
// sign out button is disabled with spinner
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,
);
await flushPromises();
// devices refreshed
expect(mockClient.getDevices).toHaveBeenCalled();
});
it("does not allow removing multiple devices at once", async () => {
mockClient.getDevices.mockResolvedValue({
devices: [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice, alicesInactiveDevice],
});
render(getComponent());
await act(async () => {
await flushPromises();
});
// sessions don't have checkboxes
expect(
screen.queryByTestId(`device-tile-checkbox-${alicesMobileDevice.device_id}`),
).not.toBeInTheDocument();
// no select all
expect(screen.queryByLabelText("Select all")).not.toBeInTheDocument();
});
it("does not allow signing out of all other devices from other sessions context menu", async () => {
mockClient.getDevices.mockResolvedValue({
devices: [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice],
});
render(getComponent());
await act(async () => {
await flushPromises();
});
// no context menu because 'sign out all' is the only option
// and it is not allowed when server is oidc-aware
expect(screen.queryByTestId("other-sessions-menu")).not.toBeInTheDocument();
});
});
});
});
describe("Rename sessions", () => {

View file

@ -53,6 +53,21 @@ exports[`<SessionManagerTab /> Sign out Signs out of current device 1`] = `
</div>
`;
exports[`<SessionManagerTab /> Sign out for an OIDC-aware server Signs out of current device 1`] = `
<div
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_danger_inline"
data-testid="device-detail-sign-out-cta"
role="button"
tabindex="0"
>
<span
class="mx_DeviceDetails_signOutButtonContent"
>
Sign out of this session
</span>
</div>
`;
exports[`<SessionManagerTab /> current session section renders current session section with a verified session 1`] = `
<div
class="mx_SettingsSubsection"

View file

@ -0,0 +1,61 @@
/*
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 { M_AUTHENTICATION } from "matrix-js-sdk/src/matrix";
import { getDelegatedAuthAccountUrl } from "../../../src/utils/oidc/getDelegatedAuthAccountUrl";
describe("getDelegatedAuthAccountUrl()", () => {
it("should return undefined when wk is undefined", () => {
expect(getDelegatedAuthAccountUrl(undefined)).toBeUndefined();
});
it("should return undefined when wk has no authentication config", () => {
expect(getDelegatedAuthAccountUrl({})).toBeUndefined();
});
it("should return undefined when wk authentication config has no configured account url", () => {
expect(
getDelegatedAuthAccountUrl({
[M_AUTHENTICATION.stable!]: {
issuer: "issuer.org",
},
}),
).toBeUndefined();
});
it("should return the account url for authentication config using the unstable prefix", () => {
expect(
getDelegatedAuthAccountUrl({
[M_AUTHENTICATION.unstable!]: {
issuer: "issuer.org",
account: "issuer.org/account",
},
}),
).toEqual("issuer.org/account");
});
it("should return the account url for authentication config using the stable prefix", () => {
expect(
getDelegatedAuthAccountUrl({
[M_AUTHENTICATION.stable!]: {
issuer: "issuer.org",
account: "issuer.org/account",
},
}),
).toEqual("issuer.org/account");
});
});