From 2cb8db36a5fee8f0692d54422778d21fa4d1c67c Mon Sep 17 00:00:00 2001 From: GatewayJ <835269233@qq.com> Date: Mon, 2 Mar 2026 14:19:01 +0800 Subject: [PATCH] fix(iam): user group policy and delete group (fixes #2028) (#2043) Co-authored-by: GatewayJ <8352692332qq.com> --- .gitignore | 1 + crates/e2e_test/src/group_delete_test.rs | 132 +++++++++++++++++++++++ crates/iam/src/manager.rs | 28 ++--- crates/iam/src/store/object.rs | 4 +- 4 files changed, 151 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index c80107ae6..e54161a48 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ .DS_Store .idea .vscode +.cursor .direnv/ /test /logs diff --git a/crates/e2e_test/src/group_delete_test.rs b/crates/e2e_test/src/group_delete_test.rs index 80a698b27..2fcccb60b 100644 --- a/crates/e2e_test/src/group_delete_test.rs +++ b/crates/e2e_test/src/group_delete_test.rs @@ -12,10 +12,26 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! E2E tests for group management (fixes #2028). + use crate::common::{RustFSTestEnvironment, awscurl_delete, awscurl_get, awscurl_put, init_logging}; +use aws_sdk_s3::config::{Credentials, Region}; +use aws_sdk_s3::{Client, Config}; use serial_test::serial; use tracing::info; +fn create_user_s3_client(env: &RustFSTestEnvironment, access_key: &str, secret_key: &str) -> Client { + let credentials = Credentials::new(access_key, secret_key, None, None, "e2e-group-test"); + let config = Config::builder() + .credentials_provider(credentials) + .region(Region::new("us-east-1")) + .endpoint_url(&env.url) + .force_path_style(true) + .behavior_version_latest() + .build(); + Client::from_conf(config) +} + /// Test that deleting a group with members fails, and deleting an empty group succeeds. #[tokio::test(flavor = "multi_thread")] #[serial] @@ -74,3 +90,119 @@ async fn test_delete_group_requires_empty_membership() -> Result<(), Box Result<(), Box> { + init_logging(); + + let mut env = RustFSTestEnvironment::new().await?; + env.start_rustfs_server(vec![]).await?; + + let user_name = "grouponlyuser"; + let user_secret = "grouponlysecret"; + let group_name = "policygroup"; + let policy_name = "ListBucketsOnlyPolicy"; + + // 1. Create canned policy that allows ListAllMyBuckets only + let policy_doc = serde_json::json!({ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:ListAllMyBuckets"], + "Resource": ["*"] + }] + }); + let add_policy_url = format!("{}/rustfs/admin/v3/add-canned-policy?name={}", env.url, policy_name); + awscurl_put(&add_policy_url, &policy_doc.to_string(), &env.access_key, &env.secret_key).await?; + info!("Created canned policy {}", policy_name); + + // 2. Create user with no explicit policy + let add_user_url = format!("{}/rustfs/admin/v3/add-user?accessKey={}", env.url, user_name); + let user_body = serde_json::json!({ + "secretKey": user_secret, + "status": "enabled" + }); + awscurl_put(&add_user_url, &user_body.to_string(), &env.access_key, &env.secret_key).await?; + info!("Created user {} with no explicit policy", user_name); + + // 3. Add user to group (creates group with this member; user_group_memberships must be updated) + let update_members_url = format!("{}/rustfs/admin/v3/update-group-members", env.url); + let add_member_body = serde_json::json!({ + "group": group_name, + "members": [user_name], + "isRemove": false, + "groupStatus": "enabled" + }); + awscurl_put(&update_members_url, &add_member_body.to_string(), &env.access_key, &env.secret_key).await?; + info!("Added {} to group {}", user_name, group_name); + + // 4. Attach policy to group + let set_policy_url = format!( + "{}/rustfs/admin/v3/set-user-or-group-policy?policyName={}&userOrGroup={}&isGroup=true", + env.url, policy_name, group_name + ); + awscurl_put(&set_policy_url, "", &env.access_key, &env.secret_key).await?; + info!("Attached policy {} to group {}", policy_name, group_name); + + // 5. User with only group (no user policy) should be able to list buckets + let user_client = create_user_s3_client(&env, user_name, user_secret); + let list_result = user_client.list_buckets().send().await; + list_result.map_err(|e| -> Box { + format!("User with only group membership should get group policies and list buckets: {}", e).into() + })?; + info!("User with only group successfully listed buckets (group policies applied)"); + + Ok(()) +} + +/// Test that after deleting a user who was the only member of a group, the group can be deleted +/// (regression test for #2028.2: delete group uses backend membership, not stale cache). +#[tokio::test(flavor = "multi_thread")] +#[serial] +#[ignore = "requires awscurl and spawns a real RustFS server"] +async fn test_delete_group_after_deleting_user() -> Result<(), Box> { + init_logging(); + + let mut env = RustFSTestEnvironment::new().await?; + env.start_rustfs_server(vec![]).await?; + + let user_name = "solemember"; + let user_secret = "solemembersecret"; + let group_name = "soledeletegroup"; + + // 1. Create user + let add_user_url = format!("{}/rustfs/admin/v3/add-user?accessKey={}", env.url, user_name); + let user_body = serde_json::json!({ + "secretKey": user_secret, + "status": "enabled" + }); + awscurl_put(&add_user_url, &user_body.to_string(), &env.access_key, &env.secret_key).await?; + info!("Created user {}", user_name); + + // 2. Add user to group + let update_members_url = format!("{}/rustfs/admin/v3/update-group-members", env.url); + let add_member_body = serde_json::json!({ + "group": group_name, + "members": [user_name], + "isRemove": false, + "groupStatus": "enabled" + }); + awscurl_put(&update_members_url, &add_member_body.to_string(), &env.access_key, &env.secret_key).await?; + info!("Added {} to group {}", user_name, group_name); + + // 3. Delete the user (backend and cache update so group membership becomes empty) + let remove_user_url = format!("{}/rustfs/admin/v3/remove-user?accessKey={}", env.url, user_name); + awscurl_delete(&remove_user_url, &env.access_key, &env.secret_key).await?; + info!("Deleted user {}", user_name); + + // 4. Deleting the group should succeed (backend has empty members; no stale cache) + let delete_group_url = format!("{}/rustfs/admin/v3/group/{}", env.url, group_name); + awscurl_delete(&delete_group_url, &env.access_key, &env.secret_key).await?; + info!("Deleted group {} after user was removed", group_name); + + Ok(()) +} diff --git a/crates/iam/src/manager.rs b/crates/iam/src/manager.rs index 54750e0e2..c42b88561 100644 --- a/crates/iam/src/manager.rs +++ b/crates/iam/src/manager.rs @@ -1386,11 +1386,9 @@ where let user_group_memberships = self.cache.user_group_memberships.load(); members.iter().for_each(|member| { - if let Some(m) = user_group_memberships.get(member) { - let mut m = m.clone(); - m.insert(group.to_string()); - Cache::add_or_update(&self.cache.user_group_memberships, member, &m, OffsetDateTime::now_utc()); - } + let mut m = user_group_memberships.get(member).cloned().unwrap_or_default(); + m.insert(group.to_string()); + Cache::add_or_update(&self.cache.user_group_memberships, member, &m, OffsetDateTime::now_utc()); }); Ok(OffsetDateTime::now_utc()) @@ -1518,13 +1516,19 @@ where } } - let gi = self - .cache - .groups - .load() - .get(group) - .cloned() - .ok_or(Error::NoSuchGroup(group.to_string()))?; + let gi = if members.is_empty() { + // Reload from backend so we see latest members (e.g. after user was deleted elsewhere) + let mut m = HashMap::new(); + self.api.load_group(group, &mut m).await?; + m.get(group).cloned().ok_or(Error::NoSuchGroup(group.to_string()))? + } else { + self.cache + .groups + .load() + .get(group) + .cloned() + .ok_or(Error::NoSuchGroup(group.to_string()))? + }; if members.is_empty() && !gi.members.is_empty() { return Err(Error::GroupNotEmpty); diff --git a/crates/iam/src/store/object.rs b/crates/iam/src/store/object.rs index 4f6e16119..c79878c1f 100644 --- a/crates/iam/src/store/object.rs +++ b/crates/iam/src/store/object.rs @@ -597,7 +597,7 @@ impl Store for ObjectStore { async fn delete_group_info(&self, name: &str) -> Result<()> { self.delete_iam_config(get_group_info_path(name)).await.map_err(|err| { if is_err_config_not_found(&err) { - Error::NoSuchPolicy + Error::NoSuchGroup(name.to_string()) } else { err } @@ -607,7 +607,7 @@ impl Store for ObjectStore { async fn load_group(&self, name: &str, m: &mut HashMap) -> Result<()> { let u: GroupInfo = self.load_iam_config(get_group_info_path(name)).await.map_err(|err| { if is_err_config_not_found(&err) { - Error::NoSuchPolicy + Error::NoSuchGroup(name.to_string()) } else { err }