From 37a3cbc497f4bdf64af1a16a93feeedffe136fe3 Mon Sep 17 00:00:00 2001 From: GatewayJ <835269233@qq.com> Date: Sun, 26 Apr 2026 22:13:14 +0800 Subject: [PATCH] fix(admin): map IAM not found errors to 404 (#2685) Co-authored-by: GatewayJ <8352692332qq.com> --- rustfs/src/admin/handlers/group.rs | 15 ++--- rustfs/src/admin/handlers/iam_error.rs | 62 ++++++++++++++++++++ rustfs/src/admin/handlers/mod.rs | 1 + rustfs/src/admin/handlers/policies.rs | 9 +-- rustfs/src/admin/handlers/service_account.rs | 13 ++-- rustfs/src/admin/handlers/user.rs | 6 +- 6 files changed, 85 insertions(+), 21 deletions(-) create mode 100644 rustfs/src/admin/handlers/iam_error.rs diff --git a/rustfs/src/admin/handlers/group.rs b/rustfs/src/admin/handlers/group.rs index 276502085..ac1f5660f 100644 --- a/rustfs/src/admin/handlers/group.rs +++ b/rustfs/src/admin/handlers/group.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::iam_error::iam_error_to_s3_error; use crate::{ admin::{ auth::validate_admin_request, @@ -155,7 +156,7 @@ impl Operation for GetGroup { let g = iam_store.get_group_description(&query.group).await.map_err(|e| { warn!("get group failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })?; let body = serde_json::to_vec(&g).map_err(|e| s3_error!(InternalError, "marshal body failed, e: {:?}", e))?; @@ -222,7 +223,7 @@ impl Operation for DeleteGroup { } _ => { if is_err_no_such_group(&e) { - s3_error!(NoSuchKey, "group '{group}' does not exist") + iam_error_to_s3_error(e) } else { s3_error!(InternalError, "{e}") } @@ -327,11 +328,11 @@ impl Operation for SetGroupStatus { match status { "enabled" => iam_store.set_group_status(&query.group, true).await.map_err(|e| { warn!("enable group failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })?, "disabled" => iam_store.set_group_status(&query.group, false).await.map_err(|e| { warn!("enable group failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })?, _ => { return Err(s3_error!(InvalidArgument, "invalid status")); @@ -437,7 +438,7 @@ impl Operation for UpdateGroupMembers { } Err(e) => { if !is_err_no_such_user(&e) { - return Err(S3Error::with_message(S3ErrorCode::InternalError, e.to_string())); + return Err(iam_error_to_s3_error(e)); } } } @@ -450,7 +451,7 @@ impl Operation for UpdateGroupMembers { .await .map_err(|e| { warn!("remove group members failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })? } else { warn!("add group members"); @@ -467,7 +468,7 @@ impl Operation for UpdateGroupMembers { .await .map_err(|e| { warn!("add group members failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })? }; diff --git a/rustfs/src/admin/handlers/iam_error.rs b/rustfs/src/admin/handlers/iam_error.rs new file mode 100644 index 000000000..7390d1693 --- /dev/null +++ b/rustfs/src/admin/handlers/iam_error.rs @@ -0,0 +1,62 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use rustfs_iam::error::Error as IamError; +use s3s::{S3Error, S3ErrorCode}; + +pub(super) fn iam_error_to_s3_error(err: IamError) -> S3Error { + let code = match &err { + IamError::NoSuchUser(_) + | IamError::NoSuchAccount(_) + | IamError::NoSuchServiceAccount(_) + | IamError::NoSuchTempAccount(_) + | IamError::NoSuchGroup(_) + | IamError::NoSuchPolicy => S3ErrorCode::NoSuchResource, + _ => S3ErrorCode::InternalError, + }; + + let message = err.to_string(); + let mut s3_error = S3Error::with_message(code, message); + s3_error.set_source(Box::new(err)); + s3_error +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn iam_not_found_errors_map_to_not_found_status_class() { + let errors = [ + IamError::NoSuchUser("user".to_string()), + IamError::NoSuchAccount("account".to_string()), + IamError::NoSuchServiceAccount("service".to_string()), + IamError::NoSuchTempAccount("temp".to_string()), + IamError::NoSuchGroup("group".to_string()), + IamError::NoSuchPolicy, + ]; + + for err in errors { + let s3_error = iam_error_to_s3_error(err); + assert_eq!(s3_error.code(), &S3ErrorCode::NoSuchResource); + } + } + + #[test] + fn non_not_found_iam_errors_remain_internal_errors() { + let s3_error = iam_error_to_s3_error(IamError::IamSysNotInitialized); + + assert_eq!(s3_error.code(), &S3ErrorCode::InternalError); + } +} diff --git a/rustfs/src/admin/handlers/mod.rs b/rustfs/src/admin/handlers/mod.rs index 852b44d83..5013a061d 100644 --- a/rustfs/src/admin/handlers/mod.rs +++ b/rustfs/src/admin/handlers/mod.rs @@ -19,6 +19,7 @@ pub mod event; pub mod group; pub mod heal; pub mod health; +mod iam_error; pub mod is_admin; pub mod kms; pub mod kms_dynamic; diff --git a/rustfs/src/admin/handlers/policies.rs b/rustfs/src/admin/handlers/policies.rs index 45d0e5862..1b91da5fd 100644 --- a/rustfs/src/admin/handlers/policies.rs +++ b/rustfs/src/admin/handlers/policies.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::iam_error::iam_error_to_s3_error; use crate::{ admin::{ auth::validate_admin_request, @@ -456,7 +457,7 @@ impl Operation for SetPolicyForUserOrGroup { } else { iam_store.get_group_description(&query.user_or_group).await.map_err(|e| { warn!("get group description failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })?; } @@ -674,7 +675,7 @@ async fn collect_group_policy_mappings( for group in groups { let group_desc = iam_store.get_group_description(&group).await.map_err(|e| { warn!("get group description failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })?; let policies = split_policy_names(&group_desc.policy); if policies.is_empty() { @@ -866,14 +867,14 @@ async fn handle_builtin_policy_association(req: S3Request, is_attach: bool let user_info = iam_store.get_user_info(&assoc_req.user).await.map_err(|e| { warn!("get user info failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })?; (assoc_req.user, false, direct_user_policy_names(&user_info)) } else { let group_desc = iam_store.get_group_description(&assoc_req.group).await.map_err(|e| { warn!("get group description failed, e: {:?}", e); - S3Error::with_message(S3ErrorCode::InternalError, e.to_string()) + iam_error_to_s3_error(e) })?; (assoc_req.group, true, split_policy_names(&group_desc.policy)) diff --git a/rustfs/src/admin/handlers/service_account.rs b/rustfs/src/admin/handlers/service_account.rs index c04e65234..94a53b611 100644 --- a/rustfs/src/admin/handlers/service_account.rs +++ b/rustfs/src/admin/handlers/service_account.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::iam_error::iam_error_to_s3_error; use crate::admin::handlers::site_replication::site_replication_iam_change_hook; use crate::admin::utils::{encode_compatible_admin_payload, has_space_be, is_compat_admin_request, read_compatible_admin_body}; use crate::auth::{constant_time_eq, get_condition_values, get_session_token}; @@ -104,7 +105,7 @@ fn is_service_account_owner_of(caller: &StoredCredentials, 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) { - s3_error!(InvalidRequest, "service account not exist") + iam_error_to_s3_error(err) } else { s3_error!(InternalError, "{action}") } @@ -113,7 +114,7 @@ fn map_service_account_lookup_error(err: rustfs_iam::error::Error, action: &str) fn map_temp_account_lookup_error(err: rustfs_iam::error::Error, action: &str) -> S3Error { debug!("{action}, e: {:?}", err); if is_err_no_such_temp_account(&err) { - s3_error!(InvalidRequest, "access key not exist") + iam_error_to_s3_error(err) } else { s3_error!(InternalError, "{action}") } @@ -1511,8 +1512,8 @@ mod tests { "get service account failed", ); - assert_eq!(*err.code(), S3ErrorCode::InvalidRequest); - assert_eq!(err.message(), Some("service account not exist")); + assert_eq!(*err.code(), S3ErrorCode::NoSuchResource); + assert_eq!(err.message(), Some("service account 'missing' does not exist")); } #[test] @@ -1522,8 +1523,8 @@ mod tests { "get temporary account failed", ); - assert_eq!(*err.code(), S3ErrorCode::InvalidRequest); - assert_eq!(err.message(), Some("access key not exist")); + assert_eq!(*err.code(), S3ErrorCode::NoSuchResource); + assert_eq!(err.message(), Some("temp account 'missing' does not exist")); } #[test] diff --git a/rustfs/src/admin/handlers/user.rs b/rustfs/src/admin/handlers/user.rs index 2f9a2bd7a..18e553744 100644 --- a/rustfs/src/admin/handlers/user.rs +++ b/rustfs/src/admin/handlers/user.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::iam_error::iam_error_to_s3_error; use super::{account_info, group, service_account, user_iam, user_lifecycle, user_policy_binding}; use crate::{ admin::{ @@ -534,10 +535,7 @@ impl Operation for GetUserInfo { ) .await?; - let info = iam_store - .get_user_info(ak) - .await - .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, e.to_string()))?; + let info = iam_store.get_user_info(ak).await.map_err(iam_error_to_s3_error)?; let data = serde_json::to_vec(&info) .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("marshal user err {e}")))?;