From a67cfbb6963e599e5e5a3c4fd743ad067342d505 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 20 Dec 2024 11:29:08 +0000 Subject: [PATCH 1/4] 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> --- .../views/settings/tabs/user/AccountUserSettingsTab.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx b/src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx index cd52b2a76b2..8c8243f2b2f 100644 --- a/src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx @@ -186,7 +186,9 @@ const AccountUserSettingsTab: React.FC = ({ closeSettingsFn }) => { canSetDisplayName={canSetDisplayName} canSetAvatar={canSetAvatar} /> - + {!isAccountManagedExternally && !canMake3pidChanges && ( + + )} Date: Fri, 20 Dec 2024 12:11:27 +0000 Subject: [PATCH 2/4] Show manage device button instead of sign out button for other devices in OIDC mode Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../views/dialogs/oidc/OidcLogoutDialog.tsx | 66 ------------- .../settings/devices/CurrentDeviceSection.tsx | 4 + .../views/settings/devices/DeviceDetails.tsx | 92 +++++++++++-------- .../settings/devices/FilteredDeviceList.tsx | 16 +++- .../settings/tabs/user/SessionManagerTab.tsx | 38 ++------ src/i18n/strings/en_EN.json | 2 +- src/utils/oidc/getOidcLogoutUrl.ts | 20 ---- src/utils/oidc/urls.ts | 32 +++++++ 8 files changed, 111 insertions(+), 159 deletions(-) delete mode 100644 src/components/views/dialogs/oidc/OidcLogoutDialog.tsx delete mode 100644 src/utils/oidc/getOidcLogoutUrl.ts create mode 100644 src/utils/oidc/urls.ts diff --git a/src/components/views/dialogs/oidc/OidcLogoutDialog.tsx b/src/components/views/dialogs/oidc/OidcLogoutDialog.tsx deleted file mode 100644 index 83bcd8736f8..00000000000 --- a/src/components/views/dialogs/oidc/OidcLogoutDialog.tsx +++ /dev/null @@ -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 = ({ - delegatedAuthAccountUrl, - deviceId, - onFinished, -}) => { - const [hasOpenedLogoutLink, setHasOpenedLogoutLink] = useState(false); - const logoutUrl = getOidcLogoutUrl(delegatedAuthAccountUrl, deviceId); - - return ( - -
- {_t("auth|oidc|logout_redirect_warning")} -
-
- {hasOpenedLogoutLink ? ( - onFinished(true)}> - {_t("action|close")} - - ) : ( - <> - onFinished(false)}> - {_t("action|cancel")} - - setHasOpenedLogoutLink(true)} - kind="primary" - href={logoutUrl} - target="_blank" - > - {_t("action|continue")} - - - )} -
-
- ); -}; diff --git a/src/components/views/settings/devices/CurrentDeviceSection.tsx b/src/components/views/settings/devices/CurrentDeviceSection.tsx index 153a9c5d4bd..9a3eb41015f 100644 --- a/src/components/views/settings/devices/CurrentDeviceSection.tsx +++ b/src/components/views/settings/devices/CurrentDeviceSection.tsx @@ -34,6 +34,7 @@ interface Props { onSignOutCurrentDevice: () => void; signOutAllOtherSessions?: () => void; saveDeviceName: (deviceName: string) => Promise; + delegatedAuthAccountUrl?: string; } type CurrentDeviceSectionHeadingProps = Pick< @@ -90,6 +91,7 @@ const CurrentDeviceSection: React.FC = ({ onSignOutCurrentDevice, signOutAllOtherSessions, saveDeviceName, + delegatedAuthAccountUrl, }) => { const [isExpanded, setIsExpanded] = useState(false); @@ -126,6 +128,8 @@ const CurrentDeviceSection: React.FC = ({ onSignOutDevice={onSignOutCurrentDevice} saveDeviceName={saveDeviceName} className="mx_CurrentDeviceSection_deviceDetails" + delegatedAuthAccountUrl={delegatedAuthAccountUrl} + isCurrentDevice /> ) : ( <> diff --git a/src/components/views/settings/devices/DeviceDetails.tsx b/src/components/views/settings/devices/DeviceDetails.tsx index 1a418f5dd52..2b5e8dc6d66 100644 --- a/src/components/views/settings/devices/DeviceDetails.tsx +++ b/src/components/views/settings/devices/DeviceDetails.tsx @@ -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 { @@ -51,6 +53,7 @@ const DeviceDetails: React.FC = ({ supportsMSC3881, className, isCurrentDevice, + delegatedAuthAccountUrl, }) => { const metadata: MetadataTable[] = [ { @@ -117,32 +120,34 @@ const DeviceDetails: React.FC = ({ isCurrentDevice={isCurrentDevice} /> -
-

{_t("settings|sessions|details_heading")}

- {metadata.map(({ heading, values, id }, index) => ( - - {heading && ( - - - - - - )} - - {values.map(({ label, value }) => ( - - - - - ))} - -
{heading}
{label}{value}
- ))} -
+ {!delegatedAuthAccountUrl && ( +
+

{_t("settings|sessions|details_heading")}

+ {metadata.map(({ heading, values, id }, index) => ( + + {heading && ( + + + + + + )} + + {values.map(({ label, value }) => ( + + + + + ))} + +
{heading}
{label}{value}
+ ))} +
+ )} {showPushNotificationSection && (
= ({
)}
- - - {_t("settings|sessions|sign_out")} - {isSigningOut && } - - + {delegatedAuthAccountUrl && !isCurrentDevice ? ( + + {_t("settings|sessions|manage")} + + ) : ( + + + {_t("settings|sessions|sign_out")} + {isSigningOut && } + + + )}
); diff --git a/src/components/views/settings/devices/FilteredDeviceList.tsx b/src/components/views/settings/devices/FilteredDeviceList.tsx index e5f1a6a9a3b..b1f2180a78a 100644 --- a/src/components/views/settings/devices/FilteredDeviceList.tsx +++ b/src/components/views/settings/devices/FilteredDeviceList.tsx @@ -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; 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} /> )} @@ -250,7 +255,7 @@ export const FilteredDeviceList = forwardRef( setPushNotifications, setSelectedDeviceIds, supportsMSC3881, - disableMultipleSignout, + delegatedAuthAccountUrl, }: Props, ref: ForwardedRef, ) => { @@ -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} /> ))} diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx index 5e9445bb995..71ff94ed7d0 100644 --- a/src/components/views/settings/tabs/user/SessionManagerTab.tsx +++ b/src/components/views/settings/tabs/user/SessionManagerTab.tsx @@ -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 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, @@ -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(); await deleteDevicesWithInteractiveAuth(matrixClient, deviceIds, async (success) => { @@ -323,6 +300,7 @@ const SessionManagerTab: React.FC<{ onSignOutCurrentDevice={onSignOutCurrentDevice} signOutAllOtherSessions={signOutAllOtherSessions} otherSessionsCount={otherSessionsCount} + delegatedAuthAccountUrl={delegatedAuthAccountUrl} /> {shouldShowOtherSessions && ( )} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index f3c514fca38..b2db2d6e8e8 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -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…", @@ -2823,6 +2822,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", diff --git a/src/utils/oidc/getOidcLogoutUrl.ts b/src/utils/oidc/getOidcLogoutUrl.ts deleted file mode 100644 index be632b815ae..00000000000 --- a/src/utils/oidc/getOidcLogoutUrl.ts +++ /dev/null @@ -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(); -}; diff --git a/src/utils/oidc/urls.ts b/src/utils/oidc/urls.ts new file mode 100644 index 00000000000..9e95e81c343 --- /dev/null +++ b/src/utils/oidc/urls.ts @@ -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(); +}; From 7a34c5d1e1eabfcd1b09d4a46be78824b67e5a33 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 20 Dec 2024 12:12:31 +0000 Subject: [PATCH 3/4] Tidy up Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../views/settings/devices/DeviceDetails.tsx | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/components/views/settings/devices/DeviceDetails.tsx b/src/components/views/settings/devices/DeviceDetails.tsx index 2b5e8dc6d66..569a2a01991 100644 --- a/src/components/views/settings/devices/DeviceDetails.tsx +++ b/src/components/views/settings/devices/DeviceDetails.tsx @@ -41,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 = ({ device, pusher, @@ -98,18 +114,6 @@ const DeviceDetails: React.FC = ({ 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 (
@@ -157,7 +161,7 @@ const DeviceDetails: React.FC = ({ // 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" From e1a826768a9e48981e0128c72716bb843ee86403 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 20 Dec 2024 12:38:58 +0000 Subject: [PATCH 4/4] Fix tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- scripts/analyse_unused_exports.ts | 1 + .../tabs/user/AccountUserSettingsTab.tsx | 2 +- .../CurrentDeviceSection-test.tsx.snap | 2 +- .../tabs/user/AccountUserSettingsTab-test.tsx | 7 ++++- .../tabs/user/SessionManagerTab-test.tsx | 27 +++---------------- .../AccountUserSettingsTab-test.tsx.snap | 2 +- 6 files changed, 13 insertions(+), 28 deletions(-) diff --git a/scripts/analyse_unused_exports.ts b/scripts/analyse_unused_exports.ts index 46e594d2b74..a95f1c65e4e 100755 --- a/scripts/analyse_unused_exports.ts +++ b/scripts/analyse_unused_exports.ts @@ -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"); // We ignore js-sdk by default as it may export for other non element-web projects if (!includeJSSDK) ignore.push("matrix-js-sdk"); diff --git a/src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx b/src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx index 8c8243f2b2f..cb1d2a3edd3 100644 --- a/src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/AccountUserSettingsTab.tsx @@ -186,7 +186,7 @@ const AccountUserSettingsTab: React.FC = ({ closeSettingsFn }) => { canSetDisplayName={canSetDisplayName} canSetAvatar={canSetAvatar} /> - {!isAccountManagedExternally && !canMake3pidChanges && ( + {(!isAccountManagedExternally || canMake3pidChanges) && ( )} - Verify or sign out from this session for best security and reliability. + Verify your current session for enhanced secure messaging. `; -exports[` deactive account should render section when account deactivation feature is enabled 1`] = ` +exports[` deactivate account should render section when account deactivation feature is enabled 1`] = `