From 090d60e00a27623b333b5803425a313dd87b18f4 Mon Sep 17 00:00:00 2001 From: GatewayJ <835269233@qq.com> Date: Wed, 6 May 2026 08:47:36 +0800 Subject: [PATCH] fix(auth): authorize DeleteObjects per key (#2814) --- .../src/existing_object_tag_policy_test.rs | 130 +++++++++++++++++- rustfs/src/app/object_usecase.rs | 13 ++ rustfs/src/storage/access.rs | 39 +++++- 3 files changed, 169 insertions(+), 13 deletions(-) diff --git a/crates/e2e_test/src/existing_object_tag_policy_test.rs b/crates/e2e_test/src/existing_object_tag_policy_test.rs index 6e82a6582..9a26e95db 100644 --- a/crates/e2e_test/src/existing_object_tag_policy_test.rs +++ b/crates/e2e_test/src/existing_object_tag_policy_test.rs @@ -21,7 +21,7 @@ use crate::common::{ }; use aws_sdk_s3::config::{Credentials, Region}; use aws_sdk_s3::primitives::ByteStream; -use aws_sdk_s3::types::{Tag, Tagging}; +use aws_sdk_s3::types::{Delete, ObjectIdentifier, Tag, Tagging}; use aws_sdk_s3::{Client, Config}; use serial_test::serial; use tracing::info; @@ -318,11 +318,18 @@ async fn test_e2e_sts_assume_role_session_policy_existing_object_tag() -> Result let rw = serde_json::to_string(&serde_json::json!({ "Version": "2012-10-17", - "Statement": [{ - "Effect": "Allow", - "Action": ["s3:*"], - "Resource": ["arn:aws:s3:::*"] - }] + "Statement": [ + { + "Effect": "Allow", + "Action": ["s3:*"], + "Resource": ["arn:aws:s3:::*"] + }, + { + "Effect": "Allow", + "Action": ["sts:AssumeRole"], + "Resource": ["arn:aws:s3:::*"] + } + ] }))?; admin_add_canned_policy(&env, &policy_readwrite, &rw).await?; admin_attach_policy_to_user(&env, &policy_readwrite, &parent).await?; @@ -362,3 +369,114 @@ async fn test_e2e_sts_assume_role_session_policy_existing_object_tag() -> Result info!("test_e2e_sts_assume_role_session_policy_existing_object_tag passed"); Ok(()) } + +/// STS inline session policy: DeleteObjects must evaluate `s3:DeleteObject` per requested object key. +#[tokio::test] +#[serial] +async fn test_e2e_sts_session_policy_delete_objects_object_prefix_only() -> Result<(), Box> { + init_logging(); + if !awscurl_available() { + info!("Skipping test_e2e_sts_session_policy_delete_objects_object_prefix_only: awscurl not available"); + return Ok(()); + } + + let suffix = Uuid::new_v4(); + let parent = format!("e2e-sts-del-par-{suffix}"); + let parent_secret = "longSecretKeyForParentDelete99!"; + let policy_readwrite = format!("e2e-sts-del-rw-{suffix}"); + let bucket = format!("e2e-sts-del-bkt-{suffix}"); + let allowed_key = "allowed/table/data.parquet"; + let denied_key = "denied/table/data.parquet"; + + let mut env = RustFSTestEnvironment::new().await?; + env.start_rustfs_server(vec![]).await?; + + let admin = env.create_s3_client(); + admin_create_user(&env, &parent, parent_secret).await?; + + let rw = serde_json::to_string(&serde_json::json!({ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": ["s3:*"], + "Resource": ["arn:aws:s3:::*"] + }, + { + "Effect": "Allow", + "Action": ["sts:AssumeRole"], + "Resource": ["arn:aws:s3:::*"] + } + ] + }))?; + admin_add_canned_policy(&env, &policy_readwrite, &rw).await?; + admin_attach_policy_to_user(&env, &policy_readwrite, &parent).await?; + + let parent_client = user_client(&env, &parent, parent_secret); + parent_client.create_bucket().bucket(&bucket).send().await?; + parent_client + .put_object() + .bucket(&bucket) + .key(allowed_key) + .body(ByteStream::from_static(b"allowed-delete-data")) + .send() + .await?; + parent_client + .put_object() + .bucket(&bucket) + .key(denied_key) + .body(ByteStream::from_static(b"denied-delete-data")) + .send() + .await?; + + let session_policy = serde_json::json!({ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:DeleteObject"], + "Resource": [format!("arn:aws:s3:::{}/allowed/*", bucket)] + }] + }) + .to_string(); + + let (ak, sk, token) = assume_role_with_session_policy(&env, &parent, parent_secret, &session_policy).await?; + let session_client = sts_session_client(&env, &ak, &sk, &token); + + let delete = Delete::builder() + .objects(ObjectIdentifier::builder().key(allowed_key).build()?) + .objects(ObjectIdentifier::builder().key(denied_key).build()?) + .build()?; + + let result = session_client.delete_objects().bucket(&bucket).delete(delete).send().await?; + + assert_eq!(result.deleted().len(), 1, "only the allowed-prefix object should be deleted"); + assert!( + result.deleted().iter().any(|deleted| deleted.key() == Some(allowed_key)), + "DeleteObjects response should report the allowed-prefix object as deleted" + ); + + assert_eq!(result.errors().len(), 1, "the out-of-prefix object should return one per-key error"); + let error = &result.errors()[0]; + assert_eq!(error.key(), Some(denied_key)); + assert_eq!(error.code(), Some("AccessDenied")); + + let allowed_head = parent_client.head_object().bucket(&bucket).key(allowed_key).send().await; + assert!(allowed_head.is_err(), "allowed-prefix object should have been deleted"); + + parent_client + .head_object() + .bucket(&bucket) + .key(denied_key) + .send() + .await + .expect("out-of-prefix object should remain after per-key AccessDenied"); + + let _ = admin.delete_object().bucket(&bucket).key(allowed_key).send().await; + let _ = admin.delete_object().bucket(&bucket).key(denied_key).send().await; + let _ = admin.delete_bucket().bucket(&bucket).send().await; + admin_remove_user(&env, &parent).await; + admin_remove_policy(&env, &policy_readwrite).await; + + info!("test_e2e_sts_session_policy_delete_objects_object_prefix_only passed"); + Ok(()) +} diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index 1a108ed0a..38377bc00 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -3003,6 +3003,19 @@ impl DefaultObjectUsecase { continue; } + if bypass_governance { + let auth_res = authorize_request(&mut req, Action::S3Action(S3Action::BypassGovernanceRetentionAction)).await; + if let Err(e) = auth_res { + delete_results[idx].error = Some(Error { + code: Some("AccessDenied".to_string()), + key: Some(obj_id.key.clone()), + message: Some(e.to_string()), + version_id: version_id.clone(), + }); + continue; + } + } + let mut object = ObjectToDelete { object_name: obj_id.key.clone(), version_id: version_uuid, diff --git a/rustfs/src/storage/access.rs b/rustfs/src/storage/access.rs index b77a6a6bf..473d3b87c 100644 --- a/rustfs/src/storage/access.rs +++ b/rustfs/src/storage/access.rs @@ -1069,13 +1069,6 @@ impl S3Access for FS { req_info.object = None; req_info.version_id = None; - authorize_request(req, Action::S3Action(S3Action::DeleteObjectAction)).await?; - - // 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?; - } - Ok(()) } @@ -2401,6 +2394,38 @@ mod tests { assert_eq!(req_info.version_id, None); } + #[tokio::test] + async fn delete_objects_defers_object_authorization_to_usecase() { + let input = DeleteObjectsInput::builder() + .bucket("test-bucket".to_string()) + .delete(Delete { + objects: vec![ObjectIdentifier { + key: "prefix/test-key".to_string(), + version_id: None, + ..Default::default() + }], + quiet: None, + }) + .build() + .expect("delete objects input should build"); + + let mut req = build_request(input, Method::POST); + req.extensions.insert(ReqInfo { + cred: Some(rustfs_credentials::Credentials::default()), + ..ReqInfo::default() + }); + + FS::new() + .delete_objects(&mut req) + .await + .expect("DeleteObjects access hook should not require bucket-level DeleteObject"); + + let req_info = req.extensions.get::().expect("req info should remain available"); + assert_eq!(req_info.bucket.as_deref(), Some("test-bucket")); + assert_eq!(req_info.object, None); + assert_eq!(req_info.version_id, None); + } + #[tokio::test] async fn abort_multipart_upload_rejects_unauthorized_request() { let fs = FS::new();