fix(server): revoke viewer tokens on logout + stop logging chat content
Security follow-ups (audit 2026-05-30, both reviewed APPROVE): - MEDIUM: viewer tokens were never blacklisted on logout, so a minted session-scoped viewer token stayed valid up to its 5-min TTL after the user logged out. Add a per-user ViewerTokenRegistry (Arc<Mutex<HashMap<sub, Vec<(token, expires_at)>>>>, prune-on-insert) on AppState; mint_viewer_token registers each token under the user sub; logout drains take_for_user(sub) and blacklists each via the existing token_blacklist. The viewer WS already calls is_revoked, so no WS change. Key chain user.user_id == ViewerClaims.sub == registry key verified consistent. 8 new tests. - LOW: relay chat logs now emit content length, not the chat body (support-chat can carry secrets/PII). cargo fmt/clippy(-D warnings)/test green on GURU-5070 (37 agent + 61 server). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -60,10 +60,27 @@ pub async fn logout(
|
||||
// Extract token from headers
|
||||
let token = extract_token_from_headers(request.headers())?;
|
||||
|
||||
// Add token to blacklist
|
||||
// Add the login JWT to the blacklist.
|
||||
state.token_blacklist.revoke(&token).await;
|
||||
|
||||
info!("User {} logged out (token revoked)", user.username);
|
||||
// Also revoke any outstanding session-scoped VIEWER tokens this user minted
|
||||
// (CRITICAL #2). The login-JWT blacklist alone leaves a viewer token minted
|
||||
// before logout valid for the rest of its 5-minute TTL, keeping a live
|
||||
// viewer/remote-control plane open after logout. `user.user_id` is the `sub`
|
||||
// the viewer tokens were registered under (the same claim stamped into them).
|
||||
// The viewer WS already blacklist-checks the exact token string, so adding
|
||||
// them here is sufficient — no WS change needed. take_for_user drains and
|
||||
// clears the registry entry; expired tokens are pruned (not returned).
|
||||
let viewer_tokens = state.viewer_tokens.take_for_user(&user.user_id);
|
||||
let revoked_viewer_count = viewer_tokens.len();
|
||||
for viewer_token in viewer_tokens {
|
||||
state.token_blacklist.revoke(&viewer_token).await;
|
||||
}
|
||||
|
||||
info!(
|
||||
"User {} logged out (login token revoked, {} viewer token(s) revoked)",
|
||||
user.username, revoked_viewer_count
|
||||
);
|
||||
|
||||
Ok(Json(LogoutResponse {
|
||||
message: "Logged out successfully".to_string(),
|
||||
@@ -196,3 +213,74 @@ pub async fn cleanup_blacklist(
|
||||
remaining_count: remaining,
|
||||
}))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::auth::{JwtConfig, TokenBlacklist, ViewerAccess, ViewerTokenRegistry};
|
||||
use std::time::Duration;
|
||||
use uuid::Uuid;
|
||||
|
||||
/// End-to-end (component-level) proof of CRITICAL #2: a viewer token minted
|
||||
/// under a user, then revoked at logout via the registry drain, is rejected
|
||||
/// by the SAME blacklist check the viewer WS runs (`is_revoked`).
|
||||
///
|
||||
/// This exercises the real mint → register → logout-drain → blacklist path
|
||||
/// without the HTTP/DB plumbing of the full handler. Uses local component
|
||||
/// instances only (no process-global env), so it is parallel-safe.
|
||||
#[tokio::test]
|
||||
async fn logout_revokes_minted_viewer_token() {
|
||||
let jwt = JwtConfig::new("test-secret-at-least-32-chars-long!!".to_string(), 24);
|
||||
let registry = ViewerTokenRegistry::new();
|
||||
let blacklist = TokenBlacklist::new();
|
||||
|
||||
let user_sub = Uuid::new_v4().to_string();
|
||||
let session_id = Uuid::new_v4();
|
||||
let tenant_id = Uuid::new_v4();
|
||||
|
||||
// Mint a viewer token and register it under the user (as mint_viewer_token does).
|
||||
let viewer_token = jwt
|
||||
.create_viewer_token(&user_sub, session_id, tenant_id, ViewerAccess::Control)
|
||||
.unwrap();
|
||||
registry.register(&user_sub, &viewer_token, Duration::from_secs(300));
|
||||
|
||||
// The viewer WS check (is_revoked) passes BEFORE logout — token is live.
|
||||
assert!(!blacklist.is_revoked(&viewer_token).await);
|
||||
|
||||
// Logout drains the user's viewer tokens into the blacklist (handler logic).
|
||||
for tok in registry.take_for_user(&user_sub) {
|
||||
blacklist.revoke(&tok).await;
|
||||
}
|
||||
|
||||
// After logout the same WS check now REJECTS the viewer token.
|
||||
assert!(blacklist.is_revoked(&viewer_token).await);
|
||||
// The token also remains a structurally valid viewer JWT (not expired),
|
||||
// proving revocation — not natural expiry — is what blocks it.
|
||||
assert!(jwt.validate_viewer_token(&viewer_token).is_ok());
|
||||
}
|
||||
|
||||
/// A different user's logout must NOT revoke this user's viewer token.
|
||||
#[tokio::test]
|
||||
async fn logout_does_not_revoke_other_users_viewer_token() {
|
||||
let jwt = JwtConfig::new("test-secret-at-least-32-chars-long!!".to_string(), 24);
|
||||
let registry = ViewerTokenRegistry::new();
|
||||
let blacklist = TokenBlacklist::new();
|
||||
|
||||
let user_a = Uuid::new_v4().to_string();
|
||||
let user_b = Uuid::new_v4().to_string();
|
||||
let session_id = Uuid::new_v4();
|
||||
let tenant_id = Uuid::new_v4();
|
||||
|
||||
let token_b = jwt
|
||||
.create_viewer_token(&user_b, session_id, tenant_id, ViewerAccess::ViewOnly)
|
||||
.unwrap();
|
||||
registry.register(&user_b, &token_b, Duration::from_secs(300));
|
||||
|
||||
// user_a logs out — drains only user_a's (empty) token set.
|
||||
for tok in registry.take_for_user(&user_a) {
|
||||
blacklist.revoke(&tok).await;
|
||||
}
|
||||
|
||||
// user_b's token is untouched.
|
||||
assert!(!blacklist.is_revoked(&token_b).await);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user