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

Remove "Upgrade your encryption" flow in CreateSecretStorageDialog #28290

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import React, { createRef } from "react";
import FileSaver from "file-saver";
import { logger } from "matrix-js-sdk/src/logger";
import { AuthDict, CrossSigningKeys, MatrixError, UIAFlow, UIAResponse } from "matrix-js-sdk/src/matrix";
import { GeneratedSecretStorageKey, KeyBackupInfo } from "matrix-js-sdk/src/crypto-api";
import { GeneratedSecretStorageKey } from "matrix-js-sdk/src/crypto-api";
import classNames from "classnames";
import CheckmarkIcon from "@vector-im/compound-design-tokens/assets/web/icons/check";

Expand Down Expand Up @@ -70,16 +70,6 @@ interface IState {
downloaded: boolean;
setPassphrase: boolean;

/** Information on the current key backup version, as returned by the server.
*
* `null` could mean any of:
* * we haven't yet requested the data from the server.
* * we were unable to reach the server.
* * the server returned key backup version data we didn't understand or was malformed.
* * there is actually no backup on the server.
*/
backupInfo: KeyBackupInfo | null;

// does the server offer a UI auth flow with just m.login.password
// for /keys/device_signing/upload?
canUploadKeysWithPasswordOnly: boolean | null;
Expand Down Expand Up @@ -131,15 +121,17 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
this.queryKeyUploadAuth();
}

const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.createSecretStorageKey();
const phase = keyFromCustomisations ? Phase.Loading : Phase.ChooseKeyPassphrase;

this.state = {
phase: Phase.Loading,
phase,
passPhrase: "",
passPhraseValid: false,
passPhraseConfirm: "",
copied: false,
downloaded: false,
setPassphrase: false,
backupInfo: null,
// does the server offer a UI auth flow with just m.login.password
// for /keys/device_signing/upload?
accountPasswordCorrect: null,
Expand All @@ -149,40 +141,15 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
accountPassword,
};

this.getInitialPhase();
}

private getInitialPhase(): void {
const keyFromCustomisations = ModuleRunner.instance.extensions.cryptoSetup.createSecretStorageKey();
if (keyFromCustomisations) {
logger.log("CryptoSetupExtension: Created key via extension, jumping to bootstrap step");
this.recoveryKey = {
privateKey: keyFromCustomisations,
};
this.bootstrapSecretStorage();
return;
}

this.fetchBackupInfo();
if (keyFromCustomisations) this.initExtension(keyFromCustomisations);
}

/**
* Attempt to get information on the current backup from the server, and update the state.
*
* Updates {@link IState.backupInfo} and set the phase to {@link Phase.ChooseKeyPassphrase} if successful.
*/
private async fetchBackupInfo(): Promise<void> {
try {
const cli = MatrixClientPeg.safeGet();
const backupInfo = await cli.getKeyBackupVersion();
this.setState({
phase: Phase.ChooseKeyPassphrase,
backupInfo,
});
} catch (e) {
console.error("Error fetching backup data from server", e);
this.setState({ phase: Phase.LoadError });
}
private initExtension(keyFromCustomisations: Uint8Array): void {
logger.log("CryptoSetupExtension: Created key via extension, jumping to bootstrap step");
this.recoveryKey = {
privateKey: keyFromCustomisations,
};
this.bootstrapSecretStorage();
}

private async queryKeyUploadAuth(): Promise<void> {
Expand Down Expand Up @@ -296,16 +263,28 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
};

private bootstrapSecretStorage = async (): Promise<void> => {
const cli = MatrixClientPeg.safeGet();
const crypto = cli.getCrypto()!;
const { forceReset } = this.props;

let backupInfo;
// First, we try to get the keybackup info
florianduros marked this conversation as resolved.
Show resolved Hide resolved
if (!forceReset) {
try {
this.setState({ phase: Phase.Loading });
backupInfo = await cli.getKeyBackupVersion();
} catch (e) {
logger.error("Error fetching backup data from server", e);
this.setState({ phase: Phase.LoadError });
return;
}
}

this.setState({
phase: Phase.Storing,
error: undefined,
});

const cli = MatrixClientPeg.safeGet();
const crypto = cli.getCrypto()!;

const { forceReset } = this.props;

try {
if (forceReset) {
logger.log("Forcing secret storage reset");
Expand All @@ -327,7 +306,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
});
await crypto.bootstrapSecretStorage({
createSecretStorageKey: async () => this.recoveryKey!,
setupNewKeyBackup: !this.state.backupInfo,
setupNewKeyBackup: !backupInfo,
});
}
await initialiseDehydration(true);
Expand All @@ -346,8 +325,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent<IProp
};

private onLoadRetryClick = (): void => {
this.setState({ phase: Phase.Loading });
this.fetchBackupInfo();
this.bootstrapSecretStorage();
};

private onShowKeyContinueClick = (): void => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ import React from "react";
import { mocked, MockedObject } from "jest-mock";
import { MatrixClient, MatrixError } from "matrix-js-sdk/src/matrix";
import { sleep } from "matrix-js-sdk/src/utils";
import { waitFor } from "@testing-library/dom";

import { filterConsole, flushPromises, stubClient } from "../../../../../test-utils";
import { filterConsole, stubClient } from "../../../../../test-utils";
import CreateSecretStorageDialog from "../../../../../../src/async-components/views/dialogs/security/CreateSecretStorageDialog";

describe("CreateSecretStorageDialog", () => {
Expand All @@ -41,13 +40,6 @@ describe("CreateSecretStorageDialog", () => {
return render(<CreateSecretStorageDialog onFinished={onFinished} {...props} />);
}

it("shows a loading spinner initially", async () => {
const { container } = renderComponent();
expect(screen.getByTestId("spinner")).toBeDefined();
expect(container).toMatchSnapshot();
await flushPromises();
});

it("handles the happy path", async () => {
const result = renderComponent();
await result.findByText(
Expand Down Expand Up @@ -81,13 +73,6 @@ describe("CreateSecretStorageDialog", () => {
await screen.findByText("Unable to set up secret storage");
});

it("when there is an error fetching the backup version handles the error sensibly", async () => {
mockClient.getKeyBackupVersion.mockRejectedValue(new Error("error"));
renderComponent();

await waitFor(() => expect(screen.queryByText("Unable to query secret storage status")).not.toBeNull());
});

describe("when there is an error fetching the backup version", () => {
filterConsole("Error fetching backup data from server");

Expand All @@ -97,9 +82,19 @@ describe("CreateSecretStorageDialog", () => {
});

const result = renderComponent();
// We go though the dialog until we have to get the key backup
await userEvent.click(result.getByRole("button", { name: "Continue" }));
await userEvent.click(screen.getByRole("button", { name: "Copy" }));
await userEvent.click(screen.getByRole("button", { name: "Continue" }));

// XXX the error message is... misleading.
await screen.findByText("Unable to query secret storage status");
expect(result.container).toMatchSnapshot();

// Now we can get the backup and we retry
mockClient.getKeyBackupVersion.mockRestore();
await userEvent.click(screen.getByRole("button", { name: "Retry" }));
await screen.findByText("Your keys are now being backed up from this device.");
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -217,46 +217,6 @@ exports[`CreateSecretStorageDialog handles the happy path 2`] = `
</div>
`;

exports[`CreateSecretStorageDialog shows a loading spinner initially 1`] = `
<div>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
<div
aria-labelledby="mx_BaseDialog_title"
class="mx_CreateSecretStorageDialog"
data-focus-lock-disabled="false"
role="dialog"
>
<div
class="mx_Dialog_header"
/>
<div>
<div>
<div
class="mx_Spinner"
>
<div
aria-label="Loading…"
class="mx_Spinner_icon"
data-testid="spinner"
role="progressbar"
style="width: 32px; height: 32px;"
/>
</div>
</div>
</div>
</div>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
</div>
`;

exports[`CreateSecretStorageDialog when there is an error fetching the backup version shows an error 1`] = `
<div>
<div
Expand Down
Loading