diff --git a/crates/policy/src/auth/credentials.rs b/crates/policy/src/auth/credentials.rs index c3d8ee622..93773aca1 100644 --- a/crates/policy/src/auth/credentials.rs +++ b/crates/policy/src/auth/credentials.rs @@ -123,7 +123,7 @@ pub fn create_new_credentials_with_metadata( } if sk.len() < SECRET_KEY_MIN_LEN || sk.len() > SECRET_KEY_MAX_LEN { - return Err(Error::InvalidAccessKeyLength); + return Err(Error::InvalidSecretKeyLength); } if token_secret.is_empty() { @@ -420,7 +420,8 @@ impl TryFrom for Credentials { #[cfg(test)] mod reserved_chars_tests { - use super::contains_reserved_chars; + use super::{contains_reserved_chars, create_new_credentials_with_metadata}; + use std::collections::HashMap; #[test] fn detects_any_reserved_char_not_just_the_substring() { @@ -440,4 +441,13 @@ mod reserved_chars_tests { assert!(!contains_reserved_chars("AKIAIOSFODNN7EXAMPLE")); assert!(!contains_reserved_chars("group-name_1")); } + + #[test] + fn credential_creation_reports_secret_key_length_errors() { + let err = + create_new_credentials_with_metadata("AKIAIOSFODNN7EXAMPLE", "short", &HashMap::new(), "token-secret-for-tests") + .expect_err("short secret key should fail"); + + assert!(matches!(err, crate::error::Error::InvalidSecretKeyLength)); + } } diff --git a/rustfs/src/admin/handlers/iam_error.rs b/rustfs/src/admin/handlers/iam_error.rs index 00f05eec8..0ec3e3244 100644 --- a/rustfs/src/admin/handlers/iam_error.rs +++ b/rustfs/src/admin/handlers/iam_error.rs @@ -23,6 +23,7 @@ pub(crate) fn iam_error_to_s3_error(err: IamError) -> S3Error { | IamError::NoSuchTempAccount(_) | IamError::NoSuchGroup(_) | IamError::NoSuchPolicy => S3ErrorCode::NoSuchResource, + IamError::InvalidAccessKeyLength | IamError::InvalidSecretKeyLength => S3ErrorCode::InvalidArgument, _ => S3ErrorCode::InternalError, }; @@ -54,7 +55,17 @@ mod tests { } #[test] - fn non_not_found_iam_errors_remain_internal_errors() { + fn credential_length_errors_map_to_invalid_argument() { + let errors = [IamError::InvalidAccessKeyLength, IamError::InvalidSecretKeyLength]; + + for err in errors { + let s3_error = iam_error_to_s3_error(err); + assert_eq!(s3_error.code(), &S3ErrorCode::InvalidArgument); + } + } + + #[test] + fn non_validation_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/service_account.rs b/rustfs/src/admin/handlers/service_account.rs index 590730ed4..11029ba3f 100644 --- a/rustfs/src/admin/handlers/service_account.rs +++ b/rustfs/src/admin/handlers/service_account.rs @@ -397,7 +397,12 @@ impl Operation for AddServiceAccount { error = ?e, "admin service account state" ); - s3_error!(InternalError, "create service account failed, e: {:?}", e) + match e { + rustfs_iam::error::Error::InvalidAccessKeyLength | rustfs_iam::error::Error::InvalidSecretKeyLength => { + iam_error_to_s3_error(e) + } + err => s3_error!(InternalError, "create service account failed, e: {:?}", err), + } })?; if let Err(err) = site_replication_iam_change_hook(SRIAMItem { @@ -1562,6 +1567,24 @@ mod tests { assert_eq!(err.message(), Some("service account 'missing' does not exist")); } + #[test] + fn add_service_account_maps_iam_create_errors_to_s3_errors() { + let src = include_str!("service_account.rs"); + let create_start = src + .find("impl Operation for AddServiceAccount") + .expect("AddServiceAccount operation should exist"); + let create_block = &src[create_start..]; + let create_end = create_block + .find("if let Err(err) = site_replication_iam_change_hook") + .expect("site replication hook marker should exist"); + let create_block = &create_block[..create_end]; + + assert!( + create_block.contains("iam_error_to_s3_error(e)"), + "service-account create validation errors must map to client S3 errors" + ); + } + #[test] fn map_temp_account_lookup_error_reports_missing_access_keys_explicitly() { let err = map_temp_account_lookup_error(