From ecf0db9bb77112fce303cb49564fa090b29337e6 Mon Sep 17 00:00:00 2001 From: GatewayJ <835269233@qq.com> Date: Thu, 23 Apr 2026 09:54:43 +0800 Subject: [PATCH] fix(admin): enforce owner check for service account update (#2646) Co-authored-by: GatewayJ <8352692332qq.com> --- rustfs/src/admin/handlers/service_account.rs | 42 ++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/rustfs/src/admin/handlers/service_account.rs b/rustfs/src/admin/handlers/service_account.rs index 6d5818c5a..cdd7d3188 100644 --- a/rustfs/src/admin/handlers/service_account.rs +++ b/rustfs/src/admin/handlers/service_account.rs @@ -79,6 +79,16 @@ fn delete_service_account_success_status(path: &str) -> StatusCode { } } +fn is_service_account_owner_of(caller: &StoredCredentials, target_parent_user: &str) -> bool { + let caller_parent = if caller.parent_user.is_empty() { + caller.access_key.as_str() + } else { + caller.parent_user.as_str() + }; + + caller_parent == target_parent_user +} + fn map_service_account_lookup_error(err: rustfs_iam::error::Error, action: &str) -> S3Error { debug!("{action}, e: {:?}", err); if is_err_no_such_service_account(&err) { @@ -554,6 +564,15 @@ impl Operation for UpdateServiceAccount { return Err(s3_error!(AccessDenied, "access denied")); } + let (svc_account, _) = iam_store + .get_service_account(&access_key) + .await + .map_err(|e| map_service_account_lookup_error(e, "get service account failed"))?; + + if !is_service_account_owner_of(&cred, &svc_account.parent_user) { + return Err(s3_error!(AccessDenied, "access denied")); + } + let new_secret_key = update_req.new_secret_key.clone(); let new_status = update_req.new_status.clone(); let new_name = update_req.new_name.clone(); @@ -1484,4 +1503,27 @@ mod tests { assert!(policy.version.is_empty()); assert!(policy.statements.is_empty()); } + + #[test] + fn update_service_account_requires_requester_parent_match() { + let parent_owner = StoredCredentials { + access_key: "owner-user".to_string(), + parent_user: String::new(), + ..Default::default() + }; + let derived_owner = StoredCredentials { + access_key: "sa-user".to_string(), + parent_user: "owner-user".to_string(), + ..Default::default() + }; + let foreign_user = StoredCredentials { + access_key: "other".to_string(), + parent_user: String::new(), + ..Default::default() + }; + + assert!(is_service_account_owner_of(&parent_owner, "owner-user")); + assert!(is_service_account_owner_of(&derived_owner, "owner-user")); + assert!(!is_service_account_owner_of(&foreign_user, "owner-user")); + } }