Replaces URL-embedded JWT tokens with a single-use, 60-second handoff code that the SPA exchanges via server-to-server POST. The URL now contains only `?handoff=<code>` — no tokens are placed in the browser history, proxy access logs, or Referer header. Backend: new SsoHandoff store (DashMap, 60s TTL, atomic DashMap::remove for single-use), POST /api/v1/auth/sso/handoff endpoint, 7 new tests. Frontend: SsoCallbackPage rewritten to use useSearchParams + POST exchange, with history.replaceState to clear the handoff code from the address bar. Switched from window.location.search to useSearchParams() for test compatibility. New Vitest infrastructure (vitest, @testing-library/react, jsdom) and 6 new tests. CI fix in ccba9e3: cargo fmt --all and added searchParams to useEffect dep array to satisfy CI's Rust Format and Frontend Lint checks. Refs: closes #4
This commit is contained in:
committed by
GitHub
parent
3bdae4bcc5
commit
f58d7a6f17
@ -12,7 +12,7 @@ use pm_auth::{
|
|||||||
use pm_core::{
|
use pm_core::{
|
||||||
config::AppConfig, db, logging, models::ApprovedEntry, request_id::request_id_middleware,
|
config::AppConfig, db, logging, models::ApprovedEntry, request_id::request_id_middleware,
|
||||||
};
|
};
|
||||||
use routes::sso::{OidcCache, SsoSession};
|
use routes::sso::{OidcCache, SsoHandoff, SsoSession};
|
||||||
use routes::ws::WsTicket;
|
use routes::ws::WsTicket;
|
||||||
use serde_json::{json, Value};
|
use serde_json::{json, Value};
|
||||||
use std::{net::SocketAddr, sync::Arc, time::Duration};
|
use std::{net::SocketAddr, sync::Arc, time::Duration};
|
||||||
@ -36,6 +36,9 @@ pub struct AppState {
|
|||||||
pub ws_tickets: Arc<DashMap<String, WsTicket>>,
|
pub ws_tickets: Arc<DashMap<String, WsTicket>>,
|
||||||
/// In-memory store for SSO PKCE sessions (state → code_verifier).
|
/// In-memory store for SSO PKCE sessions (state → code_verifier).
|
||||||
pub sso_sessions: Arc<DashMap<String, SsoSession>>,
|
pub sso_sessions: Arc<DashMap<String, SsoSession>>,
|
||||||
|
/// In-memory store for SSO handoff codes (single-use, 60s TTL).
|
||||||
|
/// See `tasks/sso-token-handoff-spec.md` §4.1.
|
||||||
|
pub sso_handoffs: Arc<DashMap<String, SsoHandoff>>,
|
||||||
/// Cached OIDC discovery document and JWKS for SSO id_token verification.
|
/// Cached OIDC discovery document and JWKS for SSO id_token verification.
|
||||||
pub oidc_cache: Arc<Mutex<OidcCache>>,
|
pub oidc_cache: Arc<Mutex<OidcCache>>,
|
||||||
/// Internal certificate authority for mTLS client cert issuance.
|
/// Internal certificate authority for mTLS client cert issuance.
|
||||||
@ -104,6 +107,7 @@ async fn main() -> anyhow::Result<()> {
|
|||||||
|
|
||||||
let ws_tickets: Arc<DashMap<String, WsTicket>> = Arc::new(DashMap::new());
|
let ws_tickets: Arc<DashMap<String, WsTicket>> = Arc::new(DashMap::new());
|
||||||
let sso_sessions: Arc<DashMap<String, SsoSession>> = Arc::new(DashMap::new());
|
let sso_sessions: Arc<DashMap<String, SsoSession>> = Arc::new(DashMap::new());
|
||||||
|
let sso_handoffs: Arc<DashMap<String, SsoHandoff>> = Arc::new(DashMap::new());
|
||||||
let oidc_cache: Arc<Mutex<OidcCache>> = Arc::new(Mutex::new(OidcCache::default()));
|
let oidc_cache: Arc<Mutex<OidcCache>> = Arc::new(Mutex::new(OidcCache::default()));
|
||||||
let approved_enrollments: Arc<DashMap<String, ApprovedEntry>> = Arc::new(DashMap::new());
|
let approved_enrollments: Arc<DashMap<String, ApprovedEntry>> = Arc::new(DashMap::new());
|
||||||
|
|
||||||
@ -163,6 +167,27 @@ async fn main() -> anyhow::Result<()> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Background task: purge expired SSO handoff codes every 60 seconds.
|
||||||
|
// See `tasks/sso-token-handoff-spec.md` §4.3. Handoffs are also
|
||||||
|
// atomically removed on exchange (single-use), so this task only
|
||||||
|
// cleans up codes that the SPA never POSTed back for.
|
||||||
|
{
|
||||||
|
let handoffs = sso_handoffs.clone();
|
||||||
|
tokio::spawn(async move {
|
||||||
|
let mut interval = tokio::time::interval(Duration::from_secs(60));
|
||||||
|
loop {
|
||||||
|
interval.tick().await;
|
||||||
|
let now = std::time::Instant::now();
|
||||||
|
let before = handoffs.len();
|
||||||
|
handoffs.retain(|_, v| v.expires_at > now);
|
||||||
|
let removed = before.saturating_sub(handoffs.len());
|
||||||
|
if removed > 0 {
|
||||||
|
tracing::debug!(removed, "Purged expired SSO handoff codes");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
let state = AppState {
|
let state = AppState {
|
||||||
db: pool,
|
db: pool,
|
||||||
config: Arc::new(config.clone()),
|
config: Arc::new(config.clone()),
|
||||||
@ -170,6 +195,7 @@ async fn main() -> anyhow::Result<()> {
|
|||||||
auth_config,
|
auth_config,
|
||||||
ws_tickets,
|
ws_tickets,
|
||||||
sso_sessions,
|
sso_sessions,
|
||||||
|
sso_handoffs,
|
||||||
ca: Arc::new(ca),
|
ca: Arc::new(ca),
|
||||||
approved_enrollments,
|
approved_enrollments,
|
||||||
oidc_cache,
|
oidc_cache,
|
||||||
|
|||||||
341
crates/pm-web/src/routes/sso.rs
Executable file → Normal file
341
crates/pm-web/src/routes/sso.rs
Executable file → Normal file
@ -12,11 +12,12 @@ use axum::{
|
|||||||
extract::State,
|
extract::State,
|
||||||
http::StatusCode,
|
http::StatusCode,
|
||||||
response::{IntoResponse, Json, Redirect},
|
response::{IntoResponse, Json, Redirect},
|
||||||
routing::get,
|
routing::{get, post},
|
||||||
Router,
|
Router,
|
||||||
};
|
};
|
||||||
use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _};
|
use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _};
|
||||||
use chrono::Utc;
|
use chrono::Utc;
|
||||||
|
use dashmap::DashMap;
|
||||||
use jsonwebtoken::{decode, decode_header, Algorithm, DecodingKey, Validation};
|
use jsonwebtoken::{decode, decode_header, Algorithm, DecodingKey, Validation};
|
||||||
use pm_auth::{jwt::issue_access_token, refresh};
|
use pm_auth::{jwt::issue_access_token, refresh};
|
||||||
use pm_core::audit::{log_event, AuditAction};
|
use pm_core::audit::{log_event, AuditAction};
|
||||||
@ -40,6 +41,140 @@ pub struct SsoSession {
|
|||||||
pub created_at: chrono::DateTime<Utc>,
|
pub created_at: chrono::DateTime<Utc>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Single-use, short-lived payload that the SSO callback hands to the SPA
|
||||||
|
/// via a `?handoff=<code>` query param. The SPA exchanges it via
|
||||||
|
/// `POST /api/v1/auth/sso/handoff` for the actual JWT access/refresh
|
||||||
|
/// tokens. Mirrors the WS-ticket pattern (issue #10): in-memory, atomic
|
||||||
|
/// single-use consume, TTL enforced on read.
|
||||||
|
///
|
||||||
|
/// See `tasks/sso-token-handoff-spec.md` §4.1 for the full design.
|
||||||
|
#[derive(Clone)]
|
||||||
|
pub struct SsoHandoff {
|
||||||
|
/// JWT access token (short-lived, 15 min TTL).
|
||||||
|
pub access_token: String,
|
||||||
|
/// Opaque refresh token (long-lived, rotating).
|
||||||
|
pub raw_refresh: String,
|
||||||
|
/// JSON-serialized user object (id, username, display_name, role, etc.).
|
||||||
|
pub user_json: Value,
|
||||||
|
/// Access token TTL in seconds (for the `expires_in` field in the response).
|
||||||
|
pub access_ttl: u64,
|
||||||
|
/// Expiry instant; the exchange endpoint rejects codes past this time.
|
||||||
|
pub expires_at: std::time::Instant,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// TTL for SSO handoff codes. Short by design: the SPA should POST to
|
||||||
|
/// `/api/v1/auth/sso/handoff` within seconds of the redirect landing.
|
||||||
|
///
|
||||||
|
/// `dead_code` is allowed here because Phase 1 introduces the store
|
||||||
|
/// ahead of its consumer; the SSO callback rewrite in Phase 2 of
|
||||||
|
/// `tasks/sso-token-handoff-spec.md` inserts handoffs with this TTL and
|
||||||
|
/// the exchange handler reads it back to validate freshness.
|
||||||
|
#[allow(dead_code)]
|
||||||
|
pub const HANDOFF_TTL_SECS: u64 = 60;
|
||||||
|
|
||||||
|
/// Generate a cryptographically random handoff code (32 bytes,
|
||||||
|
/// base64url-encoded, ~43 chars). Uses the same `rand` crate family as
|
||||||
|
/// the WS-ticket path.
|
||||||
|
///
|
||||||
|
/// `dead_code` is allowed here for the same reason as `HANDOFF_TTL_SECS`
|
||||||
|
/// — Phase 2 wires it into the SSO callback redirect construction.
|
||||||
|
#[allow(dead_code)]
|
||||||
|
pub fn generate_handoff_code() -> String {
|
||||||
|
use rand::RngCore;
|
||||||
|
let mut bytes = [0u8; 32];
|
||||||
|
rand::thread_rng().fill_bytes(&mut bytes);
|
||||||
|
URL_SAFE_NO_PAD.encode(bytes)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Request body for `POST /api/v1/auth/sso/handoff`.
|
||||||
|
///
|
||||||
|
/// The SPA sends the handoff code it received in the SSO callback
|
||||||
|
/// redirect's `?handoff=...` query param, and the backend exchanges it
|
||||||
|
/// for the actual access/refresh tokens. The code is single-use and
|
||||||
|
/// 60-second TTL.
|
||||||
|
#[derive(Debug, Deserialize)]
|
||||||
|
pub struct HandoffRequest {
|
||||||
|
pub handoff_code: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
// ============================================================
|
||||||
|
// Handoff exchange handler
|
||||||
|
// ============================================================
|
||||||
|
|
||||||
|
/// `POST /api/v1/auth/sso/handoff` — exchange a single-use handoff code
|
||||||
|
/// for the JWT access/refresh tokens + user object. Public route (no
|
||||||
|
/// JWT required) — the handoff code IS the credential.
|
||||||
|
///
|
||||||
|
/// See `tasks/sso-token-handoff-spec.md` §4.2 for the full design.
|
||||||
|
async fn sso_handoff_exchange(
|
||||||
|
State(state): State<AppState>,
|
||||||
|
Json(req): Json<HandoffRequest>,
|
||||||
|
) -> (StatusCode, Json<Value>) {
|
||||||
|
sso_handoff_exchange_inner(&state.sso_handoffs, &req.handoff_code).await
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Core exchange logic, separated from the HTTP handler so tests can
|
||||||
|
/// drive it with a bare `DashMap` (no need to construct a full
|
||||||
|
/// `AppState` with a real `sqlx::PgPool` and `Arc<AppConfig>`).
|
||||||
|
///
|
||||||
|
/// Marked `async` so the race test can use `tokio::join!` to drive
|
||||||
|
/// two concurrent exchanges against the same code; the function body
|
||||||
|
/// has no `.await` points (it only does a DashMap read and a return),
|
||||||
|
/// so this is a zero-cost abstraction.
|
||||||
|
async fn sso_handoff_exchange_inner(
|
||||||
|
handoffs: &DashMap<String, SsoHandoff>,
|
||||||
|
code: &str,
|
||||||
|
) -> (StatusCode, Json<Value>) {
|
||||||
|
// Atomically remove the entry (single-use guarantee). If two
|
||||||
|
// requests race with the same code, DashMap::remove is atomic so
|
||||||
|
// only one wins.
|
||||||
|
let removed = handoffs.remove(code);
|
||||||
|
let Some((_code, handoff)) = removed else {
|
||||||
|
tracing::warn!(
|
||||||
|
reason = "unknown_or_already_consumed",
|
||||||
|
"SSO handoff exchange failed"
|
||||||
|
);
|
||||||
|
return (
|
||||||
|
StatusCode::BAD_REQUEST,
|
||||||
|
Json(json!({
|
||||||
|
"error": { "code": "invalid_handoff", "message": "Handoff code is invalid or has expired" }
|
||||||
|
})),
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
// Check expiry (the cleanup task also removes expired entries, but
|
||||||
|
// there's a race between expiry and the next cleanup tick — check
|
||||||
|
// here too so we never return a token for an expired handoff).
|
||||||
|
if handoff.expires_at <= std::time::Instant::now() {
|
||||||
|
tracing::warn!(reason = "expired", "SSO handoff exchange failed");
|
||||||
|
return (
|
||||||
|
StatusCode::BAD_REQUEST,
|
||||||
|
Json(json!({
|
||||||
|
"error": { "code": "invalid_handoff", "message": "Handoff code is invalid or has expired" }
|
||||||
|
})),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Log success without leaking the handoff code or the tokens
|
||||||
|
let user_id = handoff
|
||||||
|
.user_json
|
||||||
|
.get("id")
|
||||||
|
.and_then(|v| v.as_str())
|
||||||
|
.unwrap_or("unknown");
|
||||||
|
tracing::info!(user_id = %user_id, "SSO handoff exchanged");
|
||||||
|
|
||||||
|
(
|
||||||
|
StatusCode::OK,
|
||||||
|
Json(json!({
|
||||||
|
"access_token": handoff.access_token,
|
||||||
|
"refresh_token": handoff.raw_refresh,
|
||||||
|
"token_type": "Bearer",
|
||||||
|
"expires_in": handoff.access_ttl,
|
||||||
|
"user": handoff.user_json,
|
||||||
|
})),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug, Deserialize)]
|
#[derive(Debug, Deserialize)]
|
||||||
struct TokenResponse {
|
struct TokenResponse {
|
||||||
#[allow(dead_code)]
|
#[allow(dead_code)]
|
||||||
@ -116,6 +251,12 @@ pub fn public_router() -> Router<AppState> {
|
|||||||
.route("/login", get(sso_login))
|
.route("/login", get(sso_login))
|
||||||
.route("/callback", get(sso_callback))
|
.route("/callback", get(sso_callback))
|
||||||
.route("/config", get(sso_config))
|
.route("/config", get(sso_config))
|
||||||
|
// Issue #4: single-use handoff exchange. The SPA POSTs the
|
||||||
|
// `?handoff=<code>` it received from the SSO callback redirect
|
||||||
|
// and gets the JWT access/refresh tokens in the JSON response.
|
||||||
|
// Public route (no JWT) — the handoff code IS the credential.
|
||||||
|
// See `tasks/sso-token-handoff-spec.md` §4.2.
|
||||||
|
.route("/handoff", post(sso_handoff_exchange))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Backward-compatible Azure SSO routes — redirect to generic SSO endpoints.
|
/// Backward-compatible Azure SSO routes — redirect to generic SSO endpoints.
|
||||||
@ -604,13 +745,32 @@ async fn sso_callback(
|
|||||||
"mfa_enabled": user.mfa_enabled,
|
"mfa_enabled": user.mfa_enabled,
|
||||||
});
|
});
|
||||||
|
|
||||||
let redirect_url = format!(
|
// Issue #4 fix: instead of embedding access/refresh tokens in the
|
||||||
"{}?access_token={}&refresh_token={}&token_type=Bearer&expires_in={}&user={}",
|
// redirect URL (which leaks through browser history, proxy access
|
||||||
callback_url,
|
// logs, and the Referer header), generate a single-use, 60s handoff
|
||||||
urlencoding::encode(&access_token),
|
// code, store the payload in `sso_handoffs`, and put ONLY the code
|
||||||
urlencoding::encode(&raw_refresh.0),
|
// in the redirect. The SPA POSTs to `/api/v1/auth/sso/handoff` to
|
||||||
access_ttl,
|
// exchange the code for tokens. See `tasks/sso-token-handoff-spec.md`
|
||||||
urlencoding::encode(&user_json.to_string()),
|
// §4.1.
|
||||||
|
let handoff_code = generate_handoff_code();
|
||||||
|
state.sso_handoffs.insert(
|
||||||
|
handoff_code.clone(),
|
||||||
|
SsoHandoff {
|
||||||
|
access_token: access_token.clone(),
|
||||||
|
raw_refresh: raw_refresh.0.clone(),
|
||||||
|
user_json: user_json.clone(),
|
||||||
|
access_ttl: access_ttl as u64,
|
||||||
|
expires_at: std::time::Instant::now()
|
||||||
|
+ std::time::Duration::from_secs(HANDOFF_TTL_SECS),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
let redirect_url = format!("{}?handoff={}", callback_url, handoff_code);
|
||||||
|
|
||||||
|
tracing::info!(
|
||||||
|
user_id = %user.id,
|
||||||
|
auth_provider = %auth_provider,
|
||||||
|
"SSO handoff issued"
|
||||||
);
|
);
|
||||||
|
|
||||||
Ok(Redirect::to(&redirect_url))
|
Ok(Redirect::to(&redirect_url))
|
||||||
@ -836,3 +996,168 @@ async fn fetch_jwks(jwks_uri: &str) -> Result<serde_json::Value, String> {
|
|||||||
.await
|
.await
|
||||||
.map_err(|e| format!("Failed to parse JWKS response: {}", e))
|
.map_err(|e| format!("Failed to parse JWKS response: {}", e))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
//! Unit tests for the SSO handoff exchange endpoint and cleanup task.
|
||||||
|
//!
|
||||||
|
//! Per `tasks/sso-token-handoff-spec.md` §6.1–6.2.
|
||||||
|
//!
|
||||||
|
//! The tests call `sso_handoff_exchange_inner` directly with a bare
|
||||||
|
//! `DashMap<String, SsoHandoff>`. This avoids the need to construct
|
||||||
|
//! a full `AppState` (which has `sqlx::PgPool` and `Arc<AppConfig>`
|
||||||
|
//! fields that can't be cheaply mocked) and keeps the tests focused
|
||||||
|
//! on the exchange logic. The HTTP handler is a thin wrapper that
|
||||||
|
//! extracts the code from the request body and delegates.
|
||||||
|
|
||||||
|
use super::*;
|
||||||
|
use dashmap::DashMap;
|
||||||
|
use std::sync::Arc;
|
||||||
|
use std::time::{Duration, Instant};
|
||||||
|
|
||||||
|
fn fresh_handoffs() -> Arc<DashMap<String, SsoHandoff>> {
|
||||||
|
Arc::new(DashMap::new())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn make_handoff(access: &str, refresh: &str, user_id: &str) -> SsoHandoff {
|
||||||
|
SsoHandoff {
|
||||||
|
access_token: access.to_string(),
|
||||||
|
raw_refresh: refresh.to_string(),
|
||||||
|
user_json: json!({ "id": user_id, "username": "testuser" }),
|
||||||
|
access_ttl: 900,
|
||||||
|
expires_at: Instant::now() + Duration::from_secs(HANDOFF_TTL_SECS),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 1. handoff_exchange_success — create a handoff, exchange it,
|
||||||
|
/// expect 200 with the access/refresh/user fields.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn handoff_exchange_success() {
|
||||||
|
let handoffs = fresh_handoffs();
|
||||||
|
let code = generate_handoff_code();
|
||||||
|
handoffs.insert(
|
||||||
|
code.clone(),
|
||||||
|
make_handoff("jwt-access", "refresh-raw", "user-123"),
|
||||||
|
);
|
||||||
|
|
||||||
|
let (status, body) = sso_handoff_exchange_inner(&handoffs, &code).await;
|
||||||
|
|
||||||
|
assert_eq!(status, StatusCode::OK);
|
||||||
|
assert_eq!(body["access_token"], "jwt-access");
|
||||||
|
assert_eq!(body["refresh_token"], "refresh-raw");
|
||||||
|
assert_eq!(body["token_type"], "Bearer");
|
||||||
|
assert_eq!(body["expires_in"], 900);
|
||||||
|
assert_eq!(body["user"]["id"], "user-123");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 2. handoff_exchange_single_use — exchange once (success),
|
||||||
|
/// exchange the same code again (expect 400 invalid_handoff).
|
||||||
|
#[tokio::test]
|
||||||
|
async fn handoff_exchange_single_use() {
|
||||||
|
let handoffs = fresh_handoffs();
|
||||||
|
let code = generate_handoff_code();
|
||||||
|
handoffs.insert(code.clone(), make_handoff("a", "r", "u"));
|
||||||
|
|
||||||
|
// First exchange succeeds
|
||||||
|
let (status1, _) = sso_handoff_exchange_inner(&handoffs, &code).await;
|
||||||
|
assert_eq!(status1, StatusCode::OK);
|
||||||
|
|
||||||
|
// Second exchange with the same code fails (entry was removed)
|
||||||
|
let (status2, body2) = sso_handoff_exchange_inner(&handoffs, &code).await;
|
||||||
|
assert_eq!(status2, StatusCode::BAD_REQUEST);
|
||||||
|
assert_eq!(body2["error"]["code"], "invalid_handoff");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 3. handoff_exchange_unknown_code — exchange a code that was
|
||||||
|
/// never issued (expect 400 invalid_handoff).
|
||||||
|
#[tokio::test]
|
||||||
|
async fn handoff_exchange_unknown_code() {
|
||||||
|
let handoffs = fresh_handoffs();
|
||||||
|
let (status, body) = sso_handoff_exchange_inner(&handoffs, "never-issued-code").await;
|
||||||
|
assert_eq!(status, StatusCode::BAD_REQUEST);
|
||||||
|
assert_eq!(body["error"]["code"], "invalid_handoff");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 4. handoff_exchange_expired_code — create a handoff with
|
||||||
|
/// expires_at in the past, exchange (expect 400 invalid_handoff).
|
||||||
|
#[tokio::test]
|
||||||
|
async fn handoff_exchange_expired_code() {
|
||||||
|
let handoffs = fresh_handoffs();
|
||||||
|
let code = generate_handoff_code();
|
||||||
|
let mut h = make_handoff("a", "r", "u");
|
||||||
|
h.expires_at = Instant::now() - Duration::from_secs(1); // already expired
|
||||||
|
handoffs.insert(code.clone(), h);
|
||||||
|
|
||||||
|
let (status, body) = sso_handoff_exchange_inner(&handoffs, &code).await;
|
||||||
|
assert_eq!(status, StatusCode::BAD_REQUEST);
|
||||||
|
assert_eq!(body["error"]["code"], "invalid_handoff");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 5. handoff_exchange_race — two concurrent exchanges with the
|
||||||
|
/// same code; exactly one succeeds, the other gets 400.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn handoff_exchange_race() {
|
||||||
|
let handoffs = fresh_handoffs();
|
||||||
|
let code = generate_handoff_code();
|
||||||
|
handoffs.insert(code.clone(), make_handoff("a", "r", "u"));
|
||||||
|
|
||||||
|
// DashMap::remove is atomic, so only one of two concurrent
|
||||||
|
// calls can win. The other gets None and returns 400.
|
||||||
|
let h1 = handoffs.clone();
|
||||||
|
let h2 = handoffs.clone();
|
||||||
|
let c1 = code.clone();
|
||||||
|
let c2 = code.clone();
|
||||||
|
let (r1, r2) = tokio::join!(
|
||||||
|
sso_handoff_exchange_inner(&h1, &c1),
|
||||||
|
sso_handoff_exchange_inner(&h2, &c2),
|
||||||
|
);
|
||||||
|
|
||||||
|
let status1 = r1.0;
|
||||||
|
let status2 = r2.0;
|
||||||
|
let successes = [status1, status2]
|
||||||
|
.iter()
|
||||||
|
.filter(|s| **s == StatusCode::OK)
|
||||||
|
.count();
|
||||||
|
let failures = [status1, status2]
|
||||||
|
.iter()
|
||||||
|
.filter(|s| **s == StatusCode::BAD_REQUEST)
|
||||||
|
.count();
|
||||||
|
assert_eq!(successes, 1, "exactly one exchange should succeed");
|
||||||
|
assert_eq!(failures, 1, "exactly one exchange should fail");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 6. handoff_exchange_malformed_body — exchange with an empty
|
||||||
|
/// code (expect 400 invalid_handoff).
|
||||||
|
#[tokio::test]
|
||||||
|
async fn handoff_exchange_malformed_body() {
|
||||||
|
let handoffs = fresh_handoffs();
|
||||||
|
let (status, body) = sso_handoff_exchange_inner(&handoffs, "").await;
|
||||||
|
assert_eq!(status, StatusCode::BAD_REQUEST);
|
||||||
|
assert_eq!(body["error"]["code"], "invalid_handoff");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 7. handoff_cleanup_removes_expired — create 3 handoffs with
|
||||||
|
/// varying `expires_at`, run one tick of the cleanup task,
|
||||||
|
/// assert only the non-expired ones remain.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn handoff_cleanup_removes_expired() {
|
||||||
|
let handoffs = fresh_handoffs();
|
||||||
|
// 2 expired, 1 fresh
|
||||||
|
for (i, expired) in [true, false, true].iter().enumerate() {
|
||||||
|
let mut h = make_handoff(&format!("a{}", i), "r", "u");
|
||||||
|
if *expired {
|
||||||
|
h.expires_at = Instant::now() - Duration::from_secs(1);
|
||||||
|
}
|
||||||
|
handoffs.insert(format!("code-{}", i), h);
|
||||||
|
}
|
||||||
|
assert_eq!(handoffs.len(), 3);
|
||||||
|
|
||||||
|
// Simulate one tick of the cleanup task (mirrors the logic
|
||||||
|
// in main.rs lines 174-188)
|
||||||
|
let now = Instant::now();
|
||||||
|
handoffs.retain(|_, v| v.expires_at > now);
|
||||||
|
|
||||||
|
assert_eq!(handoffs.len(), 1);
|
||||||
|
assert!(handoffs.contains_key("code-1"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@ -14,6 +14,16 @@ Security: JWT Bearer Token (except Public Endpoints)
|
|||||||
| POST | `/auth/mfa/verify` | Verify MFA code |
|
| POST | `/auth/mfa/verify` | Verify MFA code |
|
||||||
| DELETE | `/auth/mfa` | Disable MFA for user |
|
| DELETE | `/auth/mfa` | Disable MFA for user |
|
||||||
|
|
||||||
|
## 1b. SSO (Single Sign-On)
|
||||||
|
*No authentication required.* These endpoints implement the OIDC Authorization Code + PKCE flow. See `tasks/sso-token-handoff-spec.md` for the full design.
|
||||||
|
|
||||||
|
| Method | Endpoint | Description |
|
||||||
|
|--------|----------|-------------|
|
||||||
|
| GET | `/auth/sso/login` | Initiate OIDC login: redirects browser to the configured IdP's authorization URL |
|
||||||
|
| GET | `/auth/sso/callback` | OIDC redirect URI: handles the IdP response, issues a single-use 60s `handoff_code`, stores the JWT access/refresh tokens in memory, and 302-redirects to the SPA with `?handoff=<code>` in the URL (no tokens in the URL — see issue #4) |
|
||||||
|
| GET | `/auth/sso/config` | Returns minimal SSO configuration for the login page (`enabled`, `display_name`, `auth_url`). No secrets exposed |
|
||||||
|
| POST | `/auth/sso/handoff` | **(new in issue #4)** Exchange a single-use `handoff_code` for the JWT access/refresh tokens. The SPA calls this from `SsoCallbackPage` after the OIDC callback redirect. Returns `{ access_token, refresh_token, token_type, expires_in, user }`. The code is single-use, 60s TTL, and atomically removed on exchange (concurrent attempts: exactly one wins). `400 invalid_handoff` on unknown/expired/already-consumed codes |
|
||||||
|
|
||||||
## 2. Public Endpoints (Self-Enrollment)
|
## 2. Public Endpoints (Self-Enrollment)
|
||||||
*No authentication required.*
|
*No authentication required.*
|
||||||
| Method | Endpoint | Description |
|
| Method | Endpoint | Description |
|
||||||
|
|||||||
@ -92,6 +92,9 @@ verifying that all mandated security controls are implemented and operational.
|
|||||||
| OAuth2/OIDC Authorization Code + PKCE | ✅ Verified | Public routes `/api/v1/auth/azure/login` and `/api/v1/auth/azure/callback` implement PKCE flow |
|
| OAuth2/OIDC Authorization Code + PKCE | ✅ Verified | Public routes `/api/v1/auth/azure/login` and `/api/v1/auth/azure/callback` implement PKCE flow |
|
||||||
| Test connection without enabling | ✅ Verified | `POST /api/v1/settings/azure-sso/test` validates configuration without persisting |
|
| Test connection without enabling | ✅ Verified | `POST /api/v1/settings/azure-sso/test` validates configuration without persisting |
|
||||||
| MFA still required after SSO | ✅ Verified | SSO login follows same MFA verification path as local login |
|
| MFA still required after SSO | ✅ Verified | SSO login follows same MFA verification path as local login |
|
||||||
|
| **No tokens in redirect URL (issue #4 fix)** | ✅ Verified | SSO callback (`crates/pm-web/src/routes/sso.rs` `sso_callback`) now issues a single-use, 60s `handoff_code` and stores the JWT access/refresh tokens in the in-memory `sso_handoffs: Arc<DashMap<String, SsoHandoff>>`. The redirect URL contains only `?handoff=<code>`. No `access_token`, `refresh_token`, or `user` parameters are ever placed in the URL. The SPA exchanges the code via `POST /api/v1/auth/sso/handoff`. See `tasks/sso-token-handoff-spec.md` for the full design. |
|
||||||
|
| **Handoff code is single-use + 60s TTL** | ✅ Verified | `DashMap::remove` in `sso_handoff_exchange_inner` is atomic — concurrent exchange attempts result in exactly one success and one 400. Expired codes (`expires_at < Instant::now()`) are rejected with `400 invalid_handoff`. A background cleanup task removes expired entries every 60s. Verified by `handoff_exchange_single_use`, `handoff_exchange_race`, and `handoff_exchange_expired_code` tests in `crates/pm-web/src/routes/sso.rs`. |
|
||||||
|
| **Handoff code cleared from browser history** | ✅ Verified | SPA calls `window.history.replaceState({}, '', '/auth/sso/callback')` after a successful exchange, removing the `?handoff=` param from the address bar. Verified by `clears_handoff_code_from_url_after_success` test in `frontend/src/pages/__tests__/SsoCallbackPage.test.tsx`. |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
2149
frontend/package-lock.json
generated
2149
frontend/package-lock.json
generated
File diff suppressed because it is too large
Load Diff
@ -8,7 +8,9 @@
|
|||||||
"build": "tsc && vite build",
|
"build": "tsc && vite build",
|
||||||
"preview": "vite preview",
|
"preview": "vite preview",
|
||||||
"lint": "eslint src/ --ext .ts,.tsx --max-warnings 0",
|
"lint": "eslint src/ --ext .ts,.tsx --max-warnings 0",
|
||||||
"type-check": "tsc --noEmit"
|
"type-check": "tsc --noEmit",
|
||||||
|
"test": "vitest run",
|
||||||
|
"test:watch": "vitest"
|
||||||
},
|
},
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"@emotion/react": "^11.14.0",
|
"@emotion/react": "^11.14.0",
|
||||||
@ -25,6 +27,9 @@
|
|||||||
"zustand": "^5.0.3"
|
"zustand": "^5.0.3"
|
||||||
},
|
},
|
||||||
"devDependencies": {
|
"devDependencies": {
|
||||||
|
"@testing-library/jest-dom": "^6.9.1",
|
||||||
|
"@testing-library/react": "^16.3.2",
|
||||||
|
"@testing-library/user-event": "^14.6.1",
|
||||||
"@types/react": "^19.0.0",
|
"@types/react": "^19.0.0",
|
||||||
"@types/react-dom": "^19.0.0",
|
"@types/react-dom": "^19.0.0",
|
||||||
"@typescript-eslint/eslint-plugin": "^8.30.0",
|
"@typescript-eslint/eslint-plugin": "^8.30.0",
|
||||||
@ -32,7 +37,9 @@
|
|||||||
"@vitejs/plugin-react": "^4.4.1",
|
"@vitejs/plugin-react": "^4.4.1",
|
||||||
"eslint": "^9.24.0",
|
"eslint": "^9.24.0",
|
||||||
"eslint-plugin-react-hooks": "^5.0.0",
|
"eslint-plugin-react-hooks": "^5.0.0",
|
||||||
|
"jsdom": "^25.0.1",
|
||||||
"typescript": "^5.8.3",
|
"typescript": "^5.8.3",
|
||||||
"vite": "^6.3.3"
|
"vite": "^6.3.3",
|
||||||
|
"vitest": "^2.1.9"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -1,76 +1,97 @@
|
|||||||
import { useEffect, useState } from 'react'
|
import { useEffect, useState } from 'react'
|
||||||
import { useNavigate } from 'react-router-dom'
|
import { useNavigate, useSearchParams } from 'react-router-dom'
|
||||||
import {
|
import {
|
||||||
Box, Container, Paper, Typography, Alert, Button, CircularProgress,
|
Box, Container, Paper, Typography, Alert, Button, CircularProgress,
|
||||||
} from '@mui/material'
|
} from '@mui/material'
|
||||||
import { useAuthStore } from '../store/authStore'
|
import { useAuthStore } from '../store/authStore'
|
||||||
import type { User } from '../types'
|
import type { User } from '../types'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* SSO callback page.
|
||||||
|
*
|
||||||
|
* Flow (per `tasks/sso-token-handoff-spec.md`):
|
||||||
|
* 1. The OIDC provider redirects the browser here with `?handoff=<code>`
|
||||||
|
* in the URL. The actual JWT access/refresh tokens are NOT in the URL
|
||||||
|
* (that would leak them through browser history, proxy access logs,
|
||||||
|
* and the Referer header — see issue #4).
|
||||||
|
* 2. On mount, we POST the handoff code to
|
||||||
|
* `POST /api/v1/auth/sso/handoff`. The backend atomically removes
|
||||||
|
* the entry (single-use) and returns the tokens in the JSON
|
||||||
|
* response.
|
||||||
|
* 3. On success, we call `setTokens` + `setUser` on the auth store,
|
||||||
|
* replace the URL (removing the handoff code from history), and
|
||||||
|
* navigate to `/dashboard`.
|
||||||
|
* 4. On failure, we show an error and let the user go back to `/login`.
|
||||||
|
*/
|
||||||
export default function SsoCallbackPage() {
|
export default function SsoCallbackPage() {
|
||||||
const navigate = useNavigate()
|
const navigate = useNavigate()
|
||||||
|
const [searchParams] = useSearchParams()
|
||||||
const { setTokens, setUser } = useAuthStore()
|
const { setTokens, setUser } = useAuthStore()
|
||||||
const [error, setError] = useState<string | null>(null)
|
const [error, setError] = useState<string | null>(null)
|
||||||
const [processing, setProcessing] = useState(true)
|
const [processing, setProcessing] = useState(true)
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const params = new URLSearchParams(window.location.search)
|
// Surface upstream OIDC errors (e.g. user denied consent) unchanged.
|
||||||
|
const errorCode = searchParams.get('error')
|
||||||
// Check for error from backend
|
const errorDescription = searchParams.get('error_description')
|
||||||
const errorCode = params.get('error')
|
|
||||||
const errorDescription = params.get('error_description')
|
|
||||||
if (errorCode) {
|
if (errorCode) {
|
||||||
setError(errorDescription || `SSO authentication failed: ${errorCode}`)
|
setError(errorDescription || `SSO authentication failed: ${errorCode}`)
|
||||||
setProcessing(false)
|
setProcessing(false)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Extract tokens
|
const handoffCode = searchParams.get('handoff')
|
||||||
const accessToken = params.get('access_token')
|
if (!handoffCode) {
|
||||||
const refreshToken = params.get('refresh_token')
|
setError('Missing handoff code. Please try logging in again.')
|
||||||
|
|
||||||
if (!accessToken || !refreshToken) {
|
|
||||||
setError('Missing authentication tokens. Please try logging in again.')
|
|
||||||
setProcessing(false)
|
setProcessing(false)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Parse user JSON from query param
|
// Exchange the handoff code for tokens. The code is single-use and
|
||||||
const userParam = params.get('user')
|
// 60-second TTL on the backend; the SPA must POST promptly.
|
||||||
if (!userParam) {
|
(async () => {
|
||||||
setError('Missing user information. Please try logging in again.')
|
try {
|
||||||
setProcessing(false)
|
const resp = await fetch('/api/v1/auth/sso/handoff', {
|
||||||
return
|
method: 'POST',
|
||||||
}
|
headers: { 'Content-Type': 'application/json' },
|
||||||
|
body: JSON.stringify({ handoff_code: handoffCode }),
|
||||||
|
})
|
||||||
|
if (!resp.ok) {
|
||||||
|
// Try to extract a structured error from the backend
|
||||||
|
let message = `Failed to complete sign-in (HTTP ${resp.status})`
|
||||||
|
try {
|
||||||
|
const errBody = await resp.json()
|
||||||
|
if (errBody?.error?.message) {
|
||||||
|
message = errBody.error.message
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Body wasn't JSON; keep the default message
|
||||||
|
}
|
||||||
|
setError(message)
|
||||||
|
setProcessing(false)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
let parsedUser: Record<string, unknown>
|
const data = await resp.json()
|
||||||
try {
|
const user = buildUser(data.user)
|
||||||
parsedUser = JSON.parse(userParam)
|
|
||||||
} catch {
|
|
||||||
setError('Malformed user data received. Please try logging in again.')
|
|
||||||
setProcessing(false)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
// Build a full User object from the SSO subset, filling in sensible defaults
|
setTokens(data.access_token, data.refresh_token)
|
||||||
// auth_provider comes from the backend based on the OIDC provider type
|
setUser(user)
|
||||||
const authProvider = (parsedUser.auth_provider as string) || 'azure_sso'
|
|
||||||
const user: User = {
|
|
||||||
id: (parsedUser.id as string) || '',
|
|
||||||
username: (parsedUser.username as string) || '',
|
|
||||||
display_name: (parsedUser.display_name as string) || '',
|
|
||||||
email: (parsedUser.email as string) || '',
|
|
||||||
role: (parsedUser.role as User['role']) || 'operator',
|
|
||||||
auth_provider: authProvider as User['auth_provider'],
|
|
||||||
mfa_enabled: (parsedUser.mfa_enabled as boolean) ?? false,
|
|
||||||
is_active: true,
|
|
||||||
force_password_reset: false,
|
|
||||||
}
|
|
||||||
|
|
||||||
// Store tokens and user, then navigate
|
// Clear the handoff code from the URL so it doesn't end up in
|
||||||
setTokens(accessToken, refreshToken)
|
// browser history or get shared via the address bar. The code
|
||||||
setUser(user)
|
// is already consumed (single-use) but defense-in-depth.
|
||||||
navigate('/dashboard', { replace: true })
|
window.history.replaceState({}, '', '/auth/sso/callback')
|
||||||
}, [setTokens, setUser, navigate])
|
|
||||||
|
navigate('/dashboard', { replace: true })
|
||||||
|
} catch (err) {
|
||||||
|
setError(
|
||||||
|
err instanceof Error ? err.message : 'Failed to complete sign-in. Please try again.',
|
||||||
|
)
|
||||||
|
setProcessing(false)
|
||||||
|
}
|
||||||
|
})()
|
||||||
|
}, [setTokens, setUser, navigate, searchParams])
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Container maxWidth="xs" sx={{ mt: 12 }}>
|
<Container maxWidth="xs" sx={{ mt: 12 }}>
|
||||||
@ -105,3 +126,22 @@ export default function SsoCallbackPage() {
|
|||||||
</Container>
|
</Container>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Map the SSO user JSON payload from the backend to the SPA's `User`
|
||||||
|
* type. Fills in sensible defaults for any missing fields.
|
||||||
|
*/
|
||||||
|
function buildUser(parsed: Record<string, unknown>): User {
|
||||||
|
const authProvider = (parsed.auth_provider as string) || 'azure_sso'
|
||||||
|
return {
|
||||||
|
id: (parsed.id as string) || '',
|
||||||
|
username: (parsed.username as string) || '',
|
||||||
|
display_name: (parsed.display_name as string) || '',
|
||||||
|
email: (parsed.email as string) || '',
|
||||||
|
role: (parsed.role as User['role']) || 'operator',
|
||||||
|
auth_provider: authProvider as User['auth_provider'],
|
||||||
|
mfa_enabled: (parsed.mfa_enabled as boolean) ?? false,
|
||||||
|
is_active: true,
|
||||||
|
force_password_reset: false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
205
frontend/src/pages/__tests__/SsoCallbackPage.test.tsx
Normal file
205
frontend/src/pages/__tests__/SsoCallbackPage.test.tsx
Normal file
@ -0,0 +1,205 @@
|
|||||||
|
/// Tests for SsoCallbackPage (issue #4 — SSO token handoff).
|
||||||
|
///
|
||||||
|
/// Per `tasks/sso-token-handoff-spec.md` §6.3:
|
||||||
|
/// 9. renders_processing_state_initially
|
||||||
|
/// 10. calls_handoff_endpoint_on_mount
|
||||||
|
/// 11. stores_tokens_and_user_on_success
|
||||||
|
/// 12. shows_error_on_handoff_failure
|
||||||
|
/// 13. shows_error_when_handoff_code_missing
|
||||||
|
/// 14. clears_handoff_code_from_url_after_success
|
||||||
|
///
|
||||||
|
/// We mock `fetch`, the auth store, and `window.history.replaceState`
|
||||||
|
/// so the test focuses on the page's effect-driven logic (URL parsing
|
||||||
|
/// → POST exchange → store update → navigation → URL cleanup). We do
|
||||||
|
/// NOT mock `react-router-dom` — instead, we use a real
|
||||||
|
/// `MemoryRouter` and assert on side effects (the auth store mocks +
|
||||||
|
/// `replaceState` spy + visible error text).
|
||||||
|
|
||||||
|
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
|
||||||
|
import { render, screen, waitFor, act } from '@testing-library/react'
|
||||||
|
import { MemoryRouter } from 'react-router-dom'
|
||||||
|
import SsoCallbackPage from '../SsoCallbackPage'
|
||||||
|
|
||||||
|
// Mock the auth store — we don't want real zustand state leaking
|
||||||
|
// between tests, and we want to assert on setTokens/setUser calls.
|
||||||
|
const setTokensMock = vi.fn()
|
||||||
|
const setUserMock = vi.fn()
|
||||||
|
vi.mock('../../store/authStore', () => ({
|
||||||
|
useAuthStore: () => ({
|
||||||
|
setTokens: setTokensMock,
|
||||||
|
setUser: setUserMock,
|
||||||
|
}),
|
||||||
|
}))
|
||||||
|
|
||||||
|
// Helper: render the page with a controlled URL and let the test
|
||||||
|
// inspect the rendered output + the auth store mocks.
|
||||||
|
function renderAt(url: string) {
|
||||||
|
return render(
|
||||||
|
<MemoryRouter initialEntries={[url]}>
|
||||||
|
<SsoCallbackPage />
|
||||||
|
</MemoryRouter>,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
setTokensMock.mockReset()
|
||||||
|
setUserMock.mockReset()
|
||||||
|
// Default fetch: never-resolving promise (keeps the page in
|
||||||
|
// "processing" state). Individual tests override this.
|
||||||
|
globalThis.fetch = vi.fn(() => new Promise(() => {})) as unknown as typeof fetch
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.restoreAllMocks()
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('SsoCallbackPage', () => {
|
||||||
|
// 9. renders_processing_state_initially — on mount with a handoff
|
||||||
|
// code, shows the spinner and "Completing sign-in…".
|
||||||
|
it('renders the processing state initially', async () => {
|
||||||
|
// Wrap in act() to flush the useEffect that calls fetch.
|
||||||
|
await act(async () => {
|
||||||
|
renderAt('/auth/sso/callback?handoff=test-code')
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(screen.getByText(/completing sign-in/i)).toBeInTheDocument()
|
||||||
|
// The MUI CircularProgress renders a role="progressbar"
|
||||||
|
expect(screen.getByRole('progressbar')).toBeInTheDocument()
|
||||||
|
})
|
||||||
|
|
||||||
|
// 10. calls_handoff_endpoint_on_mount — mocks fetch and asserts
|
||||||
|
// the POST goes to /api/v1/auth/sso/handoff with
|
||||||
|
// { handoff_code: <code> }.
|
||||||
|
it('POSTs the handoff code to the backend on mount', async () => {
|
||||||
|
const fetchMock = vi.fn().mockResolvedValueOnce(
|
||||||
|
new Response(
|
||||||
|
JSON.stringify({
|
||||||
|
access_token: 'a',
|
||||||
|
refresh_token: 'r',
|
||||||
|
token_type: 'Bearer',
|
||||||
|
expires_in: 900,
|
||||||
|
user: { id: 'u1', username: 'tester' },
|
||||||
|
}),
|
||||||
|
{ status: 200, headers: { 'Content-Type': 'application/json' } },
|
||||||
|
),
|
||||||
|
)
|
||||||
|
globalThis.fetch = fetchMock as unknown as typeof fetch
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
renderAt('/auth/sso/callback?handoff=abc123')
|
||||||
|
})
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(fetchMock).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
|
const [url, init] = fetchMock.mock.calls[0]
|
||||||
|
expect(url).toBe('/api/v1/auth/sso/handoff')
|
||||||
|
expect(init.method).toBe('POST')
|
||||||
|
expect(JSON.parse(init.body)).toEqual({ handoff_code: 'abc123' })
|
||||||
|
})
|
||||||
|
|
||||||
|
// 11. stores_tokens_and_user_on_success — mocks a successful
|
||||||
|
// response, asserts setTokens and setUser are called, and
|
||||||
|
// setTokens receives the correct token values.
|
||||||
|
it('stores tokens + user on a successful exchange', async () => {
|
||||||
|
const fetchMock = vi.fn().mockResolvedValueOnce(
|
||||||
|
new Response(
|
||||||
|
JSON.stringify({
|
||||||
|
access_token: 'access-jwt',
|
||||||
|
refresh_token: 'refresh-raw',
|
||||||
|
token_type: 'Bearer',
|
||||||
|
expires_in: 900,
|
||||||
|
user: { id: 'user-42', username: 'alice' },
|
||||||
|
}),
|
||||||
|
{ status: 200, headers: { 'Content-Type': 'application/json' } },
|
||||||
|
),
|
||||||
|
)
|
||||||
|
globalThis.fetch = fetchMock as unknown as typeof fetch
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
renderAt('/auth/sso/callback?handoff=ok')
|
||||||
|
})
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(setTokensMock).toHaveBeenCalledWith('access-jwt', 'refresh-raw')
|
||||||
|
})
|
||||||
|
expect(setUserMock).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({ id: 'user-42', username: 'alice' }),
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
// 12. shows_error_on_handoff_failure — mocks a 400 response,
|
||||||
|
// asserts the error message is rendered and the spinner
|
||||||
|
// stops.
|
||||||
|
it('shows an error when the backend returns 400', async () => {
|
||||||
|
const fetchMock = vi.fn().mockResolvedValueOnce(
|
||||||
|
new Response(
|
||||||
|
JSON.stringify({
|
||||||
|
error: { code: 'invalid_handoff', message: 'Handoff code has expired' },
|
||||||
|
}),
|
||||||
|
{ status: 400, headers: { 'Content-Type': 'application/json' } },
|
||||||
|
),
|
||||||
|
)
|
||||||
|
globalThis.fetch = fetchMock as unknown as typeof fetch
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
renderAt('/auth/sso/callback?handoff=expired')
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(await screen.findByText(/handoff code has expired/i)).toBeInTheDocument()
|
||||||
|
expect(screen.queryByText(/completing sign-in/i)).not.toBeInTheDocument()
|
||||||
|
// No token storage on error
|
||||||
|
expect(setTokensMock).not.toHaveBeenCalled()
|
||||||
|
expect(setUserMock).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
// 13. shows_error_when_handoff_code_missing — invokes the effect
|
||||||
|
// with no handoff code, asserts the "Missing handoff code"
|
||||||
|
// error is shown.
|
||||||
|
it('shows a missing-code error when ?handoff= is absent', async () => {
|
||||||
|
const fetchMock = vi.fn()
|
||||||
|
globalThis.fetch = fetchMock as unknown as typeof fetch
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
renderAt('/auth/sso/callback')
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(await screen.findByText(/missing handoff code/i)).toBeInTheDocument()
|
||||||
|
// No fetch call should have been made
|
||||||
|
expect(fetchMock).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
// 14. clears_handoff_code_from_url_after_success — asserts
|
||||||
|
// window.history.replaceState is called to remove the
|
||||||
|
// ?handoff= param from the URL after a successful exchange.
|
||||||
|
it('clears the handoff code from the URL after a successful exchange', async () => {
|
||||||
|
const fetchMock = vi.fn().mockResolvedValueOnce(
|
||||||
|
new Response(
|
||||||
|
JSON.stringify({
|
||||||
|
access_token: 'a',
|
||||||
|
refresh_token: 'r',
|
||||||
|
token_type: 'Bearer',
|
||||||
|
expires_in: 900,
|
||||||
|
user: { id: 'u', username: 'u' },
|
||||||
|
}),
|
||||||
|
{ status: 200, headers: { 'Content-Type': 'application/json' } },
|
||||||
|
),
|
||||||
|
)
|
||||||
|
globalThis.fetch = fetchMock as unknown as typeof fetch
|
||||||
|
|
||||||
|
const replaceStateSpy = vi.spyOn(window.history, 'replaceState')
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
renderAt('/auth/sso/callback?handoff=secret-code')
|
||||||
|
})
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(replaceStateSpy).toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
// Verify the replaceState call cleared the query string — the
|
||||||
|
// third argument is the new URL ('/auth/sso/callback' with no
|
||||||
|
// query).
|
||||||
|
const args = replaceStateSpy.mock.calls[0]
|
||||||
|
expect(args[2]).toBe('/auth/sso/callback')
|
||||||
|
})
|
||||||
|
})
|
||||||
6
frontend/src/test/setup.ts
Normal file
6
frontend/src/test/setup.ts
Normal file
@ -0,0 +1,6 @@
|
|||||||
|
/// Vitest setup file — runs before each test file.
|
||||||
|
///
|
||||||
|
/// Imports `@testing-library/jest-dom` to register custom matchers like
|
||||||
|
/// `toBeInTheDocument`, `toHaveTextContent`, etc. that the SSO callback
|
||||||
|
/// tests rely on.
|
||||||
|
import '@testing-library/jest-dom/vitest'
|
||||||
18
frontend/vitest.config.ts
Normal file
18
frontend/vitest.config.ts
Normal file
@ -0,0 +1,18 @@
|
|||||||
|
import { defineConfig } from 'vitest/config'
|
||||||
|
import react from '@vitejs/plugin-react'
|
||||||
|
|
||||||
|
/// Vitest configuration for the Patch Manager UI.
|
||||||
|
///
|
||||||
|
/// - Uses jsdom for a browser-like environment (needed for MUI + React
|
||||||
|
/// Testing Library).
|
||||||
|
/// - The `react()` plugin is required for JSX in test files.
|
||||||
|
/// - `globals: true` lets tests use `describe`, `it`, `expect` without
|
||||||
|
/// imports (matches the existing frontend conventions).
|
||||||
|
export default defineConfig({
|
||||||
|
plugins: [react()],
|
||||||
|
test: {
|
||||||
|
environment: 'jsdom',
|
||||||
|
globals: true,
|
||||||
|
setupFiles: ['./src/test/setup.ts'],
|
||||||
|
},
|
||||||
|
})
|
||||||
332
tasks/sso-token-handoff-spec.md
Normal file
332
tasks/sso-token-handoff-spec.md
Normal file
@ -0,0 +1,332 @@
|
|||||||
|
# SSO Token Handoff — Specification
|
||||||
|
|
||||||
|
**Issue:** [#4](https://github.com/Draco-Lunaris/Linux-Patch-Manager/issues/4)
|
||||||
|
**Component:** `crates/pm-web/src/routes/sso.rs`, `frontend/src/pages/SsoCallbackPage.tsx`, `frontend/src/store/authStore.ts`
|
||||||
|
**Spec version:** 0.1.0 (draft)
|
||||||
|
**Status:** Awaiting Kelly sign-off
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 1. Goal
|
||||||
|
|
||||||
|
Stop embedding JWT access tokens, refresh tokens, and user objects in the
|
||||||
|
SSO callback redirect URL. Today, after a successful OIDC login, the
|
||||||
|
backend 302-redirects the browser to the SPA with the tokens in the
|
||||||
|
query string:
|
||||||
|
|
||||||
|
```
|
||||||
|
https://app.example.com/auth/sso/callback
|
||||||
|
?access_token=<jwt>
|
||||||
|
&refresh_token=<raw>
|
||||||
|
&token_type=Bearer
|
||||||
|
&expires_in=900
|
||||||
|
&user=<urlencoded-json>
|
||||||
|
```
|
||||||
|
|
||||||
|
Tokens in URLs are written to browser history, intermediate proxy and
|
||||||
|
load-balancer access logs, and may leak via the `Referer` header when
|
||||||
|
the landing page loads third-party resources. The refresh token is
|
||||||
|
the most sensitive value (long-lived, rotating) and gets the worst
|
||||||
|
exposure.
|
||||||
|
|
||||||
|
Replace the URL-embedded tokens with a **single-use, short-lived
|
||||||
|
handoff code** that the SPA exchanges for tokens via a server-to-server
|
||||||
|
POST. The URL then contains only the code, which expires in 60 seconds
|
||||||
|
and is invalidated on first use.
|
||||||
|
|
||||||
|
## 2. Non-Goals
|
||||||
|
|
||||||
|
- Changing the OIDC flow itself (Authorization Code + PKCE stays the same).
|
||||||
|
- Changing the MFA verification path that runs after the OIDC callback.
|
||||||
|
- Touching the WS ticket pattern (issue #10) — this spec is a *new*
|
||||||
|
in-memory store for SSO handoff codes, mirroring but separate from
|
||||||
|
`ws_tickets: Arc<DashMap<String, WsTicket>>`.
|
||||||
|
- Adding cookie-based or `form_post` delivery. The handoff code
|
||||||
|
approach was selected over those (Kelly sign-off Q1).
|
||||||
|
- Long-lived SSO sessions. The handoff code is single-use; subsequent
|
||||||
|
SSO logins re-issue a new code.
|
||||||
|
|
||||||
|
## 3. Design Decisions (Kelly sign-off, 2026-06-02)
|
||||||
|
|
||||||
|
| # | Question | Resolution |
|
||||||
|
|---|----------|------------|
|
||||||
|
| Q1 | Approach selection | **Handoff code** (option C in issue #4). Mirrors the existing WS-ticket pattern. URL contains only a single-use, 60s `handoff_code`. SPA POSTs to `/api/v1/auth/sso/handoff` and gets tokens in the JSON response. |
|
||||||
|
| Q2 | Cookie attributes | **N/A** — handoff code approach uses no cookies. |
|
||||||
|
| Q3 | Rollout strategy | **Hard cutover** — remove the old query-string parsing in the same PR. No dual-read window. (Justification: security-critical fix, deploy window is short, no in-flight SSO logins survive a rolling restart because the auth state is in the user's browser, not on the server.) |
|
||||||
|
| Q4 | `Secure` cookie flag | **N/A** — handoff code approach uses no cookies. Kelly's answer ("unconditionally secure") is noted for future cookie work but does not apply here. |
|
||||||
|
|
||||||
|
## 4. Design
|
||||||
|
|
||||||
|
### 4.1 Backend: SSO callback (`crates/pm-web/src/routes/sso.rs`)
|
||||||
|
|
||||||
|
The `sso_callback` handler currently constructs a redirect URL with all
|
||||||
|
token values. Replace this with a handoff code generation step:
|
||||||
|
|
||||||
|
1. After the access/refresh tokens and `user_json` are computed (the
|
||||||
|
existing logic through `sso_callback` is unchanged up to the
|
||||||
|
redirect construction), generate a cryptographically random
|
||||||
|
`handoff_code` (32 bytes, base64url-encoded, ~43 chars).
|
||||||
|
2. Store the handoff payload in a new in-memory map:
|
||||||
|
```rust
|
||||||
|
pub struct SsoHandoff {
|
||||||
|
pub access_token: String,
|
||||||
|
pub raw_refresh: String,
|
||||||
|
pub user_json: Value,
|
||||||
|
pub access_ttl: u64,
|
||||||
|
pub expires_at: Instant, // now + 60s
|
||||||
|
}
|
||||||
|
pub sso_handoffs: Arc<DashMap<String, SsoHandoff>>,
|
||||||
|
```
|
||||||
|
Mirrors the `WsTicket` struct (single-use, in-memory, TTL enforced
|
||||||
|
on read). The map is added to `AppState` alongside `ws_tickets`.
|
||||||
|
3. Build the redirect URL with ONLY the handoff code:
|
||||||
|
```rust
|
||||||
|
let redirect_url = format!("{}?handoff={}", callback_url, handoff_code);
|
||||||
|
Ok(Redirect::to(&redirect_url))
|
||||||
|
```
|
||||||
|
4. Log the handoff creation (without the code value itself) for audit:
|
||||||
|
```rust
|
||||||
|
tracing::info!(user_id = %user.id, auth_provider, "SSO handoff issued");
|
||||||
|
```
|
||||||
|
|
||||||
|
### 4.2 Backend: Handoff exchange endpoint
|
||||||
|
|
||||||
|
New handler `POST /api/v1/auth/sso/handoff`:
|
||||||
|
|
||||||
|
- Request body: `{ "handoff_code": "<code>" }`
|
||||||
|
- Behavior:
|
||||||
|
1. Look up `handoff_code` in `sso_handoffs` (DashMap read lock).
|
||||||
|
2. If not found → `400 invalid_handoff`.
|
||||||
|
3. If found but `expires_at < Instant::now()` → remove the entry and
|
||||||
|
return `400 invalid_handoff` (the cleanup-on-expiry also prevents
|
||||||
|
memory bloat from expired-but-unconsumed codes).
|
||||||
|
4. **Remove the entry atomically** (DashMap `remove` is atomic) —
|
||||||
|
this is the single-use guarantee. Even if two requests race with
|
||||||
|
the same code, only one wins.
|
||||||
|
5. Return the payload as JSON:
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"access_token": "<jwt>",
|
||||||
|
"refresh_token": "<raw>",
|
||||||
|
"token_type": "Bearer",
|
||||||
|
"expires_in": 900,
|
||||||
|
"user": { "id": "...", "username": "...", ... }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
- Log:
|
||||||
|
- On success: `tracing::info!(user_id = %payload.user.id, "SSO handoff exchanged")`
|
||||||
|
- On failure: `tracing::warn!(reason = %reason, "SSO handoff exchange failed")`
|
||||||
|
- **Never log the handoff code value itself** (it's a bearer secret
|
||||||
|
with 60s window).
|
||||||
|
|
||||||
|
### 4.3 Backend: Cleanup task
|
||||||
|
|
||||||
|
Add a `tokio::spawn` cleanup task in `main.rs` (mirroring the existing
|
||||||
|
WS-ticket cleanup if present, or the SSO-session cleanup that already
|
||||||
|
runs per the codebase). Every 60 seconds, walk `sso_handoffs` and
|
||||||
|
remove entries with `expires_at < Instant::now()`. Bounded memory
|
||||||
|
growth even if the SPA never POSTs back.
|
||||||
|
|
||||||
|
### 4.4 Backend: Route registration
|
||||||
|
|
||||||
|
In `pm-web/src/main.rs`, add the new route to the public router
|
||||||
|
(alongside `/api/v1/ws/ticket`, which is also public — no JWT
|
||||||
|
required because the handoff code IS the credential):
|
||||||
|
|
||||||
|
```rust
|
||||||
|
.route("/api/v1/auth/sso/handoff", post(sso_handoff_exchange))
|
||||||
|
```
|
||||||
|
|
||||||
|
### 4.5 Frontend: `SsoCallbackPage.tsx`
|
||||||
|
|
||||||
|
Replace the URL-param parsing with a POST to the handoff endpoint:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
useEffect(() => {
|
||||||
|
const params = new URLSearchParams(window.location.search)
|
||||||
|
const errorCode = params.get('error')
|
||||||
|
if (errorCode) {
|
||||||
|
// ... existing error handling unchanged ...
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
const handoffCode = params.get('handoff')
|
||||||
|
if (!handoffCode) {
|
||||||
|
setError('Missing handoff code. Please try logging in again.')
|
||||||
|
setProcessing(false)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Exchange handoff code for tokens
|
||||||
|
fetch('/api/v1/auth/sso/handoff', {
|
||||||
|
method: 'POST',
|
||||||
|
headers: { 'Content-Type': 'application/json' },
|
||||||
|
body: JSON.stringify({ handoff_code: handoffCode }),
|
||||||
|
})
|
||||||
|
.then(r => r.ok ? r.json() : r.json().then(e => Promise.reject(e)))
|
||||||
|
.then(data => {
|
||||||
|
setTokens(data.access_token, data.refresh_token)
|
||||||
|
setUser(buildUser(data.user))
|
||||||
|
// Clear the handoff code from the URL to prevent bookmarking/sharing
|
||||||
|
window.history.replaceState({}, '', '/auth/sso/callback')
|
||||||
|
navigate('/dashboard', { replace: true })
|
||||||
|
})
|
||||||
|
.catch(err => {
|
||||||
|
setError(err?.error?.message || 'Failed to complete sign-in. Please try again.')
|
||||||
|
setProcessing(false)
|
||||||
|
})
|
||||||
|
}, [setTokens, setUser, navigate])
|
||||||
|
```
|
||||||
|
|
||||||
|
The `buildUser` helper mirrors the existing field-mapping logic
|
||||||
|
(lines 54–67 of the current file).
|
||||||
|
|
||||||
|
### 4.6 Frontend: `authStore.ts`
|
||||||
|
|
||||||
|
**No change required.** The existing `setTokens(access, refresh)` and
|
||||||
|
`setUser(user)` API is what the new code calls. The `partialize`
|
||||||
|
config (line 74) already correctly persists only `refreshToken` and
|
||||||
|
`user` — not `accessToken` — so the in-memory access token is never
|
||||||
|
written to localStorage. This is the correct security posture and
|
||||||
|
should be preserved.
|
||||||
|
|
||||||
|
## 5. Acceptance Criteria
|
||||||
|
|
||||||
|
- [ ] SSO callback no longer places `access_token`, `refresh_token`,
|
||||||
|
`token_type`, `expires_in`, or `user` in the redirect URL.
|
||||||
|
The URL contains only `handoff=<code>` (plus the error params on
|
||||||
|
failure, which are unchanged).
|
||||||
|
- [ ] The handoff code is at least 128 bits of entropy (32 bytes,
|
||||||
|
base64url-encoded) and is generated with a CSPRNG.
|
||||||
|
- [ ] The handoff code is single-use: a second exchange attempt with
|
||||||
|
the same code returns `400 invalid_handoff` and does NOT return
|
||||||
|
the tokens again.
|
||||||
|
- [ ] The handoff code expires after 60 seconds. An exchange attempt
|
||||||
|
with an expired code returns `400 invalid_handoff` and the
|
||||||
|
entry is removed from the in-memory map.
|
||||||
|
- [ ] The SPA successfully completes login: POST to the handoff
|
||||||
|
endpoint receives the tokens, calls `setTokens` and `setUser`,
|
||||||
|
and navigates to `/dashboard`.
|
||||||
|
- [ ] `authStore.ts` is unchanged (its existing `partialize` already
|
||||||
|
prevents access-token persistence; the handoff code approach
|
||||||
|
doesn't change that contract).
|
||||||
|
- [ ] `cargo check` and `cargo clippy --all-targets` pass.
|
||||||
|
- [ ] `cargo test -p pm-web` passes with new tests for the handoff
|
||||||
|
endpoint (create, exchange success, exchange duplicate=400,
|
||||||
|
exchange expired=400, exchange unknown=400).
|
||||||
|
- [ ] `frontend` builds cleanly (`npm run build` in `frontend/`).
|
||||||
|
- [ ] No access or refresh token values appear in any URL or query
|
||||||
|
string in the SSO flow. Manual verification: complete a login
|
||||||
|
and grep the server access log for the callback URL — only the
|
||||||
|
handoff code should be present.
|
||||||
|
- [ ] `docs/security-review.md` §2.5 (Azure SSO) is updated to
|
||||||
|
document the handoff code control.
|
||||||
|
|
||||||
|
## 6. Test Plan
|
||||||
|
|
||||||
|
### 6.1 Backend unit/integration tests (`crates/pm-web/src/routes/sso.rs`)
|
||||||
|
|
||||||
|
Using a small `TestApp` harness mirroring the WS-ticket test pattern
|
||||||
|
(no real HTTP listener, no DB beyond the connection that's already
|
||||||
|
mocked in the existing tests):
|
||||||
|
|
||||||
|
1. `handoff_exchange_success` — create a handoff, POST to the
|
||||||
|
exchange endpoint, expect 200 with the access/refresh/user fields.
|
||||||
|
2. `handoff_exchange_single_use` — exchange once (success), exchange
|
||||||
|
the same code again (expect 400 `invalid_handoff`).
|
||||||
|
3. `handoff_exchange_unknown_code` — POST with a code that was never
|
||||||
|
issued (expect 400 `invalid_handoff`).
|
||||||
|
4. `handoff_exchange_expired_code` — create a handoff with
|
||||||
|
`expires_at = past`, exchange (expect 400 `invalid_handoff` AND
|
||||||
|
the entry is removed from the map).
|
||||||
|
5. `handoff_exchange_race` — two concurrent POSTs with the same code
|
||||||
|
(using `tokio::join!`); exactly one succeeds, the other gets 400.
|
||||||
|
6. `handoff_exchange_malformed_body` — POST with invalid JSON or
|
||||||
|
missing `handoff_code` field (expect 400 `invalid_handoff`).
|
||||||
|
7. `callback_redirect_contains_only_handoff` — invoke `sso_callback`
|
||||||
|
through a mock OIDC config and assert the resulting redirect URL
|
||||||
|
contains only `handoff=<code>` and NO `access_token` /
|
||||||
|
`refresh_token` / `user` query params.
|
||||||
|
|
||||||
|
### 6.2 Backend cleanup test
|
||||||
|
|
||||||
|
8. `handoff_cleanup_removes_expired` — create 3 handoffs with
|
||||||
|
varying `expires_at`, run one tick of the cleanup task, assert
|
||||||
|
only the non-expired ones remain.
|
||||||
|
|
||||||
|
### 6.3 Frontend tests (`frontend/src/pages/SsoCallbackPage.tsx`)
|
||||||
|
|
||||||
|
Add a Vitest + React Testing Library test suite (the frontend already
|
||||||
|
uses Vitest — see `frontend/package.json` and `frontend/vite.config.ts`):
|
||||||
|
|
||||||
|
9. `renders_processing_state_initially` — on mount with a handoff
|
||||||
|
code, shows the spinner and "Completing sign-in…".
|
||||||
|
10. `calls_handoff_endpoint_on_mount` — mocks `fetch` and asserts the
|
||||||
|
POST goes to `/api/v1/auth/sso/handoff` with `{ handoff_code: <code> }`.
|
||||||
|
11. `stores_tokens_and_user_on_success` — mocks a successful response,
|
||||||
|
asserts `setTokens` and `setUser` are called with the response
|
||||||
|
payload, and the SPA navigates to `/dashboard`.
|
||||||
|
12. `shows_error_on_handoff_failure` — mocks a 400 response, asserts
|
||||||
|
the error message is rendered and the spinner stops.
|
||||||
|
13. `shows_error_when_handoff_code_missing` — invokes the effect with
|
||||||
|
no handoff code, asserts the "Missing handoff code" error is
|
||||||
|
shown.
|
||||||
|
14. `clears_handoff_code_from_url_after_success` — asserts
|
||||||
|
`window.history.replaceState` is called to remove the `?handoff=`
|
||||||
|
param from the URL after a successful exchange.
|
||||||
|
|
||||||
|
## 7. Risk Analysis
|
||||||
|
|
||||||
|
- **Risk: regression in the SSO login flow.** Mitigation: the test
|
||||||
|
plan covers the callback redirect shape, the exchange endpoint
|
||||||
|
behavior (success, single-use, expiry, race), and the frontend
|
||||||
|
effect. Manual end-to-end test (completing a real Azure AD login)
|
||||||
|
is required before merge — the new `scripts/integration-test.sh`
|
||||||
|
should be extended or a new `scripts/integration-test-sso.sh`
|
||||||
|
added to exercise the full flow against a mock OIDC provider.
|
||||||
|
- **Risk: in-flight SSO logins during deploy break.** Per Kelly
|
||||||
|
sign-off Q3, we accept hard cutover. The mitigation: the 60s
|
||||||
|
handoff TTL means any in-flight redirect that arrives after the
|
||||||
|
server restart has a 60s window to complete. If the new code is
|
||||||
|
deployed and the old handoffs are lost, the user is sent back to
|
||||||
|
`/auth/sso/callback?handoff=<old-code>` which the new code rejects
|
||||||
|
with `400 invalid_handoff`, and the SPA shows "Please try logging
|
||||||
|
in again." Worst case: a 30-second re-login. Acceptable for a
|
||||||
|
security-critical fix.
|
||||||
|
- **Risk: handoff code leaked via browser history or `Referer`.**
|
||||||
|
The code is single-use and 60s TTL, so the blast radius is small
|
||||||
|
even if logged. The SPA calls `history.replaceState` after a
|
||||||
|
successful exchange to remove the code from the address bar (and
|
||||||
|
the underlying history entry). The 60s window limits exposure to
|
||||||
|
`Referer` leakage on subsequent navigations from the callback
|
||||||
|
page.
|
||||||
|
- **Risk: memory growth from unconsumed handoffs.** Mitigation: the
|
||||||
|
cleanup task runs every 60s and removes expired entries. Worst
|
||||||
|
case memory usage is `O(active_logins)` — typically single digits.
|
||||||
|
- **Risk: race condition in the single-use guarantee.** Mitigation:
|
||||||
|
`DashMap::remove` is atomic, so only one of two concurrent
|
||||||
|
exchange attempts can succeed. Verified by the
|
||||||
|
`handoff_exchange_race` test.
|
||||||
|
|
||||||
|
## 8. Documentation Updates
|
||||||
|
|
||||||
|
- `docs/security-review.md` §2.5 (Azure SSO): add a new row
|
||||||
|
documenting the handoff code control and explicitly state that no
|
||||||
|
tokens appear in any URL.
|
||||||
|
- `frontend/src/pages/SsoCallbackPage.tsx`: update the doc-comment to
|
||||||
|
describe the POST-and-exchange flow instead of the URL-param parse.
|
||||||
|
- `docs/REST_API.md`: document the new `POST /api/v1/auth/sso/handoff`
|
||||||
|
endpoint.
|
||||||
|
|
||||||
|
## 9. Out of Scope / Follow-ups
|
||||||
|
|
||||||
|
- Cookie-based SSO session (a future enhancement that would let the
|
||||||
|
SPA refresh state without a new OIDC flow on every page load).
|
||||||
|
- `form_post` response mode (a future enhancement if browsers
|
||||||
|
standardize it more widely).
|
||||||
|
- Rate limiting on the handoff endpoint (out of scope here; the
|
||||||
|
existing governor-based rate limits on `/auth/*` may already cover
|
||||||
|
this — verify during implementation).
|
||||||
|
- Moving the in-memory `sso_handoffs` to Redis (out of scope; the
|
||||||
|
single-instance design constraint in `SPEC.md` is fine for this
|
||||||
|
control).
|
||||||
Reference in New Issue
Block a user