diff --git a/.changeset/cache-device-verification-status.md b/.changeset/cache-device-verification-status.md new file mode 100644 index 0000000000..bf3c5cde0b --- /dev/null +++ b/.changeset/cache-device-verification-status.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Cache cross-signing device verification so sliding sync device list updates no longer re-verify every device on every response. diff --git a/src/app/hooks/useDeviceVerificationStatus.test.tsx b/src/app/hooks/useDeviceVerificationStatus.test.tsx new file mode 100644 index 0000000000..09324b2420 --- /dev/null +++ b/src/app/hooks/useDeviceVerificationStatus.test.tsx @@ -0,0 +1,293 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, act, waitFor } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; + +import { CryptoEvent, type CryptoApi } from '$types/matrix-sdk'; + +import { + useDeviceVerificationStatus, + useUnverifiedDeviceCount, + VerificationStatus, +} from './useDeviceVerificationStatus'; + +const { mockMx, getListenerCount } = vi.hoisted(() => { + const listeners = new Map void>>(); + const mx = { + on(event: string, cb: (...args: never[]) => void) { + const set = listeners.get(event) ?? new Set(); + set.add(cb); + listeners.set(event, set); + return mx; + }, + removeListener(event: string, cb: (...args: never[]) => void) { + listeners.get(event)?.delete(cb); + return mx; + }, + emit(event: string, ...args: never[]) { + listeners.get(event)?.forEach((cb) => cb(...args)); + }, + getListenerCount(event: string) { + return listeners.get(event)?.size ?? 0; + }, + }; + return { mockMx: mx, getListenerCount: (event: string) => mx.getListenerCount(event) }; +}); + +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => mockMx, +})); + +const USER_ID = '@me:example.org'; +const DEVICE_ID = 'DEVICEONE'; + +const getDeviceVerificationStatus = + vi.fn<(userId: string, deviceId: string) => Promise<{ crossSigningVerified: boolean } | null>>(); +const crypto = { getDeviceVerificationStatus } as unknown as CryptoApi; + +const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return ({ children }: { children: ReactNode }) => ( + {children} + ); +}; + +describe('useDeviceVerificationStatus', () => { + beforeEach(() => { + getDeviceVerificationStatus.mockReset(); + getDeviceVerificationStatus.mockResolvedValue({ crossSigningVerified: true }); + }); + + it('reports the cross-signing verification status', async () => { + const { result } = renderHook(() => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), { + wrapper: createWrapper(), + }); + + expect(result.current).toBe(VerificationStatus.Unknown); + await waitFor(() => expect(result.current).toBe(VerificationStatus.Verified)); + }); + + it('reports Unsupported when the device has no verification status', async () => { + getDeviceVerificationStatus.mockResolvedValue(null); + + const { result } = renderHook(() => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current).toBe(VerificationStatus.Unsupported)); + }); + + it('stays Unknown without a device id and never verifies', async () => { + const { result } = renderHook(() => useDeviceVerificationStatus(crypto, USER_ID, undefined), { + wrapper: createWrapper(), + }); + + await act(async () => {}); + + expect(result.current).toBe(VerificationStatus.Unknown); + expect(getDeviceVerificationStatus).not.toHaveBeenCalled(); + }); + + it('stays Unknown without a crypto api and never verifies', async () => { + const { result } = renderHook( + () => useDeviceVerificationStatus(undefined, USER_ID, DEVICE_ID), + { wrapper: createWrapper() } + ); + + await act(async () => {}); + + expect(result.current).toBe(VerificationStatus.Unknown); + expect(getDeviceVerificationStatus).not.toHaveBeenCalled(); + }); + + it('verifies a device once when several consumers observe it', async () => { + const { result } = renderHook( + () => + [ + useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), + useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), + useUnverifiedDeviceCount(crypto, USER_ID, [DEVICE_ID]), + ] as const, + { wrapper: createWrapper() } + ); + + await waitFor(() => expect(result.current[0]).toBe(VerificationStatus.Verified)); + await waitFor(() => expect(result.current[2]).toBe(0)); + + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(1); + }); + + it('does not re-verify while no crypto event arrives', async () => { + const { result, rerender } = renderHook( + () => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), + { wrapper: createWrapper() } + ); + + await waitFor(() => expect(result.current).toBe(VerificationStatus.Verified)); + rerender(); + rerender(); + + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(1); + }); + + it('re-verifies when a consumer remounts after the previous one unmounted', async () => { + const wrapper = createWrapper(); + const first = renderHook(() => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), { + wrapper, + }); + + await waitFor(() => expect(first.result.current).toBe(VerificationStatus.Verified)); + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(1); + first.unmount(); + + const second = renderHook(() => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), { + wrapper, + }); + + await waitFor(() => expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(2)); + second.unmount(); + }); + + it.each([ + ['DevicesUpdated for the user', CryptoEvent.DevicesUpdated, [[USER_ID], false]], + ['KeysChanged', CryptoEvent.KeysChanged, [{}]], + ['UserTrustStatusChanged for the user', CryptoEvent.UserTrustStatusChanged, [USER_ID, {}]], + ])('re-verifies on %s', async (_label, event, args) => { + const { result } = renderHook(() => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current).toBe(VerificationStatus.Verified)); + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(1); + + getDeviceVerificationStatus.mockResolvedValue({ crossSigningVerified: false }); + await act(async () => { + mockMx.emit(event, ...(args as never[])); + }); + + await waitFor(() => expect(result.current).toBe(VerificationStatus.Unverified)); + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(2); + }); + + it('ignores device updates for a different user', async () => { + const { result } = renderHook(() => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), { + wrapper: createWrapper(), + }); + + await waitFor(() => expect(result.current).toBe(VerificationStatus.Verified)); + + await act(async () => { + mockMx.emit(CryptoEvent.DevicesUpdated, ...(['@other:example.org'] as never[])); + mockMx.emit(CryptoEvent.UserTrustStatusChanged, ...(['@other:example.org', {}] as never[])); + }); + + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(1); + }); + + it('triggers a new verification call when the crypto instance changes for the same user/device', async () => { + const wrapper = createWrapper(); + const getDeviceVerificationStatus2 = vi + .fn<(userId: string, deviceId: string) => Promise<{ crossSigningVerified: boolean } | null>>() + .mockResolvedValue({ crossSigningVerified: true }); + const crypto2 = { + getDeviceVerificationStatus: getDeviceVerificationStatus2, + } as unknown as CryptoApi; + + const { result, rerender } = renderHook( + ({ c }) => useDeviceVerificationStatus(c, USER_ID, DEVICE_ID), + { + wrapper, + initialProps: { c: crypto }, + } + ); + + await waitFor(() => expect(result.current).toBe(VerificationStatus.Verified)); + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(1); + + rerender({ c: crypto2 }); + + await waitFor(() => expect(getDeviceVerificationStatus2).toHaveBeenCalledTimes(1)); + expect(getDeviceVerificationStatus2).toHaveBeenCalledWith(USER_ID, DEVICE_ID); + }); + + it('does not trigger duplicate invalidations for unaffected users when UserTrustStatusChanged fires', async () => { + const USER_B = '@userB:example.org'; + const wrapper = createWrapper(); + + const { result: resultA } = renderHook( + () => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), + { wrapper } + ); + const { result: resultB } = renderHook( + () => useDeviceVerificationStatus(crypto, USER_B, DEVICE_ID), + { wrapper } + ); + + await waitFor(() => expect(resultA.current).toBe(VerificationStatus.Verified)); + await waitFor(() => expect(resultB.current).toBe(VerificationStatus.Verified)); + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(2); + + getDeviceVerificationStatus.mockResolvedValue({ crossSigningVerified: false }); + + await act(async () => { + mockMx.emit(CryptoEvent.UserTrustStatusChanged, ...([USER_B, {}] as never[])); + }); + + await waitFor(() => expect(resultB.current).toBe(VerificationStatus.Unverified)); + expect(getDeviceVerificationStatus).toHaveBeenCalledTimes(3); + }); + + it('shares event subscriptions across multiple consumers and cleans up when all unmount', async () => { + const wrapper = createWrapper(); + const { unmount: unmount1 } = renderHook( + () => useDeviceVerificationStatus(crypto, USER_ID, DEVICE_ID), + { wrapper } + ); + const { unmount: unmount2 } = renderHook( + () => useDeviceVerificationStatus(crypto, '@user2:example.org', DEVICE_ID), + { wrapper } + ); + + expect(getListenerCount(CryptoEvent.DevicesUpdated)).toBe(1); + expect(getListenerCount(CryptoEvent.KeysChanged)).toBe(1); + expect(getListenerCount(CryptoEvent.UserTrustStatusChanged)).toBe(1); + + unmount1(); + expect(getListenerCount(CryptoEvent.DevicesUpdated)).toBe(1); + + unmount2(); + expect(getListenerCount(CryptoEvent.DevicesUpdated)).toBe(0); + expect(getListenerCount(CryptoEvent.KeysChanged)).toBe(0); + expect(getListenerCount(CryptoEvent.UserTrustStatusChanged)).toBe(0); + }); +}); + +describe('useUnverifiedDeviceCount', () => { + beforeEach(() => { + getDeviceVerificationStatus.mockReset(); + }); + + it('counts only devices that are not cross-signing verified', async () => { + getDeviceVerificationStatus.mockImplementation((_userId: string, deviceId: string) => + Promise.resolve({ crossSigningVerified: deviceId === 'VERIFIED' }) + ); + + const { result } = renderHook( + () => useUnverifiedDeviceCount(crypto, USER_ID, ['VERIFIED', 'UNVERIFIED1', 'UNVERIFIED2']), + { wrapper: createWrapper() } + ); + + await waitFor(() => expect(result.current).toBe(2)); + }); + + it('returns 0 without a crypto api', () => { + const { result } = renderHook(() => useUnverifiedDeviceCount(undefined, USER_ID, [DEVICE_ID]), { + wrapper: createWrapper(), + }); + + expect(result.current).toBe(0); + expect(getDeviceVerificationStatus).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/hooks/useDeviceVerificationStatus.ts b/src/app/hooks/useDeviceVerificationStatus.ts index cfb2006cf6..fb1b759bfb 100644 --- a/src/app/hooks/useDeviceVerificationStatus.ts +++ b/src/app/hooks/useDeviceVerificationStatus.ts @@ -1,10 +1,8 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useEffect } from 'react'; +import { useQueries, useQuery, useQueryClient } from '@tanstack/react-query'; import { type CryptoApi, type CryptoEventHandlerMap, CryptoEvent } from '$types/matrix-sdk'; import { verifiedDevice } from '$utils/matrix-crypto'; -import { fulfilledPromiseSettledResult } from '$utils/common'; -import { useAlive } from './useAlive'; import { useMatrixClient } from './useMatrixClient'; -import { useDeviceListChange } from './useDeviceList'; export const useCrossSigningKeysChange = ( onChange: CryptoEventHandlerMap[CryptoEvent.KeysChanged] @@ -18,6 +16,20 @@ export const useCrossSigningKeysChange = ( }, [mx, onChange]); }; +// onUserIdentityUpdated only emits KeysChanged for our own identity, so another user's +// cross-signing status changes are observable through this event alone. +export const useUserTrustStatusChange = ( + onChange: CryptoEventHandlerMap[CryptoEvent.UserTrustStatusChanged] +) => { + const mx = useMatrixClient(); + useEffect(() => { + mx.on(CryptoEvent.UserTrustStatusChanged, onChange); + return () => { + mx.removeListener(CryptoEvent.UserTrustStatusChanged, onChange); + }; + }, [mx, onChange]); +}; + export enum VerificationStatus { Unknown, Unverified, @@ -25,43 +37,104 @@ export enum VerificationStatus { Unsupported, } -export const useDeviceVerificationDetect = ( +const DEVICE_VERIFICATION_QUERY_KEY = 'device-verification'; + +const cryptoIds = new WeakMap(); +let cryptoIdCounter = 0; + +const getCryptoId = (crypto: CryptoApi | undefined): string => { + if (!crypto) return 'none'; + let id = cryptoIds.get(crypto); + if (!id) { + id = `crypto_${++cryptoIdCounter}`; + cryptoIds.set(crypto, id); + } + return id; +}; + +// Cache per device so repeated consumers share one verification result. +// Event-driven invalidation refreshes the cache when device or trust data may have changed. +const deviceVerificationQuery = ( crypto: CryptoApi | undefined, userId: string, - deviceId: string | undefined, - callback: (status: VerificationStatus) => void -): void => { - const mx = useMatrixClient(); + deviceId: string | undefined +) => ({ + queryKey: [DEVICE_VERIFICATION_QUERY_KEY, userId, deviceId ?? '', getCryptoId(crypto)], + queryFn: async () => { + if (!crypto || !deviceId) return null; + return verifiedDevice(crypto, userId, deviceId); + }, + enabled: Boolean(crypto) && Boolean(deviceId), + staleTime: Infinity, + // Crypto listeners are shared only while a consumer is mounted. Refresh on + // remount so changes that happened while the UI was closed cannot stay hidden + // behind an indefinitely fresh inactive query. + refetchOnMount: 'always' as const, +}); + +type SubscriptionState = { + refCount: number; + cleanup: () => void; +}; - const updateStatus = useCallback(async () => { - if (crypto && deviceId) { - const data = await verifiedDevice(crypto, userId, deviceId); - if (data === null) { - callback(VerificationStatus.Unsupported); - return; - } - callback(data ? VerificationStatus.Verified : VerificationStatus.Unverified); - return; - } - callback(VerificationStatus.Unknown); - }, [crypto, deviceId, userId, callback]); +const clientSubscriptions = new WeakMap, SubscriptionState>(); + +const useInvalidateDeviceVerification = (crypto: CryptoApi | undefined): void => { + const mx = useMatrixClient(); + const queryClient = useQueryClient(); useEffect(() => { - updateStatus(); - }, [mx, updateStatus, userId]); - - useDeviceListChange( - useCallback( - (userIds) => { - if (userIds.includes(userId)) { - updateStatus(); + if (!crypto) return undefined; + + let sub = clientSubscriptions.get(mx); + if (!sub) { + const onDevicesUpdated: CryptoEventHandlerMap[CryptoEvent.DevicesUpdated] = (userIds) => { + for (const uid of userIds) { + queryClient.invalidateQueries({ queryKey: [DEVICE_VERIFICATION_QUERY_KEY, uid] }); } - }, - [userId, updateStatus] - ) - ); + }; + const onKeysChanged: CryptoEventHandlerMap[CryptoEvent.KeysChanged] = () => { + queryClient.invalidateQueries({ queryKey: [DEVICE_VERIFICATION_QUERY_KEY] }); + }; + const onUserTrustStatusChanged: CryptoEventHandlerMap[CryptoEvent.UserTrustStatusChanged] = ( + changedUserId + ) => { + queryClient.invalidateQueries({ + queryKey: [DEVICE_VERIFICATION_QUERY_KEY, changedUserId], + }); + }; + + mx.on(CryptoEvent.DevicesUpdated, onDevicesUpdated); + mx.on(CryptoEvent.KeysChanged, onKeysChanged); + mx.on(CryptoEvent.UserTrustStatusChanged, onUserTrustStatusChanged); + + sub = { + refCount: 0, + cleanup: () => { + mx.removeListener(CryptoEvent.DevicesUpdated, onDevicesUpdated); + mx.removeListener(CryptoEvent.KeysChanged, onKeysChanged); + mx.removeListener(CryptoEvent.UserTrustStatusChanged, onUserTrustStatusChanged); + }, + }; + clientSubscriptions.set(mx, sub); + } + + sub.refCount++; + + return () => { + sub!.refCount--; + if (sub!.refCount === 0) { + sub!.cleanup(); + clientSubscriptions.delete(mx); + } + }; + }, [mx, crypto, queryClient]); +}; - useCrossSigningKeysChange(useCallback(() => updateStatus(), [updateStatus])); +const toVerificationStatus = (verified: boolean | null | undefined): VerificationStatus => { + if (verified === undefined) return VerificationStatus.Unknown; + if (verified === null) return VerificationStatus.Unsupported; + return verified ? VerificationStatus.Verified : VerificationStatus.Unverified; }; export const useDeviceVerificationStatus = ( @@ -69,11 +142,11 @@ export const useDeviceVerificationStatus = ( userId: string, deviceId: string | undefined ): VerificationStatus => { - const [verificationStatus, setVerificationStatus] = useState(VerificationStatus.Unknown); + useInvalidateDeviceVerification(crypto); - useDeviceVerificationDetect(crypto, userId, deviceId, setVerificationStatus); + const { data } = useQuery(deviceVerificationQuery(crypto, userId, deviceId)); - return verificationStatus; + return toVerificationStatus(data); }; export const useUnverifiedDeviceCount = ( @@ -81,42 +154,14 @@ export const useUnverifiedDeviceCount = ( userId: string, devices: string[] ): number | undefined => { - const [unverifiedCount, setUnverifiedCount] = useState(); - const alive = useAlive(); - - const updateCount = useCallback(async () => { - let count = 0; - if (crypto) { - const promises = devices.map((deviceId) => verifiedDevice(crypto, userId, deviceId)); - const result = await Promise.allSettled(promises); - const settledResult = fulfilledPromiseSettledResult(result); - settledResult.forEach((status) => { - if (status === false) { - count += 1; - } - }); - } - if (alive()) { - setUnverifiedCount(count); - } - }, [crypto, userId, devices, alive]); - - useDeviceListChange( - useCallback( - (userIds) => { - if (userIds.includes(userId)) { - updateCount(); - } - }, - [userId, updateCount] - ) - ); - - useCrossSigningKeysChange(useCallback(() => updateCount(), [updateCount])); - - useEffect(() => { - updateCount(); - }, [updateCount]); - - return unverifiedCount; + useInvalidateDeviceVerification(crypto); + + return useQueries({ + queries: devices.map((deviceId) => deviceVerificationQuery(crypto, userId, deviceId)), + combine: (results) => { + if (!crypto) return 0; + if (results.some((result) => result.isPending)) return undefined; + return results.filter((result) => result.data === false).length; + }, + }); };