From e8cad77fdcc00dfc5406a836b761b1d73e2da1b3 Mon Sep 17 00:00:00 2001 From: GatewayJ <18332154+GatewayJ@users.noreply.github.com> Date: Tue, 18 Aug 2026 22:30:01 +0800 Subject: [PATCH 1/2] fix(console): enforce protected API routing --- src/console/handlers/auth.rs | 11 +++++--- src/console/middleware/auth.rs | 27 +++++++------------ src/console/routes/mod.rs | 17 +++++++++--- src/console/server.rs | 47 +++++++++++++++++++++++++++------- 4 files changed, 68 insertions(+), 34 deletions(-) diff --git a/src/console/handlers/auth.rs b/src/console/handlers/auth.rs index 39b02249..bcda8ed1 100755 --- a/src/console/handlers/auth.rs +++ b/src/console/handlers/auth.rs @@ -342,11 +342,16 @@ mod tests { let state = AppState::new("test-secret".to_string()); let session_id = state.create_session("k8s-token".to_string())?; let cookie = format!("session={session_id}"); + let protected = Router::new() + .route("/api/v1/protected", get(|| async { "ok" })) + .route_layer(middleware::from_fn_with_state( + state.clone(), + auth_middleware, + )); let app = Router::new() .route("/api/v1/logout", post(logout)) - .route("/api/v1/protected", get(|| async { "ok" })) - .with_state(state.clone()) - .layer(middleware::from_fn_with_state(state, auth_middleware)); + .merge(protected) + .with_state(state); let logout_response = app .clone() diff --git a/src/console/middleware/auth.rs b/src/console/middleware/auth.rs index 020930bc..f5478da3 100755 --- a/src/console/middleware/auth.rs +++ b/src/console/middleware/auth.rs @@ -34,20 +34,6 @@ pub async fn auth_middleware( if request.method() == Method::OPTIONS { return Ok(next.run(request).await); } - // Unauthenticated paths - let path = request.uri().path(); - if path == "/healthz" - || path == "/readyz" - || path == "/metrics" - || path.starts_with("/api/v1/login") - || path.starts_with("/api/v1/logout") - || path.starts_with("/swagger-ui") - || path.starts_with("/api-docs") - || !path.starts_with("/api/v1") - { - return Ok(next.run(request).await); - } - // Parse session cookie let cookies = request .headers() @@ -124,18 +110,23 @@ mod tests { } #[tokio::test] - async fn static_paths_do_not_require_session() -> Result<(), Box> { + async fn options_requests_bypass_authentication() -> Result<(), Box> { let state = AppState::new("test-secret".to_string()); let app = Router::new() - .route("/", get(|| async { "ui" })) + .route("/protected", get(|| async { "ok" })) .with_state(state.clone()) .layer(middleware::from_fn_with_state(state, auth_middleware)); let response = app - .oneshot(Request::builder().uri("/").body(Body::empty())?) + .oneshot( + Request::builder() + .method(Method::OPTIONS) + .uri("/protected") + .body(Body::empty())?, + ) .await?; - assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.status(), StatusCode::METHOD_NOT_ALLOWED); Ok(()) } } diff --git a/src/console/routes/mod.rs b/src/console/routes/mod.rs index 12d0e144..19a24cfd 100755 --- a/src/console/routes/mod.rs +++ b/src/console/routes/mod.rs @@ -29,16 +29,22 @@ use crate::{ /// Login / session routes (partially unauthenticated) pub fn auth_routes() -> Router { - auth_routes_with_config(AdmissionConfig::for_endpoint( + public_auth_routes_with_config(AdmissionConfig::for_endpoint( AdmissionEndpoint::ConsoleLogin, )) + .merge(session_routes()) } -pub(crate) fn auth_routes_with_config(config: AdmissionConfig) -> Router { - auth_routes_with_admission(AdmissionControl::new(config)) +pub(crate) fn public_auth_routes_with_config(config: AdmissionConfig) -> Router { + public_auth_routes_with_admission(AdmissionControl::new(config)) } +#[cfg(test)] fn auth_routes_with_admission(admission: AdmissionControl) -> Router { + public_auth_routes_with_admission(admission).merge(session_routes()) +} + +fn public_auth_routes_with_admission(admission: AdmissionControl) -> Router { let login = Router::new().route( "/login", post(handlers::auth::login).route_layer(middleware::from_fn_with_state( @@ -49,7 +55,10 @@ fn auth_routes_with_admission(admission: AdmissionControl) -> Router { Router::new() .merge(login) .route("/logout", post(handlers::auth::logout)) - .route("/session", get(handlers::auth::session_check)) +} + +pub(crate) fn session_routes() -> Router { + Router::new().route("/session", get(handlers::auth::session_check)) } async fn enforce_login_admission( diff --git a/src/console/server.rs b/src/console/server.rs index 9512b268..4b0d47fd 100755 --- a/src/console/server.rs +++ b/src/console/server.rs @@ -102,15 +102,11 @@ pub async fn run(port: u16) -> Result<(), Box> { // OpenAPI / Swagger (unauthenticated) .merge(SwaggerUi::new("/swagger-ui").url("/api-docs/openapi.json", ApiDoc::openapi())) // REST API v1 - .nest("/api/v1", api_routes(login_admission_config)) + .nest("/api/v1", api_routes(login_admission_config, state.clone())) // Shared state .with_state(state.clone()); let app = with_static_frontend(app) - // Middleware runs in reverse order: Trace -> Compression -> Cors -> auth - .layer(middleware::from_fn_with_state( - state.clone(), - crate::console::middleware::auth::auth_middleware, - )) + // Middleware runs in reverse order: Trace -> Compression -> Cors .layer( CorsLayer::new() .allow_origin(cors_origins) @@ -144,15 +140,25 @@ pub async fn run(port: u16) -> Result<(), Box> { } /// Merge all `/api/v1` route trees. -fn api_routes(login_admission_config: AdmissionConfig) -> Router { - Router::new() - .merge(routes::auth_routes_with_config(login_admission_config)) +fn api_routes(login_admission_config: AdmissionConfig, state: AppState) -> Router { + let protected = Router::new() + .merge(routes::session_routes()) .merge(routes::tenant_routes()) .merge(routes::pool_routes()) .merge(routes::pod_routes()) .merge(routes::event_routes()) .merge(routes::cluster_routes()) .merge(routes::topology_routes()) + .route_layer(middleware::from_fn_with_state( + state, + crate::console::middleware::auth::auth_middleware, + )); + + Router::new() + .merge(routes::public_auth_routes_with_config( + login_admission_config, + )) + .merge(protected) } fn with_static_frontend(app: Router) -> Router { @@ -321,6 +327,29 @@ mod tests { Ok(()) } + #[tokio::test] + async fn api_router_only_exposes_explicit_public_auth_routes() + -> Result<(), Box> { + let state = AppState::new("test-secret".to_string()); + let app = api_routes( + AdmissionConfig::for_endpoint(AdmissionEndpoint::ConsoleLogin), + state.clone(), + ) + .with_state(state); + + let protected = app + .clone() + .oneshot(Request::get("/session").body(Body::empty())?) + .await?; + assert_eq!(protected.status(), StatusCode::UNAUTHORIZED); + + let public = app + .oneshot(Request::post("/logout").body(Body::empty())?) + .await?; + assert_eq!(public.status(), StatusCode::OK); + Ok(()) + } + fn temp_static_dir() -> std::io::Result { let id = NEXT_TEMP_DIR_ID.fetch_add(1, Ordering::Relaxed); let nanos = std::time::SystemTime::now() From 629f3cc769550fe0484c1566002b716fcc2a0f1f Mon Sep 17 00:00:00 2001 From: GatewayJ <18332154+GatewayJ@users.noreply.github.com> Date: Sat, 22 Aug 2026 18:50:37 +0800 Subject: [PATCH 2/2] fix(ci): satisfy Rust 1.98 clippy --- src/console/middleware/auth.rs | 20 +++++++++-------- src/reconcile/pool_lifecycle.rs | 40 ++++++++++++++++----------------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/console/middleware/auth.rs b/src/console/middleware/auth.rs index f5478da3..653f8f4b 100755 --- a/src/console/middleware/auth.rs +++ b/src/console/middleware/auth.rs @@ -29,10 +29,10 @@ pub async fn auth_middleware( State(state): State, mut request: Request, next: Next, -) -> Result { +) -> Response { // Allow CORS preflight without 401 (browser would treat as CORS failure) if request.method() == Method::OPTIONS { - return Ok(next.run(request).await); + return next.run(request).await; } // Parse session cookie let cookies = request @@ -41,18 +41,20 @@ pub async fn auth_middleware( .and_then(|v| v.to_str().ok()) .unwrap_or(""); - let token = session_cookie_value(cookies) - .ok_or_else(|| unauthorized_response("Missing or invalid session"))?; + let Some(token) = session_cookie_value(cookies) else { + return unauthorized_response("Missing or invalid session"); + }; - let claims = state - .resolve_session(token) - .map_err(|source| Error::Session { source }.into_response())? - .ok_or_else(|| unauthorized_response("Missing or invalid session"))?; + let claims = match state.resolve_session(token) { + Ok(Some(claims)) => claims, + Ok(None) => return unauthorized_response("Missing or invalid session"), + Err(source) => return Error::Session { source }.into_response(), + }; // Stash claims for handlers request.extensions_mut().insert(claims); - Ok(next.run(request).await) + next.run(request).await } fn unauthorized_response(message: &str) -> Response { diff --git a/src/reconcile/pool_lifecycle.rs b/src/reconcile/pool_lifecycle.rs index c723dafd..816c0bc6 100644 --- a/src/reconcile/pool_lifecycle.rs +++ b/src/reconcile/pool_lifecycle.rs @@ -224,7 +224,7 @@ async fn reconcile_single_pool_lifecycle( .await { Ok(status) => status, - Err(decision) => return decision, + Err(decision) => return *decision, }; return cleanup_decommissioned_pool(ctx, tenant, namespace, pool, status).await; @@ -570,69 +570,69 @@ async fn verify_decommissioned_pool_for_cleanup( pool: &Pool, existing: &PoolDecommissionStatus, cluster_domain: &str, -) -> Result { +) -> Result> { let matched_pool = match find_rustfs_pool(client, tenant, namespace, pool, cluster_domain).await { Ok(matched_pool) => matched_pool, Err(error) if error.is_retriable() => { - return Err(cleanup_retriable_decision( + return Err(Box::new(cleanup_retriable_decision( existing.clone(), error.reason(), error.message(), - )); + ))); } Err(error) => { - return Err(failed_decision( + return Err(Box::new(failed_decision( existing.request_id.clone(), error.reason(), error.message(), - )); + ))); } }; let pool_id = matched_pool.item.id.to_string(); let Some(existing_pool_id) = existing.rustfs_pool_id.as_deref() else { - return Err(failed_decision( + return Err(Box::new(failed_decision( existing.request_id.clone(), "RustfsPoolIdentityMissing", "recorded decommission status is missing rustfsPoolID; refusing cleanup", - )); + ))); }; if existing_pool_id != pool_id { let message = format!( "recorded RustFS pool id '{}' no longer matches observed pool id '{}'", existing_pool_id, pool_id ); - return Err(failed_decision( + return Err(Box::new(failed_decision( existing.request_id.clone(), "RustfsPoolIdentityMismatch", &message, - )); + ))); } let Some(existing_hash) = existing.endpoint_set_hash.as_deref() else { - return Err(failed_decision( + return Err(Box::new(failed_decision( existing.request_id.clone(), "RustfsPoolIdentityMissing", "recorded decommission status is missing endpointSetHash; refusing cleanup", - )); + ))); }; if existing_hash != matched_pool.expected_endpoint_set_hash { - return Err(failed_decision( + return Err(Box::new(failed_decision( existing.request_id.clone(), "RustfsPoolIdentityMismatch", "recorded endpoint set hash no longer matches the expected pool cmdline", - )); + ))); } let rustfs_status = match client.pool_status_by_id(&pool_id).await { Ok(status) => status, Err(error) => { - return Err(cleanup_retriable_decision( + return Err(Box::new(cleanup_retriable_decision( existing.clone(), "RustfsDecommissionStatusFailed", &error.to_string(), - )); + ))); } }; @@ -656,19 +656,19 @@ async fn verify_decommissioned_pool_for_cleanup( &matched_pool.expected_endpoint_set_hash, ) .map_err(|message| { - failed_decision( + Box::new(failed_decision( existing.request_id.clone(), "RustfsPoolIdentityMismatch", &message, - ) + )) })?; if !matches!(status.phase, Some(PoolDecommissionPhase::Complete)) { - return Err(failed_decision( + return Err(Box::new(failed_decision( existing.request_id.clone(), "RustfsDecommissionNotComplete", "RustFS no longer reports the pool decommission as complete; refusing cleanup", - )); + ))); } Ok(status)