mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-11 15:46:53 +00:00
fix(iam): expand OIDC auth diagnostics (#4281)
* fix(iam): expand OIDC auth diagnostics * fix(iam): accept RFC3339 OIDC timestamps * chore(iam): log OIDC policy mapping diagnostics * chore(iam): log OIDC claim and policy details * chore(iam): lower OIDC diagnostic log verbosity * fix(iam): gate OIDC diagnostics behind debug * chore: update yanked num-bigint lockfile
This commit is contained in:
@@ -38,7 +38,7 @@ use s3s::{Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, s3_error}
|
||||
use serde::de::DeserializeOwned;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use time::OffsetDateTime;
|
||||
use tracing::{error, info, warn};
|
||||
use tracing::{debug, error, warn};
|
||||
use url::Url;
|
||||
|
||||
const LOG_COMPONENT_ADMIN: &str = "admin";
|
||||
@@ -448,17 +448,34 @@ impl Operation for OidcAuthorizeHandler {
|
||||
|
||||
// Optional: redirect_after query parameter (must be a safe relative path)
|
||||
let redirect_after = extract_safe_redirect_after(&req.uri)?;
|
||||
let redirect_after_log = redirect_after.clone();
|
||||
|
||||
let auth_url = oidc_sys
|
||||
.authorize_url(provider_id, &redirect_uri, redirect_after)
|
||||
.await
|
||||
.map_err(|e| S3Error::with_message(S3ErrorCode::InvalidRequest, format!("authorize failed: {e}")))?;
|
||||
.map_err(|e| {
|
||||
error!(
|
||||
event = EVENT_ADMIN_OIDC_STATE,
|
||||
component = LOG_COMPONENT_ADMIN,
|
||||
subsystem = LOG_SUBSYSTEM_OIDC,
|
||||
result = "authorize_url_failed",
|
||||
provider_id = %provider_id,
|
||||
redirect_uri = %redirect_uri,
|
||||
redirect_after = ?redirect_after_log,
|
||||
error = %e,
|
||||
"admin oidc state"
|
||||
);
|
||||
S3Error::with_message(S3ErrorCode::InvalidRequest, format!("authorize failed: {e}"))
|
||||
})?;
|
||||
|
||||
info!(
|
||||
debug!(
|
||||
event = EVENT_ADMIN_OIDC_STATE,
|
||||
component = LOG_COMPONENT_ADMIN,
|
||||
subsystem = LOG_SUBSYSTEM_OIDC,
|
||||
provider_id = %provider_id,
|
||||
redirect_uri = %redirect_uri,
|
||||
redirect_after = ?redirect_after_log,
|
||||
auth_url = %auth_url,
|
||||
state = "authorize_redirect",
|
||||
"admin oidc state"
|
||||
);
|
||||
@@ -490,20 +507,14 @@ impl Operation for OidcCallbackHandler {
|
||||
return Err(s3_error!(InvalidRequest, "invalid provider_id"));
|
||||
}
|
||||
|
||||
// Extract code and state from query parameters
|
||||
let code =
|
||||
extract_query_param(&req.uri, "code").ok_or_else(|| s3_error!(InvalidRequest, "missing 'code' query parameter"))?;
|
||||
let state =
|
||||
extract_query_param(&req.uri, "state").ok_or_else(|| s3_error!(InvalidRequest, "missing 'state' query parameter"))?;
|
||||
|
||||
// Check for error response from IdP
|
||||
if let Some(error) = extract_query_param(&req.uri, "error") {
|
||||
let desc = extract_query_param(&req.uri, "error_description").unwrap_or_default();
|
||||
if let Some((error, desc)) = extract_idp_callback_error(&req.uri) {
|
||||
warn!(
|
||||
event = EVENT_ADMIN_OIDC_STATE,
|
||||
component = LOG_COMPONENT_ADMIN,
|
||||
subsystem = LOG_SUBSYSTEM_OIDC,
|
||||
result = "idp_callback_error",
|
||||
provider_id = %provider_id,
|
||||
error_code = %error,
|
||||
error_description = %desc,
|
||||
"admin oidc state"
|
||||
@@ -514,6 +525,12 @@ impl Operation for OidcCallbackHandler {
|
||||
));
|
||||
}
|
||||
|
||||
// Extract code and state from query parameters
|
||||
let code =
|
||||
extract_query_param(&req.uri, "code").ok_or_else(|| s3_error!(InvalidRequest, "missing 'code' query parameter"))?;
|
||||
let state =
|
||||
extract_query_param(&req.uri, "state").ok_or_else(|| s3_error!(InvalidRequest, "missing 'state' query parameter"))?;
|
||||
|
||||
let oidc_sys = current_oidc_handle().ok_or_else(|| s3_error!(InternalError, "OIDC not initialized"))?;
|
||||
|
||||
let redirect_uri = derive_callback_uri(&req, provider_id)?;
|
||||
@@ -526,13 +543,19 @@ impl Operation for OidcCallbackHandler {
|
||||
component = LOG_COMPONENT_ADMIN,
|
||||
subsystem = LOG_SUBSYSTEM_OIDC,
|
||||
result = "code_exchange_failed",
|
||||
requested_provider_id = %provider_id,
|
||||
redirect_uri = %redirect_uri,
|
||||
code = %code,
|
||||
state = %state,
|
||||
code_len = code.len(),
|
||||
state_len = state.len(),
|
||||
error = %e,
|
||||
"admin oidc state"
|
||||
);
|
||||
S3Error::with_message(S3ErrorCode::AccessDenied, format!("code exchange failed: {e}"))
|
||||
})?;
|
||||
|
||||
info!(
|
||||
debug!(
|
||||
event = EVENT_ADMIN_OIDC_STATE,
|
||||
component = LOG_COMPONENT_ADMIN,
|
||||
subsystem = LOG_SUBSYSTEM_OIDC,
|
||||
@@ -544,13 +567,15 @@ impl Operation for OidcCallbackHandler {
|
||||
// Map claims to policies and groups
|
||||
let (policies, groups) = oidc_sys.map_claims_to_policies(&actual_provider_id, &claims);
|
||||
|
||||
info!(
|
||||
debug!(
|
||||
event = EVENT_ADMIN_OIDC_STATE,
|
||||
component = LOG_COMPONENT_ADMIN,
|
||||
subsystem = LOG_SUBSYSTEM_OIDC,
|
||||
provider_id = %actual_provider_id,
|
||||
policy_count = policies.len(),
|
||||
group_count = groups.len(),
|
||||
policies = ?policies,
|
||||
groups = ?groups,
|
||||
state = "claims_mapped",
|
||||
"admin oidc state"
|
||||
);
|
||||
@@ -674,6 +699,12 @@ fn extract_query_param(uri: &http::Uri, key: &str) -> Option<String> {
|
||||
})
|
||||
}
|
||||
|
||||
fn extract_idp_callback_error(uri: &http::Uri) -> Option<(String, String)> {
|
||||
let error = extract_query_param(uri, "error")?;
|
||||
let desc = extract_query_param(uri, "error_description").unwrap_or_default();
|
||||
Some((error, desc))
|
||||
}
|
||||
|
||||
fn extract_safe_redirect_after(uri: &http::Uri) -> S3Result<Option<String>> {
|
||||
let redirect_after = extract_query_param(uri, "redirect_after");
|
||||
match redirect_after {
|
||||
@@ -1172,6 +1203,18 @@ mod tests {
|
||||
assert_eq!(extract_query_param(&uri, "redirect_after"), Some("/dashboard".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_idp_callback_error_without_code() {
|
||||
let uri: http::Uri = "http://localhost/callback?error=access_denied&error_description=Denied%20by%20IdP&state=xyz789"
|
||||
.parse()
|
||||
.expect("valid callback URI should parse");
|
||||
|
||||
let (error, desc) = extract_idp_callback_error(&uri).expect("IdP callback error should be detected");
|
||||
assert_eq!(error, "access_denied");
|
||||
assert_eq!(desc, "Denied by IdP");
|
||||
assert_eq!(extract_query_param(&uri, "code"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_host_authority_rejects_userinfo() {
|
||||
assert!(parse_host_authority("evil.com@victim.com").is_err());
|
||||
|
||||
@@ -46,7 +46,7 @@ use s3s::{
|
||||
use serde::Deserialize;
|
||||
use serde_json::Value;
|
||||
use serde_urlencoded::from_bytes;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::{BTreeMap, HashMap};
|
||||
use time::{Duration, OffsetDateTime};
|
||||
use tracing::{debug, error, info, warn};
|
||||
|
||||
@@ -131,6 +131,85 @@ fn resolve_oidc_session_identity(claims: &OidcClaims) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
async fn log_oidc_policy_diagnostics(
|
||||
iam_store: &rustfs_iam::sys::IamSys<rustfs_iam::store::object::ObjectStore>,
|
||||
provider_id: &str,
|
||||
parent_user: &str,
|
||||
policies: &[String],
|
||||
groups: &[String],
|
||||
) {
|
||||
if policies.is_empty() {
|
||||
let policy_documents = BTreeMap::<String, Value>::new();
|
||||
let missing_policies = Vec::<String>::new();
|
||||
let combined_policy = Value::Null;
|
||||
debug!(
|
||||
provider_id = %provider_id,
|
||||
parent_user = %parent_user,
|
||||
policy_count = 0,
|
||||
group_count = groups.len(),
|
||||
policies = ?policies,
|
||||
groups = ?groups,
|
||||
policy_documents = ?policy_documents,
|
||||
missing_policies = ?missing_policies,
|
||||
combined_policy = ?combined_policy,
|
||||
"OIDC STS policy diagnostics"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
match iam_store.list_policy_docs("").await {
|
||||
Ok(policy_docs) => {
|
||||
let mut policy_documents = BTreeMap::new();
|
||||
let mut missing_policies = Vec::new();
|
||||
for policy_name in policies {
|
||||
match policy_docs.get(policy_name) {
|
||||
Some(policy_doc) => {
|
||||
let policy_doc_json = serde_json::to_value(policy_doc).unwrap_or_else(|err| {
|
||||
serde_json::json!({
|
||||
"serialization_error": err.to_string(),
|
||||
})
|
||||
});
|
||||
policy_documents.insert(policy_name.clone(), policy_doc_json);
|
||||
}
|
||||
None => missing_policies.push(policy_name.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
let combined_policy = iam_store.get_combined_policy(policies).await;
|
||||
let combined_policy_json = serde_json::to_value(&combined_policy).unwrap_or_else(|err| {
|
||||
serde_json::json!({
|
||||
"serialization_error": err.to_string(),
|
||||
})
|
||||
});
|
||||
|
||||
debug!(
|
||||
provider_id = %provider_id,
|
||||
parent_user = %parent_user,
|
||||
policy_count = policies.len(),
|
||||
group_count = groups.len(),
|
||||
policies = ?policies,
|
||||
groups = ?groups,
|
||||
missing_policies = ?missing_policies,
|
||||
policy_documents = ?policy_documents,
|
||||
combined_policy = ?combined_policy_json,
|
||||
"OIDC STS policy diagnostics"
|
||||
);
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(
|
||||
provider_id = %provider_id,
|
||||
parent_user = %parent_user,
|
||||
policy_count = policies.len(),
|
||||
group_count = groups.len(),
|
||||
policies = ?policies,
|
||||
groups = ?groups,
|
||||
error = %err,
|
||||
"OIDC STS policy diagnostics failed"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn register_admin_auth_route(r: &mut S3Router<AdminOperation>) -> std::io::Result<()> {
|
||||
r.insert(Method::POST, "/", AdminOperation(&AssumeRoleHandle {}))?;
|
||||
|
||||
@@ -347,12 +426,25 @@ async fn handle_assume_role_with_web_identity(body: AssumeRoleRequest) -> S3Resu
|
||||
let (policies, groups) = oidc_sys.map_claims_to_policies(&provider_id, &claims);
|
||||
|
||||
if !has_identity_authorization_context(&policies, &groups) {
|
||||
warn!(
|
||||
provider_id = %provider_id,
|
||||
username = %claims.username,
|
||||
sub = %claims.sub,
|
||||
policy_count = policies.len(),
|
||||
group_count = groups.len(),
|
||||
"AssumeRoleWithWebIdentity has no mapped policies or groups"
|
||||
);
|
||||
return Err(s3_error!(InvalidArgument, "no policies are available for this OIDC token"));
|
||||
}
|
||||
|
||||
info!(
|
||||
"AssumeRoleWithWebIdentity: user='{}', provider='{}', policies={:?}, groups={:?}",
|
||||
claims.username, provider_id, policies, groups
|
||||
debug!(
|
||||
provider_id = %provider_id,
|
||||
username = %claims.username,
|
||||
policy_count = policies.len(),
|
||||
group_count = groups.len(),
|
||||
policies = ?policies,
|
||||
groups = ?groups,
|
||||
"AssumeRoleWithWebIdentity mapped OIDC policies and groups"
|
||||
);
|
||||
|
||||
let mut duration = if body.duration_seconds > 0 {
|
||||
@@ -429,9 +521,19 @@ pub async fn create_oidc_sts_credentials(
|
||||
|
||||
// Set the parent user: prefer username, then email, then sub
|
||||
let parent_user = resolve_oidc_session_identity(claims);
|
||||
info!(
|
||||
"OIDC STS credential: parent_user='{}' (email='{}', username='{}', sub='{}')",
|
||||
parent_user, claims.email, claims.username, claims.sub
|
||||
debug!(
|
||||
provider_id = %provider_id,
|
||||
parent_user = %parent_user,
|
||||
email = %claims.email,
|
||||
username = %claims.username,
|
||||
sub = %claims.sub,
|
||||
policy_count = policies.len(),
|
||||
group_count = groups.len(),
|
||||
policies = ?policies,
|
||||
groups = ?groups,
|
||||
roles_claim_key = ?roles_claim_key,
|
||||
has_session_policy = session_policy.is_some(),
|
||||
"OIDC STS credential claims prepared"
|
||||
);
|
||||
token_claims.insert("parent".to_string(), Value::String(parent_user.clone()));
|
||||
|
||||
@@ -457,6 +559,9 @@ pub async fn create_oidc_sts_credentials(
|
||||
// Store temp user in IAM
|
||||
let iam_store =
|
||||
crate::admin::runtime_sources::current_ready_iam_handle().map_err(|_| s3_error!(InternalError, "IAM not initialized"))?;
|
||||
if tracing::enabled!(tracing::Level::DEBUG) {
|
||||
log_oidc_policy_diagnostics(&iam_store, provider_id, &new_cred.parent_user, policies, groups).await;
|
||||
}
|
||||
|
||||
let updated_at = iam_store
|
||||
.set_temp_user(&new_cred.access_key, &new_cred, None)
|
||||
|
||||
Reference in New Issue
Block a user