diff --git a/.changeset/fix-backup-key-recovery-verified-device.md b/.changeset/fix-backup-key-recovery-verified-device.md new file mode 100644 index 000000000..f6d7a5bee --- /dev/null +++ b/.changeset/fix-backup-key-recovery-verified-device.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +# Fix being unable to restore key backup on a verified device diff --git a/src/app/components/BackupRestore.test.tsx b/src/app/components/BackupRestore.test.tsx new file mode 100644 index 000000000..de7257b9e --- /dev/null +++ b/src/app/components/BackupRestore.test.tsx @@ -0,0 +1,100 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { TypedEventEmitter } from 'matrix-js-sdk/lib/models/typed-event-emitter'; +import type { CryptoApi, KeyBackupInfo } from '$types/matrix-sdk'; +import type { SecretStorageKeyContent } from '$types/matrix/accountData'; +import { BackupRestoreTile } from './BackupRestore'; + +const emitter = new TypedEventEmitter void>>(); +const mockClient = Object.assign(emitter, { + secretStorage: { checkKey: vi.fn<() => Promise>().mockResolvedValue(true) }, +}); + +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => mockClient, +})); + +vi.mock('$client/secretStorageKeys', () => ({ + storePrivateKey: vi.fn<() => void>(), +})); + +const KEY_ID = 'key-id'; +const KEY_CONTENT = { algorithm: 'm.secret_storage.v1.aes-hmac-sha2' } as SecretStorageKeyContent; +const BACKUP_INFO = { version: '3', count: 42 } as KeyBackupInfo; + +type CryptoOverrides = { + backupInfo?: KeyBackupInfo | null; + backupKey?: Uint8Array | null; +}; + +const createCrypto = ({ backupInfo = BACKUP_INFO, backupKey = null }: CryptoOverrides = {}) => + ({ + getActiveSessionBackupVersion: vi + .fn() + .mockResolvedValue(backupInfo ? '3' : null), + getKeyBackupInfo: vi.fn().mockResolvedValue(backupInfo), + getSessionBackupPrivateKey: vi + .fn() + .mockResolvedValue(backupKey), + isKeyBackupTrusted: vi + .fn() + .mockResolvedValue({ trusted: true, matchesDecryptionKey: true }), + restoreKeyBackup: vi.fn<() => Promise>().mockResolvedValue(undefined), + loadSessionBackupPrivateKeyFromSecretStorage: vi + .fn() + .mockResolvedValue(undefined), + }) as unknown as CryptoApi; + +const recoveryPrompt = () => screen.queryByText(/does not hold the backup decryption key/i); + +describe('BackupRestoreTile', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('offers recovery when a backup exists but its key is not in the crypto store', async () => { + render( + + ); + + await waitFor(() => expect(recoveryPrompt()).not.toBeNull()); + expect(screen.getByText('Recovery Key')).toBeTruthy(); + }); + + it('stays out of the way once the key is cached', async () => { + render( + + ); + + await waitFor(() => expect(screen.getByText('Encryption Backup')).toBeTruthy()); + expect(recoveryPrompt()).toBeNull(); + }); + + it('does not offer recovery when the server has no backup at all', async () => { + render( + + ); + + await waitFor(() => expect(screen.getByText(/No backup present on server/i)).toBeTruthy()); + expect(recoveryPrompt()).toBeNull(); + }); + + it('cannot offer recovery without secret storage configured', async () => { + render(); + + await waitFor(() => expect(screen.getByText('Encryption Backup')).toBeTruthy()); + expect(recoveryPrompt()).toBeNull(); + }); +}); diff --git a/src/app/components/BackupRestore.tsx b/src/app/components/BackupRestore.tsx index a16ed784d..515277942 100644 --- a/src/app/components/BackupRestore.tsx +++ b/src/app/components/BackupRestore.tsx @@ -1,12 +1,13 @@ import type { MouseEventHandler } from 'react'; import { useCallback, useState } from 'react'; -import { useAtom } from 'jotai'; +import { useAtom, useAtomValue } from 'jotai'; import type { CryptoApi, KeyBackupInfo } from '$types/matrix-sdk'; import type { RectCords } from 'folds'; import { Badge, Box, Button, + Chip, color, config, IconButton, @@ -18,14 +19,23 @@ import { Text, } from 'folds'; import FocusTrap from 'focus-trap-react'; -import { BackupProgressStatus, backupRestoreProgressAtom } from '$state/backupRestore'; +import type { SecretStorageKeyContent } from '$types/matrix/accountData'; +import { storePrivateKey } from '$client/secretStorageKeys'; +import { + BackupProgressStatus, + backupRestoreErrorAtom, + backupRestoreProgressAtom, + isMissingBackupKeyError, +} from '$state/backupRestore'; import { AsyncStatus, useAsyncCallback } from '$hooks/useAsyncCallback'; import { useKeyBackupInfo, useKeyBackupStatus, useKeyBackupSync, useKeyBackupTrust, + useSessionBackupKeyUsable, } from '$hooks/useKeyBackup'; +import { SecretStorageKeyMethod, SecretStorageKeyPrompt } from './SecretStorage'; import { stopPropagation } from '$utils/keyboard'; import { useRestoreBackupOnVerification } from '$hooks/useRestoreBackupOnVerification'; import { @@ -36,6 +46,75 @@ import { } from '$components/icons/phosphor'; import { InfoCard } from './info-card'; +type BackupKeyRecoveryProps = { + crypto: CryptoApi; + secretStorageKeyId: string; + secretStorageKeyContent: SecretStorageKeyContent; +}; +function BackupKeyRecovery({ + crypto, + secretStorageKeyId, + secretStorageKeyContent, +}: BackupKeyRecoveryProps) { + const hasPassphrase = !!secretStorageKeyContent.passphrase; + const [method, setMethod] = useState( + hasPassphrase ? SecretStorageKeyMethod.RecoveryPassphrase : SecretStorageKeyMethod.RecoveryKey + ); + + const [unlockState, unlockBackup] = useAsyncCallback( + useCallback( + async (recoveryKey: Uint8Array) => { + storePrivateKey(secretStorageKeyId, recoveryKey); + // Emits KeyBackupDecryptionKeyCached, which drives the restore. + await crypto.loadSessionBackupPrivateKeyFromSecretStorage(); + }, + [crypto, secretStorageKeyId] + ) + ); + + const otherMethod = + method === SecretStorageKeyMethod.RecoveryPassphrase + ? SecretStorageKeyMethod.RecoveryKey + : SecretStorageKeyMethod.RecoveryPassphrase; + + return ( + + + This device does not hold the backup decryption key. Provide your recovery details to unlock + the backup and restore your message history. + + + {hasPassphrase && ( + + setMethod(otherMethod)} + > + + {otherMethod === SecretStorageKeyMethod.RecoveryPassphrase + ? 'Use Recovery Passphrase' + : 'Use Recovery Key'} + + + + )} + {unlockState.status === AsyncStatus.Error && ( + + {unlockState.error.message} + + )} + + ); +} + type BackupStatusProps = { enabled: boolean; }; @@ -138,15 +217,23 @@ function BackupTrustInfo({ crypto, backupInfo }: BackupTrustInfoProps) { type BackupRestoreTileProps = { crypto: CryptoApi; + secretStorageKeyId?: string; + secretStorageKeyContent?: SecretStorageKeyContent; }; -export function BackupRestoreTile({ crypto }: BackupRestoreTileProps) { +export function BackupRestoreTile({ + crypto, + secretStorageKeyId, + secretStorageKeyContent, +}: BackupRestoreTileProps) { const [restoreProgress, setRestoreProgress] = useAtom(backupRestoreProgressAtom); + const autoRestoreError = useAtomValue(backupRestoreErrorAtom); const restoring = restoreProgress.status === BackupProgressStatus.Fetching || restoreProgress.status === BackupProgressStatus.Loading; const backupEnabled = useKeyBackupStatus(crypto); const backupInfo = useKeyBackupInfo(crypto); + const backupKeyUsable = useSessionBackupKeyUsable(crypto); const [remainingSession, syncFailure] = useKeyBackupSync(); const [menuCords, setMenuCords] = useState(); @@ -170,6 +257,13 @@ export function BackupRestoreTile({ crypto }: BackupRestoreTileProps) { restoreBackup(); }; + // backupKeyUsable is the structural signal; the error match only covers a + // restore that failed for this reason before the lookup settled. + const needsBackupKey = + !!backupInfo && + (backupKeyUsable === false || + (restoreState.status === AsyncStatus.Error && isMissingBackupKeyError(restoreState.error))); + return ( {restoreState.error.message} )} + {autoRestoreError && ( + + {autoRestoreError} + + )} + {needsBackupKey && secretStorageKeyId && secretStorageKeyContent && ( + + )} ); } diff --git a/src/app/components/ManualVerification.tsx b/src/app/components/ManualVerification.tsx index 5ce90cc6e..980623c01 100644 --- a/src/app/components/ManualVerification.tsx +++ b/src/app/components/ManualVerification.tsx @@ -11,15 +11,11 @@ import { useMatrixClient } from '$hooks/useMatrixClient'; import { AsyncStatus, useAsyncCallback } from '$hooks/useAsyncCallback'; import { AsyncError } from '$components/AsyncError'; import { SettingTile } from './setting-tile'; -import { SecretStorageRecoveryKey, SecretStorageRecoveryPassphrase } from './SecretStorage'; +import { SecretStorageKeyMethod, SecretStorageKeyPrompt } from './SecretStorage'; -enum ManualVerificationMethod { - RecoveryPassphrase = 'passphrase', - RecoveryKey = 'key', -} type ManualVerificationMethodSwitcherProps = { - value: ManualVerificationMethod; - onChange: (value: ManualVerificationMethod) => void; + value: SecretStorageKeyMethod; + onChange: (value: SecretStorageKeyMethod) => void; }; function ManualVerificationMethodSwitcher({ value, @@ -31,7 +27,7 @@ function ManualVerificationMethodSwitcher({ setMenuCords(evt.currentTarget.getBoundingClientRect()); }; - const handleSelect = (method: ManualVerificationMethod) => { + const handleSelect = (method: SecretStorageKeyMethod) => { setMenuCords(undefined); onChange(method); }; @@ -47,8 +43,8 @@ function ManualVerificationMethodSwitcher({ onClick={handleMenu} > - {value === ManualVerificationMethod.RecoveryPassphrase && 'Recovery Passphrase'} - {value === ManualVerificationMethod.RecoveryKey && 'Recovery Key'} + {value === SecretStorageKeyMethod.RecoveryPassphrase && 'Recovery Passphrase'} + {value === SecretStorageKeyMethod.RecoveryKey && 'Recovery Key'} handleSelect(ManualVerificationMethod.RecoveryPassphrase)} + onClick={() => handleSelect(SecretStorageKeyMethod.RecoveryPassphrase)} > Recovery Passphrase @@ -85,9 +81,9 @@ function ManualVerificationMethodSwitcher({ handleSelect(ManualVerificationMethod.RecoveryKey)} + onClick={() => handleSelect(SecretStorageKeyMethod.RecoveryKey)} > Recovery Key @@ -116,9 +112,7 @@ export function ManualVerificationTile({ const hasPassphrase = !!secretStorageKeyContent.passphrase; const [method, setMethod] = useState( - hasPassphrase - ? ManualVerificationMethod.RecoveryPassphrase - : ManualVerificationMethod.RecoveryKey + hasPassphrase ? SecretStorageKeyMethod.RecoveryPassphrase : SecretStorageKeyMethod.RecoveryKey ); const verifyAndRestoreBackup = useCallback( @@ -163,22 +157,12 @@ export function ManualVerificationTile({ ) : ( - {method === ManualVerificationMethod.RecoveryKey && ( - - )} - {method === ManualVerificationMethod.RecoveryPassphrase && - secretStorageKeyContent.passphrase && ( - - )} + )} diff --git a/src/app/components/SecretStorage.tsx b/src/app/components/SecretStorage.tsx index d40893723..1b63d1087 100644 --- a/src/app/components/SecretStorage.tsx +++ b/src/app/components/SecretStorage.tsx @@ -13,6 +13,11 @@ import { useAlive } from '$hooks/useAlive'; import { PasswordInput } from './password-input'; import { Button } from '$components/button'; +export enum SecretStorageKeyMethod { + RecoveryPassphrase = 'passphrase', + RecoveryKey = 'key', +} + type SecretStorageRecoveryPassphraseProps = { processing?: boolean; keyContent: SecretStorageKeyContent; @@ -113,6 +118,38 @@ export function SecretStorageRecoveryPassphrase({ ); } +type SecretStorageKeyPromptProps = { + method: SecretStorageKeyMethod; + processing?: boolean; + keyContent: SecretStorageKeyContent; + onDecodedRecoveryKey: (recoveryKey: Uint8Array) => void; +}; +export function SecretStorageKeyPrompt({ + method, + processing, + keyContent, + onDecodedRecoveryKey, +}: SecretStorageKeyPromptProps) { + if (method === SecretStorageKeyMethod.RecoveryPassphrase && keyContent.passphrase) { + return ( + + ); + } + + return ( + + ); +} + type SecretStorageRecoveryKeyProps = { processing?: boolean; keyContent: SecretStorageKeyContent; diff --git a/src/app/features/settings/devices/Devices.tsx b/src/app/features/settings/devices/Devices.tsx index b430b89b1..128636784 100644 --- a/src/app/features/settings/devices/Devices.tsx +++ b/src/app/features/settings/devices/Devices.tsx @@ -121,7 +121,11 @@ export function Devices({ requestBack, requestClose }: DevicesProps) { /> )} {crypto && verificationStatus === VerificationStatus.Verified && ( - + )} ) : ( diff --git a/src/app/hooks/useKeyBackup.test.tsx b/src/app/hooks/useKeyBackup.test.tsx new file mode 100644 index 000000000..361bf4a6e --- /dev/null +++ b/src/app/hooks/useKeyBackup.test.tsx @@ -0,0 +1,89 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { TypedEventEmitter } from 'matrix-js-sdk/lib/models/typed-event-emitter'; +import type { CryptoApi } from '$types/matrix-sdk'; +import { CryptoEvent } from '$types/matrix-sdk'; +import { useSessionBackupKeyUsable } from './useKeyBackup'; + +const emitter = new TypedEventEmitter void>>(); + +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => emitter, +})); + +type CryptoParts = { + key?: CryptoApi['getSessionBackupPrivateKey']; + version?: CryptoApi['getActiveSessionBackupVersion']; +}; + +const createCrypto = ({ key, version }: CryptoParts) => + ({ + getSessionBackupPrivateKey: + key ?? + vi.fn().mockResolvedValue(new Uint8Array([1, 2, 3])), + getActiveSessionBackupVersion: + version ?? vi.fn().mockResolvedValue('3'), + }) as unknown as CryptoApi; + +describe('useSessionBackupKeyUsable', () => { + beforeEach(() => { + emitter.removeAllListeners(); + }); + + it('reports true when both the key and the backup version are in the store', async () => { + const { result } = renderHook(() => useSessionBackupKeyUsable(createCrypto({}))); + + await waitFor(() => expect(result.current).toBe(true)); + }); + + it('reports false when the store has no key', async () => { + const crypto = createCrypto({ + key: vi.fn().mockResolvedValue(null), + }); + + const { result } = renderHook(() => useSessionBackupKeyUsable(crypto)); + + await waitFor(() => expect(result.current).toBe(false)); + }); + + // restoreKeyBackup raises "No decryption key found in crypto store" for a + // missing version too, which getSessionBackupPrivateKey alone cannot see. + it('reports false when the key is present but the backup version is not', async () => { + const crypto = createCrypto({ + version: vi.fn().mockResolvedValue(null), + }); + + const { result } = renderHook(() => useSessionBackupKeyUsable(crypto)); + + await waitFor(() => expect(result.current).toBe(false)); + }); + + it('stays unknown rather than false when the lookup rejects', async () => { + const crypto = createCrypto({ + key: vi + .fn() + .mockRejectedValue(new Error('store closed')), + }); + + const { result } = renderHook(() => useSessionBackupKeyUsable(crypto)); + + await waitFor(() => expect(crypto.getSessionBackupPrivateKey).toHaveBeenCalled()); + expect(result.current).toBeUndefined(); + }); + + it('re-checks once the key gets cached', async () => { + const crypto = createCrypto({ + key: vi + .fn() + .mockResolvedValueOnce(null) + .mockResolvedValue(new Uint8Array([1])), + }); + + const { result } = renderHook(() => useSessionBackupKeyUsable(crypto)); + await waitFor(() => expect(result.current).toBe(false)); + + emitter.emit(CryptoEvent.KeyBackupDecryptionKeyCached as never); + + await waitFor(() => expect(result.current).toBe(true)); + }); +}); diff --git a/src/app/hooks/useKeyBackup.ts b/src/app/hooks/useKeyBackup.ts index 2c6eb1736..cd2fedfa9 100644 --- a/src/app/hooks/useKeyBackup.ts +++ b/src/app/hooks/useKeyBackup.ts @@ -5,7 +5,7 @@ import type { KeyBackupInfo, } from '$types/matrix-sdk'; import { CryptoEvent } from '$types/matrix-sdk'; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import * as Sentry from '@sentry/react'; import { useMatrixClient } from './useMatrixClient'; import { useMatrixEvent } from './useMatrixEvent'; @@ -56,6 +56,41 @@ export const useKeyBackupDecryptionKeyCached = ( useMatrixEvent(mx, CryptoEvent.KeyBackupDecryptionKeyCached, onChange); }; +/** + * Whether this device can actually restore from backup. `restoreKeyBackup` + * requires BOTH a decryption key and a backup version in the store — it raises + * the same "No decryption key found in crypto store" for either being absent, + * while `getSessionBackupPrivateKey` only reports on the key. `undefined` while + * unknown (first lookup in flight, or the lookup failed). + */ +export const useSessionBackupKeyUsable = (crypto: CryptoApi): boolean | undefined => { + const alive = useAlive(); + const [usable, setUsable] = useState(); + const requestRef = useRef(0); + + const fetchUsable = useCallback(() => { + requestRef.current += 1; + const request = requestRef.current; + Promise.all([crypto.getSessionBackupPrivateKey(), crypto.getActiveSessionBackupVersion()]) + .then(([key, version]) => { + // A later lookup already answered; this one is stale. + if (alive() && request === requestRef.current) setUsable(key !== null && version !== null); + }) + .catch(() => { + if (alive() && request === requestRef.current) setUsable(undefined); + }); + }, [crypto, alive]); + + useEffect(() => { + fetchUsable(); + }, [fetchUsable]); + + useKeyBackupStatusChange(fetchUsable); + useKeyBackupDecryptionKeyCached(fetchUsable); + + return usable; +}; + export const useKeyBackupSync = (): [number, string | undefined] => { const [remaining, setRemaining] = useState(0); const [failure, setFailure] = useState(); diff --git a/src/app/hooks/useRestoreBackupOnVerification.test.tsx b/src/app/hooks/useRestoreBackupOnVerification.test.tsx new file mode 100644 index 000000000..4a31e8daa --- /dev/null +++ b/src/app/hooks/useRestoreBackupOnVerification.test.tsx @@ -0,0 +1,72 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { TypedEventEmitter } from 'matrix-js-sdk/lib/models/typed-event-emitter'; +import { Provider, useAtomValue } from 'jotai'; +import type { ReactNode } from 'react'; +import { CryptoEvent } from '$types/matrix-sdk'; +import { backupRestoreErrorAtom } from '$state/backupRestore'; +import { useRestoreBackupOnVerification } from './useRestoreBackupOnVerification'; + +const restoreKeyBackup = vi.fn<() => Promise>(); +const emitter = new TypedEventEmitter void>>(); +const mockClient = Object.assign(emitter, { + getCrypto: () => ({ restoreKeyBackup }), +}); + +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => mockClient, +})); + +vi.mock('@sentry/react', () => ({ + addBreadcrumb: vi.fn<() => void>(), + metrics: { count: vi.fn<() => void>() }, +})); + +function Wrapper({ children }: { children: ReactNode }) { + return {children}; +} + +const useSubject = () => { + useRestoreBackupOnVerification(); + return useAtomValue(backupRestoreErrorAtom); +}; + +const emitKeyCached = () => emitter.emit(CryptoEvent.KeyBackupDecryptionKeyCached as never); + +describe('useRestoreBackupOnVerification', () => { + beforeEach(() => { + restoreKeyBackup.mockReset(); + }); + + it('surfaces a restore failure instead of rejecting silently', async () => { + restoreKeyBackup.mockRejectedValue(new Error('No decryption key found in crypto store')); + + const { result } = renderHook(useSubject, { wrapper: Wrapper }); + emitKeyCached(); + + await waitFor(() => expect(result.current).toBe('No decryption key found in crypto store')); + }); + + it('leaves no error behind on a successful restore', async () => { + restoreKeyBackup.mockResolvedValue(undefined); + + const { result } = renderHook(useSubject, { wrapper: Wrapper }); + emitKeyCached(); + + await waitFor(() => expect(restoreKeyBackup).toHaveBeenCalled()); + expect(result.current).toBeUndefined(); + }); + + it('clears a previous failure when a later restore is attempted', async () => { + restoreKeyBackup.mockRejectedValueOnce(new Error('boom')); + + const { result } = renderHook(useSubject, { wrapper: Wrapper }); + emitKeyCached(); + await waitFor(() => expect(result.current).toBe('boom')); + + restoreKeyBackup.mockResolvedValue(undefined); + emitKeyCached(); + + await waitFor(() => expect(result.current).toBeUndefined()); + }); +}); diff --git a/src/app/hooks/useRestoreBackupOnVerification.ts b/src/app/hooks/useRestoreBackupOnVerification.ts index 8a8cf93d7..5fc30b2ac 100644 --- a/src/app/hooks/useRestoreBackupOnVerification.ts +++ b/src/app/hooks/useRestoreBackupOnVerification.ts @@ -1,11 +1,18 @@ import { useSetAtom } from 'jotai'; import { useCallback } from 'react'; -import { backupRestoreProgressAtom } from '$state/backupRestore'; +import * as Sentry from '@sentry/react'; +import { + backupRestoreErrorAtom, + backupRestoreProgressAtom, + resetBackupRestoreAtom, +} from '$state/backupRestore'; import { useMatrixClient } from './useMatrixClient'; import { useKeyBackupDecryptionKeyCached } from './useKeyBackup'; export const useRestoreBackupOnVerification = () => { const setRestoreProgress = useSetAtom(backupRestoreProgressAtom); + const setRestoreError = useSetAtom(backupRestoreErrorAtom); + const resetRestore = useSetAtom(resetBackupRestoreAtom); const mx = useMatrixClient(); @@ -14,11 +21,26 @@ export const useRestoreBackupOnVerification = () => { const crypto = mx.getCrypto(); if (!crypto) return; - crypto.restoreKeyBackup({ - progressCallback(progress) { - setRestoreProgress(progress); - }, - }); - }, [mx, setRestoreProgress]) + resetRestore(); + crypto + .restoreKeyBackup({ + progressCallback(progress) { + setRestoreProgress(progress); + }, + }) + .catch((err: unknown) => { + const message = err instanceof Error ? err.message : String(err); + Sentry.addBreadcrumb({ + category: 'crypto', + message: 'Key backup restore failed', + level: 'error', + data: { error: message }, + }); + Sentry.metrics.count('sable.crypto.key_backup_restore_failures', 1); + // Progress stays at Fetching if the import threw partway. + resetRestore(); + setRestoreError(message); + }); + }, [mx, setRestoreProgress, setRestoreError, resetRestore]) ); }; diff --git a/src/app/pages/client/ClientRoot.tsx b/src/app/pages/client/ClientRoot.tsx index 25301c7ae..2bbbec9b4 100644 --- a/src/app/pages/client/ClientRoot.tsx +++ b/src/app/pages/client/ClientRoot.tsx @@ -27,6 +27,8 @@ import { startClient, stopClient, } from '$client/initMatrix'; +import { clearSecretStorageKeys } from '$client/secretStorageKeys'; +import { resetBackupRestoreAtom } from '$state/backupRestore'; import { SplashScreen } from '$components/splash-screen'; import { ServerConfigsLoader } from '$components/ServerConfigsLoader'; import { CapabilitiesProvider } from '$hooks/useCapabilities'; @@ -245,6 +247,7 @@ export function ClientRoot({ children }: ClientRootProps) { const sessions = useAtomValue(sessionsAtom); const [activeSessionId, setActiveSessionId] = useAtom(activeSessionIdAtom); const setSessions = useSetAtom(sessionsAtom); + const resetBackupRestore = useSetAtom(resetBackupRestoreAtom); const activeSession: Session | undefined = sessions.find((s) => s.userId === activeSessionId) ?? sessions[0]; @@ -309,14 +312,22 @@ export function ClientRoot({ children }: ClientRootProps) { '— reloading client' ); void pushSessionToSW(activeSession.baseUrl, activeSession.accessToken, activeSession.userId); - if (mx?.clientRunning) { + // Unconditional: stopClient is what stops the crypto backend, and a client + // that never reached clientRunning still holds an open crypto store. + if (mx) { stopClient(mx); } + // The cache is keyed by 4S key id only, so the previous account's key + // would otherwise stay in memory for the next one. + clearSecretStorageKeys(); + // Jotai atoms live in the default store for the tab's lifetime, so the + // previous account's restore state would be read as this one's. + resetBackupRestore(); loadedUserIdRef.current = undefined; setLoadState({ status: AsyncStatus.Idle }); navigate(getHomePath(), { replace: true }); } - }, [activeSession, mx, navigate, setLoadState]); + }, [activeSession, mx, navigate, setLoadState, resetBackupRestore]); const handleLogout = useCallback(async () => { if (!mx || !activeSession) return; @@ -335,7 +346,7 @@ export function ClientRoot({ children }: ClientRootProps) { useEffect( () => () => { - if (mx?.clientRunning) { + if (mx) { log.log('ClientRoot unmounting — stopping client', mx.getUserId()); stopClient(mx); } diff --git a/src/app/state/backupRestore.test.ts b/src/app/state/backupRestore.test.ts new file mode 100644 index 000000000..60faf13e5 --- /dev/null +++ b/src/app/state/backupRestore.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest'; +import { isMissingBackupKeyError } from './backupRestore'; + +describe('isMissingBackupKeyError', () => { + it('matches the error the crypto store raises without a cached key', () => { + expect(isMissingBackupKeyError(new Error('No decryption key found in crypto store'))).toBe( + true + ); + }); + + it('matches the same error as a plain string', () => { + expect(isMissingBackupKeyError('No decryption key found in crypto store')).toBe(true); + }); + + it('ignores case', () => { + expect(isMissingBackupKeyError(new Error('NO DECRYPTION KEY FOUND'))).toBe(true); + }); + + it('does not match unrelated restore failures', () => { + expect(isMissingBackupKeyError(new Error('Backup version not found'))).toBe(false); + expect(isMissingBackupKeyError(new Error('M_LIMIT_EXCEEDED'))).toBe(false); + }); + + it('handles absent and non-stringifiable values', () => { + expect(isMissingBackupKeyError(undefined)).toBe(false); + expect(isMissingBackupKeyError(null)).toBe(false); + expect(isMissingBackupKeyError({})).toBe(false); + }); +}); diff --git a/src/app/state/backupRestore.ts b/src/app/state/backupRestore.ts index 8b411ebee..720e92c97 100644 --- a/src/app/state/backupRestore.ts +++ b/src/app/state/backupRestore.ts @@ -33,6 +33,21 @@ const baseBackupRestoreProgressAtom = atom({ status: BackupProgressStatus.Idle, }); +export const backupRestoreErrorAtom = atom(undefined); + +// Progress sticks at Fetching if a restore throws after it starts importing, and +// both atoms outlive an account switch since there is no jotai Provider. +export const resetBackupRestoreAtom = atom(null, (_get, set) => { + set(baseBackupRestoreProgressAtom, { status: BackupProgressStatus.Idle }); + set(backupRestoreErrorAtom, undefined); +}); + +export const isMissingBackupKeyError = (error: unknown): boolean => { + if (error === undefined || error === null) return false; + const message = error instanceof Error ? error.message : typeof error === 'string' ? error : ''; + return message.toLowerCase().includes('no decryption key found'); +}; + export const backupRestoreProgressAtom = atom< IBackupProgress, [ImportRoomKeyProgressData], diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index b463b3139..144984d83 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -30,7 +30,7 @@ import * as Sentry from '@sentry/react'; import { pushSessionToSW } from '../sw-session'; import { assertAuthMetadataIssuer, createSessionTokenRefresher } from './oidcTokenRefresher'; import { revokeOAuthToken } from './oauthTokenRevocation'; -import { cryptoCallbacks } from './secretStorageKeys'; +import { clearSecretStorageKeys, cryptoCallbacks } from './secretStorageKeys'; import type { SlidingSyncDiagnostics } from './slidingSync'; import { scopeEphemeralExtensions, SlidingSyncManager } from './slidingSync'; import { PresenceSyncManager } from './presenceSync'; @@ -610,6 +610,7 @@ export const logoutClient = async (mx: MatrixClient, session?: Session) => { SlidingSyncSidebarCache.clear(session.userId); clearCachedVersions(session.baseUrl, session.userId); clearCachedUserProfiles(session.userId); + clearSecretStorageKeys(); const storeName: SessionStoreName = getSessionStoreName(session); await mx.clearStores({ cryptoDatabasePrefix: storeName.rustCryptoPrefix }); await deleteDatabase(storeName.sync); diff --git a/src/client/secretStorageKeys.test.ts b/src/client/secretStorageKeys.test.ts new file mode 100644 index 000000000..c19038c76 --- /dev/null +++ b/src/client/secretStorageKeys.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { clearSecretStorageKeys, cryptoCallbacks, storePrivateKey } from './secretStorageKeys'; + +const KEY_A = 'key-account-a'; +const KEY_B = 'key-account-b'; + +const ask = (...keyIds: string[]) => + cryptoCallbacks.getSecretStorageKey({ + keys: Object.fromEntries(keyIds.map((id) => [id, {}])), + }); + +describe('secretStorageKeys', () => { + beforeEach(() => { + clearSecretStorageKeys(); + }); + + it('returns a stored key for the requested key id', async () => { + storePrivateKey(KEY_A, new Uint8Array([1, 2, 3])); + + const result = await ask(KEY_A); + + expect(result?.[0]).toBe(KEY_A); + expect(result?.[1]).toEqual(new Uint8Array([1, 2, 3])); + }); + + it('does not answer for a key id it has never seen', async () => { + storePrivateKey(KEY_A, new Uint8Array([1])); + + expect(await ask(KEY_B)).toBeUndefined(); + }); + + it('rejects a non-Uint8Array key', () => { + expect(() => storePrivateKey(KEY_A, 'not-a-key' as unknown as Uint8Array)).toThrow( + 'Unable to store, privateKey is invalid.' + ); + }); + + // The cache is module-level and shared by every client in the tab, so an + // account switch has to empty it or the previous account's 4S key lingers. + it('holds nothing after a switch clears it', async () => { + storePrivateKey(KEY_A, new Uint8Array([1])); + storePrivateKey(KEY_B, new Uint8Array([2])); + + clearSecretStorageKeys(); + + expect(await ask(KEY_A)).toBeUndefined(); + expect(await ask(KEY_B)).toBeUndefined(); + }); + + it('lets the next account store its own key after a clear', async () => { + storePrivateKey(KEY_A, new Uint8Array([1])); + clearSecretStorageKeys(); + storePrivateKey(KEY_B, new Uint8Array([9])); + + expect((await ask(KEY_B))?.[1]).toEqual(new Uint8Array([9])); + expect(await ask(KEY_A)).toBeUndefined(); + }); +});