diff --git a/app/src/utils/__tests__/desktopDeepLinkListener.test.ts b/app/src/utils/__tests__/desktopDeepLinkListener.test.ts index 235b19f1b..82bc070ea 100644 --- a/app/src/utils/__tests__/desktopDeepLinkListener.test.ts +++ b/app/src/utils/__tests__/desktopDeepLinkListener.test.ts @@ -10,6 +10,7 @@ import { } from '../../store/deepLinkAuthState'; import { getStoredCoreMode } from '../configPersistence'; import { + classifyAuthStoreFailure, registerAuthDeepLinkState, setupDesktopDeepLinkListener, } from '../desktopDeepLinkListener'; @@ -227,6 +228,43 @@ describe('desktopDeepLinkListener', () => { expect(state.errorMessage).toBe('Sign-in failed. Please try again.'); }); + it('injection #1: store-time /auth/me failure bounces to signin — no session applied, no /home nav', async () => { + // Root-cause hypothesis: `auth_store_session` validates the JWT against the + // backend GET /auth/me BEFORE persisting (credentials/ops.rs). If that call + // errors/times out, store_session returns Err → applySessionToken rethrows → + // the session is NEVER persisted and the login event NEVER fires, so the user + // stays on the signin page even though OAuth "succeeded". + vi.mocked(storeSession).mockRejectedValueOnce( + new Error('Session validation failed (GET /auth/me): 503 Service Unavailable') + ); + + // The `core-state:session-token-updated` event is the ONLY trigger that drives + // CoreStateProvider → refresh → authenticated React state. If it never fires, + // the app cannot leave the signin page. + const sessionTokenUpdated = vi.fn(); + window.addEventListener('core-state:session-token-updated', sessionTokenUpdated); + window.location.hash = '#/'; // reset any prior test's navigation + + try { + vi.mocked(getCurrent).mockResolvedValue([authDeepLinkWithState('token=abc&key=auth')]); + await setupDesktopDeepLinkListener(); + await waitForAuthSettled(); + + // store WAS attempted (we reached the persistence call)... + expect(storeSession).toHaveBeenCalledWith('abc', {}); + // ...but it FAILED, so the session-applied event was never dispatched... + expect(sessionTokenUpdated).not.toHaveBeenCalled(); + // ...and we never navigated to /home (ProtectedRoute/PublicRoute keep signin). + expect(window.location.hash).not.toBe('#/home'); + // Surfaced as the generic toast; processing cleared. + const state = getDeepLinkAuthState(); + expect(state.errorMessage).toBe('Sign-in failed. Please try again.'); + expect(state.isProcessing).toBe(false); + } finally { + window.removeEventListener('core-state:session-token-updated', sessionTokenUpdated); + } + }); + it('does not make the E2E deep-link helper wait for auth readiness', async () => { let resolveReadiness!: (_value: { ready: true }) => void; waitForOAuthAuthReadiness.mockReturnValueOnce( @@ -320,3 +358,37 @@ describe('desktopDeepLinkListener', () => { expect(suppressEvents[suppressEvents.length - 1].until).toBe(0); }); }); + +describe('classifyAuthStoreFailure', () => { + it.each([ + ['Session validation failed (GET /auth/me): operation timed out', 'auth_me_timeout'], + ['error sending request: deadline has elapsed', 'auth_me_timeout'], + ['GET /auth/me failed (401 Unauthorized): bad token', 'auth_me_unauthorized'], + ['Session validation failed (GET /auth/me): 503 Service Unavailable', 'auth_me_gateway'], + ['upstream returned 502 Bad Gateway', 'auth_me_gateway'], + ['fetch failed: ECONNREFUSED', 'network'], + ['Session validation failed (GET /auth/me): something odd', 'auth_me_other'], + ['totally unrelated explosion', 'other'], + ])('classifies %j as %s', (message, expected) => { + expect(classifyAuthStoreFailure(message)).toBe(expected); + }); + + // Contract pin: the classifier matches substrings of the Rust-produced error + // (credentials/ops.rs: `Session validation failed (GET /auth/me): {reason}`, + // with {reason} from rest.rs `GET /auth/me failed ({status}): {text}` or a + // reqwest transport error). If Rust rewords that prefix, these must fail CI + // rather than letting an arm silently degrade to 'other'. + it('pins the real Rust store_session failure strings to meaningful kinds', () => { + const gateway = + 'Session validation failed (GET /auth/me): GET /auth/me failed (503): {"error":"unavailable"}'; + const timeout = + 'Session validation failed (GET /auth/me): error sending request for url (https://api.tinyhumans.ai/auth/me): operation timed out'; + const bare = 'Session validation failed (GET /auth/me): something unexpected'; + + expect(classifyAuthStoreFailure(gateway)).toBe('auth_me_gateway'); + expect(classifyAuthStoreFailure(timeout)).toBe('auth_me_timeout'); + // The bare prefix is still recognized via the auth/me anchor — NOT 'other'. + expect(classifyAuthStoreFailure(bare)).toBe('auth_me_other'); + expect(classifyAuthStoreFailure(bare)).not.toBe('other'); + }); +}); diff --git a/app/src/utils/desktopDeepLinkListener.ts b/app/src/utils/desktopDeepLinkListener.ts index 74a936669..02ace9316 100644 --- a/app/src/utils/desktopDeepLinkListener.ts +++ b/app/src/utils/desktopDeepLinkListener.ts @@ -265,6 +265,25 @@ const handleAuthDeepLink = async (parsed: URL, requireStateNonce = true) => { { requiresAppDataReset: true } ); } else { + const kind = classifyAuthStoreFailure(rawMessage); + // Capture a SYNTHETIC error keyed only by `kind` — never the raw error. + // Two reasons (both raised in review): + // 1. PII: the upstream `/auth/me` failure embeds the verbatim backend + // response body (`rest.rs`: `GET /auth/me failed ({status}): {text}`), + // and `beforeSend` does NOT scrub `exception.values[].value`. Severing + // the message (vs. scrubbing) guarantees no body/email/token-adjacent + // text ships. + // 2. Timeout shape: a hang surfaces as `CoreRpcError(kind='timeout')`, + // which `beforeSend` drops via `isCoreRpcTimeoutError(originalException)` + // BEFORE our tag applies. A plain `Error` makes `originalException` + // non-matching, so the lead cause finally reaches Sentry. + // The PII-free `kind` tag + stable fingerprint are all we need to group. + Sentry.captureException(new Error(`auth store failed: ${kind}`), { + level: 'error', + tags: { surface: 'react', phase: 'deep-link-auth-store', auth_store_failure: kind }, + fingerprint: ['deep-link-auth', 'session-store-failed', kind], + }); + console.warn('[DeepLink][auth] session store failed — staying on signin (kind=%s)', kind); failDeepLinkAuthProcessing('Sign-in failed. Please try again.'); } } @@ -279,6 +298,28 @@ const isDecryptionFailure = (message: string): boolean => { ); }; +/** + * Classify a sign-in *store* failure into a short, PII-free kind. A store-time + * `/auth/me` failure (esp. a timeout) is the lead root cause of "OAuth succeeded + * but the app is back on the login page", yet it currently emits NO Sentry signal + * on any layer: the FE has no console-capture integration, the Rust core drops + * `"timeout"`/408/504 as transient (`observability.rs`), and the backend only + * pages genuine 500s (`shouldHandleError: status === 500`, BACKEND-ALPHAHUMAN-40) + * — so gateway/timeout 5xx never reach Sentry. Tagging the kind here is the one + * place the bounce becomes debuggable. Returns a stable enum-like string (no URLs, + * no tokens) safe to use as a Sentry tag / fingerprint. + */ +export const classifyAuthStoreFailure = (message: string): string => { + const m = message.toLowerCase(); + if (/timed out|timeout|operation timed out|deadline/.test(m)) return 'auth_me_timeout'; + if (/\b401\b|unauthorized/.test(m)) return 'auth_me_unauthorized'; + if (/\b50[234]\b|bad gateway|service unavailable|gateway timeout/.test(m)) + return 'auth_me_gateway'; + if (/network|fetch failed|connection|dns|unreachable/.test(m)) return 'network'; + if (/auth\/me|session validation failed/.test(m)) return 'auth_me_other'; + return 'other'; +}; + /** * Handle `openhuman://payment/success?session_id=...` deep links. * Fired when a Stripe checkout session completes and the browser redirects diff --git a/src/openhuman/credentials/ops.rs b/src/openhuman/credentials/ops.rs index ae8604287..4e912224f 100644 --- a/src/openhuman/credentials/ops.rs +++ b/src/openhuman/credentials/ops.rs @@ -151,10 +151,26 @@ pub async fn store_session( .ok_or_else(|| "local session requires a user payload".to_string())? } else { let client = BackendOAuthClient::new(&api_url).map_err(|e| e.to_string())?; - client - .fetch_current_user(trimmed_token) - .await - .map_err(|e| format!("Session validation failed (GET /auth/me): {e:#}"))? + match client.fetch_current_user(trimmed_token).await { + Ok(user) => user, + Err(e) => { + // This is the store-time validation gate: if it fails the profile + // is NEVER persisted, so the user bounces straight back to the + // signin page after a "successful" OAuth. Timeouts/gateway 5xx are + // otherwise dropped by the Sentry transient classifier, so log an + // explicit, grep-friendly WARN to the app log regardless. + let reason = format!("{e:#}"); + tracing::warn!( + domain = "credentials", + operation = "store_session", + "[credentials][auth-store] GET /auth/me validation FAILED on {} — session NOT persisted; user will bounce to signin: {reason}", + api_url.trim_end_matches('/') + ); + return Err(format!( + "Session validation failed (GET /auth/me): {reason}" + )); + } + } }; let mut metadata = std::collections::HashMap::new(); diff --git a/tests/app_state_credentials_raw_coverage_e2e.rs b/tests/app_state_credentials_raw_coverage_e2e.rs index 59d1363a8..83af1fb32 100644 --- a/tests/app_state_credentials_raw_coverage_e2e.rs +++ b/tests/app_state_credentials_raw_coverage_e2e.rs @@ -8,6 +8,7 @@ use openhuman_core::openhuman::app_state::{ StoredOnboardingTasks, }; use openhuman_core::openhuman::config::rpc as config_rpc; +use openhuman_core::openhuman::credentials::ops::store_session; use openhuman_core::openhuman::credentials::profiles::{ AuthProfile, AuthProfileKind, AuthProfilesStore, TokenSet, }; @@ -188,6 +189,89 @@ async fn auth_me_server( (url, task, shutdown_tx) } +/// Like `auth_me_server` but always replies HTTP 500, so `store_session`'s +/// `GET /auth/me` validation gate fails — exercising the WARN + `Err` path that +/// leaves the session unpersisted (the "OAuth succeeded but app is back on the +/// signin page" bug). +async fn auth_me_failing_server() -> ( + String, + tokio::task::JoinHandle<()>, + tokio::sync::oneshot::Sender<()>, +) { + let listener = tokio::net::TcpListener::bind(("127.0.0.1", 0)) + .await + .expect("bind failing auth/me listener"); + let url = format!("http://{}", listener.local_addr().expect("listener addr")); + let (shutdown_tx, mut shutdown_rx) = tokio::sync::oneshot::channel::<()>(); + let task = tokio::spawn(async move { + loop { + tokio::select! { + _ = &mut shutdown_rx => break, + accepted = listener.accept() => { + let Ok((mut stream, _)) = accepted else { break; }; + let mut req = [0_u8; 2048]; + let _ = stream.read(&mut req).await; + let body = "{\"error\":\"mock /auth/me 500\"}"; + let response = format!( + "HTTP/1.1 500 Internal Server Error\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", + body.len(), + body + ); + let _ = stream.write_all(response.as_bytes()).await; + let _ = stream.shutdown().await; + } + } + } + }); + (url, task, shutdown_tx) +} + +/// Store-time `/auth/me` failure must surface as `Err` (and NOT persist a +/// profile), which is what bounces the user back to signin after a "successful" +/// OAuth. Covers the WARN/`Err` gate added in `credentials::ops::store_session`. +#[tokio::test] +async fn store_session_auth_me_failure_returns_err_and_does_not_persist() { + let _lock = env_lock(); + let (api_url, server_task, shutdown_tx) = auth_me_failing_server().await; + let harness = setup(&api_url); + let config = harness.config().await; + + // Non-local token (3 dot-parts, not ".local") forces the backend + // `GET /auth/me` validation path inside `store_session`. + let result = store_session(&config, "header.payload.signature", None, None).await; + + let _ = shutdown_tx.send(()); + let _ = server_task.await; + + let err = result.expect_err("store_session must fail when GET /auth/me returns 500"); + // Lock the cross-layer error-string contract: this exact prefix is what the + // frontend `classifyAuthStoreFailure` matches on. Assert `starts_with` (not + // just `contains`) so a reword in `store_session`/`rest.rs` fails CI here + // instead of silently degrading the FE classifier to 'other'. + assert!( + err.starts_with("Session validation failed (GET /auth/me):"), + "store_session error must keep the contract prefix; got: {err}" + ); + + // Explicitly verify the failure path persisted NOTHING — the gate returns + // before the persist step, so the snapshot must read back as unauthenticated + // with no session token (a partial regression that wrote a profile would + // otherwise still pass on the Err check alone). This is what leaves the user + // on the signin page. + let snap = snapshot() + .await + .expect("snapshot after failed store_session") + .value; + assert!( + !snap.auth.is_authenticated, + "auth must remain unauthenticated after a failed store_session" + ); + assert!( + snap.session_token.is_none(), + "session token must not be persisted after a failed store_session" + ); +} + #[tokio::test] async fn round14_snapshot_preserves_rich_local_state_with_backend_or_stored_user() { let _lock = env_lock();