diff --git a/src/app/components/BackupRestore.test.tsx b/src/app/components/BackupRestore.test.tsx index d2ccb2e628..311df7acaa 100644 --- a/src/app/components/BackupRestore.test.tsx +++ b/src/app/components/BackupRestore.test.tsx @@ -9,7 +9,10 @@ import { BackupRestoreTile } from './BackupRestore'; const decodeRecoveryKey = vi.hoisted(() => vi.fn<(key: string) => Uint8Array>()); const emitter = new TypedEventEmitter void>>(); const mockClient = Object.assign(emitter, { - secretStorage: { checkKey: vi.fn<() => Promise>().mockResolvedValue(true) }, + secretStorage: { + checkKey: vi.fn<() => Promise>().mockResolvedValue(true), + get: vi.fn<(name: string) => Promise>().mockResolvedValue('stored-key'), + }, getSafeUserId: () => '@me:example.org', getDeviceId: () => 'DEVICE', }); diff --git a/src/app/components/BackupRestore.tsx b/src/app/components/BackupRestore.tsx index 97643286da..0ac32a0123 100644 --- a/src/app/components/BackupRestore.tsx +++ b/src/app/components/BackupRestore.tsx @@ -46,6 +46,7 @@ import { menuIcon, } from '$components/icons/phosphor'; import { InfoCard } from './info-card'; +import { restoreCrossSigningFromSecretStorage } from '$utils/matrix-crypto'; type BackupKeyRecoveryProps = { crypto: CryptoApi; @@ -70,7 +71,7 @@ function BackupKeyRecovery({ storePrivateKey(secretStorageKeyId, recoveryKey); await cryptoBackend.processDeviceLists({ changed: [mx.getSafeUserId()] }); - await cryptoBackend.bootstrapCrossSigning({}); + await restoreCrossSigningFromSecretStorage(mx, cryptoBackend); await cryptoBackend.bootstrapSecretStorage({}); // Emits KeyBackupDecryptionKeyCached, which drives the restore. diff --git a/src/app/components/DeviceVerification.tsx b/src/app/components/DeviceVerification.tsx index 34fdc796a0..ddf503971a 100644 --- a/src/app/components/DeviceVerification.tsx +++ b/src/app/components/DeviceVerification.tsx @@ -363,12 +363,23 @@ export function ReceiveSelfDeviceVerification() { const crypto = mx.getCrypto(); if (!crypto?.getVerificationRequestsToDeviceInProgress) return undefined; - const pending = crypto - .getVerificationRequestsToDeviceInProgress(mx.getSafeUserId()) - .find( - (candidate) => candidate.isSelfVerification && !candidate.initiatedByMe && candidate.pending - ); - if (pending) setRequest(pending); + // The OlmMachine can be freed between the clientRunning check and this call. + try { + const pending = crypto + .getVerificationRequestsToDeviceInProgress(mx.getSafeUserId()) + .find( + (candidate) => + candidate.isSelfVerification && !candidate.initiatedByMe && candidate.pending + ); + if (pending) setRequest(pending); + } catch (error) { + Sentry.addBreadcrumb({ + category: 'crypto', + message: 'Could not read in-progress verification requests', + level: 'warning', + data: { error: error instanceof Error ? error.message : String(error) }, + }); + } return undefined; }, [mx]); diff --git a/src/app/components/DeviceVerificationSetup.test.tsx b/src/app/components/DeviceVerificationSetup.test.tsx new file mode 100644 index 0000000000..33bc422b9e --- /dev/null +++ b/src/app/components/DeviceVerificationSetup.test.tsx @@ -0,0 +1,72 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { CryptoApi } from '$types/matrix-sdk'; +import { DeviceVerificationSetup } from './DeviceVerificationSetup'; + +const userHasCrossSigningKeys = vi.hoisted(() => vi.fn<() => Promise>()); +const createRecoveryKeyFromPassphrase = vi.hoisted(() => + vi.fn() +); +const bootstrapSecretStorage = vi.hoisted(() => vi.fn<() => Promise>()); +const bootstrapCrossSigning = vi.hoisted(() => vi.fn<() => Promise>()); +const resetKeyBackup = vi.hoisted(() => vi.fn<() => Promise>()); + +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => ({ + getSafeUserId: () => '@me:example.org', + getCrypto: () => + ({ + userHasCrossSigningKeys, + createRecoveryKeyFromPassphrase, + bootstrapSecretStorage, + bootstrapCrossSigning, + resetKeyBackup, + }) as unknown as CryptoApi, + }), +})); + +vi.mock('$client/secretStorageKeys', () => ({ clearSecretStorageKeys: vi.fn<() => void>() })); + +const submitSetup = () => { + const form = document.querySelector('form') as HTMLFormElement; + fireEvent.submit(form); +}; + +describe('DeviceVerificationSetup', () => { + beforeEach(() => { + vi.clearAllMocks(); + createRecoveryKeyFromPassphrase.mockResolvedValue({ + encodedPrivateKey: 'recovery-key', + privateKey: new Uint8Array([1, 2, 3]), + }); + bootstrapSecretStorage.mockResolvedValue(undefined); + bootstrapCrossSigning.mockResolvedValue(undefined); + resetKeyBackup.mockResolvedValue(undefined); + }); + + it('refuses to set up again when the account already has cross-signing keys', async () => { + userHasCrossSigningKeys.mockResolvedValue(true); + render( undefined} />); + + submitSetup(); + + await waitFor(() => + expect(screen.getByText(/already has device verification set up/)).toBeInTheDocument() + ); + expect(createRecoveryKeyFromPassphrase).not.toHaveBeenCalled(); + expect(bootstrapSecretStorage).not.toHaveBeenCalled(); + expect(bootstrapCrossSigning).not.toHaveBeenCalled(); + expect(resetKeyBackup).not.toHaveBeenCalled(); + }); + + it('sets up when the account has no cross-signing keys', async () => { + userHasCrossSigningKeys.mockResolvedValue(false); + render( undefined} />); + + submitSetup(); + + await waitFor(() => expect(bootstrapCrossSigning).toHaveBeenCalled()); + expect(resetKeyBackup).toHaveBeenCalled(); + expect(userHasCrossSigningKeys).toHaveBeenCalledWith('@me:example.org', true); + }); +}); diff --git a/src/app/components/DeviceVerificationSetup.tsx b/src/app/components/DeviceVerificationSetup.tsx index 1c1911b1bf..e11f5f7705 100644 --- a/src/app/components/DeviceVerificationSetup.tsx +++ b/src/app/components/DeviceVerificationSetup.tsx @@ -158,6 +158,12 @@ function SetupVerification({ onComplete, reset }: Readonly vi.fn<(key: string) => Uint8Array>()); const checkKey = vi.hoisted(() => vi.fn<() => Promise>()); +const getSecret = vi.hoisted(() => vi.fn<(name: string) => Promise>()); const storePrivateKey = vi.hoisted(() => vi.fn<() => void>()); const processDeviceLists = vi.hoisted(() => vi.fn<() => Promise>()); const bootstrapCrossSigning = vi.hoisted(() => vi.fn<() => Promise>()); @@ -19,7 +20,7 @@ vi.mock('$hooks/useMatrixClient', () => ({ useMatrixClient: () => ({ getSafeUserId: () => '@me:example.org', getDeviceId: () => 'DEVICE', - secretStorage: { checkKey }, + secretStorage: { checkKey, get: getSecret }, getCrypto: () => ({ processDeviceLists, @@ -54,6 +55,7 @@ describe('ManualVerificationTile', () => { vi.clearAllMocks(); decodeRecoveryKey.mockReturnValue(recoveryKey); checkKey.mockResolvedValue(true); + getSecret.mockResolvedValue('stored-key'); processDeviceLists.mockResolvedValue(undefined); bootstrapCrossSigning.mockResolvedValue(undefined); bootstrapSecretStorage.mockResolvedValue(undefined); @@ -84,4 +86,17 @@ describe('ManualVerificationTile', () => { await waitFor(() => expect(screen.getByText('Device verified!')).toBeInTheDocument()); expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ['device-verification'] }); }); + + it('does not bootstrap when the cross-signing keys are missing from secret storage', async () => { + getSecret.mockResolvedValue(undefined); + renderTile(new QueryClient()); + + submitRecoveryKey('valid-key'); + + await waitFor(() => + expect(screen.getByText(/Could not read your cross-signing keys/)).toBeInTheDocument() + ); + expect(bootstrapCrossSigning).not.toHaveBeenCalled(); + expect(bootstrapSecretStorage).not.toHaveBeenCalled(); + }); }); diff --git a/src/app/components/ManualVerification.tsx b/src/app/components/ManualVerification.tsx index 3765e4fe1d..a52c2fc7d8 100644 --- a/src/app/components/ManualVerification.tsx +++ b/src/app/components/ManualVerification.tsx @@ -11,6 +11,7 @@ import { storePrivateKey } from '$client/secretStorageKeys'; import { stopPropagation } from '$utils/keyboard'; import { useMatrixClient } from '$hooks/useMatrixClient'; import { useRefreshDeviceVerificationStatus } from '$hooks/useDeviceVerificationStatus'; +import { restoreCrossSigningFromSecretStorage } from '$utils/matrix-crypto'; import { AsyncStatus, useAsyncCallback } from '$hooks/useAsyncCallback'; import { AsyncError } from '$components/AsyncError'; import { SettingTile } from './setting-tile'; @@ -130,7 +131,7 @@ export function ManualVerificationTile({ storePrivateKey(secretStorageKeyId, recoveryKey); await crypto.processDeviceLists({ changed: [mx.getSafeUserId()] }); - await crypto.bootstrapCrossSigning({}); + await restoreCrossSigningFromSecretStorage(mx, crypto); await crypto.bootstrapSecretStorage({}); await crypto.loadSessionBackupPrivateKeyFromSecretStorage(); diff --git a/src/app/features/settings/devices/Devices.tsx b/src/app/features/settings/devices/Devices.tsx index 43cd48f461..deebd4371e 100644 --- a/src/app/features/settings/devices/Devices.tsx +++ b/src/app/features/settings/devices/Devices.tsx @@ -13,7 +13,7 @@ import { VerificationStatus, } from '$hooks/useDeviceVerificationStatus'; import { useSecretStorageDefaultKeyId, useSecretStorageKeyContent } from '$hooks/useSecretStorage'; -import { useCrossSigningActive } from '$hooks/useCrossSigning'; +import { CrossSigningStatus, useCrossSigningStatus } from '$hooks/useCrossSigning'; import { BackupRestoreTile } from '$components/BackupRestore'; import { LocalBackup } from './LocalBackup'; import { DeviceLogoutBtn, DeviceKeyDetails, DeviceTile, DeviceTilePlaceholder } from './DeviceTile'; @@ -41,7 +41,8 @@ type DevicesProps = { export function Devices({ requestBack, requestClose }: DevicesProps) { const mx = useMatrixClient(); const crypto = mx.getCrypto(); - const crossSigningActive = useCrossSigningActive(); + const crossSigningStatus = useCrossSigningStatus(); + const crossSigningActive = crossSigningStatus === CrossSigningStatus.Active; const [devices, refreshDeviceList] = useDeviceList(); useEffect(() => { @@ -90,7 +91,10 @@ export function Devices({ requestBack, requestClose }: DevicesProps) { description="To verify device identity and grant access to encrypted messages." after={ <> - + {crossSigningActive && ( setOpen(false), []); @@ -290,7 +291,13 @@ export function EnableVerification({ visible }: EnableVerificationProps) { return ( <> {visible && ( -