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
15 changes: 14 additions & 1 deletion claude-notes/plans/2026-05-28-hub-mcp-loopback-pkce.md
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,9 @@ relevant item only if a spike contradicts an assumption.
IdP anyway (they expire on their own ≤1 h timer; the hub-side
`sub_denylist` deferred to future work is the right closure for
that window, same as it is for stolen-token-without-clear).
*(2026-08-03 update: shipped as bd-jkih1ql7 — the revocation
ledger now gates the Bearer path; see
`2026-08-03-bearer-revocation-and-mcp-auth-followups.md` F1.)*
- **Best-effort: revocation failure does NOT block local cleanup.**
Network errors, 5xx, expired-tokens-returning-200/400-with-
`invalid_token` — the local delete proceeds regardless. The
Expand Down Expand Up @@ -992,6 +995,10 @@ relevant item only if a spike contradicts an assumption.
hub for up to ≤1 h (ID) / indefinitely (refresh, until user
revokes grant). Closing the ID-token window still requires
the hub-side `sub_denylist` deferred from v1.
*(2026-08-03 update: the ID-token window is now closed for
hub-side events — bans and logout-everywhere gate the Bearer
path (bd-jkih1ql7). The refresh-token residual stands; see
`2026-08-03-bearer-revocation-and-mcp-auth-followups.md` F1.)*
- **Unchanged:** brand-confusion residual. If an attacker
already has code execution on the victim's machine, they can
drive a real loopback flow under our `client_id` and capture
Expand Down Expand Up @@ -1188,4 +1195,10 @@ unchecked above):
it. Revisit if we move to self-hosted OIDC.
- **`sub_denylist` on the hub side** to close the ≤1 h stolen-ID-token
window. Already noted as future work in the existing device-flow
plan; cross-listed here.
plan; cross-listed here. **Done (2026-08-03, bd-jkih1ql7):** rather
than a separate denylist, the existing revocation ledger (bans +
logout-everywhere `not_before` floors) is enforced on the Bearer
path — 403 `user_banned` / 401 `bearer_revoked`, anchored at the
token's `iat`, failing closed when `iat` is absent. Refresh-token
theft remains a Google-side revocation matter. See
`2026-08-03-bearer-revocation-and-mcp-auth-followups.md` F1.

Large diffs are not rendered by default.

71 changes: 69 additions & 2 deletions crates/quarto-hub/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,20 +553,30 @@ impl HubContext {
/// mint-time validation in `auth_callback`/`auth_session`, whose
/// input is a fresh Google credential from the request body — never
/// the cookie (that path is [`Self::authenticate_session`]).
///
/// Mint-time semantics: the revocation ledger is **skipped** here
/// (see [`RevocationEnforcement::Skip`]) — bans gate mint
/// explicitly in the handlers, and the `min_auth_time` clamp
/// handles `not_before` floors so same-second re-login after a
/// logout-everywhere keeps working.
pub async fn authenticate_claims(
&self,
token: Option<&str>,
) -> std::result::Result<OidcClaims, StatusCode> {
self.authenticate_claims_for_kind(token, "unknown").await
self.authenticate_claims_for_kind(token, "unknown", RevocationEnforcement::Skip)
.await
}

/// Variant of [`authenticate_claims`] that records the
/// `credential_kind` (`"bearer"` / `"unknown"`) on every audit
/// event, as required by Phase 2 of the device-flow plan.
/// event, as required by Phase 2 of the device-flow plan, and lets
/// the caller pick the revocation-ledger posture (request
/// credentials enforce; mint-time validation skips).
pub async fn authenticate_claims_for_kind(
&self,
token: Option<&str>,
credential_kind: &'static str,
revocation: RevocationEnforcement,
) -> std::result::Result<OidcClaims, StatusCode> {
let auth_config = self.auth_config().ok_or_else(|| {
tracing::event!(
Expand Down Expand Up @@ -660,6 +670,46 @@ impl HubContext {
return Err(status);
}

// Revocation ledger on the request-credential path (bd-jkih1ql7):
// bans and logout-everywhere floors must bite Bearer credentials
// too. Anchored at the token's `iat`; a missing `iat` (the type
// admits it, OIDC requires it) fails closed against any
// `not_before` entry. Must run before the `auth_ok` emission —
// an allow-then-deny pair for one request would corrupt the
// audit log.
if revocation == RevocationEnforcement::Enforce {
let anchor = token_data.claims.iat.unwrap_or(0);
match self.revocations.check(&token_data.claims.sub, anchor).await {
RevocationStatus::Banned => {
tracing::event!(
target: "quarto_hub::audit",
tracing::Level::INFO,
action = "auth_fail",
outcome = "deny",
credential_kind = credential_kind,
sub = %token_data.claims.sub,
detail = "user_banned",
);
return Err(StatusCode::FORBIDDEN);
}
RevocationStatus::Revoked => {
// Deliberately not `session_revoked` — it isn't a
// session; the dead thing is the Bearer token itself.
tracing::event!(
target: "quarto_hub::audit",
tracing::Level::INFO,
action = "auth_fail",
outcome = "deny",
credential_kind = credential_kind,
sub = %token_data.claims.sub,
detail = "bearer_revoked",
);
return Err(StatusCode::UNAUTHORIZED);
}
RevocationStatus::Ok => {}
}
}

tracing::event!(
target: "quarto_hub::audit",
tracing::Level::INFO,
Expand Down Expand Up @@ -833,13 +883,30 @@ impl HubContext {
.authenticate_claims_for_kind(
Some(token),
crate::server::CredentialKind::Bearer.label(),
RevocationEnforcement::Enforce,
)
.await
.map(AuthenticatedUser::Google),
}
}
}

/// Whether [`HubContext::authenticate_claims_for_kind`] enforces the
/// revocation ledger (bans + `not_before` floors) on the validated
/// claims.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RevocationEnforcement {
/// Request-credential path (the Bearer arm of
/// [`HubContext::authenticate_credential`]): a banned `sub` is 403,
/// an `iat` below the user's `not_before` floor is 401.
Enforce,
/// Mint-time validation (`auth_callback` / `auth_session`): bans
/// gate mint explicitly in the handlers, and the `min_auth_time`
/// clamp handles the floor — a raw `iat` check here would break
/// same-second re-login after logout-everywhere.
Skip,
}

/// A successfully validated request credential, tagged with the path
/// that verified it.
#[derive(Debug, Clone)]
Expand Down
15 changes: 13 additions & 2 deletions crates/quarto-hub/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1209,9 +1209,18 @@ struct AuthMeResponse {
email: String,
name: Option<String>,
picture: Option<String>,
/// Token expiry (epoch seconds) so the client can schedule silent
/// refresh from the real expiry instead of assuming a fixed lifetime.
/// Expiry (epoch seconds) **of the presented credential** — the
/// semantics depend on `credential`: a *sliding* session expiry on
/// the cookie path (authenticated activity extends it; the SPA
/// schedules its expiry re-check from it), but the Google token's
/// *fixed* expiry on the Bearer path (nothing slides; the client
/// refreshes at the IdP). `credential` is the discriminator
/// (bd-aw8f3sp8).
exp: i64,
/// Which verification path authenticated this request: `"session"`
/// (hub-minted cookie) or `"bearer"` (Google ID token). Mirrors the
/// [`crate::context::AuthenticatedUser`] variants.
credential: &'static str,
}

/// Query parameters for GET /auth/actor.
Expand Down Expand Up @@ -1276,12 +1285,14 @@ async fn auth_me(
name: v.claims.name,
picture: v.claims.picture,
exp: v.claims.exp,
credential: "session",
},
crate::context::AuthenticatedUser::Google(claims) => AuthMeResponse {
email: claims.email,
name: claims.name,
picture: claims.picture,
exp: claims.exp,
credential: "bearer",
},
};
Ok(Json(response))
Expand Down
Loading
Loading