Skip to content
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

Add key storage toggle to Encryption settings #29113

Open
wants to merge 45 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
5a74298
Add key storage toggle to Encryption settings
dbkr Jan 27, 2025
98950de
Keys in the acceptable order
dbkr Jan 27, 2025
4eb07d4
Fix some tests
dbkr Jan 28, 2025
97057de
Fix import
dbkr Jan 28, 2025
4ec09f0
Fix toast showing condition
dbkr Jan 28, 2025
4db196e
Fix import order
dbkr Jan 28, 2025
98114c8
Fix playwright tests
dbkr Jan 28, 2025
6a20703
Merge remote-tracking branch 'origin/develop' into dbkr/key_storage_t…
dbkr Feb 5, 2025
b7b2ea3
Fix bits lost in merge
dbkr Feb 5, 2025
e408715
Add key storage delete confirm screen
dbkr Feb 5, 2025
aa6de76
Fix hardcoded Element string
dbkr Feb 5, 2025
5ac2004
Fix type imports
dbkr Feb 5, 2025
1304587
Fix tests
dbkr Feb 5, 2025
a26efc5
Tests for key storage delete panel
dbkr Feb 5, 2025
1b99071
Fix test
dbkr Feb 5, 2025
6b238d1
Type import
dbkr Feb 5, 2025
87d44a7
Test for the view model
dbkr Feb 6, 2025
4ea6a33
Merge remote-tracking branch 'origin/develop' into dbkr/key_storage_t…
dbkr Feb 6, 2025
df4c23b
Fix type import
dbkr Feb 6, 2025
4a3a373
Actually fix type imports
dbkr Feb 6, 2025
e70afdb
Test updating
dbkr Feb 6, 2025
40f9bd9
Add playwright test & clarify slightly confusing comment
dbkr Feb 6, 2025
16c76cb
Show the advnced section whatever the state of key storage
dbkr Feb 6, 2025
f818d6e
Update screenshots
dbkr Feb 6, 2025
fc9bc09
Copy css to its own file
dbkr Feb 7, 2025
25f8fe2
Add missing doc & merge loading states
dbkr Feb 7, 2025
1178d77
Add tsdoc & loading alt text to spinner
dbkr Feb 7, 2025
7c2d9f4
Turn comments into proper tsdoc
dbkr Feb 7, 2025
cfd55a6
Switch to TypedEventEmitter and remove unnecessary loading state
dbkr Feb 7, 2025
f586c43
Add screenshot
dbkr Feb 7, 2025
e8483e0
Use higher level interface
dbkr Feb 7, 2025
8ca4a8b
Merge the two hooks in EncryptionUserSettingsTab
dbkr Feb 7, 2025
2ef05c5
Remove unused import
dbkr Feb 7, 2025
7149b3d
Don't check key backup enabled state separately
dbkr Feb 7, 2025
64f84cb
Update snapshot
dbkr Feb 7, 2025
0c5f2b0
Merge branch 'develop' into dbkr/key_storage_toggle
dbkr Feb 7, 2025
9c4625d
Use fixed recovery key function
dbkr Feb 7, 2025
f0d9e05
Amalgamate duplicated CSS files
dbkr Feb 10, 2025
de18311
Merge remote-tracking branch 'origin/develop' into dbkr/key_storage_t…
dbkr Feb 10, 2025
c4525b9
Have "key storage disabled" as a separate state
dbkr Feb 10, 2025
d1aef9f
Update snapshot
dbkr Feb 10, 2025
96a70f2
Fix... bad merge?
dbkr Feb 10, 2025
98edffa
Add backup enabled mock to more tests
dbkr Feb 10, 2025
a5cec9e
More snapshots
dbkr Feb 10, 2025
0162c82
Use defer util
dbkr Feb 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions playwright/e2e/settings/encryption-user-tab/advanced.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ test.describe("Advanced section in Encryption tab", () => {
await expect(section.getByRole("button", { name: "Reset cryptographic identity" })).toBeVisible();

// After resetting the identity, the user should set up a new recovery key
await expect(
util.getEncryptionRecoverySection().getByRole("button", { name: "Set up recovery" }),
).toBeVisible();
await expect(page.getByRole("checkbox", { name: "Allow key storage" })).not.toBeChecked();

await checkDeviceIsCrossSigned(app);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,19 @@ test.describe("Encryption tab", () => {
// The user is prompted to reset their identity
await expect(dialog.getByText("Forgot your recovery key? You’ll need to reset your identity.")).toBeVisible();
});

test("should warn before turning off key storage", async ({ page, app, util }) => {
await verifySession(app, "new passphrase");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I opened #29214

await util.openEncryptionTab();

await page.getByRole("checkbox", { name: "Allow key storage" }).click();

await expect(
page.getByRole("heading", { name: "Are you sure you want to turn off key storage and delete it?" }),
).toBeVisible();
dbkr marked this conversation as resolved.
Show resolved Hide resolved

await page.getByRole("button", { name: "Delete key storage" }).click();

await expect(page.getByRole("checkbox", { name: "Allow key storage" })).not.toBeChecked();
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions res/css/_components.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
@import "./components/views/settings/devices/_FilteredDeviceListHeader.pcss";
@import "./components/views/settings/devices/_SecurityRecommendations.pcss";
@import "./components/views/settings/devices/_SelectableDeviceTile.pcss";
@import "./components/views/settings/encryption/_KeyStoragePanel.pcss";
@import "./components/views/settings/shared/_SettingsSubsection.pcss";
@import "./components/views/settings/shared/_SettingsSubsectionHeading.pcss";
@import "./components/views/spaces/_QuickThemeSwitcher.pcss";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.mx_KeyBackupPanel_toggleRow {
flex-direction: row;
}
7 changes: 5 additions & 2 deletions src/DeviceListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,11 @@ export default class DeviceListener {
// prompt the user to enter their recovery key.
showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC);
} else if (defaultKeyId === null) {
// the user just hasn't set up 4S yet: prompt them to do so
showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY);
// the user just hasn't set up 4S yet: prompt them to do so (unless they've explicitly said no to backups)
const disabledEvent = cli.getAccountData("m.org.matrix.custom.backup_disabled");
if (!disabledEvent || !disabledEvent.getContent()?.disabled) {
showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY);
}
} else {
// some other condition... yikes! Show the 'set up encryption' toast: this is what we previously did
// in 'other' situations. Possibly we should consider prompting for a full reset in this case?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
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 { useCallback, useEffect, useState } from "react";
import { logger } from "matrix-js-sdk/src/logger";

import { useMatrixClientContext } from "../../../../contexts/MatrixClientContext";

interface KeyStoragePanelState {
// 'null' means no backup is active. 'undefined' means we're still loading.
isEnabled: boolean | undefined;
dbkr marked this conversation as resolved.
Show resolved Hide resolved
setEnabled: (enable: boolean) => void;
loading: boolean; // true if the state is still loading for the first time
busy: boolean; // true if the status is in the process of being changed
dbkr marked this conversation as resolved.
Show resolved Hide resolved
}

export function useKeyStoragePanelViewModel(): KeyStoragePanelState {
const [isEnabled, setIsEnabled] = useState<boolean | undefined>(undefined);
const [loading, setLoading] = useState(true);
// Whilst the change is being made, the toggle will reflect the pending value rather than the actual state
const [pendingValue, setPendingValue] = useState<boolean | undefined>(undefined);

const matrixClient = useMatrixClientContext();

const checkStatus = useCallback(async () => {
const crypto = matrixClient.getCrypto();
if (!crypto) {
logger.error("Can't check key backup status: no crypto module available");
return;
}
const info = await crypto.getKeyBackupInfo();
setIsEnabled(Boolean(info?.version));
}, [matrixClient]);

useEffect(() => {
(async () => {
await checkStatus();
setLoading(false);
})();
}, [checkStatus]);

const setEnabled = useCallback(
async (enable: boolean) => {
setPendingValue(enable);
try {
const crypto = matrixClient.getCrypto();
if (!crypto) {
logger.error("Can't change key backup status: no crypto module available");
return;
}
if (enable) {
const currentKeyBackup = await crypto.checkKeyBackupAndEnable();
if (currentKeyBackup === null) {
await crypto.resetKeyBackup();
}

// resetKeyBackup fires this off in the background without waiting, so we need to do it
// explicitly and wait for it, otherwise it won't be enabled yet when we check again.
await crypto.checkKeyBackupAndEnable();

// Set the flag so that EX no longer thinks the user wants backup disabled
await matrixClient.setAccountData("m.org.matrix.custom.backup_disabled", { disabled: false });
} else {
// Get the key backup version we're using
const info = await crypto.getKeyBackupInfo();
if (!info?.version) {
logger.error("Can't delete key backup version: no version available");
return;
}

// Bye bye backup
await crypto.deleteKeyBackupVersion(info.version);

// also turn off 4S, since this is also storing keys on the server.
// Delete the cross signing keys from secret storage
await matrixClient.deleteAccountData("m.cross_signing.master");
await matrixClient.deleteAccountData("m.cross_signing.self_signing");
await matrixClient.deleteAccountData("m.cross_signing.user_signing");
// and the key backup key (we just turned it off anyway)
await matrixClient.deleteAccountData("m.megolm_backup.v1");

// Delete the key information
const defaultKeyEvent = matrixClient.getAccountData("m.secret_storage.default_key");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Genuine question, should we not use matrixClient.secretStorage.getDefaultKeyId() instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm - good question. We are dealing with the data directly which is why just reading it straight off felt appropriate, but I can certainly see that we ought to be using the interface if it's there. I think I'll change it.

if (defaultKeyEvent) {
if (defaultKeyEvent.getContent()?.key) {
await matrixClient.deleteAccountData(
`m.secret_storage.key.${defaultKeyEvent.getContent().key}`,
);
}
// ...and the default key pointer
await matrixClient.deleteAccountData("m.secret_storage.default_key");
}

// finally, set a flag to say that the user doesn't want key backup.
// Element X uses this to determine whether to set up automatically,
// so this will stop EX turning it back on spontaneously.
await matrixClient.setAccountData("m.org.matrix.custom.backup_disabled", { disabled: true });
}

await checkStatus();
} finally {
setPendingValue(undefined);
}
},
[setPendingValue, checkStatus, matrixClient],
);

return {
isEnabled: pendingValue ?? isEnabled,
setEnabled,
loading,
busy: pendingValue !== undefined,
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* 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 { Breadcrumb, Button, VisualList, VisualListItem } from "@vector-im/compound-web";
import CrossIcon from "@vector-im/compound-design-tokens/assets/web/icons/close";
import ErrorIcon from "@vector-im/compound-design-tokens/assets/web/icons/error";
import React, { useCallback, useState } from "react";

import { _t } from "../../../../languageHandler";
import { EncryptionCard } from "./EncryptionCard";
import { useKeyStoragePanelViewModel } from "../../../viewmodels/settings/encryption/KeyStoragePanelViewModel";
import SdkConfig from "../../../../SdkConfig";

interface ResetIdentityPanelProps {
onFinish: () => void;
dbkr marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Confirms that the user really wants to turn off and delete their key storage
*/
export function DeleteKeyStoragePanel({ onFinish }: ResetIdentityPanelProps): JSX.Element {
const { setEnabled } = useKeyStoragePanelViewModel();
const [busy, setBusy] = useState(false);

const onDeleteClick = useCallback(async () => {
setBusy(true);
try {
await setEnabled(false);
} finally {
setBusy(false);
}
onFinish();
}, [setEnabled, onFinish]);

return (
<>
<Breadcrumb
backLabel={_t("action|back")}
onBackClick={onFinish}
pages={[_t("settings|encryption|title"), _t("settings|encryption|delete_key_storage|breadcrumb_page")]}
onPageClick={onFinish}
/>
<EncryptionCard
Icon={ErrorIcon}
destructive={true}
title={_t("settings|encryption|delete_key_storage|title")}
className="mx_ResetIdentityPanel"
dbkr marked this conversation as resolved.
Show resolved Hide resolved
>
<div className="mx_ResetIdentityPanel_content">
{_t("settings|encryption|delete_key_storage|description")}
<VisualList>
<VisualListItem Icon={CrossIcon} destructive={true}>
{_t("settings|encryption|delete_key_storage|list_first")}
</VisualListItem>
<VisualListItem Icon={CrossIcon} destructive={true}>
{_t("settings|encryption|delete_key_storage|list_second", { brand: SdkConfig.get().brand })}
</VisualListItem>
</VisualList>
</div>
<div className="mx_ResetIdentityPanel_footer">
<Button destructive={true} onClick={onDeleteClick} disabled={busy}>
{_t("settings|encryption|delete_key_storage|confirm")}
</Button>
<Button kind="tertiary" onClick={onFinish}>
{_t("action|cancel")}
</Button>
</div>
</EncryptionCard>
</>
);
}
70 changes: 70 additions & 0 deletions src/components/views/settings/encryption/KeyStoragePanel.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* 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 React, { useCallback } from "react";
import { InlineField, InlineSpinner, Label, Root, ToggleControl } from "@vector-im/compound-web";

import type { FormEvent } from "react";
import { SettingsSection } from "../shared/SettingsSection";
import { _t } from "../../../../languageHandler";
import { SettingsHeader } from "../SettingsHeader";
import { useKeyStoragePanelViewModel } from "../../../viewmodels/settings/encryption/KeyStoragePanelViewModel";

interface Props {
onKeyStorageDisableClick: () => void;
dbkr marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* This component allows the user to set up or change their recovery key.
*/
export const KeyStoragePanel: React.FC<Props> = ({ onKeyStorageDisableClick }) => {
const { isEnabled, setEnabled, loading, busy } = useKeyStoragePanelViewModel();

const onKeyBackupChange = useCallback(
(e: FormEvent<HTMLInputElement>) => {
if (e.currentTarget.checked) {
setEnabled(true);
} else {
onKeyStorageDisableClick();
}
},
[setEnabled, onKeyStorageDisableClick],
);

if (loading) {
return <InlineSpinner />;
dbkr marked this conversation as resolved.
Show resolved Hide resolved
}

return (
<SettingsSection
legacy={false}
heading={
<SettingsHeader
hasRecommendedTag={isEnabled === false}
label={_t("settings|encryption|key_storage|title")}
/>
}
subHeading={_t("settings|encryption|key_storage|description", undefined, {
a: (sub) => (
<a href="https://element.io/help#encryption5" target="_blank" rel="noreferrer noopener">
{sub}
</a>
),
})}
>
<Root className="mx_KeyBackupPanel_toggleRow">
<InlineField
name="keyStorage"
control={<ToggleControl name="keyStorage" checked={isEnabled} onChange={onKeyBackupChange} />}
>
<Label>{_t("settings|encryption|key_storage|allow_key_storage")}</Label>
</InlineField>
{busy && <InlineSpinner />}
</Root>
</SettingsSection>
);
};
Loading
Loading