From 97b618bc2bdf1959234fdd091e05d9f53eedd28d Mon Sep 17 00:00:00 2001
From: GatewayJ <835269233@qq.com>
Date: Tue, 21 Jul 2026 20:36:25 +0800
Subject: [PATCH] fix(iam): reject cross-identity access key collisions (#5085)

fix(iam): reject service account access key collisions
---
 crates/iam/src/manager.rs              |  6 +-
 crates/iam/src/sys.rs                  | 83 +++++++++++++++++++++++++-
 rustfs/src/admin/handlers/iam_error.rs |  1 +
 3 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/crates/iam/src/manager.rs b/crates/iam/src/manager.rs
index dd8f6ac6f..861f00ae1 100644
--- a/crates/iam/src/manager.rs
+++ b/crates/iam/src/manager.rs
@@ -783,14 +783,10 @@ where
         }
 
         let cache = self.cache.snapshot();
-        let users = Arc::clone(&cache.users);
-        if let Some(x) = users.get(&cred.access_key)
-            && x.credentials.is_service_account()
-        {
+        if cache.users.contains_key(&cred.access_key) || cache.sts_accounts.contains_key(&cred.access_key) {
             return Err(Error::AccessKeyAlreadyExists);
         }
         drop(cache);
-        drop(users);
 
         let u = UserIdentity::new(cred);
 
diff --git a/crates/iam/src/sys.rs b/crates/iam/src/sys.rs
index 34b866997..a46a6312b 100644
--- a/crates/iam/src/sys.rs
+++ b/crates/iam/src/sys.rs
@@ -1429,6 +1429,7 @@ mod tests {
         /// When true, parent user has no groups and no mapped policies (empty `policy_db_get`).
         empty_policies: bool,
         saved_sts_users: Arc<Mutex<HashMap<String, UserIdentity>>>,
+        saved_service_account_count: Arc<Mutex<usize>>,
     }
 
     impl StsTestMockStore {
@@ -1436,8 +1437,16 @@ mod tests {
             Self {
                 empty_policies,
                 saved_sts_users: Arc::new(Mutex::new(HashMap::new())),
+                saved_service_account_count: Arc::new(Mutex::new(0)),
             }
         }
+
+        fn saved_service_account_count(&self) -> usize {
+            *self
+                .saved_service_account_count
+                .lock()
+                .expect("saved_service_account_count mutex poisoned")
+        }
     }
 
     #[async_trait::async_trait]
@@ -1461,10 +1470,16 @@ mod tests {
         async fn save_user_identity(
             &self,
             name: &str,
-            _user_type: UserType,
+            user_type: UserType,
             item: UserIdentity,
             _ttl: Option<usize>,
         ) -> Result<()> {
+            if user_type == UserType::Svc {
+                *self
+                    .saved_service_account_count
+                    .lock()
+                    .expect("saved_service_account_count mutex poisoned") += 1;
+            }
             self.saved_sts_users
                 .lock()
                 .expect("saved_sts_users mutex poisoned")
@@ -1735,6 +1750,72 @@ mod tests {
         assert_eq!(err, Error::AccessKeyAlreadyExists);
     }
 
+    #[tokio::test]
+    async fn service_account_creation_rejects_regular_user_access_key() {
+        ensure_test_global_credentials();
+
+        let iam_sys = test_iam_sys().await;
+        let access_key = "REGULARUSERACCESSKEY1";
+        let user = AddOrUpdateUserReq {
+            secret_key: "regularUserSecret123".to_string(),
+            policy: None,
+            status: rustfs_madmin::AccountStatus::Enabled,
+        };
+
+        iam_sys
+            .store
+            .add_user(access_key, &user)
+            .await
+            .expect("regular user should be created");
+
+        let err = iam_sys
+            .new_service_account("svc-parent-user", None, service_account_opts(access_key, "serviceAccountSecret123"))
+            .await
+            .expect_err("service-account creation must reject a regular-user access key");
+
+        assert_eq!(err, Error::AccessKeyAlreadyExists);
+        assert_eq!(iam_sys.store.api.saved_service_account_count(), 0);
+        let existing = iam_sys.get_user(access_key).await.expect("regular user should remain cached");
+        assert!(!existing.credentials.is_service_account());
+        assert_eq!(existing.credentials.secret_key, user.secret_key);
+    }
+
+    #[tokio::test]
+    async fn service_account_creation_rejects_sts_access_key() {
+        ensure_test_global_credentials();
+
+        let iam_sys = test_iam_sys().await;
+        let access_key = "TEMPORARYSERVICEKEY12";
+        let temp_cred = Credentials {
+            access_key: access_key.to_string(),
+            secret_key: "temporarySecretKey123".to_string(),
+            session_token: "temporary-session-token".to_string(),
+            expiration: Some(OffsetDateTime::now_utc() + time::Duration::hours(1)),
+            status: ACCOUNT_ON.to_string(),
+            parent_user: "sts-parent-user".to_string(),
+            ..Default::default()
+        };
+
+        iam_sys
+            .set_temp_user(access_key, &temp_cred, None)
+            .await
+            .expect("temporary credentials should be created");
+
+        let err = iam_sys
+            .new_service_account("svc-parent-user", None, service_account_opts(access_key, "serviceAccountSecret123"))
+            .await
+            .expect_err("service-account creation must reject a temporary access key");
+
+        assert_eq!(err, Error::AccessKeyAlreadyExists);
+        assert_eq!(iam_sys.store.api.saved_service_account_count(), 0);
+        let existing = iam_sys
+            .get_user(access_key)
+            .await
+            .expect("temporary account should remain cached");
+        assert!(existing.credentials.is_temp());
+        assert_eq!(existing.credentials.secret_key, temp_cred.secret_key);
+    }
+
     #[tokio::test]
     async fn user_creation_rejects_service_account_access_key() {
         ensure_test_global_credentials();
diff --git a/rustfs/src/admin/handlers/iam_error.rs b/rustfs/src/admin/handlers/iam_error.rs
index 3fe3b4394..24790434b 100644
--- a/rustfs/src/admin/handlers/iam_error.rs
+++ b/rustfs/src/admin/handlers/iam_error.rs
@@ -71,6 +71,7 @@ mod tests {
         let s3_error = iam_error_to_s3_error(IamError::AccessKeyAlreadyExists);
 
         assert_eq!(s3_error.code(), &S3ErrorCode::InvalidArgument);
+        assert_eq!(s3_error.status_code(), Some(http::StatusCode::BAD_REQUEST));
         assert_eq!(s3_error.message(), Some("access key is already in use"));
     }