OpenCloud
/
element-web
mirror of https://github.com/vector-im/element-web
1
0
Fork 0
Code Issues Releases Wiki Activity

OIDC settings tweaks (#28787) 

* Hide 3pid account settings if account is managed externally

As they would be disabled and just confusing otherwise

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Show manage device button instead of sign out button for other devices in OIDC mode

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Tidy up

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Fix tests

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
pull/28813/head
Michael Telatynski 2024-12-23 22:35:43 +00:00 committed by GitHub
parent 9d5141cfaa
commit 16d2cccb73
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 143 additions and 200 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
scripts/analyse_unused_exports.ts
View File

@ -19,6 +19,7 @@ ignore.push("/OpenSpotlightPayload.ts");
ignore.push("/PinnedMessageBadge.tsx");
ignore.push("/editor/mock.ts");
ignore.push("DeviceIsolationModeController.ts");
ignore.push("urls.ts");
ignore.push("/json.ts");
ignore.push("/ReleaseAnnouncementStore.ts");
ignore.push("/WidgetLayoutStore.ts");

66
src/components/views/dialogs/oidc/OidcLogoutDialog.tsx
View File

@ -1,66 +0,0 @@
/*
Copyright 2024 New Vector Ltd.
Copyright 2023 The Matrix.org Foundation C.I.C.
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
Please see LICENSE files in the repository root for full details.
*/
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("action|sign_out")} contentId="mx_Dialog_content">
<div className="mx_Dialog_content" id="mx_Dialog_content">
{_t("auth|oidc|logout_redirect_warning")}
</div>
<div className="mx_Dialog_buttons">
{hasOpenedLogoutLink ? (
<AccessibleButton kind="primary" onClick={() => onFinished(true)}>
{_t("action|close")}
</AccessibleButton>
) : (
<>
<AccessibleButton kind="secondary" onClick={() => onFinished(false)}>
{_t("action|cancel")}
</AccessibleButton>
<AccessibleButton
element="a"
onClick={() => setHasOpenedLogoutLink(true)}
kind="primary"
href={logoutUrl}
target="_blank"
>
{_t("action|continue")}
</AccessibleButton>
</>
)}
</div>
</BaseDialog>
);
};

4
src/components/views/settings/devices/CurrentDeviceSection.tsx
View File

@ -34,6 +34,7 @@ interface Props {
onSignOutCurrentDevice: () => void;
signOutAllOtherSessions?: () => void;
saveDeviceName: (deviceName: string) => Promise<void>;
delegatedAuthAccountUrl?: string;
}
type CurrentDeviceSectionHeadingProps = Pick<
@ -90,6 +91,7 @@ const CurrentDeviceSection: React.FC<Props> = ({
onSignOutCurrentDevice,
signOutAllOtherSessions,
saveDeviceName,
delegatedAuthAccountUrl,
}) => {
const [isExpanded, setIsExpanded] = useState(false);
@ -126,6 +128,8 @@ const CurrentDeviceSection: React.FC<Props> = ({
onSignOutDevice={onSignOutCurrentDevice}
saveDeviceName={saveDeviceName}
className="mx_CurrentDeviceSection_deviceDetails"
delegatedAuthAccountUrl={delegatedAuthAccountUrl}
isCurrentDevice
/>
) : (
<>

122
src/components/views/settings/devices/DeviceDetails.tsx
View File

@ -18,6 +18,7 @@ import ToggleSwitch from "../../elements/ToggleSwitch";
import { DeviceDetailHeading } from "./DeviceDetailHeading";
import { DeviceVerificationStatusCard } from "./DeviceVerificationStatusCard";
import { ExtendedDevice } from "./types";
import { getManageDeviceUrl } from "../../../../utils/oidc/urls.ts";
interface Props {
device: ExtendedDevice;
@ -31,6 +32,7 @@ interface Props {
supportsMSC3881?: boolean;
className?: string;
isCurrentDevice?: boolean;
delegatedAuthAccountUrl?: string;
}
interface MetadataTable {
@ -39,6 +41,22 @@ interface MetadataTable {
values: { label: string; value?: string | React.ReactNode }[];
}
function isPushNotificationsEnabled(pusher?: IPusher, notificationSettings?: LocalNotificationSettings): boolean {
if (pusher) return !!pusher[PUSHER_ENABLED.name];
if (notificationSettings) return !notificationSettings.is_silenced;
return true;
}
function isCheckboxDisabled(
pusher?: IPusher,
notificationSettings?: LocalNotificationSettings,
supportsMSC3881?: boolean,
): boolean {
if (notificationSettings) return false;
if (pusher && !supportsMSC3881) return true;
return false;
}
const DeviceDetails: React.FC<Props> = ({
device,
pusher,
@ -51,6 +69,7 @@ const DeviceDetails: React.FC<Props> = ({
supportsMSC3881,
className,
isCurrentDevice,
delegatedAuthAccountUrl,
}) => {
const metadata: MetadataTable[] = [
{
@ -95,18 +114,6 @@ const DeviceDetails: React.FC<Props> = ({
const showPushNotificationSection = !!pusher || !!localNotificationSettings;
function isPushNotificationsEnabled(pusher?: IPusher, notificationSettings?: LocalNotificationSettings): boolean {
if (pusher) return !!pusher[PUSHER_ENABLED.name];
if (localNotificationSettings) return !localNotificationSettings.is_silenced;
return true;
}
function isCheckboxDisabled(pusher?: IPusher, notificationSettings?: LocalNotificationSettings): boolean {
if (localNotificationSettings) return false;
if (pusher && !supportsMSC3881) return true;
return false;
}
return (
<div className={classNames("mx_DeviceDetails", className)} data-testid={`device-detail-${device.device_id}`}>
<section className="mx_DeviceDetails_section">
@ -117,32 +124,34 @@ const DeviceDetails: React.FC<Props> = ({
isCurrentDevice={isCurrentDevice}
/>
</section>
<section className="mx_DeviceDetails_section">
<p className="mx_DeviceDetails_sectionHeading">{_t("settings|sessions|details_heading")}</p>
{metadata.map(({ heading, values, id }, index) => (
<table
className="mx_DeviceDetails_metadataTable"
key={index}
data-testid={`device-detail-metadata-${id}`}
>
{heading && (
<thead>
<tr>
<th>{heading}</th>
</tr>
</thead>
)}
<tbody>
{values.map(({ label, value }) => (
<tr key={label}>
<td className="mxDeviceDetails_metadataLabel">{label}</td>
<td className="mxDeviceDetails_metadataValue">{value}</td>
</tr>
))}
</tbody>
</table>
))}
</section>
{!delegatedAuthAccountUrl && (
<section className="mx_DeviceDetails_section">
<p className="mx_DeviceDetails_sectionHeading">{_t("settings|sessions|details_heading")}</p>
{metadata.map(({ heading, values, id }, index) => (
<table
className="mx_DeviceDetails_metadataTable"
key={index}
data-testid={`device-detail-metadata-${id}`}
>
{heading && (
<thead>
<tr>
<th>{heading}</th>
</tr>
</thead>
)}
<tbody>
{values.map(({ label, value }) => (
<tr key={label}>
<td className="mxDeviceDetails_metadataLabel">{label}</td>
<td className="mxDeviceDetails_metadataValue">{value}</td>
</tr>
))}
</tbody>
</table>
))}
</section>
)}
{showPushNotificationSection && (
<section
className="mx_DeviceDetails_section mx_DeviceDetails_pushNotifications"
@ -152,7 +161,7 @@ const DeviceDetails: React.FC<Props> = ({
// For backwards compatibility, if `enabled` is missing
// default to `true`
checked={isPushNotificationsEnabled(pusher, localNotificationSettings)}
disabled={isCheckboxDisabled(pusher, localNotificationSettings)}
disabled={isCheckboxDisabled(pusher, localNotificationSettings, supportsMSC3881)}
onChange={(checked) => setPushNotifications?.(device.device_id, checked)}
title={_t("settings|sessions|push_toggle")}
data-testid="device-detail-push-notification-checkbox"
@ -166,17 +175,30 @@ const DeviceDetails: React.FC<Props> = ({
</section>
)}
<section className="mx_DeviceDetails_section">
<AccessibleButton
onClick={onSignOutDevice}
kind="danger_inline"
disabled={isSigningOut}
data-testid="device-detail-sign-out-cta"
>
<span className="mx_DeviceDetails_signOutButtonContent">
{_t("settings|sessions|sign_out")}
{isSigningOut && <Spinner w={16} h={16} />}
</span>
</AccessibleButton>
{delegatedAuthAccountUrl && !isCurrentDevice ? (
<AccessibleButton
element="a"
onClick={null}
kind="link_inline"
href={getManageDeviceUrl(delegatedAuthAccountUrl, device.device_id)}
target="_blank"
data-testid="device-detail-sign-out-cta"
>
<span className="mx_DeviceDetails_signOutButtonContent">{_t("settings|sessions|manage")}</span>
</AccessibleButton>
) : (
<AccessibleButton
onClick={onSignOutDevice}
kind="danger_inline"
disabled={isSigningOut}
data-testid="device-detail-sign-out-cta"
>
<span className="mx_DeviceDetails_signOutButtonContent">
{_t("settings|sessions|sign_out")}
{isSigningOut && <Spinner w={16} h={16} />}
</span>
</AccessibleButton>
)}
</section>
</div>
);

16
src/components/views/settings/devices/FilteredDeviceList.tsx
View File

@ -41,10 +41,12 @@ interface Props {
setSelectedDeviceIds: (deviceIds: ExtendedDevice["device_id"][]) => void;
supportsMSC3881?: boolean | undefined;
/**
* Only allow sessions to be signed out individually
* If the user's account is managed externally then sessions must be signed out individually
* Removes checkboxes and multi selection header
* Removes session info as that can be seen in the account management
* Changes sign out button to be a manage button
*/
disableMultipleSignout?: boolean;
delegatedAuthAccountUrl?: string;
}
const isDeviceSelected = (
@ -172,6 +174,7 @@ const DeviceListItem: React.FC<{
setPushNotifications: (deviceId: string, enabled: boolean) => Promise<void>;
supportsMSC3881?: boolean | undefined;
isSelectDisabled?: boolean;
delegatedAuthAccountUrl?: string;
}> = ({
device,
pusher,
@ -187,6 +190,7 @@ const DeviceListItem: React.FC<{
toggleSelected,
supportsMSC3881,
isSelectDisabled,
delegatedAuthAccountUrl,
}) => {
const tileContent = (
<>
@ -222,6 +226,7 @@ const DeviceListItem: React.FC<{
setPushNotifications={setPushNotifications}
supportsMSC3881={supportsMSC3881}
className="mx_FilteredDeviceList_deviceDetails"
delegatedAuthAccountUrl={delegatedAuthAccountUrl}
/>
)}
</li>
@ -250,7 +255,7 @@ export const FilteredDeviceList = forwardRef(
setPushNotifications,
setSelectedDeviceIds,
supportsMSC3881,
disableMultipleSignout,
delegatedAuthAccountUrl,
}: Props,
ref: ForwardedRef<HTMLDivElement>,
) => {
@ -311,7 +316,7 @@ export const FilteredDeviceList = forwardRef(
selectedDeviceCount={selectedDeviceIds.length}
isAllSelected={isAllSelected}
toggleSelectAll={toggleSelectAll}
isSelectDisabled={disableMultipleSignout}
isSelectDisabled={!!delegatedAuthAccountUrl}
>
{selectedDeviceIds.length ? (
<>
@ -361,7 +366,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}
isSelectDisabled={!!delegatedAuthAccountUrl}
onDeviceExpandToggle={() => onDeviceExpandToggle(device.device_id)}
onSignOutDevice={() => onSignOutDevices([device.device_id])}
saveDeviceName={(deviceName: string) => saveDeviceName(device.device_id, deviceName)}
@ -373,6 +378,7 @@ export const FilteredDeviceList = forwardRef(
setPushNotifications={setPushNotifications}
toggleSelected={() => toggleSelection(device.device_id)}
supportsMSC3881={supportsMSC3881}
delegatedAuthAccountUrl={delegatedAuthAccountUrl}
/>
))}
</ol>

4
src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx
View File

@ -186,7 +186,9 @@ const AccountUserSettingsTab: React.FC<IProps> = ({ closeSettingsFn }) => {
canSetDisplayName={canSetDisplayName}
canSetAvatar={canSetAvatar}
/>
<UserPersonalInfoSettings canMake3pidChanges={canMake3pidChanges} />
{(!isAccountManagedExternally || canMake3pidChanges) && (
<UserPersonalInfoSettings canMake3pidChanges={canMake3pidChanges} />
)}
<AccountSection
canChangePassword={canChangePassword}
onPasswordChanged={onPasswordChanged}

38
src/components/views/settings/tabs/user/SessionManagerTab.tsx
View File

@ -31,7 +31,7 @@ import QuestionDialog from "../../../dialogs/QuestionDialog";
import { FilterVariation } from "../../devices/filter";
import { OtherSessionsSectionHeading } from "../../devices/OtherSessionsSectionHeading";
import { SettingsSection } from "../../shared/SettingsSection";
import { OidcLogoutDialog } from "../../../dialogs/oidc/OidcLogoutDialog";
import { getManageDeviceUrl } from "../../../../../utils/oidc/urls.ts";
import { SDKContext } from "../../../../../contexts/SDKContext";
import Spinner from "../../../elements/Spinner";
@ -58,16 +58,6 @@ const confirmSignOut = async (sessionsToSignOutCount: number): Promise<boolean>
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 = (
matrixClient: MatrixClient,
onSignoutResolvedCallback: () => Promise<void>,
@ -93,20 +83,10 @@ const useSignOut = (
if (!deviceIds.length) {
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);
if (!userConfirmedSignout) {
return;
}
const userConfirmedSignout = await confirmSignOut(deviceIds.length);
if (!userConfirmedSignout) {
return;
}
let success = false;
@ -115,11 +95,8 @@ const useSignOut = (
if (delegatedAuthAccountUrl) {
const [deviceId] = deviceIds;
try {
success = await confirmDelegatedAuthSignOut(delegatedAuthAccountUrl, deviceId);
} catch (error) {
logger.error("Error deleting OIDC-aware sessions", error);
}
const url = getManageDeviceUrl(delegatedAuthAccountUrl, deviceId);
window.open(url, "_blank");
} else {
const deferredSuccess = defer<boolean>();
await deleteDevicesWithInteractiveAuth(matrixClient, deviceIds, async (success) => {
@ -323,6 +300,7 @@ const SessionManagerTab: React.FC<{
onSignOutCurrentDevice={onSignOutCurrentDevice}
signOutAllOtherSessions={signOutAllOtherSessions}
otherSessionsCount={otherSessionsCount}
delegatedAuthAccountUrl={delegatedAuthAccountUrl}
/>
{shouldShowOtherSessions && (
<SettingsSubsection
@ -356,7 +334,7 @@ const SessionManagerTab: React.FC<{
setPushNotifications={setPushNotifications}
ref={filteredDeviceListRef}
supportsMSC3881={supportsMSC3881}
disableMultipleSignout={disableMultipleSignout}
delegatedAuthAccountUrl={delegatedAuthAccountUrl}
/>
</SettingsSubsection>
)}

2
src/i18n/strings/en_EN.json
View File

@ -238,7 +238,6 @@
"oidc": {
"error_title": "We couldn't log you in",
"generic_auth_error": "Something went wrong during authentication. Go to the sign in page and try again.",
"logout_redirect_warning": "You will be redirected to your server's authentication provider to complete sign out.",
"missing_or_invalid_stored_state": "We asked the browser to remember which homeserver you use to let you sign in, but unfortunately your browser has forgotten it. Go to the sign in page and try again."
},
"password_field_keep_going_prompt": "Keep going…",
@ -2821,6 +2820,7 @@
"inactive_sessions_list_description": "Consider signing out from old sessions (%(inactiveAgeDays)s days or older) you don't use anymore.",
"ip": "IP address",
"last_activity": "Last activity",
"manage": "Manage this session",
"mobile_session": "Mobile session",
"n_sessions_selected": {
"one": "%(count)s session selected",

20
src/utils/oidc/getOidcLogoutUrl.ts
View File

@ -1,20 +0,0 @@
/*
Copyright 2024 New Vector Ltd.
Copyright 2023 The Matrix.org Foundation C.I.C.
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
Please see LICENSE files in the repository root for full details.
*/
/**
* 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();
};

32
src/utils/oidc/urls.ts Normal file
View File

@ -0,0 +1,32 @@
/*
Copyright 2024 New Vector Ltd.
Copyright 2023 The Matrix.org Foundation C.I.C.
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
Please see LICENSE files in the repository root for full details.
*/
enum Action {
Profile = "org.matrix.profile",
SessionsList = "org.matrix.sessions_list",
SessionView = "org.matrix.session_view",
SessionEnd = "org.matrix.session_end",
AccountDeactivate = "org.matrix.account_deactivate",
CrossSigningReset = "org.matrix.cross_signing_reset",
}
const getUrl = (authUrl: string, action: Action): URL => {
const url = new URL(authUrl);
url.searchParams.set("action", action);
return url;
};
/**
* Create a delegated auth account management URL with logout params as per MSC4191
* https://github.com/matrix-org/matrix-spec-proposals/blob/quenting/account-deeplink/proposals/4191-account-deeplink.md#possible-actions
*/
export const getManageDeviceUrl = (delegatedAuthAccountUrl: string, deviceId: string): string => {
const url = getUrl(delegatedAuthAccountUrl, Action.SessionView);
url.searchParams.set("device_id", deviceId);
return url.toString();
};

2
test/unit-tests/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap
View File

@ -49,7 +49,7 @@ HTMLCollection [
<p
class="mx_DeviceSecurityCard_description"
>
Verify or sign out from this session for best security and reliability.
Verify your current session for enhanced secure messaging.
<div
class="mx_AccessibleButton mx_LearnMore_button mx_AccessibleButton_hasKind mx_AccessibleButton_kind_link_inline"
role="button"

7
test/unit-tests/components/views/settings/tabs/user/AccountUserSettingsTab-test.tsx
View File

@ -114,7 +114,7 @@ describe("<AccountUserSettingsTab />", () => {
expect(manageAccountLink.getAttribute("href")).toMatch(accountManagementLink);
});
describe("deactive account", () => {
describe("deactivate account", () => {
it("should not render section when account deactivation feature is disabled", () => {
jest.spyOn(SettingsStore, "getValue").mockImplementation(
(settingName) => settingName !== UIFeature.Deactivate,
@ -198,6 +198,11 @@ describe("<AccountUserSettingsTab />", () => {
describe("3pids", () => {
beforeEach(() => {
const mockOidcClientStore = {
accountManagementEndpoint: undefined,
} as unknown as OidcClientStore;
jest.spyOn(stores, "oidcClientStore", "get").mockReturnValue(mockOidcClientStore);
mockClient.getCapabilities.mockResolvedValue({
"m.3pid_changes": {
enabled: true,

27
test/unit-tests/components/views/settings/tabs/user/SessionManagerTab-test.tsx
View File

@ -1234,34 +1234,13 @@ describe("<SessionManagerTab />", () => {
toggleDeviceDetails(getByTestId, alicesMobileDevice.device_id);
const deviceDetails = getByTestId(`device-detail-${alicesMobileDevice.device_id}`);
const signOutButton = deviceDetails.querySelector(
const manageDeviceButton = deviceDetails.querySelector(
'[data-testid="device-detail-sign-out-cta"]',
) as Element;
fireEvent.click(signOutButton);
await screen.findByRole("dialog");
expect(
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(
expect(manageDeviceButton).toHaveAttribute(
"href",
`https://issuer.org/account?action=session_end&device_id=${alicesMobileDevice.device_id}`,
`https://issuer.org/account?action=org.matrix.session_view&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 were refreshed
expect(mockClient.getDevices).toHaveBeenCalled();
});
it("does not allow removing multiple devices at once", async () => {

2
test/unit-tests/components/views/settings/tabs/user/__snapshots__/AccountUserSettingsTab-test.tsx.snap
View File

@ -169,7 +169,7 @@ exports[`<AccountUserSettingsTab /> 3pids should display 3pid email addresses an
</div>
`;
exports[`<AccountUserSettingsTab /> deactive account should render section when account deactivation feature is enabled 1`] = `
exports[`<AccountUserSettingsTab /> deactivate account should render section when account deactivation feature is enabled 1`] = `
<div
class="mx_SettingsSection"
>