-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wire up the "Forgot recovery key" button for the "Key storage out of sync" toast #29138
Changes from all commits
b62aebb
8b2aaa8
2f0c5d2
33abac3
d08b013
4d99bef
bc48eac
7ec1f01
823a05e
6df7df9
5508c97
a612305
939939a
711e4f7
6681772
60d1fa1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/* | ||
* Copyright 2025 New Vector Ltd. | ||
* | ||
* SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial | ||
* Please see LICENSE files in the repository root for full details. | ||
*/ | ||
|
||
import { GeneratedSecretStorageKey } from "matrix-js-sdk/src/crypto-api"; | ||
|
||
import { test, expect } from "../../element-web-test"; | ||
import { createBot, deleteCachedSecrets, logIntoElement } from "./utils"; | ||
|
||
test.describe("Key storage out of sync toast", () => { | ||
let recoveryKey: GeneratedSecretStorageKey; | ||
|
||
test.beforeEach(async ({ page, homeserver, credentials }) => { | ||
const res = await createBot(page, homeserver, credentials); | ||
recoveryKey = res.recoveryKey; | ||
|
||
await logIntoElement(page, credentials, recoveryKey.encodedPrivateKey); | ||
|
||
await deleteCachedSecrets(page); | ||
|
||
// We won't be prompted for crypto setup unless we have an e2e room, so make one | ||
await page.getByRole("button", { name: "Add room" }).click(); | ||
await page.getByRole("menuitem", { name: "New room" }).click(); | ||
await page.getByRole("textbox", { name: "Name" }).fill("Test room"); | ||
await page.getByRole("button", { name: "Create room" }).click(); | ||
}); | ||
|
||
test("should prompt for recovery key if 'enter recovery key' pressed", { tag: "@screenshot" }, async ({ page }) => { | ||
// Need to wait for 2 to appear since playwright only evaluates 'first()' initially, so the waiting won't work | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
wat? |
||
await expect(page.getByRole("alert")).toHaveCount(2); | ||
await expect(page.getByRole("alert").first()).toMatchScreenshot("key-storage-out-of-sync-toast.png"); | ||
|
||
await page.getByRole("button", { name: "Enter recovery key" }).click(); | ||
await page.locator(".mx_Dialog").getByRole("button", { name: "use your Security Key" }).click(); | ||
|
||
await page.getByRole("textbox", { name: "Security key" }).fill(recoveryKey.encodedPrivateKey); | ||
await page.getByRole("button", { name: "Continue" }).click(); | ||
|
||
await expect(page.getByRole("button", { name: "Enter recovery key" })).not.toBeVisible(); | ||
}); | ||
|
||
test("should open settings to reset flow if 'forgot recovery key' pressed", async ({ page, app, credentials }) => { | ||
await expect(page.getByRole("button", { name: "Enter recovery key" })).toBeVisible(); | ||
|
||
await page.getByRole("button", { name: "Forgot recovery key?" }).click(); | ||
|
||
await expect( | ||
page.getByRole("heading", { name: "Forgot your recovery key? You’ll need to reset your identity." }), | ||
).toBeVisible(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -214,6 +214,11 @@ export async function logIntoElement(page: Page, credentials: Credentials, secur | |
// if a securityKey was given, verify the new device | ||
if (securityKey !== undefined) { | ||
await page.locator(".mx_AuthPage").getByRole("button", { name: "Verify with Security Key" }).click(); | ||
|
||
const useSecurityKey = page.locator(".mx_Dialog").getByRole("button", { name: "use your Security Key" }); | ||
if (await useSecurityKey.isVisible()) { | ||
await useSecurityKey.click(); | ||
} | ||
Comment on lines
+218
to
+221
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment would be good here. Why is this sometimes necessary and sometimes not? |
||
// Fill in the security key | ||
await page.locator(".mx_Dialog").locator('input[type="password"]').fill(securityKey); | ||
await page.locator(".mx_Dialog_primary:not([disabled])", { hasText: "Continue" }).click(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,7 @@ import { EncryptionUserSettingsTab } from "../settings/tabs/user/EncryptionUserS | |
interface IProps { | ||
initialTabId?: UserTab; | ||
showMsc4108QrCode?: boolean; | ||
showResetIdentity?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. documentation please. What does this do? Is |
||
sdkContext: SdkContextClass; | ||
onFinished(): void; | ||
} | ||
|
@@ -91,8 +92,9 @@ function titleForTabID(tabId: UserTab): React.ReactNode { | |
export default function UserSettingsDialog(props: IProps): JSX.Element { | ||
const voipEnabled = useSettingValue(UIFeature.Voip); | ||
const mjolnirEnabled = useSettingValue("feature_mjolnir"); | ||
// store this prop in state as changing tabs back and forth should clear it | ||
// store these props in state as changing tabs back and forth should clear it | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/it/them/ |
||
const [showMsc4108QrCode, setShowMsc4108QrCode] = useState(props.showMsc4108QrCode); | ||
const [showResetIdentity, setShowResetIdentity] = useState(props.showResetIdentity); | ||
|
||
const getTabs = (): NonEmptyArray<Tab<UserTab>> => { | ||
const tabs: Tab<UserTab>[] = []; | ||
|
@@ -184,7 +186,12 @@ export default function UserSettingsDialog(props: IProps): JSX.Element { | |
); | ||
|
||
tabs.push( | ||
new Tab(UserTab.Encryption, _td("settings|encryption|title"), <KeyIcon />, <EncryptionUserSettingsTab />), | ||
new Tab( | ||
UserTab.Encryption, | ||
_td("settings|encryption|title"), | ||
<KeyIcon />, | ||
<EncryptionUserSettingsTab initialState={showResetIdentity ? "reset_identity_forgot" : undefined} />, | ||
), | ||
); | ||
|
||
if (showLabsFlags() || SettingsStore.getFeatureSettingNames().some((k) => SettingsStore.getBetaInfo(k))) { | ||
|
@@ -219,8 +226,9 @@ export default function UserSettingsDialog(props: IProps): JSX.Element { | |
const [activeTabId, _setActiveTabId] = useActiveTabWithDefault(getTabs(), UserTab.Account, props.initialTabId); | ||
const setActiveTabId = (tabId: UserTab): void => { | ||
_setActiveTabId(tabId); | ||
// Clear this so switching away from the tab and back to it will not show the QR code again | ||
// Clear these so switching away from the tab and back to it will not show the QR code again | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how is |
||
setShowMsc4108QrCode(false); | ||
setShowResetIdentity(false); | ||
}; | ||
|
||
const [activeToast, toastRack] = useActiveToast(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,12 +25,21 @@ interface ResetIdentityPanelProps { | |
* Called when the cancel button is clicked or when we go back in the breadcrumbs. | ||
*/ | ||
onCancelClick: () => void; | ||
|
||
/** | ||
* The variant of the panel to show. We show more warnings in the 'compromised' variant (no use in showing a user this | ||
* warning if they have to reset because they no longer have their key) | ||
* "compromised" is shown when the user chooses 'reset' explicitly in settings, usually because they believe their | ||
* identity has been compromised. | ||
* "forgot" is shown when the user has just forgotten their passphrase. | ||
*/ | ||
Comment on lines
+29
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Formatting is weird here. Newlines don't do anything in tsdoc; either commit to new paragraphs with a blank line, or match the generated output by removing the linebreaks (presumably the former in this case). You can't just sit on the fence. |
||
variant: "compromised" | "forgot"; | ||
} | ||
|
||
/** | ||
* The panel for resetting the identity of the current user. | ||
*/ | ||
export function ResetIdentityPanel({ onCancelClick, onFinish }: ResetIdentityPanelProps): JSX.Element { | ||
export function ResetIdentityPanel({ onCancelClick, onFinish, variant }: ResetIdentityPanelProps): JSX.Element { | ||
const matrixClient = useMatrixClientContext(); | ||
|
||
return ( | ||
|
@@ -44,7 +53,11 @@ export function ResetIdentityPanel({ onCancelClick, onFinish }: ResetIdentityPan | |
<EncryptionCard | ||
Icon={ErrorIcon} | ||
destructive={true} | ||
title={_t("settings|encryption|advanced|breadcrumb_title")} | ||
title={ | ||
variant === "forgot" | ||
? _t("settings|encryption|advanced|breadcrumb_title_forgot") | ||
: _t("settings|encryption|advanced|breadcrumb_title") | ||
} | ||
className="mx_ResetIdentityPanel" | ||
> | ||
<div className="mx_ResetIdentityPanel_content"> | ||
|
@@ -59,7 +72,7 @@ export function ResetIdentityPanel({ onCancelClick, onFinish }: ResetIdentityPan | |
{_t("settings|encryption|advanced|breadcrumb_third_description")} | ||
</VisualListItem> | ||
</VisualList> | ||
<span>{_t("settings|encryption|advanced|breadcrumb_warning")}</span> | ||
{variant === "compromised" && <span>{_t("settings|encryption|advanced|breadcrumb_warning")}</span>} | ||
</div> | ||
<div className="mx_ResetIdentityPanel_footer"> | ||
<Button | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,23 +32,35 @@ import { RecoveryPanelOutOfSync } from "../../encryption/RecoveryPanelOutOfSync" | |
* This happens when the user has a recovery key and the user clicks on "Change recovery key" button of the RecoveryPanel. | ||
* - "set_recovery_key": The panel to show when the user is setting up their recovery key. | ||
* This happens when the user doesn't have a key a recovery key and the user clicks on "Set up recovery key" button of the RecoveryPanel. | ||
* - "reset_identity": The panel to show when the user is resetting their identity. | ||
* - `secrets_not_cached`: The secrets are not cached locally. This can happen if we verified another device and secret-gossiping failed, or the other device itself lacked the secrets. | ||
* - "reset_identity_compromised": The panel to show when the user is resetting their identity, in te case where their key is compromised. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. te |
||
* - "reset_identity_forgot": The panel to show when the user is resetting their identity, in the case where they forgot their recovery key. | ||
* - `secrets_not_cached`: The secrets are not cached locally. This can happen if we verified another device and secret-gossiping failed, or the other device itself lacked the secrets. | ||
* If the "set_up_encryption" and "secrets_not_cached" conditions are both filled, "set_up_encryption" prevails. | ||
Comment on lines
+35
to
38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what on earth is going on with the indentation here? |
||
* | ||
*/ | ||
type State = | ||
export type State = | ||
| "loading" | ||
| "main" | ||
| "set_up_encryption" | ||
| "change_recovery_key" | ||
| "set_recovery_key" | ||
| "reset_identity" | ||
| "reset_identity_compromised" | ||
| "reset_identity_forgot" | ||
| "secrets_not_cached"; | ||
|
||
export function EncryptionUserSettingsTab(): JSX.Element { | ||
const [state, setState] = useState<State>("loading"); | ||
const checkEncryptionState = useCheckEncryptionState(setState); | ||
interface EncryptionUserSettingsTabProps { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, my reading of https://github.com/element-hq/element-web/blob/develop/code_style.md#react is that the Props interface should be called |
||
/** | ||
* If the tab should start in a state other than the deasult | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. desault |
||
*/ | ||
initialState?: State; | ||
} | ||
|
||
/** | ||
* The encryption settings tab. | ||
*/ | ||
export function EncryptionUserSettingsTab({ initialState = "loading" }: EncryptionUserSettingsTabProps): JSX.Element { | ||
const [state, setState] = useState<State>(initialState); | ||
|
||
const checkEncryptionState = useCheckEncryptionState(state, setState); | ||
|
||
let content: JSX.Element; | ||
switch (state) { | ||
|
@@ -70,7 +82,7 @@ export function EncryptionUserSettingsTab(): JSX.Element { | |
} | ||
/> | ||
<Separator kind="section" /> | ||
<AdvancedPanel onResetIdentityClick={() => setState("reset_identity")} /> | ||
<AdvancedPanel onResetIdentityClick={() => setState("reset_identity_compromised")} /> | ||
</> | ||
); | ||
break; | ||
|
@@ -84,8 +96,23 @@ export function EncryptionUserSettingsTab(): JSX.Element { | |
/> | ||
); | ||
break; | ||
case "reset_identity": | ||
content = <ResetIdentityPanel onCancelClick={() => setState("main")} onFinish={() => setState("main")} />; | ||
case "reset_identity_compromised": | ||
content = ( | ||
<ResetIdentityPanel | ||
variant="compromised" | ||
onCancelClick={() => setState("main")} | ||
onFinish={() => setState("main")} | ||
/> | ||
); | ||
break; | ||
case "reset_identity_forgot": | ||
content = ( | ||
<ResetIdentityPanel | ||
variant="forgot" | ||
onCancelClick={() => setState("main")} | ||
onFinish={() => setState("main")} | ||
/> | ||
); | ||
break; | ||
} | ||
|
||
|
@@ -111,7 +138,7 @@ export function EncryptionUserSettingsTab(): JSX.Element { | |
* @param setState - callback passed from the EncryptionUserSettingsTab to set the current `State`. | ||
* @returns a callback function, which will re-run the logic and update the state. | ||
*/ | ||
function useCheckEncryptionState(setState: (state: State) => void): () => Promise<void> { | ||
function useCheckEncryptionState(state: State, setState: (state: State) => void): () => Promise<void> { | ||
const matrixClient = useMatrixClientContext(); | ||
|
||
const checkEncryptionState = useCallback(async () => { | ||
|
@@ -129,8 +156,8 @@ function useCheckEncryptionState(setState: (state: State) => void): () => Promis | |
|
||
// Initialise the state when the component is mounted | ||
useEffect(() => { | ||
checkEncryptionState(); | ||
}, [checkEncryptionState]); | ||
if (state === "loading") checkEncryptionState(); | ||
}, [checkEncryptionState, state]); | ||
|
||
// Also return the callback so that the component can re-run the logic. | ||
return checkEncryptionState; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,10 @@ import GenericToast from "../components/views/toasts/GenericToast"; | |
import { ModuleRunner } from "../modules/ModuleRunner"; | ||
import { SetupEncryptionStore } from "../stores/SetupEncryptionStore"; | ||
import Spinner from "../components/views/elements/Spinner"; | ||
import { OpenToTabPayload } from "../dispatcher/payloads/OpenToTabPayload"; | ||
import { Action } from "../dispatcher/actions"; | ||
import { UserTab } from "../components/views/dialogs/UserTab"; | ||
import defaultDispatcher from "../dispatcher/dispatcher"; | ||
|
||
const TOAST_KEY = "setupencryption"; | ||
|
||
|
@@ -104,10 +108,6 @@ export enum Kind { | |
KEY_STORAGE_OUT_OF_SYNC = "key_storage_out_of_sync", | ||
} | ||
|
||
const onReject = (): void => { | ||
DeviceListener.sharedInstance().dismissEncryptionSetup(); | ||
}; | ||
|
||
/** | ||
* Show a toast prompting the user for some action related to setting up their encryption. | ||
* | ||
|
@@ -123,7 +123,7 @@ export const showToast = (kind: Kind): void => { | |
return; | ||
} | ||
|
||
const onAccept = async (): Promise<void> => { | ||
const onPrimaryClick = async (): Promise<void> => { | ||
if (kind === Kind.VERIFY_THIS_SESSION) { | ||
Modal.createDialog(SetupEncryptionDialog, {}, undefined, /* priority = */ false, /* static = */ true); | ||
} else { | ||
|
@@ -142,16 +142,29 @@ export const showToast = (kind: Kind): void => { | |
} | ||
}; | ||
|
||
const onSecondaryClick = (): void => { | ||
if (kind === Kind.KEY_STORAGE_OUT_OF_SYNC) { | ||
const payload: OpenToTabPayload = { | ||
action: Action.ViewUserSettings, | ||
initialTabId: UserTab.Encryption, | ||
props: { showResetIdentity: true }, | ||
}; | ||
defaultDispatcher.dispatch(payload); | ||
Comment on lines
+147
to
+152
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a comment in here might not go amiss. What does this payload actually do? |
||
} else { | ||
DeviceListener.sharedInstance().dismissEncryptionSetup(); | ||
} | ||
}; | ||
|
||
ToastStore.sharedInstance().addOrReplaceToast({ | ||
key: TOAST_KEY, | ||
title: getTitle(kind), | ||
icon: getIcon(kind), | ||
props: { | ||
description: getDescription(kind), | ||
primaryLabel: getSetupCaption(kind), | ||
onPrimaryClick: onAccept, | ||
onPrimaryClick, | ||
secondaryLabel: getSecondaryButtonLabel(kind), | ||
onSecondaryClick: onReject, | ||
onSecondaryClick, | ||
overrideWidth: kind === Kind.KEY_STORAGE_OUT_OF_SYNC ? "366px" : undefined, | ||
}, | ||
component: GenericToast, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(don't we already have some tests for this toast somewhere else?)