fix(iam): user group policy and delete group (fixes #2028) (#2043)

Co-authored-by: GatewayJ <8352692332qq.com>

.gitignore

@@ -2,6 +2,7 @@
 .DS_Store
 .idea
 .vscode
+.cursor
 .direnv/
 /test
 /logs

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<dyn std
 
     Ok(())
 }
+
+/// Test that a user with only group membership (no explicit user policy) gets group policies
+/// and can perform actions allowed by the group (regression test for #2028.1).
+#[tokio::test(flavor = "multi_thread")]
+#[serial]
+#[ignore = "requires awscurl and spawns a real RustFS server"]
+async fn test_user_with_only_group_gets_group_policies() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
+    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<dyn std::error::Error + Send + Sync> {
+        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<dyn std::error::Error + Send + Sync>> {
+    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(())
+}

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);

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<String, GroupInfo>) -> 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
             }