From da58f8e2912437ad91867adc36df9823553637c8 Mon Sep 17 00:00:00 2001 From: yxrxy Date: Tue, 10 Feb 2026 11:23:21 +0800 Subject: [PATCH] fix: Allow non-admin users to read bucket quota configuration. (#1759) Co-authored-by: houseme Co-authored-by: heihutu <30542132+heihutu@users.noreply.github.com> --- crates/e2e_test/src/quota_test.rs | 56 +++++++++++++++++++++++++++++- crates/policy/src/policy/action.rs | 6 ++-- crates/policy/src/policy/policy.rs | 1 + rustfs/src/admin/handlers/quota.rs | 8 ++--- 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/crates/e2e_test/src/quota_test.rs b/crates/e2e_test/src/quota_test.rs index f3c53622e..000c16002 100644 --- a/crates/e2e_test/src/quota_test.rs +++ b/crates/e2e_test/src/quota_test.rs @@ -422,7 +422,7 @@ mod integration_tests { // Check if we can upload 1KB (should succeed - we haven't used the full quota yet) let check_result = env.check_bucket_quota("PUT", 1024).await?; assert!(check_result.get("allowed").unwrap().as_bool().unwrap()); - assert_eq!(check_result.get("remaining_quota").unwrap().as_u64().unwrap(), 512 * 1024 - 1024); + assert_eq!(check_result.get("remaining_quota").unwrap().as_u64().unwrap(), 523264); // 511 * 1024 // Check if we can upload 600KB (should fail - would exceed quota) let check_result = env.check_bucket_quota("PUT", 600 * 1024).await?; @@ -570,6 +570,60 @@ mod integration_tests { Ok(()) } + /// Test that a normal user with `readwrite` policy can read quota but cannot set/clear quota. + #[tokio::test] + #[serial] + async fn test_quota_normal_user_permissions() -> Result<(), Box> { + init_logging(); + let env = QuotaTestEnv::new().await?; + env.create_bucket().await?; + + // Admin sets quota first + env.set_bucket_quota(1024 * 1024).await?; + + // Create a normal user via admin API + let normal_ak = "normaluser"; + let normal_sk = "normaluser123"; + let add_user_url = format!("{}/rustfs/admin/v3/add-user?accessKey={}", env.env.url, normal_ak); + let user_body = serde_json::json!({ "secretKey": normal_sk, "status": "enabled" }).to_string(); + awscurl_put(&add_user_url, &user_body, &env.env.access_key, &env.env.secret_key).await?; + + // Attach `readwrite` policy to the normal user + let policy_url = format!( + "{}/rustfs/admin/v3/set-user-or-group-policy?policyName=readwrite&userOrGroup={}&isGroup=false", + env.env.url, normal_ak + ); + awscurl_put(&policy_url, "", &env.env.access_key, &env.env.secret_key).await?; + + // Normal user reads quota — should succeed + let get_url = format!("{}/rustfs/admin/v3/quota/{}", env.env.url, env.bucket_name); + let resp = awscurl_get(&get_url, normal_ak, normal_sk).await?; + let quota_info: serde_json::Value = serde_json::from_str(&resp)?; + assert_eq!(quota_info.get("quota").and_then(|v| v.as_u64()), Some(1024 * 1024)); + + // Normal user reads quota stats — should succeed + let stats_url = format!("{}/rustfs/admin/v3/quota-stats/{}", env.env.url, env.bucket_name); + let resp = awscurl_get(&stats_url, normal_ak, normal_sk).await?; + assert!(resp.contains("quota_limit")); + + // Normal user sets quota — should be denied + let set_resp = awscurl_put( + &get_url, + &serde_json::json!({"quota": 2048, "quota_type": "HARD"}).to_string(), + normal_ak, + normal_sk, + ) + .await; + assert!(set_resp.is_err(), "normal user should not be able to set quota"); + + // Normal user clears quota — should be denied + let del_resp = awscurl_delete(&get_url, normal_ak, normal_sk).await; + assert!(del_resp.is_err(), "normal user should not be able to clear quota"); + + env.cleanup_bucket().await?; + Ok(()) + } + #[tokio::test] #[serial] async fn test_quota_copy_operations() -> Result<(), Box> { diff --git a/crates/policy/src/policy/action.rs b/crates/policy/src/policy/action.rs index 7af21b494..d5070aa48 100644 --- a/crates/policy/src/policy/action.rs +++ b/crates/policy/src/policy/action.rs @@ -326,6 +326,9 @@ pub enum S3Action { ResetBucketReplicationStateAction, #[strum(serialize = "s3:PutObjectFanOut")] PutObjectFanOutAction, + /// Allow non-admin users to read bucket quota configuration. + #[strum(serialize = "s3:GetBucketQuota")] + GetBucketQuotaAction, } // #[derive(Serialize, Deserialize, Hash, PartialEq, Eq, Clone, EnumString, IntoStaticStr, Debug, Copy)] @@ -469,8 +472,6 @@ pub enum AdminAction { ListUserPoliciesAdminAction, #[strum(serialize = "admin:SetBucketQuota")] SetBucketQuotaAdminAction, - #[strum(serialize = "admin:GetBucketQuota")] - GetBucketQuotaAdminAction, #[strum(serialize = "admin:SetBucketTarget")] SetBucketTargetAction, #[strum(serialize = "admin:GetBucketTarget")] @@ -559,7 +560,6 @@ impl AdminAction { | AdminAction::UpdatePolicyAssociationAction | AdminAction::ListUserPoliciesAdminAction | AdminAction::SetBucketQuotaAdminAction - | AdminAction::GetBucketQuotaAdminAction | AdminAction::SetBucketTargetAction | AdminAction::GetBucketTargetAction | AdminAction::ReplicationDiff diff --git a/crates/policy/src/policy/policy.rs b/crates/policy/src/policy/policy.rs index 91dd243fb..259f44057 100644 --- a/crates/policy/src/policy/policy.rs +++ b/crates/policy/src/policy/policy.rs @@ -320,6 +320,7 @@ pub mod default { let mut hash_set = HashSet::new(); hash_set.insert(Action::S3Action(S3Action::GetBucketLocationAction)); hash_set.insert(Action::S3Action(S3Action::GetObjectAction)); + hash_set.insert(Action::S3Action(S3Action::GetBucketQuotaAction)); hash_set }), not_actions: ActionSet(Default::default()), diff --git a/rustfs/src/admin/handlers/quota.rs b/rustfs/src/admin/handlers/quota.rs index e715d85e6..cbbef7331 100644 --- a/rustfs/src/admin/handlers/quota.rs +++ b/rustfs/src/admin/handlers/quota.rs @@ -21,7 +21,7 @@ use hyper::StatusCode; use matchit::Params; use rustfs_ecstore::bucket::quota::checker::QuotaChecker; use rustfs_ecstore::bucket::quota::{BucketQuota, QuotaError, QuotaOperation}; -use rustfs_policy::policy::action::{Action, AdminAction}; +use rustfs_policy::policy::action::{Action, AdminAction, S3Action}; use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use serde::{Deserialize, Serialize}; use serde_json; @@ -185,7 +185,7 @@ impl Operation for GetBucketQuotaHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::GetBucketQuotaAdminAction)], + vec![Action::S3Action(S3Action::GetBucketQuotaAction)], None, ) .await?; @@ -313,7 +313,7 @@ impl Operation for GetBucketQuotaStatsHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::GetBucketQuotaAdminAction)], + vec![Action::S3Action(S3Action::GetBucketQuotaAction)], None, ) .await?; @@ -380,7 +380,7 @@ impl Operation for CheckBucketQuotaHandler { &cred, owner, false, - vec![Action::AdminAction(AdminAction::GetBucketQuotaAdminAction)], + vec![Action::S3Action(S3Action::GetBucketQuotaAction)], None, ) .await?;