From 4c44bc649a59b1ad6680f4e1954f757df2bc9188 Mon Sep 17 00:00:00 2001
From: GatewayJ <835269233@qq.com>
Date: Wed, 12 Aug 2026 20:44:35 +0800
Subject: [PATCH] fix(admin): clarify invalid group name errors (#5986)

---
 crates/e2e_test/src/group_delete_test.rs | 52 +++++++++++++++++++++++-
 rustfs/src/admin/handlers/group.rs       |  2 +-
 rustfs/src/admin/handlers/iam_error.rs   | 16 ++++++--
 3 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/crates/e2e_test/src/group_delete_test.rs b/crates/e2e_test/src/group_delete_test.rs
index 2fcccb60b..147feb89d 100644
--- a/crates/e2e_test/src/group_delete_test.rs
+++ b/crates/e2e_test/src/group_delete_test.rs
@@ -14,7 +14,7 @@
 
 //! E2E tests for group management (fixes #2028).
 
-use crate::common::{RustFSTestEnvironment, awscurl_delete, awscurl_get, awscurl_put, init_logging};
+use crate::common::{RustFSTestEnvironment, admin_request, 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;
@@ -32,6 +32,56 @@ fn create_user_s3_client(env: &RustFSTestEnvironment, access_key: &str, secret_k
     Client::from_conf(config)
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn update_group_members_rejects_invalid_new_group_names() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
+    init_logging();
+
+    let mut env = RustFSTestEnvironment::new().await?;
+    env.start_rustfs_server(vec![]).await?;
+
+    let invalid_groups = [
+        ("test group", "group name contains whitespace"),
+        ("test=group", "group name contains reserved characters =,"),
+        ("test,group", "group name contains reserved characters =,"),
+    ];
+
+    for (group, expected_message) in invalid_groups {
+        let body = serde_json::json!({
+            "group": group,
+            "members": [],
+            "isRemove": false,
+            "groupStatus": "enabled"
+        })
+        .to_string();
+        let (status, response_body) = admin_request(
+            &env.url,
+            http::Method::PUT,
+            "/rustfs/admin/v3/update-group-members",
+            Some(body),
+            &env.access_key,
+            &env.secret_key,
+        )
+        .await?;
+
+        assert_eq!(
+            status,
+            reqwest::StatusCode::BAD_REQUEST,
+            "invalid group {group:?} must return HTTP 400, body: {response_body}"
+        );
+        assert!(
+            response_body.contains("<Code>InvalidArgument</Code>"),
+            "invalid group {group:?} must return InvalidArgument, body: {response_body}"
+        );
+        assert!(
+            response_body.contains(&format!("<Message>{expected_message}</Message>")),
+            "invalid group {group:?} returned an unexpected message: {response_body}"
+        );
+    }
+
+    env.stop_server();
+    Ok(())
+}
+
 /// Test that deleting a group with members fails, and deleting an empty group succeeds.
 #[tokio::test(flavor = "multi_thread")]
 #[serial]
diff --git a/rustfs/src/admin/handlers/group.rs b/rustfs/src/admin/handlers/group.rs
index 74babfa13..93e440fbb 100644
--- a/rustfs/src/admin/handlers/group.rs
+++ b/rustfs/src/admin/handlers/group.rs
@@ -619,7 +619,7 @@ impl Operation for UpdateGroupMembers {
                 && is_err_no_such_group(&err)
                 && has_space_be(&args.group)
             {
-                return Err(s3_error!(InvalidArgument, "group not found"));
+                return Err(s3_error!(InvalidArgument, "group name contains whitespace"));
             }
 
             iam_store
diff --git a/rustfs/src/admin/handlers/iam_error.rs b/rustfs/src/admin/handlers/iam_error.rs
index 24790434b..ec685f167 100644
--- a/rustfs/src/admin/handlers/iam_error.rs
+++ b/rustfs/src/admin/handlers/iam_error.rs
@@ -23,9 +23,10 @@ pub(crate) fn iam_error_to_s3_error(err: IamError) -> S3Error {
         | IamError::NoSuchTempAccount(_)
         | IamError::NoSuchGroup(_)
         | IamError::NoSuchPolicy => S3ErrorCode::NoSuchResource,
-        IamError::InvalidAccessKeyLength | IamError::InvalidSecretKeyLength | IamError::AccessKeyAlreadyExists => {
-            S3ErrorCode::InvalidArgument
-        }
+        IamError::InvalidAccessKeyLength
+        | IamError::InvalidSecretKeyLength
+        | IamError::AccessKeyAlreadyExists
+        | IamError::GroupNameContainsReservedChars => S3ErrorCode::InvalidArgument,
         _ => S3ErrorCode::InternalError,
     };
 
@@ -75,6 +76,15 @@ mod tests {
         assert_eq!(s3_error.message(), Some("access key is already in use"));
     }
 
+    #[test]
+    fn reserved_group_name_maps_to_invalid_argument() {
+        let s3_error = iam_error_to_s3_error(IamError::GroupNameContainsReservedChars);
+
+        assert_eq!(s3_error.code(), &S3ErrorCode::InvalidArgument);
+        assert_eq!(s3_error.status_code(), Some(http::StatusCode::BAD_REQUEST));
+        assert_eq!(s3_error.message(), Some("group name contains reserved characters =,"));
+    }
+
     #[test]
     fn non_validation_iam_errors_remain_internal_errors() {
         let s3_error = iam_error_to_s3_error(IamError::IamSysNotInitialized);