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

Enable key backup by default #28267

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions src/CreateCrossSigning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDia
async function canUploadKeysWithPasswordOnly(cli: MatrixClient): Promise<boolean> {
try {
await cli.uploadDeviceSigningKeys(undefined, {} as CrossSigningKeys);
// We should never get here: the server should always require
// UI auth to upload device signing keys. If we do, we upload
// no keys which would be a no-op.
// If we get here, it's because the server is allowing us to upload keys without
// auth the first time due to MSC3967. Therefore, yes, we can upload keys
// (with or without password, technically, but that's fine).
logger.log("uploadDeviceSigningKeys unexpectedly succeeded without UI auth!");
return false;
return true;
} catch (error) {
if (!(error instanceof MatrixError) || !error.data || !error.data.flows) {
logger.log("uploadDeviceSigningKeys advertised no flows!");
Expand Down
28 changes: 18 additions & 10 deletions src/DeviceListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,21 +295,29 @@ export default class DeviceListener {
await crypto.getUserDeviceInfo([cli.getSafeUserId()]);

// cross signing isn't enabled - nag to enable it
// There are 2 different toasts for:
// There are 3 different toasts for:
if (!(await crypto.getCrossSigningKeyId()) && (await crypto.userHasCrossSigningKeys())) {
// Cross-signing on account but this device doesn't trust the master key (verify this session)
// Toast 1. Cross-signing on account but this device doesn't trust the master key (verify this session)
showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION);
this.checkKeyBackupStatus();
} else {
// No cross-signing or key backup on account (set up encryption)
await cli.waitForClientWellKnown();
if (isSecureBackupRequired(cli) && isLoggedIn()) {
// If we're meant to set up, and Secure Backup is required,
// trigger the flow directly without a toast once logged in.
hideSetupEncryptionToast();
accessSecretStorage();
const backupInfo = await this.getKeyBackupInfo();
if (backupInfo) {
// Toast 2: Key backup is enabled but recovery (4S) is not set up: prompt user to set up recovery.
// Since we now enable key backup at registration time, this will be the common case for
// new users.
showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY);
} else {
showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION);
// Toast 3: No cross-signing or key backup on account (set up encryption)
await cli.waitForClientWellKnown();
if (isSecureBackupRequired(cli) && isLoggedIn()) {
// If we're meant to set up, and Secure Backup is required,
// trigger the flow directly without a toast once logged in.
hideSetupEncryptionToast();
accessSecretStorage();
} else {
showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION);
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/structures/auth/E2eSetup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix";

import AuthPage from "../../views/auth/AuthPage";
import CompleteSecurityBody from "../../views/auth/CompleteSecurityBody";
import CreateCrossSigningDialog from "../../views/dialogs/security/CreateCrossSigningDialog";
import InitialCryptoSetupDialog from "../../views/dialogs/security/InitialCryptoSetupDialog";

interface IProps {
matrixClient: MatrixClient;
Expand All @@ -25,7 +25,7 @@ export default class E2eSetup extends React.Component<IProps> {
return (
<AuthPage>
<CompleteSecurityBody>
<CreateCrossSigningDialog
<InitialCryptoSetupDialog
matrixClient={this.props.matrixClient}
onFinished={this.props.onFinished}
accountPassword={this.props.accountPassword}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,27 @@ interface Props {
}

/*
* Walks the user through the process of creating a cross-signing keys. In most
* cases, only a spinner is shown, but for more complex auth like SSO, the user
* may need to complete some steps to proceed.
* Walks the user through the process of creating a cross-signing keys and setting
* up message key backup. In most cases, only a spinner is shown, but for more
* complex auth like SSO, the user may need to complete some steps to proceed.
*/
const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => {
const InitialCryptoSetupDialog: React.FC<Props> = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => {
const [error, setError] = useState(false);

const bootstrapCrossSigning = useCallback(async () => {
const doSetup = useCallback(async () => {
const cryptoApi = matrixClient.getCrypto();
if (!cryptoApi) return;

setError(false);

try {
await createCrossSigning(matrixClient, tokenLogin, accountPassword);

const backupInfo = await matrixClient.getKeyBackupVersion();
if (backupInfo === null) {
await cryptoApi.resetKeyBackup();
}

onFinished(true);
} catch (e) {
if (tokenLogin) {
Expand All @@ -58,8 +64,8 @@ const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPasswo
}, [onFinished]);

useEffect(() => {
bootstrapCrossSigning();
}, [bootstrapCrossSigning]);
doSetup();
}, [doSetup]);

let content;
if (error) {
Expand All @@ -69,7 +75,7 @@ const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPasswo
<div className="mx_Dialog_buttons">
<DialogButtons
primaryButton={_t("action|retry")}
onPrimaryButtonClick={bootstrapCrossSigning}
onPrimaryButtonClick={doSetup}
onCancel={onCancel}
/>
</div>
Expand All @@ -96,4 +102,4 @@ const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPasswo
);
};

export default CreateCrossSigningDialog;
export default InitialCryptoSetupDialog;
3 changes: 3 additions & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,9 @@
"warning": "If you didn't remove the recovery method, an attacker may be trying to access your account. Change your account password and set a new recovery method immediately in Settings."
},
"reset_all_button": "Forgotten or lost all recovery methods? <a>Reset all</a>",
"set_up_recovery": "Set up recovery",
"set_up_recovery_later": "Not now",
"set_up_recovery_toast_title": "Set up recovery to protect your account",
"set_up_toast_description": "Safeguard against losing access to encrypted messages & data",
"set_up_toast_title": "Set up Secure Backup",
"setup_secure_backup": {
Expand Down
38 changes: 35 additions & 3 deletions src/toasts/SetupEncryptionToast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
Please see LICENSE files in the repository root for full details.
*/

import KeyboardIcon from "@vector-im/compound-design-tokens/assets/web/icons/key";
import { ComponentType } from "react";

import Modal from "../Modal";
import { _t } from "../languageHandler";
import DeviceListener from "../DeviceListener";
Expand All @@ -23,40 +26,69 @@ const getTitle = (kind: Kind): string => {
switch (kind) {
case Kind.SET_UP_ENCRYPTION:
return _t("encryption|set_up_toast_title");
case Kind.SET_UP_RECOVERY:
return _t("encryption|set_up_recovery_toast_title");
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verify_toast_title");
}
};

const getIcon = (kind: Kind): string => {
const getIcon = (kind: Kind): string | undefined => {
switch (kind) {
case Kind.SET_UP_ENCRYPTION:
return "secure_backup";
case Kind.SET_UP_RECOVERY:
return undefined;
case Kind.VERIFY_THIS_SESSION:
return "verification_warning";
}
};

// Gets the icon displayed on the prinary button
const getPrimaryIcon = (kind: Kind): ComponentType<React.SVGAttributes<SVGElement>> | undefined => {
switch (kind) {
case Kind.SET_UP_RECOVERY:
return KeyboardIcon;
default:
return undefined;
}
};

const getSetupCaption = (kind: Kind): string => {
switch (kind) {
case Kind.SET_UP_ENCRYPTION:
return _t("action|continue");
case Kind.SET_UP_RECOVERY:
return _t("encryption|set_up_recovery");
case Kind.VERIFY_THIS_SESSION:
return _t("action|verify");
}
};

const getSecondaryButtonLabel = (kind: Kind): string => {
switch (kind) {
case Kind.SET_UP_RECOVERY:
return _t("encryption|set_up_recovery_later");
case Kind.SET_UP_ENCRYPTION:
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verification|unverified_sessions_toast_reject");
}
};

const getDescription = (kind: Kind): string => {
switch (kind) {
case Kind.SET_UP_ENCRYPTION:
return _t("encryption|set_up_toast_description");
case Kind.SET_UP_RECOVERY:
return _t("encryption|set_up_recovery_toast_title");
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verify_toast_description");
}
};

export enum Kind {
SET_UP_ENCRYPTION = "set_up_encryption",
SET_UP_RECOVERY = "set_up_recovery",
VERIFY_THIS_SESSION = "verify_this_session",
}

Expand Down Expand Up @@ -101,9 +133,9 @@ export const showToast = (kind: Kind): void => {
description: getDescription(kind),
primaryLabel: getSetupCaption(kind),
onPrimaryClick: onAccept,
secondaryLabel: _t("encryption|verification|unverified_sessions_toast_reject"),
secondaryLabel: getSecondaryButtonLabel(kind),
onSecondaryClick: onReject,
destructive: "secondary",
PrimaryIcon: getPrimaryIcon(kind),
},
component: GenericToast,
priority: kind === Kind.VERIFY_THIS_SESSION ? 95 : 40,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import { mocked } from "jest-mock";
import { MatrixClient } from "matrix-js-sdk/src/matrix";

import { createCrossSigning } from "../../../../../src/CreateCrossSigning";
import CreateCrossSigningDialog from "../../../../../src/components/views/dialogs/security/CreateCrossSigningDialog";
import InitialCryptoSetupDialog from "../../../../../src/components/views/dialogs/security/InitialCryptoSetupDialog";
import { createTestClient } from "../../../../test-utils";

jest.mock("../../../../../src/CreateCrossSigning", () => ({
createCrossSigning: jest.fn(),
}));

describe("CreateCrossSigningDialog", () => {
describe("InitialCryptoSetupDialog", () => {
let client: MatrixClient;
let createCrossSigningResolve: () => void;
let createCrossSigningReject: (e: Error) => void;
Expand All @@ -43,7 +43,7 @@ describe("CreateCrossSigningDialog", () => {
const onFinished = jest.fn();

render(
<CreateCrossSigningDialog
<InitialCryptoSetupDialog
matrixClient={client}
accountPassword="hunter2"
tokenLogin={false}
Expand All @@ -61,7 +61,7 @@ describe("CreateCrossSigningDialog", () => {

it("should display an error if createCrossSigning fails", async () => {
render(
<CreateCrossSigningDialog
<InitialCryptoSetupDialog
matrixClient={client}
accountPassword="hunter2"
tokenLogin={false}
Expand All @@ -78,7 +78,7 @@ describe("CreateCrossSigningDialog", () => {
const onFinished = jest.fn();

render(
<CreateCrossSigningDialog
<InitialCryptoSetupDialog
matrixClient={client}
accountPassword="hunter2"
tokenLogin={true}
Expand All @@ -95,7 +95,7 @@ describe("CreateCrossSigningDialog", () => {
const onFinished = jest.fn();

render(
<CreateCrossSigningDialog
<InitialCryptoSetupDialog
matrixClient={client}
accountPassword="hunter2"
tokenLogin={false}
Expand All @@ -113,7 +113,7 @@ describe("CreateCrossSigningDialog", () => {

it("should retry when the retry button is clicked", async () => {
render(
<CreateCrossSigningDialog
<InitialCryptoSetupDialog
matrixClient={client}
accountPassword="hunter2"
tokenLogin={false}
Expand Down
1 change: 1 addition & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ export function createTestClient(): MatrixClient {
getLocalAliases: jest.fn().mockReturnValue([]),
uploadDeviceSigningKeys: jest.fn(),
isKeyBackupKeyStored: jest.fn().mockResolvedValue(null),
getKeyBackupVersion: jest.fn(),
} as unknown as MatrixClient;

client.reEmitter = new ReEmitter(client);
Expand Down
4 changes: 2 additions & 2 deletions test/unit-tests/DeviceListener-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,13 +352,13 @@ describe("DeviceListener", () => {
mockCrypto!.getCrossSigningKeyId.mockResolvedValue("abc");
});

it("shows set up encryption toast when user has a key backup available", async () => {
it("shows set up recovery toast when user has a key backup available", async () => {
// non falsy response
mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo);
await createAndStart();

expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith(
SetupEncryptionToast.Kind.SET_UP_ENCRYPTION,
SetupEncryptionToast.Kind.SET_UP_RECOVERY,
);
});
});
Expand Down
2 changes: 2 additions & 0 deletions test/unit-tests/components/structures/MatrixChat-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ describe("<MatrixChat />", () => {
whoami: jest.fn(),
logout: jest.fn(),
getDeviceId: jest.fn(),
getKeyBackupVersion: jest.fn(),
});
let mockClient: Mocked<MatrixClient>;
const serverConfig = {
Expand Down Expand Up @@ -1003,6 +1004,7 @@ describe("<MatrixChat />", () => {
userHasCrossSigningKeys: jest.fn().mockResolvedValue(false),
// This needs to not finish immediately because we need to test the screen appears
bootstrapCrossSigning: jest.fn().mockImplementation(() => bootstrapDeferred.promise),
resetKeyBackup: jest.fn(),
isEncryptionEnabledInRoom: jest.fn().mockResolvedValue(false),
};
loginClient.getCrypto.mockReturnValue(mockCrypto as any);
Expand Down
Loading