From 3da95063f7a8628dda662874d3b69a1d7340e7fd Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 18 Sep 2024 14:56:23 +0200 Subject: [PATCH 1/5] Replace `MatrixClient.keyBackupKeyFromRecoveryKey` by `decodeRecoveryKey` --- .../views/dialogs/security/AccessSecretStorageDialog.tsx | 3 ++- .../views/dialogs/security/RestoreKeyBackupDialog.tsx | 4 ++-- .../views/dialogs/AccessSecretStorageDialog-test.tsx | 5 ++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx index 3759e11063..0c4e875607 100644 --- a/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx +++ b/src/components/views/dialogs/security/AccessSecretStorageDialog.tsx @@ -10,6 +10,7 @@ import { debounce } from "lodash"; import classNames from "classnames"; import React, { ChangeEvent, FormEvent } from "react"; import { logger } from "matrix-js-sdk/src/logger"; +import { decodeRecoveryKey } from "matrix-js-sdk/src/crypto-api"; import { SecretStorage } from "matrix-js-sdk/src/matrix"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; @@ -100,7 +101,7 @@ export default class AccessSecretStorageDialog extends React.PureComponent { beforeEach(() => { mockClient = getMockClientWithEventEmitter({ - keyBackupKeyFromRecoveryKey: jest.fn(), checkSecretStorageKey: jest.fn(), isValidRecoveryKey: jest.fn(), }); @@ -88,8 +87,8 @@ describe("AccessSecretStorageDialog", () => { const checkPrivateKey = jest.fn().mockResolvedValue(true); renderComponent({ onFinished, checkPrivateKey }); - mockClient.keyBackupKeyFromRecoveryKey.mockImplementation(() => { - throw new Error("that's no key"); + mockClient.checkSecretStorageKey.mockImplementation(() => { + throw new Error("invalid key"); }); await enterSecurityKey(); From a529ef22d34bf3a5dd350ab850b04bbfd647671d Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 18 Sep 2024 15:15:48 +0200 Subject: [PATCH 2/5] Replace `MatrixClient.isValidRecoveryKey` by local check with `decodeRecoveryKey` --- .../security/RestoreKeyBackupDialog.tsx | 20 ++++++++++++++++--- .../AccessSecretStorageDialog-test.tsx | 3 --- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx b/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx index 9a53728e7b..c5963b966c 100644 --- a/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx +++ b/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx @@ -8,10 +8,10 @@ Please see LICENSE files in the repository root for full details. */ import React, { ChangeEvent } from "react"; -import { decodeRecoveryKey, MatrixClient, MatrixError, SecretStorage } from "matrix-js-sdk/src/matrix"; +import { MatrixClient, MatrixError, SecretStorage } from "matrix-js-sdk/src/matrix"; +import { decodeRecoveryKey, KeyBackupInfo } from "matrix-js-sdk/src/crypto-api"; import { IKeyBackupRestoreResult } from "matrix-js-sdk/src/crypto/keybackup"; import { logger } from "matrix-js-sdk/src/logger"; -import { KeyBackupInfo } from "matrix-js-sdk/src/crypto-api"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; import { _t } from "../../../../languageHandler"; @@ -118,10 +118,24 @@ export default class RestoreKeyBackupDialog extends React.PureComponent => {}, /* forceReset = */ true); }; + /** + * Check if the recovery key is valid + * @param recoveryKey + * @private + */ + private isValidRecoveryKey(recoveryKey: string): boolean { + try { + decodeRecoveryKey(recoveryKey); + return true; + } catch (e) { + return false; + } + } + private onRecoveryKeyChange = (e: ChangeEvent): void => { this.setState({ recoveryKey: e.target.value, - recoveryKeyValid: MatrixClientPeg.safeGet().isValidRecoveryKey(e.target.value), + recoveryKeyValid: this.isValidRecoveryKey(e.target.value), }); }; diff --git a/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx b/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx index 13d5c171a8..c07808ac11 100644 --- a/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx +++ b/test/components/views/dialogs/AccessSecretStorageDialog-test.tsx @@ -59,13 +59,11 @@ describe("AccessSecretStorageDialog", () => { beforeEach(() => { mockClient = getMockClientWithEventEmitter({ checkSecretStorageKey: jest.fn(), - isValidRecoveryKey: jest.fn(), }); }); it("Closes the dialog when the form is submitted with a valid key", async () => { mockClient.checkSecretStorageKey.mockResolvedValue(true); - mockClient.isValidRecoveryKey.mockReturnValue(true); const onFinished = jest.fn(); const checkPrivateKey = jest.fn().mockResolvedValue(true); @@ -114,7 +112,6 @@ describe("AccessSecretStorageDialog", () => { }; const checkPrivateKey = jest.fn().mockResolvedValue(false); renderComponent({ checkPrivateKey, keyInfo }); - mockClient.isValidRecoveryKey.mockReturnValue(false); await enterSecurityKey("Security Phrase"); expect(screen.getByPlaceholderText("Security Phrase")).toHaveValue(securityKey); From b0d8efd900049bdce0178ef92b79fc1d8865e8a6 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 18 Sep 2024 15:55:51 +0200 Subject: [PATCH 3/5] Replace old `decodeRecoveryKey` import --- src/SecurityManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index 6bfd361140..ccbc5fcfb3 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -7,8 +7,8 @@ Please see LICENSE files in the repository root for full details. */ import { ICryptoCallbacks, SecretStorage } from "matrix-js-sdk/src/matrix"; +import { decodeRecoveryKey } from "matrix-js-sdk/src/crypto-api"; import { deriveKey } from "matrix-js-sdk/src/crypto/key_passphrase"; -import { decodeRecoveryKey } from "matrix-js-sdk/src/crypto/recoverykey"; import { logger } from "matrix-js-sdk/src/logger"; import type CreateSecretStorageDialog from "./async-components/views/dialogs/security/CreateSecretStorageDialog"; From c4fc1740d40117a9f71d8806cc2da7ddfbcdd42c Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 18 Sep 2024 15:58:18 +0200 Subject: [PATCH 4/5] Remove `matrix-js-sdk/src/crypto/recoverykey` import of eslint exception --- .eslintrc.js | 1 - 1 file changed, 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index 444388d492..a2060e1f04 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -123,7 +123,6 @@ module.exports = { "!matrix-js-sdk/src/crypto/keybackup", "!matrix-js-sdk/src/crypto/deviceinfo", "!matrix-js-sdk/src/crypto/key_passphrase", - "!matrix-js-sdk/src/crypto/recoverykey", "!matrix-js-sdk/src/crypto/dehydration", "!matrix-js-sdk/src/oidc", "!matrix-js-sdk/src/oidc/discovery", From 75c08731f6abb7097fbb17f47f2378bbb683b53d Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 19 Sep 2024 16:16:34 +0200 Subject: [PATCH 5/5] Add tests for `RestoreKeyBackupDialog` --- .../security/RestoreKeyBackupDialog-test.tsx | 51 +++ .../RestoreKeyBackupDialog-test.tsx.snap | 298 ++++++++++++++++++ 2 files changed, 349 insertions(+) create mode 100644 test/components/views/dialogs/security/RestoreKeyBackupDialog-test.tsx create mode 100644 test/components/views/dialogs/security/__snapshots__/RestoreKeyBackupDialog-test.tsx.snap diff --git a/test/components/views/dialogs/security/RestoreKeyBackupDialog-test.tsx b/test/components/views/dialogs/security/RestoreKeyBackupDialog-test.tsx new file mode 100644 index 0000000000..3e52b473b6 --- /dev/null +++ b/test/components/views/dialogs/security/RestoreKeyBackupDialog-test.tsx @@ -0,0 +1,51 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * 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 from "react"; +import { screen, render, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +// Needed to be able to mock decodeRecoveryKey +// eslint-disable-next-line no-restricted-imports +import * as recoveryKeyModule from "matrix-js-sdk/src/crypto-api/recovery-key"; + +import RestoreKeyBackupDialog from "../../../../../src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx"; +import { stubClient } from "../../../../test-utils"; + +describe("", () => { + beforeEach(() => { + stubClient(); + jest.spyOn(recoveryKeyModule, "decodeRecoveryKey").mockReturnValue(new Uint8Array(32)); + }); + + it("should render", async () => { + const { asFragment } = render(); + await waitFor(() => expect(screen.getByText("Enter Security Key")).toBeInTheDocument()); + expect(asFragment()).toMatchSnapshot(); + }); + + it("should display an error when recovery key is invalid", async () => { + jest.spyOn(recoveryKeyModule, "decodeRecoveryKey").mockImplementation(() => { + throw new Error("Invalid recovery key"); + }); + const { asFragment } = render(); + await waitFor(() => expect(screen.getByText("Enter Security Key")).toBeInTheDocument()); + + await userEvent.type(screen.getByRole("textbox"), "invalid key"); + await waitFor(() => expect(screen.getByText("👎 Not a valid Security Key")).toBeInTheDocument()); + expect(asFragment()).toMatchSnapshot(); + }); + + it("should not raise an error when recovery is valid", async () => { + const { asFragment } = render(); + await waitFor(() => expect(screen.getByText("Enter Security Key")).toBeInTheDocument()); + + await userEvent.type(screen.getByRole("textbox"), "valid key"); + await waitFor(() => expect(screen.getByText("👍 This looks like a valid Security Key!")).toBeInTheDocument()); + expect(asFragment()).toMatchSnapshot(); + }); +}); diff --git a/test/components/views/dialogs/security/__snapshots__/RestoreKeyBackupDialog-test.tsx.snap b/test/components/views/dialogs/security/__snapshots__/RestoreKeyBackupDialog-test.tsx.snap new file mode 100644 index 0000000000..de0bddbe33 --- /dev/null +++ b/test/components/views/dialogs/security/__snapshots__/RestoreKeyBackupDialog-test.tsx.snap @@ -0,0 +1,298 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` should display an error when recovery key is invalid 1`] = ` + +
+