diff --git a/crates/manager-core/src/auth_api.rs b/crates/manager-core/src/auth_api.rs index 04f4d25..0195e6d 100644 --- a/crates/manager-core/src/auth_api.rs +++ b/crates/manager-core/src/auth_api.rs @@ -1,10 +1,14 @@ //! `/api/v1/admin/auth/*` — login, logout, who-am-I. //! -//! Login mints an opaque session token, stores its SHA-256, sets the -//! `picloud_session` HttpOnly cookie, and also returns the raw token in -//! the JSON body for non-browser clients. The same token works as -//! `Authorization: Bearer …` afterward; there is no separate "API -//! token" concept yet. +//! Login mints an opaque session token, stores its SHA-256, and returns +//! the raw token in the JSON body. Clients must send it as +//! `Authorization: Bearer …` on subsequent requests; the same token is +//! used for the session and admin-API authentication paths (there is no +//! separate "API token" concept yet). +//! +//! Cookie auth was removed in the audit 2026-06-11 C-1 fix — same-origin +//! user-route HTML defeated the previous `SameSite=Lax` cookie. The +//! dashboard already uses Bearer; non-browser clients always have. //! //! Logout deletes the session row regardless of whether the supplied //! token matched anything (idempotent). `me` returns the row that the @@ -12,7 +16,7 @@ use axum::body::Body; use axum::extract::{Extension, Request, State}; -use axum::http::{header, HeaderMap, HeaderValue, StatusCode}; +use axum::http::{header, StatusCode}; use axum::middleware::from_fn_with_state; use axum::response::{IntoResponse, Json, Response}; use axum::routing::{get, post}; @@ -25,7 +29,7 @@ use serde_json::json; use picloud_shared::Principal; use crate::auth::{generate_session_token, hash_token, verify_password}; -use crate::auth_middleware::{require_authenticated, AuthState, SESSION_COOKIE}; +use crate::auth_middleware::{require_authenticated, AuthState}; pub fn auth_router(state: AuthState) -> Router { // /login + /logout are unguarded (login is how you get in; logout @@ -139,19 +143,8 @@ async fn login(State(state): State, Json(input): Json) tracing::warn!(?err, "failed to touch admin last_login_at"); } - let mut headers = HeaderMap::new(); - headers.insert( - header::SET_COOKIE, - HeaderValue::from_str(&build_cookie(&token.raw, state.ttl)).unwrap_or_else(|_| { - // Cookie text is ASCII-clean by construction; this branch is - // unreachable in practice but the type signature requires it. - HeaderValue::from_static("") - }), - ); - ( StatusCode::OK, - headers, Json(LoginResponse { user: AdminUserDto { id: user_row.id, @@ -167,23 +160,15 @@ async fn login(State(state): State, Json(input): Json) } async fn logout(State(state): State, req: Request) -> Response { - // Pull token without requiring a valid session (logout is idempotent - // and we still want to clear the cookie on the client side). + // Pull token without requiring a valid session (logout is idempotent). let token = extract_token_for_logout(&req); if let Some(raw) = token { let hash = hash_token(&raw); if let Err(err) = state.sessions.delete(&hash).await { tracing::error!(?err, "admin_sessions delete failed"); - // Still clear the cookie below. } } - - let mut headers = HeaderMap::new(); - headers.insert( - header::SET_COOKIE, - HeaderValue::from_static("picloud_session=; HttpOnly; Path=/; SameSite=Lax; Max-Age=0"), - ); - (StatusCode::NO_CONTENT, headers).into_response() + StatusCode::NO_CONTENT.into_response() } async fn me( @@ -213,51 +198,18 @@ async fn me( // Helpers // ---------------------------------------------------------------------------- -fn build_cookie(raw_token: &str, ttl: std::time::Duration) -> String { - // Secure is on by default; flip to off for HTTP-only dev with - // PICLOUD_COOKIE_SECURE=0. The header-injected bearer token works - // either way, so this is purely for browsers that prefer the cookie - // path (e.g., direct API hits without the dashboard's auth.ts). - let secure = std::env::var("PICLOUD_COOKIE_SECURE").ok().is_none_or(|v| { - !matches!( - v.to_ascii_lowercase().as_str(), - "0" | "false" | "no" | "off" - ) - }); - let secure_attr = if secure { "; Secure" } else { "" }; - format!( - "{SESSION_COOKIE}={raw_token}; HttpOnly{secure_attr}; SameSite=Lax; Path=/; Max-Age={}", - ttl.as_secs() - ) -} - fn extract_token_for_logout(req: &Request) -> Option { - // Same precedence as the middleware — Authorization first, cookie - // fallback. Duplicated here because logout has to read the request - // before any middleware would run. - if let Some(value) = req.headers().get(header::AUTHORIZATION) { - if let Ok(s) = value.to_str() { - if let Some(token) = s.strip_prefix("Bearer ") { - let trimmed = token.trim(); - if !trimmed.is_empty() { - return Some(trimmed.to_string()); - } - } - } + // Duplicated from auth_middleware::extract_token because logout runs + // before the auth middleware (logout is idempotent and must accept + // already-expired tokens). Cookie auth was dropped in the audit + // 2026-06-11 C-1 fix — Bearer only. + let value = req.headers().get(header::AUTHORIZATION)?; + let s = value.to_str().ok()?; + let token = s.strip_prefix("Bearer ")?.trim(); + if token.is_empty() { + return None; } - if let Some(value) = req.headers().get(header::COOKIE) { - if let Ok(s) = value.to_str() { - for chunk in s.split(';') { - let chunk = chunk.trim(); - if let Some(rest) = chunk.strip_prefix(&format!("{SESSION_COOKIE}=")) { - if !rest.is_empty() { - return Some(rest.to_string()); - } - } - } - } - } - None + Some(token.to_string()) } fn invalid_credentials() -> Response { diff --git a/crates/manager-core/src/auth_middleware.rs b/crates/manager-core/src/auth_middleware.rs index b0f1ea3..8621653 100644 --- a/crates/manager-core/src/auth_middleware.rs +++ b/crates/manager-core/src/auth_middleware.rs @@ -1,7 +1,7 @@ //! Authentication middleware — resolves the caller's `Principal` from -//! either a session cookie / Bearer session-token OR an API key -//! (`Authorization: Bearer pic_…`). Both paths converge on the same -//! request extension so downstream handlers see one shape. +//! a Bearer session-token OR an API key (`Authorization: Bearer pic_…`). +//! Both paths converge on the same request extension so downstream +//! handlers see one shape. //! //! Capability checks live in `crate::authz` and are called per-handler //! (after the relevant resource is loaded, so the capability binds to @@ -10,8 +10,14 @@ //! //! Token discriminator: the `pic_` prefix on a Bearer value selects //! the API-key path; anything else (raw 32-byte base64-url-encoded -//! string) takes the session path. The session cookie can only ever -//! carry a session token (cookies are never API keys). +//! string) takes the session path. +//! +//! Audit 2026-06-11 (C-1): cookie auth removed. The platform co-hosts +//! attacker-controlled user-route HTML on the same origin as the admin +//! API; `SameSite=Lax` did not block same-origin POSTs from a malicious +//! script under `/`. Admin clients must send +//! `Authorization: Bearer ` (the dashboard already does; +//! see `dashboard/src/lib/api.ts`). use std::collections::HashMap; use std::sync::{Arc, Mutex}; @@ -88,8 +94,6 @@ impl PrincipalCache { } } -pub const SESSION_COOKIE: &str = "picloud_session"; - /// Prefix on the wire that selects the API-key path. The body that /// follows is `base32(32 random bytes)`; the first 8 chars of the body /// index into `api_keys.prefix` for verification. @@ -355,34 +359,17 @@ async fn username_for(state: &AuthState, id: AdminUserId) -> Option { } } -/// Pull the bearer token out of an `Authorization` header (preferred) -/// or the `picloud_session` cookie (fallback for browser clients). -/// Same shape as Phase 3a; the cookie only ever carries session -/// tokens — no `pic_` prefix expected there. +/// Pull the bearer token out of an `Authorization` header. Cookie auth +/// was removed in the audit 2026-06-11 C-1 fix; same-origin user-route +/// HTML made `SameSite=Lax` insufficient against CSRF. fn extract_token(req: &Request) -> Option { - if let Some(value) = req.headers().get(header::AUTHORIZATION) { - if let Ok(s) = value.to_str() { - if let Some(token) = s.strip_prefix("Bearer ") { - let trimmed = token.trim(); - if !trimmed.is_empty() { - return Some(trimmed.to_string()); - } - } - } + let value = req.headers().get(header::AUTHORIZATION)?; + let s = value.to_str().ok()?; + let token = s.strip_prefix("Bearer ")?.trim(); + if token.is_empty() { + return None; } - if let Some(value) = req.headers().get(header::COOKIE) { - if let Ok(s) = value.to_str() { - for chunk in s.split(';') { - let chunk = chunk.trim(); - if let Some(rest) = chunk.strip_prefix(&format!("{SESSION_COOKIE}=")) { - if !rest.is_empty() { - return Some(rest.to_string()); - } - } - } - } - } - None + Some(token.to_string()) } /// Sentinel returned from the resolve functions when a DB error should @@ -438,13 +425,16 @@ mod tests { } #[test] - fn extracts_cookie_token() { + fn cookie_alone_is_rejected_post_audit_2026_06_11() { + // C-1 closure: `picloud_session` cookie is no longer accepted as + // an auth fallback; a same-origin malicious script could ride the + // cookie via SameSite=Lax POST. Bearer-only. let r = req_with_header("cookie", "foo=bar; picloud_session=xyz; baz=qux"); - assert_eq!(extract_token(&r).as_deref(), Some("xyz")); + assert_eq!(extract_token(&r), None); } #[test] - fn bearer_wins_over_cookie() { + fn bearer_wins_when_both_present() { let r = Request::builder() .header("authorization", "Bearer header-token") .header("cookie", "picloud_session=cookie-token") @@ -454,7 +444,7 @@ mod tests { } #[test] - fn returns_none_when_neither_present() { + fn returns_none_when_no_authorization_header() { let r = Request::builder().body(Body::empty()).unwrap(); assert_eq!(extract_token(&r), None); } diff --git a/crates/manager-core/src/lib.rs b/crates/manager-core/src/lib.rs index 8e40413..aebb91d 100644 --- a/crates/manager-core/src/lib.rs +++ b/crates/manager-core/src/lib.rs @@ -133,7 +133,7 @@ pub use auth_bootstrap::{ #[allow(deprecated)] pub use auth_middleware::{ attach_principal_if_present, require_admin, require_authenticated, AuthState, AuthedAdmin, - API_KEY_PREFIX, API_KEY_PREFIX_LEN, SESSION_COOKIE, + API_KEY_PREFIX, API_KEY_PREFIX_LEN, }; pub use authz::{can, require, AuthzDenied, AuthzError, AuthzRepo, Capability, Decision}; pub use cron_scheduler::spawn_cron_scheduler;