From a995ec0315c3451967becb6c7fe7bac011a48519 Mon Sep 17 00:00:00 2001 From: weisd Date: Tue, 28 Apr 2026 13:57:10 +0800 Subject: [PATCH] fix(iam): preserve portable IAM storage and derived auth (#2713) --- crates/iam/src/manager.rs | 23 +++- crates/iam/src/store/object.rs | 52 ++++----- crates/iam/src/sys.rs | 205 +++++++++++++++++++++++++++++++-- 3 files changed, 235 insertions(+), 45 deletions(-) diff --git a/crates/iam/src/manager.rs b/crates/iam/src/manager.rs index 25bf09e9f..c31448355 100644 --- a/crates/iam/src/manager.rs +++ b/crates/iam/src/manager.rs @@ -645,6 +645,20 @@ where } } + let sts_accounts = self.cache.sts_accounts.load(); + for (_, v) in sts_accounts.iter() { + if v.credentials.parent_user == access_key { + user_exists = true; + if v.credentials.is_temp() && !v.credentials.is_service_account() { + let mut u = v.clone(); + u.credentials.secret_key = String::new(); + u.credentials.session_token = String::new(); + + ret.push(u); + } + } + } + if !user_exists { return Err(Error::NoSuchUser(access_key.to_string())); } @@ -653,8 +667,8 @@ where } pub async fn list_sts_accounts(&self, access_key: &str) -> Result> { - let users = self.cache.users.load(); - Ok(users + let sts_accounts = self.cache.sts_accounts.load(); + Ok(sts_accounts .values() .filter_map(|x| { if !access_key.is_empty() @@ -1421,14 +1435,13 @@ where } pub async fn is_temp_user(&self, access_key: &str) -> Result<(bool, String)> { - let users = self.cache.users.load(); - let u = match users.get(access_key) { + let u = match self.get_user(access_key).await { Some(u) => u, None => return Err(Error::NoSuchUser(access_key.to_string())), }; if u.credentials.is_temp() { - Ok((true, u.credentials.parent_user.clone())) + Ok((true, u.credentials.parent_user)) } else { Ok((false, String::new())) } diff --git a/crates/iam/src/store/object.rs b/crates/iam/src/store/object.rs index 2ac778dac..2cf71203b 100644 --- a/crates/iam/src/store/object.rs +++ b/crates/iam/src/store/object.rs @@ -227,20 +227,13 @@ impl ObjectStore { Ok(encrypted) } - fn encrypt_data(data: &[u8]) -> Result> { + fn prepare_data_for_storage(data: &[u8]) -> Result> { if keyring::encrypt_key().is_some() { let encrypted = Self::encrypt_data_with_master_key(data)?; return Ok(encrypted); } - let cred = get_global_action_cred().unwrap_or_default(); - let password = if !cred.access_key.is_empty() && !cred.secret_key.is_empty() { - format!("{}:{}", cred.access_key, cred.secret_key).into_bytes() - } else { - cred.secret_key.into_bytes() - }; - let en = rustfs_crypto::encrypt_stream_io(&password, data)?; - Ok(en) + Ok(data.to_vec()) } fn should_lazy_rewrite(source: DecryptSource) -> bool { @@ -343,8 +336,8 @@ impl ObjectStore { } #[cfg(test)] - fn encrypt_data_for_test(data: &[u8]) -> Result> { - Self::encrypt_data(data) + fn prepare_data_for_storage_for_test(data: &[u8]) -> Result> { + Self::prepare_data_for_storage(data) } async fn load_iamconfig_bytes_with_metadata(&self, path: impl AsRef + Send) -> Result<(Vec, ObjectInfo)> { @@ -656,7 +649,7 @@ impl Store for ObjectStore { #[tracing::instrument(skip(self, item, path))] async fn save_iam_config(&self, item: Item, path: impl AsRef + Send) -> Result<()> { let mut data = serde_json::to_vec(&item)?; - data = Self::encrypt_data(&data)?; + data = Self::prepare_data_for_storage(&data)?; let mut attempts = 0; let max_attempts = 5; @@ -1458,28 +1451,29 @@ mod tests { } #[test] - fn test_encrypt_data_produces_stream_io_format() { - let _ = test_cred(); + #[serial] + fn test_prepare_data_defaults_to_plaintext_without_iam_master_key() { let plain = br#"{"Version":1,"policy":"readonly"}"#; - let encrypted = ObjectStore::encrypt_data_for_test(plain).expect("encrypt should succeed"); - // stream_io header: salt(32) + alg_id(1) + nonce_prefix(8) = 41 bytes - const STREAM_IO_HEADER_LEN: usize = 41; - assert!( - encrypted.len() >= STREAM_IO_HEADER_LEN, - "encrypted should have at least 41-byte stream_io header" + + with_vars( + [ + (keyring::ENV_IAM_MASTER_KEY, None::<&str>), + (keyring::ENV_IAM_MASTER_KEY_OLD_KEYS, None::<&str>), + ], + || { + let stored = ObjectStore::prepare_data_for_storage_for_test(plain).expect("store bytes should build"); + assert_eq!(stored, plain); + + let decrypted = ObjectStore::decrypt_data_with_source(&stored).expect("plaintext should load"); + assert_eq!(plain, decrypted.plain.as_slice()); + assert_eq!(decrypted.source, DecryptSource::Plaintext); + }, ); - assert!( - encrypted[32] == 0x00 || encrypted[32] == 0x01 || encrypted[32] == 0x02, - "alg_id should be 0x00, 0x01, or 0x02" - ); - // Round-trip: encrypt then decrypt - let decrypted = ObjectStore::decrypt_data_with_source(&encrypted).expect("decrypt should succeed"); - assert_eq!(plain, decrypted.plain.as_slice()); } #[test] #[serial] - fn test_encrypt_data_prefers_iam_master_key_roundtrip() { + fn test_prepare_data_uses_iam_master_key_roundtrip() { let _ = test_cred(); let plain = br#"{"Version":1,"policy":"master-key"}"#; let master_key = "iam-master-key-roundtrip"; @@ -1490,7 +1484,7 @@ mod tests { (keyring::ENV_IAM_MASTER_KEY_OLD_KEYS, None), ], || { - let encrypted = ObjectStore::encrypt_data_for_test(plain).expect("encrypt with iam master key"); + let encrypted = ObjectStore::prepare_data_for_storage_for_test(plain).expect("encrypt with iam master key"); let by_master = rustfs_crypto::decrypt_stream_io(master_key.as_bytes(), &encrypted).expect("decrypt via master key"); diff --git a/crates/iam/src/sys.rs b/crates/iam/src/sys.rs index 1e0c8443b..7285bd947 100644 --- a/crates/iam/src/sys.rs +++ b/crates/iam/src/sys.rs @@ -867,16 +867,6 @@ impl IamSys { }; } - let Ok((is_temp, parent_user)) = self.is_temp_user(args.account).await else { - return PreparedIamAuth { - needs_existing_object_tag: false, - mode: PreparedIamMode::Deny, - }; - }; - if is_temp { - return self.prepare_sts_auth(args, &parent_user).await; - } - let Ok((is_svc, parent_user)) = self.is_service_account(args.account).await else { return PreparedIamAuth { needs_existing_object_tag: false, @@ -887,6 +877,16 @@ impl IamSys { return self.prepare_service_account_auth(args, &parent_user).await; } + let Ok((is_temp, parent_user)) = self.is_temp_user(args.account).await else { + return PreparedIamAuth { + needs_existing_object_tag: false, + mode: PreparedIamMode::Deny, + }; + }; + if is_temp { + return self.prepare_sts_auth(args, &parent_user).await; + } + self.prepare_regular_auth(args).await } @@ -1295,7 +1295,7 @@ mod tests { use crate::manager::get_default_policyes; use crate::store::{GroupInfo, MappedPolicy, Store, UserType}; use rustfs_credentials::{Credentials, get_global_action_cred, init_global_action_credentials}; - use rustfs_policy::auth::UserIdentity; + use rustfs_policy::auth::{UserIdentity, get_new_credentials_with_metadata}; use rustfs_policy::policy::Args; use rustfs_policy::policy::action::{Action, AdminAction, S3Action}; use rustfs_policy::policy::policy_uses_existing_object_tag_conditions; @@ -1677,6 +1677,189 @@ mod tests { assert_eq!(claims.get("exp").and_then(|v| v.as_i64()), Some(updated_expiration.unix_timestamp())); } + #[tokio::test] + async fn test_created_access_token_authorizes_with_parent_policy() { + ensure_test_global_credentials(); + + let store = StsTestMockStore { empty_policies: false }; + let cache_manager = IamCache::new(store).await; + let iam_sys = IamSys::new(cache_manager); + + let parent_user = "sts-fallback-test-parent"; + let groups = Some(vec!["testgroup".to_string()]); + let (cred, _) = iam_sys + .new_service_account( + parent_user, + groups.clone(), + NewServiceAccountOpts { + access_key: "ACCESSTOKENTESTUSER".to_string(), + secret_key: "accessTokenTestSecret".to_string(), + ..Default::default() + }, + ) + .await + .expect("access token should be created"); + + let stored = iam_sys + .get_user(&cred.access_key) + .await + .expect("created access token should be cached"); + assert!(stored.credentials.is_service_account()); + assert_eq!(stored.credentials.parent_user, parent_user); + + let claims = stored + .credentials + .claims + .as_ref() + .expect("created access token should have decoded JWT claims"); + assert_eq!(claims.get("parent").and_then(Value::as_str), Some(parent_user)); + assert_eq!( + claims.get(&iam_policy_claim_name_sa()).and_then(Value::as_str), + Some(INHERITED_POLICY_TYPE) + ); + + let (is_service_account, resolved_parent) = iam_sys + .is_service_account(&cred.access_key) + .await + .expect("created access token should be recognized as a service account"); + assert!(is_service_account); + assert_eq!(resolved_parent, parent_user); + + let (redacted, policy) = iam_sys + .get_service_account(&cred.access_key) + .await + .expect("created access token should be readable"); + assert_eq!(redacted.access_key, cred.access_key); + assert_eq!(redacted.parent_user, parent_user); + assert!(redacted.secret_key.is_empty()); + assert!(redacted.session_token.is_empty()); + assert!(policy.is_none()); + + let args = Args { + account: &cred.access_key, + groups: &groups, + action: Action::S3Action(S3Action::ListBucketAction), + bucket: "mybucket", + conditions: &HashMap::new(), + is_owner: false, + object: "", + claims, + deny_only: false, + }; + + let prepared = iam_sys.prepare_auth(&args).await; + assert!( + matches!(prepared.mode, PreparedIamMode::ServiceAccount { .. }), + "created access token must use service-account authorization path" + ); + assert!( + iam_sys.eval_prepared(&prepared, &args).await, + "created access token should be allowed through the parent's group policy" + ); + } + + #[tokio::test] + async fn test_created_sts_credentials_authorize_with_session_token_claims() { + ensure_test_global_credentials(); + + let store = StsTestMockStore { empty_policies: false }; + let cache_manager = IamCache::new(store).await; + let iam_sys = IamSys::new(cache_manager); + + let parent_user = "sts-fallback-test-parent"; + let token_secret = get_global_action_cred() + .expect("global action credentials should be initialized") + .secret_key; + let mut claims = HashMap::new(); + claims.insert("parent".to_string(), Value::String(parent_user.to_string())); + claims.insert( + "exp".to_string(), + Value::Number(serde_json::Number::from( + (OffsetDateTime::now_utc() + time::Duration::hours(1)).unix_timestamp(), + )), + ); + + let mut cred = get_new_credentials_with_metadata(&claims, &token_secret).expect("STS credentials should be created"); + cred.parent_user = parent_user.to_string(); + + iam_sys + .set_temp_user(&cred.access_key, &cred, None) + .await + .expect("STS credentials should be persisted in the temp-user cache"); + + let stored = iam_sys + .get_user(&cred.access_key) + .await + .expect("created STS credentials should be cached"); + assert!(stored.credentials.is_temp()); + assert!(!stored.credentials.is_service_account()); + assert_eq!(stored.credentials.parent_user, parent_user); + + let (is_temp, resolved_parent) = iam_sys + .is_temp_user(&cred.access_key) + .await + .expect("created STS credentials should be recognized as temp"); + assert!(is_temp); + assert_eq!(resolved_parent, parent_user); + + let listed = iam_sys + .list_sts_accounts(parent_user) + .await + .expect("created STS credentials should be listable by parent"); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].access_key, cred.access_key); + assert_eq!(listed[0].parent_user, parent_user); + assert!(listed[0].secret_key.is_empty()); + assert!(listed[0].session_token.is_empty()); + + let temp_accounts = iam_sys + .list_temp_accounts(parent_user) + .await + .expect("created STS credentials should be listable as temp accounts"); + assert_eq!(temp_accounts.len(), 1); + assert_eq!(temp_accounts[0].credentials.access_key, cred.access_key); + assert_eq!(temp_accounts[0].credentials.parent_user, parent_user); + assert!(temp_accounts[0].credentials.secret_key.is_empty()); + assert!(temp_accounts[0].credentials.session_token.is_empty()); + + let (redacted, policy) = iam_sys + .get_temporary_account(&cred.access_key) + .await + .expect("created STS credentials should be readable"); + assert_eq!(redacted.access_key, cred.access_key); + assert_eq!(redacted.parent_user, parent_user); + assert!(redacted.secret_key.is_empty()); + assert!(redacted.session_token.is_empty()); + assert!(policy.is_none()); + + let decoded_claims = get_claims_from_token_with_secret(&cred.session_token, &token_secret) + .expect("created STS session token should decode with the active signing key"); + assert_eq!(decoded_claims.get("parent").and_then(Value::as_str), Some(parent_user)); + + let groups: Option> = None; + let args = Args { + account: &cred.access_key, + groups: &groups, + action: Action::S3Action(S3Action::ListBucketAction), + bucket: "mybucket", + conditions: &HashMap::new(), + is_owner: false, + object: "", + claims: &decoded_claims, + deny_only: false, + }; + + let prepared = iam_sys.prepare_auth(&args).await; + assert!( + matches!(prepared.mode, PreparedIamMode::Sts { .. }), + "created STS credentials must use STS authorization path" + ); + assert!( + iam_sys.eval_prepared(&prepared, &args).await, + "created STS credentials should inherit the parent user's group policy" + ); + } + /// Regression test: temp credentials without groups in args still receive group-attached /// policies via the parent user (groups fallback). Without the fallback, policy_db_get /// would get None for groups and the user would have no group policies, so the action