diff --git a/book/src/admin.md b/book/src/admin.md index c6c20cc5..1797962d 100644 --- a/book/src/admin.md +++ b/book/src/admin.md @@ -65,6 +65,30 @@ acting username. A **last-admin guard** stops you deleting or demoting the only remaining admin (so the portal can't be locked out); the `RUSCKER_ADMIN_TOKEN` break-glass login is the other safety net. +### Two-factor authentication for selected apps + +In an app's **Access & scale** settings, enable **Require 2FA** +(`require-mfa`) to require a user-owned authenticator-app code before the +proxy will select or start that app's container. The same enrolled TOTP +factor is reused across protected apps; the switch is a per-app step-up +policy, not a separate enrollment for every app. + +On first access, a signed-in user without a factor is guided through setup +and receives recovery codes. Later access redirects to a challenge when the +browser has no current trusted-device proof. **MFA validity days** controls +that cadence (7 days by default, at most 30); `0` means every new login +session must prove MFA, even if the browser still has a trusted-device +cookie. API routes do not redirect: they return `401` without a login and +`403` when MFA is still required. + +Users can open **Two-factor authentication** in their account to **forget +this device** or **forget all trusted devices** without ending their login +sessions. If a phone or recovery-code set is lost, an Admin can open the +user's edit page and **Reset 2FA**; this deletes the factor, recovery codes +and device grants, so the next protected-app visit starts guided enrollment +again. The `RUSCKER_ADMIN_TOKEN` remains an audited break-glass bypass for +emergencies and should not be used for routine app access. + ## Screens The sections below follow the panel's tab order: daily drivers first diff --git a/crates/ruscker-admin/assets/i18n/en/landing.ftl b/crates/ruscker-admin/assets/i18n/en/landing.ftl index 0b20bff8..0937a3d5 100644 --- a/crates/ruscker-admin/assets/i18n/en/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/en/landing.ftl @@ -646,7 +646,6 @@ spec-form-require-mfa = Require 2FA spec-form-require-mfa-hint = Users without a configured TOTP factor will be guided through enrollment on first access to a protected app. spec-form-mfa-validity = Ask again after N days spec-form-mfa-validity-hint = Blank = 7 days. Use 0 to require a new proof in every login session, with no remembered device. -spec-form-mfa-staged-note = 2FA enforcement arrives in an upcoming release; for now, this app is not yet protected. spec-form-identity-headers = Send identity headers to the app spec-form-identity-headers-hint = Adds X-SP-UserId and X-SP-UserGroups for signed-in users. Off by default; enable only for apps that need and trust this identity. spec-form-identity-claims = Additional identity claims diff --git a/crates/ruscker-admin/assets/i18n/es/landing.ftl b/crates/ruscker-admin/assets/i18n/es/landing.ftl index ec256030..ea9121b9 100644 --- a/crates/ruscker-admin/assets/i18n/es/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/es/landing.ftl @@ -646,7 +646,6 @@ spec-form-require-mfa = Exigir 2FA spec-form-require-mfa-hint = Los usuarios sin un factor TOTP configurado recibirán instrucciones para registrarlo en el primer acceso a una app protegida. spec-form-mfa-validity = Volver a solicitar después de N días spec-form-mfa-validity-hint = Vacío = 7 días. Usa 0 para exigir una nueva prueba en cada sesión de inicio de sesión, sin dispositivo recordado. -spec-form-mfa-staged-note = La aplicación de 2FA llegará en una próxima versión; por ahora, esta app aún no está protegida. spec-form-identity-headers = Enviar cabeceras de identidad a la app spec-form-identity-headers-hint = Añade X-SP-UserId y X-SP-UserGroups para usuarios autenticados. Desactivado por defecto; actívalo solo para apps que necesiten y confíen en esta identidad. spec-form-identity-claims = Datos de identidad adicionales diff --git a/crates/ruscker-admin/assets/i18n/fr/landing.ftl b/crates/ruscker-admin/assets/i18n/fr/landing.ftl index 7fcf19d9..b5c40e60 100644 --- a/crates/ruscker-admin/assets/i18n/fr/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/fr/landing.ftl @@ -646,7 +646,6 @@ spec-form-require-mfa = Exiger la 2FA spec-form-require-mfa-hint = Les utilisateurs sans facteur TOTP configuré seront guidés pour l’inscrire lors du premier accès à une app protégée. spec-form-mfa-validity = Redemander après N jours spec-form-mfa-validity-hint = Vide = 7 jours. Utilisez 0 pour exiger une nouvelle preuve à chaque session de connexion, sans appareil mémorisé. -spec-form-mfa-staged-note = L’application de la 2FA arrivera dans une prochaine version ; pour l’instant, cette app n’est pas encore protégée. spec-form-identity-headers = Envoyer les en-têtes d’identité à l’app spec-form-identity-headers-hint = Ajoute X-SP-UserId et X-SP-UserGroups pour les utilisateurs connectés. Désactivé par défaut ; activez uniquement pour les apps qui ont besoin de cette identité et lui font confiance. spec-form-identity-claims = Attributs d’identité supplémentaires diff --git a/crates/ruscker-admin/assets/i18n/pt/landing.ftl b/crates/ruscker-admin/assets/i18n/pt/landing.ftl index 7325d55c..b7de315c 100644 --- a/crates/ruscker-admin/assets/i18n/pt/landing.ftl +++ b/crates/ruscker-admin/assets/i18n/pt/landing.ftl @@ -650,7 +650,6 @@ spec-form-require-mfa = Exigir 2FA spec-form-require-mfa-hint = Usuários sem um fator TOTP configurado serão orientados a cadastrá-lo no primeiro acesso a um app protegido. spec-form-mfa-validity = Solicitar novamente após N dias spec-form-mfa-validity-hint = Em branco = 7 dias. Use 0 para exigir nova prova em cada sessão de login, sem dispositivo lembrado. -spec-form-mfa-staged-note = A exigência de 2FA chega em uma próxima versão; por enquanto, este app ainda não está protegido. spec-form-identity-headers = Enviar cabeçalhos de identidade ao app spec-form-identity-headers-hint = Adiciona X-SP-UserId e X-SP-UserGroups para usuários autenticados. Desativado por padrão; ative apenas para apps que precisam e confiam nessa identidade. spec-form-identity-claims = Dados adicionais de identidade diff --git a/crates/ruscker-admin/src/routes/proxy.rs b/crates/ruscker-admin/src/routes/proxy.rs index 63a63884..5036d6f7 100644 --- a/crates/ruscker-admin/src/routes/proxy.rs +++ b/crates/ruscker-admin/src/routes/proxy.rs @@ -170,6 +170,19 @@ const API_PREFIX: &str = "/api/"; static INFLIGHT: std::sync::LazyLock> = std::sync::LazyLock::new(dashmap::DashMap::new); +/// Break-glass MFA bypasses are deliberately loud, but a browser page load +/// can generate dozens of proxied subrequests. Deduplicate the persistent +/// audit row per `(admin session, spec)` while retaining a WARN for every +/// request. Reserving the entry before the database write also prevents two +/// concurrent first requests from inserting duplicate rows. +static MFA_BREAK_GLASS_AUDITS: std::sync::LazyLock< + dashmap::DashMap<(String, String), std::time::Instant>, +> = std::sync::LazyLock::new(dashmap::DashMap::new); + +const MFA_BREAK_GLASS_AUDIT_COOLDOWN: std::time::Duration = + std::time::Duration::from_secs(15 * 60); +const MFA_BREAK_GLASS_AUDIT_CAP: usize = 256; + /// RAII guard: bumps a replica's in-flight count on creation and drops /// it on `Drop`, covering every return/error path of the forward. pub(crate) struct InflightGuard(ruscker_core::ReplicaId); @@ -435,6 +448,13 @@ async fn forward( // is sent to log in; everyone else (and all API clients) get // a flat 403 (CORS-wrapped for the `/api/` family). if route_prefix == APP_PREFIX && session.0.is_none() { + if ws_upgrade.0.is_some() { + return ( + StatusCode::UNAUTHORIZED, + "authentication required before WebSocket upgrade\n", + ) + .into_response(); + } // Proxy routes are not wrapped by the chrome's // `prefix_base_path` Location-rewriter, so build the // base-prefixed login URL ourselves (#294) — otherwise a @@ -461,6 +481,94 @@ async fn forward( } } + // 2e. Per-app MFA step-up (#1005). This MUST stay ahead of every + // backend/replica path below: an enrollment/challenge redirect may + // never wake or start a container. Unprotected specs skip this block + // entirely, including all MFA database reads. + if spec.effective_require_mfa() { + let admin_session = session.0.as_ref(); + let session_id = cookies + .get(crate::auth::COOKIE_NAME) + .map(|cookie| cookie.value().to_string()); + + if admin_session.is_some_and(|session| { + session.role == crate::auth::Role::Admin && session.actor.is_none() + }) { + let session_id = session_id.as_deref().unwrap_or("missing-session-cookie"); + tracing::warn!( + spec = %spec.id, + "break-glass admin session bypassed per-app MFA" + ); + audit_mfa_break_glass_bypass(&state, session_id, &spec.id).await; + } else { + let Some(admin_session) = admin_session else { + if route_prefix == API_PREFIX { + return with_cors( + ( + StatusCode::UNAUTHORIZED, + "MFA-protected API requires an authenticated user session\n", + ) + .into_response(), + cors_on, + ); + } + if ws_upgrade.0.is_some() { + return ( + StatusCode::UNAUTHORIZED, + "MFA proof required before WebSocket upgrade\n", + ) + .into_response(); + } + return mfa_app_redirect(&state, &spec.id, &upstream_path, &req, "login"); + }; + + let (Some(username), Some(session_id)) = + (admin_session.actor.as_deref(), session_id.as_deref()) + else { + return with_cors( + ( + StatusCode::FORBIDDEN, + "MFA proof required for this protected app\n", + ) + .into_response(), + cors_on, + ); + }; + let decision = crate::mfa::evaluate(&state, username, session_id, &cookies, &spec).await; + if decision != crate::mfa::MfaDecision::Satisfied { + if route_prefix == API_PREFIX { + return with_cors( + ( + StatusCode::FORBIDDEN, + "MFA proof required for this API; complete it in the web portal\n", + ) + .into_response(), + cors_on, + ); + } + if ws_upgrade.0.is_some() { + return ( + StatusCode::UNAUTHORIZED, + "MFA proof required before WebSocket upgrade\n", + ) + .into_response(); + } + let destination = match decision { + crate::mfa::MfaDecision::EnrollmentRequired => "enroll", + crate::mfa::MfaDecision::ChallengeRequired => "challenge", + crate::mfa::MfaDecision::Satisfied => unreachable!(), + }; + return mfa_app_redirect( + &state, + &spec.id, + &upstream_path, + &req, + destination, + ); + } + } + } + // 3. Backend required to proxy. if state.backend.is_none() { return with_cors( @@ -825,6 +933,105 @@ async fn forward( } } +/// Preserve the route's raw percent-encoding and query string for the MFA +/// round trip. Axum has already matched this request as `/app/{spec}/{rest}`; +/// the decoded route values provide a safe fallback, and `local_next_path` +/// applies the same open-redirect guard used by the challenge handlers. +fn mfa_app_next(spec_id: &str, upstream_path: &str, req: &Request) -> String { + let raw = req + .uri() + .path_and_query() + .map(|value| value.as_str()) + .unwrap_or_else(|| req.uri().path()); + if let Some(next) = super::local_next_path(Some(raw)) { + return next.to_string(); + } + + let mut fallback = format!("{APP_PREFIX}{spec_id}{upstream_path}"); + if let Some(query) = req.uri().query() { + fallback.push('?'); + fallback.push_str(query); + } + super::local_next_path(Some(&fallback)) + .unwrap_or("/") + .to_string() +} + +fn mfa_app_redirect( + state: &AppState, + spec_id: &str, + upstream_path: &str, + req: &Request, + destination: &str, +) -> Response { + let next = mfa_app_next(spec_id, upstream_path, req); + let path = match destination { + "login" => format!("{}/admin/login", state.base_path), + "enroll" => format!("{}/admin/account/mfa", state.base_path), + "challenge" => format!("{}/admin/account/mfa/challenge", state.base_path), + _ => unreachable!(), + }; + Redirect::to(&super::with_next_query(&path, &next)).into_response() +} + +async fn audit_mfa_break_glass_bypass(state: &AppState, session_id: &str, spec_id: &str) { + let now = std::time::Instant::now(); + let key = (session_id.to_string(), spec_id.to_string()); + let should_record = match MFA_BREAK_GLASS_AUDITS.entry(key) { + dashmap::mapref::entry::Entry::Occupied(mut entry) => { + if now.duration_since(*entry.get()) < MFA_BREAK_GLASS_AUDIT_COOLDOWN { + false + } else { + entry.insert(now); + true + } + } + dashmap::mapref::entry::Entry::Vacant(entry) => { + entry.insert(now); + true + } + }; + if !should_record { + return; + } + + if MFA_BREAK_GLASS_AUDITS.len() > MFA_BREAK_GLASS_AUDIT_CAP { + MFA_BREAK_GLASS_AUDITS + .retain(|_, at| now.duration_since(*at) < MFA_BREAK_GLASS_AUDIT_COOLDOWN); + } + + let Some(db) = state.db.as_ref() else { + // No DB to persist to: drop the dedup reservation so this bypass is + // re-attempted if a database is later attached, rather than being + // silently suppressed for 15 minutes (codex review, #1005). + MFA_BREAK_GLASS_AUDITS.remove(&(session_id.to_string(), spec_id.to_string())); + tracing::warn!( + spec = %spec_id, + "cannot persist break-glass MFA bypass audit without a config database" + ); + return; + }; + let target = format!("spec:{spec_id}"); + if let Err(err) = crate::db::audit::record( + db, + "token", + "mfa.break_glass_bypass", + &target, + None, + ) + .await + { + // Roll back the reservation on a transient write failure so the + // NEXT bypass retries the audit instead of the key suppressing it + // for the whole cooldown, leaving a persistent audit gap even after + // the DB recovers (codex review, #1005). Over-auditing (a possible + // duplicate row if a concurrent write succeeded) is safe; an audit + // gap is not. + MFA_BREAK_GLASS_AUDITS.remove(&(session_id.to_string(), spec_id.to_string())); + tracing::warn!(error = ?err, spec = %spec_id, "audit break-glass MFA bypass failed"); + } +} + /// Wrap a response body so `guard` only drops once the body is fully /// consumed/dropped — keeping the replica's in-flight count accurate for /// long downloads/streams (#424). diff --git a/crates/ruscker-admin/templates/admin/spec_form.html b/crates/ruscker-admin/templates/admin/spec_form.html index c78e5cc3..0df80f77 100644 --- a/crates/ruscker-admin/templates/admin/spec_form.html +++ b/crates/ruscker-admin/templates/admin/spec_form.html @@ -604,7 +604,6 @@

{{ self.t("spec-form-mfa-validity-hint") }}
-
{{ self.t("spec-form-mfa-staged-note") }}
diff --git a/crates/ruscker-admin/tests/mfa_guard.rs b/crates/ruscker-admin/tests/mfa_guard.rs new file mode 100644 index 00000000..32373a97 --- /dev/null +++ b/crates/ruscker-admin/tests/mfa_guard.rs @@ -0,0 +1,546 @@ +//! Per-app MFA enforcement on the real proxy router (#1005 slice 4). +//! +//! Successful cases use a ready replica backed by a loopback HTTP echo; +//! blocked cases prove the request returns before any upstream/backend work. + +use async_trait::async_trait; +use axum::body::{to_bytes, Body}; +use axum::http::{header, HeaderMap, Request, StatusCode}; +use chrono::{Duration, Utc}; +use ruscker_admin::auth::{AdminAuth, Role, COOKIE_NAME}; +use ruscker_admin::db::ConfigDb; +use ruscker_admin::mfa::DEVICE_COOKIE; +use ruscker_admin::{router, AppState}; +use ruscker_config::Config; +use ruscker_core::{ + ContainerBackend, CoreResult, Replica, ReplicaId, ReplicaMetrics, ReplicaRegistry, + ReplicaState, +}; +use std::net::SocketAddr; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpListener; +use tower::ServiceExt; + +const CONFIG: &str = r#" +proxy: + specs: + - id: protected-app + display-name: Protected app + container-image: test/app + require-mfa: true + inject-base-href: false + - id: session-app + display-name: Session-bound app + container-image: test/app + require-mfa: true + mfa-validity-days: 0 + inject-base-href: false + - id: protected-api + display-name: Protected API + type: api + container-image: test/api + require-mfa: true + api: + cors: true + - id: open-app + display-name: Open app + container-image: test/app + inject-base-href: false +"#; + +const PASSWORD: &str = "CorrectPass9!"; + +#[derive(Default)] +struct RecordingBackend { + spawned: AtomicBool, +} + +#[async_trait] +impl ContainerBackend for RecordingBackend { + async fn spawn(&self, _spec_id: &str, _image: &str) -> CoreResult { + self.spawned.store(true, Ordering::SeqCst); + panic!("MFA-blocked request attempted to spawn a replica") + } + + async fn stop(&self, _replica_id: &ReplicaId) -> CoreResult<()> { + Ok(()) + } + + async fn list(&self) -> CoreResult> { + Ok(Vec::new()) + } + + async fn metrics(&self, _replica_id: &ReplicaId) -> CoreResult { + Ok(ReplicaMetrics { + cpu_percent: 0.0, + memory_bytes: 0, + network_rx_bytes: 0, + network_tx_bytes: 0, + }) + } +} + +async fn open_db() -> ConfigDb { + let path = std::env::temp_dir().join(format!( + "ruscker-mfa-guard-{}.db", + uuid::Uuid::new_v4().simple() + )); + ConfigDb::Sqlite(ruscker_admin::db::open(&path).await.expect("open test DB")) +} + +async fn echo_upstream(expected_requests: usize) -> (SocketAddr, tokio::task::JoinHandle) { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let task = tokio::spawn(async move { + for _ in 0..expected_requests { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut raw = Vec::new(); + let mut chunk = [0_u8; 4096]; + loop { + let n = stream.read(&mut chunk).await.unwrap(); + if n == 0 { + break; + } + raw.extend_from_slice(&chunk[..n]); + if raw.windows(4).any(|window| window == b"\r\n\r\n") { + break; + } + } + stream + .write_all( + b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok", + ) + .await + .unwrap(); + } + expected_requests + }); + (addr, task) +} + +fn state( + db: Option, + spec_id: Option<&str>, + upstream: SocketAddr, + backend: Arc, + base_path: &str, +) -> AppState { + let config = Config::from_yaml(CONFIG).expect("parse MFA guard config"); + let mut replicas = ReplicaRegistry::new(); + if let Some(spec_id) = spec_id { + replicas.add(Replica { + id: ReplicaId(uuid::Uuid::new_v4()), + spec_id: spec_id.to_string(), + container_id: "mfa-guard-test".into(), + upstream, + state: ReplicaState::Ready, + started_at: Utc::now(), + sessions_active: 0, + sessions_max: if spec_id.ends_with("api") { 100 } else { 1 }, + host: None, + }); + } + AppState { + config: Arc::new(config), + base_path: Arc::from(base_path), + locales: Arc::new(ruscker_admin::i18n::Locales::load().expect("load locales")), + admin_auth: AdminAuth::with_token("break-glass-token"), + admin_sessions: Arc::new(ruscker_admin::auth::InMemoryAdminSessionStore::default()), + log_buffer: None, + login_limiter: Arc::new(ruscker_admin::auth::LoginRateLimiter::default_policy()), + api_limiter: Arc::new(ruscker_admin::ratelimit::ApiRateLimiter::new()), + db, + images_dir: None, + master_key: ruscker_admin::crypto::MasterKey::parse(&"ab".repeat(32)).unwrap(), + backend: Some(backend), + replicas: Arc::new(tokio::sync::RwLock::new(replicas)), + cookie_key: ruscker_proxy::sticky::CookieKey::random(), + spawn_locks: Arc::new(dashmap::DashMap::new()), + sessions: Arc::new(ruscker_admin::sessions::InMemorySessionStore::new()), + logout_index: Arc::new(dashmap::DashMap::new()), + leader: Arc::new(ruscker_admin::leader::AlwaysLeader), + metrics: ruscker_admin::metrics_cache::MetricsCache::new(), + draining: Arc::new(AtomicBool::new(false)), + spec_cache: Arc::new(dashmap::DashMap::new()), + identity_cache: Default::default(), + catalog_cache: Arc::new(tokio::sync::RwLock::new(None)), + access_counter: Arc::new(ruscker_admin::access_counter::AccessCounter::default()), + alerts: ruscker_admin::alerts::AlertSink::default(), + } +} + +async fn create_user(db: &ConfigDb, username: &str) { + ruscker_admin::db::users::create( + db, + username, + PASSWORD, + Role::Viewer, + false, + &[], + Some("test"), + ) + .await + .unwrap(); +} + +async fn create_session(state: &AppState, role: Role, actor: Option<&str>) -> (String, String) { + let id = state + .admin_sessions + .create(role, actor.map(str::to_string)) + .await; + (id.clone(), format!("{COOKIE_NAME}={id}")) +} + +async fn enroll(state: &AppState, db: &ConfigDb, username: &str) { + let enrollment = ruscker_admin::mfa::begin(username).unwrap(); + let (secret_enc, nonce) = state + .master_key + .encrypt(enrollment.secret_base32.as_bytes()) + .unwrap(); + let ceremony = uuid::Uuid::new_v4().to_string(); + ruscker_admin::db::mfa::begin_enrollment(db, username, &secret_enc, &nonce, &ceremony) + .await + .unwrap(); + ruscker_admin::db::mfa::confirm_enrollment(db, username, username, &ceremony) + .await + .unwrap(); +} + +async fn issue_grant( + db: &ConfigDb, + username: &str, + session_id: &str, +) -> (String, String) { + let factor = ruscker_admin::db::mfa::fetch(db, username) + .await + .unwrap() + .unwrap(); + let token = ruscker_admin::mfa::generate_device_token().unwrap(); + let token_hash = ruscker_admin::mfa::hash_device_token(&token).unwrap(); + let verified_at = Utc::now(); + let id = ruscker_admin::db::mfa_grants::issue( + db, + username, + &token_hash, + &ruscker_admin::mfa::session_binding(session_id), + factor.confirmed_at.unwrap(), + verified_at, + verified_at + Duration::days(i64::from(ruscker_config::MAX_MFA_VALIDITY_DAYS)), + factor.security_epoch, + None, + None, + "mfa.verify", + username, + ) + .await + .unwrap() + .expect("issue grant"); + (id, token) +} + +fn cookie_jar(session_cookie: &str, grant: Option<&(String, String)>) -> String { + match grant { + Some((id, token)) => format!("{session_cookie}; {DEVICE_COOKIE}={id}.{token}"), + None => session_cookie.to_string(), + } +} + +async fn send( + state: AppState, + uri: &str, + cookie: Option<&str>, +) -> (StatusCode, HeaderMap, String) { + let mut request = Request::builder().method("GET").uri(uri); + if let Some(cookie) = cookie { + request = request.header(header::COOKIE, cookie); + } + let response = router(state) + .oneshot(request.body(Body::empty()).unwrap()) + .await + .unwrap(); + let status = response.status(); + let headers = response.headers().clone(); + let body = to_bytes(response.into_body(), 1 << 20).await.unwrap(); + (status, headers, String::from_utf8(body.to_vec()).unwrap()) +} + +fn location(headers: &HeaderMap) -> Option<&str> { + headers.get(header::LOCATION).and_then(|value| value.to_str().ok()) +} + +#[tokio::test] +async fn protected_app_guides_login_enrollment_and_challenge() { + let db = open_db().await; + let backend = Arc::new(RecordingBackend::default()); + let state = state( + Some(db.clone()), + Some("protected-app"), + SocketAddr::from(([127, 0, 0, 1], 9)), + backend.clone(), + "/box", + ); + + let (status, headers, _) = send( + state.clone(), + "/box/app/protected-app/report%20one?tab=a%2Fb&sort=name", + None, + ) + .await; + assert_eq!(status, StatusCode::SEE_OTHER); + assert_eq!( + location(&headers), + Some("/box/admin/login?next=%2Fapp%2Fprotected-app%2Freport%2520one%3Ftab%3Da%252Fb%26sort%3Dname") + ); + + create_user(&db, "alice").await; + let (_, session_cookie) = create_session(&state, Role::Viewer, Some("alice")).await; + let (status, headers, _) = send( + state.clone(), + "/box/app/protected-app/data?view=1", + Some(&session_cookie), + ) + .await; + assert_eq!(status, StatusCode::SEE_OTHER); + assert_eq!( + location(&headers), + Some("/box/admin/account/mfa?next=%2Fapp%2Fprotected-app%2Fdata%3Fview%3D1") + ); + + enroll(&state, &db, "alice").await; + let (status, headers, _) = send( + state.clone(), + "/box/app/protected-app/data?view=1", + Some(&session_cookie), + ) + .await; + assert_eq!(status, StatusCode::SEE_OTHER); + assert_eq!( + location(&headers), + Some("/box/admin/account/mfa/challenge?next=%2Fapp%2Fprotected-app%2Fdata%3Fview%3D1") + ); + assert!(!backend.spawned.load(Ordering::SeqCst)); +} + +#[tokio::test] +async fn protected_app_with_valid_grant_reaches_upstream() { + let db = open_db().await; + let (upstream, reached) = echo_upstream(1).await; + let backend = Arc::new(RecordingBackend::default()); + let state = state( + Some(db.clone()), + Some("protected-app"), + upstream, + backend.clone(), + "", + ); + create_user(&db, "app-user").await; + let (session_id, session_cookie) = + create_session(&state, Role::Viewer, Some("app-user")).await; + enroll(&state, &db, "app-user").await; + + let grant = issue_grant(&db, "app-user", &session_id).await; + let browser = cookie_jar(&session_cookie, Some(&grant)); + let (status, _, body) = send( + state, + "/app/protected-app/data?view=1", + Some(&browser), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body, "ok"); + assert_eq!(reached.await.unwrap(), 1); + assert!(!backend.spawned.load(Ordering::SeqCst)); +} + +#[tokio::test] +async fn protected_api_fails_closed_without_html_redirects() { + let db = open_db().await; + let state = state( + Some(db.clone()), + Some("protected-api"), + SocketAddr::from(([127, 0, 0, 1], 9)), + Arc::new(RecordingBackend::default()), + "", + ); + + let (status, headers, body) = send(state.clone(), "/api/protected-api/data", None).await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + assert!(location(&headers).is_none()); + assert!(body.contains("authenticated user session")); + assert_eq!(headers.get("access-control-allow-origin").unwrap(), "*"); + + create_user(&db, "bob").await; + let (_, session_cookie) = create_session(&state, Role::Viewer, Some("bob")).await; + enroll(&state, &db, "bob").await; + let (status, headers, body) = send( + state.clone(), + "/api/protected-api/data", + Some(&session_cookie), + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + assert!(location(&headers).is_none()); + assert!(body.contains("complete it in the web portal")); + assert_eq!(headers.get("access-control-allow-origin").unwrap(), "*"); +} + +#[tokio::test] +async fn protected_api_with_valid_grant_reaches_upstream() { + let db = open_db().await; + let (upstream, reached) = echo_upstream(1).await; + let state = state( + Some(db.clone()), + Some("protected-api"), + upstream, + Arc::new(RecordingBackend::default()), + "", + ); + create_user(&db, "api-user").await; + let (session_id, session_cookie) = + create_session(&state, Role::Viewer, Some("api-user")).await; + enroll(&state, &db, "api-user").await; + + let grant = issue_grant(&db, "api-user", &session_id).await; + let browser = cookie_jar(&session_cookie, Some(&grant)); + let (status, _, body) = send( + state, + "/api/protected-api/data", + Some(&browser), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body, "ok"); + assert_eq!(reached.await.unwrap(), 1); +} + +#[tokio::test] +async fn zero_day_grant_is_bound_to_the_login_session_that_proved_mfa() { + let db = open_db().await; + let state = state( + Some(db.clone()), + None, + SocketAddr::from(([127, 0, 0, 1], 9)), + Arc::new(RecordingBackend::default()), + "", + ); + create_user(&db, "carol").await; + enroll(&state, &db, "carol").await; + let (bound_id, bound_cookie) = create_session(&state, Role::Viewer, Some("carol")).await; + let (_, other_cookie) = create_session(&state, Role::Viewer, Some("carol")).await; + let grant = issue_grant(&db, "carol", &bound_id).await; + + let wrong_browser = cookie_jar(&other_cookie, Some(&grant)); + let (status, headers, _) = send( + state.clone(), + "/app/session-app/", + Some(&wrong_browser), + ) + .await; + assert_eq!(status, StatusCode::SEE_OTHER); + assert!(location(&headers).unwrap().starts_with("/admin/account/mfa/challenge?next=")); + + let (upstream, reached) = echo_upstream(1).await; + state.replicas.write().await.add(Replica { + id: ReplicaId(uuid::Uuid::new_v4()), + spec_id: "session-app".into(), + container_id: "mfa-session-test".into(), + upstream, + state: ReplicaState::Ready, + started_at: Utc::now(), + sessions_active: 0, + sessions_max: 1, + host: None, + }); + let right_browser = cookie_jar(&bound_cookie, Some(&grant)); + let (status, _, body) = send( + state, + "/app/session-app/", + Some(&right_browser), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body, "ok"); + assert_eq!(reached.await.unwrap(), 1); +} + +#[tokio::test] +async fn blocked_request_never_enters_the_spawn_coalescer() { + let db = open_db().await; + let backend = Arc::new(RecordingBackend::default()); + let state = state( + Some(db.clone()), + None, + SocketAddr::from(([127, 0, 0, 1], 9)), + backend.clone(), + "", + ); + create_user(&db, "dora").await; + let (_, session_cookie) = create_session(&state, Role::Viewer, Some("dora")).await; + + // No Accept:text/html: absent the MFA guard this request would block on + // pick_or_spawn instead of taking the cold-start splash path. + let (status, headers, _) = send( + state, + "/app/protected-app/data", + Some(&session_cookie), + ) + .await; + assert_eq!(status, StatusCode::SEE_OTHER); + assert!(location(&headers).unwrap().starts_with("/admin/account/mfa?next=")); + assert!(!backend.spawned.load(Ordering::SeqCst)); +} + +#[tokio::test] +async fn break_glass_bypasses_and_audits_once_per_session_and_spec() { + let db = open_db().await; + let (upstream, reached) = echo_upstream(2).await; + let state = state( + Some(db.clone()), + Some("protected-api"), + upstream, + Arc::new(RecordingBackend::default()), + "", + ); + let (_, token_cookie) = create_session(&state, Role::Admin, None).await; + + for _ in 0..2 { + let (status, _, body) = send( + state.clone(), + "/api/protected-api/data", + Some(&token_cookie), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body, "ok"); + } + assert_eq!(reached.await.unwrap(), 2); + + let ConfigDb::Sqlite(pool) = &db else { + unreachable!() + }; + let (count,): (i64,) = sqlx::query_as( + "SELECT COUNT(*) FROM audit_log + WHERE action = 'mfa.break_glass_bypass' + AND actor = 'token' + AND target = 'spec:protected-api'", + ) + .fetch_one(pool) + .await + .unwrap(); + assert_eq!(count, 1); +} + +#[tokio::test] +async fn unprotected_spec_still_forwards_without_a_database_or_mfa_state() { + let (upstream, reached) = echo_upstream(1).await; + let backend = Arc::new(RecordingBackend::default()); + let state = state(None, Some("open-app"), upstream, backend.clone(), ""); + let (status, headers, body) = send(state, "/app/open-app/data", None).await; + assert_eq!(status, StatusCode::OK); + assert!(location(&headers).is_none()); + assert_eq!(body, "ok"); + assert_eq!(reached.await.unwrap(), 1); + assert!(!backend.spawned.load(Ordering::SeqCst)); +} diff --git a/crates/ruscker-cli/src/main.rs b/crates/ruscker-cli/src/main.rs index 6666e59b..96409752 100644 --- a/crates/ruscker-cli/src/main.rs +++ b/crates/ruscker-cli/src/main.rs @@ -983,9 +983,9 @@ fn format_warning(w: &Warning) -> String { "spec {spec} sets mfa-validity-days but require-mfa is not true — the validity setting has no effect" ) } - Warning::MfaNotYetEnforced { spec } => { + Warning::MfaOnExternalSpec { spec } => { format!( - "spec {spec} sets require-mfa: true, but MFA is not yet enforced — this app is NOT protected; enforcement ships in an upcoming release (#1005)" + "spec {spec} sets require-mfa but is an external link — Ruscker never proxies it, so MFA is NOT enforced on the linked app" ) } Warning::InvalidRateLimit { spec_id, value } => { diff --git a/crates/ruscker-config/src/validate.rs b/crates/ruscker-config/src/validate.rs index 60db59bf..0da22c6e 100644 --- a/crates/ruscker-config/src/validate.rs +++ b/crates/ruscker-config/src/validate.rs @@ -94,10 +94,11 @@ pub enum Warning { MfaValidityWithoutRequire { spec: String, }, - /// TODO(#1005 slice 4): remove once the proxy guard enforces MFA. - /// `require-mfa` is security-sensitive but not consumed at runtime yet; - /// this warning prevents a silent false sense of protection. - MfaNotYetEnforced { + /// `require-mfa` is set on an External spec. External specs are plain + /// links — Ruscker never proxies them, so the MFA guard can't run and + /// the flag has no effect. Warn so an operator doesn't believe the + /// linked app is protected (#1005; same transparency policy as #970). + MfaOnExternalSpec { spec: String, }, /// `api.rate-limit` is set but doesn't parse as `N/unit` @@ -562,13 +563,13 @@ fn check_spec(spec: &Spec, warnings: &mut Vec) { }); } } - // TODO(#1005 slice 4): remove once the proxy guard enforces MFA. - if spec.effective_require_mfa() { - warnings.push(Warning::MfaNotYetEnforced { + // External specs are never proxied, so the MFA guard can't run on them + // (#1005). Warn instead of silently ignoring the flag. + if spec.effective_require_mfa() && spec.kind() == SpecKind::External { + warnings.push(Warning::MfaOnExternalSpec { spec: spec.id.clone(), }); } - if let Some(t) = spec.template_properties.type_field() { if !KNOWN_TYPES.contains(&t) { warnings.push(Warning::UnknownTypeProperty { @@ -854,10 +855,28 @@ mod tests { warning, Warning::MfaValidityOutOfRange { .. } | Warning::MfaValidityWithoutRequire { .. } - | Warning::MfaNotYetEnforced { .. } + | Warning::MfaOnExternalSpec { .. } ) } + #[test] + fn flags_require_mfa_on_external_spec() { + let yaml = "proxy:\n specs:\n - id: linked\n external-url: https://example.test\n require-mfa: true\n"; + let report = Config::from_yaml(yaml).expect("parse").validate(); + assert!(report.warnings.iter().any(|w| matches!( + w, + Warning::MfaOnExternalSpec { spec } if spec == "linked" + ))); + // A proxied (container) spec with require-mfa is fine — the guard + // runs there, so no such warning. + let ok = "proxy:\n specs:\n - id: app\n container-image: a:1\n require-mfa: true\n"; + let report = Config::from_yaml(ok).expect("parse").validate(); + assert!(!report + .warnings + .iter() + .any(|w| matches!(w, Warning::MfaOnExternalSpec { .. }))); + } + #[test] fn flags_mfa_validity_above_maximum_only_when_out_of_range() { let yaml = "proxy:\n specs:\n - id: guarded\n container-image: a:1\n require-mfa: true\n mfa-validity-days: 31\n"; @@ -910,33 +929,15 @@ mod tests { } #[test] - fn flags_required_mfa_as_not_yet_enforced_only_when_enabled() { - let yaml = "proxy:\n specs:\n - id: staged\n container-image: a:1\n require-mfa: true\n"; + fn spec_without_mfa_fields_has_no_mfa_warning() { + let yaml = "proxy:\n specs:\n - id: plain\n container-image: a:1\n"; let report = Config::from_yaml(yaml).expect("parse").validate(); - assert_eq!( - report - .warnings - .iter() - .filter(|w| matches!(w, Warning::MfaNotYetEnforced { .. })) - .count(), - 1 - ); - assert!(report.warnings.iter().any(|w| matches!( - w, - Warning::MfaNotYetEnforced { spec } if spec == "staged" - ))); - - let disabled = yaml.replace("true", "false"); - let report = Config::from_yaml(&disabled).expect("parse").validate(); - assert!(!report - .warnings - .iter() - .any(|w| matches!(w, Warning::MfaNotYetEnforced { .. }))); + assert!(!report.warnings.iter().any(is_mfa_warning)); } #[test] - fn spec_without_mfa_fields_has_no_mfa_warning() { - let yaml = "proxy:\n specs:\n - id: plain\n container-image: a:1\n"; + fn required_mfa_is_enforced_and_needs_no_staged_rollout_warning() { + let yaml = "proxy:\n specs:\n - id: guarded\n display-name: Guarded\n description: Protected\n container-image: a:1\n require-mfa: true\n"; let report = Config::from_yaml(yaml).expect("parse").validate(); assert!(!report.warnings.iter().any(is_mfa_warning)); } diff --git a/docs/SECURITY.md b/docs/SECURITY.md index b85d3318..c8cd9774 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -99,6 +99,18 @@ Status: living document. Tracks the Phase 5 security audit (`InMemoryAdminSessionStore`); for HA, point Ruscker at a shared Postgres via `--admin-session-store-url` (#185) so sessions survive a load-balancer hop. +- **[implemented]** Per-app TOTP step-up MFA (#1005) — the factor belongs + to the user and is enrolled once; each spec may opt in with + `require-mfa` and choose how long a successful proof is trusted. Device + grants store only salted token hashes, are bound to the factor's security + epoch and confirmation time, and are revoked by password/factor resets or + the user's **forget devices** actions. A zero-day policy additionally binds + the proof to the current opaque login session. The proxy guard runs before + replica selection/spawn. `RUSCKER_ADMIN_TOKEN` break-glass sessions bypass + the user factor but emit a warning on every request and a cooldown-deduped + `mfa.break_glass_bypass` audit row. The reserved `__ruscker_mfa_*` cookies + are consumed by Ruscker and stripped before proxying, so trusted-device and + enrollment bearers never reach app containers (#258). - **[implemented]** Role-based access control (#101/#107) — three roles (**Viewer** = dashboard read-only; **Editor** = apps + media + dashboard incl. stop/restart; **Admin** = everything, incl. user diff --git a/docs/YAML_SCHEMA.md b/docs/YAML_SCHEMA.md index 09797c8a..5e7cf132 100644 --- a/docs/YAML_SCHEMA.md +++ b/docs/YAML_SCHEMA.md @@ -398,8 +398,16 @@ a successful user-owned TOTP proof for 7 days unless `mfa-validity-days` overrides the window; `0` means proof is valid only in the current login session (no remembered device), and values above 30 clamp to 30. A user without an enrolled TOTP factor will be guided through -enrollment on first access to a protected app. **Staged rollout: not yet -enforced; the proxy guard ships in an upcoming release (#1005).** +enrollment on first access to a protected app. The proxy enforces this guard +before selecting, waking or spawning a replica: `/app` navigation redirects +to enrollment or the MFA challenge, while `/api` fails closed with `401` +(no login session) or `403` (MFA unsatisfied) and no HTML redirect. Emergency +Admin sessions created with `RUSCKER_ADMIN_TOKEN` bypass the factor so an +operator cannot be locked out; every bypass is warned and audit-logged as +`mfa.break_glass_bypass` (deduplicated per session and app for 15 minutes). +`require-mfa` has **no effect on External-link specs** — Ruscker never +proxies them, so there is no request to guard; `validate` warns if you set +it there. `add-default-http-headers` is a ShinyProxy-compatible, per-spec opt-in that forwards `X-SP-UserId` and comma-separated `X-SP-UserGroups` on HTTP