From 26065900c00bf44347e6fd1928ca4fc935650e8a Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:29:55 -0700 Subject: [PATCH 01/19] feat(sts): let platform IdP tokens act as accounts that trust them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A token from a platform issuer, GitHub Actions to begin with, now acts at `/.sts` as the account its `RoleArn` names (`arn:aws:iam:::role/FullAccess`), and only if that account trusts the token's issuer and subject (ADR-014). The proxy reads the token's issuer unverified to route it, checks the Role and that `RoleArn` names an account, then verifies the token against the issuer's JWKS with that issuer's own audiences and a required `exp`, since multistore checks `exp` only when the claim is present. Only then does it ask `POST /api/v1/accounts/{account}/trusts/exchanges` with the verified issuer and subject, as the account. A yes is cached for 60 seconds per account, issuer and subject. A no, the route's 403, is not cached, so a trust just added works on the next attempt. The credentials' principal is the account, never the token's subject, and every refusal of the trust reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. Platform issuers are configured in `PLATFORM_ISSUERS`, a JSON object from issuer to the audiences its tokens must carry, so that one issuer's audience never admits another's token (ADR-009); an issuer with no audience is refused. `AUTH_ISSUER` and `AUTH_AUDIENCE` still configure the person issuer, whose tokens act as their own subject as before. Production trusts GitHub with the proxy's origin as the audience, which is how source.coop's workflow snippet mints the token; staging and previews use staging's origin. CI now configures GitHub as a platform issuer, as production does, and the stub answers the trusts route: yes for this repository's workflows on one account, no otherwise. The credentialed write tests name that account. `aws-actions/configure-aws-credentials` still fails after a successful exchange, on `GetCallerIdentity`, until developmentseed/multistore#126 lands. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- .github/workflows/ci.yml | 20 +++-- .github/workflows/staging.yml | 15 ++-- Cargo.lock | 1 + Cargo.toml | 3 + README.md | 11 ++- src/config.rs | 17 ++++- src/lib.rs | 140 +++++++++++++++++++++++++++++++--- src/platform.rs | 89 +++++++++++++++++++++ src/source_api/cache.rs | 56 ++++++++++++++ src/sts.rs | 10 +++ tests/platform.rs | 95 +++++++++++++++++++++++ tests/sts.rs | 19 +++++ tests/stub_api.py | 57 ++++++++++++++ tests/test_federation.py | 20 +++-- tests/test_platform_trust.py | 118 ++++++++++++++++++++++++++++ tests/test_writes.py | 28 ++++--- wrangler.preview.toml | 4 + wrangler.toml | 9 +++ 18 files changed, 660 insertions(+), 52 deletions(-) create mode 100644 src/platform.rs create mode 100644 tests/platform.rs create mode 100644 tests/test_platform_trust.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe7fe6dc..0012e058 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,7 +85,7 @@ jobs: permissions: contents: read # Mint a GitHub OIDC token as the write tests' caller identity — the - # worker verifies it against GitHub's JWKS (AUTH_ISSUER below). + # worker verifies it against GitHub's JWKS (PLATFORM_ISSUERS below). id-token: write steps: - uses: actions/checkout@v4 @@ -106,16 +106,20 @@ jobs: # dependency in this job can't mint federation assertions any AWS role # trusts. Federated end-to-end coverage lives in the deployed-environment # smoke tests (tests/test_federation.py, wired into staging.yml) instead. - # Inbound callers authenticate with GitHub Actions OIDC tokens: - # AUTH_ISSUER points at GitHub and AUTH_AUDIENCE matches the audience - # requested in "Mint caller identity token" below. + # Inbound callers authenticate with GitHub Actions OIDC tokens. GitHub + # is a platform issuer, as in production: PLATFORM_ISSUERS names the + # audience requested in "Mint caller identity token" below, and a + # token acts as an account the stub says trusts this repository. No + # person-issuer token exists in CI; AUTH_ISSUER names an issuer that + # mints nothing, and AUTH_AUDIENCE keeps /.sts from answering 501. run: | { openssl genpkey -algorithm RSA -out /tmp/oidc.pem -pkeyopt rsa_keygen_bits:2048 printf 'OIDC_PROVIDER_KEY="%s"\n' "$(cat /tmp/oidc.pem)" echo "SESSION_TOKEN_KEY=$(openssl rand -base64 32)" - echo "AUTH_ISSUER=https://token.actions.githubusercontent.com" + echo "AUTH_ISSUER=https://auth.example.invalid" echo "AUTH_AUDIENCE=source-data-proxy-ci" + echo 'PLATFORM_ISSUERS={"https://token.actions.githubusercontent.com": ["source-data-proxy-ci"]}' echo "SOURCE_API_URL=http://localhost:9000" } > .dev.vars - name: Mint caller identity token (GitHub OIDC) @@ -144,9 +148,9 @@ jobs: fi echo "::add-mask::$token" echo "CI_WRITE_ID_TOKEN=$token" >> "$GITHUB_ENV" - # A second, validly-signed token whose audience mismatches - # AUTH_AUDIENCE: test_writes.py asserts the /.sts aud gate rejects - # it (signature checks alone would let it through). + # A second, validly-signed token whose audience is not GitHub's in + # PLATFORM_ISSUERS: test_writes.py asserts the /.sts aud gate + # rejects it (signature checks alone would let it through). wrong=$(curl -sSf -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ "$ACTIONS_ID_TOKEN_REQUEST_URL&audience=not-the-data-proxy" | jq -r '.value') if [ -z "$wrong" ] || [ "$wrong" = "null" ]; then diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 835fbc6f..a8e24df2 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -58,14 +58,12 @@ jobs: # A GitHub Actions OIDC token, minted per run — short-lived by design # and never stored, unlike a token parked in a repo secret. # - # Dormant until Source registers GitHub as a valid IdP, so that products - # can accept writes from GitHub Actions. Two things must land first: - # the deployment's AUTH_ISSUER must accept GitHub's issuer (today it is - # a single Ory URL — src/config.rs reads AUTH_ISSUER as one String, - # unlike the comma-separated AUTH_AUDIENCE, so this needs a code change - # too), and the audience Source expects must be set as - # FEDERATION_TEST_AUDIENCE. Until then no token is minted and the - # copy-source authz test skips; the rest of the suite is unaffected. + # Dormant until FEDERATION_TEST_AUDIENCE is set to the staging proxy's + # origin, the audience staging's PLATFORM_ISSUERS accepts for GitHub, + # and FEDERATION_TEST_TRUST_ACCOUNT to a staging service account that + # trusts this repository's workflows (ADR-014): the token acts as that + # account. Until then no token is minted and the copy-source authz + # test skips; the rest of the suite is unaffected. if: vars.FEDERATION_TEST_AUDIENCE != '' run: | set -euo pipefail @@ -88,6 +86,7 @@ jobs: FEDERATION_WRITE_PRODUCT: ${{ vars.FEDERATION_WRITE_PRODUCT }} # Set by the mint step above, and only when it runs. CI_WRITE_ID_TOKEN: ${{ env.CI_WRITE_ID_TOKEN }} + CI_TRUST_ACCOUNT: ${{ vars.FEDERATION_TEST_TRUST_ACCOUNT }} # boto3: the copy-source authz test signs SigV4 through the AWS SDK # rather than hand-rolling requests. run: uvx --with requests --with boto3 pytest tests/test_federation.py -v diff --git a/Cargo.lock b/Cargo.lock index 917f059e..0a1a78b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1891,6 +1891,7 @@ dependencies = [ name = "source-data-proxy" version = "2.3.4" dependencies = [ + "base64", "console_error_panic_hook", "getrandom 0.4.3", "hmac", diff --git a/Cargo.toml b/Cargo.toml index e5f80da3..f687b363 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,6 +57,9 @@ percent-encoding = "2" hmac = "0.12" sha2 = "0.10" +# Reading a platform IdP's token before verifying it (issuer, key id) +base64 = "0.22" + # Tracing tracing = "0.1" diff --git a/README.md b/README.md index 1e431a8b..d5f38c37 100644 --- a/README.md +++ b/README.md @@ -110,8 +110,9 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: | ---------------------------- | --------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | | `SOURCE_API_URL` | `https://source.coop` | Source Cooperative API base URL | | `LOG_LEVEL` | `WARN` | Tracing level (`TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`) | -| `AUTH_ISSUER` | `https://auth.source.coop` | OIDC issuer trusted for `/.sts` token exchange | +| `AUTH_ISSUER` | `https://auth.source.coop` | The person issuer trusted for `/.sts` token exchange; its tokens act as their own subject | | `AUTH_AUDIENCE` | — | Comma-separated OAuth client ID(s) that `/.sts` subject tokens must be issued to (`aud` claim); a token is accepted if it matches any. Unset = `/.sts` token exchange is disabled (returns 501) | +| `PLATFORM_ISSUERS` | — | JSON object from each platform issuer URL to the audiences its tokens must carry, such as `{"https://token.actions.githubusercontent.com": ["https://data.source.coop"]}`. An issuer with no audience is refused. Unset = no platform issuer is trusted | | `OIDC_PROVIDER_ISSUER` | `https://data.source.coop` | Issuer URL for minted JWTs and OIDC discovery | | `OIDC_PROVIDER_KID` | `data-proxy-1` | Key ID for the active signing key | | `OIDC_PROVIDER_KID_PREVIOUS` | — | Key ID for the previous key (during rotation) | @@ -128,7 +129,7 @@ A service account's API key (ADR-013) is an opaque `sck_` secret that source.coo ### Roles -Every exchange at `/.sts`, of an ID token or an API key, names a Role in `RoleArn`, either bare or as the resource of an ARN of any partition and account (`arn:aws:iam::000000000000:role/ReadOnly`), since AWS SDKs insist on an ARN. The Roles are hardcoded (ADR-014): +Every exchange at `/.sts`, of an ID token or an API key, names a Role in `RoleArn`, either bare or as the resource of an ARN of any partition and account (`arn:aws:iam::000000000000:role/ReadOnly`), since AWS SDKs insist on an ARN. The account matters only to a platform token (below). The Roles are hardcoded (ADR-014): | Role | Credentials may | | ------------ | -------------------------------------------------------- | @@ -138,6 +139,12 @@ Every exchange at `/.sts`, of an ID token or an API key, names a Role in `RoleAr Any other name is refused with `MalformedPolicyDocument`, never mapped to a default. A Role only subtracts: its ceiling is sealed into the session token and checked locally before the account's own permissions are looked up (ADR-011), and a request it refuses gets the same `AccessDenied` as any other refusal. +### Platform identity providers + +A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. At `/.sts` it acts as the account in `RoleArn`, `arn:aws:iam:::role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014). The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. A yes is cached for 60 seconds per account, issuer and subject, and the credentials' principal is the account, never the token's subject. Any other answer reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. + +`aws-actions/configure-aws-credentials` fails after the exchange succeeds: it checks the credentials it exports with `GetCallerIdentity`, which the proxy cannot answer until developmentseed/multistore#126 lands. Until then a workflow saves its token to a file and lets an AWS SDK exchange it, with `AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_ROLE_ARN`, `AWS_ENDPOINT_URL_STS=/.sts`, `AWS_ENDPOINT_URL_S3=` and `AWS_REGION`. + ### Secrets **GitHub environment secrets are the source of truth.** The deploy workflow diff --git a/src/config.rs b/src/config.rs index 5b1cf663..cbc1e8ed 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,6 @@ //! Process-wide configuration parsed once from Worker env vars + secrets. +use std::collections::HashMap; use std::sync::OnceLock; use multistore_oidc_provider::jwt::JwtSigner; @@ -100,6 +101,13 @@ fn build_config(env: &Env) -> AppConfig { tracing::warn!("AUTH_AUDIENCE not set: /.sts token exchange is disabled (returns 501)"); } + // Platform identity providers (GitHub Actions, say), each with its own + // audiences. Unset trusts none. + let platform_issuers = env + .var("PLATFORM_ISSUERS") + .map(|v| crate::platform::parse_issuers(&v.to_string())) + .unwrap_or_default(); + // Ceiling for client-requested DurationSeconds on /.sts. Unset → 3600 (1h), // matching multistore's own default so behavior is unchanged until raised. let sts_max_session_duration_secs = match env.var("STS_MAX_SESSION_DURATION_SECS") { @@ -141,6 +149,7 @@ fn build_config(env: &Env) -> AppConfig { session_token_key, auth_issuer, auth_audiences, + platform_issuers, sts_max_session_duration_secs, ip_hash_salt, } @@ -151,13 +160,19 @@ pub struct AppConfig { pub oidc: OidcConfig, /// AES key for sealing/unsealing STS session tokens. pub session_token_key: TokenKey, - /// OIDC issuer URL for the Source Cooperative auth provider (e.g. `https://auth.source.coop`). + /// OIDC issuer URL for the Source Cooperative auth provider (e.g. + /// `https://auth.source.coop`): the person issuer, whose tokens say who the + /// caller is. pub auth_issuer: String, /// OAuth client IDs that subject tokens presented to `/.sts` may be issued /// to (the `aud` claim); a token is accepted if it matches any. Parsed from /// the comma-separated `AUTH_AUDIENCE`. Empty disables `/.sts` entirely /// (returns 501) rather than accepting any audience. pub auth_audiences: Vec, + /// Platform issuers and the audiences each one's tokens must carry, from + /// the JSON object in `PLATFORM_ISSUERS`. A platform token acts as the + /// account `RoleArn` names, if that account trusts it (ADR-014). + pub platform_issuers: HashMap>, /// Ceiling for client-requested STS session length (`DurationSeconds`), /// in seconds. From `STS_MAX_SESSION_DURATION_SECS`; defaults to 3600 (1h). pub sts_max_session_duration_secs: u64, diff --git a/src/lib.rs b/src/lib.rs index e0f58572..51eb9ed3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,7 @@ mod keys; mod location; mod object_path; mod pagination; +mod platform; mod source_api; mod sts; @@ -236,14 +237,19 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result (u16, String) { - let message = if request_id.is_empty() { + build_sts_error_response(&ProxyError::InvalidOidcToken(with_request_id( + message, request_id, + ))) +} + +/// `message` with the request id, if there is one: SDKs show a user the +/// message and nothing else, and the id is what finds the log line. +fn with_request_id(message: &str, request_id: &str) -> String { + if request_id.is_empty() { message.to_string() } else { format!("{message} (request id {request_id})") - }; - build_sts_error_response(&ProxyError::InvalidOidcToken(message)) + } } /// Hash the key, ask source.coop for its standing, and mint for the account @@ -649,14 +661,122 @@ async fn within_rate_limit(env: &Env, client_ip: &str) -> bool { } } -/// An STS-shaped error body for a status `build_sts_error_response` has no -/// variant for. +/// An STS-shaped error body with a code or message `build_sts_error_response` +/// does not produce. fn sts_error_xml(code: &str, message: &str) -> String { format!( "\n{code}{message}" ) } +// ── Platform identity providers ───────────────────────────────────── + +/// The exchange of a platform issuer's token, if this request carries one: +/// `None` for any other token, which the STS route takes. Parameters come from +/// the query string or the form body, never both, as at the STS route. Every +/// refusal of the account's trust reads the same, whether the account does not +/// exist or does not trust the token. +async fn platform_exchange( + config: &AppConfig, + parts: &RequestParts, + api_auth: &ApiAuth, + request_id: &str, +) -> Option<(u16, String)> { + let sts = try_parse_sts_request(parts.query.as_deref()) + .or_else(|| try_parse_sts_request(parts.form_body.as_deref()))? + .ok()?; + let (header, claims) = platform::unverified(&sts.web_identity_token)?; + let issuer = claims.get("iss")?.as_str()?; + let audiences = config.platform_issuers.get(issuer)?; + Some( + match exchange_platform_token( + config, &sts, &header, issuer, audiences, api_auth, request_id, + ) + .await + { + Ok(creds) => build_sts_response(&creds), + // `exchange_platform_token` has logged who asked to act as whom. + Err(ProxyError::AccessDenied) => ( + 403, + sts_error_xml( + "AccessDenied", + &with_request_id( + "Not authorized to perform sts:AssumeRoleWithWebIdentity", + request_id, + ), + ), + ), + Err(e) => { + tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); + build_sts_error_response(&e) + } + }, + ) +} + +/// Verify a platform issuer's token, then mint for the account `RoleArn` +/// names if that account trusts the token's issuer and subject (ADR-014). The +/// credentials act as the account, never as the token's subject. Everything +/// local comes first, so a token that fails it costs the Source API nothing. +async fn exchange_platform_token( + config: &AppConfig, + sts: &multistore_sts::request::StsRequest, + header: &serde_json::Value, + issuer: &str, + audiences: &[String], + api_auth: &ApiAuth, + request_id: &str, +) -> Result { + let role = sts::role( + &sts.role_arn, + issuer.to_string(), + audiences.to_vec(), + config.sts_max_session_duration_secs, + ) + .ok_or_else(|| ProxyError::RoleNotFound(sts.role_arn.clone()))?; + // No angle brackets in the message: the STS error body carries it unescaped. + let account = sts::account(&sts.role_arn).ok_or_else(|| { + ProxyError::InvalidRequest( + "RoleArn must name the account to act as: arn:aws:iam::ACCOUNT:role/ROLE".into(), + ) + })?; + let subject = platform::verify( + &sts.web_identity_token, + header, + issuer, + &role, + &jwks_cache(), + ) + .await?; + source_api::cache::get_or_fetch_trust( + &config.api_base_url, + account, + issuer, + &subject, + api_auth, + request_id, + ) + .await + .map_err(|e| match e { + ProxyError::AccessDenied => { + tracing::warn!(%request_id, %issuer, %subject, %account, "account does not trust the token"); + e + } + // The route answers for any account; a 404 means the API does not + // serve it, which is a deployment mismatch, not a refusal. + ProxyError::BucketNotFound(_) => ProxyError::Internal("trusts route not found".into()), + e => e, + })?; + let creds = keys::credentials_for( + &role, + account, + sts.duration_seconds, + &config.session_token_key, + )?; + tracing::info!(%request_id, %issuer, %subject, %account, role = %role.role_id, "platform token exchanged"); + Ok(creds) +} + // ── CORS ──────────────────────────────────────────────────────────── fn add_cors(resp: web_sys::Response) -> web_sys::Response { diff --git a/src/platform.rs b/src/platform.rs new file mode 100644 index 00000000..27180a8c --- /dev/null +++ b/src/platform.rs @@ -0,0 +1,89 @@ +//! Platform identity providers at `/.sts` (ADR-009, ADR-014): GitHub Actions +//! and the like, whose tokens say which workload is calling but not which +//! account it may act as. The account is the one `RoleArn` names, and only if +//! that account trusts the token's issuer and subject, which the Source API +//! answers. This module is the wasm-free half: which issuers are platform +//! issuers, reading a token before it is verified, and verifying it. + +use std::collections::HashMap; + +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use base64::Engine; +use multistore::error::ProxyError; +use multistore::types::RoleConfig; +use multistore_sts::jwks::{find_key, verify_token}; +use multistore_sts::JwksCache; +use serde_json::Value; + +/// The platform issuers in `PLATFORM_ISSUERS`, a JSON object from each issuer +/// URL to the audiences its tokens must carry. The audiences are per issuer so +/// that one issuer's audience never admits another's token (ADR-009). An +/// issuer with no audience is left out, as the person issuer is disabled +/// without one: a token minted for any other service could be exchanged here. +/// A value that does not parse trusts no platform issuer. +pub fn parse_issuers(json: &str) -> HashMap> { + let issuers: HashMap> = match serde_json::from_str(json) { + Ok(issuers) => issuers, + Err(e) => { + tracing::error!( + "PLATFORM_ISSUERS is not an object of issuer to audiences ({e}); trusting none" + ); + return HashMap::new(); + } + }; + issuers + .into_iter() + .filter(|(issuer, audiences)| { + if audiences.is_empty() { + tracing::error!(%issuer, "platform issuer has no audience; refusing its tokens"); + } + !audiences.is_empty() + }) + .collect() +} + +/// A token's header and claims, read without verifying anything: enough to +/// route it to its issuer and find the key it names. `None` if it is not a +/// JWT, an API key for one. +pub fn unverified(token: &str) -> Option<(Value, Value)> { + let mut segments = token.split('.').map(|segment| { + let json = URL_SAFE_NO_PAD.decode(segment).ok()?; + serde_json::from_slice::(&json).ok() + }); + Some((segments.next()??, segments.next()??)) +} + +/// Verify a platform issuer's token as the STS route verifies the person +/// issuer's (signature against the issuer's published keys, issuer, the +/// audiences `role` requires, `exp` and `nbf`) and return its subject. +pub async fn verify( + token: &str, + header: &Value, + issuer: &str, + role: &RoleConfig, + jwks: &JwksCache, +) -> Result { + let kid = header + .get("kid") + .and_then(Value::as_str) + .ok_or_else(|| ProxyError::InvalidOidcToken("JWT missing kid".into()))?; + let keys = jwks.get_or_fetch(issuer).await?; + let claims = verify_token(token, find_key(&keys, kid)?, issuer, role)?; + subject(&claims).map(str::to_string) +} + +/// The subject of verified `claims`, which must also carry an expiry: +/// multistore checks `exp` only when it is present, and a third-party token +/// with none would be replayable for good (ADR-004). +pub fn subject(claims: &Value) -> Result<&str, ProxyError> { + if claims.get("exp").and_then(Value::as_i64).is_none() { + return Err(ProxyError::InvalidOidcToken( + "token has no exp claim".into(), + )); + } + claims + .get("sub") + .and_then(Value::as_str) + .filter(|sub| !sub.is_empty()) + .ok_or_else(|| ProxyError::InvalidOidcToken("token has no sub claim".into())) +} diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 5cb7743a..0e952257 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -52,6 +52,11 @@ const PERMISSIONS_CACHE_SECS: u32 = 60; // 1 minute /// cached too — an unknown key costs one lookup a minute, not one a request. const KEY_STANDING_CACHE_SECS: u32 = 60; // 1 minute +/// Whether an account trusts a platform token's issuer and subject +/// (`/accounts/{id}/trusts/exchanges`). The permissions TTL, for the same +/// reason: a trust that is removed should stop minting quickly (ADR-014). +const TRUST_CACHE_SECS: u32 = 60; // 1 minute + // ── Public cache functions ───────────────────────────────────────── /// Fetch a single product's metadata, cached for `PRODUCT_CACHE_SECS`. @@ -204,6 +209,57 @@ pub async fn get_or_fetch_key_standing( .await } +/// Whether `account` trusts `issuer`'s `subject` to act as it: `Ok` if so, +/// `AccessDenied` if not, the way a role's own trust policy decides an +/// assume-role call. Asked as the account itself. Only a yes is cached, for +/// `TRUST_CACHE_SECS`: the route says no with a 403, and no 403 is cached, so +/// a trust just added works on the next attempt. +pub async fn get_or_fetch_trust( + api_base_url: &str, + account: &str, + issuer: &str, + subject: &str, + api_auth: &crate::ApiAuth, + request_id: &str, +) -> Result<(), ProxyError> { + let api_url = format!( + "{}/api/v1/accounts/{}/trusts/exchanges", + api_base_url, + utf8_percent_encode(account, PATH_SEGMENT), + ); + // The Cache API keys on URLs: the account is in the path, and the issuer + // and subject vary with it. + let cache_key = format!( + "{api_url}?issuer={}&subject={}", + utf8_percent_encode(issuer, PATH_SEGMENT), + utf8_percent_encode(subject, PATH_SEGMENT), + ); + let body = serde_json::json!({ "issuer": issuer, "subject": subject }).to_string(); + let answer: TrustAnswer = cached_fetch( + &cache_key, + &api_url, + "POST", + Some(&body), + TRUST_CACHE_SECS, + api_auth, + request_id, + ApiCaller::Account(account), + ) + .await?; + if answer.trusted { + Ok(()) + } else { + Err(ProxyError::AccessDenied) + } +} + +/// The trusts route's answer. Its status already says yes (200) or no (403); +/// the body is read too, so that a 200 saying no mints nothing. +#[derive(serde::Deserialize)] +struct TrustAnswer { + trusted: bool, +} + // ── Internal helpers ────────────────────────────────────────────── /// Build a cache key that includes the caller's identity so that diff --git a/src/sts.rs b/src/sts.rs index 58e895b3..dc85ca2a 100644 --- a/src/sts.rs +++ b/src/sts.rs @@ -100,6 +100,16 @@ fn role_name(role_arn: &str) -> Option<&str> { role_arn.splitn(6, ':').nth(5)?.strip_prefix("role/") } +/// The account segment of an ARN-form `role_arn` +/// (`arn:aws:iam:::role/`): the account a platform IdP's token +/// asks to act as (ADR-014). `None` for a bare name or an empty account. +pub(crate) fn account(role_arn: &str) -> Option<&str> { + match role_arn.splitn(6, ':').collect::>()[..] { + ["arn", _, _, _, account, _] if !account.is_empty() => Some(account), + _ => None, + } +} + impl CredentialRegistry for StsCredentialRegistry { async fn get_credential( &self, diff --git a/tests/platform.rs b/tests/platform.rs new file mode 100644 index 00000000..c1c3f305 --- /dev/null +++ b/tests/platform.rs @@ -0,0 +1,95 @@ +//! Native unit tests for the wasm-free half of platform-IdP exchanges +//! (`platform`), included via `#[path]` like `tests/keys.rs`. Verification +//! against a real issuer's keys and the trust lookup run in the worker and are +//! covered by `tests/test_platform_trust.py`. + +#[path = "../src/platform.rs"] +mod platform; + +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use base64::Engine; +use serde_json::json; + +const GITHUB: &str = "https://token.actions.githubusercontent.com"; + +// ── configuration ────────────────────────────────────────────────── + +#[test] +fn each_issuer_keeps_its_own_audiences() { + let issuers = platform::parse_issuers( + r#"{"https://token.actions.githubusercontent.com": ["https://data.source.coop"], + "https://gitlab.com": ["a", "b"]}"#, + ); + assert_eq!(issuers[GITHUB], ["https://data.source.coop"]); + assert_eq!(issuers["https://gitlab.com"], ["a", "b"]); +} + +#[test] +fn an_issuer_without_an_audience_is_not_trusted() { + let issuers = platform::parse_issuers(r#"{"https://token.actions.githubusercontent.com": []}"#); + assert!(issuers.is_empty()); +} + +#[test] +fn a_value_that_does_not_parse_trusts_no_issuer() { + for value in [ + "", + GITHUB, + r#"["https://token.actions.githubusercontent.com"]"#, + ] { + assert!(platform::parse_issuers(value).is_empty(), "{value}"); + } +} + +// ── reading a token before verifying it ──────────────────────────── + +fn segment(value: serde_json::Value) -> String { + URL_SAFE_NO_PAD.encode(value.to_string()) +} + +#[test] +fn a_jwt_is_read_without_verifying_it() { + let token = format!( + "{}.{}.not-a-signature", + segment(json!({"alg": "RS256", "kid": "k1"})), + segment(json!({"iss": GITHUB, "sub": "repo:o/r:ref:refs/heads/main"})), + ); + let (header, claims) = platform::unverified(&token).unwrap(); + assert_eq!(header["kid"], "k1"); + assert_eq!(claims["iss"], GITHUB); +} + +#[test] +fn anything_else_is_not_read() { + let claims_not_json = format!("{}.not-json.sig", segment(json!({"alg": "RS256"}))); + for token in [ + "", + "not-a-jwt", + "sck_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + claims_not_json.as_str(), + ] { + assert!(platform::unverified(token).is_none(), "{token}"); + } +} + +// ── claims a platform token must carry ───────────────────────────── + +#[test] +fn a_verified_token_names_its_subject() { + let claims = json!({"sub": "repo:o/r:ref:refs/heads/main", "exp": 1_900_000_000}); + assert_eq!( + platform::subject(&claims).unwrap(), + "repo:o/r:ref:refs/heads/main" + ); +} + +#[test] +fn a_token_without_an_expiry_or_a_subject_is_refused() { + for claims in [ + json!({"sub": "repo:o/r:ref:refs/heads/main"}), + json!({"exp": 1_900_000_000}), + json!({"sub": "", "exp": 1_900_000_000}), + ] { + assert!(platform::subject(&claims).is_err(), "{claims}"); + } +} diff --git a/tests/sts.rs b/tests/sts.rs index 7077edaa..1dab3f71 100644 --- a/tests/sts.rs +++ b/tests/sts.rs @@ -66,3 +66,22 @@ fn unknown_names_are_refused_not_defaulted() { assert_eq!(named(arn), None, "{arn}"); } } + +#[test] +fn the_account_is_the_arns_account_segment() { + assert_eq!( + sts::account("arn:aws:iam::acme--nightly-sync:role/FullAccess"), + Some("acme--nightly-sync") + ); + assert_eq!( + sts::account("arn:aws:iam::000000000000:role/_default"), + Some("000000000000") + ); + for role_arn in [ + "FullAccess", + "arn:aws:iam:::role/FullAccess", + "arn:aws:iam::acme", + ] { + assert_eq!(sts::account(role_arn), None, "{role_arn}"); + } +} diff --git a/tests/stub_api.py b/tests/stub_api.py index 664b83c8..6b362e19 100644 --- a/tests/stub_api.py +++ b/tests/stub_api.py @@ -21,12 +21,15 @@ SOURCE_API_URL in .dev.vars. """ +import base64 import hashlib import json import os +import re import zlib from http.server import BaseHTTPRequestHandler, HTTPServer from pathlib import Path +from urllib.parse import unquote PORT = 9000 @@ -152,9 +155,37 @@ def _hash(key): KEY_EXCHANGE_COUNTS = {} +# ── Account trusts ───────────────────────────────────────────────── +# Whether an account trusts a platform token's issuer and subject, at POST +# /api/v1/accounts/{account}/trusts/exchanges (ADR-014). The proxy asks as the +# account itself. Only TRUST_ACCOUNT trusts anyone: GitHub Actions workflows +# in this repository, whatever event minted the token. A counter per account +# lets test_platform_trust.py prove the proxy caches a yes. +TRUST_ACCOUNT = "ci-tests--github-ci" +TRUSTED_ISSUER = "https://token.actions.githubusercontent.com" +TRUSTED_SUBJECT_PREFIX = "repo:source-cooperative/data.source.coop:" +TRUST_EXCHANGE_COUNTS = {} + +# Who the proxy said it was asking as, per product path, so a test can check +# which principal a session carries: see test_platform_trust.py. +PRODUCT_LOOKUP_SUBJECTS = {} + + +def _bearer_subject(authorization): + """The `sub` of the proxy's assertion, unverified: the stub has no key.""" + try: + payload = authorization.removeprefix("Bearer ").split(".")[1] + return json.loads(base64.urlsafe_b64decode(payload + "=" * (-len(payload) % 4)))["sub"] + except (IndexError, ValueError, KeyError): + return None + + class Handler(BaseHTTPRequestHandler): def do_POST(self): path = self.path.split("?")[0] + trust = re.fullmatch(r"/api/v1/accounts/([^/]+)/trusts/exchanges", path) + if trust: + return self._trust_exchange(unquote(trust.group(1))) if path != "/api/v1/service-account-keys/exchanges": return self._send(404, b"{}") # The proxy authenticates as itself; the stub cannot verify the @@ -170,11 +201,37 @@ def do_POST(self): status, body = KEY_STANDINGS.get(key_hash, (200, {"active": False})) self._send(status, json.dumps(body).encode()) + def _trust_exchange(self, account): + # Anyone but the account itself is refused, as the real route does. + if _bearer_subject(self.headers.get("Authorization", "")) != account: + return self._send(401, b'{"error": "Unauthorized"}') + length = int(self.headers.get("content-length") or 0) + try: + body = json.loads(self.rfile.read(length)) + issuer, subject = body["issuer"], body["subject"] + except (ValueError, KeyError, TypeError): + return self._send(400, b"{}") + TRUST_EXCHANGE_COUNTS[account] = TRUST_EXCHANGE_COUNTS.get(account, 0) + 1 + trusted = ( + account == TRUST_ACCOUNT + and issuer == TRUSTED_ISSUER + and subject.startswith(TRUSTED_SUBJECT_PREFIX) + ) + self._send(200 if trusted else 403, json.dumps({"trusted": trusted}).encode()) + def do_GET(self): path = self.path.split("?")[0] # Test-only: how many times each key's standing was asked for. if path == "/_stub/key-exchange-counts": return self._send(200, json.dumps(KEY_EXCHANGE_COUNTS).encode()) + # Test-only: how many times each account's trust was asked for. + if path == "/_stub/trust-exchange-counts": + return self._send(200, json.dumps(TRUST_EXCHANGE_COUNTS).encode()) + # Test-only: the subject each product was last looked up as. + if path == "/_stub/product-lookup-subjects": + return self._send(200, json.dumps(PRODUCT_LOOKUP_SUBJECTS).encode()) + if path.startswith("/api/v1/products/") and self.headers.get("Authorization"): + PRODUCT_LOOKUP_SUBJECTS[path] = _bearer_subject(self.headers["Authorization"]) if path == f"/api/v1/products/{WRITE_ACCOUNT}/{ERR_500_PRODUCT}": return self._send(500, b"{}") if path == f"/api/v1/products/{WRITE_ACCOUNT}/{ERR_BAD_JSON_PRODUCT}": diff --git a/tests/test_federation.py b/tests/test_federation.py index fff5ad26..9d2b5d45 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -31,17 +31,15 @@ OIDC token, minted per run by staging.yml (short- lived by design, never stored) once FEDERATION_TEST_AUDIENCE is set. - - Dormant until Source registers GitHub as a valid - IdP so products can accept writes from GitHub - Actions. That needs the deployment's AUTH_ISSUER to - accept GitHub's issuer — today it is a single Ory - URL, and ``src/config.rs`` reads AUTH_ISSUER as one - String (unlike the comma-separated AUTH_AUDIENCE), - so it is a code change as well as config — and the - audience Source expects to be set as - FEDERATION_TEST_AUDIENCE. Until then this test - skips. The caller must also hold write on + CI_TRUST_ACCOUNT the account that token acts as: a service account + that trusts this repository's workflows (ADR-014). + + Dormant until FEDERATION_TEST_AUDIENCE is set to + the staging proxy's origin, the audience its + PLATFORM_ISSUERS accepts for GitHub, and a staging + service account that trusts this repository is set + as FEDERATION_TEST_TRUST_ACCOUNT. Until then this + test skips. That account must also hold write on FEDERATION_WRITE_PRODUCT """ diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py new file mode 100644 index 00000000..9974965d --- /dev/null +++ b/tests/test_platform_trust.py @@ -0,0 +1,118 @@ +"""Platform-IdP tokens at /.sts (ADR-014), against the stub Source API. + +CI trusts GitHub Actions as a platform issuer (PLATFORM_ISSUERS in ci.yml). A +verified GitHub token acts as the account its RoleArn names, and only if the +stub's trusts route says that account trusts the token's issuer and subject: +TRUST_ACCOUNT trusts this repository's workflows, and no other account trusts +anything. The tests that need a real token run where CI mints one (see +test_writes.py); the others pin what is refused before any trust lookup. +""" + +import base64 +import json +import uuid +import xml.etree.ElementTree as ET + +import pytest +import requests + +from stub_api import TRUST_ACCOUNT, TRUSTED_ISSUER, WRITE_ACCOUNT +from test_writes import ID_TOKEN, PROXY_URL, needs_token + +STUB_URL = "http://localhost:9000" +# The worker takes its request id from `cf-ray`, which `wrangler dev` does not +# set; the tests supply one. +RAY = "ci-ray-trust" + + +def exchange(token, role_arn): + params = {"Action": "AssumeRoleWithWebIdentity", "RoleArn": role_arn, "WebIdentityToken": token} + return requests.post(f"{PROXY_URL}/.sts", data=params, headers={"cf-ray": RAY}) + + +def as_account(account): + return f"arn:aws:iam::{account}:role/FullAccess" + + +def sts_fields(resp): + return {el.tag.rpartition("}")[2]: el.text for el in ET.fromstring(resp.text).iter()} + + +def trust_lookups(account): + return requests.get(f"{STUB_URL}/_stub/trust-exchange-counts").json().get(account, 0) + + +def forged(): + """A token that says it is GitHub's, signed by no one.""" + + def segment(value): + return base64.urlsafe_b64encode(json.dumps(value).encode()).rstrip(b"=").decode() + + claims = { + "iss": TRUSTED_ISSUER, + "sub": "repo:source-cooperative/data.source.coop:ref:refs/heads/main", + "aud": "source-data-proxy-ci", + "exp": 4_000_000_000, + } + return ".".join([segment({"alg": "RS256", "kid": "forged"}), segment(claims), segment("x")]) + + +def test_a_platform_token_must_name_the_account_it_acts_as(): + resp = exchange(forged(), "arn:aws:iam:::role/FullAccess") + assert resp.status_code == 400 + assert "RoleArn must name the account" in sts_fields(resp)["Message"] + + +def test_a_forged_token_is_refused_before_any_trust_lookup(): + before = trust_lookups(TRUST_ACCOUNT) + resp = exchange(forged(), as_account(TRUST_ACCOUNT)) + assert resp.status_code == 400 + assert sts_fields(resp)["Code"] == "InvalidIdentityToken" + assert trust_lookups(TRUST_ACCOUNT) == before + + +@needs_token +def test_a_trusted_workflow_gets_credentials_that_act_as_the_account(): + """The session's principal is the account, not the token's subject: the + stub records who the proxy asked about a product as.""" + import boto3 + from botocore.config import Config + from botocore.exceptions import ClientError + + resp = exchange(ID_TOKEN, as_account(TRUST_ACCOUNT)) + assert resp.status_code == 200, resp.text[:300] + fields = sts_fields(resp) + client = boto3.client( + "s3", + endpoint_url=PROXY_URL, + aws_access_key_id=fields["AccessKeyId"], + aws_secret_access_key=fields["SecretAccessKey"], + aws_session_token=fields["SessionToken"], + region_name="us-east-1", + config=Config(s3={"addressing_style": "path"}), + ) + # A product the stub has never heard of, so the lookup is not cached. + product = f"principal-probe-{uuid.uuid4().hex}" + with pytest.raises(ClientError): + client.get_object(Bucket=WRITE_ACCOUNT, Key=f"{product}/x") + subjects = requests.get(f"{STUB_URL}/_stub/product-lookup-subjects").json() + assert subjects[f"/api/v1/products/{WRITE_ACCOUNT}/{product}"] == TRUST_ACCOUNT + + +@needs_token +def test_an_account_that_does_not_trust_the_workflow_refuses_it(): + resp = exchange(ID_TOKEN, as_account("ci-tests--someone-else")) + assert resp.status_code == 403 + fields = sts_fields(resp) + assert fields["Code"] == "AccessDenied" + assert fields["Message"] == ( + f"Not authorized to perform sts:AssumeRoleWithWebIdentity (request id {RAY})" + ) + + +@needs_token +def test_a_trusted_answer_is_cached(): + exchange(ID_TOKEN, as_account(TRUST_ACCOUNT)) + before = trust_lookups(TRUST_ACCOUNT) + assert exchange(ID_TOKEN, as_account(TRUST_ACCOUNT)).status_code == 200 + assert trust_lookups(TRUST_ACCOUNT) == before, "second exchange within the TTL asked the API again" diff --git a/tests/test_writes.py b/tests/test_writes.py index 0122ec3f..5d5da408 100644 --- a/tests/test_writes.py +++ b/tests/test_writes.py @@ -3,15 +3,15 @@ Data requests to the proxy are SigV4-only (Bearer JWTs are rejected), so an authenticated write follows the real client flow end-to-end: - 1. Obtain an OIDC identity token whose `aud` is in the worker's AUTH_AUDIENCE. - In CI this is a GitHub Actions OIDC token (AUTH_ISSUER = - https://token.actions.githubusercontent.com); the proxy verifies it via - OIDC discovery against GitHub's JWKS. - 2. Exchange it at POST /.sts (AssumeRoleWithWebIdentity, RoleArn=_default) - for temporary credentials whose SessionToken is sealed under - SESSION_TOKEN_KEY. + 1. Obtain a GitHub Actions OIDC token with an audience the worker accepts + for GitHub, a platform issuer (PLATFORM_ISSUERS in ci.yml). The proxy + verifies it via OIDC discovery against GitHub's JWKS. + 2. Exchange it at POST /.sts (AssumeRoleWithWebIdentity) with a RoleArn + naming TRUST_ACCOUNT, which the stub says trusts this repository's + workflows, for temporary credentials whose SessionToken is sealed under + SESSION_TOKEN_KEY (ADR-014). 3. SigV4-sign S3 requests with those credentials; the proxy unseals the - token, verifies the signature, and recovers the subject (the JWT's `sub`). + token, verifies the signature, and recovers the principal: the account. Two tiers, so the suite degrades gracefully: @@ -33,11 +33,15 @@ import pytest import requests +from stub_api import TRUST_ACCOUNT as STUB_TRUST_ACCOUNT from stub_api import WRITE_ACCOUNT, WRITE_PRODUCT PROXY_URL = os.environ.get("PROXY_URL", "http://localhost:8787") ID_TOKEN = os.environ.get("CI_WRITE_ID_TOKEN") WRONG_AUD_TOKEN = os.environ.get("CI_WRONG_AUDIENCE_TOKEN") +# The account the caller's token acts as: the stub's, or against a deployed +# proxy (staging.yml) a service account there that trusts this repository. +TRUST_ACCOUNT = os.environ.get("CI_TRUST_ACCOUNT") or STUB_TRUST_ACCOUNT # When CI declares a token must exist (same-repo runs export CI_EXPECT_OIDC), # a missing token means the mint->env plumbing broke: run the tests and fail @@ -60,7 +64,7 @@ def sts_exchange(token, *, form_body=False): request body, with no query string — instead of in the query string.""" params = { "Action": "AssumeRoleWithWebIdentity", - "RoleArn": "_default", + "RoleArn": f"arn:aws:iam::{TRUST_ACCOUNT}:role/FullAccess", "WebIdentityToken": token, } if form_body: @@ -274,9 +278,9 @@ def test_sts_rejects_tampered_signature(): @needs_wrong_aud_token def test_sts_rejects_wrong_audience(): - """A validly-signed token whose aud isn't in AUTH_AUDIENCE must be - rejected — this is the gate that keeps other GitHub OIDC consumers' - tokens from minting credentials here.""" + """A validly-signed token whose aud isn't one PLATFORM_ISSUERS lists for + GitHub must be rejected — this is the gate that keeps other GitHub OIDC + consumers' tokens from minting credentials here.""" # Presence assert, not just the skipif: with the token missing, # sts_exchange(None) sends no WebIdentityToken and the 4xx assertion # below would pass vacuously — testing nothing. diff --git a/wrangler.preview.toml b/wrangler.preview.toml index b5ae42fb..e5ddb9df 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -21,6 +21,10 @@ OIDC_PROVIDER_KID = "data-proxy-1" # AUTH_AUDIENCE must be set or /.sts fail-closes with 501. AUTH_ISSUER = "https://auth.staging.source.coop" AUTH_AUDIENCE = "1123dfa8-469f-44fe-b9f4-9b76f06fd325,a79c9537-be78-454a-9ea1-b96a1be811cc" # staging frontend + source-coop-cli client_ids +# Staging's platform issuers too: a GitHub Actions workflow calling a preview +# mints its token for the staging proxy's origin, since a preview's own +# hostname differs per PR. +PLATFORM_ISSUERS = '{"https://token.actions.githubusercontent.com": ["https://data.staging.source.coop"]}' [build] command = "cargo install -q worker-build@0.7.5 && worker-build --release" diff --git a/wrangler.toml b/wrangler.toml index da445edf..6c45eb4e 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -21,6 +21,11 @@ AUTH_ISSUER = "https://auth.source.coop" LOG_LEVEL = "WARN" OIDC_PROVIDER_ISSUER = "https://data.source.coop" OIDC_PROVIDER_KID = "data-proxy-1" +# Platform identity providers, each with the audiences its tokens must carry. +# A platform token acts as the account RoleArn names, if that account trusts +# its issuer and subject (ADR-014). GitHub Actions workflows mint their token +# for the proxy's own origin, as source.coop's workflow snippet does. +PLATFORM_ISSUERS = '{"https://token.actions.githubusercontent.com": ["https://data.source.coop"]}' SOURCE_API_URL = "https://source.coop" # Ceiling for client-requested /.sts DurationSeconds (seconds). 43200 = 12h, # 86400 = 24h. Unset → 3600 (1h). Clients must request DurationSeconds to use @@ -41,6 +46,9 @@ STS_MAX_SESSION_DURATION_SECS = "43200" # AUTH_AUDIENCE - comma-separated OAuth client_id(s) that /.sts subject tokens # must be issued to (the `aud` claim). A token is accepted if # it matches any. Unset = /.sts is disabled (returns 501). +# Optional vars: +# PLATFORM_ISSUERS - JSON object from platform issuer URL to the audiences its +# tokens must carry. Unset = no platform issuer is trusted. # TODO: Set different sampling rates for prod vs staging [observability] enabled = true @@ -88,6 +96,7 @@ AUTH_AUDIENCE = "1123dfa8-469f-44fe-b9f4-9b76f06fd325,a79c9537-be78-454a-9ea1-b9 AUTH_ISSUER = "https://auth.staging.source.coop" OIDC_PROVIDER_ISSUER = "https://data.staging.source.coop" OIDC_PROVIDER_KID = "data-proxy-1" +PLATFORM_ISSUERS = '{"https://token.actions.githubusercontent.com": ["https://data.staging.source.coop"]}' SOURCE_API_URL = "https://staging.source.coop" STS_MAX_SESSION_DURATION_SECS = "43200" # 12h ceiling for client-requested /.sts DurationSeconds From d7bf1da56ce011ccb6623913e7efb4f8ccd8ea7d Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:32:32 -0700 Subject: [PATCH 02/19] docs(adrs): record platform issuers and their trust path ADR-009's migration planned to add platform issuers to `_default` and warned that a CI token would then act as its subject's whole account. ADR-014's trust path replaces that: a platform token acts only as an account that trusts its issuer and subject. ADR-009 gains a note saying so, and its status now reads implemented in part. ADR-004 now notes platform issuers under its trust model, and its `exp` warning says platform tokens must carry the claim, which the proxy checks itself. ADR-001's `source_identity` now says it is the account an API key or a trusted platform token names, never a platform token's subject. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- adrs/001-s3-credentials.md | 4 ++-- adrs/004-sts.md | 7 +++++-- adrs/009-platform-idps.md | 5 ++++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/adrs/001-s3-credentials.md b/adrs/001-s3-credentials.md index 0078274c..dbe8702c 100644 --- a/adrs/001-s3-credentials.md +++ b/adrs/001-s3-credentials.md @@ -58,7 +58,7 @@ The sealed payload carries: | `secret_access_key` | The signing secret, recovered by unsealing | | `expiration` | Enforced at unseal time; an expired token fails closed | | `assumed_role_id` | The Role assumed at exchange time: `_default`, `FullAccess` or `ReadOnly` (ADR-004) | -| `source_identity` | The original OIDC `sub` — the caller's Ory identity | +| `source_identity` | Who the credentials act as: an Ory ID token's `sub`, or the account an API key or a trusted platform token names (ADR-013, ADR-014) | | `allowed_scopes` | The Role's ceiling, sealed at mint time: empty for `FullAccess` and `_default`, reads of every product for `ReadOnly` (see below) | | `session_token` | A discarded random placeholder. The credential set is sealed *before* this field is overwritten with the sealed blob, so the value inside the envelope is not the token itself | @@ -67,7 +67,7 @@ Key properties of this design: - **Verification is fully stateless.** The proxy decrypts the token on each request and recovers the `SecretAccessKey` directly. No database lookup, no key derivation, and no asymmetric verification on the request hot path — which matters on Workers, where in-memory state does not persist across invocations. - **The token is opaque to the caller.** Unlike a JWT, a client cannot read the sealed payload. Scope and identity metadata are not disclosed to whoever holds the credential. - **`allowed_scopes` is enforced by the bucket registry, not by multistore.** multistore's own consumer, `multistore::auth::authorize`, has no call site in the pinned crate; the gateway delegates authorization to the registry instead (ADR-005), which checks the ceiling before any lookup (ADR-011, #236). The registry reads an empty vec as **no ceiling**, the reverse of `authorize`, where empty means deny-all — which is also why the registry overrides `authorize_key` rather than inheriting the default. The only non-empty ceiling is `ReadOnly`'s: every product (`*`), read actions only. -- **`source_identity` preserves the original subject**, which is what the proxy presents to the policy store (see ADR-005). +- **`source_identity` is the principal**, which is what the proxy presents to the policy store (see ADR-005): the original subject for an Ory ID token, never a platform token's subject. - **Authenticated encryption.** GCM provides integrity as well as confidentiality: a tampered token fails to decrypt rather than decoding into attacker-chosen values. ### SigV4 Verification Flow diff --git a/adrs/004-sts.md b/adrs/004-sts.md index 66767963..1a6dbe4a 100644 --- a/adrs/004-sts.md +++ b/adrs/004-sts.md @@ -5,7 +5,7 @@ **RFC:** RFC-001 §7 **Depends on:** ADR-001 **Implementation:** `src/sts.rs`, `src/lib.rs`, `src/config.rs`; `source.coop:src/lib/actions/proxy-credentials.ts` -**Implemented by:** #116 (initial `/.sts` exchange), #163 (multiple accepted audiences), #165 (configurable max session duration), #185 (ARN-shaped `_default` alias), #196 (form-encoded POST bodies, wiring [multistore#112](https://github.com/developmentseed/multistore/pull/112)), #236 (`FullAccess` and `ReadOnly` Roles) · source.coop#283 (OIDC auth), source.coop#391 (in-browser uploads via the proxy), source.coop#402 (mid-upload credential refresh) +**Implemented by:** #116 (initial `/.sts` exchange), #163 (multiple accepted audiences), #165 (configurable max session duration), #185 (ARN-shaped `_default` alias), #196 (form-encoded POST bodies, wiring [multistore#112](https://github.com/developmentseed/multistore/pull/112)), #236 (`FullAccess` and `ReadOnly` Roles), #237 (platform issuers) · source.coop#283 (OIDC auth), source.coop#391 (in-browser uploads via the proxy), source.coop#402 (mid-upload credential refresh) --- @@ -82,6 +82,9 @@ A single built-in Role, `_default`, is served from a hardcoded registry: **Issuer.** `AUTH_ISSUER` names the single trusted OIDC issuer: Source Cooperative's Ory-based auth system (`https://auth.source.coop`, or the staging equivalent). A token from any other issuer is rejected before any network call. +> [!NOTE] +> Platform issuers (ADR-009) are now trusted alongside it, each with its own audiences in `PLATFORM_ISSUERS`. Their tokens take a separate path ahead of the STS route and act as the account `RoleArn` names, if that account trusts the token's issuer and subject (ADR-014, #237). + **Audience.** `AUTH_AUDIENCE` is a comma-separated allowlist of OAuth client IDs; a token is accepted if its `aud` matches any entry. Production lists the web frontend and `source-coop-cli`. **The audience restriction is load-bearing and the endpoint fails closed without it.** Without it, an ID token that a user granted to *any* third-party OAuth client registered with the issuer could be exchanged for that user's proxy credentials. When `AUTH_AUDIENCE` is unset the STS route is never mounted and `/.sts` returns `501 NotImplemented`, rather than being served unrestricted. @@ -115,7 +118,7 @@ flowchart TD ``` > [!WARNING] -> **`exp` is only checked when the claim is present.** Upstream, both time claims are guarded by `if let Some(..)`, so a token carrying no `exp` is accepted and never expires. A missing `aud`, by contrast, is fail-closed. This is harmless with a single trusted issuer that always sets `exp` (Ory does), but it becomes a real exposure the moment ADR-009 admits issuers we do not control — it should be fixed upstream before then. +> **`exp` is only checked when the claim is present.** Upstream, both time claims are guarded by `if let Some(..)`, so a token carrying no `exp` is accepted and never expires. A missing `aud`, by contrast, is fail-closed. This is harmless with a single trusted issuer that always sets `exp` (Ory does), but it becomes a real exposure the moment ADR-009 admits issuers we do not control — it should be fixed upstream before then. #237 admits them and requires `exp` on their tokens itself; the upstream fix, developmentseed/multistore#146, is not yet in a release. Steps 2 and 3 reject **before any network call**: the issuer is matched and the algorithm pinned prior to fetching JWKS, so a token from an untrusted issuer costs nothing to refuse. diff --git a/adrs/009-platform-idps.md b/adrs/009-platform-idps.md index e76ec13a..ece4d4d4 100644 --- a/adrs/009-platform-idps.md +++ b/adrs/009-platform-idps.md @@ -1,6 +1,6 @@ # ADR-009: Multi-Issuer Platform Identity Providers -**Status:** Proposed — not implemented +**Status:** Proposed — implemented in part (#237: platform issuers with per-issuer audiences, whose tokens act as an account that trusts them, per ADR-014) **Date:** 2026-08-09 **RFC:** RFC-001 §7 **Depends on:** ADR-004 @@ -61,6 +61,9 @@ ADR-004's rule — an issuer with no audience restriction disables exchange rath ### Migration +> [!NOTE] +> Superseded in part by ADR-014, as implemented in #237. Platform issuers are not added to `_default`: `AUTH_ISSUER` stays the one person issuer, and `PLATFORM_ISSUERS` maps each platform issuer to its own audiences (step 2). A platform token acts only as the account `RoleArn` names, and only if that account trusts the token's issuer and subject, so the note below no longer applies: a CI token reaches the memberships of one account that trusts it, not those of whoever its subject might map to. + 1. Parse `AUTH_ISSUER` as a comma-separated list, mirroring `AUTH_AUDIENCE`; a single value remains valid, so existing deployments are unaffected. 2. Move the issuer→audience mapping into a structured variable, since a flat pair of lists cannot express per-issuer requirements. 3. Populate `trusted_oidc_issuers` on the `_default` Role from the parsed list. From e96907c38cbe7888cb211b53029f83527b7b0c80 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:38:28 -0700 Subject: [PATCH 03/19] docs(sts): say exactly which trust answers are cached `get_or_fetch_trust` said only a yes is cached. `cached_fetch` caches every 200, so a `200 {"trusted": false}`, which the route does not send today, would be cached too, and still refused. The comments now say that the route's yes is a 200 cached like any other and its no is a 403 that never is. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- src/source_api/cache.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 0e952257..2410b9fa 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -211,9 +211,9 @@ pub async fn get_or_fetch_key_standing( /// Whether `account` trusts `issuer`'s `subject` to act as it: `Ok` if so, /// `AccessDenied` if not, the way a role's own trust policy decides an -/// assume-role call. Asked as the account itself. Only a yes is cached, for -/// `TRUST_CACHE_SECS`: the route says no with a 403, and no 403 is cached, so -/// a trust just added works on the next attempt. +/// assume-role call. Asked as the account itself. The route says yes with a +/// 200, cached for `TRUST_CACHE_SECS` like every 200, and no with a 403, which +/// is never cached, so a trust just added works on the next attempt. pub async fn get_or_fetch_trust( api_base_url: &str, account: &str, @@ -254,7 +254,8 @@ pub async fn get_or_fetch_trust( } /// The trusts route's answer. Its status already says yes (200) or no (403); -/// the body is read too, so that a 200 saying no mints nothing. +/// the body is read too, so that a 200 saying no, cached like any 200, still +/// mints nothing. #[derive(serde::Deserialize)] struct TrustAnswer { trusted: bool, From aded1acdfdb3cf07adc0aac0d1b8b0b0716b440f Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Fri, 25 Sep 2026 14:51:47 -0700 Subject: [PATCH 04/19] fix(sts): name only service accounts and bound replayed platform tokens A platform token's `RoleArn` must now name a service account, `{owner}--{name}` as source.coop's `SERVICE_ACCOUNT_ID_REGEX` defines it (82 characters at most). Anything else is refused as an untrusting account is, before the token is verified and before the proxy signs anything as it. The minted principal is that segment as given, and source.coop resolves a principal as an Ory identity first; an Ory id fits the person and organisation handle grammar, so without this, a trust on such an account would let platform credentials act as a person. source.coop writes trusts only to service accounts today, and ADR-014 makes trusts a service account's, so this is defense in depth. Anyone can mint a GitHub token for the proxy's audience in their own workflow, and a refusal was not cached, so replaying one valid token cost a trusts lookup per request. Two bounds follow. The per-address rate limit from the API-key exchange now also covers platform exchanges, after everything local and before the trusts lookup; the binding is renamed `STS_EXCHANGE_LIMIT`, since it now bounds every exchange that costs a Source API call, and stays at 100 a minute. And a refusal from the trusts route is cached for 10 seconds under its own key, so a replay flood from many addresses costs about one lookup per 10 seconds per account, issuer and subject, while a trust just added still works within seconds. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd --- README.md | 4 +- src/lib.rs | 127 ++++++++++++++++++++++------------- src/source_api/cache.rs | 37 +++++++--- src/sts.rs | 21 ++++++ tests/sts.rs | 41 +++++++++++ tests/test_platform_trust.py | 23 ++++++- wrangler.preview.toml | 5 +- wrangler.toml | 13 ++-- 8 files changed, 203 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index d5f38c37..6c4a0227 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: | Binding | Kind | Description | | -------------------- | ----------- | -------------------------------------------------------------------------------------------------------------------------------------------- | -| `KEY_EXCHANGE_LIMIT` | `ratelimit` | Per-client-IP limit on API-key exchanges at `/.sts` (ADR-013). Declared under `[[ratelimits]]` in every `wrangler*.toml`; a deployment without it logs an error and exchanges without a limit | +| `STS_EXCHANGE_LIMIT` | `ratelimit` | Per-client-IP limit on `/.sts` exchanges that cost a Source API call: API keys (ADR-013) and platform tokens (ADR-014). Declared under `[[ratelimits]]` in every `wrangler*.toml`; a deployment without it logs an error and exchanges without a limit | ### API keys @@ -141,7 +141,7 @@ Any other name is refused with `MalformedPolicyDocument`, never mapped to a defa ### Platform identity providers -A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. At `/.sts` it acts as the account in `RoleArn`, `arn:aws:iam:::role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014). The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. A yes is cached for 60 seconds per account, issuer and subject, and the credentials' principal is the account, never the token's subject. Any other answer reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. +A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. At `/.sts` it acts as the service account in `RoleArn`, `arn:aws:iam::--:role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014); an account that is not a service account is refused before anything else. The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then, within `STS_EXCHANGE_LIMIT`, asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. Per account, issuer and subject, a yes is cached for 60 seconds and a no for 10, and the credentials' principal is the account, never the token's subject. Every refusal reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. `aws-actions/configure-aws-credentials` fails after the exchange succeeds: it checks the credentials it exports with `GetCallerIdentity`, which the proxy cannot answer until developmentseed/multistore#126 lands. Until then a workflow saves its token to a file and lets an AWS SDK exchange it, with `AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_ROLE_ARN`, `AWS_ENDPOINT_URL_STS=/.sts`, `AWS_ENDPOINT_URL_S3=` and `AWS_REGION`. diff --git a/src/lib.rs b/src/lib.rs index 51eb9ed3..ca66d763 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -247,7 +247,8 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result web_sys::Response { response } -/// The rate-limiter binding for API-key exchanges, keyed by client IP. -const KEY_EXCHANGE_LIMIT: &str = "KEY_EXCHANGE_LIMIT"; +/// The rate-limiter binding for `/.sts` exchanges that cost a Source API call, +/// of API keys and platform tokens alike, keyed by client IP. +const STS_EXCHANGE_LIMIT: &str = "STS_EXCHANGE_LIMIT"; /// The API-key exchange, if this request is one: `None` when it is not an /// `AssumeRoleWithWebIdentity` carrying an `sck_` key, so the STS route takes @@ -527,13 +529,7 @@ async fn api_key_exchange( let client_ip = header_str(&parts.headers, "cf-connecting-ip"); if !within_rate_limit(env, client_ip).await { tracing::warn!(%request_id, reason = "rate_limited", "API key exchange refused"); - return Some(( - 429, - sts_error_xml( - "Throttling", - "too many API key exchanges from this address; retry later", - ), - )); + return Some(throttled()); } Some( @@ -644,7 +640,7 @@ async fn within_rate_limit(env: &Env, client_ip: &str) -> bool { } else { client_ip }; - match env.rate_limiter(KEY_EXCHANGE_LIMIT) { + match env.rate_limiter(STS_EXCHANGE_LIMIT) { Ok(limiter) => match limiter.limit(key.to_string()).await { Ok(outcome) => outcome.success, Err(e) => { @@ -654,13 +650,24 @@ async fn within_rate_limit(env: &Env, client_ip: &str) -> bool { }, Err(_) => { tracing::error!( - "{KEY_EXCHANGE_LIMIT} binding is not configured; API-key exchanges are unlimited" + "{STS_EXCHANGE_LIMIT} binding is not configured; exchanges are unlimited" ); true } } } +/// The answer to an exchange over `STS_EXCHANGE_LIMIT`, which SDKs back off on. +fn throttled() -> (u16, String) { + ( + 429, + sts_error_xml( + "Throttling", + "too many exchanges from this address; retry later", + ), + ) +} + /// An STS-shaped error body with a code or message `build_sts_error_response` /// does not produce. fn sts_error_xml(code: &str, message: &str) -> String { @@ -673,12 +680,11 @@ fn sts_error_xml(code: &str, message: &str) -> String { /// The exchange of a platform issuer's token, if this request carries one: /// `None` for any other token, which the STS route takes. Parameters come from -/// the query string or the form body, never both, as at the STS route. Every -/// refusal of the account's trust reads the same, whether the account does not -/// exist or does not trust the token. +/// the query string or the form body, never both, as at the STS route. async fn platform_exchange( config: &AppConfig, parts: &RequestParts, + env: &Env, api_auth: &ApiAuth, request_id: &str, ) -> Option<(u16, String)> { @@ -688,58 +694,70 @@ async fn platform_exchange( let (header, claims) = platform::unverified(&sts.web_identity_token)?; let issuer = claims.get("iss")?.as_str()?; let audiences = config.platform_issuers.get(issuer)?; + let client_ip = header_str(&parts.headers, "cf-connecting-ip"); Some( match exchange_platform_token( - config, &sts, &header, issuer, audiences, api_auth, request_id, + config, env, client_ip, &sts, &header, issuer, audiences, api_auth, request_id, ) .await { Ok(creds) => build_sts_response(&creds), - // `exchange_platform_token` has logged who asked to act as whom. - Err(ProxyError::AccessDenied) => ( - 403, - sts_error_xml( - "AccessDenied", - &with_request_id( - "Not authorized to perform sts:AssumeRoleWithWebIdentity", - request_id, - ), - ), - ), - Err(e) => { - tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); - build_sts_error_response(&e) - } + Err(response) => response, }, ) } -/// Verify a platform issuer's token, then mint for the account `RoleArn` -/// names if that account trusts the token's issuer and subject (ADR-014). The -/// credentials act as the account, never as the token's subject. Everything -/// local comes first, so a token that fails it costs the Source API nothing. +/// Verify a platform issuer's token, then mint for the service account +/// `RoleArn` names if that account trusts the token's issuer and subject +/// (ADR-014). The credentials act as the account, never as the token's +/// subject. Everything local comes first, so a token that fails it costs the +/// Source API nothing; what does cost a call is rate-limited per address. +/// Every refusal of the account's trust reads the same, whatever the reason. +#[allow(clippy::too_many_arguments)] async fn exchange_platform_token( config: &AppConfig, + env: &Env, + client_ip: &str, sts: &multistore_sts::request::StsRequest, header: &serde_json::Value, issuer: &str, audiences: &[String], api_auth: &ApiAuth, request_id: &str, -) -> Result { +) -> Result { + let failed = |e: ProxyError| { + tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); + build_sts_error_response(&e) + }; + let not_authorized = || { + let message = "Not authorized to perform sts:AssumeRoleWithWebIdentity"; + ( + 403, + sts_error_xml("AccessDenied", &with_request_id(message, request_id)), + ) + }; + let role = sts::role( &sts.role_arn, issuer.to_string(), audiences.to_vec(), config.sts_max_session_duration_secs, ) - .ok_or_else(|| ProxyError::RoleNotFound(sts.role_arn.clone()))?; + .ok_or_else(|| failed(ProxyError::RoleNotFound(sts.role_arn.clone())))?; // No angle brackets in the message: the STS error body carries it unescaped. let account = sts::account(&sts.role_arn).ok_or_else(|| { - ProxyError::InvalidRequest( + failed(ProxyError::InvalidRequest( "RoleArn must name the account to act as: arn:aws:iam::ACCOUNT:role/ROLE".into(), - ) + )) })?; + // Only a service account trusts subjects (ADR-014), and the credentials' + // principal is this segment as given, which source.coop tries as an Ory + // identity first. So anything else is refused before the token is + // verified or anything is signed as it. + if !sts::is_service_account_id(account) { + tracing::warn!(%request_id, %issuer, %account, "RoleArn names no service account"); + return Err(not_authorized()); + } let subject = platform::verify( &sts.web_identity_token, header, @@ -747,8 +765,15 @@ async fn exchange_platform_token( &role, &jwks_cache(), ) - .await?; - source_api::cache::get_or_fetch_trust( + .await + .map_err(failed)?; + // Anyone can mint a token for this audience in their own workflow, and + // every exchange from here on may cost the Source API a call. + if !within_rate_limit(env, client_ip).await { + tracing::warn!(%request_id, %issuer, reason = "rate_limited", "platform token exchange refused"); + return Err(throttled()); + } + match source_api::cache::get_or_fetch_trust( &config.api_base_url, account, issuer, @@ -757,22 +782,28 @@ async fn exchange_platform_token( request_id, ) .await - .map_err(|e| match e { - ProxyError::AccessDenied => { + { + Ok(()) => {} + Err(ProxyError::AccessDenied) => { tracing::warn!(%request_id, %issuer, %subject, %account, "account does not trust the token"); - e + return Err(not_authorized()); } // The route answers for any account; a 404 means the API does not // serve it, which is a deployment mismatch, not a refusal. - ProxyError::BucketNotFound(_) => ProxyError::Internal("trusts route not found".into()), - e => e, - })?; + Err(ProxyError::BucketNotFound(_)) => { + return Err(failed(ProxyError::Internal( + "trusts route not found".into(), + ))) + } + Err(e) => return Err(failed(e)), + } let creds = keys::credentials_for( &role, account, sts.duration_seconds, &config.session_token_key, - )?; + ) + .map_err(failed)?; tracing::info!(%request_id, %issuer, %subject, %account, role = %role.role_id, "platform token exchanged"); Ok(creds) } diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 2410b9fa..bb4f3175 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -57,6 +57,12 @@ const KEY_STANDING_CACHE_SECS: u32 = 60; // 1 minute /// reason: a trust that is removed should stop minting quickly (ADR-014). const TRUST_CACHE_SECS: u32 = 60; // 1 minute +/// A refusal from the trusts route, cached under its own key: long enough that +/// replaying one token its account does not trust costs about one lookup per +/// 10 seconds, however many addresses it comes from, and short enough that a +/// trust just added works within seconds. +const REFUSED_TRUST_CACHE_SECS: u32 = 10; + // ── Public cache functions ───────────────────────────────────────── /// Fetch a single product's metadata, cached for `PRODUCT_CACHE_SECS`. @@ -212,8 +218,9 @@ pub async fn get_or_fetch_key_standing( /// Whether `account` trusts `issuer`'s `subject` to act as it: `Ok` if so, /// `AccessDenied` if not, the way a role's own trust policy decides an /// assume-role call. Asked as the account itself. The route says yes with a -/// 200, cached for `TRUST_CACHE_SECS` like every 200, and no with a 403, which -/// is never cached, so a trust just added works on the next attempt. +/// 200, cached for `TRUST_CACHE_SECS` like every 200, and no with a 403 (a 401 +/// for an account it cannot resolve), which `cached_fetch` leaves uncached and +/// this caches for `REFUSED_TRUST_CACHE_SECS` under a key of its own. pub async fn get_or_fetch_trust( api_base_url: &str, account: &str, @@ -234,8 +241,13 @@ pub async fn get_or_fetch_trust( utf8_percent_encode(issuer, PATH_SEGMENT), utf8_percent_encode(subject, PATH_SEGMENT), ); + let refused_key = format!("{cache_key}&refused"); + let cache = worker::Cache::default(); + if matches!(cache.get(&refused_key, false).await, Ok(Some(_))) { + return Err(ProxyError::AccessDenied); + } let body = serde_json::json!({ "issuer": issuer, "subject": subject }).to_string(); - let answer: TrustAnswer = cached_fetch( + let answer = cached_fetch::( &cache_key, &api_url, "POST", @@ -245,8 +257,11 @@ pub async fn get_or_fetch_trust( request_id, ApiCaller::Account(account), ) - .await?; - if answer.trusted { + .await; + if matches!(answer, Err(ProxyError::AccessDenied)) { + cache_put(&cache, &refused_key, "{}", REFUSED_TRUST_CACHE_SECS).await; + } + if answer?.trusted { Ok(()) } else { Err(ProxyError::AccessDenied) @@ -379,16 +394,20 @@ async fn cached_fetch( let result: T = serde_json::from_str(&text) .map_err(|e| ProxyError::Internal(format!("JSON parse failed: {} for {}", e, api_url)))?; - // ── Store in cache ───────────────────────────────────────── + cache_put(&cache, cache_key, &text, ttl_secs).await; + Ok(result) +} + +/// Store `text` under `cache_key` for `ttl_secs`. A failed put costs only a +/// later lookup, so it is logged rather than returned. +async fn cache_put(cache: &worker::Cache, cache_key: &str, text: &str, ttl_secs: u32) { let headers = worker::Headers::new(); let _ = headers.set("content-type", "application/json"); let _ = headers.set("cache-control", &format!("max-age={}", ttl_secs)); - if let Ok(cache_resp) = worker::Response::ok(&text) { + if let Ok(cache_resp) = worker::Response::ok(text) { let cache_resp = cache_resp.with_headers(headers); if let Err(e) = cache.put(cache_key, cache_resp).await { tracing::warn!("cache put failed: {}", e); } } - - Ok(result) } diff --git a/src/sts.rs b/src/sts.rs index dc85ca2a..db2f080a 100644 --- a/src/sts.rs +++ b/src/sts.rs @@ -110,6 +110,27 @@ pub(crate) fn account(role_arn: &str) -> Option<&str> { } } +/// Whether `id` is a service account's id, `{owner}--{name}`: source.coop's +/// `SERVICE_ACCOUNT_ID_REGEX` and its 82-character limit. Each half is at least +/// two of `a-z`, `0-9` and inner single hyphens, so the one `--` is the +/// separator, and no person's or organisation's handle, nor an Ory identity +/// id, can match. +pub(crate) fn is_service_account_id(id: &str) -> bool { + let half = |part: &str| { + part.len() >= 2 + && part + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') + && !part.starts_with('-') + && !part.ends_with('-') + && !part.contains("--") + }; + id.len() <= 82 + && id + .split_once("--") + .is_some_and(|(owner, name)| half(owner) && half(name)) +} + impl CredentialRegistry for StsCredentialRegistry { async fn get_credential( &self, diff --git a/tests/sts.rs b/tests/sts.rs index 1dab3f71..e97bcedc 100644 --- a/tests/sts.rs +++ b/tests/sts.rs @@ -85,3 +85,44 @@ fn the_account_is_the_arns_account_segment() { assert_eq!(sts::account(role_arn), None, "{role_arn}"); } } + +#[test] +fn a_service_account_id_is_owner_dash_dash_name() { + let longest = format!("{}--{}", "a".repeat(40), "b".repeat(40)); + for id in [ + "acme--nightly-sync", + "ab--cd", + "my-org-1--a1-b2", + longest.as_str(), + ] { + assert!(sts::is_service_account_id(id), "{id}"); + } +} + +#[test] +fn nothing_else_is_a_service_account_id() { + let too_long = format!("{}--{}", "a".repeat(40), "b".repeat(41)); + for id in [ + "", + // A person's or organisation's handle. + "alice", + "my-org", + // An Ory identity id, which fits the handle grammar. + "2c5b4f0e-8a3b-4e2d-9a1f-3c4d5e6f7a8b", + "000000000000", + "Acme--sync", + "acme--sync_1", + "acme--", + "--sync", + "a--sync", + "acme--s", + "acme---sync", + "acme--sync--x", + "-acme--sync", + "acme--sync-", + "acme---", + too_long.as_str(), + ] { + assert!(!sts::is_service_account_id(id), "{id}"); + } +} diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py index 9974965d..46f1aa88 100644 --- a/tests/test_platform_trust.py +++ b/tests/test_platform_trust.py @@ -63,6 +63,22 @@ def test_a_platform_token_must_name_the_account_it_acts_as(): assert "RoleArn must name the account" in sts_fields(resp)["Message"] +@pytest.mark.parametrize( + "account", + ["alice", "2c5b4f0e-8a3b-4e2d-9a1f-3c4d5e6f7a8b", "000000000000"], + ids=["person", "ory-identity-id", "placeholder"], +) +def test_only_a_service_account_can_be_named(account): + """Refused as an account that does not trust the token is, and before the + token is verified: this one is forged, and still no lookup happens.""" + resp = exchange(forged(), as_account(account)) + assert resp.status_code == 403 + assert sts_fields(resp)["Message"] == ( + f"Not authorized to perform sts:AssumeRoleWithWebIdentity (request id {RAY})" + ) + assert trust_lookups(account) == 0 + + def test_a_forged_token_is_refused_before_any_trust_lookup(): before = trust_lookups(TRUST_ACCOUNT) resp = exchange(forged(), as_account(TRUST_ACCOUNT)) @@ -101,13 +117,18 @@ def test_a_trusted_workflow_gets_credentials_that_act_as_the_account(): @needs_token def test_an_account_that_does_not_trust_the_workflow_refuses_it(): - resp = exchange(ID_TOKEN, as_account("ci-tests--someone-else")) + untrusting = as_account("ci-tests--someone-else") + resp = exchange(ID_TOKEN, untrusting) assert resp.status_code == 403 fields = sts_fields(resp) assert fields["Code"] == "AccessDenied" assert fields["Message"] == ( f"Not authorized to perform sts:AssumeRoleWithWebIdentity (request id {RAY})" ) + # The refusal is cached briefly, so replaying the token costs no lookup. + before = trust_lookups("ci-tests--someone-else") + assert exchange(ID_TOKEN, untrusting).status_code == 403 + assert trust_lookups("ci-tests--someone-else") == before @needs_token diff --git a/wrangler.preview.toml b/wrangler.preview.toml index e5ddb9df..4262d9be 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -52,8 +52,9 @@ dataset = "source_data_proxy_staging" binding = "PUBLIC_LOG_STREAM" service = "public-log-stream-staging" -# API-key exchange rate limit, per client IP; see wrangler.toml. +# Limit on /.sts exchanges that cost a Source API call, per client IP; see +# wrangler.toml. [[ratelimits]] -name = "KEY_EXCHANGE_LIMIT" +name = "STS_EXCHANGE_LIMIT" namespace_id = "1003" simple = { limit = 100, period = 60 } diff --git a/wrangler.toml b/wrangler.toml index 6c45eb4e..ce6b814d 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -75,12 +75,13 @@ dataset = "source_data_proxy_production" binding = "PUBLIC_LOG_STREAM" service = "public-log-stream" -# API-key exchanges at /.sts, per client IP. Every attempt costs the Source -# API one lookup for a distinct key, so this bounds a flood of junk keys from -# one place; a legitimate client exchanges about once a session, so even a -# cluster behind one NAT stays far under it. See src/lib.rs `api_key_exchange`. +# Exchanges at /.sts that cost a Source API call (API keys and platform +# tokens), per client IP. It bounds a flood of junk keys, or of one replayed +# platform token, from one place; a legitimate client exchanges about once a +# session or job, so even a cluster behind one NAT stays far under it. See +# src/lib.rs `within_rate_limit`. [[ratelimits]] -name = "KEY_EXCHANGE_LIMIT" +name = "STS_EXCHANGE_LIMIT" namespace_id = "1001" simple = { limit = 100, period = 60 } @@ -109,7 +110,7 @@ binding = "PUBLIC_LOG_STREAM" service = "public-log-stream-staging" [[env.staging.ratelimits]] -name = "KEY_EXCHANGE_LIMIT" +name = "STS_EXCHANGE_LIMIT" namespace_id = "1002" simple = { limit = 100, period = 60 } From 89ae5b113a2b83967b1c680298ba7435236d51f6 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 21:17:50 -0700 Subject: [PATCH 05/19] refactor(sts): apply review suggestions on platform token exchange Hold a 200 trusted:false answer for REFUSED_TRUST_CACHE_SECS, as a 403 is, instead of the 60s positive TTL. Bundle the unverified token's parts into PlatformToken so exchange_platform_token no longer needs clippy::too_many_arguments. Co-Authored-By: Claude Opus 5.5 --- src/lib.rs | 33 +++++++++++++++++++++++---------- src/source_api/cache.rs | 20 ++++++++++++-------- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ca66d763..6a835f11 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -694,37 +694,50 @@ async fn platform_exchange( let (header, claims) = platform::unverified(&sts.web_identity_token)?; let issuer = claims.get("iss")?.as_str()?; let audiences = config.platform_issuers.get(issuer)?; + let token = PlatformToken { + sts: &sts, + header: &header, + issuer, + audiences, + }; let client_ip = header_str(&parts.headers, "cf-connecting-ip"); Some( - match exchange_platform_token( - config, env, client_ip, &sts, &header, issuer, audiences, api_auth, request_id, - ) - .await - { + match exchange_platform_token(config, env, client_ip, &token, api_auth, request_id).await { Ok(creds) => build_sts_response(&creds), Err(response) => response, }, ) } +/// An exchange request whose token names a configured platform issuer, not +/// yet verified. +struct PlatformToken<'a> { + sts: &'a multistore_sts::request::StsRequest, + header: &'a serde_json::Value, + issuer: &'a str, + audiences: &'a [String], +} + /// Verify a platform issuer's token, then mint for the service account /// `RoleArn` names if that account trusts the token's issuer and subject /// (ADR-014). The credentials act as the account, never as the token's /// subject. Everything local comes first, so a token that fails it costs the /// Source API nothing; what does cost a call is rate-limited per address. /// Every refusal of the account's trust reads the same, whatever the reason. -#[allow(clippy::too_many_arguments)] async fn exchange_platform_token( config: &AppConfig, env: &Env, client_ip: &str, - sts: &multistore_sts::request::StsRequest, - header: &serde_json::Value, - issuer: &str, - audiences: &[String], + token: &PlatformToken<'_>, api_auth: &ApiAuth, request_id: &str, ) -> Result { + let &PlatformToken { + sts, + header, + issuer, + audiences, + } = token; let failed = |e: ProxyError| { tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); build_sts_error_response(&e) diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index bb4f3175..9c764cef 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -258,19 +258,23 @@ pub async fn get_or_fetch_trust( ApiCaller::Account(account), ) .await; - if matches!(answer, Err(ProxyError::AccessDenied)) { + let trusted = match answer { + Ok(answer) => answer.trusted, + Err(ProxyError::AccessDenied) => false, + Err(e) => return Err(e), + }; + if !trusted { + // A 200 saying no was cached like any 200: drop it, so a no is held + // for `REFUSED_TRUST_CACHE_SECS` whichever way the route said it. + let _ = cache.delete(cache_key.as_str(), false).await; cache_put(&cache, &refused_key, "{}", REFUSED_TRUST_CACHE_SECS).await; + return Err(ProxyError::AccessDenied); } - if answer?.trusted { - Ok(()) - } else { - Err(ProxyError::AccessDenied) - } + Ok(()) } /// The trusts route's answer. Its status already says yes (200) or no (403); -/// the body is read too, so that a 200 saying no, cached like any 200, still -/// mints nothing. +/// the body is read too, so that a 200 saying no still mints nothing. #[derive(serde::Deserialize)] struct TrustAnswer { trusted: bool, From ec3f79c3b8ecaf91f2f9776ef8eb71bda67f5054 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 23:47:34 -0700 Subject: [PATCH 06/19] fix(sts): escape caller text in exchange error bodies (#246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > [!NOTE] > Stacked on #237. Until #237's branch has its latest review fixes pushed, this diff also shows those five commits; the change here is the last commit alone. ## What I'm changing The API-key and platform-token paths at `/.sts` now escape their error messages. Those messages can carry text the caller sent, `RoleArn` or a token's `kid` or `alg`, and multistore's `build_sts_error_response` wrote it unescaped into an `application/xml` body served with `access-control-allow-origin: *`. So: - markup in `RoleArn` ran as XHTML on the proxy's origin, for example `…`; - an `&` in `RoleArn` made the body unparseable for SDKs. ## How I did it - `sts_refusal` maps a `ProxyError` to status, code and message the way multistore's builder does, then builds the body with `sts_error_xml`. - `sts_error_xml`, which every error on these two paths goes through, escapes `&`, `<` and `>` in the message. - `build_sts_error_response` is no longer used in this crate. **Not covered:** the person issuer's route. Its errors come from multistore's router (`with_sts`), which calls the same unescaped builder, so the fix there belongs in multistore's `build_sts_error_response`. ## How to test it `tests/test_platform_trust.py::test_a_refusal_escapes_what_the_caller_sent` sends a forged GitHub token with a `RoleArn` containing an XHTML script element and `&`. It checks that the reply parses as XML and that the message carries the `RoleArn` as text. Locally against `wrangler dev` and the stub, 47 tests pass and 16 skip (the skipped ones need a GitHub token). ## PR Checklist - [x] This PR has **no** breaking changes. - [x] I have updated or added new tests to cover the changes in this PR. - [ ] This PR affects the [Source Cooperative Frontend & API](https://github.com/source-cooperative/source.coop). ## Related Issues Follows the review of #237. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- README.md | 2 +- src/config.rs | 13 ++++-- src/lib.rs | 79 ++++++++++++++++++++++++------------ src/source_api/cache.rs | 34 ++++++++-------- tests/test_platform_trust.py | 20 ++++++++- wrangler.preview.toml | 2 +- wrangler.toml | 2 +- 7 files changed, 100 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 6c4a0227..6b20a90f 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: | `SOURCE_API_URL` | `https://source.coop` | Source Cooperative API base URL | | `LOG_LEVEL` | `WARN` | Tracing level (`TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`) | | `AUTH_ISSUER` | `https://auth.source.coop` | The person issuer trusted for `/.sts` token exchange; its tokens act as their own subject | -| `AUTH_AUDIENCE` | — | Comma-separated OAuth client ID(s) that `/.sts` subject tokens must be issued to (`aud` claim); a token is accepted if it matches any. Unset = `/.sts` token exchange is disabled (returns 501) | +| `AUTH_AUDIENCE` | — | Comma-separated OAuth client ID(s) that `/.sts` subject tokens must be issued to (`aud` claim); a token is accepted if it matches any. Unset = person-token exchange at `/.sts` is disabled (returns 501); API keys and platform tokens still exchange | | `PLATFORM_ISSUERS` | — | JSON object from each platform issuer URL to the audiences its tokens must carry, such as `{"https://token.actions.githubusercontent.com": ["https://data.source.coop"]}`. An issuer with no audience is refused. Unset = no platform issuer is trusted | | `OIDC_PROVIDER_ISSUER` | `https://data.source.coop` | Issuer URL for minted JWTs and OIDC discovery | | `OIDC_PROVIDER_KID` | `data-proxy-1` | Key ID for the active signing key | diff --git a/src/config.rs b/src/config.rs index cbc1e8ed..8e77f0b5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -97,16 +97,23 @@ fn build_config(env: &Env) -> AppConfig { if auth_audiences.is_empty() { // Fail closed: without an audience restriction, an ID token minted for // ANY OAuth client of AUTH_ISSUER could be exchanged for a user's - // credentials, so /.sts is disabled entirely (returns 501) until set. - tracing::warn!("AUTH_AUDIENCE not set: /.sts token exchange is disabled (returns 501)"); + // credentials, so its exchange is disabled (returns 501) until set. + tracing::warn!( + "AUTH_AUDIENCE not set: person-token exchange at /.sts is disabled (returns 501)" + ); } // Platform identity providers (GitHub Actions, say), each with its own // audiences. Unset trusts none. - let platform_issuers = env + let mut platform_issuers = env .var("PLATFORM_ISSUERS") .map(|v| crate::platform::parse_issuers(&v.to_string())) .unwrap_or_default(); + // The platform path claims its issuers' tokens ahead of the STS route, so + // an entry for the person issuer would refuse every person exchange. + if platform_issuers.remove(&auth_issuer).is_some() { + tracing::error!(issuer = %auth_issuer, "PLATFORM_ISSUERS names AUTH_ISSUER; ignoring that entry"); + } // Ceiling for client-requested DurationSeconds on /.sts. Unset → 3600 (1h), // matching multistore's own default so behavior is unchanged until raised. diff --git a/src/lib.rs b/src/lib.rs index 6a835f11..515db29b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,7 +40,7 @@ use multistore_oidc_provider::{HttpExchange, OidcCredentialProvider, OidcProvide use multistore_path_mapping::{MappedRegistry, PathMapping}; use multistore_sts::jwks::JwksCache; use multistore_sts::route_handler::StsRouterExt; -use multistore_sts::{build_sts_error_response, build_sts_response, try_parse_sts_request}; +use multistore_sts::{build_sts_response, try_parse_sts_request}; use object_path::{extract_path_segments, is_keyless_write, mapped_copy_source}; use std::sync::OnceLock; use sts::StsCredentialRegistry; @@ -171,23 +171,6 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result Result { tracing::warn!(%request_id, error = %e, "API key exchange failed"); - build_sts_error_response(&e) + sts_refusal(&e, request_id) } }, ) @@ -559,9 +560,27 @@ async fn api_key_exchange( /// `InvalidIdentityToken`, with the request id in the message. fn key_refusal(message: &str, request_id: &str) -> (u16, String) { - build_sts_error_response(&ProxyError::InvalidOidcToken(with_request_id( - message, request_id, - ))) + sts_refusal(&ProxyError::InvalidOidcToken(message.into()), request_id) +} + +/// The STS error for `e`, mapped as multistore's `build_sts_error_response` +/// maps it, with the request id in the message. Built here because that one +/// writes the message unescaped, and it can carry a caller's `RoleArn` or a +/// token's `kid`. +fn sts_refusal(e: &ProxyError, request_id: &str) -> (u16, String) { + let (status, code, message) = match e { + ProxyError::RoleNotFound(r) => ( + 400, + "MalformedPolicyDocument", + format!("role not found: {r}"), + ), + ProxyError::InvalidOidcToken(m) => (400, "InvalidIdentityToken", m.clone()), + ProxyError::InvalidRequest(m) => (400, "InvalidParameterValue", m.clone()), + ProxyError::AccessDenied => (403, "AccessDenied", "access denied".to_string()), + _ => (500, "InternalError", "internal error".to_string()), + }; + let message = with_request_id(&message, request_id); + (status, sts_error_xml(code, &message)) } /// `message` with the request id, if there is one: SDKs show a user the @@ -668,9 +687,13 @@ fn throttled() -> (u16, String) { ) } -/// An STS-shaped error body with a code or message `build_sts_error_response` -/// does not produce. +/// An STS-shaped error body. The message is escaped: it can carry text the +/// caller sent, and the body is served as XML. fn sts_error_xml(code: &str, message: &str) -> String { + let message = message + .replace('&', "&") + .replace('<', "<") + .replace('>', ">"); format!( "\n{code}{message}" ) @@ -688,9 +711,12 @@ async fn platform_exchange( api_auth: &ApiAuth, request_id: &str, ) -> Option<(u16, String)> { - let sts = try_parse_sts_request(parts.query.as_deref()) + let mut sts = try_parse_sts_request(parts.query.as_deref()) .or_else(|| try_parse_sts_request(parts.form_body.as_deref()))? .ok()?; + // SDKs send a token file's contents as-is, and `jq -r … > file` ends it in + // a newline, which the signature segment's base64 decode rejects. + sts.web_identity_token = sts.web_identity_token.trim().to_string(); let (header, claims) = platform::unverified(&sts.web_identity_token)?; let issuer = claims.get("iss")?.as_str()?; let audiences = config.platform_issuers.get(issuer)?; @@ -740,7 +766,7 @@ async fn exchange_platform_token( } = token; let failed = |e: ProxyError| { tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); - build_sts_error_response(&e) + sts_refusal(&e, request_id) }; let not_authorized = || { let message = "Not authorized to perform sts:AssumeRoleWithWebIdentity"; @@ -757,7 +783,6 @@ async fn exchange_platform_token( config.sts_max_session_duration_secs, ) .ok_or_else(|| failed(ProxyError::RoleNotFound(sts.role_arn.clone())))?; - // No angle brackets in the message: the STS error body carries it unescaped. let account = sts::account(&sts.role_arn).ok_or_else(|| { failed(ProxyError::InvalidRequest( "RoleArn must name the account to act as: arn:aws:iam::ACCOUNT:role/ROLE".into(), diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 9c764cef..a06d8b07 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -57,10 +57,9 @@ const KEY_STANDING_CACHE_SECS: u32 = 60; // 1 minute /// reason: a trust that is removed should stop minting quickly (ADR-014). const TRUST_CACHE_SECS: u32 = 60; // 1 minute -/// A refusal from the trusts route, cached under its own key: long enough that -/// replaying one token its account does not trust costs about one lookup per -/// 10 seconds, however many addresses it comes from, and short enough that a -/// trust just added works within seconds. +/// A refusal from the trusts route: long enough that replaying one token its +/// account does not trust costs about one lookup per 10 seconds per data +/// center, and short enough that a trust just added works within seconds. const REFUSED_TRUST_CACHE_SECS: u32 = 10; // ── Public cache functions ───────────────────────────────────────── @@ -220,7 +219,7 @@ pub async fn get_or_fetch_key_standing( /// assume-role call. Asked as the account itself. The route says yes with a /// 200, cached for `TRUST_CACHE_SECS` like every 200, and no with a 403 (a 401 /// for an account it cannot resolve), which `cached_fetch` leaves uncached and -/// this caches for `REFUSED_TRUST_CACHE_SECS` under a key of its own. +/// this caches as `{"trusted":false}` for `REFUSED_TRUST_CACHE_SECS`. pub async fn get_or_fetch_trust( api_base_url: &str, account: &str, @@ -241,11 +240,6 @@ pub async fn get_or_fetch_trust( utf8_percent_encode(issuer, PATH_SEGMENT), utf8_percent_encode(subject, PATH_SEGMENT), ); - let refused_key = format!("{cache_key}&refused"); - let cache = worker::Cache::default(); - if matches!(cache.get(&refused_key, false).await, Ok(Some(_))) { - return Err(ProxyError::AccessDenied); - } let body = serde_json::json!({ "issuer": issuer, "subject": subject }).to_string(); let answer = cached_fetch::( &cache_key, @@ -260,17 +254,21 @@ pub async fn get_or_fetch_trust( .await; let trusted = match answer { Ok(answer) => answer.trusted, - Err(ProxyError::AccessDenied) => false, + Err(ProxyError::AccessDenied) => { + let cache = worker::Cache::default(); + let refused = r#"{"trusted":false}"#; + cache_put(&cache, &cache_key, refused, REFUSED_TRUST_CACHE_SECS).await; + false + } Err(e) => return Err(e), }; - if !trusted { - // A 200 saying no was cached like any 200: drop it, so a no is held - // for `REFUSED_TRUST_CACHE_SECS` whichever way the route said it. - let _ = cache.delete(cache_key.as_str(), false).await; - cache_put(&cache, &refused_key, "{}", REFUSED_TRUST_CACHE_SECS).await; - return Err(ProxyError::AccessDenied); + // ponytail: a 200 saying no (which the route never sends) is held for + // TRUST_CACHE_SECS like any 200; still refused, only slower to flip to yes. + if trusted { + Ok(()) + } else { + Err(ProxyError::AccessDenied) } - Ok(()) } /// The trusts route's answer. Its status already says yes (200) or no (403); diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py index 46f1aa88..9e84f480 100644 --- a/tests/test_platform_trust.py +++ b/tests/test_platform_trust.py @@ -63,6 +63,15 @@ def test_a_platform_token_must_name_the_account_it_acts_as(): assert "RoleArn must name the account" in sts_fields(resp)["Message"] +def test_a_refusal_escapes_what_the_caller_sent(): + """RoleArn is echoed in the message; markup in it stays text.""" + role_arn = 'arn:aws:iam::ab--cd:role/&' + resp = exchange(forged(), role_arn) + assert resp.status_code == 400 + assert " Date: Wed, 30 Sep 2026 23:53:47 -0700 Subject: [PATCH 07/19] refactor(sts): parse exchange parameters once for both short-circuits The API-key and platform paths each parsed the query string and form body again. Parse once in fetch, trim the token there, and pass the request-scoped values the exchanges share as one Exchange. Co-Authored-By: Claude Opus 5.5 --- src/lib.rs | 125 +++++++++++++++++++++++++++++------------------------ 1 file changed, 69 insertions(+), 56 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 515db29b..2dad18c6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -227,12 +227,30 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result Some((parsed, true)), + None => try_parse_sts_request(parts.form_body.as_deref()).map(|p| (p, false)), + }; + if let Some((Ok(mut sts), in_url)) = parsed { + // SDKs send a token file's contents as-is, and `jq -r … > file` + // ends it in a newline, which a JWT's base64 decode rejects. + sts.web_identity_token = sts.web_identity_token.trim().to_string(); + let client_ip = header_str(&parts.headers, "cf-connecting-ip"); + let exchange = Exchange { + config, + env: &env, + client_ip, + api_auth: &api_auth, + request_id: &request_id, + }; + if let Some(result) = api_key_exchange(&exchange, &sts, in_url).await { + return Ok(finish(result, &request_id)); + } + if let Some(result) = platform_exchange(&exchange, &sts).await { + return Ok(finish(result, &request_id)); + } } } @@ -494,47 +512,52 @@ fn finish((status, xml): (u16, String), request_id: &str) -> web_sys::Response { /// of API keys and platform tokens alike, keyed by client IP. const STS_EXCHANGE_LIMIT: &str = "STS_EXCHANGE_LIMIT"; -/// The API-key exchange, if this request is one: `None` when it is not an -/// `AssumeRoleWithWebIdentity` carrying an `sck_` key, so the STS route takes -/// it. A key is accepted from the form body only — Cloudflare logs the URL — -/// and the refusal for one in the query string says so, because that is the -/// one mistake a user can fix. +/// What every exchange ahead of the STS route needs from the request. +struct Exchange<'a> { + config: &'a AppConfig, + env: &'a Env, + client_ip: &'a str, + api_auth: &'a ApiAuth, + request_id: &'a str, +} + +/// The API-key exchange, if this request is one: `None` when it does not carry +/// an `sck_` key, so the STS route takes it. A key is accepted from the form +/// body only — Cloudflare logs the URL — and the refusal for one in the query +/// string says so, because that is the one mistake a user can fix. async fn api_key_exchange( - config: &AppConfig, - parts: &RequestParts, - env: &Env, - api_auth: &ApiAuth, - request_id: &str, + exchange: &Exchange<'_>, + sts: &multistore_sts::request::StsRequest, + in_url: bool, ) -> Option<(u16, String)> { - if let Some(parsed) = try_parse_sts_request(parts.query.as_deref()) { - let is_key = parsed - .as_ref() - .is_ok_and(|sts| keys::looks_like_api_key(&sts.web_identity_token)); - if !is_key { - return None; // a token in the query string is the STS route's - } + let &Exchange { + config, + env, + client_ip, + api_auth, + request_id, + } = exchange; + if !keys::looks_like_api_key(&sts.web_identity_token) { + return None; + } + if in_url { tracing::warn!(%request_id, reason = "query_string", "API key exchange refused"); return Some(key_refusal( "API key must be sent in the request body, not the URL", request_id, )); } - let sts = try_parse_sts_request(parts.form_body.as_deref())?.ok()?; - if !keys::looks_like_api_key(&sts.web_identity_token) { - return None; - } // Every attempt costs a lookup for a distinct key, so the flood to bound is // distinct junk keys from one place. Legitimate exchanges are rare — once // per session — so even a cluster behind one NAT stays well under the limit. - let client_ip = header_str(&parts.headers, "cf-connecting-ip"); if !within_rate_limit(env, client_ip).await { tracing::warn!(%request_id, reason = "rate_limited", "API key exchange refused"); return Some(throttled()); } Some( - match exchange_api_key(config, &sts, api_auth, request_id).await { + match exchange_api_key(config, sts, api_auth, request_id).await { Ok(creds) => build_sts_response(&creds), // A key that fails its shape or checksum was cut short or mistyped, // which the user can fix; saying so reveals nothing, since the @@ -702,37 +725,24 @@ fn sts_error_xml(code: &str, message: &str) -> String { // ── Platform identity providers ───────────────────────────────────── /// The exchange of a platform issuer's token, if this request carries one: -/// `None` for any other token, which the STS route takes. Parameters come from -/// the query string or the form body, never both, as at the STS route. +/// `None` for any other token, which the STS route takes. async fn platform_exchange( - config: &AppConfig, - parts: &RequestParts, - env: &Env, - api_auth: &ApiAuth, - request_id: &str, + exchange: &Exchange<'_>, + sts: &multistore_sts::request::StsRequest, ) -> Option<(u16, String)> { - let mut sts = try_parse_sts_request(parts.query.as_deref()) - .or_else(|| try_parse_sts_request(parts.form_body.as_deref()))? - .ok()?; - // SDKs send a token file's contents as-is, and `jq -r … > file` ends it in - // a newline, which the signature segment's base64 decode rejects. - sts.web_identity_token = sts.web_identity_token.trim().to_string(); let (header, claims) = platform::unverified(&sts.web_identity_token)?; let issuer = claims.get("iss")?.as_str()?; - let audiences = config.platform_issuers.get(issuer)?; + let audiences = exchange.config.platform_issuers.get(issuer)?; let token = PlatformToken { - sts: &sts, + sts, header: &header, issuer, audiences, }; - let client_ip = header_str(&parts.headers, "cf-connecting-ip"); - Some( - match exchange_platform_token(config, env, client_ip, &token, api_auth, request_id).await { - Ok(creds) => build_sts_response(&creds), - Err(response) => response, - }, - ) + Some(match exchange_platform_token(exchange, &token).await { + Ok(creds) => build_sts_response(&creds), + Err(response) => response, + }) } /// An exchange request whose token names a configured platform issuer, not @@ -751,13 +761,16 @@ struct PlatformToken<'a> { /// Source API nothing; what does cost a call is rate-limited per address. /// Every refusal of the account's trust reads the same, whatever the reason. async fn exchange_platform_token( - config: &AppConfig, - env: &Env, - client_ip: &str, + exchange: &Exchange<'_>, token: &PlatformToken<'_>, - api_auth: &ApiAuth, - request_id: &str, ) -> Result { + let &Exchange { + config, + env, + client_ip, + api_auth, + request_id, + } = exchange; let &PlatformToken { sts, header, From c11716884f5d4890735dd3a7fa3bf36482d3bef9 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 23:54:49 -0700 Subject: [PATCH 08/19] fix(sts): refuse a platform token sent in the URL Cloudflare logs the URL, and production ships a sample of those logs: a GitHub token there could be replayed for credentials until it expires. Platform tokens are now accepted from the form body only, as API keys already are. test_writes.sts_exchange sends form bodies by default. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/lib.rs | 16 ++++++++++++++-- tests/test_platform_trust.py | 11 +++++++++++ tests/test_writes.py | 28 ++++++++++++---------------- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 6b20a90f..7b96d29b 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ Any other name is refused with `MalformedPolicyDocument`, never mapped to a defa ### Platform identity providers -A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. At `/.sts` it acts as the service account in `RoleArn`, `arn:aws:iam::--:role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014); an account that is not a service account is refused before anything else. The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then, within `STS_EXCHANGE_LIMIT`, asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. Per account, issuer and subject, a yes is cached for 60 seconds and a no for 10, and the credentials' principal is the account, never the token's subject. Every refusal reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. +A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. It is presented at `/.sts` from a POST form body only, as an API key is: a token in the URL is refused, because the URL is logged. It acts as the service account in `RoleArn`, `arn:aws:iam::--:role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014); an account that is not a service account is refused before anything else. The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then, within `STS_EXCHANGE_LIMIT`, asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. Per account, issuer and subject, a yes is cached for 60 seconds and a no for 10, and the credentials' principal is the account, never the token's subject. Every refusal reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. `aws-actions/configure-aws-credentials` fails after the exchange succeeds: it checks the credentials it exports with `GetCallerIdentity`, which the proxy cannot answer until developmentseed/multistore#126 lands. Until then a workflow saves its token to a file and lets an AWS SDK exchange it, with `AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_ROLE_ARN`, `AWS_ENDPOINT_URL_STS=/.sts`, `AWS_ENDPOINT_URL_S3=` and `AWS_REGION`. diff --git a/src/lib.rs b/src/lib.rs index 2dad18c6..a2ac5243 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -248,7 +248,7 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result String { // ── Platform identity providers ───────────────────────────────────── /// The exchange of a platform issuer's token, if this request carries one: -/// `None` for any other token, which the STS route takes. +/// `None` for any other token, which the STS route takes. Like an API key, the +/// token is accepted from the form body only: Cloudflare logs the URL, and a +/// logged token could be replayed for credentials until it expires. async fn platform_exchange( exchange: &Exchange<'_>, sts: &multistore_sts::request::StsRequest, + in_url: bool, ) -> Option<(u16, String)> { let (header, claims) = platform::unverified(&sts.web_identity_token)?; let issuer = claims.get("iss")?.as_str()?; let audiences = exchange.config.platform_issuers.get(issuer)?; + if in_url { + let request_id = exchange.request_id; + tracing::warn!(%request_id, %issuer, reason = "query_string", "platform token exchange refused"); + let message = "WebIdentityToken must be sent in the request body, not the URL"; + return Some(sts_refusal( + &ProxyError::InvalidRequest(message.into()), + request_id, + )); + } let token = PlatformToken { sts, header: &header, diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py index 9e84f480..db8b1af5 100644 --- a/tests/test_platform_trust.py +++ b/tests/test_platform_trust.py @@ -88,6 +88,17 @@ def test_only_a_service_account_can_be_named(account): assert trust_lookups(account) == 0 +def test_a_platform_token_in_the_url_is_refused_before_any_trust_lookup(): + before = trust_lookups(TRUST_ACCOUNT) + params = {"Action": "AssumeRoleWithWebIdentity", "RoleArn": as_account(TRUST_ACCOUNT), "WebIdentityToken": forged()} + resp = requests.post(f"{PROXY_URL}/.sts", params=params, headers={"cf-ray": RAY}) + assert resp.status_code == 400 + assert sts_fields(resp)["Message"] == ( + f"WebIdentityToken must be sent in the request body, not the URL (request id {RAY})" + ) + assert trust_lookups(TRUST_ACCOUNT) == before + + def test_a_forged_token_is_refused_before_any_trust_lookup(): before = trust_lookups(TRUST_ACCOUNT) resp = exchange(forged(), as_account(TRUST_ACCOUNT)) diff --git a/tests/test_writes.py b/tests/test_writes.py index 5d5da408..0cdc377c 100644 --- a/tests/test_writes.py +++ b/tests/test_writes.py @@ -57,19 +57,19 @@ ) -def sts_exchange(token, *, form_body=False): +def sts_exchange(token, *, in_url=False): """POST /.sts with the given web identity token; return the raw response. - `form_body` sends the parameters the way AWS SDKs do — form-encoded in the - request body, with no query string — instead of in the query string.""" + The parameters are form-encoded in the body, the way AWS SDKs send them; + `in_url` sends them in the query string instead.""" params = { "Action": "AssumeRoleWithWebIdentity", "RoleArn": f"arn:aws:iam::{TRUST_ACCOUNT}:role/FullAccess", "WebIdentityToken": token, } - if form_body: - return requests.post(f"{PROXY_URL}/.sts", data=params) - return requests.post(f"{PROXY_URL}/.sts", params=params) + if in_url: + return requests.post(f"{PROXY_URL}/.sts", params=params) + return requests.post(f"{PROXY_URL}/.sts", data=params) @functools.lru_cache(maxsize=1) @@ -211,15 +211,11 @@ def test_sts_exchange_issues_credentials(): @needs_token -def test_sts_exchange_accepts_a_form_encoded_body(): - """The same exchange, sent the way an AWS SDK sends it: parameters - form-encoded in the POST body rather than in the query string.""" - resp = sts_exchange(ID_TOKEN, form_body=True) - assert resp.status_code == 200, ( - f"form-encoded /.sts exchange failed ({resp.status_code}): {resp.text[:300]}" - ) - fields = {el.tag.rpartition("}")[2]: el.text for el in ET.fromstring(resp.text).iter()} - assert fields["AccessKeyId"].startswith("STSPRXY") +def test_sts_exchange_refuses_a_platform_token_in_the_url(): + """Cloudflare logs the URL, and a logged token could be replayed.""" + resp = sts_exchange(ID_TOKEN, in_url=True) + assert resp.status_code == 400, resp.text[:300] + assert "must be sent in the request body" in resp.text def test_sts_form_encoded_body_reaches_the_sts_handler(): @@ -228,7 +224,7 @@ def test_sts_form_encoded_body_reaches_the_sts_handler(): through to the S3 pipeline. Without the body being collected before dispatch, the STS handler never sees the `Action` param and never matches, so the failure mode is a non-STS error rather than a 200.""" - resp = sts_exchange("not-a-jwt", form_body=True) + resp = sts_exchange("not-a-jwt") assert resp.status_code == 400, ( f"expected an STS rejection ({resp.status_code}): {resp.text[:300]}" ) From 8ccf862d517675687bb3665588d9a34caed7d4e3 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 23:55:59 -0700 Subject: [PATCH 09/19] fix(config): accept PLATFORM_ISSUERS as a TOML table Env::var fails on an object-valued var and the error was discarded, so writing PLATFORM_ISSUERS as [vars.PLATFORM_ISSUERS], Cloudflare's documented form for a JSON value, disabled every platform issuer with no log. Read it with object_var and take either a string of JSON or the object itself. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/config.rs | 14 +++++++++----- src/platform.rs | 11 ++++++++--- tests/platform.rs | 16 +++++++++++----- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 7b96d29b..e5f49072 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: | `LOG_LEVEL` | `WARN` | Tracing level (`TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`) | | `AUTH_ISSUER` | `https://auth.source.coop` | The person issuer trusted for `/.sts` token exchange; its tokens act as their own subject | | `AUTH_AUDIENCE` | — | Comma-separated OAuth client ID(s) that `/.sts` subject tokens must be issued to (`aud` claim); a token is accepted if it matches any. Unset = person-token exchange at `/.sts` is disabled (returns 501); API keys and platform tokens still exchange | -| `PLATFORM_ISSUERS` | — | JSON object from each platform issuer URL to the audiences its tokens must carry, such as `{"https://token.actions.githubusercontent.com": ["https://data.source.coop"]}`. An issuer with no audience is refused. Unset = no platform issuer is trusted | +| `PLATFORM_ISSUERS` | — | JSON object from each platform issuer URL to the audiences its tokens must carry, such as `{"https://token.actions.githubusercontent.com": ["https://data.source.coop"]}`, written as a string or as a TOML table. An issuer with no audience is refused. Unset = no platform issuer is trusted | | `OIDC_PROVIDER_ISSUER` | `https://data.source.coop` | Issuer URL for minted JWTs and OIDC discovery | | `OIDC_PROVIDER_KID` | `data-proxy-1` | Key ID for the active signing key | | `OIDC_PROVIDER_KID_PREVIOUS` | — | Key ID for the previous key (during rotation) | diff --git a/src/config.rs b/src/config.rs index 8e77f0b5..925a3c7e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -104,11 +104,15 @@ fn build_config(env: &Env) -> AppConfig { } // Platform identity providers (GitHub Actions, say), each with its own - // audiences. Unset trusts none. - let mut platform_issuers = env - .var("PLATFORM_ISSUERS") - .map(|v| crate::platform::parse_issuers(&v.to_string())) - .unwrap_or_default(); + // audiences. Unset trusts none. `var` takes only a string and + // `object_var` only an object, which is how a TOML table arrives. + let mut platform_issuers = match env.var("PLATFORM_ISSUERS") { + Ok(json) => crate::platform::parse_issuers(serde_json::Value::String(json.to_string())), + Err(_) => env + .object_var::("PLATFORM_ISSUERS") + .map(crate::platform::parse_issuers) + .unwrap_or_default(), + }; // The platform path claims its issuers' tokens ahead of the STS route, so // an entry for the person issuer would refuse every person exchange. if platform_issuers.remove(&auth_issuer).is_some() { diff --git a/src/platform.rs b/src/platform.rs index 27180a8c..ad66190e 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -20,9 +20,14 @@ use serde_json::Value; /// that one issuer's audience never admits another's token (ADR-009). An /// issuer with no audience is left out, as the person issuer is disabled /// without one: a token minted for any other service could be exchanged here. -/// A value that does not parse trusts no platform issuer. -pub fn parse_issuers(json: &str) -> HashMap> { - let issuers: HashMap> = match serde_json::from_str(json) { +/// A value that does not parse trusts no platform issuer. The variable may be +/// a string of JSON or, written as a TOML table, the object itself. +pub fn parse_issuers(value: Value) -> HashMap> { + let value = match value { + Value::String(json) => serde_json::from_str(&json).unwrap_or(Value::String(json)), + value => value, + }; + let issuers: HashMap> = match serde_json::from_value(value) { Ok(issuers) => issuers, Err(e) => { tracing::error!( diff --git a/tests/platform.rs b/tests/platform.rs index c1c3f305..4e8a8eb4 100644 --- a/tests/platform.rs +++ b/tests/platform.rs @@ -16,17 +16,23 @@ const GITHUB: &str = "https://token.actions.githubusercontent.com"; #[test] fn each_issuer_keeps_its_own_audiences() { - let issuers = platform::parse_issuers( + let issuers = platform::parse_issuers(json!( r#"{"https://token.actions.githubusercontent.com": ["https://data.source.coop"], - "https://gitlab.com": ["a", "b"]}"#, - ); + "https://gitlab.com": ["a", "b"]}"# + )); assert_eq!(issuers[GITHUB], ["https://data.source.coop"]); assert_eq!(issuers["https://gitlab.com"], ["a", "b"]); } +#[test] +fn a_toml_table_reads_as_the_string_does() { + let issuers = platform::parse_issuers(json!({GITHUB: ["https://data.source.coop"]})); + assert_eq!(issuers[GITHUB], ["https://data.source.coop"]); +} + #[test] fn an_issuer_without_an_audience_is_not_trusted() { - let issuers = platform::parse_issuers(r#"{"https://token.actions.githubusercontent.com": []}"#); + let issuers = platform::parse_issuers(json!({GITHUB: []})); assert!(issuers.is_empty()); } @@ -37,7 +43,7 @@ fn a_value_that_does_not_parse_trusts_no_issuer() { GITHUB, r#"["https://token.actions.githubusercontent.com"]"#, ] { - assert!(platform::parse_issuers(value).is_empty(), "{value}"); + assert!(platform::parse_issuers(json!(value)).is_empty(), "{value}"); } } From cc81361377b748f484c97deef1e3b1c0b8f8eb68 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 23:56:50 -0700 Subject: [PATCH 10/19] fix(sts): rate-limit only platform exchanges the trust cache misses STS_EXCHANGE_LIMIT was charged before either trust cache was read, so a job matrix behind one NAT address got 429s for answers that cost the Source API nothing, and throttled API-key exchanges from that address too. Read the cached answer first and charge the limit only before a lookup. Co-Authored-By: Claude Opus 5.5 --- src/lib.rs | 34 ++++++++++++++------------- src/source_api/cache.rs | 51 +++++++++++++++++++++++++++++++---------- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a2ac5243..37857270 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -830,22 +830,24 @@ async fn exchange_platform_token( ) .await .map_err(failed)?; - // Anyone can mint a token for this audience in their own workflow, and - // every exchange from here on may cost the Source API a call. - if !within_rate_limit(env, client_ip).await { - tracing::warn!(%request_id, %issuer, reason = "rate_limited", "platform token exchange refused"); - return Err(throttled()); - } - match source_api::cache::get_or_fetch_trust( - &config.api_base_url, - account, - issuer, - &subject, - api_auth, - request_id, - ) - .await - { + let api = config.api_base_url.as_str(); + let answer = match source_api::cache::cached_trust(api, account, issuer, &subject).await { + Some(answer) => answer, + None => { + // Anyone can mint a token for this audience in their own workflow, + // so a lookup the cache cannot answer is rate-limited; a cached + // answer costs the Source API nothing and is not. + if !within_rate_limit(env, client_ip).await { + tracing::warn!(%request_id, %issuer, reason = "rate_limited", "platform token exchange refused"); + return Err(throttled()); + } + source_api::cache::get_or_fetch_trust( + api, account, issuer, &subject, api_auth, request_id, + ) + .await + } + }; + match answer { Ok(()) => {} Err(ProxyError::AccessDenied) => { tracing::warn!(%request_id, %issuer, %subject, %account, "account does not trust the token"); diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index a06d8b07..1334e562 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -228,18 +228,7 @@ pub async fn get_or_fetch_trust( api_auth: &crate::ApiAuth, request_id: &str, ) -> Result<(), ProxyError> { - let api_url = format!( - "{}/api/v1/accounts/{}/trusts/exchanges", - api_base_url, - utf8_percent_encode(account, PATH_SEGMENT), - ); - // The Cache API keys on URLs: the account is in the path, and the issuer - // and subject vary with it. - let cache_key = format!( - "{api_url}?issuer={}&subject={}", - utf8_percent_encode(issuer, PATH_SEGMENT), - utf8_percent_encode(subject, PATH_SEGMENT), - ); + let (api_url, cache_key) = trust_urls(api_base_url, account, issuer, subject); let body = serde_json::json!({ "issuer": issuer, "subject": subject }).to_string(); let answer = cached_fetch::( &cache_key, @@ -264,6 +253,44 @@ pub async fn get_or_fetch_trust( }; // ponytail: a 200 saying no (which the route never sends) is held for // TRUST_CACHE_SECS like any 200; still refused, only slower to flip to yes. + trust_result(trusted) +} + +/// The answer `get_or_fetch_trust` has cached, if any, without asking the +/// Source API: so a caller can spend its rate limit only on a real lookup. +pub async fn cached_trust( + api_base_url: &str, + account: &str, + issuer: &str, + subject: &str, +) -> Option> { + let (_, cache_key) = trust_urls(api_base_url, account, issuer, subject); + let mut cached = worker::Cache::default() + .get(&cache_key, false) + .await + .ok()??; + let answer: TrustAnswer = serde_json::from_str(&cached.text().await.ok()?).ok()?; + Some(trust_result(answer.trusted)) +} + +/// The trusts route for `account`, and the cache key for its answer about +/// `issuer`'s `subject`. The Cache API keys on URLs: the account is in the +/// path, and the issuer and subject vary with it. +fn trust_urls(api_base_url: &str, account: &str, issuer: &str, subject: &str) -> (String, String) { + let api_url = format!( + "{}/api/v1/accounts/{}/trusts/exchanges", + api_base_url, + utf8_percent_encode(account, PATH_SEGMENT), + ); + let cache_key = format!( + "{api_url}?issuer={}&subject={}", + utf8_percent_encode(issuer, PATH_SEGMENT), + utf8_percent_encode(subject, PATH_SEGMENT), + ); + (api_url, cache_key) +} + +fn trust_result(trusted: bool) -> Result<(), ProxyError> { if trusted { Ok(()) } else { From 2a1171c42e783f4e91c7e318470a8bce45cf4c79 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 23:58:39 -0700 Subject: [PATCH 11/19] fix(sts): make a platform issuer's key failures retryable A failed or hung fetch of GitHub's JWKS, or a token signed with a key published since the cached copy, came back as 400 InvalidIdentityToken, which SDKs never retry, and a hung fetch stalled the request until the runtime killed it. - Bound the key fetch by STS_REQUEST_TIMEOUT and report any failure as a 500 InternalError, which SDKs retry. - On an unknown key id, look again through a second cache held a minute, so a rotated key is found at once and a forged key id costs the issuer at most one fetch a minute per isolate. platform::verify now takes the keys and stays native. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + Cargo.toml | 3 +++ src/lib.rs | 48 ++++++++++++++++++++++++++++++++++++++---------- src/platform.rs | 28 +++++++++++++++------------- 4 files changed, 57 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0a1a78b6..a3fa476c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1893,6 +1893,7 @@ version = "2.3.4" dependencies = [ "base64", "console_error_panic_hook", + "futures-util", "getrandom 0.4.3", "hmac", "http", diff --git a/Cargo.toml b/Cargo.toml index f687b363..16047e6d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,6 +80,9 @@ multistore-cf-workers = { version = "0.7.2", features = ["azure", "gcp"] } # unnecessary here. The `form` feature gates `.form()`, which the STS # `post_form` call in FetchHttpExchange needs. reqwest = { version = "0.13", default-features = false, features = ["form"] } +# Racing an issuer's key fetch against a timeout (`fetch_keys`); already in +# the tree via worker. +futures-util = { version = "0.3", default-features = false } console_error_panic_hook = "0.1" js-sys = "0.3" wasm-bindgen = "0.2" diff --git a/src/lib.rs b/src/lib.rs index 37857270..9fdbc8f4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,6 +23,7 @@ mod sts; use crate::config::AppConfig; use crate::source_api::{ApiAuth, SourceCoopRegistry}; use analytics::log_analytics; +use futures_util::future::Either; use handlers::{AccountListHandler, IndexHandler}; use multistore::api::response::ErrorResponse; use multistore::error::ProxyError; @@ -38,7 +39,7 @@ use multistore_oidc_provider::backend_auth::{AwsBackendAuth, MaybeOidcAuth}; use multistore_oidc_provider::route_handler::OidcRouterExt; use multistore_oidc_provider::{HttpExchange, OidcCredentialProvider, OidcProviderError}; use multistore_path_mapping::{MappedRegistry, PathMapping}; -use multistore_sts::jwks::JwksCache; +use multistore_sts::jwks::{find_key, JwksCache, JwksResponse}; use multistore_sts::route_handler::StsRouterExt; use multistore_sts::{build_sts_response, try_parse_sts_request}; use object_path::{extract_path_segments, is_keyless_write, mapped_copy_source}; @@ -70,6 +71,38 @@ fn jwks_cache() -> JwksCache { .clone() } +/// A platform issuer's keys, looked up again when `JWKS_CACHE`'s copy lacks a +/// token's key id: an issuer that rotates (GitHub does) publishes a key before +/// it signs with it. Held a minute, so a forged key id costs the issuer at +/// most one fetch a minute per isolate. +static FRESH_JWKS_CACHE: OnceLock = OnceLock::new(); + +/// `issuer`'s published keys, including `kid` if the issuer publishes it. +async fn platform_keys(issuer: &str, kid: &str) -> Result { + let keys = fetch_keys(&jwks_cache(), issuer).await?; + if find_key(&keys, kid).is_ok() { + return Ok(keys); + } + let fresh = FRESH_JWKS_CACHE + .get_or_init(|| JwksCache::new(http_client(), std::time::Duration::from_secs(60))); + fetch_keys(fresh, issuer).await +} + +/// `issuer`'s keys from `cache`, bounded by `STS_REQUEST_TIMEOUT`. A failure +/// is the issuer's, not the token's: an `InternalError` the caller's SDK +/// retries, where `InvalidIdentityToken` would fail it outright. +async fn fetch_keys(cache: &JwksCache, issuer: &str) -> Result { + let fetch = std::pin::pin!(cache.get_or_fetch(issuer)); + let timeout = std::pin::pin!(worker::Delay::from(STS_REQUEST_TIMEOUT)); + match futures_util::future::select(fetch, timeout).await { + Either::Left((Ok(keys), _)) => Ok(keys), + Either::Left((Err(e), _)) => Err(ProxyError::Internal(format!( + "keys for {issuer} unavailable: {e}" + ))), + Either::Right(_) => Err(ProxyError::Internal(format!("keys for {issuer} timed out"))), + } +} + /// Bound the outbound STS `AssumeRoleWithWebIdentity` call. Without it a slow or /// hung federation lets the whole request stall until the Cloudflare edge kills /// it with a non-XML `error code: NNNN` body, which the caller's AWS SDK can't @@ -821,15 +854,10 @@ async fn exchange_platform_token( tracing::warn!(%request_id, %issuer, %account, "RoleArn names no service account"); return Err(not_authorized()); } - let subject = platform::verify( - &sts.web_identity_token, - header, - issuer, - &role, - &jwks_cache(), - ) - .await - .map_err(failed)?; + let kid = platform::kid(header).map_err(failed)?; + let keys = platform_keys(issuer, kid).await.map_err(failed)?; + let subject = + platform::verify(&sts.web_identity_token, kid, &keys, issuer, &role).map_err(failed)?; let api = config.api_base_url.as_str(); let answer = match source_api::cache::cached_trust(api, account, issuer, &subject).await { Some(answer) => answer, diff --git a/src/platform.rs b/src/platform.rs index ad66190e..d2b8e437 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -11,8 +11,7 @@ use base64::engine::general_purpose::URL_SAFE_NO_PAD; use base64::Engine; use multistore::error::ProxyError; use multistore::types::RoleConfig; -use multistore_sts::jwks::{find_key, verify_token}; -use multistore_sts::JwksCache; +use multistore_sts::jwks::{find_key, verify_token, JwksResponse}; use serde_json::Value; /// The platform issuers in `PLATFORM_ISSUERS`, a JSON object from each issuer @@ -58,22 +57,25 @@ pub fn unverified(token: &str) -> Option<(Value, Value)> { Some((segments.next()??, segments.next()??)) } -/// Verify a platform issuer's token as the STS route verifies the person -/// issuer's (signature against the issuer's published keys, issuer, the +/// The id of the key a token's header says signed it. +pub fn kid(header: &Value) -> Result<&str, ProxyError> { + header + .get("kid") + .and_then(Value::as_str) + .ok_or_else(|| ProxyError::InvalidOidcToken("JWT missing kid".into())) +} + +/// Verify a platform issuer's token against `keys`, the issuer's published +/// keys, as the STS route verifies the person issuer's (signature, issuer, the /// audiences `role` requires, `exp` and `nbf`) and return its subject. -pub async fn verify( +pub fn verify( token: &str, - header: &Value, + kid: &str, + keys: &JwksResponse, issuer: &str, role: &RoleConfig, - jwks: &JwksCache, ) -> Result { - let kid = header - .get("kid") - .and_then(Value::as_str) - .ok_or_else(|| ProxyError::InvalidOidcToken("JWT missing kid".into()))?; - let keys = jwks.get_or_fetch(issuer).await?; - let claims = verify_token(token, find_key(&keys, kid)?, issuer, role)?; + let claims = verify_token(token, find_key(keys, kid)?, issuer, role)?; subject(&claims).map(str::to_string) } From 6e2a5b701c1a5eb88bcfc42b0773277ef7c28424 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 23:59:45 -0700 Subject: [PATCH 12/19] docs: bring exchange comments in line with the code ApiCaller::Account said the account was always one the proxy had authenticated, which the trusts lookup is not. The API-key and rate-limit comments predated the standing cache, the shared limit and charging platform exchanges only on a trust-cache miss; the README now also says how an unseen key id and a failed key fetch are handled. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 ++-- src/lib.rs | 12 +++++++----- src/source_api/auth.rs | 4 +++- wrangler.preview.toml | 2 +- wrangler.toml | 11 ++++++----- 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index e5f49072..fad9748f 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: | Binding | Kind | Description | | -------------------- | ----------- | -------------------------------------------------------------------------------------------------------------------------------------------- | -| `STS_EXCHANGE_LIMIT` | `ratelimit` | Per-client-IP limit on `/.sts` exchanges that cost a Source API call: API keys (ADR-013) and platform tokens (ADR-014). Declared under `[[ratelimits]]` in every `wrangler*.toml`; a deployment without it logs an error and exchanges without a limit | +| `STS_EXCHANGE_LIMIT` | `ratelimit` | Per-client-IP limit on `/.sts` exchanges that may cost a Source API call: every API-key exchange (ADR-013), and each platform-token exchange whose trust answer is not cached (ADR-014). Declared under `[[ratelimits]]` in every `wrangler*.toml`; a deployment without it logs an error and exchanges without a limit | ### API keys @@ -141,7 +141,7 @@ Any other name is refused with `MalformedPolicyDocument`, never mapped to a defa ### Platform identity providers -A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. It is presented at `/.sts` from a POST form body only, as an API key is: a token in the URL is refused, because the URL is logged. It acts as the service account in `RoleArn`, `arn:aws:iam::--:role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014); an account that is not a service account is refused before anything else. The proxy verifies the token against the issuer's JWKS, with that issuer's own audiences and a required `exp`, then, within `STS_EXCHANGE_LIMIT`, asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. Per account, issuer and subject, a yes is cached for 60 seconds and a no for 10, and the credentials' principal is the account, never the token's subject. Every refusal reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. +A token from a platform issuer in `PLATFORM_ISSUERS`, such as GitHub Actions, says which workload is calling but not which account it may act as. It is presented at `/.sts` from a POST form body only, as an API key is: a token in the URL is refused, because the URL is logged. It acts as the service account in `RoleArn`, `arn:aws:iam::--:role/FullAccess`, and only if that account trusts the token's issuer and subject (ADR-014); an account that is not a service account is refused before anything else. The proxy verifies the token against the issuer's JWKS (looked up again for a key id it has not seen; a failure to fetch it is a retryable `InternalError`), with that issuer's own audiences and a required `exp`, then, unless a cached answer settles it and within `STS_EXCHANGE_LIMIT`, asks `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with `{"issuer", "subject"}`, as the account. Per account, issuer and subject, a yes is cached for 60 seconds and a no for 10, and the credentials' principal is the account, never the token's subject. Every refusal reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`. A token from `AUTH_ISSUER` still acts as its own subject and ignores the account in `RoleArn`. `aws-actions/configure-aws-credentials` fails after the exchange succeeds: it checks the credentials it exports with `GetCallerIdentity`, which the proxy cannot answer until developmentseed/multistore#126 lands. Until then a workflow saves its token to a file and lets an AWS SDK exchange it, with `AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_ROLE_ARN`, `AWS_ENDPOINT_URL_STS=/.sts`, `AWS_ENDPOINT_URL_S3=` and `AWS_REGION`. diff --git a/src/lib.rs b/src/lib.rs index 9fdbc8f4..0520a6b4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -541,8 +541,9 @@ fn finish((status, xml): (u16, String), request_id: &str) -> web_sys::Response { response } -/// The rate-limiter binding for `/.sts` exchanges that cost a Source API call, -/// of API keys and platform tokens alike, keyed by client IP. +/// The rate-limiter binding for `/.sts` exchanges that may cost a Source API +/// call, keyed by client IP: every API-key exchange, and each platform-token +/// exchange the trust cache cannot answer. const STS_EXCHANGE_LIMIT: &str = "STS_EXCHANGE_LIMIT"; /// What every exchange ahead of the STS route needs from the request. @@ -581,9 +582,10 @@ async fn api_key_exchange( )); } - // Every attempt costs a lookup for a distinct key, so the flood to bound is - // distinct junk keys from one place. Legitimate exchanges are rare — once - // per session — so even a cluster behind one NAT stays well under the limit. + // A repeated key is answered from the standing cache, but each distinct key + // costs a lookup, so the flood to bound is distinct junk keys from one + // place. Legitimate key exchanges are rare, once per session; the limit is + // shared with platform-token lookups from the same address. if !within_rate_limit(env, client_ip).await { tracing::warn!(%request_id, reason = "rate_limited", "API key exchange refused"); return Some(throttled()); diff --git a/src/source_api/auth.rs b/src/source_api/auth.rs index 41da850c..ee95e9c0 100644 --- a/src/source_api/auth.rs +++ b/src/source_api/auth.rs @@ -12,7 +12,9 @@ pub(crate) const PROXY_SELF_SUBJECT: &str = "urn:source:data-proxy"; pub(crate) enum ApiCaller<'a> { /// No credentials: the API answers as it would any stranger. Anonymous, - /// On behalf of an account the proxy has authenticated. + /// As an account: one the proxy has authenticated, or, for the trusts + /// lookup alone, the account a platform token names, before its trust is + /// established (ADR-014). Account(&'a str), /// The proxy itself. Proxy, diff --git a/wrangler.preview.toml b/wrangler.preview.toml index 23810e61..9ee5a3f7 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -52,7 +52,7 @@ dataset = "source_data_proxy_staging" binding = "PUBLIC_LOG_STREAM" service = "public-log-stream-staging" -# Limit on /.sts exchanges that cost a Source API call, per client IP; see +# Limit on /.sts exchanges that may cost a Source API call, per client IP; see # wrangler.toml. [[ratelimits]] name = "STS_EXCHANGE_LIMIT" diff --git a/wrangler.toml b/wrangler.toml index 564e0382..b34d104d 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -75,11 +75,12 @@ dataset = "source_data_proxy_production" binding = "PUBLIC_LOG_STREAM" service = "public-log-stream" -# Exchanges at /.sts that cost a Source API call (API keys and platform -# tokens), per client IP. It bounds a flood of junk keys, or of one replayed -# platform token, from one place; a legitimate client exchanges about once a -# session or job, so even a cluster behind one NAT stays far under it. See -# src/lib.rs `within_rate_limit`. +# Exchanges at /.sts that may cost a Source API call, per client IP: every API +# key exchange, and each platform-token exchange the trust cache cannot answer. +# It bounds a flood of junk keys, or of platform tokens naming one account after +# another, from one place. A cached trust answer is not charged, so a job +# matrix behind one NAT sharing a subject stays far under it. See src/lib.rs +# `within_rate_limit`. [[ratelimits]] name = "STS_EXCHANGE_LIMIT" namespace_id = "1001" From 6c9126de961d3bb8275407e1017264eb4cc64ed6 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 1 Oct 2026 00:00:48 -0700 Subject: [PATCH 13/19] test: drop a redundant assertion from the service-account test A forged token past the service-account check would fail verification with a 400, and the trust lookup only follows verification, so the 403 already proves no lookup happened. Co-Authored-By: Claude Opus 5.5 --- tests/test_platform_trust.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py index db8b1af5..461ee912 100644 --- a/tests/test_platform_trust.py +++ b/tests/test_platform_trust.py @@ -79,13 +79,13 @@ def test_a_refusal_escapes_what_the_caller_sent(): ) def test_only_a_service_account_can_be_named(account): """Refused as an account that does not trust the token is, and before the - token is verified: this one is forged, and still no lookup happens.""" + token is verified: this one is forged, so a later refusal would be a 400, + and the trust lookup only follows verification.""" resp = exchange(forged(), as_account(account)) assert resp.status_code == 403 assert sts_fields(resp)["Message"] == ( f"Not authorized to perform sts:AssumeRoleWithWebIdentity (request id {RAY})" ) - assert trust_lookups(account) == 0 def test_a_platform_token_in_the_url_is_refused_before_any_trust_lookup(): From e4845416adc60a36109fa0654a03a82a27578ccf Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 1 Oct 2026 00:08:49 -0700 Subject: [PATCH 14/19] refactor(sts): drop the Exchange and PlatformToken structs They only bundled arguments. Each exchange now takes the request's values directly, within clippy's argument limit, and the platform path is one function whose verify-and-mint body is an async block, so its early refusals still return Err. Co-Authored-By: Claude Opus 5.5 --- src/lib.rs | 246 ++++++++++++++++++++++------------------------------- 1 file changed, 101 insertions(+), 145 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0520a6b4..8acc43e5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -270,18 +270,13 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result file` // ends it in a newline, which a JWT's base64 decode rejects. sts.web_identity_token = sts.web_identity_token.trim().to_string(); - let client_ip = header_str(&parts.headers, "cf-connecting-ip"); - let exchange = Exchange { - config, - env: &env, - client_ip, - api_auth: &api_auth, - request_id: &request_id, - }; - if let Some(result) = api_key_exchange(&exchange, &sts, in_url).await { + let ip = header_str(&parts.headers, "cf-connecting-ip"); + let (auth, id) = (&api_auth, request_id.as_str()); + if let Some(result) = api_key_exchange(config, &env, ip, auth, id, &sts, in_url).await { return Ok(finish(result, &request_id)); } - if let Some(result) = platform_exchange(&exchange, &sts, in_url).await { + if let Some(result) = platform_exchange(config, &env, ip, auth, id, &sts, in_url).await + { return Ok(finish(result, &request_id)); } } @@ -546,31 +541,19 @@ fn finish((status, xml): (u16, String), request_id: &str) -> web_sys::Response { /// exchange the trust cache cannot answer. const STS_EXCHANGE_LIMIT: &str = "STS_EXCHANGE_LIMIT"; -/// What every exchange ahead of the STS route needs from the request. -struct Exchange<'a> { - config: &'a AppConfig, - env: &'a Env, - client_ip: &'a str, - api_auth: &'a ApiAuth, - request_id: &'a str, -} - /// The API-key exchange, if this request is one: `None` when it does not carry /// an `sck_` key, so the STS route takes it. A key is accepted from the form /// body only — Cloudflare logs the URL — and the refusal for one in the query /// string says so, because that is the one mistake a user can fix. async fn api_key_exchange( - exchange: &Exchange<'_>, + config: &AppConfig, + env: &Env, + client_ip: &str, + api_auth: &ApiAuth, + request_id: &str, sts: &multistore_sts::request::StsRequest, in_url: bool, ) -> Option<(u16, String)> { - let &Exchange { - config, - env, - client_ip, - api_auth, - request_id, - } = exchange; if !keys::looks_like_api_key(&sts.web_identity_token) { return None; } @@ -763,16 +746,26 @@ fn sts_error_xml(code: &str, message: &str) -> String { /// `None` for any other token, which the STS route takes. Like an API key, the /// token is accepted from the form body only: Cloudflare logs the URL, and a /// logged token could be replayed for credentials until it expires. +/// +/// The token is verified, then credentials are minted for the service account +/// `RoleArn` names if that account trusts the token's issuer and subject +/// (ADR-014). They act as the account, never as the token's subject. +/// Everything local comes first, so a token that fails it costs the Source API +/// nothing; what does cost a call is rate-limited per address. Every refusal +/// of the account's trust reads the same, whatever the reason. async fn platform_exchange( - exchange: &Exchange<'_>, + config: &AppConfig, + env: &Env, + client_ip: &str, + api_auth: &ApiAuth, + request_id: &str, sts: &multistore_sts::request::StsRequest, in_url: bool, ) -> Option<(u16, String)> { let (header, claims) = platform::unverified(&sts.web_identity_token)?; let issuer = claims.get("iss")?.as_str()?; - let audiences = exchange.config.platform_issuers.get(issuer)?; + let audiences = config.platform_issuers.get(issuer)?; if in_url { - let request_id = exchange.request_id; tracing::warn!(%request_id, %issuer, reason = "query_string", "platform token exchange refused"); let message = "WebIdentityToken must be sent in the request body, not the URL"; return Some(sts_refusal( @@ -780,127 +773,90 @@ async fn platform_exchange( request_id, )); } - let token = PlatformToken { - sts, - header: &header, - issuer, - audiences, - }; - Some(match exchange_platform_token(exchange, &token).await { - Ok(creds) => build_sts_response(&creds), - Err(response) => response, - }) -} - -/// An exchange request whose token names a configured platform issuer, not -/// yet verified. -struct PlatformToken<'a> { - sts: &'a multistore_sts::request::StsRequest, - header: &'a serde_json::Value, - issuer: &'a str, - audiences: &'a [String], -} + let minted: Result = async { + let failed = |e: ProxyError| { + tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); + sts_refusal(&e, request_id) + }; + let not_authorized = || { + let message = "Not authorized to perform sts:AssumeRoleWithWebIdentity"; + ( + 403, + sts_error_xml("AccessDenied", &with_request_id(message, request_id)), + ) + }; -/// Verify a platform issuer's token, then mint for the service account -/// `RoleArn` names if that account trusts the token's issuer and subject -/// (ADR-014). The credentials act as the account, never as the token's -/// subject. Everything local comes first, so a token that fails it costs the -/// Source API nothing; what does cost a call is rate-limited per address. -/// Every refusal of the account's trust reads the same, whatever the reason. -async fn exchange_platform_token( - exchange: &Exchange<'_>, - token: &PlatformToken<'_>, -) -> Result { - let &Exchange { - config, - env, - client_ip, - api_auth, - request_id, - } = exchange; - let &PlatformToken { - sts, - header, - issuer, - audiences, - } = token; - let failed = |e: ProxyError| { - tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed"); - sts_refusal(&e, request_id) - }; - let not_authorized = || { - let message = "Not authorized to perform sts:AssumeRoleWithWebIdentity"; - ( - 403, - sts_error_xml("AccessDenied", &with_request_id(message, request_id)), + let role = sts::role( + &sts.role_arn, + issuer.to_string(), + audiences.to_vec(), + config.sts_max_session_duration_secs, ) - }; - - let role = sts::role( - &sts.role_arn, - issuer.to_string(), - audiences.to_vec(), - config.sts_max_session_duration_secs, - ) - .ok_or_else(|| failed(ProxyError::RoleNotFound(sts.role_arn.clone())))?; - let account = sts::account(&sts.role_arn).ok_or_else(|| { - failed(ProxyError::InvalidRequest( - "RoleArn must name the account to act as: arn:aws:iam::ACCOUNT:role/ROLE".into(), - )) - })?; - // Only a service account trusts subjects (ADR-014), and the credentials' - // principal is this segment as given, which source.coop tries as an Ory - // identity first. So anything else is refused before the token is - // verified or anything is signed as it. - if !sts::is_service_account_id(account) { - tracing::warn!(%request_id, %issuer, %account, "RoleArn names no service account"); - return Err(not_authorized()); - } - let kid = platform::kid(header).map_err(failed)?; - let keys = platform_keys(issuer, kid).await.map_err(failed)?; - let subject = - platform::verify(&sts.web_identity_token, kid, &keys, issuer, &role).map_err(failed)?; - let api = config.api_base_url.as_str(); - let answer = match source_api::cache::cached_trust(api, account, issuer, &subject).await { - Some(answer) => answer, - None => { - // Anyone can mint a token for this audience in their own workflow, - // so a lookup the cache cannot answer is rate-limited; a cached - // answer costs the Source API nothing and is not. - if !within_rate_limit(env, client_ip).await { - tracing::warn!(%request_id, %issuer, reason = "rate_limited", "platform token exchange refused"); - return Err(throttled()); - } - source_api::cache::get_or_fetch_trust( - api, account, issuer, &subject, api_auth, request_id, - ) - .await - } - }; - match answer { - Ok(()) => {} - Err(ProxyError::AccessDenied) => { - tracing::warn!(%request_id, %issuer, %subject, %account, "account does not trust the token"); + .ok_or_else(|| failed(ProxyError::RoleNotFound(sts.role_arn.clone())))?; + let account = sts::account(&sts.role_arn).ok_or_else(|| { + failed(ProxyError::InvalidRequest( + "RoleArn must name the account to act as: arn:aws:iam::ACCOUNT:role/ROLE".into(), + )) + })?; + // Only a service account trusts subjects (ADR-014), and the credentials' + // principal is this segment as given, which source.coop tries as an Ory + // identity first. So anything else is refused before the token is + // verified or anything is signed as it. + if !sts::is_service_account_id(account) { + tracing::warn!(%request_id, %issuer, %account, "RoleArn names no service account"); return Err(not_authorized()); } - // The route answers for any account; a 404 means the API does not - // serve it, which is a deployment mismatch, not a refusal. - Err(ProxyError::BucketNotFound(_)) => { - return Err(failed(ProxyError::Internal( - "trusts route not found".into(), - ))) + let kid = platform::kid(&header).map_err(failed)?; + let keys = platform_keys(issuer, kid).await.map_err(failed)?; + let subject = + platform::verify(&sts.web_identity_token, kid, &keys, issuer, &role).map_err(failed)?; + let api = config.api_base_url.as_str(); + let answer = match source_api::cache::cached_trust(api, account, issuer, &subject).await { + Some(answer) => answer, + None => { + // Anyone can mint a token for this audience in their own workflow, + // so a lookup the cache cannot answer is rate-limited; a cached + // answer costs the Source API nothing and is not. + if !within_rate_limit(env, client_ip).await { + tracing::warn!(%request_id, %issuer, reason = "rate_limited", "platform token exchange refused"); + return Err(throttled()); + } + source_api::cache::get_or_fetch_trust( + api, account, issuer, &subject, api_auth, request_id, + ) + .await + } + }; + match answer { + Ok(()) => {} + Err(ProxyError::AccessDenied) => { + tracing::warn!(%request_id, %issuer, %subject, %account, "account does not trust the token"); + return Err(not_authorized()); + } + // The route answers for any account; a 404 means the API does not + // serve it, which is a deployment mismatch, not a refusal. + Err(ProxyError::BucketNotFound(_)) => { + return Err(failed(ProxyError::Internal( + "trusts route not found".into(), + ))) + } + Err(e) => return Err(failed(e)), } - Err(e) => return Err(failed(e)), + let creds = keys::credentials_for( + &role, + account, + sts.duration_seconds, + &config.session_token_key, + ) + .map_err(failed)?; + tracing::info!(%request_id, %issuer, %subject, %account, role = %role.role_id, "platform token exchanged"); + Ok(creds) } - let creds = keys::credentials_for( - &role, - account, - sts.duration_seconds, - &config.session_token_key, - ) - .map_err(failed)?; - tracing::info!(%request_id, %issuer, %subject, %account, role = %role.role_id, "platform token exchanged"); - Ok(creds) + .await; + Some(match minted { + Ok(creds) => build_sts_response(&creds), + Err(response) => response, + }) } // ── CORS ──────────────────────────────────────────────────────────── From 6404e4292018e7759b2e9ed3280966f38388700a Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 1 Oct 2026 00:12:26 -0700 Subject: [PATCH 15/19] refactor(cache): inline the trust answer helpers Co-Authored-By: Claude Opus 5.5 --- src/source_api/cache.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/source_api/cache.rs b/src/source_api/cache.rs index 1334e562..e3824996 100644 --- a/src/source_api/cache.rs +++ b/src/source_api/cache.rs @@ -244,16 +244,21 @@ pub async fn get_or_fetch_trust( let trusted = match answer { Ok(answer) => answer.trusted, Err(ProxyError::AccessDenied) => { - let cache = worker::Cache::default(); let refused = r#"{"trusted":false}"#; - cache_put(&cache, &cache_key, refused, REFUSED_TRUST_CACHE_SECS).await; + cache_put( + &worker::Cache::default(), + &cache_key, + refused, + REFUSED_TRUST_CACHE_SECS, + ) + .await; false } Err(e) => return Err(e), }; // ponytail: a 200 saying no (which the route never sends) is held for // TRUST_CACHE_SECS like any 200; still refused, only slower to flip to yes. - trust_result(trusted) + trusted.then_some(()).ok_or(ProxyError::AccessDenied) } /// The answer `get_or_fetch_trust` has cached, if any, without asking the @@ -270,7 +275,7 @@ pub async fn cached_trust( .await .ok()??; let answer: TrustAnswer = serde_json::from_str(&cached.text().await.ok()?).ok()?; - Some(trust_result(answer.trusted)) + Some(answer.trusted.then_some(()).ok_or(ProxyError::AccessDenied)) } /// The trusts route for `account`, and the cache key for its answer about @@ -290,14 +295,6 @@ fn trust_urls(api_base_url: &str, account: &str, issuer: &str, subject: &str) -> (api_url, cache_key) } -fn trust_result(trusted: bool) -> Result<(), ProxyError> { - if trusted { - Ok(()) - } else { - Err(ProxyError::AccessDenied) - } -} - /// The trusts route's answer. Its status already says yes (200) or no (403); /// the body is read too, so that a 200 saying no still mints nothing. #[derive(serde::Deserialize)] From 8938bb25dd4abb4c7495497d5798d4128b28c11a Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 23:37:32 -0700 Subject: [PATCH 16/19] test(ci): cover the person route and per-issuer audiences - Run a second worker whose AUTH_ISSUER is GitHub, so CI's token is a person token there: tests/test_person_route.py exchanges it at _default and signs with the result, and refuses a wrong audience and a tampered signature. No CI test reached the person route with a validly signed token since GitHub became a platform issuer. - Give the main worker's AUTH_AUDIENCE the wrong-audience token's audience, so a platform path that checked the person audiences instead of GitHub's own would fail CI. - The stub trusts exactly the subject of the token CI minted, as source.coop matches a trust, instead of a repository prefix. - Pin that a 200 saying {"trusted": false} is still refused. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 29 ++++++++++++++--- tests/stub_api.py | 15 ++++++--- tests/test_person_route.py | 62 ++++++++++++++++++++++++++++++++++++ tests/test_platform_trust.py | 9 +++++- 4 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 tests/test_person_route.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0012e058..488165d0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,16 +109,19 @@ jobs: # Inbound callers authenticate with GitHub Actions OIDC tokens. GitHub # is a platform issuer, as in production: PLATFORM_ISSUERS names the # audience requested in "Mint caller identity token" below, and a - # token acts as an account the stub says trusts this repository. No - # person-issuer token exists in CI; AUTH_ISSUER names an issuer that - # mints nothing, and AUTH_AUDIENCE keeps /.sts from answering 501. + # token acts as an account the stub says trusts this repository. + # AUTH_ISSUER names an issuer that mints nothing. AUTH_AUDIENCE is the + # wrong-audience token's audience: if the platform path ever checked + # the person audiences instead of GitHub's own, test_writes.py's + # audience tests would fail. The person route runs on a second worker; + # see "Start wrangler dev". run: | { openssl genpkey -algorithm RSA -out /tmp/oidc.pem -pkeyopt rsa_keygen_bits:2048 printf 'OIDC_PROVIDER_KEY="%s"\n' "$(cat /tmp/oidc.pem)" echo "SESSION_TOKEN_KEY=$(openssl rand -base64 32)" echo "AUTH_ISSUER=https://auth.example.invalid" - echo "AUTH_AUDIENCE=source-data-proxy-ci" + echo "AUTH_AUDIENCE=not-the-data-proxy" echo 'PLATFORM_ISSUERS={"https://token.actions.githubusercontent.com": ["source-data-proxy-ci"]}' echo "SOURCE_API_URL=http://localhost:9000" } > .dev.vars @@ -148,6 +151,10 @@ jobs: fi echo "::add-mask::$token" echo "CI_WRITE_ID_TOKEN=$token" >> "$GITHUB_ENV" + # The stub trusts exactly this token's subject, as source.coop + # matches a trust (see tests/stub_api.py). + sub=$(python3 -c 'import base64,json,sys; p=sys.argv[1].split(".")[1]; print(json.loads(base64.urlsafe_b64decode(p + "=" * (-len(p) % 4)))["sub"])' "$token") + echo "CI_TRUSTED_SUBJECT=$sub" >> "$GITHUB_ENV" # A second, validly-signed token whose audience is not GitHub's in # PLATFORM_ISSUERS: test_writes.py asserts the /.sts aud gate # rejects it (signature checks alone would let it through). @@ -175,8 +182,15 @@ jobs: # may appear later in the config are untouched. sed -i '/^\[build\]/,/^\[/ s/^command = .*/command = "echo skip"/' wrangler.toml wrangler dev --port 8787 > /tmp/wrangler.log 2>&1 & + # A second worker whose person issuer is GitHub, so CI's token + # exercises the person route (tests/test_person_route.py). Its + # PLATFORM_ISSUERS still names GitHub, which the proxy drops at load. + wrangler dev --port 8788 --inspector-port 9230 --persist-to /tmp/wrangler-person \ + --var AUTH_ISSUER:https://token.actions.githubusercontent.com \ + --var AUTH_AUDIENCE:source-data-proxy-ci > /tmp/wrangler-person.log 2>&1 & curl --retry 120 --retry-delay 1 --retry-connrefused --silent --fail http://localhost:8787/ > /dev/null - echo "Server ready" + curl --retry 120 --retry-delay 1 --retry-connrefused --silent --fail http://localhost:8788/ > /dev/null + echo "Servers ready" - name: Run integration tests # Contract tests are excluded: they hit the live prod API, so they run # on a schedule instead (.github/workflows/contract.yml) — a prod @@ -186,6 +200,8 @@ jobs: # var went missing (renamed, mint plumbing broken), the tests fail # loudly instead of skipping green (see tests/test_writes.py). CI_EXPECT_OIDC: ${{ github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository }} + # Unset on fork PRs, which have no token to exchange there. + PERSON_PROXY_URL: ${{ env.CI_WRITE_ID_TOKEN && 'http://localhost:8788' || '' }} run: uvx --with requests --with boto3 pytest tests/ -v --ignore=tests/test_contract.py - name: Dump server logs # Both servers run in the background, so their output isn't captured @@ -201,6 +217,9 @@ jobs: echo "::group::wrangler dev" cat /tmp/wrangler.log || true echo "::endgroup::" + echo "::group::wrangler dev (person issuer)" + cat /tmp/wrangler-person.log || true + echo "::endgroup::" audit: name: Security Audit diff --git a/tests/stub_api.py b/tests/stub_api.py index 6b362e19..457a22f7 100644 --- a/tests/stub_api.py +++ b/tests/stub_api.py @@ -158,12 +158,15 @@ def _hash(key): # ── Account trusts ───────────────────────────────────────────────── # Whether an account trusts a platform token's issuer and subject, at POST # /api/v1/accounts/{account}/trusts/exchanges (ADR-014). The proxy asks as the -# account itself. Only TRUST_ACCOUNT trusts anyone: GitHub Actions workflows -# in this repository, whatever event minted the token. A counter per account -# lets test_platform_trust.py prove the proxy caches a yes. +# account itself. Only TRUST_ACCOUNT trusts anyone, and only the exact subject +# of the token CI minted for this run (CI_TRUSTED_SUBJECT, set by ci.yml), as +# source.coop matches a trust exactly. SAYS_NO_ACCOUNT answers no with a 200, +# which the real route never does, so the proxy's read of the body is pinned. +# A counter per account lets test_platform_trust.py prove the proxy caches. TRUST_ACCOUNT = "ci-tests--github-ci" +SAYS_NO_ACCOUNT = "ci-tests--says-no-with-200" TRUSTED_ISSUER = "https://token.actions.githubusercontent.com" -TRUSTED_SUBJECT_PREFIX = "repo:source-cooperative/data.source.coop:" +TRUSTED_SUBJECT = os.environ.get("CI_TRUSTED_SUBJECT") TRUST_EXCHANGE_COUNTS = {} # Who the proxy said it was asking as, per product path, so a test can check @@ -212,10 +215,12 @@ def _trust_exchange(self, account): except (ValueError, KeyError, TypeError): return self._send(400, b"{}") TRUST_EXCHANGE_COUNTS[account] = TRUST_EXCHANGE_COUNTS.get(account, 0) + 1 + if account == SAYS_NO_ACCOUNT: + return self._send(200, b'{"trusted": false}') trusted = ( account == TRUST_ACCOUNT and issuer == TRUSTED_ISSUER - and subject.startswith(TRUSTED_SUBJECT_PREFIX) + and subject == TRUSTED_SUBJECT ) self._send(200 if trusted else 403, json.dumps({"trusted": trusted}).encode()) diff --git a/tests/test_person_route.py b/tests/test_person_route.py new file mode 100644 index 00000000..f5c03c07 --- /dev/null +++ b/tests/test_person_route.py @@ -0,0 +1,62 @@ +"""The person issuer's STS route, with a validly signed token. + +CI's main worker has no person issuer that mints anything. A second worker +(PERSON_PROXY_URL in ci.yml) names GitHub Actions as AUTH_ISSUER instead, so +CI's GitHub token is a person token there: it exchanges at `_default` and acts +as its own subject. That worker's PLATFORM_ISSUERS also names GitHub, which +the proxy drops at load, so these tests also pin that the person route wins. +""" + +import os +import xml.etree.ElementTree as ET + +import pytest +import requests + +from test_writes import ID_TOKEN, WRONG_AUD_TOKEN + +PERSON_PROXY_URL = os.environ.get("PERSON_PROXY_URL") + +pytestmark = pytest.mark.skipif( + not PERSON_PROXY_URL, reason="person-issuer worker not configured (set PERSON_PROXY_URL)" +) + + +def exchange(token): + params = {"Action": "AssumeRoleWithWebIdentity", "RoleArn": "_default", "WebIdentityToken": token} + return requests.post(f"{PERSON_PROXY_URL}/.sts", data=params) + + +def test_a_person_token_gets_credentials_that_sign(): + assert ID_TOKEN, "PERSON_PROXY_URL is set but CI_WRITE_ID_TOKEN is not" + import boto3 + from botocore.config import Config + + resp = exchange(ID_TOKEN) + assert resp.status_code == 200, resp.text[:300] + fields = {el.tag.rpartition("}")[2]: el.text for el in ET.fromstring(resp.text).iter()} + client = boto3.client( + "s3", + endpoint_url=PERSON_PROXY_URL, + aws_access_key_id=fields["AccessKeyId"], + aws_secret_access_key=fields["SecretAccessKey"], + aws_session_token=fields["SessionToken"], + region_name="us-east-1", + config=Config(s3={"addressing_style": "path"}), + ) + # A product-scoped list runs SigV4 verification, which unseals the token. + client.list_objects_v2(Bucket="cholmes", Prefix="admin-boundaries/", MaxKeys=1) + + +def test_a_person_token_for_another_audience_is_refused(): + assert WRONG_AUD_TOKEN, "PERSON_PROXY_URL is set but CI_WRONG_AUDIENCE_TOKEN is not" + resp = exchange(WRONG_AUD_TOKEN) + assert resp.status_code == 400, resp.text[:300] + assert "InvalidIdentityToken" in resp.text + + +def test_a_person_token_with_a_tampered_signature_is_refused(): + assert ID_TOKEN, "PERSON_PROXY_URL is set but CI_WRITE_ID_TOKEN is not" + tampered = ID_TOKEN[:-1] + ("A" if ID_TOKEN[-1] != "A" else "B") + resp = exchange(tampered) + assert resp.status_code == 400, resp.text[:300] diff --git a/tests/test_platform_trust.py b/tests/test_platform_trust.py index 461ee912..b8bc7278 100644 --- a/tests/test_platform_trust.py +++ b/tests/test_platform_trust.py @@ -16,7 +16,7 @@ import pytest import requests -from stub_api import TRUST_ACCOUNT, TRUSTED_ISSUER, WRITE_ACCOUNT +from stub_api import SAYS_NO_ACCOUNT, TRUST_ACCOUNT, TRUSTED_ISSUER, WRITE_ACCOUNT from test_writes import ID_TOKEN, PROXY_URL, needs_token STUB_URL = "http://localhost:9000" @@ -144,6 +144,13 @@ def test_a_token_read_from_a_file_may_end_in_a_newline(): assert resp.status_code == 200, resp.text[:300] +@needs_token +def test_a_200_that_says_no_is_still_a_refusal(): + resp = exchange(ID_TOKEN, as_account(SAYS_NO_ACCOUNT)) + assert resp.status_code == 403, resp.text[:300] + assert sts_fields(resp)["Code"] == "AccessDenied" + + @needs_token def test_an_account_that_does_not_trust_the_workflow_refuses_it(): untrusting = as_account("ci-tests--someone-else") From 2ee443125b85531d3daf2d53d7f3d9990737430a Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 30 Sep 2026 23:37:39 -0700 Subject: [PATCH 17/19] ci(staging): mint the smoke-test token only with a trust account With FEDERATION_TEST_AUDIENCE set but FEDERATION_TEST_TRUST_ACCOUNT unset, the token named the stub's account, which staging lacks, and the copy-source test failed instead of skipping. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/staging.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index a8e24df2..f7564546 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -62,9 +62,10 @@ jobs: # origin, the audience staging's PLATFORM_ISSUERS accepts for GitHub, # and FEDERATION_TEST_TRUST_ACCOUNT to a staging service account that # trusts this repository's workflows (ADR-014): the token acts as that - # account. Until then no token is minted and the copy-source authz - # test skips; the rest of the suite is unaffected. - if: vars.FEDERATION_TEST_AUDIENCE != '' + # account. Until both are set no token is minted and the copy-source + # authz test skips; the rest of the suite is unaffected. (A token with + # no trust account would name the stub's, which staging lacks.) + if: vars.FEDERATION_TEST_AUDIENCE != '' && vars.FEDERATION_TEST_TRUST_ACCOUNT != '' run: | set -euo pipefail token=$(curl -sSf -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ From dc8b141b4c41dc65c5aebccd49078fa965e89098 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 1 Oct 2026 00:18:56 -0700 Subject: [PATCH 18/19] ci: give the person-issuer worker its own dev host wrangler.toml's [dev] host is localhost:8787, so the second worker verified SigV4 against the wrong Host and refused the credentials it had just minted. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 488165d0..6f22479f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -185,7 +185,9 @@ jobs: # A second worker whose person issuer is GitHub, so CI's token # exercises the person route (tests/test_person_route.py). Its # PLATFORM_ISSUERS still names GitHub, which the proxy drops at load. - wrangler dev --port 8788 --inspector-port 9230 --persist-to /tmp/wrangler-person \ + # --host overrides wrangler.toml's localhost:8787, which SigV4 would + # otherwise be verified against. + wrangler dev --port 8788 --host localhost:8788 --inspector-port 9230 --persist-to /tmp/wrangler-person \ --var AUTH_ISSUER:https://token.actions.githubusercontent.com \ --var AUTH_AUDIENCE:source-data-proxy-ci > /tmp/wrangler-person.log 2>&1 & curl --retry 120 --retry-delay 1 --retry-connrefused --silent --fail http://localhost:8787/ > /dev/null From d8613f7dad8c799d8cb5215c47e71cb93b7ec19c Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 1 Oct 2026 09:06:06 -0700 Subject: [PATCH 19/19] refactor(sts): inline the key id lookup Co-Authored-By: Claude Opus 5.5 --- src/lib.rs | 4 +++- src/platform.rs | 8 -------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8acc43e5..903f1331 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -806,7 +806,9 @@ async fn platform_exchange( tracing::warn!(%request_id, %issuer, %account, "RoleArn names no service account"); return Err(not_authorized()); } - let kid = platform::kid(&header).map_err(failed)?; + let kid = header["kid"] + .as_str() + .ok_or_else(|| failed(ProxyError::InvalidOidcToken("JWT missing kid".into())))?; let keys = platform_keys(issuer, kid).await.map_err(failed)?; let subject = platform::verify(&sts.web_identity_token, kid, &keys, issuer, &role).map_err(failed)?; diff --git a/src/platform.rs b/src/platform.rs index d2b8e437..15f5b44b 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -57,14 +57,6 @@ pub fn unverified(token: &str) -> Option<(Value, Value)> { Some((segments.next()??, segments.next()??)) } -/// The id of the key a token's header says signed it. -pub fn kid(header: &Value) -> Result<&str, ProxyError> { - header - .get("kid") - .and_then(Value::as_str) - .ok_or_else(|| ProxyError::InvalidOidcToken("JWT missing kid".into())) -} - /// Verify a platform issuer's token against `keys`, the issuer's published /// keys, as the STS route verifies the person issuer's (signature, issuer, the /// audiences `role` requires, `exp` and `nbf`) and return its subject.