mirror of
https://github.com/rustfs/rustfs.git
synced 2026-11-14 05:40:27 +00:00
refactor(storage): decide DeleteObject in one owned function
DeleteObject's composite access decision moves out of the legacy S3Access hook into access_owned::object_misc::authorize_delete_object, generic over AuthorizationRequest, so the legacy hook and the gateway edge call the same code. The legacy hook loads the bucket generation, calls it, and installs the guard it returns; its order and answers do not change: NoSuchBucket before policy, DeleteObject[Version] for the version storage deletes (the source-version header for a replica delete), ForceDeleteObject, the authenticated-caller rule for a recursive force delete, and BypassGovernanceRetention. Not built here: the gateway edge of DeleteObject and its use case on the owned DTOs (they route the legacy body through the seam and wait for the re-pin). Tests (access_owned/object_misc_test.rs): one table test runs both request forms (s3s request and owned envelope) through the decision and compares the answer and the state handed on (ReqInfo bucket, key, version; guard ready or failed): 5 refused (missing bucket before policy, anonymous, caller without a policy, no caller installed, invalid force-delete value) and 3 allowed (replica delete naming no version gets its source version, any other delete ignores that header, an unavailable generation is handed on as a failed guard). Mutations (each run collects every failing case): - NoSuchBucket check removed, invalid force-delete read as false, replica version swap removed (one build): the missing-bucket, invalid force-delete and replica cases went red, in both request forms - policy decision skipped: the anonymous and no-policy cases went red, in both request forms Verified locally: clippy -D warnings with and without gateway-stack (on the base before the last rebase, which touched none of these files); 64 delete-related lib tests (app::object::delete, the lifecycle delete tests, storage::access delete tests) passed, and the parity test passed in its own run. Not mutation-checked here: ForceDeleteObject, the authenticated-caller rule and BypassGovernanceRetention need a caller a policy partly allows; the existing use-case tests cover the predicate.
This commit is contained in:
overtrue
@@ -12,9 +12,10 @@
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
use self::access_owned::object_misc::authorize_delete_object;
|
||||
use super::ObjectOptions;
|
||||
use super::ecfs::FS;
|
||||
use super::{ECStore, PolicySys, ReplicationStatusType, StorageError, get_lock_acquire_timeout, is_err_bucket_not_found};
|
||||
use super::{ECStore, PolicySys, StorageError, get_lock_acquire_timeout, is_err_bucket_not_found};
|
||||
use crate::auth::facts::{
|
||||
AuthzFacts, AuthzPrincipal, AuthzTarget, base_conditions, build_args, build_conditions, forward_secret_headers,
|
||||
};
|
||||
@@ -3339,67 +3340,10 @@ impl S3Access for FS {
|
||||
let bucket_generation = load_bucket_generation(self.server_ctx(), req, &bucket).await;
|
||||
#[cfg(test)]
|
||||
wait_for_object_generation_test_hook(&bucket).await;
|
||||
// Preserve DeleteObject's established NoSuchBucket response instead of
|
||||
// letting policy lookup turn a missing bucket into AccessDenied.
|
||||
if let Err(err) = &bucket_generation
|
||||
&& err.code() == &S3ErrorCode::NoSuchBucket
|
||||
{
|
||||
return Err(S3Error::with_message(
|
||||
S3ErrorCode::NoSuchBucket,
|
||||
err.message().unwrap_or("The specified bucket does not exist").to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let replica_request = req
|
||||
.headers
|
||||
.get(AMZ_BUCKET_REPLICATION_STATUS)
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.is_some_and(|value| value == ReplicationStatusType::Replica.as_str());
|
||||
// A replica delete may name its version through the source-version
|
||||
// header; the policy decision must see the same version storage deletes.
|
||||
// Storage consumes that header only for a replica request, so any other
|
||||
// request is authorized for the version it names in the query alone.
|
||||
let version_id = if replica_request {
|
||||
super::options::destination_version_id(req.input.version_id.as_deref(), &req.headers)
|
||||
} else {
|
||||
req.input.version_id.clone()
|
||||
};
|
||||
let req_info = ext_req_info_mut(&mut req.extensions)?;
|
||||
req_info.bucket = Some(req.input.bucket.clone());
|
||||
req_info.object = Some(req.input.key.clone());
|
||||
req_info.version_id = version_id;
|
||||
let authenticated = req_info.is_owner || req_info.cred.is_some();
|
||||
let action = delete_object_authorize_action(req_info.version_id.as_deref());
|
||||
|
||||
authorize_request(req, action).await?;
|
||||
|
||||
let force_delete = match force_delete_header(&req.headers) {
|
||||
Ok(value) => value.unwrap_or(false),
|
||||
Err(_) => return Err(invalid_force_delete_header()),
|
||||
};
|
||||
// The REPLICA header is caller-supplied, so it must not skip this
|
||||
// check. s3:ReplicateDelete is still required separately for replica
|
||||
// deletes; s3:* does not grant s3:ForceDeleteObject.
|
||||
if force_delete {
|
||||
authorize_request(req, Action::S3Action(S3Action::ForceDeleteObjectAction)).await?;
|
||||
}
|
||||
if !recursive_force_delete_has_authenticated_caller(&req.headers, authenticated, replica_request) {
|
||||
audit_request_denial(
|
||||
req,
|
||||
DenialAuditor::global(),
|
||||
"recursive_force_delete_requires_authenticated_caller",
|
||||
Action::S3Action(S3Action::ForceDeleteObjectAction),
|
||||
);
|
||||
return Err(s3_error!(AccessDenied, "Recursive force-delete requires an authenticated caller"));
|
||||
}
|
||||
|
||||
// S3 Standard: When bypass_governance header is set, must have s3:BypassGovernanceRetention permission
|
||||
if has_bypass_governance_header(&req.headers) {
|
||||
authorize_request(req, Action::S3Action(S3Action::BypassGovernanceRetentionAction)).await?;
|
||||
}
|
||||
|
||||
req.install_pending_delete_bucket_generation(PendingDeleteBucketGenerationGuard::from_result(bucket_generation));
|
||||
|
||||
let key = req.input.key.clone();
|
||||
let version_id = req.input.version_id.clone();
|
||||
let guard = authorize_delete_object(req, &bucket, &key, version_id.as_deref(), bucket_generation).await?;
|
||||
req.install_pending_delete_bucket_generation(guard);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -20,3 +20,92 @@
|
||||
//! policy evaluation itself (`authorize_request`), or the operation's use case.
|
||||
//! Upstream: the legacy hook in `storage/access.rs` and the batch's `server/gateway_handlers/object_misc.rs`.
|
||||
//! Downstream: the shared shapes in `storage/access.rs`.
|
||||
//!
|
||||
//! DeleteObject: the caller loads the bucket generation before the decision and installs the guard
|
||||
//! returned; the decisions its use case adds afterwards (`ReplicateDelete`, every descendant of a
|
||||
//! recursive force delete) are not here.
|
||||
|
||||
use crate::storage::ReplicationStatusType;
|
||||
use crate::storage::access::{
|
||||
AuthorizationRequest, BucketGenerationGuard, PendingDeleteBucketGenerationGuard, S3Error, S3ErrorCode, S3Result,
|
||||
audit_request_denial, authorize_request, delete_object_authorize_action, has_bypass_governance_header,
|
||||
invalid_force_delete_header, missing_req_info_error, recursive_force_delete_has_authenticated_caller, s3_error,
|
||||
};
|
||||
use crate::storage::denial_audit::DenialAuditor;
|
||||
use crate::storage::options::destination_version_id;
|
||||
use rustfs_policy::policy::action::{Action, S3Action};
|
||||
use rustfs_utils::http::{AMZ_BUCKET_REPLICATION_STATUS, force_delete_header};
|
||||
|
||||
/// DeleteObject's decision, in the legacy order: `NoSuchBucket` before policy, then
|
||||
/// `DeleteObject[Version]` for the version storage deletes (a replica delete names it through its
|
||||
/// source-version header), `ForceDeleteObject` for a force delete, the authenticated-caller rule
|
||||
/// for a recursive force delete, and `BypassGovernanceRetention` when the header asks for it.
|
||||
///
|
||||
/// `bucket_generation` is the bucket's generation as loaded before the decision; the guard it
|
||||
/// becomes is returned for the caller to install.
|
||||
pub(crate) async fn authorize_delete_object<R: AuthorizationRequest + ?Sized>(
|
||||
req: &mut R,
|
||||
bucket: &str,
|
||||
key: &str,
|
||||
version_id: Option<&str>,
|
||||
bucket_generation: S3Result<BucketGenerationGuard>,
|
||||
) -> S3Result<PendingDeleteBucketGenerationGuard> {
|
||||
if let Err(err) = &bucket_generation
|
||||
&& err.code() == &S3ErrorCode::NoSuchBucket
|
||||
{
|
||||
return Err(S3Error::with_message(
|
||||
S3ErrorCode::NoSuchBucket,
|
||||
err.message().unwrap_or("The specified bucket does not exist").to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let replica_request = req
|
||||
.headers()
|
||||
.get(AMZ_BUCKET_REPLICATION_STATUS)
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.is_some_and(|value| value == ReplicationStatusType::Replica.as_str());
|
||||
// Storage reads the source-version header only for a replica request, so any other request is
|
||||
// authorized for the version its query names.
|
||||
let version_id = if replica_request {
|
||||
destination_version_id(version_id, req.headers())
|
||||
} else {
|
||||
version_id.map(str::to_owned)
|
||||
};
|
||||
let req_info = req.req_info_mut().ok_or_else(missing_req_info_error)?;
|
||||
req_info.bucket = Some(bucket.to_owned());
|
||||
req_info.object = Some(key.to_owned());
|
||||
req_info.version_id = version_id;
|
||||
let authenticated = req_info.is_owner || req_info.cred.is_some();
|
||||
let action = delete_object_authorize_action(req_info.version_id.as_deref());
|
||||
|
||||
authorize_request(req, action).await?;
|
||||
|
||||
let force_delete = match force_delete_header(req.headers()) {
|
||||
Ok(value) => value.unwrap_or(false),
|
||||
Err(_) => return Err(invalid_force_delete_header()),
|
||||
};
|
||||
// The REPLICA header is caller-supplied, so it does not skip this check; `s3:*` does not
|
||||
// grant `s3:ForceDeleteObject`.
|
||||
if force_delete {
|
||||
authorize_request(req, Action::S3Action(S3Action::ForceDeleteObjectAction)).await?;
|
||||
}
|
||||
if !recursive_force_delete_has_authenticated_caller(req.headers(), authenticated, replica_request) {
|
||||
audit_request_denial(
|
||||
req,
|
||||
DenialAuditor::global(),
|
||||
"recursive_force_delete_requires_authenticated_caller",
|
||||
Action::S3Action(S3Action::ForceDeleteObjectAction),
|
||||
);
|
||||
return Err(s3_error!(AccessDenied, "Recursive force-delete requires an authenticated caller"));
|
||||
}
|
||||
|
||||
if has_bypass_governance_header(req.headers()) {
|
||||
authorize_request(req, Action::S3Action(S3Action::BypassGovernanceRetentionAction)).await?;
|
||||
}
|
||||
|
||||
Ok(PendingDeleteBucketGenerationGuard::from_result(bucket_generation))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "object_misc_test.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,221 @@
|
||||
// Copyright 2024 RustFS Team
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
//! Both request forms through the one decision: the same answer and the same state handed on
|
||||
//! (the caller's `ReqInfo` and the guard), allowed and refused.
|
||||
|
||||
use super::authorize_delete_object;
|
||||
use crate::app::RequestEnvelope;
|
||||
use crate::storage::access::{
|
||||
AuthorizationRequest, BucketGenerationGuard, PendingDeleteBucketGenerationGuard, ReqInfo, S3Error, S3ErrorCode, S3Request,
|
||||
S3Result,
|
||||
};
|
||||
use crate::storage::storage_api::runtime_sources_consumer::ServerContextSlot;
|
||||
use http::{Extensions, HeaderMap, HeaderValue, Method, Uri};
|
||||
use rustfs_utils::http::{AMZ_BUCKET_REPLICATION_STATUS, SUFFIX_SOURCE_VERSION_ID, insert_header};
|
||||
use std::sync::Arc;
|
||||
|
||||
const BUCKET: &str = "e2-delete-parity";
|
||||
const QUERY_VERSION: &str = "11111111-1111-1111-1111-111111111111";
|
||||
const SOURCE_VERSION: &str = "22222222-2222-2222-2222-222222222222";
|
||||
|
||||
struct Case {
|
||||
name: &'static str,
|
||||
headers: &'static [(&'static str, &'static str)],
|
||||
query_version: Option<&'static str>,
|
||||
caller: Option<ReqInfo>,
|
||||
generation: fn() -> S3Result<BucketGenerationGuard>,
|
||||
expected: Result<Handed, S3ErrorCode>,
|
||||
}
|
||||
|
||||
/// What an allowed decision hands on: the version it authorized and whether the guard is ready.
|
||||
#[derive(Debug, PartialEq)]
|
||||
struct Handed {
|
||||
version_id: Option<String>,
|
||||
guard_ready: bool,
|
||||
}
|
||||
|
||||
fn ready() -> S3Result<BucketGenerationGuard> {
|
||||
Ok(BucketGenerationGuard {
|
||||
bucket: BUCKET.to_owned(),
|
||||
incarnation_id: uuid::Uuid::nil(),
|
||||
})
|
||||
}
|
||||
|
||||
fn missing_bucket() -> S3Result<BucketGenerationGuard> {
|
||||
Err(S3Error::with_message(S3ErrorCode::NoSuchBucket, "gone"))
|
||||
}
|
||||
|
||||
fn unavailable() -> S3Result<BucketGenerationGuard> {
|
||||
Err(S3Error::with_message(S3ErrorCode::ServiceUnavailable, "metadata unavailable"))
|
||||
}
|
||||
|
||||
fn owner() -> Option<ReqInfo> {
|
||||
Some(ReqInfo {
|
||||
cred: Some(rustfs_credentials::Credentials::default()),
|
||||
is_owner: true,
|
||||
..Default::default()
|
||||
})
|
||||
}
|
||||
|
||||
fn stranger() -> Option<ReqInfo> {
|
||||
Some(ReqInfo {
|
||||
cred: Some(rustfs_credentials::Credentials {
|
||||
access_key: "e2-delete-parity-stranger".to_owned(),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
})
|
||||
}
|
||||
|
||||
fn anonymous() -> Option<ReqInfo> {
|
||||
Some(ReqInfo::default())
|
||||
}
|
||||
|
||||
const REPLICA: (&str, &str) = (AMZ_BUCKET_REPLICATION_STATUS, "REPLICA");
|
||||
|
||||
fn cases() -> Vec<Case> {
|
||||
let handed = |version: Option<&str>, guard_ready| {
|
||||
Ok(Handed {
|
||||
version_id: version.map(str::to_owned),
|
||||
guard_ready,
|
||||
})
|
||||
};
|
||||
vec![
|
||||
Case {
|
||||
name: "a missing bucket is named before policy, even to an anonymous caller",
|
||||
headers: &[],
|
||||
query_version: Some(QUERY_VERSION),
|
||||
caller: anonymous(),
|
||||
generation: missing_bucket,
|
||||
expected: Err(S3ErrorCode::NoSuchBucket),
|
||||
},
|
||||
Case {
|
||||
name: "anonymous",
|
||||
headers: &[],
|
||||
query_version: Some(QUERY_VERSION),
|
||||
caller: anonymous(),
|
||||
generation: ready,
|
||||
expected: Err(S3ErrorCode::AccessDenied),
|
||||
},
|
||||
Case {
|
||||
name: "a caller without a policy",
|
||||
headers: &[],
|
||||
query_version: Some(QUERY_VERSION),
|
||||
caller: stranger(),
|
||||
generation: ready,
|
||||
expected: Err(S3ErrorCode::AccessDenied),
|
||||
},
|
||||
Case {
|
||||
name: "no caller installed",
|
||||
headers: &[],
|
||||
query_version: Some(QUERY_VERSION),
|
||||
caller: None,
|
||||
generation: ready,
|
||||
expected: Err(S3ErrorCode::InternalError),
|
||||
},
|
||||
Case {
|
||||
name: "an invalid force-delete value",
|
||||
headers: &[("x-rustfs-force-delete", "maybe")],
|
||||
query_version: Some(QUERY_VERSION),
|
||||
caller: owner(),
|
||||
generation: ready,
|
||||
expected: Err(S3ErrorCode::InvalidRequest),
|
||||
},
|
||||
Case {
|
||||
name: "a replica delete naming no version is authorized for its source version",
|
||||
headers: &[REPLICA],
|
||||
query_version: None,
|
||||
caller: owner(),
|
||||
generation: ready,
|
||||
expected: handed(Some(SOURCE_VERSION), true),
|
||||
},
|
||||
Case {
|
||||
name: "any other delete naming no version ignores the source-version header",
|
||||
headers: &[],
|
||||
query_version: None,
|
||||
caller: owner(),
|
||||
generation: ready,
|
||||
expected: handed(None, true),
|
||||
},
|
||||
Case {
|
||||
name: "an unavailable generation is handed on as a failed guard",
|
||||
headers: &[],
|
||||
query_version: Some(QUERY_VERSION),
|
||||
caller: owner(),
|
||||
generation: unavailable,
|
||||
expected: handed(Some(QUERY_VERSION), false),
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
fn request(case: &Case, server_ctx: &Arc<ServerContextSlot>) -> S3Request<()> {
|
||||
let mut headers = HeaderMap::new();
|
||||
insert_header(&mut headers, SUFFIX_SOURCE_VERSION_ID, SOURCE_VERSION);
|
||||
for (name, value) in case.headers {
|
||||
headers.insert(*name, HeaderValue::from_static(value));
|
||||
}
|
||||
let mut extensions = Extensions::new();
|
||||
if let Some(caller) = &case.caller {
|
||||
extensions.insert(caller.clone());
|
||||
}
|
||||
extensions.insert(Arc::clone(server_ctx));
|
||||
S3Request {
|
||||
input: (),
|
||||
method: Method::DELETE,
|
||||
uri: Uri::from_static("/e2-delete-parity/key"),
|
||||
headers,
|
||||
extensions,
|
||||
credentials: None,
|
||||
region: None,
|
||||
service: None,
|
||||
trailing_headers: None,
|
||||
}
|
||||
}
|
||||
|
||||
async fn decide<R: AuthorizationRequest + ?Sized>(req: &mut R, case: &Case) -> Result<Handed, S3ErrorCode> {
|
||||
let guard = authorize_delete_object(req, BUCKET, "key", case.query_version, (case.generation)())
|
||||
.await
|
||||
.map_err(|error| error.code().clone())?;
|
||||
let info = req.req_info().expect("an allowed decision keeps the caller");
|
||||
assert_eq!((info.bucket.as_deref(), info.object.as_deref()), (Some(BUCKET), Some("key")));
|
||||
Ok(Handed {
|
||||
version_id: info.version_id.clone(),
|
||||
guard_ready: matches!(guard, PendingDeleteBucketGenerationGuard::Ready(_)),
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn both_request_forms_decide_and_hand_on_the_same() {
|
||||
crate::app::gating_test_env::run_large_stack_test("e2-delete-object-parity", || async {
|
||||
let context = crate::app::gating_test_env::shared_gating_ambient().await;
|
||||
let server_ctx = ServerContextSlot::new();
|
||||
assert!(server_ctx.install(Arc::clone(&context)));
|
||||
// Every case runs, so one run names every case a defect breaks.
|
||||
let mut wrong = Vec::new();
|
||||
for case in cases() {
|
||||
let mut legacy = request(&case, &server_ctx);
|
||||
let mut envelope = RequestEnvelope::from_s3s(&legacy);
|
||||
for (form, answer) in [
|
||||
("legacy request", decide(&mut legacy, &case).await),
|
||||
("envelope", decide(&mut envelope, &case).await),
|
||||
] {
|
||||
if answer != case.expected {
|
||||
wrong.push(format!("{} ({form}): {answer:?}", case.name));
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(wrong.is_empty(), "{wrong:#?}");
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user