Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/app/components/DeviceVerification.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,16 @@ type VerificationAcceptProps = {
onAccept: () => Promise<void>;
};
function VerificationAccept({ onAccept }: VerificationAcceptProps) {
const [acceptState, accept] = useAsyncCallback(onAccept);
const [acceptState, accept] = useAsyncCallback<void, Error, []>(onAccept);

const accepting = acceptState.status === AsyncStatus.Loading;
return (
<Box direction="Column" gap="400">
<Text>Click accept to start the verification process.</Text>
<Text>
{acceptState.status === AsyncStatus.Error
? acceptState.error.message
: 'Click accept to start the verification process.'}
</Text>
<Button
variant="Primary"
fill="Solid"
Expand Down Expand Up @@ -317,7 +321,12 @@ export function ReceiveSelfDeviceVerification() {
const mx = useMatrixClient();
const [request, setRequest] = useState<VerificationRequest>();

useVerificationRequestReceived(setRequest);
useVerificationRequestReceived(
useCallback((received: VerificationRequest) => {
if (!received.isSelfVerification || received.initiatedByMe || !received.pending) return;
setRequest(received);
}, [])
);

useEffect(() => {
if (request) return undefined;
Expand Down
47 changes: 38 additions & 9 deletions src/app/crypto/engineCrypto/EngineCrypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@

const entries = Object.entries(value as Record<string, unknown>)
.filter(([, item]) => item !== undefined)
.sort(([a], [b]) => (a < b ? -1 : 1));

Check warning on line 136 in src/app/crypto/engineCrypto/EngineCrypto.ts

View workflow job for this annotation

GitHub Actions / Lint

unicorn(no-array-sort)

src/app/crypto/engineCrypto/EngineCrypto.ts:136:6: Use `Array#toSorted()` instead of `Array#sort()`.
return `{${entries.map(([key, item]) => `${JSON.stringify(key)}:${canonicalJson(item)}`).join(',')}}`;
};

Expand Down Expand Up @@ -779,6 +779,7 @@
}
const request = new EngineVerificationRequest(this.#engineCall, state);
this.#verificationRequests.set(transactionId, request);
if (state.phase === EnginePhase.Done || state.phase === EnginePhase.Cancelled) return true;
traceVerification('Surfacing an incoming verification request', {
sender,
transactionId,
Expand Down Expand Up @@ -2013,17 +2014,45 @@
}

async #cancelStaleRequests(userId: string): Promise<void> {
const stale = [...this.#verificationRequests.values()].filter(
(request) => request.otherUserId === userId && request.pending
);
for (const request of stale) {
traceVerification('Cancelling a stale verification request', {
flowId: request.transactionId ?? null,
let states: EngineVerificationState[] = [];
try {
states = ((await this.#call('getVerificationRequests', { userId })) ??
[]) as EngineVerificationState[];
} catch (error) {
warnVerification('Could not list stale verification requests', {
reason: error instanceof Error ? error.message : String(error),
});
// eslint-disable-next-line no-await-in-loop
await request.cancel().catch(() => undefined);
if (request.transactionId) this.#verificationRequests.delete(request.transactionId);
}

const stale = new Map<string, EngineVerificationRequest>();
for (const state of states) {
if (state.phase === EnginePhase.Done || state.phase === EnginePhase.Cancelled) continue;
stale.set(
state.flowId,
this.#verificationRequests.get(state.flowId) ??
new EngineVerificationRequest(this.#engineCall, state)
);
}
for (const request of this.#verificationRequests.values()) {
if (request.otherUserId === userId && request.pending && request.transactionId) {
stale.set(request.transactionId, request);
}
}

for (const [flowId, request] of stale) {
traceVerification('Cancelling a stale verification request', { flowId });
try {
// eslint-disable-next-line no-await-in-loop
await request.cancel();
this.#verificationRequests.delete(flowId);
} catch (error) {
warnVerification('Could not cancel a stale verification request', {
flowId,
reason: error instanceof Error ? error.message : String(error),
});
}
}
await this.#flushOutgoingRequests();
}

async requestOwnUserVerification(): Promise<VerificationRequest> {
Expand Down
56 changes: 56 additions & 0 deletions src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,3 +291,59 @@ describe('incoming verification request', () => {
expect(received).toHaveBeenCalledOnce();
});
});

describe('stale verification requests the JS map never saw', () => {
beforeEach(() => {
mockInvoke.mockReset();
vi.mocked(traceVerification).mockClear();
});

it('cancels a pending flow the engine reports on its own', async () => {
const { mx } = clientSpy();
const cancelled: string[] = [];
mockInvoke.mockImplementation(async (_identity, method, args) => {
if (method === 'getVerificationRequests') {
return [{ ...requestState, flowId: '$ghost' }];
}
if (method === 'verificationRequest.cancel') {
cancelled.push((args as { flowId: string }).flowId);
return null;
}
if (method === 'queryKeysForUsers') {
return { id: 'q1', type: 1, className: 'KeysQueryRequest', body: '{}' };
}
if (method === 'device.requestVerification') {
return { request: { ...requestState, flowId: '$new' }, outgoingRequest: null };
}
return [];
});

const crypto = new EngineCrypto(mx, { userId: '@me:e.org', deviceId: 'D' });
await crypto.requestDeviceVerification('@me:e.org', 'OTHER');

expect(cancelled).toEqual(['$ghost']);
});
});

describe('already terminal incoming request', () => {
beforeEach(() => mockInvoke.mockReset());

it('does not raise a modal for a flow the engine already cancelled', async () => {
const { mx } = clientSpy();
mockInvoke.mockImplementation(async (_identity, method) => {
if (method === 'receiveSyncChanges') {
return [{ type: 3, rawEvent: JSON.stringify(REQUEST_EVENT) }];
}
if (method === 'getVerificationRequest') return { ...requestState, phase: 5 };
return [];
});

const crypto = new EngineCrypto(mx, { userId: '@me:e.org', deviceId: 'D' });
const received = vi.fn<(request: unknown) => void>();
crypto.on(CryptoEvent.VerificationRequestReceived, received);

await crypto.preprocessToDeviceMessages([REQUEST_EVENT as never]);

expect(received).not.toHaveBeenCalled();
});
});
26 changes: 24 additions & 2 deletions src/app/hooks/useVerificationRequest.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { renderHook } from '@testing-library/react';
import { describe, expect, it, vi } from 'vitest';
import { VerifierEvent } from '$types/matrix-sdk';
import { useVerifierShowSas } from './useVerificationRequest';
import { VerificationPhase, VerifierEvent } from '$types/matrix-sdk';
import { useVerificationRequestPhase, useVerifierShowSas } from './useVerificationRequest';

describe('useVerifierShowSas', () => {
it('publishes SAS callbacks that were ready before the listener subscribed', () => {
Expand Down Expand Up @@ -33,3 +33,25 @@
expect(removeListener).toHaveBeenCalledWith(VerifierEvent.ShowSas, onCallback);
});
});

describe('useVerificationRequestPhase', () => {
it('reports the new request phase when the request is swapped out', () => {
const stub = (phase: VerificationPhase) => ({

Check warning on line 39 in src/app/hooks/useVerificationRequest.test.tsx

View workflow job for this annotation

GitHub Actions / Lint

unicorn(consistent-function-scoping)

src/app/hooks/useVerificationRequest.test.tsx:39:11: Function `stub` does not capture any variables from its parent scope
phase,
on: vi.fn<() => void>(),
removeListener: vi.fn<() => void>(),
});
const cancelled = stub(VerificationPhase.Cancelled);
const fresh = stub(VerificationPhase.Requested);

const { result, rerender } = renderHook(
({ request }) => useVerificationRequestPhase(request as never),
{ initialProps: { request: cancelled } }
);

expect(result.current).toBe(VerificationPhase.Cancelled);

rerender({ request: fresh });
expect(result.current).toBe(VerificationPhase.Requested);
});
});
4 changes: 4 additions & 0 deletions src/app/hooks/useVerificationRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ const useVerificationRequestChange = (
export const useVerificationRequestPhase = (request: VerificationRequest): VerificationPhase => {
const [phase, setPhase] = useState(() => request.phase);

useEffect(() => {
setPhase(request.phase);
}, [request]);

useVerificationRequestChange(
request,
useCallback(() => {
Expand Down
Loading