fix(crypto): settle emoji verification on the verifier promise

src-tauri/src/matrix_crypto/mod.rs

@@ -74,36 +74,6 @@ impl CryptoEngineState {
             .remove(account)
             .is_some())
     }
-
-    pub fn close_account_if(&self, account: &str, machine: &Arc<OlmMachine>) -> Result<(), String> {
-        let removed = {
-            let mut machines = self.machines.lock().map_err(|e| e.to_string())?;
-            match machines.get(account) {
-                Some(current) if Arc::ptr_eq(current, machine) => {
-                    machines.remove(account);
-                    true
-                }
-                _ => false,
-            }
-        };
-
-        if !removed {
-            return Ok(());
-        }
-
-        if let Some(listeners) = self
-            .listeners
-            .lock()
-            .map_err(|e| e.to_string())?
-            .remove(account)
-        {
-            for listener in listeners {
-                listener.abort();
-            }
-        }
-
-        Ok(())
-    }
 }
 
 #[derive(Debug, Serialize)]
@@ -388,29 +358,6 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
-    #[tokio::test]
-    async fn close_account_if_leaves_a_machine_it_does_not_own() {
-        let user: &matrix_sdk::ruma::UserId = "@race:example.org".try_into().unwrap();
-        let device: &matrix_sdk::ruma::DeviceId = "RACEDEVICE".into();
-        let account = account_key(user.as_str(), device.as_str());
-
-        let mine = Arc::new(OlmMachine::new(user, device).await);
-        let theirs = Arc::new(OlmMachine::new(user, device).await);
-
-        let state = CryptoEngineState::default();
-        state
-            .machines
-            .lock()
-            .unwrap()
-            .insert(account.clone(), Arc::clone(&theirs));
-
-        state.close_account_if(&account, &mine).unwrap();
-        assert!(state.machine(user.as_str(), device.as_str()).is_ok());
-
-        state.close_account_if(&account, &theirs).unwrap();
-        assert!(state.machine(user.as_str(), device.as_str()).is_err());
-    }
-
     #[tokio::test]
     async fn engine_plumbing() {
         let dir = std::env::temp_dir().join(format!("sable-engine-test-{}", std::process::id()));

src/app/components/DeviceVerification.test.tsx

@@ -0,0 +1,77 @@
+import { render, screen, waitFor } from '@testing-library/react';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import type { VerificationRequest } from '$types/matrix-sdk';
+import { VerificationPhase } from '$types/matrix-sdk';
+import { DeviceVerification } from './DeviceVerification';
+
+vi.mock('$hooks/useMatrixClient', () => ({
+  useMatrixClient: () => ({ getSafeUserId: () => '@me:example.org' }),
+}));
+
+vi.mock('$components/modal-overlay/ModalOverlay', () => ({
+  ModalOverlay: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+}));
+
+const sasCallbacks = {
+  sas: { emoji: [['🐶', 'Dog']] as [string, string][] },
+  confirm: vi.fn<() => Promise<void>>(),
+  mismatch: vi.fn<() => void>(),
+  cancel: vi.fn<() => void>(),
+};
+
+const verificationRequest = (verify: () => Promise<void>) => ({
+  phase: VerificationPhase.Started,
+  initiatedByMe: true,
+  verifier: {
+    verify,
+    getShowSasCallbacks: () => sasCallbacks,
+    on: vi.fn<() => void>(),
+    removeListener: vi.fn<() => void>(),
+  },
+  cancel: vi.fn<() => Promise<void>>(),
+  accept: vi.fn<() => Promise<void>>(),
+  startVerification: vi.fn<() => Promise<void>>(),
+  on: vi.fn<() => void>(),
+  removeListener: vi.fn<() => void>(),
+});
+
+const renderDialog = (request: ReturnType<typeof verificationRequest>) =>
+  render(
+    <QueryClientProvider client={new QueryClient()}>
+      <DeviceVerification
+        request={request as unknown as VerificationRequest}
+        onExit={vi.fn<() => void>()}
+      />
+    </QueryClientProvider>
+  );
+
+describe('DeviceVerification', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('reports success once the emoji matched, while the request still reads Started', async () => {
+    renderDialog(verificationRequest(() => Promise.resolve()));
+
+    expect(await screen.findByText('Your device is verified.')).toBeInTheDocument();
+  });
+
+  it('does not cancel a verification that already succeeded', async () => {
+    const request = verificationRequest(() => Promise.resolve());
+
+    renderDialog(request);
+    await screen.findByText('Your device is verified.');
+
+    screen.getByRole('button', { name: /Okay/i }).click();
+
+    await waitFor(() => expect(request.cancel).not.toHaveBeenCalled());
+  });
+
+  it('keeps showing the emoji while the verifier has not resolved', async () => {
+    renderDialog(verificationRequest(() => new Promise<void>(() => undefined)));
+
+    expect(await screen.findByText(/Confirm the emoji below/)).toBeInTheDocument();
+    expect(screen.queryByText('Your device is verified.')).toBeNull();
+  });
+});

src/app/components/DeviceVerification.tsx

@@ -1,7 +1,7 @@
 import type { ShowSasCallbacks, VerificationRequest, Verifier } from '$types/matrix-sdk';
 import { VerificationPhase, VerificationMethod } from '$types/matrix-sdk';
 import type { CSSProperties } from 'react';
-import { useCallback, useEffect, useMemo, useState } from 'react';
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { Box, config, Dialog, Header, IconButton, Spinner, Text } from 'folds';
 import { composerIcon, X } from '$components/icons/phosphor';
 import * as Sentry from '@sentry/react';
@@ -182,16 +182,26 @@ function CompareEmoji({ sasData }: { sasData: ShowSasCallbacks }) {
 type SasVerificationProps = {
   verifier: Verifier;
   onCancel: () => void;
+  onVerified: () => void;
 };
-function SasVerification({ verifier, onCancel }: SasVerificationProps) {
+function SasVerification({ verifier, onCancel, onVerified }: SasVerificationProps) {
   const [sasData, setSasData] = useState<ShowSasCallbacks>();
 
   useVerifierShowSas(verifier, setSasData);
   useVerifierCancel(verifier, onCancel);
 
   useEffect(() => {
-    verifier.verify().catch(() => undefined);
-  }, [verifier]);
+    let active = true;
+    verifier
+      .verify()
+      .then(() => {
+        if (active) onVerified();
+      })
+      .catch(() => undefined);
+    return () => {
+      active = false;
+    };
+  }, [verifier, onVerified]);
 
   if (sasData) {
     return <CompareEmoji sasData={sasData} />;
@@ -240,9 +250,20 @@ type DeviceVerificationProps = {
 };
 export function DeviceVerification({ request, onExit }: DeviceVerificationProps) {
   const phase = useVerificationRequestPhase(request);
+  const [verified, setVerified] = useState(false);
+  const verifiedRef = useRef(false);
+
+  const handleVerified = useCallback(() => {
+    verifiedRef.current = true;
+    setVerified(true);
+  }, []);
 
   const handleCancel = useCallback(() => {
-    if (request.phase !== VerificationPhase.Done && request.phase !== VerificationPhase.Cancelled) {
+    if (
+      !verifiedRef.current &&
+      request.phase !== VerificationPhase.Done &&
+      request.phase !== VerificationPhase.Cancelled
+    ) {
       request.cancel().catch(() => undefined);
     }
     onExit();
@@ -254,19 +275,24 @@ export function DeviceVerification({ request, onExit }: DeviceVerificationProps)
   }, [request]);
 
   const refreshVerificationStatus = useRefreshDeviceVerificationStatus();
+  const done = verified || phase === VerificationPhase.Done;
+  const reportedRef = useRef(false);
 
   useEffect(() => {
-    if (phase === VerificationPhase.Done) {
+    if (reportedRef.current) return;
+    if (done) {
+      reportedRef.current = true;
       refreshVerificationStatus();
       Sentry.metrics.count('sable.crypto.verification_outcome', 1, {
         attributes: { outcome: 'completed' },
       });
     } else if (phase === VerificationPhase.Cancelled) {
+      reportedRef.current = true;
       Sentry.metrics.count('sable.crypto.verification_outcome', 1, {
         attributes: { outcome: 'cancelled' },
       });
     }
-  }, [phase, refreshVerificationStatus]);
+  }, [done, phase, refreshVerificationStatus]);
 
   return (
     <ModalOverlay
@@ -298,16 +324,23 @@ export function DeviceVerification({ request, onExit }: DeviceVerificationProps)
               <VerificationWaitStart />
             ))}
           {phase === VerificationPhase.Started &&
+            !done &&
             (request.verifier ? (
-              <SasVerification verifier={request.verifier} onCancel={handleCancel} />
+              <SasVerification
+                verifier={request.verifier}
+                onCancel={handleCancel}
+                onVerified={handleVerified}
+              />
             ) : (
               <VerificationUnexpected
                 message="Unexpected Error! Verification is started but verifier is missing."
                 onClose={handleCancel}
               />
             ))}
-          {phase === VerificationPhase.Done && <VerificationDone onExit={onExit} />}
-          {phase === VerificationPhase.Cancelled && <VerificationCanceled onClose={handleCancel} />}
+          {done && <VerificationDone onExit={onExit} />}
+          {!done && phase === VerificationPhase.Cancelled && (
+            <VerificationCanceled onClose={handleCancel} />
+          )}
         </Box>
       </Dialog>
     </ModalOverlay>

src/app/features/settings/settingsLink.ts

@@ -192,6 +192,7 @@ export const settingsLinkFocusIdsBySection: Record<SettingsSectionId, readonly s
     'direct-messages',
     'direct-messages-encrypted',
     'email-notification',
+    'embedded-push-battery-optimization',
     'favicon-dot-mentions-only',
     'highlight-mentions',
     'in-app-notification-sound',