From 2e895da39f5f5119ccf2631c9b181df5b9083afe Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 24 Sep 2024 16:48:37 +0100 Subject: [PATCH] Crypto: fix display of device key (#86) * CryptographyPanel: fix display of device key * CryptographPanel: Fix HTML nesting you're not supposed to put <tr> directly inside <table>; doing so causes warnings. * Update tests --- .../views/settings/CryptographyPanel.tsx | 67 +++++++++++++----- .../views/settings/CryptographyPanel-test.tsx | 36 +++++++++- .../SecurityUserSettingsTab-test.tsx.snap | 68 ++++++++++++------- test/test-utils/client.ts | 4 +- test/test-utils/test-utils.ts | 1 + 5 files changed, 129 insertions(+), 47 deletions(-) diff --git a/src/components/views/settings/CryptographyPanel.tsx b/src/components/views/settings/CryptographyPanel.tsx index b564a85208..3f3bbae5be 100644 --- a/src/components/views/settings/CryptographyPanel.tsx +++ b/src/components/views/settings/CryptographyPanel.tsx @@ -7,6 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import React from "react"; +import { logger } from "matrix-js-sdk/src/logger"; import type ExportE2eKeysDialog from "../../../async-components/views/dialogs/security/ExportE2eKeysDialog"; import type ImportE2eKeysDialog from "../../../async-components/views/dialogs/security/ImportE2eKeysDialog"; @@ -22,25 +23,53 @@ import SettingsSubsection, { SettingsSubsectionText } from "./shared/SettingsSub interface IProps {} -interface IState {} +interface IState { + /** The device's base64-encoded Ed25519 identity key, or: + * + * * `undefined`: not yet loaded + * * `null`: encryption is not supported (or the crypto stack was not correctly initialized) + */ + deviceIdentityKey: string | undefined | null; +} export default class CryptographyPanel extends React.Component<IProps, IState> { public constructor(props: IProps) { super(props); + + const client = MatrixClientPeg.safeGet(); + const crypto = client.getCrypto(); + if (!crypto) { + this.state = { deviceIdentityKey: null }; + } else { + this.state = { deviceIdentityKey: undefined }; + crypto + .getOwnDeviceKeys() + .then((keys) => { + this.setState({ deviceIdentityKey: keys.ed25519 }); + }) + .catch((e) => { + logger.error(`CryptographyPanel: Error fetching own device keys: ${e}`); + this.setState({ deviceIdentityKey: null }); + }); + } } public render(): React.ReactNode { const client = MatrixClientPeg.safeGet(); const deviceId = client.deviceId; - let identityKey = client.getDeviceEd25519Key(); - if (!identityKey) { + let identityKey = this.state.deviceIdentityKey; + if (identityKey === undefined) { + // Should show a spinner here really, but since this will be very transitional, I can't be doing with the + // necessary styling. + identityKey = "..."; + } else if (identityKey === null) { identityKey = _t("encryption|not_supported"); } else { identityKey = FormattingUtils.formatCryptoKey(identityKey); } let importExportButtons: JSX.Element | undefined; - if (client.isCryptoEnabled()) { + if (client.getCrypto()) { importExportButtons = ( <div className="mx_CryptographyPanel_importExportButtons"> <AccessibleButton kind="primary_outline" onClick={this.onExportE2eKeysClicked}> @@ -68,20 +97,22 @@ export default class CryptographyPanel extends React.Component<IProps, IState> { <SettingsSubsection heading={_t("settings|security|cryptography_section")}> <SettingsSubsectionText> <table className="mx_CryptographyPanel_sessionInfo"> - <tr> - <th scope="row">{_t("settings|security|session_id")}</th> - <td> - <code>{deviceId}</code> - </td> - </tr> - <tr> - <th scope="row">{_t("settings|security|session_key")}</th> - <td> - <code> - <b>{identityKey}</b> - </code> - </td> - </tr> + <tbody> + <tr> + <th scope="row">{_t("settings|security|session_id")}</th> + <td> + <code>{deviceId}</code> + </td> + </tr> + <tr> + <th scope="row">{_t("settings|security|session_key")}</th> + <td> + <code> + <b>{identityKey}</b> + </code> + </td> + </tr> + </tbody> </table> </SettingsSubsectionText> {importExportButtons} diff --git a/test/components/views/settings/CryptographyPanel-test.tsx b/test/components/views/settings/CryptographyPanel-test.tsx index a993829e37..388f27e62f 100644 --- a/test/components/views/settings/CryptographyPanel-test.tsx +++ b/test/components/views/settings/CryptographyPanel-test.tsx @@ -9,13 +9,15 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { render } from "@testing-library/react"; import { MatrixClient } from "matrix-js-sdk/src/matrix"; +import { mocked } from "jest-mock"; import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; import * as TestUtils from "../../../test-utils"; import CryptographyPanel from "../../../../src/components/views/settings/CryptographyPanel"; +import { flushPromises } from "../../../test-utils"; describe("CryptographyPanel", () => { - it("shows the session ID and key", () => { + it("shows the session ID and key", async () => { const sessionId = "ABCDEFGHIJ"; const sessionKey = "AbCDeFghIJK7L/m4nOPqRSTUVW4xyzaBCDef6gHIJkl"; const sessionKeyFormatted = "<b>AbCD eFgh IJK7 L/m4 nOPq RSTU VW4x yzaB CDef 6gHI Jkl</b>"; @@ -23,7 +25,8 @@ describe("CryptographyPanel", () => { TestUtils.stubClient(); const client: MatrixClient = MatrixClientPeg.safeGet(); client.deviceId = sessionId; - client.getDeviceEd25519Key = () => sessionKey; + + mocked(client.getCrypto()!.getOwnDeviceKeys).mockResolvedValue({ ed25519: sessionKey, curve25519: "1234" }); // When we render the CryptographyPanel const rendered = render(<CryptographyPanel />); @@ -32,6 +35,35 @@ describe("CryptographyPanel", () => { const codes = rendered.container.querySelectorAll("code"); expect(codes.length).toEqual(2); expect(codes[0].innerHTML).toEqual(sessionId); + + // Initially a placeholder + expect(codes[1].innerHTML).toEqual("<b>...</b>"); + + // Then the actual key + await flushPromises(); expect(codes[1].innerHTML).toEqual(sessionKeyFormatted); }); + + it("handles errors fetching session key", async () => { + const sessionId = "ABCDEFGHIJ"; + + TestUtils.stubClient(); + const client: MatrixClient = MatrixClientPeg.safeGet(); + client.deviceId = sessionId; + + mocked(client.getCrypto()!.getOwnDeviceKeys).mockRejectedValue(new Error("bleh")); + + // When we render the CryptographyPanel + const rendered = render(<CryptographyPanel />); + + // Then it displays info about the user's session + const codes = rendered.container.querySelectorAll("code"); + + // Initially a placeholder + expect(codes[1].innerHTML).toEqual("<b>...</b>"); + + // Then "not supported key + await flushPromises(); + expect(codes[1].innerHTML).toEqual("<b><not supported></b>"); + }); }); diff --git a/test/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap index 4ccca2a02c..86874f437e 100644 --- a/test/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap +++ b/test/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap @@ -314,32 +314,52 @@ exports[`<SecurityUserSettingsTab /> renders security section 1`] = ` <table class="mx_CryptographyPanel_sessionInfo" > - <tr> - <th - scope="row" - > - Session ID: - </th> - <td> - <code /> - </td> - </tr> - <tr> - <th - scope="row" - > - Session key: - </th> - <td> - <code> - <b> - <not supported> - </b> - </code> - </td> - </tr> + <tbody> + <tr> + <th + scope="row" + > + Session ID: + </th> + <td> + <code /> + </td> + </tr> + <tr> + <th + scope="row" + > + Session key: + </th> + <td> + <code> + <b> + ... + </b> + </code> + </td> + </tr> + </tbody> </table> </div> + <div + class="mx_CryptographyPanel_importExportButtons" + > + <div + class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary_outline" + role="button" + tabindex="0" + > + Export E2E room keys + </div> + <div + class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary_outline" + role="button" + tabindex="0" + > + Import E2E room keys + </div> + </div> <div class="mx_SettingsFlag" > diff --git a/test/test-utils/client.ts b/test/test-utils/client.ts index d4f2878004..19625043e1 100644 --- a/test/test-utils/client.ts +++ b/test/test-utils/client.ts @@ -135,7 +135,6 @@ export const mockClientMethodsDevice = ( deviceId = "test-device-id", ): Partial<Record<MethodLikeKeys<MatrixClient>, unknown>> => ({ getDeviceId: jest.fn().mockReturnValue(deviceId), - getDeviceEd25519Key: jest.fn(), getDevices: jest.fn().mockResolvedValue({ devices: [] }), }); @@ -164,10 +163,9 @@ export const mockClientMethodsCrypto = (): Partial< isSecretStorageReady: jest.fn(), getSessionBackupPrivateKey: jest.fn(), getVersion: jest.fn().mockReturnValue("Version 0"), - getOwnDeviceKeys: jest.fn(), + getOwnDeviceKeys: jest.fn().mockReturnValue(new Promise(() => {})), getCrossSigningKeyId: jest.fn(), }), - getDeviceEd25519Key: jest.fn(), }); export const mockClientMethodsRooms = (rooms: Room[] = []): Partial<Record<MethodLikeKeys<MatrixClient>, unknown>> => ({ diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 092f30fb52..b0c3dda279 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -124,6 +124,7 @@ export function createTestClient(): MatrixClient { }, }, getCrypto: jest.fn().mockReturnValue({ + getOwnDeviceKeys: jest.fn(), getUserDeviceInfo: jest.fn(), getUserVerificationStatus: jest.fn(), getDeviceVerificationStatus: jest.fn(),