mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-27 15:37:02 +00:00
refactor(admin): route tier, bucket metadata, archive, transition, and oidc auth through the shared gate (#6688)
This commit is contained in:
@@ -30,11 +30,10 @@ use crate::admin::storage_api::error::StorageError;
|
||||
use crate::storage::storage_api::lock_bucket_targets_metadata;
|
||||
use crate::{
|
||||
admin::{
|
||||
auth::validate_admin_request,
|
||||
auth::authorize_admin_request,
|
||||
router::{AdminOperation, Operation, S3Router},
|
||||
},
|
||||
auth::{check_key_valid, get_session_token},
|
||||
server::{ADMIN_PREFIX, RemoteAddr},
|
||||
server::ADMIN_PREFIX,
|
||||
};
|
||||
use http::{HeaderMap, StatusCode};
|
||||
use hyper::Method;
|
||||
@@ -117,22 +116,11 @@ impl Operation for ExportBucketMetadata {
|
||||
}
|
||||
};
|
||||
|
||||
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::ExportBucketMetadataAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::ExportBucketMetadataAction)]).await?;
|
||||
|
||||
let Some(store) = object_store_from_extensions(&req.extensions) else {
|
||||
return Err(s3_error!(InternalError, "object store is not initialized"));
|
||||
@@ -412,22 +400,11 @@ impl Operation for ImportBucketMetadata {
|
||||
}
|
||||
};
|
||||
|
||||
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::ImportBucketMetadataAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::ImportBucketMetadataAction)]).await?;
|
||||
|
||||
let mut input = req.input;
|
||||
let body = match input.store_all_limited(MAX_BUCKET_METADATA_IMPORT_SIZE).await {
|
||||
@@ -1128,3 +1105,93 @@ mod import_persist_tests {
|
||||
assert!(imported_quota_requires_fleet_proof(&durable).expect("durable quota should pass preflight"));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod shared_gate_tests {
|
||||
use super::*;
|
||||
use http::Uri;
|
||||
use s3s::S3ErrorCode;
|
||||
|
||||
fn credential_less_request(method: Method, uri: &'static str) -> S3Request<Body> {
|
||||
S3Request {
|
||||
input: Body::empty(),
|
||||
method,
|
||||
uri: Uri::from_static(uri),
|
||||
headers: HeaderMap::new(),
|
||||
extensions: http::Extensions::new(),
|
||||
credentials: None,
|
||||
region: None,
|
||||
service: None,
|
||||
trailing_headers: None,
|
||||
}
|
||||
}
|
||||
|
||||
async fn assert_missing_credentials(operation: &dyn Operation, method: Method, uri: &'static str) {
|
||||
let err = operation
|
||||
.call(credential_less_request(method, uri), Params::new())
|
||||
.await
|
||||
.expect_err("a bucket metadata admin request without credentials must fail");
|
||||
assert_eq!(err.code(), &S3ErrorCode::InvalidRequest);
|
||||
assert_eq!(err.message(), Some("authentication required"));
|
||||
}
|
||||
|
||||
/// The shared gate reports "get cred failed"; the per-handler pre-check keeps
|
||||
/// the message each endpoint has always returned (rustfs/backlog#1829).
|
||||
#[tokio::test]
|
||||
async fn bucket_metadata_handlers_keep_their_missing_credentials_response() {
|
||||
assert_missing_credentials(&ExportBucketMetadata {}, Method::GET, "/rustfs/admin/v3/export-bucket-metadata").await;
|
||||
assert_missing_credentials(&ImportBucketMetadata {}, Method::PUT, "/rustfs/admin/v3/import-bucket-metadata").await;
|
||||
}
|
||||
|
||||
fn source_block<'a>(production: &'a str, marker: &str) -> &'a str {
|
||||
let block = production
|
||||
.split_once(marker)
|
||||
.unwrap_or_else(|| panic!("{marker} should exist"))
|
||||
.1;
|
||||
let end = ["\npub struct ", "\nfn ", "\n#[derive(", "\n#[cfg(test)]"]
|
||||
.into_iter()
|
||||
.filter_map(|boundary| block.find(boundary))
|
||||
.min()
|
||||
.unwrap_or(block.len());
|
||||
&block[..end]
|
||||
}
|
||||
|
||||
fn assert_shared_gate_wiring(block: &str, item: &str, actions: &[&str], binds_credentials: bool) {
|
||||
assert_eq!(
|
||||
block.matches("authorize_admin_request(").count(),
|
||||
1,
|
||||
"{item} must use exactly one shared gate"
|
||||
);
|
||||
assert_eq!(
|
||||
block.matches("Action::AdminAction(").count(),
|
||||
actions.len(),
|
||||
"{item} must preserve its exact action-vector length"
|
||||
);
|
||||
for action in actions {
|
||||
assert!(block.contains(&format!("AdminAction::{action}")), "{item} must authorize with {action}");
|
||||
}
|
||||
assert_eq!(
|
||||
block.contains("let cred = authorize_admin_request("),
|
||||
binds_credentials,
|
||||
"{item} credential binding must match its payload-processing contract"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bucket_metadata_handlers_use_the_shared_admin_gate_with_their_actions() {
|
||||
let production = include_str!("bucket_meta.rs")
|
||||
.split("\n#[cfg(test)]\n")
|
||||
.next()
|
||||
.expect("production source must precede tests");
|
||||
|
||||
for (handler, action) in [
|
||||
("ExportBucketMetadata", "ExportBucketMetadataAction"),
|
||||
("ImportBucketMetadata", "ImportBucketMetadataAction"),
|
||||
] {
|
||||
let block = source_block(production, &format!("impl Operation for {handler}"));
|
||||
assert_shared_gate_wiring(block, handler, &[action], false);
|
||||
}
|
||||
|
||||
assert!(!production.contains("check_key_valid(get_session_token"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
// 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::object_store_from_extensions;
|
||||
use crate::admin::storage_api::bucket::is_reserved_or_invalid_bucket;
|
||||
@@ -30,7 +30,6 @@ use crate::admin::storage_api::lifecycle::{
|
||||
request_manual_transition_job_cancel, save_manual_transition_job_record, update_manual_transition_job_record,
|
||||
};
|
||||
use crate::admin::storage_api::runtime::ECStore;
|
||||
use crate::auth::{check_key_valid, get_session_token};
|
||||
use crate::server::{ADMIN_PREFIX, RemoteAddr};
|
||||
use http::{HeaderMap, HeaderValue};
|
||||
use hyper::{Method, StatusCode};
|
||||
@@ -406,20 +405,16 @@ async fn authorize_manual_transition_request(req: &S3Request<Body>) -> S3Result<
|
||||
authorize_transition_admin_request(req, AdminAction::SetTierAction).await
|
||||
}
|
||||
|
||||
/// The credential pre-check keeps this endpoint family's historical
|
||||
/// missing-credentials message (the shared gate reports "get cred failed") and
|
||||
/// still yields the masked actor every transition audit log records.
|
||||
async fn authorize_transition_admin_request(req: &S3Request<Body>, action: AdminAction) -> S3Result<String> {
|
||||
let Some(input_cred) = req.credentials.as_ref() else {
|
||||
return Err(s3_error!(InvalidRequest, "authentication required"));
|
||||
};
|
||||
let actor = MaskedAccessKey(&input_cred.access_key).to_string();
|
||||
|
||||
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(actor)
|
||||
}
|
||||
@@ -1491,6 +1486,50 @@ mod tests {
|
||||
assert!(!auth_block.contains("AdminAction::ServerInfoAdminAction"));
|
||||
}
|
||||
|
||||
/// The transition wrapper now delegates to the shared admin gate, which reports
|
||||
/// "get cred failed"; its own pre-check keeps the message these endpoints have
|
||||
/// always returned (rustfs/backlog#1829).
|
||||
#[tokio::test]
|
||||
async fn transition_admin_gate_keeps_its_missing_credentials_response() {
|
||||
let err = authorize_transition_admin_request(
|
||||
&manual_transition_job_request(Method::GET, "/rustfs/admin/v3/ilm/transition/jobs/job-123"),
|
||||
AdminAction::ListTierAction,
|
||||
)
|
||||
.await
|
||||
.expect_err("a transition admin request without credentials must fail");
|
||||
|
||||
assert_eq!(err.code(), &S3ErrorCode::InvalidRequest);
|
||||
assert_eq!(err.message(), Some("authentication required"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transition_admin_gate_routes_through_the_shared_gate() {
|
||||
let production = include_str!("ilm_transition.rs")
|
||||
.split("\n#[cfg(test)]\n")
|
||||
.next()
|
||||
.expect("production source must precede tests");
|
||||
let wrapper = extract_block_between_markers(
|
||||
production,
|
||||
"async fn authorize_transition_admin_request",
|
||||
"fn transition_transaction_id_from_params",
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
wrapper.matches("authorize_admin_request(").count(),
|
||||
1,
|
||||
"the transition wrapper must use exactly one shared gate"
|
||||
);
|
||||
assert!(
|
||||
wrapper.contains("authorize_admin_request(req, vec![Action::AdminAction(action)])"),
|
||||
"the transition wrapper must forward its parameterized action unchanged"
|
||||
);
|
||||
assert!(
|
||||
wrapper.contains("MaskedAccessKey(&input_cred.access_key)"),
|
||||
"the transition wrapper must keep returning the masked actor"
|
||||
);
|
||||
assert!(!production.contains("check_key_valid(get_session_token"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn manual_transition_job_id_path_param_is_required() {
|
||||
with_manual_transition_job_params("/rustfs/admin/v3/ilm/transition/jobs/job-123", |params| {
|
||||
|
||||
@@ -27,11 +27,10 @@
|
||||
//! digest does not match. Raw `xl.meta`, object contents, drive paths,
|
||||
//! endpoints, user metadata, and encryption keys are never archive entries.
|
||||
|
||||
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::DiskError;
|
||||
use crate::storage::{StorageDiskRpcExt, all_local_disk};
|
||||
use aes_gcm::aead::{Aead, KeyInit, Payload};
|
||||
@@ -576,16 +575,10 @@ fn inspect_archive_gate_actions() -> Vec<Action> {
|
||||
#[async_trait::async_trait]
|
||||
impl Operation for InspectArchiveHandler {
|
||||
async fn call(&self, mut req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
|
||||
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::<Option<RemoteAddr>>()
|
||||
.and_then(|opt| opt.map(|addr| addr.0));
|
||||
validate_admin_request(&req.headers, &cred, owner, false, inspect_archive_gate_actions(), remote_addr).await?;
|
||||
}
|
||||
authorize_admin_request(&req, inspect_archive_gate_actions()).await?;
|
||||
|
||||
let body = req
|
||||
.input
|
||||
@@ -1003,5 +996,42 @@ mod tests {
|
||||
.await
|
||||
.expect_err("unsigned request should fail");
|
||||
assert_eq!(error.code(), &S3ErrorCode::AccessDenied);
|
||||
// The shared gate reports InvalidRequest / "get cred failed"; the pre-check
|
||||
// keeps this endpoint's own signature-required response (rustfs/backlog#1829).
|
||||
assert_eq!(error.message(), Some("Signature is required"));
|
||||
}
|
||||
|
||||
/// The handler authorizes through the shared admin gate and still derives its
|
||||
/// action vector from `inspect_archive_gate_actions()` (rustfs/backlog#1829).
|
||||
#[test]
|
||||
fn inspect_archive_handler_uses_the_shared_admin_gate_with_its_gate_actions() {
|
||||
let production = include_str!("inspect_archive.rs")
|
||||
.split("\n#[cfg(test)]\n")
|
||||
.next()
|
||||
.expect("production source must precede tests");
|
||||
let block = production
|
||||
.split_once("impl Operation for InspectArchiveHandler")
|
||||
.expect("the inspect archive handler must exist")
|
||||
.1;
|
||||
|
||||
assert_eq!(
|
||||
block.matches("authorize_admin_request(").count(),
|
||||
1,
|
||||
"InspectArchiveHandler must use exactly one shared gate"
|
||||
);
|
||||
assert!(
|
||||
block.contains("authorize_admin_request(&req, inspect_archive_gate_actions())"),
|
||||
"InspectArchiveHandler must keep deriving its actions from inspect_archive_gate_actions()"
|
||||
);
|
||||
assert_eq!(
|
||||
block.matches("Action::AdminAction(").count(),
|
||||
0,
|
||||
"InspectArchiveHandler must not inline an action vector"
|
||||
);
|
||||
assert!(
|
||||
!block.contains("let cred = authorize_admin_request("),
|
||||
"InspectArchiveHandler does not need the authenticated credentials"
|
||||
);
|
||||
assert!(!production.contains("check_key_valid(get_session_token"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
// 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::handlers::supervise_admin_mutation;
|
||||
use crate::admin::router::{AdminOperation, Operation, S3Router};
|
||||
use crate::admin::runtime_sources::{
|
||||
@@ -23,8 +23,7 @@ use crate::admin::service::federated_identity::DefaultFederatedSessionBinding;
|
||||
use crate::admin::storage_api::config::{
|
||||
read_admin_config_without_migrate, read_admin_server_config_snapshot, save_admin_server_config_snapshot,
|
||||
};
|
||||
use crate::auth::{check_key_valid, get_session_token};
|
||||
use crate::server::{ADMIN_PREFIX, CONSOLE_PREFIX, MINIO_ADMIN_PREFIX, RemoteAddr};
|
||||
use crate::server::{ADMIN_PREFIX, CONSOLE_PREFIX, MINIO_ADMIN_PREFIX};
|
||||
use http::StatusCode;
|
||||
use hyper::Method;
|
||||
use matchit::Params;
|
||||
@@ -857,23 +856,15 @@ fn redirect_response(location: &str) -> S3Result<S3Response<(StatusCode, Body)>>
|
||||
Ok(resp)
|
||||
}
|
||||
|
||||
/// The pre-check keeps this endpoint family's historical missing-credentials
|
||||
/// message; the shared gate reports "get cred failed".
|
||||
async fn authorize_oidc_config_request(req: &S3Request<Body>, 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::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await
|
||||
authorize_admin_request(req, vec![Action::AdminAction(action)]).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn parse_json_body<T: DeserializeOwned>(req: &mut S3Request<Body>) -> S3Result<T> {
|
||||
@@ -1847,4 +1838,65 @@ mod tests {
|
||||
.expect("provider KVS should exist");
|
||||
assert_eq!(kvs.get(OIDC_ISSUER), "https://app.local/realms/app");
|
||||
}
|
||||
|
||||
/// The OIDC config gate now authorizes through the shared admin gate, which
|
||||
/// reports "get cred failed"; its pre-check keeps the message these endpoints
|
||||
/// have always returned (rustfs/backlog#1829).
|
||||
#[tokio::test]
|
||||
async fn oidc_config_gate_keeps_its_missing_credentials_message() {
|
||||
for action in [AdminAction::ServerInfoAdminAction, AdminAction::ConfigUpdateAdminAction] {
|
||||
let err = authorize_oidc_config_request(&build_oidc_request("/rustfs/admin/v3/idp/openid", None, None), action)
|
||||
.await
|
||||
.expect_err("a request without credentials must be rejected");
|
||||
assert_eq!(err.code(), &S3ErrorCode::InvalidRequest);
|
||||
assert_eq!(err.message(), Some("authentication required"));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn oidc_config_gate_routes_through_the_shared_gate() {
|
||||
let production = include_str!("oidc.rs")
|
||||
.split("\n#[cfg(test)]\n")
|
||||
.next()
|
||||
.expect("production source must precede tests");
|
||||
let wrapper = production
|
||||
.split_once("async fn authorize_oidc_config_request")
|
||||
.expect("the OIDC config gate must exist")
|
||||
.1
|
||||
.split_once("\nasync fn parse_json_body")
|
||||
.expect("the OIDC config gate must be followed by parse_json_body")
|
||||
.0;
|
||||
|
||||
assert_eq!(
|
||||
wrapper.matches("authorize_admin_request(").count(),
|
||||
1,
|
||||
"the OIDC config gate must use exactly one shared gate"
|
||||
);
|
||||
assert!(
|
||||
wrapper.contains("authorize_admin_request(req, vec![Action::AdminAction(action)])"),
|
||||
"the OIDC config gate must forward its parameterized action unchanged"
|
||||
);
|
||||
assert!(
|
||||
!wrapper.contains("let cred = authorize_admin_request("),
|
||||
"the OIDC config gate does not need the authenticated credentials"
|
||||
);
|
||||
|
||||
for (handler, action) in [
|
||||
("GetOidcConfigHandler", "AdminAction::ServerInfoAdminAction"),
|
||||
("PutOidcConfigHandler", "AdminAction::ConfigUpdateAdminAction"),
|
||||
("DeleteOidcConfigHandler", "AdminAction::ConfigUpdateAdminAction"),
|
||||
("ValidateOidcConfigHandler", "AdminAction::ServerInfoAdminAction"),
|
||||
] {
|
||||
let marker = format!("impl Operation for {handler}");
|
||||
let block = production
|
||||
.split_once(marker.as_str())
|
||||
.unwrap_or_else(|| panic!("{handler} should exist"))
|
||||
.1;
|
||||
let block = &block[..block.find("\npub struct ").unwrap_or(block.len())];
|
||||
assert!(
|
||||
block.contains(&format!("authorize_oidc_config_request(&req, {action})")),
|
||||
"{handler} must keep authorizing with {action}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+116
-101
@@ -24,11 +24,10 @@ use crate::admin::storage_api::tier::{
|
||||
use crate::{
|
||||
admin::runtime_sources::{current_daily_tier_stats, current_notification_system, current_tier_config_handle},
|
||||
admin::{
|
||||
auth::validate_admin_request,
|
||||
auth::authorize_admin_request,
|
||||
router::{AdminOperation, Operation, S3Router},
|
||||
},
|
||||
auth::{check_key_valid, get_session_token},
|
||||
server::{ADMIN_PREFIX, RemoteAddr},
|
||||
server::ADMIN_PREFIX,
|
||||
};
|
||||
use http::{HeaderMap, StatusCode, Uri};
|
||||
use hyper::Method;
|
||||
@@ -220,22 +219,11 @@ impl Operation for AddTier {
|
||||
}
|
||||
};
|
||||
|
||||
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::SetTierAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::SetTierAction)]).await?;
|
||||
|
||||
let mut input = req.input;
|
||||
let body = match input.store_all_limited(MAX_ADMIN_REQUEST_BODY_SIZE).await {
|
||||
@@ -436,22 +424,11 @@ impl Operation for EditTier {
|
||||
}
|
||||
};
|
||||
|
||||
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::SetTierAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::SetTierAction)]).await?;
|
||||
|
||||
let mut input = req.input;
|
||||
let body = match input.store_all_limited(MAX_ADMIN_REQUEST_BODY_SIZE).await {
|
||||
@@ -544,22 +521,11 @@ impl Operation for ListTiers {
|
||||
}
|
||||
};
|
||||
|
||||
let Some(input_cred) = req.credentials else {
|
||||
if req.credentials.is_none() {
|
||||
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?;
|
||||
|
||||
validate_admin_request(
|
||||
&req.headers,
|
||||
&cred,
|
||||
owner,
|
||||
false,
|
||||
vec![Action::AdminAction(AdminAction::ListTierAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::ListTierAction)]).await?;
|
||||
|
||||
let tier_config_mgr_handle = current_tier_config_handle();
|
||||
let tier_config_mgr = tier_config_mgr_handle.read().await;
|
||||
@@ -589,22 +555,11 @@ impl Operation for RemoveTier {
|
||||
}
|
||||
};
|
||||
|
||||
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::SetTierAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::SetTierAction)]).await?;
|
||||
|
||||
let mut force: bool = false;
|
||||
let force_str = query.force.clone().unwrap_or_default();
|
||||
@@ -671,22 +626,11 @@ pub struct VerifyTier {}
|
||||
#[async_trait::async_trait]
|
||||
impl Operation for VerifyTier {
|
||||
async fn call(&self, req: S3Request<Body>, params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
|
||||
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::ListTierAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::ListTierAction)]).await?;
|
||||
|
||||
let tier = resolve_tier_name(&req.uri, ¶ms)?;
|
||||
let tier_config_mgr_handle = current_tier_config_handle();
|
||||
@@ -705,22 +649,11 @@ pub struct GetTierInfo {}
|
||||
#[async_trait::async_trait]
|
||||
impl Operation for GetTierInfo {
|
||||
async fn call(&self, req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
|
||||
let Some(input_cred) = req.credentials else {
|
||||
if req.credentials.is_none() {
|
||||
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?;
|
||||
|
||||
validate_admin_request(
|
||||
&req.headers,
|
||||
&cred,
|
||||
owner,
|
||||
false,
|
||||
vec![Action::AdminAction(AdminAction::ListTierAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::ListTierAction)]).await?;
|
||||
|
||||
let query = {
|
||||
if let Some(query) = req.uri.query() {
|
||||
@@ -836,22 +769,11 @@ impl Operation for ClearTier {
|
||||
async fn call(&self, req: S3Request<Body>, params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
|
||||
let query = parse_clear_tier_query(&req.uri)?;
|
||||
|
||||
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::SetTierAction)],
|
||||
req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0)),
|
||||
)
|
||||
.await?;
|
||||
authorize_admin_request(&req, vec![Action::AdminAction(AdminAction::SetTierAction)]).await?;
|
||||
|
||||
let mut force: bool = false;
|
||||
let force_str = query.force;
|
||||
@@ -1186,4 +1108,97 @@ mod tests {
|
||||
stats.insert("ARCHIVE".to_string(), archive);
|
||||
stats
|
||||
}
|
||||
|
||||
fn credential_less_request(method: Method, uri: &'static str) -> S3Request<Body> {
|
||||
S3Request {
|
||||
input: Body::empty(),
|
||||
method,
|
||||
uri: Uri::from_static(uri),
|
||||
headers: HeaderMap::new(),
|
||||
extensions: http::Extensions::new(),
|
||||
credentials: None,
|
||||
region: None,
|
||||
service: None,
|
||||
trailing_headers: None,
|
||||
}
|
||||
}
|
||||
|
||||
async fn assert_missing_credentials(operation: &dyn Operation, method: Method, uri: &'static str, message: &str) {
|
||||
let err = operation
|
||||
.call(credential_less_request(method, uri), Params::new())
|
||||
.await
|
||||
.expect_err("a tier admin request without credentials must fail");
|
||||
assert_eq!(err.code(), &S3ErrorCode::InvalidRequest);
|
||||
assert_eq!(err.message(), Some(message));
|
||||
}
|
||||
|
||||
/// The shared gate reports "get cred failed"; the per-handler pre-check keeps
|
||||
/// the message each endpoint has always returned (rustfs/backlog#1829).
|
||||
#[tokio::test]
|
||||
async fn tier_handlers_keep_their_missing_credentials_response() {
|
||||
assert_missing_credentials(&AddTier {}, Method::PUT, "/rustfs/admin/v3/tier", "authentication required").await;
|
||||
assert_missing_credentials(&EditTier {}, Method::POST, "/rustfs/admin/v3/tier/WARM", "authentication required").await;
|
||||
assert_missing_credentials(&ListTiers {}, Method::GET, "/rustfs/admin/v3/tiers", "get cred failed").await;
|
||||
assert_missing_credentials(&RemoveTier {}, Method::DELETE, "/rustfs/admin/v3/tier/WARM", "authentication required").await;
|
||||
assert_missing_credentials(&VerifyTier {}, Method::GET, "/rustfs/admin/v3/tier/WARM", "authentication required").await;
|
||||
assert_missing_credentials(&GetTierInfo {}, Method::GET, "/rustfs/admin/v3/tier-stats", "get cred failed").await;
|
||||
assert_missing_credentials(&ClearTier {}, Method::DELETE, "/rustfs/admin/v3/tiers", "authentication required").await;
|
||||
}
|
||||
|
||||
fn source_block<'a>(production: &'a str, marker: &str) -> &'a str {
|
||||
let block = production
|
||||
.split_once(marker)
|
||||
.unwrap_or_else(|| panic!("{marker} should exist"))
|
||||
.1;
|
||||
let end = ["\npub struct ", "\nfn ", "\n#[derive(", "\n#[cfg(test)]"]
|
||||
.into_iter()
|
||||
.filter_map(|boundary| block.find(boundary))
|
||||
.min()
|
||||
.unwrap_or(block.len());
|
||||
&block[..end]
|
||||
}
|
||||
|
||||
fn assert_shared_gate_wiring(block: &str, item: &str, actions: &[&str], binds_credentials: bool) {
|
||||
assert_eq!(
|
||||
block.matches("authorize_admin_request(").count(),
|
||||
1,
|
||||
"{item} must use exactly one shared gate"
|
||||
);
|
||||
assert_eq!(
|
||||
block.matches("Action::AdminAction(").count(),
|
||||
actions.len(),
|
||||
"{item} must preserve its exact action-vector length"
|
||||
);
|
||||
for action in actions {
|
||||
assert!(block.contains(&format!("AdminAction::{action}")), "{item} must authorize with {action}");
|
||||
}
|
||||
assert_eq!(
|
||||
block.contains("let cred = authorize_admin_request("),
|
||||
binds_credentials,
|
||||
"{item} credential binding must match its payload-processing contract"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tier_handlers_use_the_shared_admin_gate_with_their_actions() {
|
||||
let production = include_str!("tier.rs")
|
||||
.split("\n#[cfg(test)]\n")
|
||||
.next()
|
||||
.expect("production source must precede tests");
|
||||
|
||||
for (handler, action) in [
|
||||
("AddTier", "SetTierAction"),
|
||||
("EditTier", "SetTierAction"),
|
||||
("ListTiers", "ListTierAction"),
|
||||
("RemoveTier", "SetTierAction"),
|
||||
("VerifyTier", "ListTierAction"),
|
||||
("GetTierInfo", "ListTierAction"),
|
||||
("ClearTier", "SetTierAction"),
|
||||
] {
|
||||
let block = source_block(production, &format!("impl Operation for {handler}"));
|
||||
assert_shared_gate_wiring(block, handler, &[action], false);
|
||||
}
|
||||
|
||||
assert!(!production.contains("check_key_valid(get_session_token"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user