fix(auth): surface the silent /auth/me store-failure that bounces login to signin (#3734)

This commit is contained in:
sanil-23
2026-06-17 18:26:17 +05:30
committed by GitHub
parent decfc5b89b
commit a229edea36
4 changed files with 217 additions and 4 deletions
@@ -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');
});
});
+41
View File
@@ -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
+20 -4
View File
@@ -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();
@@ -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();