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
72 changes: 72 additions & 0 deletions app/src/utils/__tests__/desktopDeepLinkListener.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '../../store/deepLinkAuthState';
import { getStoredCoreMode } from '../configPersistence';
import {
classifyAuthStoreFailure,
registerAuthDeepLinkState,
setupDesktopDeepLinkListener,
} from '../desktopDeepLinkListener';
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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');
});
});
41 changes: 41 additions & 0 deletions app/src/utils/desktopDeepLinkListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
console.warn('[DeepLink][auth] session store failed — staying on signin (kind=%s)', kind);
failDeepLinkAuthProcessing('Sign-in failed. Please try again.');
}
}
Expand All @@ -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
Expand Down
24 changes: 20 additions & 4 deletions src/openhuman/credentials/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
};

let mut metadata = std::collections::HashMap::new();
Expand Down
84 changes: 84 additions & 0 deletions tests/app_state_credentials_raw_coverage_e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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"
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#[tokio::test]
async fn round14_snapshot_preserves_rich_local_state_with_backend_or_stored_user() {
let _lock = env_lock();
Expand Down
Loading