From f2424c764d9ef7374ddd9a9c28fb46ac38dbb802 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Wed, 5 Aug 2026 11:19:03 +0200 Subject: [PATCH] fix: fix cross-signing crypto refresh race --- .../components/ManualVerification.test.tsx | 66 +++++++++++++++ src/app/components/ManualVerification.tsx | 4 +- .../hooks/useCrossSigningResetDetect.test.tsx | 80 +++++++++++++++++++ src/app/hooks/useCrossSigningResetDetect.ts | 37 ++++++++- 4 files changed, 182 insertions(+), 5 deletions(-) create mode 100644 src/app/components/ManualVerification.test.tsx create mode 100644 src/app/hooks/useCrossSigningResetDetect.test.tsx diff --git a/src/app/components/ManualVerification.test.tsx b/src/app/components/ManualVerification.test.tsx new file mode 100644 index 0000000000..b577172605 --- /dev/null +++ b/src/app/components/ManualVerification.test.tsx @@ -0,0 +1,66 @@ +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 type { SecretStorageKeyContent } from '$types/matrix/accountData'; +import { ManualVerificationTile } from './ManualVerification'; + +const decodeRecoveryKey = vi.hoisted(() => vi.fn<(key: string) => Uint8Array>()); +const checkKey = vi.hoisted(() => vi.fn<() => Promise>()); +const storePrivateKey = vi.hoisted(() => vi.fn<() => void>()); +const processDeviceLists = vi.hoisted(() => vi.fn<() => Promise>()); +const bootstrapCrossSigning = vi.hoisted(() => vi.fn<() => Promise>()); +const bootstrapSecretStorage = vi.hoisted(() => vi.fn<() => Promise>()); +const loadSessionBackupPrivateKeyFromSecretStorage = vi.hoisted(() => vi.fn<() => Promise>()); + +vi.mock('$types/matrix-sdk', () => ({ decodeRecoveryKey })); +vi.mock('$client/secretStorageKeys', () => ({ storePrivateKey })); +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => ({ + getSafeUserId: () => '@me:example.org', + secretStorage: { checkKey }, + getCrypto: () => + ({ + processDeviceLists, + bootstrapCrossSigning, + bootstrapSecretStorage, + loadSessionBackupPrivateKeyFromSecretStorage, + }) as unknown as CryptoApi, + }), +})); + +const KEY_ID = 'key-id'; +const KEY_CONTENT = { algorithm: 'm.secret_storage.v1.aes-hmac-sha2' } as SecretStorageKeyContent; +const recoveryKey = new Uint8Array([1, 2, 3]); + +const submitRecoveryKey = (value: string) => { + const input = document.querySelector('form input') as HTMLInputElement; + fireEvent.change(input, { target: { value } }); + const form = input.closest('form') as HTMLFormElement; + Object.defineProperty(form, input.name, { value: input, configurable: true }); + fireEvent.submit(form); +}; + +describe('ManualVerificationTile', () => { + beforeEach(() => { + vi.clearAllMocks(); + decodeRecoveryKey.mockReturnValue(recoveryKey); + checkKey.mockResolvedValue(true); + processDeviceLists.mockResolvedValue(undefined); + bootstrapCrossSigning.mockResolvedValue(undefined); + bootstrapSecretStorage.mockResolvedValue(undefined); + loadSessionBackupPrivateKeyFromSecretStorage.mockResolvedValue(undefined); + }); + + it('refreshes cross-signing public keys before importing the recovery key', async () => { + render( + + ); + + submitRecoveryKey('valid-key'); + + await waitFor(() => expect(screen.getByText('Device verified!')).toBeInTheDocument()); + expect(storePrivateKey).toHaveBeenCalledWith(KEY_ID, recoveryKey); + expect(processDeviceLists).toHaveBeenCalledWith({ changed: ['@me:example.org'] }); + expect(bootstrapCrossSigning).toHaveBeenCalledAfter(processDeviceLists); + }); +}); diff --git a/src/app/components/ManualVerification.tsx b/src/app/components/ManualVerification.tsx index 980623c015..9ced6cf2ae 100644 --- a/src/app/components/ManualVerification.tsx +++ b/src/app/components/ManualVerification.tsx @@ -5,6 +5,7 @@ import { Box, Text, Chip, PopOut, Menu, config, MenuItem, color } from 'folds'; import { CaretDown, sizedIcon } from '$components/icons/phosphor'; import FocusTrap from 'focus-trap-react'; import type { SecretStorageKeyContent } from '$types/matrix/accountData'; +import type { CryptoBackend } from '$types/matrix-sdk'; import { storePrivateKey } from '$client/secretStorageKeys'; import { stopPropagation } from '$utils/keyboard'; import { useMatrixClient } from '$hooks/useMatrixClient'; @@ -117,13 +118,14 @@ export function ManualVerificationTile({ const verifyAndRestoreBackup = useCallback( async (recoveryKey: Uint8Array) => { - const crypto = mx.getCrypto(); + const crypto = mx.getCrypto() as CryptoBackend | undefined; if (!crypto) { throw new Error('Unexpected Error! Crypto object not found.'); } storePrivateKey(secretStorageKeyId, recoveryKey); + await crypto.processDeviceLists({ changed: [mx.getSafeUserId()] }); await crypto.bootstrapCrossSigning({}); await crypto.bootstrapSecretStorage({}); diff --git a/src/app/hooks/useCrossSigningResetDetect.test.tsx b/src/app/hooks/useCrossSigningResetDetect.test.tsx new file mode 100644 index 0000000000..27115494ec --- /dev/null +++ b/src/app/hooks/useCrossSigningResetDetect.test.tsx @@ -0,0 +1,80 @@ +import { act, renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { MatrixClient, MatrixEvent } from '$types/matrix-sdk'; +import { useCrossSigningResetDetect } from './useCrossSigningResetDetect'; + +const { useAccountDataCallback } = vi.hoisted(() => ({ + useAccountDataCallback: + vi.fn<(mx: MatrixClient | undefined, callback: (event: MatrixEvent) => void) => void>(), +})); + +vi.mock('./useAccountDataCallback', () => ({ useAccountDataCallback })); + +const ownUserId = '@me:example.org'; +const crossSigningEvent = { getType: () => 'm.cross_signing.master' } as MatrixEvent; + +const getAccountDataCallback = () => { + const callback = useAccountDataCallback.mock.calls[0]?.[1]; + if (!callback) throw new Error('Expected an account data callback'); + return callback; +}; + +describe('useCrossSigningResetDetect', () => { + beforeEach(() => { + vi.useFakeTimers(); + useAccountDataCallback.mockClear(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('defers and coalesces cross-signing device list refreshes', async () => { + const processDeviceLists = vi + .fn<(...args: unknown[]) => Promise>() + .mockResolvedValue(undefined); + const mx = { + getCrypto: () => ({ processDeviceLists }), + getSafeUserId: () => ownUserId, + } as unknown as MatrixClient; + + renderHook(() => useCrossSigningResetDetect(mx)); + const onAccountData = getAccountDataCallback(); + + onAccountData(crossSigningEvent); + onAccountData(crossSigningEvent); + expect(processDeviceLists).not.toHaveBeenCalled(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + + expect(processDeviceLists).toHaveBeenCalledOnce(); + expect(processDeviceLists).toHaveBeenCalledWith({ changed: [ownUserId] }); + }); + + it('cancels a queued refresh when the client is replaced', () => { + const processDeviceLists = vi + .fn<(...args: unknown[]) => Promise>() + .mockResolvedValue(undefined); + const mx = { + getCrypto: () => ({ processDeviceLists }), + getSafeUserId: () => ownUserId, + } as unknown as MatrixClient; + const props: { client: MatrixClient | undefined } = { client: mx }; + + const { rerender } = renderHook( + ({ client }: { client: MatrixClient | undefined }) => useCrossSigningResetDetect(client), + { + initialProps: props, + } + ); + const onAccountData = getAccountDataCallback(); + onAccountData(crossSigningEvent); + + rerender({ client: undefined }); + act(() => vi.runAllTimers()); + + expect(processDeviceLists).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/hooks/useCrossSigningResetDetect.ts b/src/app/hooks/useCrossSigningResetDetect.ts index 21fdc65cf0..f9365fb45f 100644 --- a/src/app/hooks/useCrossSigningResetDetect.ts +++ b/src/app/hooks/useCrossSigningResetDetect.ts @@ -1,14 +1,43 @@ -import { useCallback } from 'react'; +import { useCallback, useEffect, useRef } from 'react'; import type { CryptoBackend, MatrixClient, MatrixEvent } from '$types/matrix-sdk'; import { useAccountDataCallback } from './useAccountDataCallback'; export const useCrossSigningResetDetect = (mx: MatrixClient | undefined) => { + const refreshScheduled = useRef(false); + const refreshTimer = useRef>(); + + useEffect( + () => () => { + if (refreshTimer.current === undefined) return; + globalThis.clearTimeout(refreshTimer.current); + refreshTimer.current = undefined; + refreshScheduled.current = false; + }, + [mx] + ); + const onAccountData = useCallback( (evt: MatrixEvent) => { if (!mx || !evt.getType().startsWith('m.cross_signing.')) return; - const crypto = mx.getCrypto() as CryptoBackend | undefined; - if (!crypto) return; - void crypto.processDeviceLists({ changed: [mx.getSafeUserId()] }); + if (refreshScheduled.current) return; + refreshScheduled.current = true; + + // Wait for sync to update crypto before refreshing device lists. + refreshTimer.current = globalThis.setTimeout(() => { + refreshTimer.current = undefined; + const crypto = mx.getCrypto() as CryptoBackend | undefined; + if (!crypto) { + refreshScheduled.current = false; + return; + } + + void crypto + .processDeviceLists({ changed: [mx.getSafeUserId()] }) + .catch(() => undefined) + .finally(() => { + refreshScheduled.current = false; + }); + }, 0); }, [mx] );