diff --git a/package.json b/package.json index d80b875632..c2b9cd8df1 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ "linkifyjs": "^4.3.3", "livekit-client": "2.22.0", "marked": "^18.0.9", - "matrix-js-sdk": "42.1.0", + "matrix-js-sdk": "42.3.0", "matrix-widget-api": "^1.18.0", "nanoid": "^6.0.1", "pdfjs-dist": "^6.2.108", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 953d999b4f..c7947b78a4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -69,7 +69,7 @@ importers: version: 1.0.3 '@sableclient/matrixrtc': specifier: ^0.1.0 - version: 0.1.0(livekit-client@2.22.0(@types/dom-mediacapture-record@1.0.22))(matrix-js-sdk@42.1.0) + version: 0.1.0(livekit-client@2.22.0(@types/dom-mediacapture-record@1.0.22))(matrix-js-sdk@42.3.0) '@sableclient/tauri-plugin-livekit-mobile': specifier: ^0.2.0 version: 0.2.0 @@ -224,8 +224,8 @@ importers: specifier: ^18.0.9 version: 18.0.9 matrix-js-sdk: - specifier: 42.1.0 - version: 42.1.0 + specifier: 42.3.0 + version: 42.3.0 matrix-widget-api: specifier: ^1.18.0 version: 1.18.0 @@ -4217,8 +4217,8 @@ packages: matrix-events-sdk@0.0.1: resolution: {integrity: sha512-1QEOsXO+bhyCroIe2/A5OwaxHvBm7EsSQ46DEDn8RBIfQwN5HWBpFvyWWR4QY0KHPPnnJdI99wgRiAl7Ad5qaA==} - matrix-js-sdk@42.1.0: - resolution: {integrity: sha512-/lK6XKRyYapgMUpFp0NyFQDnIt303yMNumk/1qBXRhzb43yV9fDPQZDcw++Yp5JcFiYLdFTZh8gS+AI7gxlghg==} + matrix-js-sdk@42.3.0: + resolution: {integrity: sha512-jXXFLlaA25zQ2oxXCgbAEubGC8KpF8OabSf387W0RXfkwqhNXEbU+veeUaZMT+bCbyE5nDxkN3iBy038VbntQg==} engines: {node: '>=22.0.0'} matrix-widget-api@1.18.0: @@ -6958,10 +6958,10 @@ snapshots: '@rollup/rollup-win32-x64-msvc@4.62.3': optional: true - '@sableclient/matrixrtc@0.1.0(livekit-client@2.22.0(@types/dom-mediacapture-record@1.0.22))(matrix-js-sdk@42.1.0)': + '@sableclient/matrixrtc@0.1.0(livekit-client@2.22.0(@types/dom-mediacapture-record@1.0.22))(matrix-js-sdk@42.3.0)': dependencies: livekit-client: 2.22.0(@types/dom-mediacapture-record@1.0.22) - matrix-js-sdk: 42.1.0 + matrix-js-sdk: 42.3.0 '@sableclient/sable-call-embedded@1.1.8': {} @@ -8962,7 +8962,7 @@ snapshots: matrix-events-sdk@0.0.1: {} - matrix-js-sdk@42.1.0: + matrix-js-sdk@42.3.0: dependencies: '@babel/runtime': 8.0.0 '@matrix-org/matrix-sdk-crypto-wasm': 18.4.0 diff --git a/src/app/components/DeviceVerification.tsx b/src/app/components/DeviceVerification.tsx index 9a2f5ce9fa..9308156c88 100644 --- a/src/app/components/DeviceVerification.tsx +++ b/src/app/components/DeviceVerification.tsx @@ -17,7 +17,6 @@ import { AsyncStatus, useAsyncCallback } from '$hooks/useAsyncCallback'; import { ContainerColor } from '$styles/ContainerColor.css'; import { ModalOverlay } from '$components/modal-overlay/ModalOverlay'; import { useMatrixClient } from '$hooks/useMatrixClient'; -import type { CryptoBackend } from '$types/matrix-sdk'; import { Button } from '$components/button'; const DialogHeaderStyles: CSSProperties = { @@ -95,8 +94,6 @@ function VerificationWaitStart() { ); } -const PENDING_REQUEST_POLL_MS = 2000; - type VerificationStartProps = { onStart: () => Promise; }; @@ -329,20 +326,18 @@ export function ReceiveSelfDeviceVerification() { ); useEffect(() => { - if (request) return undefined; - const crypto = mx.getCrypto() as CryptoBackend | undefined; + if (!mx.clientRunning) return undefined; + const crypto = mx.getCrypto(); if (!crypto?.getVerificationRequestsToDeviceInProgress) return undefined; - const adopt = () => { - const pending = crypto - .getVerificationRequestsToDeviceInProgress(mx.getSafeUserId()) - .find((candidate) => candidate.isSelfVerification && !candidate.initiatedByMe); - if (pending) setRequest(pending); - }; - adopt(); - const timer = setInterval(adopt, PENDING_REQUEST_POLL_MS); - return () => clearInterval(timer); - }, [mx, request]); + const pending = crypto + .getVerificationRequestsToDeviceInProgress(mx.getSafeUserId()) + .find( + (candidate) => candidate.isSelfVerification && !candidate.initiatedByMe && candidate.pending + ); + if (pending) setRequest(pending); + return undefined; + }, [mx]); const handleExit = useCallback(() => { setRequest(undefined); diff --git a/src/app/components/ReceiveSelfDeviceVerification.test.tsx b/src/app/components/ReceiveSelfDeviceVerification.test.tsx index ff53a014c1..2ba3f4eda1 100644 --- a/src/app/components/ReceiveSelfDeviceVerification.test.tsx +++ b/src/app/components/ReceiveSelfDeviceVerification.test.tsx @@ -7,14 +7,15 @@ const getVerificationRequestsToDeviceInProgress = vi.hoisted(() => vi.fn<(userId: string) => unknown[]>() ); const listeners = vi.hoisted(() => new Map void>()); - +const matrixClient = vi.hoisted(() => ({ + clientRunning: true, + getSafeUserId: () => '@me:example.org', + getCrypto: () => ({ getVerificationRequestsToDeviceInProgress }), + on: (event: string, handler: (request: unknown) => void) => listeners.set(event, handler), + removeListener: (event: string) => listeners.delete(event), +})); vi.mock('$hooks/useMatrixClient', () => ({ - useMatrixClient: () => ({ - getSafeUserId: () => '@me:example.org', - getCrypto: () => ({ getVerificationRequestsToDeviceInProgress }), - on: (event: string, handler: (request: unknown) => void) => listeners.set(event, handler), - removeListener: (event: string) => listeners.delete(event), - }), + useMatrixClient: () => matrixClient, })); vi.mock('$components/modal-overlay/ModalOverlay', () => ({ @@ -46,10 +47,13 @@ const renderReceiver = () => describe('ReceiveSelfDeviceVerification', () => { beforeEach(() => { vi.clearAllMocks(); + getVerificationRequestsToDeviceInProgress.mockReset(); + getVerificationRequestsToDeviceInProgress.mockReturnValue([]); + matrixClient.clientRunning = true; listeners.clear(); }); - it('shows a request that arrived before it was mounted', async () => { + it('shows a pending self-verification request that arrived before mount', async () => { getVerificationRequestsToDeviceInProgress.mockReturnValue([pendingRequest]); renderReceiver(); @@ -57,11 +61,18 @@ describe('ReceiveSelfDeviceVerification', () => { await waitFor(() => expect(screen.getByText('Device Verification')).toBeInTheDocument()); }); + it('shows an incoming self-verification request from the SDK event', async () => { + renderReceiver(); + listeners.get('crypto.verificationRequestReceived')?.(pendingRequest); + + await waitFor(() => expect(screen.getByText('Device Verification')).toBeInTheDocument()); + }); + it('does not treat unmounting as the user cancelling', async () => { const cancel = vi.fn<() => Promise>(async () => undefined); - getVerificationRequestsToDeviceInProgress.mockReturnValue([{ ...pendingRequest, cancel }]); const { unmount } = renderReceiver(); + listeners.get('crypto.verificationRequestReceived')?.({ ...pendingRequest, cancel }); await waitFor(() => expect(screen.getByText('Device Verification')).toBeInTheDocument()); expect( screen.getByText('Device Verification').closest('[data-deactivate-closes]') @@ -72,15 +83,53 @@ describe('ReceiveSelfDeviceVerification', () => { }); it('ignores a request this device started', async () => { - getVerificationRequestsToDeviceInProgress.mockReturnValue([ - { ...pendingRequest, initiatedByMe: true }, - ]); - renderReceiver(); + listeners.get('crypto.verificationRequestReceived')?.({ + ...pendingRequest, + initiatedByMe: true, + }); await new Promise((resolve) => { setTimeout(resolve, 20); }); expect(screen.queryByText('Device Verification')).toBeNull(); }); + + it('does not query a disposed crypto engine', () => { + matrixClient.clientRunning = false; + getVerificationRequestsToDeviceInProgress.mockImplementation(() => { + throw new Error('null pointer passed to rust'); + }); + + renderReceiver(); + + expect(getVerificationRequestsToDeviceInProgress).not.toHaveBeenCalled(); + }); + + it('does not poll after the client stops', () => { + vi.useFakeTimers(); + + renderReceiver(); + expect(getVerificationRequestsToDeviceInProgress).toHaveBeenCalledTimes(1); + + matrixClient.clientRunning = false; + getVerificationRequestsToDeviceInProgress.mockImplementation(() => { + throw new Error('null pointer passed to rust'); + }); + vi.advanceTimersByTime(6000); + + expect(getVerificationRequestsToDeviceInProgress).toHaveBeenCalledTimes(1); + vi.useRealTimers(); + }); + + it('ignores a completed request found in progress', async () => { + getVerificationRequestsToDeviceInProgress.mockReturnValue([ + { ...pendingRequest, pending: false }, + ]); + + renderReceiver(); + + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(screen.queryByText('Device Verification')).toBeNull(); + }); }); diff --git a/src/client/initMatrix.sdk.test.ts b/src/client/initMatrix.sdk.test.ts index 0325ba4419..9f47b3811e 100644 --- a/src/client/initMatrix.sdk.test.ts +++ b/src/client/initMatrix.sdk.test.ts @@ -2,6 +2,7 @@ import 'fake-indexeddb/auto'; import { afterEach, describe, expect, it, vi } from 'vitest'; import type { MatrixClient } from '$types/matrix-sdk'; import type { Session } from '$state/sessions'; +import { getSessionStoreName } from '$state/sessions'; import type * as MatrixSdkModule from 'matrix-js-sdk/lib/matrix'; const { isTauri, invoke, initRustCrypto } = vi.hoisted(() => ({ @@ -46,6 +47,50 @@ const session = (userId: string): Session => ({ accessToken: 'access-token', }); +const storeSentinel = async (name: string): Promise => + new Promise((resolve, reject) => { + const request = indexedDB.open(name, 1); + request.addEventListener('error', () => reject(request.error)); + request.addEventListener('upgradeneeded', () => { + request.result.createObjectStore('sentinels'); + }); + request.addEventListener('success', () => { + const database = request.result; + const transaction = database.transaction('sentinels', 'readwrite'); + transaction.objectStore('sentinels').put('preserved', 'crypto'); + transaction.addEventListener('complete', () => { + database.close(); + resolve(); + }); + transaction.addEventListener('error', () => { + database.close(); + reject(transaction.error); + }); + }); + }); + +const readSentinel = async (name: string): Promise => + new Promise((resolve, reject) => { + const request = indexedDB.open(name); + request.addEventListener('error', () => reject(request.error)); + request.addEventListener('success', () => { + const database = request.result; + const transaction = database.transaction('sentinels'); + const get = transaction.objectStore('sentinels').get('crypto'); + get.addEventListener('success', () => resolve(get.result)); + get.addEventListener('error', () => reject(get.error)); + transaction.addEventListener('complete', () => database.close()); + }); + }); + +const createDeferred = (): { promise: Promise; resolve: () => void } => { + let resolve!: () => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +}; + describe('initClient SDK crypto initialization', () => { afterEach(() => { vi.restoreAllMocks(); @@ -78,4 +123,62 @@ describe('initClient SDK crypto initialization', () => { expect(invoke.mock.calls.some(([command]) => command === 'engine_open')).toBe(false); expect(invoke.mock.calls.some(([command]) => command === 'engine_wipe')).toBe(false); }); + + it('preserves local stores when SDK crypto initialization reports an identity mismatch', async () => { + const mismatch = new Error("Account in the store doesn't match account in the constructor"); + const failedSession = session('@mismatch:example.org'); + const cryptoDatabase = `${getSessionStoreName(failedSession).rustCryptoPrefix}::matrix-sdk-crypto`; + await storeSentinel(cryptoDatabase); + initRustCrypto.mockRejectedValueOnce(mismatch); + const deleteDatabase = vi.spyOn(indexedDB, 'deleteDatabase'); + vi.stubGlobal('location', { reload: vi.fn<() => void>() }); + + await expect(initClient(failedSession)).rejects.toMatchObject({ + message: expect.stringContaining('Stored encryption keys'), + cause: mismatch, + }); + + expect(await readSentinel(cryptoDatabase)).toBe('preserved'); + expect(deleteDatabase).not.toHaveBeenCalled(); + expect(window.location.reload).not.toHaveBeenCalled(); + }); + + it('shares an in-flight SDK crypto initialization for the same session', async () => { + const cryptoStartup = createDeferred(); + initRustCrypto.mockImplementation(() => cryptoStartup.promise); + const pendingSession = session('@in-flight:example.org'); + + const clients = Promise.all([initClient(pendingSession), initClient({ ...pendingSession })]); + + await vi.waitFor(() => expect(initRustCrypto).toHaveBeenCalledTimes(1)); + cryptoStartup.resolve(); + + const [first, second] = await clients; + expect(first).toBe(second); + }); + + it('rejects a conflicting session while encrypted storage is initializing', async () => { + const cryptoStartup = createDeferred(); + initRustCrypto.mockImplementation(() => cryptoStartup.promise); + const pendingSession = session('@in-flight-conflict:example.org'); + const first = initClient(pendingSession); + + await vi.waitFor(() => expect(initRustCrypto).toHaveBeenCalledTimes(1)); + await expect( + initClient({ ...pendingSession, accessToken: 'different-access-token' }) + ).rejects.toThrow('A different session is already initializing encrypted storage'); + expect(initRustCrypto).toHaveBeenCalledTimes(1); + + cryptoStartup.resolve(); + await first; + }); + + it('allows a retry after an initialization failure', async () => { + const failedSession = session('@retry-after-failure:example.org'); + initRustCrypto.mockRejectedValueOnce(new Error('SDK startup failed')); + + await expect(initClient(failedSession)).rejects.toThrow('SDK startup failed'); + await expect(initClient({ ...failedSession })).resolves.toBeDefined(); + expect(initRustCrypto).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index 90f966bdc4..790bd28f57 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -68,6 +68,10 @@ const presenceSyncByClient = new WeakMap(); // application to prevent that, so track which client owns each store. const liveClientByCryptoStore = new Map(); const cryptoStoreByClient = new WeakMap(); +const inFlightClientInitializationByCryptoStore = new Map< + string, + { sessionIdentity: string; promise: Promise } +>(); export const getCryptoStoreOwner = (storeKey: string): MatrixClient | undefined => liveClientByCryptoStore.get(storeKey); @@ -374,6 +378,18 @@ const isMismatch = (err: unknown): boolean => { ); }; +const getSessionInitializationIdentity = (session: Session): string => + JSON.stringify({ + baseUrl: session.baseUrl, + userId: session.userId, + deviceId: session.deviceId, + accessToken: session.accessToken, + refreshToken: session.refreshToken, + fallbackSdkStores: session.fallbackSdkStores, + oidcIssuer: session.oidc?.issuer, + oidcClientId: session.oidc?.clientId, + }); + type BuiltClient = { mx: MatrixClient; indexedDBStore: IndexedDBStore; @@ -506,40 +522,13 @@ const initializeClient = async ( return { ok: true, mx }; }; -export const initClient = async (session: Session): Promise => { +const initializeSession = async (session: Session): Promise => { const storeName = getSessionStoreName(session); debugLog.info('sync', 'Initializing Matrix client', { userId: session.userId, baseUrl: session.baseUrl, }); - const wipeAllStores = async () => { - log.warn('initClient: wiping all stores for', session.userId); - debugLog.warn('sync', 'Wiping all stores due to mismatch', { - userId: session.userId, - }); - Sentry.addBreadcrumb({ - category: 'crypto', - message: 'Crypto store mismatch — wiping local stores and retrying', - level: 'warning', - }); - Sentry.metrics.count('sable.crypto.store_wipe', 1); - await deleteSessionStores(storeName); - try { - const allDbs = await window.indexedDB.databases(); - await Promise.all( - allDbs.map(async ({ name }) => { - if (name && name.includes(session.userId)) { - log.warn('initClient: also wiping db', name); - await deleteDatabase(name); - } - }) - ); - } catch { - // databases() not available in all browsers - } - }; - const initStartTime = performance.now(); let initOutcome = 'success'; try { @@ -554,14 +543,18 @@ export const initClient = async (session: Session): Promise => { throw result.error; } - log.warn(`initClient: mismatch during ${result.phase} — wiping and reloading:`, result.error); - debugLog.warn('sync', 'Client initialization mismatch - wiping stores and reloading', { + log.warn( + `initClient: mismatch during ${result.phase}; preserving local stores`, + result.error + ); + debugLog.warn('sync', 'Client initialization mismatch - preserving local stores', { phase: result.phase, error: result.error, }); - await wipeAllStores(); - window.location.reload(); - throw result.error; + throw new Error( + 'Stored encryption keys belong to a different session. Local data has been preserved.', + { cause: result.error } + ); } result.mx.setMaxListeners(50); @@ -580,6 +573,28 @@ export const initClient = async (session: Session): Promise => { } }; +export const initClient = (session: Session): Promise => { + const cryptoStoreKey = getSessionStoreName(session).rustCryptoPrefix; + const sessionIdentity = getSessionInitializationIdentity(session); + const inFlight = inFlightClientInitializationByCryptoStore.get(cryptoStoreKey); + if (inFlight) { + if (inFlight.sessionIdentity === sessionIdentity) return inFlight.promise; + return Promise.reject( + new Error( + 'A different session is already initializing encrypted storage. Retry after it finishes.' + ) + ); + } + + const promise = initializeSession(session).finally(() => { + if (inFlightClientInitializationByCryptoStore.get(cryptoStoreKey)?.promise === promise) { + inFlightClientInitializationByCryptoStore.delete(cryptoStoreKey); + } + }); + inFlightClientInitializationByCryptoStore.set(cryptoStoreKey, { sessionIdentity, promise }); + return promise; +}; + export type StartClientConfig = { baseUrl?: string; sessionSlidingSyncOptIn?: boolean;