From 3a46baab133422125c967e76f682b438b38ca9d0 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Thu, 20 Aug 2026 00:00:22 +0800 Subject: [PATCH] refactor(admin): route observability handler auth through the shared gate (#6263) Co-authored-by: houseme --- rustfs/src/admin/handlers/audit.rs | 41 ++++++++++++---- rustfs/src/admin/handlers/diagnostics.rs | 34 ++++++++++---- rustfs/src/admin/handlers/event.rs | 42 +++++++++++++---- rustfs/src/admin/handlers/metrics.rs | 25 +++------- rustfs/src/admin/handlers/module_switch.rs | 49 +++++++++++++------- rustfs/src/admin/handlers/profile.rs | 25 +++------- rustfs/src/admin/handlers/profile_admin.rs | 36 +++++++++++---- rustfs/src/admin/handlers/scanner.rs | 54 +++++++++++++--------- rustfs/src/admin/handlers/usage_prefix.rs | 47 +++++++++++++------ 9 files changed, 225 insertions(+), 128 deletions(-) diff --git a/rustfs/src/admin/handlers/audit.rs b/rustfs/src/admin/handlers/audit.rs index d00c7ed49..900bbddbf 100644 --- a/rustfs/src/admin/handlers/audit.rs +++ b/rustfs/src/admin/handlers/audit.rs @@ -13,7 +13,7 @@ // limitations under the License. use crate::admin::{ - auth::validate_admin_request, + auth::authorize_admin_request, handlers::audit_runtime_config::{load_server_config_from_store, update_audit_config_and_reload}, handlers::target_descriptor::{ AdminTargetSpec, EndpointKey, RuntimeHealthStatus, TargetEndpointSource, admin_target_spec_from_builtin, @@ -23,9 +23,8 @@ use crate::admin::{ }, router::{AdminOperation, Operation, S3Router}, }; -use crate::auth::{check_key_valid, get_session_token}; use crate::server::{ - ADMIN_PREFIX, RemoteAddr, is_audit_module_enabled, refresh_audit_module_enabled, refresh_persisted_module_switches_from_store, + ADMIN_PREFIX, is_audit_module_enabled, refresh_audit_module_enabled, refresh_persisted_module_switches_from_store, }; use http::StatusCode; use hyper::Method; @@ -213,14 +212,14 @@ fn audit_target_specs() -> &'static [AdminTargetSpec] { &AUDIT_TARGET_SPECS } +/// The pre-check keeps these endpoints' historical missing-credentials message; +/// the shared gate reports "get cred failed". async fn authorize_audit_admin_request(req: &S3Request, action: AdminAction) -> S3Result<()> { - let Some(input_cred) = &req.credentials else { + if req.credentials.is_none() { return Err(s3_error!(InvalidRequest, "credentials not found")); - }; - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); - validate_admin_request(&req.headers, &cred, owner, false, vec![Action::AdminAction(action)], remote_addr).await + } + authorize_admin_request(req, vec![Action::AdminAction(action)]).await?; + Ok(()) } fn audit_target_mutation_block_reason(config: &Config, target_type: &str, target_name: &str) -> S3Result> { @@ -824,6 +823,30 @@ mod tests { }); } + /// These endpoints authorize through the shared admin gate, which reports + /// "get cred failed" for a credential-less request. The pre-check keeps the + /// message they have always returned (rustfs/backlog#1829). + #[tokio::test] + async fn audit_target_gate_keeps_its_missing_credentials_message() { + let req = S3Request { + input: Body::from(String::new()), + method: Method::PUT, + uri: http::Uri::from_static("/rustfs/admin/v3/audit/target"), + headers: http::HeaderMap::new(), + extensions: http::Extensions::new(), + credentials: None, + region: None, + service: None, + trailing_headers: None, + }; + + let err = authorize_audit_admin_request(&req, AdminAction::SetBucketTargetAction) + .await + .expect_err("a request without credentials must be rejected"); + assert_eq!(err.code(), &s3s::S3ErrorCode::InvalidRequest); + assert_eq!(err.message(), Some("credentials not found")); + } + #[test] fn audit_target_handlers_require_admin_authorization_contract() { let src = include_str!("audit.rs"); diff --git a/rustfs/src/admin/handlers/diagnostics.rs b/rustfs/src/admin/handlers/diagnostics.rs index 83c3625fe..37095a619 100644 --- a/rustfs/src/admin/handlers/diagnostics.rs +++ b/rustfs/src/admin/handlers/diagnostics.rs @@ -23,11 +23,10 @@ //! backing infrastructure (in-process log ring buffer, cross-node object //! speedtest harness). -use crate::admin::auth::validate_admin_request; +use crate::admin::auth::authorize_admin_request; use crate::admin::router::{AdminOperation, Operation, S3Router}; use crate::admin::storage_api::access::spawn_traced; -use crate::auth::{check_key_valid, get_session_token}; -use crate::server::{ADMIN_PREFIX, RemoteAddr}; +use crate::server::ADMIN_PREFIX; use crate::storage::storage_api::get_global_lock_clients; use bytes::Bytes; use futures::{Stream, StreamExt, future::join_all}; @@ -133,16 +132,15 @@ pub fn register_diagnostics_route(r: &mut S3Router) -> std::io:: // Shared auth helper // --------------------------------------------------------------------------- +/// The pre-check keeps these endpoints' historical `AccessDenied` missing-credentials +/// response; the shared gate reports `InvalidRequest` "get cred failed". async fn authorize(req: &S3Request, action: AdminAction) -> S3Result<()> { - let Some(input_cred) = req.credentials.as_ref() else { + if req.credentials.is_none() { return Err(s3_error!(AccessDenied, "Signature is required")); - }; + } - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); - - validate_admin_request(&req.headers, &cred, owner, false, vec![Action::AdminAction(action)], remote_addr).await + authorize_admin_request(req, vec![Action::AdminAction(action)]).await?; + Ok(()) } fn json_response(status: StatusCode, value: &T) -> S3Result> { @@ -1078,6 +1076,22 @@ mod tests { } } + /// These endpoints authorize through the shared admin gate, which rejects a + /// credential-less request with `InvalidRequest` "get cred failed". The + /// pre-check keeps the `AccessDenied` response they have always returned + /// (rustfs/backlog#1829). + #[tokio::test] + async fn diagnostics_gate_keeps_its_missing_credentials_response() { + let err = authorize( + &build_request(Method::GET, "/rustfs/admin/v3/top/locks"), + AdminAction::ServerInfoAdminAction, + ) + .await + .expect_err("a request without credentials must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::AccessDenied); + assert_eq!(err.message(), Some("Signature is required")); + } + #[tokio::test] async fn top_locks_handler_rejects_missing_credentials() { let err = TopLocksHandler {} diff --git a/rustfs/src/admin/handlers/event.rs b/rustfs/src/admin/handlers/event.rs index a820e1fc0..fd91a9418 100644 --- a/rustfs/src/admin/handlers/event.rs +++ b/rustfs/src/admin/handlers/event.rs @@ -13,7 +13,7 @@ // limitations under the License. use crate::admin::{ - auth::validate_admin_request, + auth::authorize_admin_request, handlers::notify_runtime_access::{get_notification_system, load_notification_config_snapshot}, handlers::supervise_admin_mutation, handlers::target_descriptor::{ @@ -26,10 +26,8 @@ use crate::admin::{ runtime_sources::{AppContext, app_context_from_req}, service::config::{preflight_dynamic_config_reload_for_context, signal_dynamic_config_reload_checked_for_context}, }; -use crate::auth::{check_key_valid, get_session_token}; use crate::server::{ - ADMIN_PREFIX, RemoteAddr, is_notify_module_enabled, refresh_notify_module_enabled, - refresh_persisted_module_switches_from_store, + ADMIN_PREFIX, is_notify_module_enabled, refresh_notify_module_enabled, refresh_persisted_module_switches_from_store, }; use http::StatusCode; use hyper::Method; @@ -264,14 +262,14 @@ fn notification_target_specs() -> &'static [AdminTargetSpec] { // --- Helper Functions --- +/// The pre-check keeps these endpoints' historical missing-credentials message; +/// the shared gate reports "get cred failed". async fn authorize_notification_admin_request(req: &S3Request, action: AdminAction) -> S3Result<()> { - let Some(input_cred) = &req.credentials else { + if req.credentials.is_none() { return Err(s3_error!(InvalidRequest, "credentials not found")); - }; - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); - validate_admin_request(&req.headers, &cred, owner, false, vec![Action::AdminAction(action)], remote_addr).await + } + authorize_admin_request(req, vec![Action::AdminAction(action)]).await?; + Ok(()) } fn target_mutation_block_reason(config: &Config, target_type: &str, target_name: &str) -> S3Result> { @@ -987,6 +985,30 @@ mod tests { ); } + /// These endpoints authorize through the shared admin gate, which reports + /// "get cred failed" for a credential-less request. The pre-check keeps the + /// message they have always returned (rustfs/backlog#1829). + #[tokio::test] + async fn notification_target_gate_keeps_its_missing_credentials_message() { + let req = S3Request { + input: Body::from(String::new()), + method: Method::PUT, + uri: http::Uri::from_static("/rustfs/admin/v3/notification/target"), + headers: http::HeaderMap::new(), + extensions: http::Extensions::new(), + credentials: None, + region: None, + service: None, + trailing_headers: None, + }; + + let err = authorize_notification_admin_request(&req, AdminAction::SetBucketTargetAction) + .await + .expect_err("a request without credentials must be rejected"); + assert_eq!(err.code(), &s3s::S3ErrorCode::InvalidRequest); + assert_eq!(err.message(), Some("credentials not found")); + } + #[test] fn notification_target_handlers_require_admin_authorization_contract() { let src = include_str!("event.rs"); diff --git a/rustfs/src/admin/handlers/metrics.rs b/rustfs/src/admin/handlers/metrics.rs index 54e723166..298338508 100644 --- a/rustfs/src/admin/handlers/metrics.rs +++ b/rustfs/src/admin/handlers/metrics.rs @@ -18,12 +18,10 @@ //! keeping the response format explicitly NDJSON. It is not a Prometheus text //! exposition endpoint. -use crate::admin::auth::validate_admin_request; +use crate::admin::auth::authorize_admin_request; use crate::admin::router::Operation; use crate::admin::storage_api::access::spawn_traced; use crate::admin::storage_api::metrics::{CollectMetricsOpts, MetricType, collect_local_metrics}; -use crate::auth::{check_key_valid, get_session_token}; -use crate::server::RemoteAddr; use bytes::Bytes; use futures::{Stream, StreamExt}; use http::{HeaderMap, HeaderValue, Uri}; @@ -182,24 +180,15 @@ impl ByteStream for MetricsStream {} pub struct MetricsHandler {} +/// The pre-check keeps this endpoint's historical `AccessDenied` missing-credentials +/// response; the shared gate reports `InvalidRequest` "get cred failed". async fn authorize_metrics_request(req: &S3Request) -> S3Result<()> { - let Some(input_cred) = req.credentials.as_ref() else { + if req.credentials.is_none() { return Err(s3_error!(AccessDenied, "Signature is required")); - }; + } - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); - - validate_admin_request( - &req.headers, - &cred, - owner, - false, - vec![Action::AdminAction(AdminAction::GetMetricsAction)], - remote_addr, - ) - .await + authorize_admin_request(req, vec![Action::AdminAction(AdminAction::GetMetricsAction)]).await?; + Ok(()) } #[async_trait::async_trait] diff --git a/rustfs/src/admin/handlers/module_switch.rs b/rustfs/src/admin/handlers/module_switch.rs index b5d8f4d5b..f560df991 100644 --- a/rustfs/src/admin/handlers/module_switch.rs +++ b/rustfs/src/admin/handlers/module_switch.rs @@ -17,14 +17,13 @@ use crate::admin::service::config::{ preflight_dynamic_config_reload_for_context, signal_dynamic_config_reload_checked_for_context, }; use crate::admin::{ - auth::validate_admin_request, + auth::authorize_admin_request, handlers::supervise_admin_mutation, router::{AdminOperation, Operation, S3Router}, }; -use crate::auth::{check_key_valid, get_session_token}; use crate::server::{ ADMIN_PREFIX, MODULE_SWITCHES_SIGNAL_SUBSYSTEM, ModuleSwitchSnapshot, ModuleSwitchSource, PersistedModuleSwitches, - RemoteAddr, apply_audit_module_switch_for_context, current_module_switch_snapshot, mark_event_notifier_reconciled, + apply_audit_module_switch_for_context, current_module_switch_snapshot, mark_event_notifier_reconciled, mark_event_notifier_unreconciled, refresh_audit_module_enabled, refresh_notify_module_enabled, refresh_persisted_module_switches_from, refresh_persisted_module_switches_from_store, save_persisted_module_switches_to, validate_module_switch_update, @@ -114,23 +113,15 @@ fn build_response( Ok(S3Response::with_headers((status, Body::from(data)), header)) } +/// The pre-check keeps these endpoints' historical missing-credentials message; +/// the shared gate reports "get cred failed". async fn authorize_module_switch_request(req: &S3Request, action: AdminAction) -> S3Result<()> { - let Some(input_cred) = &req.credentials else { + if req.credentials.is_none() { return Err(s3_error!(InvalidRequest, "authentication required")); - }; + } - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - - validate_admin_request( - &req.headers, - &cred, - owner, - false, - vec![Action::AdminAction(action)], - req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)), - ) - .await + authorize_admin_request(req, vec![Action::AdminAction(action)]).await?; + Ok(()) } async fn refresh_module_switch_snapshot() -> S3Result { @@ -269,6 +260,30 @@ impl Operation for UpdateModuleSwitchesHandler { mod tests { use super::{ModuleSwitchDiscovery, ModuleSwitchSource, ModuleSwitchesResponse}; + /// These endpoints authorize through the shared admin gate, which reports + /// "get cred failed" for a credential-less request. The pre-check keeps the + /// message they have always returned (rustfs/backlog#1829). + #[tokio::test] + async fn module_switch_gate_keeps_its_missing_credentials_message() { + let req = s3s::S3Request { + input: s3s::Body::from(String::new()), + method: http::Method::GET, + uri: http::Uri::from_static("/rustfs/admin/v3/module-switches"), + headers: http::HeaderMap::new(), + extensions: http::Extensions::new(), + credentials: None, + region: None, + service: None, + trailing_headers: None, + }; + + let err = super::authorize_module_switch_request(&req, rustfs_policy::policy::action::AdminAction::ServerInfoAdminAction) + .await + .expect_err("a request without credentials must be rejected"); + assert_eq!(err.code(), &s3s::S3ErrorCode::InvalidRequest); + assert_eq!(err.message(), Some("authentication required")); + } + #[test] fn module_switch_handlers_require_admin_authorization_contract() { let src = include_str!("module_switch.rs"); diff --git a/rustfs/src/admin/handlers/profile.rs b/rustfs/src/admin/handlers/profile.rs index 6a0771a80..fd2ee6b4f 100644 --- a/rustfs/src/admin/handlers/profile.rs +++ b/rustfs/src/admin/handlers/profile.rs @@ -12,33 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::admin::{auth::validate_admin_request, router::Operation}; -use crate::auth::{check_key_valid, get_session_token}; -use crate::server::RemoteAddr; +use crate::admin::{auth::authorize_admin_request, router::Operation}; use http::StatusCode; use matchit::Params; use rustfs_policy::policy::action::{Action, AdminAction}; use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use tracing::info; +/// The pre-check keeps these endpoints' historical `AccessDenied` missing-credentials +/// response; the shared gate reports `InvalidRequest` "get cred failed". pub(super) async fn authorize_profile_request(req: &S3Request) -> S3Result<()> { - let Some(input_cred) = req.credentials.as_ref() else { + if req.credentials.is_none() { return Err(s3_error!(AccessDenied, "Signature is required")); - }; + } - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); - - validate_admin_request( - &req.headers, - &cred, - owner, - false, - vec![Action::AdminAction(AdminAction::ProfilingAdminAction)], - remote_addr, - ) - .await + authorize_admin_request(req, vec![Action::AdminAction(AdminAction::ProfilingAdminAction)]).await?; + Ok(()) } pub(super) fn profile_not_implemented_response(message: String) -> S3Response<(StatusCode, Body)> { diff --git a/rustfs/src/admin/handlers/profile_admin.rs b/rustfs/src/admin/handlers/profile_admin.rs index d14af9f3b..097f8766a 100644 --- a/rustfs/src/admin/handlers/profile_admin.rs +++ b/rustfs/src/admin/handlers/profile_admin.rs @@ -13,11 +13,10 @@ // limitations under the License. use super::profile::{authorize_profile_request, profile_not_implemented_response}; -use crate::admin::auth::validate_admin_request; +use crate::admin::auth::authorize_admin_request; use crate::admin::router::{AdminOperation, Operation, S3Router}; use crate::admin::storage_api::access::spawn_traced; -use crate::auth::{check_key_valid, get_session_token}; -use crate::server::{ADMIN_PREFIX, RemoteAddr}; +use crate::server::ADMIN_PREFIX; use bytes::Bytes; use futures::{Stream, StreamExt}; use http::{HeaderMap, HeaderValue}; @@ -89,14 +88,14 @@ pub fn register_profiling_route(r: &mut S3Router) -> std::io::Re } /// Authorize a request against a single admin action (profiling or trace). +/// The pre-check keeps these endpoints' historical `AccessDenied` missing-credentials +/// response; the shared gate reports `InvalidRequest` "get cred failed". async fn authorize_action(req: &S3Request, action: AdminAction) -> S3Result<()> { - let Some(input_cred) = req.credentials.as_ref() else { + if req.credentials.is_none() { return Err(s3_error!(AccessDenied, "Signature is required")); - }; - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); - validate_admin_request(&req.headers, &cred, owner, false, vec![Action::AdminAction(action)], remote_addr).await + } + authorize_admin_request(req, vec![Action::AdminAction(action)]).await?; + Ok(()) } pub struct ProfileHandler {} @@ -530,7 +529,7 @@ fn trace_value_string(value: &TraceVal) -> String { mod tests { use super::{ ProfileControlHandler, ProfileHandler, ProfileStatusHandler, ProfilingDownloadHandler, ProfilingStartHandler, - TraceHandler, TraceKindFilter, TraceStreamFilter, TraceWireRecord, + TraceHandler, TraceKindFilter, TraceStreamFilter, TraceWireRecord, authorize_action, }; use crate::admin::router::Operation; use http::{Extensions, HeaderMap, Uri}; @@ -539,6 +538,7 @@ mod tests { use rustfs_common::trace_bus::{TraceEvent, TraceFunc, TraceKind}; use rustfs_madmin::service_commands::ServiceTraceOpts; use rustfs_madmin::trace::TraceType; + use rustfs_policy::policy::action::AdminAction; use s3s::{Body, S3ErrorCode, S3Request, S3Result}; use std::time::{Duration, UNIX_EPOCH}; @@ -563,6 +563,22 @@ mod tests { TraceStreamFilter::from_request(&uri, &opts) } + /// The profiling/trace endpoints authorize through the shared admin gate, which + /// rejects a credential-less request with `InvalidRequest` "get cred failed". The + /// pre-check keeps the `AccessDenied` response they have always returned + /// (rustfs/backlog#1829). + #[tokio::test] + async fn profile_admin_gate_keeps_its_missing_credentials_response() { + let err = authorize_action( + &build_profile_request("/rustfs/admin/v3/profiling/start"), + AdminAction::ProfilingAdminAction, + ) + .await + .expect_err("a request without credentials must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::AccessDenied); + assert_eq!(err.message(), Some("Signature is required")); + } + #[tokio::test] async fn profile_handler_rejects_missing_credentials() { let result = ProfileHandler {} diff --git a/rustfs/src/admin/handlers/scanner.rs b/rustfs/src/admin/handlers/scanner.rs index fde894e9c..ad500004d 100644 --- a/rustfs/src/admin/handlers/scanner.rs +++ b/rustfs/src/admin/handlers/scanner.rs @@ -12,12 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::admin::auth::validate_admin_request; +use crate::admin::auth::authorize_admin_request; use crate::admin::router::{AdminOperation, Operation, S3Router}; use crate::admin::runtime_sources::current_scanner_metrics_report; -use crate::auth::{check_key_valid, get_session_token}; use crate::module_switches::{ENV_SCANNER_ENABLED, scanner_enabled_from_env}; -use crate::server::{ADMIN_PREFIX, RemoteAddr}; +use crate::server::ADMIN_PREFIX; use chrono::Utc; use http::{HeaderMap, HeaderValue}; use hyper::{Method, StatusCode}; @@ -154,29 +153,14 @@ pub fn register_scanner_route(r: &mut S3Router) -> std::io::Resu Ok(()) } +/// The pre-check keeps these endpoints' historical missing-credentials message; +/// the shared gate reports "get cred failed". async fn validate_scanner_status_request(req: &S3Request) -> S3Result { - let Some(input_cred) = req.credentials.as_ref() else { + if req.credentials.is_none() { return Err(s3_error!(InvalidRequest, "missing credentials")); - }; + } - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - - let remote_addr = req - .extensions - .get::>() - .and_then(|opt| opt.map(|addr| addr.0)); - validate_admin_request( - &req.headers, - &cred, - owner, - false, - vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)], - remote_addr, - ) - .await?; - - Ok(cred) + authorize_admin_request(req, vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)]).await } fn json_response(body: Vec) -> S3Result> { @@ -229,6 +213,30 @@ impl Operation for IlmExpiryStatusHandler { mod tests { use super::*; + /// These endpoints authorize through the shared admin gate, which reports + /// "get cred failed" for a credential-less request. The pre-check keeps the + /// message they have always returned (rustfs/backlog#1829). + #[tokio::test] + async fn scanner_status_gate_keeps_its_missing_credentials_message() { + let req = S3Request { + input: Body::from(String::new()), + method: Method::GET, + uri: http::Uri::from_static("/rustfs/admin/v3/scanner/status"), + headers: HeaderMap::new(), + extensions: http::Extensions::new(), + credentials: None, + region: None, + service: None, + trailing_headers: None, + }; + + let err = validate_scanner_status_request(&req) + .await + .expect_err("a request without credentials must be rejected"); + assert_eq!(err.code(), &S3ErrorCode::InvalidRequest); + assert_eq!(err.message(), Some("missing credentials")); + } + #[test] fn scanner_disabled_reason_reports_startup_env_key() { assert_eq!(scanner_disabled_reason(true), None); diff --git a/rustfs/src/admin/handlers/usage_prefix.rs b/rustfs/src/admin/handlers/usage_prefix.rs index 10cce50d5..7ef5003c0 100644 --- a/rustfs/src/admin/handlers/usage_prefix.rs +++ b/rustfs/src/admin/handlers/usage_prefix.rs @@ -19,11 +19,10 @@ //! usage caches, with a one-level sub-prefix breakdown — the data console //! buckets view MinIO serves from `loadPrefixUsageFromBackend`. -use crate::admin::auth::validate_admin_request; +use crate::admin::auth::authorize_admin_request; use crate::admin::handlers::system::data_usage_info_gate_actions; use crate::admin::router::{AdminOperation, Operation, S3Router}; -use crate::auth::{check_key_valid, get_session_token}; -use crate::server::{ADMIN_PREFIX, RemoteAddr}; +use crate::server::ADMIN_PREFIX; use http::{HeaderMap, HeaderValue, StatusCode}; use hyper::Method; use matchit::Params; @@ -70,15 +69,10 @@ fn parse_usage_prefix_query(query: Option<&str>) -> S3Result<(String, usize)> { #[async_trait::async_trait] impl Operation for BucketPrefixUsageHandler { async fn call(&self, req: S3Request, params: Params<'_, '_>) -> S3Result> { - let Some(input_cred) = req.credentials else { - return Err(s3_error!(InvalidRequest, "get cred failed")); - }; - - let (cred, owner) = - check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; - - let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); - validate_admin_request(&req.headers, &cred, owner, false, data_usage_info_gate_actions(), remote_addr).await?; + // The shared gate reports the same `InvalidRequest` "get cred failed" this + // handler has always returned for a credential-less request, so it needs no + // message-preserving pre-check. + authorize_admin_request(&req, data_usage_info_gate_actions()).await?; let bucket = params.get("bucket").unwrap_or_default().to_string(); if bucket.is_empty() { @@ -104,13 +98,40 @@ impl Operation for BucketPrefixUsageHandler { #[cfg(test)] mod tests { - use super::{DEFAULT_MAX_ENTRIES, MAX_ENTRIES_LIMIT, parse_usage_prefix_query}; + use super::{BucketPrefixUsageHandler, DEFAULT_MAX_ENTRIES, MAX_ENTRIES_LIMIT, parse_usage_prefix_query}; + use crate::admin::router::Operation; use s3s::S3Error; fn query(raw: &str) -> Result<(String, usize), S3Error> { parse_usage_prefix_query(Some(raw)) } + /// This endpoint authorizes through the shared admin gate, whose + /// credential-less rejection is the same `InvalidRequest` "get cred failed" + /// the handler returned inline before (rustfs/backlog#1829), so no + /// message-preserving pre-check is needed here. + #[tokio::test] + async fn prefix_usage_handler_keeps_its_missing_credentials_message() { + let req = s3s::S3Request { + input: s3s::Body::from(String::new()), + method: http::Method::GET, + uri: http::Uri::from_static("/rustfs/admin/v3/usage/bucket"), + headers: http::HeaderMap::new(), + extensions: http::Extensions::new(), + credentials: None, + region: None, + service: None, + trailing_headers: None, + }; + + let err = BucketPrefixUsageHandler {} + .call(req, matchit::Params::new()) + .await + .expect_err("a request without credentials must be rejected"); + assert_eq!(err.code(), &s3s::S3ErrorCode::InvalidRequest); + assert_eq!(err.message(), Some("get cred failed")); + } + #[test] fn defaults_apply_when_no_query_is_given() { assert_eq!(parse_usage_prefix_query(None).unwrap(), (String::new(), DEFAULT_MAX_ENTRIES));