Skip to content

Commit 076971b

Browse files
authored
Merge pull request #268 from quarto-dev/bugfix/bd-3o8zmz46-ws-auth-expiry
fix(bd-3o8zmz46): evidence-based session expiry handling in hub-client
2 parents 8330451 + 5b14edd commit 076971b

13 files changed

Lines changed: 630 additions & 67 deletions

File tree

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# WS auth-expiry handling (bd-3o8zmz46)
2+
3+
## Overview
4+
5+
Diagnosed 2026-06-10: when the Google ID token (1 h `exp`) expires and GIS One
6+
Tap renewal is unavailable (observed `accounts.google.com/gsi/status` 403 in
7+
dev), the auth cookie is evicted by the browser and every Automerge WS upgrade
8+
gets 401 from the hub. The SPA never notices:
9+
10+
- Browsers hide the HTTP status of a failed WS upgrade from JS, and nothing
11+
probes `/auth/me` out-of-band when sync drops.
12+
- `useAuth`'s visibilitychange handler resets `cookieSetAt = Date.now()` on
13+
every refocus where `/auth/me` still returns 200, drifting the assumed
14+
expiry up to an hour past the real one, so the `cookieExpired()` guards and
15+
the App's auth-lost effect never fire.
16+
17+
Net effect: UI stays "logged in", shows "Connection lost — working offline",
18+
and the WS adapter retries forever.
19+
20+
Evidence record (end-to-end): probed `ws://localhost:5173/ws` (Vite proxy)
21+
and `http://127.0.0.1:3000/ws` directly → both 401; user's DevTools showed
22+
the failing upgrades carry **no Cookie header** (cookie evicted at expiry);
23+
`fetch('/auth/me')` → 401 while the editor remained open; page reload landed
24+
on the login screen.
25+
26+
**Offline-mode invariant (drives all fixes):** only a definitive 401/403 from
27+
a reachable hub may clear auth. Network errors, timeouts, and 5xx must never
28+
log the user out — offline editing is a feature. Logout on evidence, not on
29+
schedule.
30+
31+
Related: bd-ey6jg70f (hub-minted sliding sessions, out of scope here).
32+
33+
## Phase 1 — server: expose token expiry on /auth/me
34+
35+
- [x] Failing test: `/auth/me` response includes `exp` (epoch seconds) equal
36+
to the token's `exp` claim
37+
- [x] Surface `exp` in `OidcClaims` (if not already deserialized) and add it
38+
to `AuthMeResponse` in `crates/quarto-hub/src/server.rs`
39+
- [x] Targeted tests pass (`cargo nextest run -p quarto-hub`)
40+
41+
## Phase 2 — client: schedule from real expiry; offline-safe catches
42+
43+
- [x] Failing tests (vitest, fake timers) for `useAuth`:
44+
- schedules silent refresh at `exp − 15 min` and hard re-check at `exp`,
45+
from the server-reported `exp` (not a local 1 h assumption)
46+
- refocus `/auth/me` 200 does NOT extend assumed expiry beyond server `exp`
47+
(the drift bug)
48+
- refocus `/auth/me` network error keeps auth (today: logs out)
49+
- expiry-time `/auth/me` network error keeps auth and reschedules a
50+
re-check (today: logs out)
51+
- expiry-time `/auth/me` 401 with failed renewal → auth cleared with
52+
`sessionExpired` flag
53+
- [x] Implement: `AuthState.expiresAt` (from `exp`, fallback +1 h if absent),
54+
remove `cookieSetAt` drift resets, offline-safe catch branches,
55+
`sessionExpired` state exposed from the hook
56+
- [x] Targeted vitest run green
57+
58+
## Phase 3 — client: WS-failure auth probe + session-expired UX
59+
60+
- [x] Failing tests for new `useAuthProbe` hook:
61+
- while sync is disconnected (and authed, project open): probes
62+
`/auth/me` immediately and then on an interval
63+
- probe 200 → no action; probe network error → no action (offline mode)
64+
- first 401/403 → `triggerRefresh()` only (renewal gets a chance)
65+
- second consecutive 401/403 → `onAuthRejected()` (clears auth)
66+
- reconnect / 200 resets the strike counter; probing stops when online
67+
- [x] Implement `useAuthProbe`, wire in `App.tsx` off `isOnline`
68+
- [x] LoginScreen shows "Session expired — please sign in again" when auth
69+
was cleared by evidence (distinct from the generic offline banner and
70+
from ordinary logout)
71+
- [x] Targeted vitest run green
72+
73+
## Phase 4 — verification & bookkeeping
74+
75+
- [x] `npm run build:all` from hub-client (production build is stricter)
76+
- [x] `npm run test:ci` from hub-client
77+
- [x] `cargo xtask verify` (full: quarto-hub Rust change + hub-client change)
78+
- [x] Commits (hub-client two-commit changelog dance), strand comment + close
79+
- [x] Honest E2E note: real-expiry flow needs a browser with a live Google
80+
session; record what was and wasn't exercised end-to-end
81+
82+
## Design details
83+
84+
- Probe three-way split maps directly onto `fetchAuthMe()`'s contract:
85+
`AuthState` = valid, `null` = definitive 401/403, throw = network/5xx.
86+
- Strike-2 semantics: first 401 triggers renewal; only a second 401 on the
87+
next probe cycle (~30 s later) clears auth. Avoids racing One Tap and
88+
avoids relying on One Tap callbacks ever firing (they may not when GIS is
89+
blocked — the coalesced `isRefreshing` flag would otherwise wedge).
90+
- Cold-starting the app while fully offline still lands on login (mount-time
91+
probe can't succeed; HttpOnly cookie unreadable client-side). Known
92+
limitation, unchanged by this strand; belongs to bd-ey6jg70f territory.
93+
94+
## Verification record (2026-06-10)
95+
96+
- `cargo xtask verify` (full): 9648 Rust tests passed; hub-client 574 unit +
97+
66 integration + 81 wasm tests passed; WASM + production builds green.
98+
- New tests: `auth_me_returns_token_exp` (Rust); 5 expiry/offline tests in
99+
`useAuth.test.tsx`; 6 tests in `useAuthProbe.test.tsx`; 2 LoginScreen tests.
100+
- Commits: `465de01f` (fix), `329e3702` (changelog).
101+
- **E2E honesty note**: the real expiry flow (Google token aging past 1 h with
102+
One Tap blocked) was NOT exercised end-to-end in a browser — it needs a live
103+
Google session. Manual recipe: sign in, delete the `quarto_hub_token` cookie
104+
in DevTools → Application, restart the hub (drops the established WS); the
105+
client should attempt renewal within ~30 s and land on the login screen with
106+
the "session expired" message within ~60 s, instead of retrying forever.

crates/quarto-hub/src/auth.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,13 @@ pub struct OidcClaims {
195195
/// [`validate_azp_and_iat`]).
196196
#[serde(default)]
197197
pub iat: Option<i64>,
198+
199+
/// Expiry (epoch seconds). Required by the validator
200+
/// (`set_required_spec_claims`), so always present in accepted
201+
/// tokens; surfaced on `/auth/me` so the client can schedule
202+
/// refresh from the real expiry (bd-3o8zmz46).
203+
#[serde(default)]
204+
pub exp: i64,
198205
}
199206

200207
/// Accept either a single-string `aud` or a `["a", "b"]` array.
@@ -709,6 +716,7 @@ mod tests {
709716
aud: vec!["test-client-id".to_string()],
710717
azp: None,
711718
iat: None,
719+
exp: 0,
712720
}
713721
}
714722

@@ -947,6 +955,7 @@ mod tests {
947955
aud: vec!["spa-client".into()],
948956
azp: None,
949957
iat: None,
958+
exp: 0,
950959
};
951960
let allowed = allowed_list();
952961
assert!(validate_azp_and_iat(&claims, allowed.iter(), 1_000_000, 60).is_ok());
@@ -963,6 +972,7 @@ mod tests {
963972
aud: vec!["spa-client".into(), "mcp-client".into()],
964973
azp: None,
965974
iat: None,
975+
exp: 0,
966976
};
967977
let allowed = allowed_list();
968978
assert_eq!(
@@ -982,6 +992,7 @@ mod tests {
982992
aud: vec!["spa-client".into()],
983993
azp: Some("attacker-client".into()),
984994
iat: None,
995+
exp: 0,
985996
};
986997
let allowed = allowed_list();
987998
assert_eq!(
@@ -1001,6 +1012,7 @@ mod tests {
10011012
aud: vec!["spa-client".into(), "mcp-client".into()],
10021013
azp: Some("spa-client".into()),
10031014
iat: None,
1015+
exp: 0,
10041016
};
10051017
let allowed = allowed_list();
10061018
assert!(validate_azp_and_iat(&claims, allowed.iter(), 1_000_000, 60).is_ok());
@@ -1017,6 +1029,7 @@ mod tests {
10171029
aud: vec!["spa-client".into()],
10181030
azp: None,
10191031
iat: Some(1_000_000 + 3600),
1032+
exp: 0,
10201033
};
10211034
let allowed = allowed_list();
10221035
assert_eq!(
@@ -1036,6 +1049,7 @@ mod tests {
10361049
aud: vec!["spa-client".into()],
10371050
azp: None,
10381051
iat: Some(1_000_000 + 30),
1052+
exp: 0,
10391053
};
10401054
let allowed = allowed_list();
10411055
assert!(validate_azp_and_iat(&claims, allowed.iter(), 1_000_000, 60).is_ok());

crates/quarto-hub/src/server.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,9 @@ struct AuthMeResponse {
733733
email: String,
734734
name: Option<String>,
735735
picture: Option<String>,
736+
/// Token expiry (epoch seconds) so the client can schedule silent
737+
/// refresh from the real expiry instead of assuming a fixed lifetime.
738+
exp: i64,
736739
}
737740

738741
/// Query parameters for GET /auth/actor.
@@ -770,6 +773,7 @@ async fn auth_me(
770773
email: claims.email,
771774
name: claims.name,
772775
picture: claims.picture,
776+
exp: claims.exp,
773777
}))
774778
}
775779

crates/quarto-hub/tests/auth_bearer.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,33 @@ async fn cookie_still_authenticates() {
10021002
assert_eq!(resp.status(), 200);
10031003
}
10041004

1005+
// ── /auth/me reports token expiry (bd-3o8zmz46) ──────────────────
1006+
1007+
#[tokio::test]
1008+
async fn auth_me_returns_token_exp() {
1009+
let (provider, hub) = shared_setup().await;
1010+
let exp = chrono::Utc::now().timestamp() + 600;
1011+
let token = provider.sign(
1012+
&ClaimsBuilder::from_provider(provider)
1013+
.sub("auth-me-exp")
1014+
.exp(exp)
1015+
.to_value(),
1016+
);
1017+
1018+
let resp = hub
1019+
.get_auth_me()
1020+
.header("cookie", format!("quarto_hub_token={token}"))
1021+
.send()
1022+
.await
1023+
.unwrap();
1024+
assert_eq!(resp.status(), 200);
1025+
let body: serde_json::Value = resp.json().await.unwrap();
1026+
assert_eq!(
1027+
body["exp"], exp,
1028+
"client schedules refresh from the real token expiry"
1029+
);
1030+
}
1031+
10051032
// ── Dual-credential 400 (bd-wzhsf CVE) ───────────────────────────
10061033

10071034
#[tokio::test]

hub-client/changelog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ be in reverse chronological order (latest first).
1515
1616
-->
1717

18+
### 2026-06-10
19+
20+
- [`465de01f`](https://github.com/quarto-dev/q2/commits/465de01f): An expired sign-in no longer presents as a permanent "working offline" state — the client now detects the rejected session, attempts silent renewal, and returns to the login screen with a "session expired" message; genuine network outages keep offline editing intact.
21+
1822
### 2026-06-09
1923

2024
- [`57bc49cf`](https://github.com/quarto-dev/q2/commits/57bc49cf): Preview renders no longer re-copy unchanged artifacts (theme CSS, fonts, shared JS) into the virtual filesystem on every keystroke — byte-identical re-writes are now skipped.

hub-client/src/App.tsx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { getUserIdentity, updateUserName } from './services/userSettings';
3434
import { useRouting } from './hooks/useRouting';
3535
import { useProjectSet } from './hooks/useProjectSet';
3636
import { useAuth } from './hooks/useAuth';
37+
import { useAuthProbe } from './hooks/useAuthProbe';
3738
import { fetchActorId } from './services/authService';
3839
import type { Route, ShareRoute, LinkProjectSetRoute } from './utils/routing';
3940
import './App.css';
@@ -63,7 +64,14 @@ const AUTH_ENABLED = !!import.meta.env.VITE_GOOGLE_CLIENT_ID;
6364

6465

6566
function App() {
66-
const { auth, loading: authLoading, logout, triggerRefresh } = useAuth();
67+
const {
68+
auth,
69+
loading: authLoading,
70+
logout,
71+
triggerRefresh,
72+
sessionExpired,
73+
expireSession,
74+
} = useAuth();
6775

6876
const [project, setProject] = useState<ProjectEntry | null>(null);
6977
const [files, setFiles] = useState<FileEntry[]>([]);
@@ -76,6 +84,17 @@ function App() {
7684
const [identities, setIdentities] = useState<Record<string, ActorIdentity>>({});
7785
const [isOnline, setIsOnline] = useState<boolean>(false);
7886

87+
// While a project's sync is disconnected, check whether the disconnect is
88+
// actually an auth rejection (browsers hide the WS upgrade status). Only
89+
// definitive 401/403 evidence ever clears auth — never network errors.
90+
// Past the token's exp, useAuth's expiry timer logs out on the first 401
91+
// (preempting this probe's two-strike); the probe governs earlier drops.
92+
useAuthProbe({
93+
enabled: AUTH_ENABLED && !!auth && !!project && !isOnline,
94+
triggerRefresh,
95+
onAuthRejected: expireSession,
96+
});
97+
7998
// Project set management (synced project list)
8099
const [projectSetState, projectSetActions] = useProjectSet();
81100

@@ -551,7 +570,12 @@ function App() {
551570
}
552571

553572
if (AUTH_ENABLED && !auth) {
554-
return <LoginScreen error={authError} />;
573+
return (
574+
<LoginScreen
575+
error={authError}
576+
message={sessionExpired ? 'Your session expired — please sign in again.' : undefined}
577+
/>
578+
);
555579
}
556580

557581
// Gate on screen name being loaded (fast IndexedDB read).

hub-client/src/components/auth/LoginScreen.test.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,16 @@ describe('LoginScreen', () => {
4949
expect(screen.getByText(/Sign-in failed/i)).toBeTruthy();
5050
expect(screen.queryByText(/Sign in with Google to continue/i)).toBeNull();
5151
});
52+
53+
it('renders a custom message (session expiry) instead of the default copy', () => {
54+
render(withProvider(<LoginScreen message="Your session expired — please sign in again." />));
55+
expect(screen.getByText(/session expired/i)).toBeTruthy();
56+
expect(screen.queryByText(/Sign in with Google to continue/i)).toBeNull();
57+
});
58+
59+
it('error copy wins over a custom message', () => {
60+
render(withProvider(<LoginScreen error message="Your session expired — please sign in again." />));
61+
expect(screen.getByText(/Sign-in failed/i)).toBeTruthy();
62+
expect(screen.queryByText(/session expired/i)).toBeNull();
63+
});
5264
});

hub-client/src/components/auth/LoginScreen.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
import { useAuthProvider } from '../../auth/AuthProvider';
1515

16-
export function LoginScreen({ error }: { error?: boolean }) {
16+
export function LoginScreen({ error, message }: { error?: boolean; message?: string }) {
1717
const provider = useAuthProvider();
1818

1919
return (
@@ -25,6 +25,10 @@ export function LoginScreen({ error }: { error?: boolean }) {
2525
<p style={{ color: 'var(--posit-red)', fontSize: '14px', margin: '0 0 16px' }}>
2626
Sign-in failed. Your account is not authorized to access this hub.
2727
</p>
28+
) : message ? (
29+
<p style={{ color: 'var(--text-secondary)', fontSize: '14px', margin: '0 0 16px' }}>
30+
{message}
31+
</p>
2832
) : (
2933
<p style={{ color: 'var(--text-secondary)', fontSize: '14px', margin: '0 0 16px' }}>
3034
Sign in with Google to continue

0 commit comments

Comments
 (0)