diff --git a/crates/e2e_test/src/admin_iam_crud_test.rs b/crates/e2e_test/src/admin_iam_crud_test.rs index 4d6754a42..508edf810 100644 --- a/crates/e2e_test/src/admin_iam_crud_test.rs +++ b/crates/e2e_test/src/admin_iam_crud_test.rs @@ -26,7 +26,9 @@ //! Later batches tracked on backlog#1154: config get/set, info, pools status, //! group lifecycle, import/export IAM. -use crate::common::{RustFSTestEnvironment, admin_ok, admin_request, init_logging}; +use crate::common::{ + RustFSTestEnvironment, admin_ok, admin_request, admin_request_with_session_token, build_test_sts_client, init_logging, +}; use aws_sdk_s3::config::{Credentials, Region}; use aws_sdk_s3::primitives::ByteStream; use aws_sdk_s3::{Client, Config}; @@ -87,6 +89,262 @@ fn bucket_rw_policy(bucket: &str) -> String { .to_string() } +async fn create_user_with_service_account_update_policy( + env: &RustFSTestEnvironment, + user: &str, + secret: &str, + policy: &str, +) -> TestResult { + admin_ok( + env, + http::Method::PUT, + &format!("/rustfs/admin/v3/add-user?accessKey={user}"), + Some(serde_json::json!({ "secretKey": secret, "status": "enabled" }).to_string()), + ) + .await?; + admin_ok( + env, + http::Method::PUT, + &format!("/rustfs/admin/v3/add-canned-policy?name={policy}"), + Some( + serde_json::json!({ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": ["admin:UpdateServiceAccount"] + }, + { + "Effect": "Allow", + "Action": ["sts:AssumeRole"], + "Resource": ["arn:aws:s3:::*"] + } + ] + }) + .to_string(), + ), + ) + .await?; + admin_ok( + env, + http::Method::POST, + "/rustfs/admin/v3/idp/builtin/policy/attach", + Some(serde_json::json!({ "policies": [policy], "user": user }).to_string()), + ) + .await?; + Ok(()) +} + +async fn create_service_account_for( + env: &RustFSTestEnvironment, + parent: &str, +) -> Result<(String, String), Box> { + let response = admin_ok( + env, + http::Method::PUT, + "/rustfs/admin/v3/add-service-accounts", + Some(serde_json::json!({ "targetUser": parent }).to_string()), + ) + .await?; + let response: serde_json::Value = serde_json::from_str(&response)?; + let access_key = response["credentials"]["accessKey"] + .as_str() + .ok_or("service account response should contain credentials.accessKey")? + .to_owned(); + let secret_key = response["credentials"]["secretKey"] + .as_str() + .ok_or("service account response should contain credentials.secretKey")? + .to_owned(); + Ok((access_key, secret_key)) +} + +async fn assert_admin_status( + env: &RustFSTestEnvironment, + credentials: (&str, &str, Option<&str>), + path: &str, + body: String, + expected: StatusCode, + context: &str, +) -> TestResult { + let (access_key, secret_key, session_token) = credentials; + let (status, response) = + admin_request_with_session_token(&env.url, http::Method::POST, path, Some(body), access_key, secret_key, session_token) + .await?; + assert_eq!(status, expected, "{context}: got {status}: {response}"); + if expected == StatusCode::FORBIDDEN { + assert!(response.contains("AccessDenied"), "{context}: expected AccessDenied body, got {response}"); + } + Ok(()) +} + +#[tokio::test] +#[serial] +async fn test_update_service_account_enforces_owner_and_parent_scope() -> TestResult { + init_logging(); + + let mut env = RustFSTestEnvironment::new().await?; + env.start_rustfs_server(vec![]).await?; + + let parent = "updateparent"; + let parent_secret = "updateparentsecret"; + let outsider = "updateoutsider"; + let outsider_secret = "updateoutsidersecret"; + let ordinary = "updateordinary"; + let ordinary_secret = "updateordinarysecret"; + create_user_with_service_account_update_policy(&env, parent, parent_secret, "update-parent-policy").await?; + create_user_with_service_account_update_policy(&env, outsider, outsider_secret, "update-outsider-policy").await?; + admin_ok( + &env, + http::Method::POST, + "/rustfs/admin/v3/idp/builtin/policy/attach", + Some(serde_json::json!({ "policies": ["consoleAdmin"], "user": outsider }).to_string()), + ) + .await?; + admin_ok( + &env, + http::Method::PUT, + &format!("/rustfs/admin/v3/add-user?accessKey={ordinary}"), + Some(serde_json::json!({ "secretKey": ordinary_secret, "status": "enabled" }).to_string()), + ) + .await?; + + let (target_access_key, _) = create_service_account_for(&env, parent).await?; + let target_path = format!("/rustfs/admin/v3/update-service-account?accessKey={target_access_key}"); + + assert_admin_status( + &env, + (&env.access_key, &env.secret_key, None), + &target_path, + serde_json::json!({}).to_string(), + StatusCode::NO_CONTENT, + "root no-op update across parents must succeed", + ) + .await?; + + let custom_policy = serde_json::json!({ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:GetObject"], + "Resource": ["arn:aws:s3:::update-scope/*"] + }] + }); + assert_admin_status( + &env, + (&env.access_key, &env.secret_key, None), + &target_path, + serde_json::json!({ "newPolicy": custom_policy }).to_string(), + StatusCode::NO_CONTENT, + "root implied-to-custom update across parents must succeed", + ) + .await?; + + assert_admin_status( + &env, + (parent, parent_secret, None), + &target_path, + serde_json::json!({ "newDescription": "updated by parent" }).to_string(), + StatusCode::NO_CONTENT, + "parent with UpdateServiceAccount may update its own service account", + ) + .await?; + + let takeover = serde_json::json!({ + "newSecretKey": "cross-parent-takeover-secret", + "newDescription": "cross-parent takeover" + }) + .to_string(); + assert_admin_status( + &env, + (ordinary, ordinary_secret, None), + &target_path, + takeover.clone(), + StatusCode::FORBIDDEN, + "ordinary user must not update another parent's service account", + ) + .await?; + assert_admin_status( + &env, + (outsider, outsider_secret, None), + &target_path, + takeover.clone(), + StatusCode::FORBIDDEN, + "non-owner consoleAdmin must not update across parents", + ) + .await?; + + let (derived_access_key, derived_secret_key) = create_service_account_for(&env, outsider).await?; + assert_admin_status( + &env, + (&derived_access_key, &derived_secret_key, None), + &target_path, + takeover.clone(), + StatusCode::FORBIDDEN, + "service-account credential must not update across parents", + ) + .await?; + + let assumed = build_test_sts_client(&env.url, outsider, outsider_secret, None, "e2e-admin-update-service-account") + .assume_role() + .role_arn("arn:aws:iam::123456789012:role/update-service-account") + .role_session_name("update-service-account-scope") + .send() + .await?; + let temporary = assumed + .credentials() + .ok_or("AssumeRole response should contain credentials")?; + assert_admin_status( + &env, + (temporary.access_key_id(), temporary.secret_access_key(), Some(temporary.session_token())), + &target_path, + takeover, + StatusCode::FORBIDDEN, + "temporary credential must not update across parents", + ) + .await?; + + let info = admin_ok( + &env, + http::Method::GET, + &format!("/rustfs/admin/v3/info-service-account?accessKey={target_access_key}"), + None, + ) + .await?; + let info: serde_json::Value = serde_json::from_str(&info)?; + assert_eq!( + info["impliedPolicy"].as_bool(), + Some(false), + "root update must replace the implied policy with a custom policy" + ); + assert!( + info["policy"].as_str().is_some_and(|policy| policy.contains("s3:GetObject")), + "custom policy must round-trip through the handler: {info}" + ); + assert_eq!( + info["description"].as_str(), + Some("updated by parent"), + "denied takeover attempts must not mutate target" + ); + + let (missing_status, missing_body) = admin_request( + &env.url, + http::Method::POST, + "/rustfs/admin/v3/update-service-account?accessKey=missing-service-account", + Some(serde_json::json!({}).to_string()), + &env.access_key, + &env.secret_key, + ) + .await?; + assert_eq!(missing_status, StatusCode::NOT_FOUND, "missing target must fail closed: {missing_body}"); + assert!( + missing_body.contains("NoSuchResource"), + "missing target must preserve the lookup error: {missing_body}" + ); + + env.stop_server(); + Ok(()) +} + /// Full user -> policy -> service-account lifecycle, proving each management /// call takes effect on the data plane, not just that the endpoint answers 200. #[tokio::test] diff --git a/crates/e2e_test/src/common.rs b/crates/e2e_test/src/common.rs index 4ed1c8e53..ff8376998 100644 --- a/crates/e2e_test/src/common.rs +++ b/crates/e2e_test/src/common.rs @@ -137,6 +137,18 @@ pub(crate) async fn signed_s3_request( content_type: Option<&str>, access_key: &str, secret_key: &str, +) -> Result> { + signed_s3_request_with_session_token(method, url, body, content_type, access_key, secret_key, None).await +} + +async fn signed_s3_request_with_session_token( + method: http::Method, + url: &str, + body: Option, + content_type: Option<&str>, + access_key: &str, + secret_key: &str, + session_token: Option<&str>, ) -> Result> { let uri = url.parse::()?; let authority = uri.authority().ok_or("S3 URL missing authority")?.to_string(); @@ -150,7 +162,14 @@ pub(crate) async fn signed_s3_request( } let content_length = i64::try_from(body.as_ref().map_or(0, String::len)).map_err(|_| "S3 request body is too large")?; - let signed = sign_v4(request.body(Body::empty())?, content_length, access_key, secret_key, "", "us-east-1"); + let signed = sign_v4( + request.body(Body::empty())?, + content_length, + access_key, + secret_key, + session_token.unwrap_or_default(), + "us-east-1", + ); let mut request = local_http_client().request(method, url); for (name, value) in signed.headers() { @@ -170,10 +189,23 @@ pub(crate) async fn admin_request( body: Option, access_key: &str, secret_key: &str, +) -> Result<(StatusCode, String), Box> { + admin_request_with_session_token(base_url, method, path_and_query, body, access_key, secret_key, None).await +} + +pub(crate) async fn admin_request_with_session_token( + base_url: &str, + method: http::Method, + path_and_query: &str, + body: Option, + access_key: &str, + secret_key: &str, + session_token: Option<&str>, ) -> Result<(StatusCode, String), Box> { let url = format!("{base_url}{path_and_query}"); let content_type = body.as_ref().map(|_| "application/json"); - let response = signed_s3_request(method, &url, body, content_type, access_key, secret_key).await?; + let response = + signed_s3_request_with_session_token(method, &url, body, content_type, access_key, secret_key, session_token).await?; let status = response.status(); let body = response.text().await?; Ok((status, body)) diff --git a/rustfs/src/admin/handlers/service_account.rs b/rustfs/src/admin/handlers/service_account.rs index 63a11dc61..ac5075068 100644 --- a/rustfs/src/admin/handlers/service_account.rs +++ b/rustfs/src/admin/handlers/service_account.rs @@ -631,7 +631,9 @@ impl Operation for UpdateServiceAccount { .await .map_err(|e| map_service_account_lookup_error(e, "get service account failed"))?; - if !is_service_account_owner_of(&cred, &svc_account.parent_user) { + // The admin action permits updates within a caller's scope; only an + // owner may cross service-account parent boundaries. + if !owner && !is_service_account_owner_of(&cred, &svc_account.parent_user) { return Err(s3_error!(AccessDenied, "access denied")); }