mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-09 22:59:59 +00:00
fix(iam): propagate cache miss load failures (#2692)
Co-authored-by: GatewayJ <8352692332qq.com> Co-authored-by: loverustfs <hello@rustfs.com> Co-authored-by: houseme <housemecn@gmail.com>
This commit is contained in:
@@ -255,18 +255,30 @@ where
|
|||||||
let mut sts_policy_map = HashMap::new();
|
let mut sts_policy_map = HashMap::new();
|
||||||
let mut policy_docs_map = HashMap::new();
|
let mut policy_docs_map = HashMap::new();
|
||||||
|
|
||||||
let _ = self.api.load_user(access_key, UserType::Svc, &mut users_map).await;
|
match self.api.load_user(access_key, UserType::Svc, &mut users_map).await {
|
||||||
|
Ok(()) => {}
|
||||||
|
Err(err) if is_err_no_such_user(&err) => {}
|
||||||
|
Err(err) => return Err(err),
|
||||||
|
}
|
||||||
|
|
||||||
let parent_user = users_map.get(access_key).map(|svc| svc.credentials.parent_user.clone());
|
let parent_user = users_map.get(access_key).map(|svc| svc.credentials.parent_user.clone());
|
||||||
|
|
||||||
if let Some(parent_user) = parent_user {
|
if let Some(parent_user) = parent_user {
|
||||||
let _ = self.api.load_user(&parent_user, UserType::Reg, &mut users_map).await;
|
match self.api.load_user(&parent_user, UserType::Reg, &mut users_map).await {
|
||||||
|
Ok(()) => {}
|
||||||
|
Err(err) if is_err_no_such_user(&err) => {}
|
||||||
|
Err(err) => return Err(err),
|
||||||
|
}
|
||||||
let _ = self
|
let _ = self
|
||||||
.api
|
.api
|
||||||
.load_mapped_policy(&parent_user, UserType::Reg, false, &mut user_policy_map)
|
.load_mapped_policy(&parent_user, UserType::Reg, false, &mut user_policy_map)
|
||||||
.await;
|
.await;
|
||||||
} else {
|
} else {
|
||||||
let _ = self.api.load_user(access_key, UserType::Reg, &mut users_map).await;
|
match self.api.load_user(access_key, UserType::Reg, &mut users_map).await {
|
||||||
|
Ok(()) => {}
|
||||||
|
Err(err) if is_err_no_such_user(&err) => {}
|
||||||
|
Err(err) => return Err(err),
|
||||||
|
}
|
||||||
if users_map.contains_key(access_key) {
|
if users_map.contains_key(access_key) {
|
||||||
let _ = self
|
let _ = self
|
||||||
.api
|
.api
|
||||||
@@ -274,7 +286,11 @@ where
|
|||||||
.await;
|
.await;
|
||||||
}
|
}
|
||||||
|
|
||||||
let _ = self.api.load_user(access_key, UserType::Sts, &mut sts_users_map).await;
|
match self.api.load_user(access_key, UserType::Sts, &mut sts_users_map).await {
|
||||||
|
Ok(()) => {}
|
||||||
|
Err(err) if is_err_no_such_user(&err) => {}
|
||||||
|
Err(err) => return Err(err),
|
||||||
|
}
|
||||||
|
|
||||||
let has_sts_user = sts_users_map.get(access_key);
|
let has_sts_user = sts_users_map.get(access_key);
|
||||||
|
|
||||||
|
|||||||
+16
-1
@@ -755,7 +755,7 @@ impl<T: Store> IamSys<T> {
|
|||||||
Ok((Some(res), ok))
|
Ok((Some(res), ok))
|
||||||
}
|
}
|
||||||
None => {
|
None => {
|
||||||
let _ = self.store.load_user(access_key).await;
|
self.store.load_user(access_key).await?;
|
||||||
|
|
||||||
if let Some(res) = self.store.get_user(access_key).await {
|
if let Some(res) = self.store.get_user(access_key).await {
|
||||||
let ok = res.credentials.is_valid();
|
let ok = res.credentials.is_valid();
|
||||||
@@ -1372,6 +1372,10 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async fn load_user(&self, name: &str, user_type: UserType, m: &mut HashMap<String, UserIdentity>) -> Result<()> {
|
async fn load_user(&self, name: &str, user_type: UserType, m: &mut HashMap<String, UserIdentity>) -> Result<()> {
|
||||||
|
if user_type == UserType::Reg && name == "load-failure-user" {
|
||||||
|
return Err(Error::Io(std::io::Error::other("load user failed")));
|
||||||
|
}
|
||||||
|
|
||||||
if user_type == UserType::Reg && name == "notify-user" {
|
if user_type == UserType::Reg && name == "notify-user" {
|
||||||
let user = UserIdentity::from(Credentials {
|
let user = UserIdentity::from(Credentials {
|
||||||
access_key: name.to_string(),
|
access_key: name.to_string(),
|
||||||
@@ -1813,6 +1817,17 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_check_key_propagates_cache_miss_load_failure() {
|
||||||
|
let store = StsTestMockStore { empty_policies: false };
|
||||||
|
let cache_manager = IamCache::new(store).await;
|
||||||
|
let iam_sys = IamSys::new(cache_manager);
|
||||||
|
|
||||||
|
let result = iam_sys.check_key("load-failure-user").await;
|
||||||
|
|
||||||
|
assert!(matches!(result, Err(Error::Io(_))));
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_prepare_auth_eval_matches_prepare_sts_auth_for_parent_policy_fallback() {
|
async fn test_prepare_auth_eval_matches_prepare_sts_auth_for_parent_policy_fallback() {
|
||||||
let store = StsTestMockStore { empty_policies: false };
|
let store = StsTestMockStore { empty_policies: false };
|
||||||
|
|||||||
@@ -175,6 +175,7 @@ impl S3Auth for IAMAuth {
|
|||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
warn!("get_secret_key failed: check_key error, access_key: {access_key}, error: {e:?}");
|
warn!("get_secret_key failed: check_key error, access_key: {access_key}, error: {e:?}");
|
||||||
|
return Err(iam_lookup_error_to_s3_error(&e));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
@@ -188,6 +189,10 @@ impl S3Auth for IAMAuth {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn iam_lookup_error_to_s3_error(_err: &IamError) -> S3Error {
|
||||||
|
s3_error!(InternalError, "IAM user lookup failed")
|
||||||
|
}
|
||||||
|
|
||||||
// check_key_valid checks the key is valid or not. return the user's credentials and if the user is the owner.
|
// check_key_valid checks the key is valid or not. return the user's credentials and if the user is the owner.
|
||||||
pub async fn check_key_valid(session_token: &str, access_key: &str) -> S3Result<(Credentials, bool)> {
|
pub async fn check_key_valid(session_token: &str, access_key: &str) -> S3Result<(Credentials, bool)> {
|
||||||
// KEYSTONE INTEGRATION: Check if Keystone credentials are present in task-local storage
|
// KEYSTONE INTEGRATION: Check if Keystone credentials are present in task-local storage
|
||||||
@@ -968,6 +973,14 @@ mod tests {
|
|||||||
assert!(error.message().unwrap_or("").contains("Your account is not signed up"));
|
assert!(error.message().unwrap_or("").contains("Your account is not signed up"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_iam_lookup_error_maps_to_internal_error() {
|
||||||
|
let result = iam_lookup_error_to_s3_error(&IamError::Io(std::io::Error::other("load user failed")));
|
||||||
|
|
||||||
|
assert_eq!(result.code(), &S3ErrorCode::InternalError);
|
||||||
|
assert_eq!(result.message(), Some("IAM user lookup failed"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_check_claims_from_token_empty_token_and_access_key() {
|
fn test_check_claims_from_token_empty_token_and_access_key() {
|
||||||
let mut cred = create_test_credentials();
|
let mut cred = create_test_credentials();
|
||||||
|
|||||||
Reference in New Issue
Block a user