From cc6909b08b439288504f380a1e2b179b2d9b231d Mon Sep 17 00:00:00 2001 From: houseme Date: Mon, 22 Jun 2026 13:54:57 +0800 Subject: [PATCH] fix(iam): verify sts temp-user persistence (#3722) --- crates/iam/src/manager.rs | 262 +++++++++++++++++++++++++++++++++++++- crates/iam/src/sys.rs | 82 +++++++----- 2 files changed, 313 insertions(+), 31 deletions(-) diff --git a/crates/iam/src/manager.rs b/crates/iam/src/manager.rs index a0776e364..5de6cbbad 100644 --- a/crates/iam/src/manager.rs +++ b/crates/iam/src/manager.rs @@ -62,6 +62,11 @@ const IAM_FORMAT_VERSION_1: i32 = 1; const INITIAL_LOAD_RETRY_DELAY: Duration = Duration::from_secs(1); #[cfg(test)] const INITIAL_LOAD_RETRY_DELAY: Duration = Duration::from_millis(1); +#[cfg(not(test))] +const TEMP_USER_PERSISTENCE_VERIFY_RETRY_DELAY: Duration = Duration::from_millis(200); +#[cfg(test)] +const TEMP_USER_PERSISTENCE_VERIFY_RETRY_DELAY: Duration = Duration::from_millis(1); +const TEMP_USER_PERSISTENCE_VERIFY_MAX_RETRIES: usize = 5; #[derive(Serialize, Deserialize)] struct IAMFormat { @@ -114,6 +119,30 @@ impl IamCache where T: Store, { + async fn verify_temp_user_persistence(&self, access_key: &str) -> Result<()> { + for attempt in 0..=TEMP_USER_PERSISTENCE_VERIFY_MAX_RETRIES { + match self.api.load_user_identity(access_key, UserType::Sts).await { + Ok(_) => return Ok(()), + Err(err) + if attempt < TEMP_USER_PERSISTENCE_VERIFY_MAX_RETRIES + && (is_err_no_such_user(&err) || is_err_config_not_found(&err)) => + { + warn!( + access_key, + attempt = attempt + 1, + max_retries = TEMP_USER_PERSISTENCE_VERIFY_MAX_RETRIES, + error = ?err, + "temporary IAM user not yet visible after save; retrying persistence verification" + ); + tokio::time::sleep(TEMP_USER_PERSISTENCE_VERIFY_RETRY_DELAY).await; + } + Err(err) => return Err(err), + } + } + + unreachable!("temp-user persistence verification loop should return on success or terminal error") + } + /// Create a new IAM system instance /// # Arguments /// * `api` - The storage backend implementing the Store trait @@ -1140,6 +1169,7 @@ where self.api .save_user_identity(access_key, UserType::Sts, u.clone(), None) .await?; + self.verify_temp_user_persistence(access_key).await?; let now = self.cache.with_write_lock(|cache| { let now = OffsetDateTime::now_utc(); @@ -2189,7 +2219,13 @@ mod tests { use super::*; use rustfs_policy::policy::{Policy, PolicyDoc}; use serde_json::json; - use std::collections::HashMap; + use std::{ + collections::HashMap, + sync::{ + Arc, Mutex, + atomic::{AtomicUsize, Ordering}, + }, + }; #[derive(Clone)] struct FailingInitialLoadStore; @@ -2317,6 +2353,230 @@ mod tests { } } + #[derive(Clone)] + struct DelayedTempUserVisibilityStore { + saved_user: Arc>>, + load_attempts: Arc, + visible_after_attempt: usize, + } + + impl DelayedTempUserVisibilityStore { + fn new(visible_after_attempt: usize) -> Self { + Self { + saved_user: Arc::new(Mutex::new(None)), + load_attempts: Arc::new(AtomicUsize::new(0)), + visible_after_attempt, + } + } + } + + #[async_trait::async_trait] + impl Store for DelayedTempUserVisibilityStore { + fn has_watcher(&self) -> bool { + false + } + + async fn save_iam_config(&self, _item: Item, _path: impl AsRef + Send) -> Result<()> { + Ok(()) + } + + async fn load_iam_config(&self, _path: impl AsRef + Send) -> Result { + Err(Error::ConfigNotFound) + } + + async fn delete_iam_config(&self, _path: impl AsRef + Send) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn save_user_identity( + &self, + _name: &str, + _user_type: UserType, + item: UserIdentity, + _ttl: Option, + ) -> Result<()> { + *self.saved_user.lock().expect("saved_user mutex poisoned") = Some(item); + Ok(()) + } + + async fn delete_user_identity(&self, _name: &str, _user_type: UserType) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_user_identity(&self, name: &str, _user_type: UserType) -> Result { + let attempt = self.load_attempts.fetch_add(1, Ordering::SeqCst) + 1; + if attempt <= self.visible_after_attempt { + return Err(Error::NoSuchUser(name.to_string())); + } + + self.saved_user + .lock() + .expect("saved_user mutex poisoned") + .clone() + .ok_or_else(|| Error::NoSuchUser(name.to_string())) + } + + async fn load_user(&self, _name: &str, _user_type: UserType, _m: &mut HashMap) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_users(&self, _user_type: UserType, _m: &mut HashMap) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_secret_key(&self, _name: &str, _user_type: UserType) -> Result { + Err(Error::InvalidArgument) + } + + async fn save_group_info(&self, _name: &str, _item: GroupInfo) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn delete_group_info(&self, _name: &str) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_group(&self, _name: &str, _m: &mut HashMap) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_groups(&self, _m: &mut HashMap) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn save_policy_doc(&self, _name: &str, _item: PolicyDoc) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn delete_policy_doc(&self, _name: &str) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_policy(&self, _name: &str) -> Result { + Err(Error::InvalidArgument) + } + + async fn load_policy_doc(&self, _name: &str, _m: &mut HashMap) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_policy_docs(&self, _m: &mut HashMap) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn save_mapped_policy( + &self, + _name: &str, + _user_type: UserType, + _is_group: bool, + _item: MappedPolicy, + _ttl: Option, + ) -> Result<()> { + Ok(()) + } + + async fn delete_mapped_policy(&self, _name: &str, _user_type: UserType, _is_group: bool) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_mapped_policy( + &self, + _name: &str, + _user_type: UserType, + _is_group: bool, + _m: &mut HashMap, + ) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_mapped_policies( + &self, + _user_type: UserType, + _is_group: bool, + _m: &mut HashMap, + ) -> Result<()> { + Err(Error::InvalidArgument) + } + + async fn load_all(&self, _cache: &Cache) -> Result<()> { + Ok(()) + } + } + + fn build_test_temp_credentials() -> Credentials { + Credentials { + access_key: "STSACCESS123".to_string(), + secret_key: "test-secret-key".to_string(), + session_token: "test-session-token".to_string(), + expiration: Some(OffsetDateTime::now_utc() + time::Duration::hours(1)), + status: STATUS_ENABLED.to_string(), + parent_user: "parent-user".to_string(), + groups: Some(vec!["console-admins".to_string()]), + claims: None, + name: None, + description: None, + } + } + + fn build_test_iam_cache(api: T) -> IamCache { + let (sender, _receiver) = mpsc::channel::(1); + IamCache { + cache: Cache::default(), + api, + state: Arc::new(AtomicU8::new(IamState::Ready as u8)), + loading: Arc::new(AtomicBool::new(false)), + roles: HashMap::new(), + send_chan: sender, + last_timestamp: AtomicI64::new(0), + sync_failures: AtomicU64::new(0), + sync_successes: AtomicU64::new(0), + last_sync_duration_millis: AtomicU64::new(0), + } + } + + #[tokio::test] + async fn set_temp_user_retries_until_sts_identity_becomes_visible() { + let store = DelayedTempUserVisibilityStore::new(2); + let cache = build_test_iam_cache(store.clone()); + let cred = build_test_temp_credentials(); + + let updated_at = cache + .set_temp_user(&cred.access_key, &cred, None) + .await + .expect("temp user should succeed once persistence becomes visible"); + + assert!(updated_at <= OffsetDateTime::now_utc()); + assert_eq!(store.load_attempts.load(Ordering::SeqCst), 3); + + let snapshot = cache.cache.snapshot(); + let sts_identity = snapshot + .sts_accounts + .get(&cred.access_key) + .expect("verified temp user should be cached"); + assert_eq!(sts_identity.credentials.parent_user, cred.parent_user); + } + + #[tokio::test] + async fn set_temp_user_fails_when_sts_identity_never_becomes_visible() { + let store = DelayedTempUserVisibilityStore::new(TEMP_USER_PERSISTENCE_VERIFY_MAX_RETRIES + 1); + let cache = build_test_iam_cache(store.clone()); + let cred = build_test_temp_credentials(); + + let err = cache + .set_temp_user(&cred.access_key, &cred, None) + .await + .expect_err("temp user should fail when persistence never becomes visible"); + + assert!(matches!(err, Error::NoSuchUser(user) if user == cred.access_key)); + assert_eq!(store.load_attempts.load(Ordering::SeqCst), TEMP_USER_PERSISTENCE_VERIFY_MAX_RETRIES + 1); + + let snapshot = cache.cache.snapshot(); + assert!( + !snapshot.sts_accounts.contains_key(&cred.access_key), + "cache must not publish a temp user that was not durably visible" + ); + } + #[tokio::test] async fn test_init_keeps_error_state_when_initial_load_fails() { let (sender, receiver) = mpsc::channel::(1); diff --git a/crates/iam/src/sys.rs b/crates/iam/src/sys.rs index e9e104cfb..75f7c89f6 100644 --- a/crates/iam/src/sys.rs +++ b/crates/iam/src/sys.rs @@ -1358,7 +1358,10 @@ mod tests { use rustfs_policy::policy::action::{Action, AdminAction, S3Action}; use rustfs_policy::policy::policy_uses_existing_object_tag_conditions; use serde_json::Value; - use std::collections::{HashMap, HashSet}; + use std::{ + collections::{HashMap, HashSet}, + sync::{Arc, Mutex}, + }; use time::OffsetDateTime; #[test] @@ -1404,6 +1407,16 @@ mod tests { struct StsTestMockStore { /// When true, parent user has no groups and no mapped policies (empty `policy_db_get`). empty_policies: bool, + saved_sts_users: Arc>>, + } + + impl StsTestMockStore { + fn new(empty_policies: bool) -> Self { + Self { + empty_policies, + saved_sts_users: Arc::new(Mutex::new(HashMap::new())), + } + } } #[async_trait::async_trait] @@ -1426,11 +1439,15 @@ mod tests { async fn save_user_identity( &self, - _name: &str, + name: &str, _user_type: UserType, - _item: UserIdentity, + item: UserIdentity, _ttl: Option, ) -> Result<()> { + self.saved_sts_users + .lock() + .expect("saved_sts_users mutex poisoned") + .insert(name.to_string(), item); Ok(()) } @@ -1438,8 +1455,13 @@ mod tests { Err(Error::InvalidArgument) } - async fn load_user_identity(&self, _name: &str, _user_type: UserType) -> Result { - Err(Error::InvalidArgument) + async fn load_user_identity(&self, name: &str, _user_type: UserType) -> Result { + self.saved_sts_users + .lock() + .expect("saved_sts_users mutex poisoned") + .get(name) + .cloned() + .ok_or_else(|| Error::NoSuchUser(name.to_string())) } async fn load_user(&self, name: &str, user_type: UserType, m: &mut HashMap) -> Result<()> { @@ -1647,7 +1669,7 @@ mod tests { async fn test_new_service_account_without_expiration_omits_exp_claim() { ensure_test_global_credentials(); - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -1670,7 +1692,7 @@ mod tests { async fn test_update_service_account_updates_exp_claim() { ensure_test_global_credentials(); - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -1724,7 +1746,7 @@ mod tests { async fn test_update_service_account_adds_exp_claim_to_non_expiring_account() { ensure_test_global_credentials(); - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -1766,7 +1788,7 @@ mod tests { async fn test_site_replicator_update_requires_explicit_internal_allowance() { ensure_test_global_credentials(); - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -1832,7 +1854,7 @@ mod tests { async fn test_created_access_token_authorizes_with_parent_policy() { ensure_test_global_credentials(); - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -1913,7 +1935,7 @@ mod tests { async fn test_created_sts_credentials_authorize_with_session_token_claims() { ensure_test_global_credentials(); - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2017,7 +2039,7 @@ mod tests { /// would be denied. #[tokio::test] async fn test_sts_groups_fallback_temp_creds_receive_parent_group_policies() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2046,7 +2068,7 @@ mod tests { #[tokio::test] async fn test_sts_claim_policy_resolves_custom_canned_policy() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2090,7 +2112,7 @@ mod tests { #[tokio::test] async fn test_sts_claim_policy_ignores_unsafe_and_missing_policy_names() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2137,7 +2159,7 @@ mod tests { #[tokio::test] async fn test_sts_claim_policy_custom_canned_policy_does_not_grant_other_actions() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2181,7 +2203,7 @@ mod tests { #[tokio::test] async fn test_sts_claim_policy_builtin_policy_remains_compatible() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2211,7 +2233,7 @@ mod tests { #[tokio::test] async fn test_sts_claim_policy_missing_policy_denies() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2243,7 +2265,7 @@ mod tests { /// so session policy Deny cannot be bypassed (see PR #2250 review). #[tokio::test] async fn test_sts_deny_only_session_policy_deny_blocks_when_iam_policies_empty() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2283,7 +2305,7 @@ mod tests { #[tokio::test] async fn test_sts_deny_only_session_policy_allow_when_no_deny_on_action() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2327,7 +2349,7 @@ mod tests { /// may miss users on follower nodes. #[tokio::test] async fn test_load_user_notification_populates_user_and_policy_caches() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2348,7 +2370,7 @@ mod tests { #[tokio::test] async fn test_group_notification_populates_new_membership_entry() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2370,7 +2392,7 @@ mod tests { #[tokio::test] async fn test_sts_policy_mapping_notification_updates_sts_policy_cache() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2389,7 +2411,7 @@ mod tests { #[tokio::test] async fn test_missing_user_notification_cleans_related_cache_state() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2455,7 +2477,7 @@ mod tests { #[tokio::test] async fn test_check_key_propagates_cache_miss_load_failure() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2466,7 +2488,7 @@ mod tests { #[tokio::test] async fn test_prepare_auth_eval_matches_prepare_sts_auth_for_parent_policy_fallback() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2494,7 +2516,7 @@ mod tests { #[tokio::test] async fn test_prepare_auth_detects_existing_object_tag_in_session_policy() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); let sts_access_key = "sts-session-tag-test-user"; @@ -2611,7 +2633,7 @@ mod tests { #[tokio::test] async fn test_prepare_auth_detects_existing_object_tag_in_encoded_session_policy() { - let store = StsTestMockStore { empty_policies: true }; + let store = StsTestMockStore::new(true); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); let sts_access_key = "sts-session-tag-encoded-test-user"; @@ -2661,7 +2683,7 @@ mod tests { #[tokio::test] async fn test_prepare_auth_service_account_inherited_ignores_session_policy_tag_hint() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2722,7 +2744,7 @@ mod tests { /// must still be returned. #[tokio::test] async fn test_policy_db_get_skips_nonexistent_groups() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager); @@ -2743,7 +2765,7 @@ mod tests { #[tokio::test] async fn test_info_policy_returns_policy_as_json_object() { - let store = StsTestMockStore { empty_policies: false }; + let store = StsTestMockStore::new(false); let cache_manager = IamCache::new(store).await.unwrap(); let iam_sys = IamSys::new(cache_manager);