refactor(admin): route plugin handler auth through authorize_admin_request

The plugin/extension admin family carried seven byte-near copies of the
admin auth preamble (extract credentials, check_key_valid, read RemoteAddr
out of the extensions, call validate_admin_request). Each copy is a place
the gate can drift, which is exactly the review surface rustfs/backlog#1829
tracks.

Every one of the seven is a per-file wrapper with no resource scope and no
audit seam, so it folds onto the shared `authorize_admin_request` gate
without changing the decision. The wrappers keep their own missing-
credentials pre-check, following the pattern established in
`kms_management.rs`: the shared gate reports "get cred failed", while these
endpoints have always reported "authentication required" (six sites) and
"missing credentials" (object_data_cache), and that response must stay
byte-identical. New tests pin each message.

Action sets, `deny_only=false`, the RemoteAddr lookup, and the wrapper
signatures are unchanged, so authenticated-but-unauthorized and
wrong-credential responses are unchanged too.
This commit is contained in:
overtrue
2026-08-19 10:54:47 +08:00
parent cd9c96a03c
commit 70b1fb7d1a
5 changed files with 185 additions and 109 deletions
+32 -17
View File
@@ -14,7 +14,7 @@
use crate::admin::storage_api::cluster::{CapabilityState, CapabilityStatus, ObservabilitySnapshot, TopologySnapshot};
use crate::admin::{
auth::validate_admin_request,
auth::authorize_admin_request,
router::{AdminOperation, Operation, S3Router},
runtime_sources::default_admin_usecase,
storage_api::cluster::{
@@ -24,11 +24,10 @@ use crate::admin::{
},
system,
};
use crate::auth::{check_key_valid, get_session_token};
use crate::cluster_snapshot::{
ClusterReadOnlySnapshot, ClusterRuntimeReadinessState, ClusterRuntimeStatusSnapshot, cluster_has_actionable_pressure,
};
use crate::server::{ADMIN_PREFIX, ReadinessDegradedReason, RemoteAddr};
use crate::server::{ADMIN_PREFIX, ReadinessDegradedReason};
use http::{HeaderMap, HeaderValue, StatusCode};
use hyper::Method;
use matchit::Params;
@@ -66,23 +65,15 @@ pub(crate) struct ClusterSnapshotDiscoveryResponse {
pub components: Option<ClusterComponentStatusView>,
}
/// The pre-check keeps this endpoint's historical missing-credentials message;
/// the shared gate reports "get cred failed".
async fn authorize_cluster_snapshot_request(req: &S3Request<Body>) -> 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(AdminAction::ServerInfoAdminAction)],
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await
authorize_admin_request(req, vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)]).await?;
Ok(())
}
fn build_json_response(
@@ -953,6 +944,30 @@ mod tests {
);
}
/// This endpoint authorizes through the shared admin gate, which reports
/// "get cred failed" for a credential-less request. The pre-check keeps the
/// message it has always returned (rustfs/backlog#1829).
#[tokio::test]
async fn cluster_snapshot_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/v4/cluster/snapshot"),
headers: http::HeaderMap::new(),
extensions: http::Extensions::new(),
credentials: None,
region: None,
service: None,
trailing_headers: None,
};
let err = super::authorize_cluster_snapshot_request(&req)
.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 cluster_snapshot_response_serializes_none_snapshot() {
let value = serde_json::to_value(ClusterSnapshotResponse { snapshot: None }).expect("serialize response");
+44 -31
View File
@@ -14,7 +14,7 @@
use crate::admin::storage_api::cluster::CapabilityStatus;
use crate::admin::{
auth::validate_admin_request,
auth::authorize_admin_request,
handlers::{cluster_snapshot, plugins_instances, system},
plugin_contract::{
PluginContractDomain, PluginInstanceDiagnosticCode, PluginInstanceDiagnosticCount, PluginInstanceEntry,
@@ -22,8 +22,7 @@ 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;
@@ -183,42 +182,26 @@ fn map_extension_instance(instance: PluginInstanceEntry) -> ExtensionInstanceEnt
}
}
/// The pre-check keeps this endpoint's historical missing-credentials message;
/// the shared gate reports "get cred failed".
async fn authorize_extension_catalog_request(req: &S3Request<Body>) -> 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(AdminAction::ServerInfoAdminAction)],
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await
authorize_admin_request(req, vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)]).await?;
Ok(())
}
/// The pre-check keeps this endpoint's historical missing-credentials message;
/// the shared gate reports "get cred failed".
async fn authorize_extension_instance_request(req: &S3Request<Body>) -> 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(AdminAction::GetBucketTargetAction)],
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await
authorize_admin_request(req, vec![Action::AdminAction(AdminAction::GetBucketTargetAction)]).await?;
Ok(())
}
fn build_json_response(
@@ -320,6 +303,36 @@ mod tests {
);
}
/// Both extension gates authorize through the shared admin gate, which reports
/// "get cred failed" for a credential-less request. The pre-check keeps the
/// message these endpoints have always returned (rustfs/backlog#1829).
#[tokio::test]
async fn extension_gates_keep_their_missing_credentials_message() {
let credential_less_request = || s3s::S3Request {
input: s3s::Body::from(String::new()),
method: http::Method::GET,
uri: http::Uri::from_static("/rustfs/admin/v4/extensions/catalog"),
headers: http::HeaderMap::new(),
extensions: http::Extensions::new(),
credentials: None,
region: None,
service: None,
trailing_headers: None,
};
for err in [
super::authorize_extension_catalog_request(&credential_less_request())
.await
.expect_err("a request without credentials must be rejected"),
super::authorize_extension_instance_request(&credential_less_request())
.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 builtin_ops_schemas_register_cleanly_in_runtime_registries() {
let mut diagnostics_registry = rustfs_targets::OpsDiagnosticsRegistry::new();
+32 -12
View File
@@ -21,12 +21,11 @@
//! that bucket, and with `bucket`+`object` it flushes that one identity — the
//! only remediation for a poisoned entry short of a node restart.
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_object_data_cache;
use crate::app::object_data_cache::ObjectDataCacheAdapter;
use crate::auth::{check_key_valid, get_session_token};
use crate::server::{ADMIN_PREFIX, RemoteAddr};
use crate::server::ADMIN_PREFIX;
use http::{HeaderMap, HeaderValue};
use hyper::{Method, StatusCode};
use matchit::Params;
@@ -76,17 +75,14 @@ pub fn register_object_data_cache_route(r: &mut S3Router<AdminOperation>) -> std
Ok(())
}
/// The pre-check keeps these endpoints' historical missing-credentials message;
/// the shared gate reports "get cred failed".
async fn authorize(req: &S3Request<Body>, action: AdminAction) -> 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::<Option<RemoteAddr>>()
.and_then(|opt| opt.map(|addr| addr.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<T: Serialize>(body: &T) -> S3Result<S3Response<(StatusCode, Body)>> {
@@ -208,6 +204,30 @@ mod tests {
assert_eq!(invalidation_outcome(&ObjectDataCacheInvalidationResult::NoOp), ("noop", 0));
}
/// 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 authorize_keeps_its_missing_credentials_message() {
let req = S3Request {
input: Body::from(String::new()),
method: Method::GET,
uri: "/rustfs/admin/v3/object-data-cache/stats".parse().expect("uri should parse"),
headers: HeaderMap::new(),
extensions: http::Extensions::new(),
credentials: None,
region: None,
service: None,
trailing_headers: None,
};
let err = authorize(&req, AdminAction::ServerInfoAdminAction)
.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 stats_handler_requires_server_info_action() {
// Guard the auth contract: the stats endpoint is a read, the flush
+32 -17
View File
@@ -13,7 +13,7 @@
// limitations under the License.
use crate::admin::{
auth::validate_admin_request,
auth::authorize_admin_request,
plugin_contract::{
PluginCatalogAdminDiscovery, PluginCatalogDomainEntry, PluginCatalogEntry, PluginCatalogResponse, PluginContractDomain,
PluginContractEntrypointKind, PluginContractPackaging, PluginDistributionContract, PluginRuntimeContract,
@@ -21,8 +21,7 @@ use crate::admin::{
router::{AdminOperation, Operation, S3Router},
runtime_sources::default_admin_usecase,
};
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;
@@ -114,23 +113,15 @@ fn merge_catalog_descriptor(plugins: &mut HashMap<&'static str, PluginCatalogEnt
}
}
/// The pre-check keeps this endpoint's historical missing-credentials message;
/// the shared gate reports "get cred failed".
async fn authorize_plugin_catalog_request(req: &S3Request<Body>) -> 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(AdminAction::ServerInfoAdminAction)],
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await
authorize_admin_request(req, vec![Action::AdminAction(AdminAction::ServerInfoAdminAction)]).await?;
Ok(())
}
fn build_json_response(
@@ -175,6 +166,30 @@ mod tests {
);
}
/// This endpoint authorizes through the shared admin gate, which reports
/// "get cred failed" for a credential-less request. The pre-check keeps the
/// message it has always returned (rustfs/backlog#1829).
#[tokio::test]
async fn plugin_catalog_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/v4/plugins/catalog"),
headers: http::HeaderMap::new(),
extensions: http::Extensions::new(),
credentials: None,
region: None,
service: None,
trailing_headers: None,
};
let err = super::authorize_plugin_catalog_request(&req)
.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 plugin_catalog_contains_representative_builtin_targets() {
let response = build_catalog_response();
+45 -32
View File
@@ -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, remove_audit_target_config, set_audit_target_config},
handlers::notify_runtime_access::{
load_notification_config_snapshot, remove_notification_target_config, set_notification_target_config,
@@ -29,10 +29,9 @@ 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, is_notify_module_enabled, refresh_audit_module_enabled,
refresh_notify_module_enabled, refresh_persisted_module_switches_from_store,
ADMIN_PREFIX, is_audit_module_enabled, is_notify_module_enabled, refresh_audit_module_enabled, refresh_notify_module_enabled,
refresh_persisted_module_switches_from_store,
};
use hyper::{Method, StatusCode};
use matchit::Params;
@@ -563,42 +562,26 @@ fn plugin_instance_matches_query(instance: &PluginInstanceEntry, query: &str) ->
.any(|field| field.to_ascii_lowercase().contains(&query))
}
/// The pre-check keeps this endpoint's historical missing-credentials message;
/// the shared gate reports "get cred failed".
async fn authorize_plugin_instance_request(req: &S3Request<Body>) -> 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(AdminAction::GetBucketTargetAction)],
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await
authorize_admin_request(req, vec![Action::AdminAction(AdminAction::GetBucketTargetAction)]).await?;
Ok(())
}
/// The pre-check keeps this endpoint's historical missing-credentials message;
/// the shared gate reports "get cred failed".
async fn authorize_plugin_instance_write_request(req: &S3Request<Body>) -> 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(AdminAction::SetBucketTargetAction)],
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
)
.await
authorize_admin_request(req, vec![Action::AdminAction(AdminAction::SetBucketTargetAction)]).await?;
Ok(())
}
fn plugin_instance_mutation_block_reason(
@@ -942,6 +925,36 @@ mod tests {
);
}
/// Both instance gates authorize through the shared admin gate, which reports
/// "get cred failed" for a credential-less request. The pre-check keeps the
/// message these endpoints have always returned (rustfs/backlog#1829).
#[tokio::test]
async fn plugin_instance_gates_keep_their_missing_credentials_message() {
let credential_less_request = || S3Request {
input: Body::from(String::new()),
method: Method::GET,
uri: Uri::from_static("/rustfs/admin/v4/plugins/instances"),
headers: HeaderMap::new(),
extensions: Extensions::new(),
credentials: None,
region: None,
service: None,
trailing_headers: None,
};
for err in [
super::authorize_plugin_instance_request(&credential_less_request())
.await
.expect_err("a request without credentials must be rejected"),
super::authorize_plugin_instance_write_request(&credential_less_request())
.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 configured_instance_without_runtime_appears_offline() {
let config = Config(HashMap::from([(