diff --git a/README.md b/README.md index f502c343..15a83e1f 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,18 @@ Set in `wrangler.toml` or via the Cloudflare dashboard: A service account's API key (ADR-013) is an opaque `sck_` secret that source.coop stores as a hash. It is presented at `/.sts` as `WebIdentityToken`, from a POST form body only — a key in the URL is refused, because the URL is logged. The proxy trims it and checks its shape and checksum (the last six characters are a CRC-32 of the thirty random ones before them, in base62), hashes it, and asks `POST {SOURCE_API_URL}/api/v1/service-account-keys/exchanges` for its standing as itself (subject `urn:source:data-proxy`), caching the answer for 60 seconds; then it mints credentials for the account the API names, exactly as it would for an ID token. A key that fails its shape or checksum was cut short or mistyped, and is refused as such without a lookup; every other refusal of the key reads `API key was not accepted (request id …)`, and the reason is in the log under that id. +### 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): + +| Role | Credentials may | +| ------------ | -------------------------------------------------------- | +| `FullAccess` | do everything the account's memberships allow | +| `ReadOnly` | do the same, except write | +| `_default` | do what `FullAccess` does; the name existing clients use | + +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. + ### Secrets **GitHub environment secrets are the source of truth.** The deploy workflow diff --git a/adrs/001-s3-credentials.md b/adrs/001-s3-credentials.md index 1523ca11..0078274c 100644 --- a/adrs/001-s3-credentials.md +++ b/adrs/001-s3-credentials.md @@ -57,16 +57,16 @@ The sealed payload carries: | `access_key_id` | The identifier the caller signs with | | `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 (currently always `_default`) | +| `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 | -| `allowed_scopes` | Scope ceiling sealed at mint time — currently empty, and not consulted on this path (see below) | +| `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 | 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 sealed but not enforced on this path.** Its only consumer, `multistore::auth::authorize`, has no call site in the pinned crate; the gateway delegates authorization to the bucket registry instead (ADR-005). Where scopes *are* evaluated, an empty vec means **deny-all**, not unlimited — which is why the registry overrides `authorize_key` rather than inheriting the default. The effective behaviour is "no ceiling", but by bypass rather than by an empty-means-unlimited rule. ADR-011 is where this field would become load-bearing, and wiring it up is part of that work rather than a given. +- **`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). - **Authenticated encryption.** GCM provides integrity as well as confidentiality: a tampered token fails to decrypt rather than decoding into attacker-chosen values. diff --git a/adrs/004-sts.md b/adrs/004-sts.md index 5909d173..66767963 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)) · 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) · source.coop#283 (OIDC auth), source.coop#391 (in-browser uploads via the proxy), source.coop#402 (mid-upload credential refresh) --- @@ -75,6 +75,8 @@ A single built-in Role, `_default`, is served from a hardcoded registry: > [!NOTE] > **Amended by ADR-014 (Service Accounts).** For a platform issuer's token, the account portion of `RoleArn` is no longer ignored: it names the service account whose trust in the token's issuer and subject is checked, and the credentials act as that account. For an Ory ID token it is still ignored, because the token itself names the person. +> +> Two more hardcoded Roles are served alongside it (ADR-014, #236): `FullAccess`, of which `_default` is now an alias, and `ReadOnly`, whose read-only ceiling is sealed into the session and enforced by the bucket registry (ADR-011). All three take the same ARN form. ### Trust Model — Issuer and Audience diff --git a/adrs/011-role-ceiling-authorization.md b/adrs/011-role-ceiling-authorization.md index 7a021647..1486e4a8 100644 --- a/adrs/011-role-ceiling-authorization.md +++ b/adrs/011-role-ceiling-authorization.md @@ -1,6 +1,6 @@ # ADR-011: Role-Ceiling Authorization -**Status:** Proposed — not implemented +**Status:** Proposed — implemented in part (#236: step 2 and the denial semantics, for the hardcoded `ReadOnly` Role's action ceiling) **Date:** 2026-08-09 **RFC:** RFC-001 §8 **Depends on:** ADR-005, ADR-010 diff --git a/src/authz.rs b/src/authz.rs index 54cef0e0..b812c7a1 100644 --- a/src/authz.rs +++ b/src/authz.rs @@ -1,14 +1,15 @@ -//! Authorization for product backends: write-action classification and the -//! authorization → federation decision ([`decide_backend_auth`]). Kept wasm-free -//! so both can be unit-tested natively (see `tests/authz.rs`), despite the -//! crate's `[lib] test = false`. +//! Authorization for product backends: write-action classification, the Role +//! ceiling ([`ceiling_permits`]) and the authorization → federation decision +//! ([`decide_backend_auth`]). Kept wasm-free so all three can be unit-tested +//! natively (see `tests/authz.rs`), despite the crate's `[lib] test = false`. use std::collections::HashMap; use multistore::error::ProxyError; -use multistore::types::Action; +use multistore::types::{AccessScope, Action}; use crate::backend_auth::{apply_backend_auth, BackendAuth}; +use crate::sts::ALL_PRODUCTS; /// Whether an S3 action mutates the backend. Reads (GET/HEAD/LIST) are served /// without a write check; everything else is a write and must be authorized. @@ -24,6 +25,23 @@ pub(crate) fn is_write_action(action: Action) -> bool { ) } +/// Whether the Role ceiling sealed into a session (ADR-011) allows `action`. +/// Checked before anything is fetched, and it only subtracts: what it allows +/// still needs the account's own permissions. +/// +/// No scopes means no ceiling: `FullAccess` and `_default` seal none. Otherwise +/// a scope must name every product ([`ALL_PRODUCTS`]) with no prefix and list +/// the action. The proxy seals nothing narrower, so a narrower scope is refused +/// rather than guessed at. +pub(crate) fn ceiling_permits(scopes: &[AccessScope], action: Action) -> bool { + scopes.is_empty() + || scopes.iter().any(|scope| { + scope.bucket == ALL_PRODUCTS + && scope.prefixes.is_empty() + && scope.actions.contains(&action) + }) +} + /// Authorize a resolved product's request and, only on success, translate the /// connection's backend authentication into multistore `backend_options`. This /// is the single authorization → federation seam: `resolve_product` performs the diff --git a/src/lib.rs b/src/lib.rs index 94554a32..e0f58572 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -581,9 +581,14 @@ async fn exchange_api_key( tracing::warn!(%request_id, reason = "malformed", "API key exchange refused"); return Err(ProxyError::InvalidOidcToken("malformed".into())); }; - if !sts::is_default_role(&sts.role_arn) { + let Some(role) = sts::role( + &sts.role_arn, + config.auth_issuer.clone(), + config.auth_audiences.clone(), + config.sts_max_session_duration_secs, + ) else { return Err(ProxyError::RoleNotFound(sts.role_arn.clone())); - } + }; let key_hash = keys::key_hash(key); let standing = source_api::cache::get_or_fetch_key_standing( &config.api_base_url, @@ -609,18 +614,13 @@ async fn exchange_api_key( return Err(ProxyError::InvalidOidcToken("inactive".into())); } }; - let role = sts::default_role( - config.auth_issuer.clone(), - config.auth_audiences.clone(), - config.sts_max_session_duration_secs, - ); let creds = keys::credentials_for( &role, &account_id, sts.duration_seconds, &config.session_token_key, )?; - tracing::info!(%request_id, key_id, %account_id, "API key exchanged"); + tracing::info!(%request_id, key_id, %account_id, role = %role.role_id, "API key exchanged"); Ok(creds) } diff --git a/src/source_api/registry.rs b/src/source_api/registry.rs index 063d2d17..f7e93dca 100644 --- a/src/source_api/registry.rs +++ b/src/source_api/registry.rs @@ -5,7 +5,7 @@ use multistore::error::ProxyError; use multistore::registry::{BucketRegistry, ResolvedBucket}; use multistore::types::{Action, BucketConfig, ResolvedIdentity, S3Operation}; -use crate::authz::{decide_backend_auth, is_write_action}; +use crate::authz::{ceiling_permits, decide_backend_auth, is_write_action}; /// Registry that resolves Source Cooperative products to multistore `BucketConfig`s /// by calling the Source Cooperative API. @@ -56,7 +56,22 @@ impl BucketRegistry for SourceCoopRegistry { .ok_or_else(|| ProxyError::BucketNotFound(name.to_string()))?; let subject = match identity { - ResolvedIdentity::Authenticated(auth) => Some(auth.principal_name.as_str()), + ResolvedIdentity::Authenticated(auth) => { + // The Role ceiling goes first and is local (ADR-011): a session + // whose Role does not allow the action is refused before + // anything is fetched, with the AccessDenied every other + // refusal gets, so the answer says nothing about the product. + // Only this log line records why. + if !ceiling_permits(&auth.allowed_scopes, operation.action()) { + tracing::info!( + principal = %auth.principal_name, + action = ?operation.action(), + "refused by the Role ceiling" + ); + return Err(ProxyError::AccessDenied); + } + Some(auth.principal_name.as_str()) + } ResolvedIdentity::Anonymous => None, }; diff --git a/src/sts.rs b/src/sts.rs index 4290aa9d..58e895b3 100644 --- a/src/sts.rs +++ b/src/sts.rs @@ -1,24 +1,30 @@ //! STS credential registry for token exchange. //! -//! Provides a hardcoded `_default` role that trusts the Source Cooperative auth -//! provider, enabling clients to exchange OIDC tokens for temporary S3-style credentials. +//! Serves the hardcoded Roles (ADR-014): `FullAccess`, everything the caller's +//! memberships allow, and `ReadOnly`, the same with writing removed. `_default` +//! is `FullAccess` under the name existing clients already use. There is no +//! lookup: account-owned Roles (ADR-010) are deferred, and a Role only ever +//! subtracts from the account's own permissions, so any caller may name either. use multistore::error::ProxyError; use multistore::registry::CredentialRegistry; -use multistore::types::{RoleConfig, StoredCredential}; +use multistore::types::{AccessScope, Action, RoleConfig, StoredCredential}; -/// Credential registry that serves a single hardcoded `_default` role. -/// -/// The default role trusts the Source Cooperative auth provider with no scope -/// restrictions, so any user holding a token for one of the configured -/// audiences (`required_audiences`) can obtain temporary credentials. +/// The bucket a Role's scope names to cover every product. Only the proxy's +/// registry reads scopes — multistore's own scope check never runs on this +/// gateway — so the wildcard means what `authz::ceiling_permits` says it does. +pub(crate) const ALL_PRODUCTS: &str = "*"; + +/// Credential registry that serves the hardcoded Roles. #[derive(Clone)] pub struct StsCredentialRegistry { - default_role: RoleConfig, + oidc_issuer: String, + required_audiences: Vec, + max_session_duration_secs: u64, } impl StsCredentialRegistry { - /// Create a new registry whose `_default` role trusts the given auth issuer. + /// Create a new registry whose Roles trust the given auth issuer. /// /// `required_audiences` restricts token exchange to subject tokens minted /// for one of these OAuth clients (the `aud` claim); a token is accepted if @@ -36,29 +42,62 @@ impl StsCredentialRegistry { max_session_duration_secs: u64, ) -> Self { Self { - default_role: default_role(oidc_issuer, required_audiences, max_session_duration_secs), + oidc_issuer, + required_audiences, + max_session_duration_secs, } } } -/// The `_default` role: trusts `oidc_issuer` for tokens minted for one of -/// `required_audiences`, with no scope restriction. Shared with the API-key -/// exchange, which mints under the same role once the API has named the -/// account (`keys::credentials_for`). -pub(crate) fn default_role( +/// The Role `role_arn` names, trusting `oidc_issuer` for tokens minted for one +/// of `required_audiences`; `None` for a name the proxy does not serve. Never a +/// fallback: a workload that asks for a Role it cannot have fails at exchange +/// rather than receiving different access than it asked for. Shared with the +/// API-key exchange, which mints under the named Role once the API has named +/// the account (`keys::credentials_for`). +pub(crate) fn role( + role_arn: &str, oidc_issuer: String, required_audiences: Vec, max_session_duration_secs: u64, -) -> RoleConfig { - RoleConfig { - role_id: "_default".to_string(), - name: "Default".to_string(), +) -> Option { + let name = role_name(role_arn)?; + let allowed_scopes = match name { + // No scopes, no ceiling: the account's permissions are the only limit. + "FullAccess" | "_default" => vec![], + // Sealed into the session; `authz::ceiling_permits` enforces it. + "ReadOnly" => vec![AccessScope { + bucket: ALL_PRODUCTS.to_string(), + prefixes: vec![], + actions: vec![Action::GetObject, Action::HeadObject, Action::ListBucket], + }], + _ => return None, + }; + Some(RoleConfig { + role_id: name.to_string(), + name: name.to_string(), trusted_oidc_issuers: vec![oidc_issuer], required_audiences, subject_conditions: vec![], - allowed_scopes: vec![], // unlimited + allowed_scopes, max_session_duration_secs, + }) +} + +/// The Role name in `role_arn`: a bare name, or the `role/` resource of +/// an ARN of any partition and account, such as +/// `arn:aws:iam::000000000000:role/ReadOnly`. +/// +/// The ARN form exists because AWS SDKs validate `RoleArn` client-side (ARN +/// shape, 20-character minimum) before the request is ever sent, so a bare name +/// can't reach the server from standard tooling (see +/// source-cooperative/data.source.coop#184). The partition and account carry no +/// meaning for the Role itself, so they are ignored rather than validated. +fn role_name(role_arn: &str) -> Option<&str> { + if !role_arn.starts_with("arn:") { + return Some(role_arn); } + role_arn.splitn(6, ':').nth(5)?.strip_prefix("role/") } impl CredentialRegistry for StsCredentialRegistry { @@ -71,29 +110,11 @@ impl CredentialRegistry for StsCredentialRegistry { } async fn get_role(&self, role_id: &str) -> Result, ProxyError> { - // TODO: Eventually look up roles via the Source Cooperative API so that - // individual repositories can define custom roles with fine-grained - // scope and subject restrictions (e.g. per-repo CI/CD access). - // For now, only the hardcoded `_default` role is supported. - if is_default_role(role_id) { - Ok(Some(self.default_role.clone())) - } else { - Ok(None) - } + Ok(role( + role_id, + self.oidc_issuer.clone(), + self.required_audiences.clone(), + self.max_session_duration_secs, + )) } } - -/// Whether `role_id` names the `_default` role — literally, or via an -/// ARN-shaped alias whose resource is `role/_default` (any partition/account, -/// e.g. `arn:aws:iam::000000000000:role/_default`). -/// -/// The alias exists because AWS SDKs validate `RoleArn` client-side (ARN shape, -/// 20-character minimum) before the request is ever sent, so a bare `_default` -/// can't reach the server from standard tooling. Accepting the alias keeps -/// `/.sts` a drop-in `AssumeRoleWithWebIdentity` target for unmodified SDKs -/// (see source-cooperative/data.source.coop#184). Same role, same trust model — -/// only the name is longer; the partition/account portion is ignored rather -/// than validated because it carries no meaning here. -pub(crate) fn is_default_role(role_id: &str) -> bool { - role_id == "_default" || (role_id.starts_with("arn:") && role_id.ends_with(":role/_default")) -} diff --git a/tests/authz.rs b/tests/authz.rs index 9f9e2df1..1aaaae28 100644 --- a/tests/authz.rs +++ b/tests/authz.rs @@ -2,19 +2,21 @@ //! (the lib itself is `cdylib` with `test = false`). Mirrors the pattern in //! `tests/backend_auth.rs`. //! -//! `authz` references `crate::backend_auth`, so that module is pulled in here too -//! (under the test crate root) so the `crate::` path resolves the same way it -//! does in the lib build. +//! `authz` references `crate::backend_auth` and `crate::sts`, so those modules +//! are pulled in here too (under the test crate root) so the `crate::` paths +//! resolve the same way they do in the lib build. #[path = "../src/authz.rs"] mod authz; #[path = "../src/backend_auth.rs"] mod backend_auth; +#[path = "../src/sts.rs"] +mod sts; -use authz::{decide_backend_auth, is_write_action}; +use authz::{ceiling_permits, decide_backend_auth, is_write_action}; use backend_auth::BackendAuth; use multistore::error::ProxyError; -use multistore::types::Action; +use multistore::types::{AccessScope, Action}; use std::collections::HashMap; #[test] @@ -38,6 +40,72 @@ fn mutations_are_writes() { } } +// ── ceiling_permits: the Role ceiling (ADR-011) ───────────────────────────── + +const EVERY_ACTION: [Action; 10] = [ + Action::GetObject, + Action::GetObjectVersion, + Action::HeadObject, + Action::PutObject, + Action::ListBucket, + Action::CreateMultipartUpload, + Action::UploadPart, + Action::CompleteMultipartUpload, + Action::AbortMultipartUpload, + Action::DeleteObject, +]; + +/// The scopes a Role seals into every session it mints. +fn sealed(role: &str) -> Vec { + sts::role(role, "https://auth.example.test".into(), vec![], 3600) + .unwrap() + .allowed_scopes +} + +/// The ceiling and the write gate must agree on what a read is, or ReadOnly +/// either refuses a read or lets a write through. +#[test] +fn read_only_allows_exactly_the_reads() { + let read_only = sealed("ReadOnly"); + for action in EVERY_ACTION { + assert_eq!( + ceiling_permits(&read_only, action), + !is_write_action(action), + "{action:?}" + ); + } +} + +#[test] +fn full_access_and_its_alias_have_no_ceiling() { + for role in ["FullAccess", "_default"] { + let scopes = sealed(role); + for action in EVERY_ACTION { + assert!(ceiling_permits(&scopes, action), "{role} {action:?}"); + } + } +} + +/// Nothing the proxy mints is narrower than every product, so a scope that is +/// must not be read as if it were. +#[test] +fn a_scope_narrower_than_every_product_permits_nothing() { + for scope in [ + AccessScope { + bucket: "acme:data".into(), + prefixes: vec![], + actions: vec![Action::GetObject], + }, + AccessScope { + bucket: "*".into(), + prefixes: vec!["public/".into()], + actions: vec![Action::GetObject], + }, + ] { + assert!(!ceiling_permits(&[scope], Action::GetObject)); + } +} + // ── decide_backend_auth: authorization → federation ordering (#142) ───────── // // The invariant under test: an unauthorized request must be denied *before* any diff --git a/tests/keys.rs b/tests/keys.rs index e7414609..bece7d9e 100644 --- a/tests/keys.rs +++ b/tests/keys.rs @@ -98,14 +98,21 @@ fn token_key() -> TokenKey { TokenKey::from_base64("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=").unwrap() } -fn role(cap: u64) -> multistore::types::RoleConfig { - sts::default_role("https://auth.example.test".into(), vec!["aud".into()], cap) +fn role(name: &str, cap: u64) -> multistore::types::RoleConfig { + sts::role( + name, + "https://auth.example.test".into(), + vec!["aud".into()], + cap, + ) + .unwrap() } #[test] fn credentials_are_sealed_for_the_account_within_floor_and_cap() { let key = token_key(); - let creds = credentials_for(&role(43_200), "acme--nightly-sync", None, &key).unwrap(); + let creds = + credentials_for(&role("_default", 43_200), "acme--nightly-sync", None, &key).unwrap(); assert_eq!(creds.source_identity, "acme--nightly-sync"); assert_eq!(creds.assumed_role_id, "_default"); assert!(creds.access_key_id.starts_with("STSPRXY")); @@ -114,14 +121,27 @@ fn credentials_are_sealed_for_the_account_within_floor_and_cap() { assert_eq!(unsealed.source_identity, "acme--nightly-sync"); let now = chrono_now(); - let default = credentials_for(&role(43_200), "a", None, &key).unwrap(); + let default = credentials_for(&role("_default", 43_200), "a", None, &key).unwrap(); assert!((default.expiration.timestamp() - now - 3600).abs() <= 2); - let floored = credentials_for(&role(43_200), "a", Some(1), &key).unwrap(); + let floored = credentials_for(&role("_default", 43_200), "a", Some(1), &key).unwrap(); assert!((floored.expiration.timestamp() - now - 900).abs() <= 2); - let capped = credentials_for(&role(3_600), "a", Some(86_400), &key).unwrap(); + let capped = credentials_for(&role("_default", 3_600), "a", Some(86_400), &key).unwrap(); assert!((capped.expiration.timestamp() - now - 3600).abs() <= 2); } +#[test] +fn credentials_carry_the_named_roles_ceiling() { + let key = token_key(); + let read_only = role("ReadOnly", 3_600); + let creds = credentials_for(&read_only, "acme--nightly-sync", None, &key).unwrap(); + let unsealed = key.unseal(&creds.session_token).unwrap().unwrap(); + assert_eq!(unsealed.assumed_role_id, "ReadOnly"); + assert_eq!( + serde_json::to_value(&unsealed.allowed_scopes).unwrap(), + serde_json::to_value(&read_only.allowed_scopes).unwrap() + ); +} + fn chrono_now() -> i64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) diff --git a/tests/sts.rs b/tests/sts.rs index 182fa7ad..7077edaa 100644 --- a/tests/sts.rs +++ b/tests/sts.rs @@ -1,44 +1,68 @@ -//! Native unit tests for the `_default` role alias in `sts`, included via -//! `#[path]` (the lib itself is `cdylib` with `test = false`). Mirrors the -//! pattern in `tests/backend_auth.rs`. +//! Native unit tests for Role lookup in `sts`, included via `#[path]` (the lib +//! itself is `cdylib` with `test = false`). Mirrors the pattern in +//! `tests/backend_auth.rs`. What each Role's ceiling allows is pinned in +//! `tests/authz.rs`, next to the check that enforces it. #[path = "../src/sts.rs"] mod sts; -use sts::is_default_role; - -#[test] -fn literal_default_accepted() { - assert!(is_default_role("_default")); +/// The Role `role_arn` resolves to, by id. +fn named(role_arn: &str) -> Option { + sts::role( + role_arn, + "https://auth.example.test".into(), + vec!["aud".into()], + 3600, + ) + .map(|role| role.role_id) } #[test] -fn arn_alias_accepted_for_any_partition_and_account() { - assert!(is_default_role("arn:aws:iam::000000000000:role/_default")); - assert!(is_default_role("arn:aws:iam::123456789012:role/_default")); - assert!(is_default_role( - "arn:aws-us-gov:iam::123456789012:role/_default" - )); +fn each_role_is_served_by_its_bare_name() { + for name in ["FullAccess", "ReadOnly", "_default"] { + assert_eq!(named(name).as_deref(), Some(name)); + } } #[test] -fn other_roles_rejected() { - assert!(!is_default_role("")); - assert!(!is_default_role("_other")); - assert!(!is_default_role("default")); - assert!(!is_default_role("arn:aws:iam::123456789012:role/other")); +fn arn_forms_are_accepted_for_any_partition_and_account() { + for arn in [ + "arn:aws:iam::000000000000:role/ReadOnly", + "arn:aws:iam::123456789012:role/ReadOnly", + "arn:aws-us-gov:iam::123456789012:role/ReadOnly", + // A service account's id, as source.coop's GitHub snippet names it. + "arn:aws:iam::acme--nightly-sync:role/ReadOnly", + ] { + assert_eq!(named(arn).as_deref(), Some("ReadOnly"), "{arn}"); + } + assert_eq!( + named("arn:aws:iam::acme--nightly-sync:role/FullAccess").as_deref(), + Some("FullAccess") + ); + // Deployed client configuration uses this one. + assert_eq!( + named("arn:aws:iam::000000000000:role/_default").as_deref(), + Some("_default") + ); } #[test] -fn alias_requires_arn_prefix_and_exact_resource() { - // No arn: prefix. - assert!(!is_default_role("role/_default")); - // Pathed resource is not the `_default` role. - assert!(!is_default_role( - "arn:aws:iam::123456789012:role/team/_default" - )); - // `_default` as a suffix of another role name. - assert!(!is_default_role( - "arn:aws:iam::123456789012:role/not_default" - )); +fn unknown_names_are_refused_not_defaulted() { + for arn in [ + "", + "default", + "readonly", + "Admin", + "arn:aws:iam::123456789012:role/other", + // No arn: prefix. + "role/_default", + // A pathed resource is not the Role. + "arn:aws:iam::123456789012:role/team/_default", + // A Role's name as the suffix of another. + "arn:aws:iam::123456789012:role/not_default", + // Not a role resource. + "arn:aws:iam::123456789012:user/ReadOnly", + ] { + assert_eq!(named(arn), None, "{arn}"); + } } diff --git a/tests/test_api_keys.py b/tests/test_api_keys.py index bfccfea5..b0f669c8 100644 --- a/tests/test_api_keys.py +++ b/tests/test_api_keys.py @@ -4,15 +4,26 @@ itself, then mints credentials for the account the stub names. These tests pin the parts that only run in the worker — the form-body-only rule, the uniform refusal, the 60s standing cache, and fail-closed on an API error — -by counting how often each key's standing reaches the stub. +by counting how often each key's standing reaches the stub, and the ceiling of +the Role a key is exchanged for. """ import re +import uuid import xml.etree.ElementTree as ET +import pytest import requests -from stub_api import ERR_500_KEY, KEY_ACCOUNT, LIVE_KEY, REVOKED_KEY, UNKNOWN_KEY, _hash +from stub_api import ( + ERR_500_KEY, + KEY_ACCOUNT, + LIVE_KEY, + REVOKED_KEY, + UNKNOWN_KEY, + WRITE_ACCOUNT, + _hash, +) PROXY_URL = "http://localhost:8787" STUB_URL = "http://localhost:9000" @@ -129,6 +140,41 @@ def test_a_wrong_role_is_reported_as_such(): assert sts_fields(resp)["Code"] == "MalformedPolicyDocument" +def test_read_only_refuses_a_write_before_anything_is_looked_up(): + """ReadOnly's ceiling is checked locally, ahead of every lookup, so it + refuses a write with AccessDenied even for a product the API has never + heard of. FullAccess, and ReadOnly reading, get past it to the product + lookup, which finds no such product. (A write that reached the upstream + would fail closed in CI anyway, so the lookup is where to tell them apart.)""" + import boto3 + from botocore.config import Config + from botocore.exceptions import ClientError + + def client(role): + fields = sts_fields(exchange(LIVE_KEY, role=f"arn:aws:iam::000000000000:role/{role}")) + return 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"}), + ) + + def error_code(call): + with pytest.raises(ClientError) as exc: + call() + return exc.value.response["Error"]["Code"] + + key = f"no-such-product-{uuid.uuid4().hex}/x.txt" + read_only, full_access = client("ReadOnly"), client("FullAccess") + put = {"Bucket": WRITE_ACCOUNT, "Key": key, "Body": b"x"} + assert error_code(lambda: read_only.put_object(**put)) == "AccessDenied" + assert error_code(lambda: full_access.put_object(**put)) == "NoSuchBucket" + assert error_code(lambda: read_only.get_object(Bucket=WRITE_ACCOUNT, Key=key)) == "NoSuchBucket" + + def test_an_api_failure_fails_closed_and_is_not_cached(): first = exchange(ERR_500_KEY) assert first.status_code == 500