From 7b578408e914e912a9c91b3eba74ef4ca9961726 Mon Sep 17 00:00:00 2001 From: Jeremi Joslin Date: Fri, 17 Jul 2026 18:26:30 +0700 Subject: [PATCH] fix(auth): reject ambiguous primary credentials Signed-off-by: Jeremi Joslin --- .../src/standalone/auth/credentials.rs | 19 +- .../src/standalone/offline_fixture.rs | 1 + .../src/standalone/tests/auth.inc | 81 +++++ .../tests/standalone_http/auth.rs | 101 ++++++ .../openapi/registry-relay.openapi.json | 69 +++- crates/registry-relay/src/api/consultation.rs | 6 + crates/registry-relay/src/api/openapi.rs | 74 +++- crates/registry-relay/src/audit/mod.rs | 10 +- crates/registry-relay/src/auth/api_key.rs | 48 ++- crates/registry-relay/src/auth/middleware.rs | 7 + .../registry-relay/src/auth/oidc/provider.rs | 40 ++- crates/registry-relay/src/error.rs | 15 +- crates/registry-relay/src/server.rs | 18 +- crates/registry-relay/tests/audit_record.rs | 319 +++++++++++++++++- crates/registry-relay/tests/auth_flow.rs | 82 +++++ crates/registry-relay/tests/error_taxonomy.rs | 2 + 16 files changed, 837 insertions(+), 55 deletions(-) diff --git a/crates/registry-notary-server/src/standalone/auth/credentials.rs b/crates/registry-notary-server/src/standalone/auth/credentials.rs index 50d3d75fe..190e15314 100644 --- a/crates/registry-notary-server/src/standalone/auth/credentials.rs +++ b/crates/registry-notary-server/src/standalone/auth/credentials.rs @@ -30,6 +30,7 @@ impl std::fmt::Debug for ResolvedCredential { #[derive(Debug, Clone, Default)] pub(in super::super) struct RequestCredentials { pub(in super::super) api_key: Option, + pub(in super::super) api_key_present: bool, pub(in super::super) authorization_present: bool, pub(in super::super) bearer_token: Option, pub(in super::super) id_token: Option, @@ -37,12 +38,13 @@ pub(in super::super) struct RequestCredentials { impl RequestCredentials { pub(in super::super) fn credential_type_count(&self) -> usize { - usize::from(self.api_key.is_some()) + usize::from(self.api_key_present || self.api_key.is_some()) + usize::from(self.authorization_present || self.bearer_token.is_some()) } pub(in super::super) fn are_absent(&self) -> bool { self.api_key.is_none() + && !self.api_key_present && !self.authorization_present && self.bearer_token.is_none() && self.id_token.is_none() @@ -105,22 +107,19 @@ pub(in super::super) fn resolve_credentials( } pub(in super::super) fn request_credentials(request: &Request) -> RequestCredentials { - let authorization = request - .headers() - .get(header::AUTHORIZATION) - .and_then(header_str); + let headers = request.headers(); + let authorization = headers.get(header::AUTHORIZATION).and_then(header_str); RequestCredentials { - api_key: request - .headers() + api_key: headers .get("x-api-key") .and_then(header_str) .map(ToOwned::to_owned), - authorization_present: authorization.is_some(), + api_key_present: headers.contains_key("x-api-key"), + authorization_present: headers.contains_key(header::AUTHORIZATION), bearer_token: authorization .and_then(|raw| parse_bearer_token(raw).ok()) .map(ToOwned::to_owned), - id_token: request - .headers() + id_token: headers .get(OIDC_ID_TOKEN_HEADER) .and_then(header_str) .map(ToOwned::to_owned), diff --git a/crates/registry-notary-server/src/standalone/offline_fixture.rs b/crates/registry-notary-server/src/standalone/offline_fixture.rs index 4dd323b79..a949da64a 100644 --- a/crates/registry-notary-server/src/standalone/offline_fixture.rs +++ b/crates/registry-notary-server/src/standalone/offline_fixture.rs @@ -419,6 +419,7 @@ impl OfflineNotaryHarness { }; let principal = authenticate_api_key( &RequestCredentials { + api_key_present: api_key.is_some(), api_key, authorization_present: false, bearer_token: None, diff --git a/crates/registry-notary-server/src/standalone/tests/auth.inc b/crates/registry-notary-server/src/standalone/tests/auth.inc index d63664ff8..8ed429945 100644 --- a/crates/registry-notary-server/src/standalone/tests/auth.inc +++ b/crates/registry-notary-server/src/standalone/tests/auth.inc @@ -355,6 +355,7 @@ }; let request = RequestCredentials { api_key: Some("api-token".to_string()), + api_key_present: true, authorization_present: true, bearer_token: Some("bearer-token".to_string()), id_token: None, @@ -382,6 +383,7 @@ }; let request = RequestCredentials { api_key: Some("api-token".to_string()), + api_key_present: true, authorization_present: true, bearer_token: None, id_token: None, @@ -395,22 +397,97 @@ assert!(matches!(err, EvidenceError::MultipleCredentials)); } + #[tokio::test] + async fn request_credentials_counts_non_utf8_authorization_header_presence() { + let authenticator = Authenticator { + api_keys: vec![ResolvedCredential { + id: "api-client".to_string(), + fingerprint: registry_platform_authcommon::fingerprint_api_key("api-token"), + scopes: vec!["farmer_registry:evidence_verification".to_string()], + authorization_details: None, + }], + bearer_tokens: Vec::new(), + oidc: None, + }; + let mut request = Request::builder() + .uri("/v1/claims") + .header("x-api-key", "api-token") + .body(Body::empty()) + .expect("request builds"); + request.headers_mut().insert( + header::AUTHORIZATION, + axum::http::HeaderValue::from_bytes(b"\xff").expect("opaque header value is valid"), + ); + + let credentials = request_credentials(&request); + assert!(credentials.authorization_present); + assert!(credentials.api_key_present); + assert!(credentials.bearer_token.is_none()); + assert_eq!(credentials.api_key.as_deref(), Some("api-token")); + assert_eq!(credentials.credential_type_count(), 2); + + let err = authenticator + .authenticate(credentials, &ReplayStores::memory()) + .await + .expect_err("raw credential channel presence must reject ambiguity"); + assert!(matches!(err, EvidenceError::MultipleCredentials)); + } + + #[tokio::test] + async fn request_credentials_counts_non_utf8_api_key_header_presence() { + let authenticator = Authenticator { + api_keys: Vec::new(), + bearer_tokens: vec![ResolvedCredential { + id: "bearer-client".to_string(), + fingerprint: registry_platform_authcommon::fingerprint_api_key("bearer-token"), + scopes: vec!["farmer_registry:evidence_verification".to_string()], + authorization_details: None, + }], + oidc: None, + }; + let mut request = Request::builder() + .uri("/v1/claims") + .header(header::AUTHORIZATION, "Bearer bearer-token") + .body(Body::empty()) + .expect("request builds"); + request.headers_mut().insert( + "x-api-key", + axum::http::HeaderValue::from_bytes(b"\xff").expect("opaque header value is valid"), + ); + + let credentials = request_credentials(&request); + assert!(credentials.api_key_present); + assert!(credentials.api_key.is_none()); + assert!(credentials.authorization_present); + assert_eq!(credentials.bearer_token.as_deref(), Some("bearer-token")); + assert_eq!(credentials.credential_type_count(), 2); + + let err = authenticator + .authenticate(credentials, &ReplayStores::memory()) + .await + .expect_err("raw credential channel presence must reject ambiguity"); + assert!(matches!(err, EvidenceError::MultipleCredentials)); + } + #[test] fn oidc_id_token_is_supplemental_not_a_separate_auth_mode() { let oidc_request = RequestCredentials { api_key: None, + api_key_present: false, authorization_present: true, bearer_token: Some("access-token".to_string()), id_token: Some("id-token".to_string()), }; let api_key_and_bearer = RequestCredentials { api_key: Some("api-token".to_string()), + api_key_present: true, authorization_present: true, bearer_token: Some("bearer-token".to_string()), id_token: None, }; let api_key_and_malformed_authorization = RequestCredentials { api_key: Some("api-token".to_string()), + api_key_present: true, authorization_present: true, bearer_token: None, id_token: None, @@ -434,6 +511,7 @@ }; let request = RequestCredentials { api_key: Some("api-token".to_string()), + api_key_present: true, authorization_present: false, bearer_token: None, id_token: None, @@ -474,6 +552,7 @@ }; let request = RequestCredentials { api_key: Some("api-token".to_string()), + api_key_present: true, authorization_present: false, bearer_token: None, id_token: None, @@ -942,6 +1021,7 @@ }; let request = RequestCredentials { api_key: Some("api-token".to_string()), + api_key_present: true, authorization_present: false, bearer_token: None, id_token: None, @@ -952,6 +1032,7 @@ let error = authenticate_static( &RequestCredentials { api_key: Some(rejected_secret.to_string()), + api_key_present: true, authorization_present: false, bearer_token: None, id_token: None, diff --git a/crates/registry-notary-server/tests/standalone_http/auth.rs b/crates/registry-notary-server/tests/standalone_http/auth.rs index b4412aa6a..3a488ee7f 100644 --- a/crates/registry-notary-server/tests/standalone_http/auth.rs +++ b/crates/registry-notary-server/tests/standalone_http/auth.rs @@ -6,6 +6,107 @@ use super::{ admin::*, audit::*, credentials::*, federation::*, http_contracts::*, oid4vci::*, preauth::*, }; +#[tokio::test] +pub(super) async fn malformed_authorization_with_valid_api_key_is_candidate_neutral() { + assert_ambiguous_primary_headers_are_candidate_neutral( + axum::http::HeaderValue::from_bytes(b"\xff").expect("opaque header value is valid"), + axum::http::HeaderValue::from_static("api-token"), + ) + .await; +} + +#[tokio::test] +pub(super) async fn valid_authorization_with_non_utf8_api_key_is_candidate_neutral() { + assert_ambiguous_primary_headers_are_candidate_neutral( + axum::http::HeaderValue::from_static("Bearer api-token"), + axum::http::HeaderValue::from_bytes(b"\xff").expect("opaque header value is valid"), + ) + .await; +} + +async fn assert_ambiguous_primary_headers_are_candidate_neutral( + authorization: axum::http::HeaderValue, + api_key: axum::http::HeaderValue, +) { + const API_KEY_MARKER: &str = "api-token"; + const PRINCIPAL_MARKER: &str = "caseworker"; + const BEARER_PRINCIPAL_MARKER: &str = "bearer-caseworker"; + const SCOPE_MARKER: &str = "farmer_registry:evidence_verification"; + + std::env::set_var( + "TEST_EVIDENCE_API_KEY_HASH", + "sha256:a00cf33cd46d9ef96c1eff33df1c9cca20b1a02468cd78ec6a4b2887d1640b51", + ); + let tmp = TempDir::new().expect("tempdir"); + let audit_path = tmp.path().join("audit.jsonl"); + let mut config = notary_only_config( + "http://127.0.0.1:1", + audit_path.to_str().expect("audit path is UTF-8"), + ); + config.auth.bearer_tokens.push(EvidenceCredentialConfig { + id: BEARER_PRINCIPAL_MARKER.to_string(), + fingerprint: env_fingerprint_ref("TEST_EVIDENCE_API_KEY_HASH"), + scopes: vec![SCOPE_MARKER.to_string()], + authorization_details: None, + }); + let app = standalone_router(config) + .await + .expect("standalone router builds"); + let server = TestServer::builder().http_transport().build(app); + + let response = server + .get("/v1/claims") + .add_header("x-api-key", api_key) + .add_header("authorization", authorization) + .await; + + response.assert_status(StatusCode::BAD_REQUEST); + let problem: Value = response.json(); + assert_eq!(problem["code"], json!("auth.multiple_credentials")); + assert_eq!( + problem["detail"], + json!("provide exactly one authentication credential") + ); + assert!(problem.get("data").is_none()); + let rendered = problem.to_string(); + for marker in [ + API_KEY_MARKER, + PRINCIPAL_MARKER, + BEARER_PRINCIPAL_MARKER, + SCOPE_MARKER, + ] { + assert!( + !rendered.contains(marker), + "problem body leaked marker {marker}" + ); + } + + let audit = std::fs::read_to_string(&audit_path).expect("audit was written"); + for marker in [ + API_KEY_MARKER, + PRINCIPAL_MARKER, + BEARER_PRINCIPAL_MARKER, + SCOPE_MARKER, + ] { + assert!(!audit.contains(marker), "audit leaked marker {marker}"); + } + let records = audit_envelopes(&audit_path) + .into_iter() + .map(|envelope| envelope.record) + .filter(|record| record["path"] == json!("/v1/claims")) + .collect::>(); + assert_eq!(records.len(), 1, "one request must emit one audit record"); + let record = &records[0]; + assert_eq!(record["status"], json!(400)); + assert_eq!(record["decision"], json!("denied")); + assert_eq!(record["error_code"], json!("auth.multiple_credentials")); + assert!(record.get("principal_id").is_none()); + assert!(record.get("principal_id_hash").is_none()); + assert_eq!(record["scopes_used"], json!([])); + assert!(record.get("relay_consultation_count").is_none()); + assert!(record.get("relay_consultation_ids").is_none()); +} + #[tokio::test] pub(super) async fn additive_api_key_and_oidc_authenticate_on_the_same_router() { set_audit_secret(); diff --git a/crates/registry-relay/openapi/registry-relay.openapi.json b/crates/registry-relay/openapi/registry-relay.openapi.json index 1f5c15299..e7177feba 100644 --- a/crates/registry-relay/openapi/registry-relay.openapi.json +++ b/crates/registry-relay/openapi/registry-relay.openapi.json @@ -2874,7 +2874,7 @@ }, "securitySchemes": { "apiKeyAuth": { - "description": "Compatibility API-key header accepted by API-key deployments. `Authorization: Bearer` takes precedence when both headers are present.", + "description": "Compatibility API-key header accepted by API-key deployments. Send exactly one primary credential channel: either this header or `Authorization: Bearer`. Requests containing both are rejected with the generic `auth.multiple_credentials` response before either credential is parsed or validated, without revealing whether either credential is valid.", "in": "header", "name": "x-api-key", "type": "apiKey" @@ -5403,6 +5403,31 @@ }, "description": "Consultation profile contract metadata." }, + "400": { + "content": { + "application/problem+json": { + "schema": { + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" + }, + { + "properties": { + "code": { + "const": "auth.multiple_credentials" + }, + "status": { + "const": 400 + } + }, + "type": "object" + } + ] + } + } + }, + "description": "The request contains more than one primary authentication credential. Stable code: `auth.multiple_credentials`." + }, "401": { "content": { "application/problem+json": { @@ -5587,26 +5612,48 @@ "content": { "application/problem+json": { "schema": { - "allOf": [ + "oneOf": [ { - "$ref": "#/components/schemas/ProblemDetails" + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" + }, + { + "properties": { + "code": { + "const": "consultation.invalid_request" + }, + "status": { + "const": 400 + } + }, + "type": "object" + } + ] }, { - "properties": { - "code": { - "const": "consultation.invalid_request" + "allOf": [ + { + "$ref": "#/components/schemas/ProblemDetails" }, - "status": { - "const": 400 + { + "properties": { + "code": { + "const": "auth.multiple_credentials" + }, + "status": { + "const": 400 + } + }, + "type": "object" } - }, - "type": "object" + ] } ] } } }, - "description": "The consultation request is invalid. Stable code: `consultation.invalid_request`." + "description": "The consultation request is invalid or contains more than one primary authentication credential. Stable codes: `consultation.invalid_request`, `auth.multiple_credentials`." }, "401": { "content": { diff --git a/crates/registry-relay/src/api/consultation.rs b/crates/registry-relay/src/api/consultation.rs index 51128f7e3..45ac6e103 100644 --- a/crates/registry-relay/src/api/consultation.rs +++ b/crates/registry-relay/src/api/consultation.rs @@ -1564,6 +1564,11 @@ mod tests { StatusCode::BAD_REQUEST, "consultation.invalid_request", ), + ( + ConsultationError::MultipleCredentials, + StatusCode::BAD_REQUEST, + "auth.multiple_credentials", + ), ( ConsultationError::InvalidCredentials, StatusCode::UNAUTHORIZED, @@ -1615,6 +1620,7 @@ mod tests { async fn consultation_problems_render_scrubbed_rfc_9457_json() { for variant in [ ConsultationError::InvalidRequest, + ConsultationError::MultipleCredentials, ConsultationError::InvalidCredentials, ConsultationError::Conflict, ConsultationError::ContractMismatch, diff --git a/crates/registry-relay/src/api/openapi.rs b/crates/registry-relay/src/api/openapi.rs index 6838f60bd..2ff311489 100644 --- a/crates/registry-relay/src/api/openapi.rs +++ b/crates/registry-relay/src/api/openapi.rs @@ -1496,6 +1496,11 @@ fn consultation_profile_responses() -> Value { "Consultation profile contract metadata.", "ConsultationProfileMetadata" ), + "400": consultation_problem_response( + 400, + "auth.multiple_credentials", + "The request contains more than one primary authentication credential." + ), "401": consultation_problem_response( 401, "auth.invalid_credentials", @@ -1522,11 +1527,7 @@ fn consultation_profile_responses() -> Value { fn consultation_execute_responses(success_description: &str, success_schema: &str) -> Value { json!({ "200": consultation_success_response(success_description, success_schema), - "400": consultation_problem_response( - 400, - "consultation.invalid_request", - "The consultation request is invalid." - ), + "400": consultation_bad_request_response(), "401": consultation_problem_response( 401, "auth.invalid_credentials", @@ -1552,6 +1553,44 @@ fn consultation_execute_responses(success_description: &str, success_schema: &st }) } +fn consultation_bad_request_response() -> Value { + json!({ + "description": "The consultation request is invalid or contains more than one primary authentication credential. Stable codes: `consultation.invalid_request`, `auth.multiple_credentials`.", + "content": { + "application/problem+json": { + "schema": { + "oneOf": [ + { + "allOf": [ + { "$ref": "#/components/schemas/ProblemDetails" }, + { + "type": "object", + "properties": { + "status": { "const": 400 }, + "code": { "const": "consultation.invalid_request" } + } + } + ] + }, + { + "allOf": [ + { "$ref": "#/components/schemas/ProblemDetails" }, + { + "type": "object", + "properties": { + "status": { "const": 400 }, + "code": { "const": "auth.multiple_credentials" } + } + } + ] + } + ] + } + } + } + }) +} + fn consultation_conflict_response() -> Value { json!({ "description": "The pinned contract hash is not active for this profile, or the authenticated Notary batch child identity conflicts with prior durable state.", @@ -1890,7 +1929,7 @@ fn security_schemes(config: &Config) -> Value { "type": "apiKey", "in": "header", "name": "x-api-key", - "description": "Compatibility API-key header accepted by API-key deployments. `Authorization: Bearer` takes precedence when both headers are present.", + "description": "Compatibility API-key header accepted by API-key deployments. Send exactly one primary credential channel: either this header or `Authorization: Bearer`. Requests containing both are rejected with the generic `auth.multiple_credentials` response before either credential is parsed or validated, without revealing whether either credential is valid.", }), ); } @@ -5904,7 +5943,7 @@ mod tests { .keys() .map(String::as_str) .collect::>(), - ["200", "401", "403", "404", "503"] + ["200", "400", "401", "403", "404", "503"] ); assert_eq!( execute["post"]["responses"] @@ -5938,7 +5977,6 @@ mod tests { })); let expected_codes = [ - ("400", "consultation.invalid_request"), ("401", "auth.invalid_credentials"), ("403", "consultation.denied"), ("404", "consultation.profile_not_found"), @@ -5952,6 +5990,26 @@ mod tests { code ); } + let bad_request_codes = execute_op["responses"]["400"]["content"] + ["application/problem+json"]["schema"]["oneOf"] + .as_array() + .expect("400 response variants") + .iter() + .map(|variant| { + variant["allOf"][1]["properties"]["code"]["const"] + .as_str() + .unwrap() + }) + .collect::>(); + assert_eq!( + bad_request_codes, + ["consultation.invalid_request", "auth.multiple_credentials"] + ); + assert_eq!( + profile["get"]["responses"]["400"]["content"]["application/problem+json"]["schema"] + ["allOf"][1]["properties"]["code"]["const"], + "auth.multiple_credentials" + ); assert_eq!( execute_op["responses"]["409"]["content"]["application/problem+json"]["schema"] ["oneOf"][0]["allOf"][1]["properties"]["code"]["const"], diff --git a/crates/registry-relay/src/audit/mod.rs b/crates/registry-relay/src/audit/mod.rs index 4270ed4f6..0eca390de 100644 --- a/crates/registry-relay/src/audit/mod.rs +++ b/crates/registry-relay/src/audit/mod.rs @@ -1239,9 +1239,9 @@ fn consultation_denial_reason( | "auth.kid_unknown" | "auth.algorithm_not_allowed", ) => Some(ConsultationDenialReason::InvalidCredentials), - Some("consultation.invalid_request" | "auth.purpose_required") => { - Some(ConsultationDenialReason::InvalidRequest) - } + Some( + "consultation.invalid_request" | "auth.multiple_credentials" | "auth.purpose_required", + ) => Some(ConsultationDenialReason::InvalidRequest), Some( "consultation.denied" | "auth.scope_denied" @@ -1815,6 +1815,10 @@ mod tests { consultation_denial_reason(StatusCode::BAD_REQUEST, Some("auth.purpose_required")), Some(ConsultationDenialReason::InvalidRequest) ); + assert_eq!( + consultation_denial_reason(StatusCode::BAD_REQUEST, Some("auth.multiple_credentials")), + Some(ConsultationDenialReason::InvalidRequest) + ); assert_eq!( consultation_denial_reason(StatusCode::FORBIDDEN, Some("consultation.denied")), Some(ConsultationDenialReason::Denied) diff --git a/crates/registry-relay/src/auth/api_key.rs b/crates/registry-relay/src/auth/api_key.rs index 73463425c..1f2168e75 100644 --- a/crates/registry-relay/src/auth/api_key.rs +++ b/crates/registry-relay/src/auth/api_key.rs @@ -3,7 +3,8 @@ //! //! ## Verification flow //! -//! 1. Read `Authorization: Bearer ` or `x-api-key: `. +//! 1. Require exactly one of `Authorization: Bearer ` or +//! `x-api-key: ` to be present. //! 2. Hash the presented high-entropy key with SHA-256. //! 3. Look up the fingerprint in the configured in-memory key map. //! On no match, return `AuthError::InvalidCredential`. @@ -18,9 +19,10 @@ //! * No retention of generated keys. Operators can use the relay binary's //! provisioning command, then store only `sha256:` fingerprints in //! secret storage and rotate by rolling config plus secret changes together. -//! * No credential source is preferred for security reasons. When both -//! headers are present, `Authorization` wins because it is the standard -//! HTTP auth surface. +//! * No credential source is preferred. Requests carrying both primary +//! credential headers are rejected from header presence alone, before +//! either value is parsed or verified. This prevents fallback, precedence, +//! and credential-validity oracles. use std::collections::HashMap; use std::pin::Pin; @@ -191,21 +193,28 @@ impl AuthProvider for ApiKeyAuth { /// /// Returns: /// * `AuthError::MissingCredential` if neither header is present. +/// * `AuthError::MultipleCredentials` if both headers are present. This check +/// happens before either header value is parsed. /// * `AuthError::MalformedCredential` if the chosen header is not UTF-8, /// has a malformed `Bearer` value, or carries an empty token. /// /// The returned token is wrapped in [`Zeroizing`] so its bytes are /// scrubbed when the caller drops it. fn extract_credential(headers: &HeaderMap) -> Result, AuthError> { - if let Some(value) = headers.get(header::AUTHORIZATION) { - return extract_bearer_value(value); - } - let value = headers.get(X_API_KEY).ok_or(AuthError::MissingCredential)?; - let token = value.to_str().map_err(|_| AuthError::MalformedCredential)?; - if token.is_empty() { - return Err(AuthError::MalformedCredential); + let authorization = headers.get(header::AUTHORIZATION); + let api_key = headers.get(X_API_KEY); + match (authorization, api_key) { + (Some(_), Some(_)) => Err(AuthError::MultipleCredentials), + (Some(value), None) => extract_bearer_value(value), + (None, Some(value)) => { + let token = value.to_str().map_err(|_| AuthError::MalformedCredential)?; + if token.is_empty() { + return Err(AuthError::MalformedCredential); + } + Ok(Zeroizing::new(token.to_string())) + } + (None, None) => Err(AuthError::MissingCredential), } - Ok(Zeroizing::new(token.to_string())) } fn extract_bearer_value(value: &axum::http::HeaderValue) -> Result, AuthError> { @@ -318,12 +327,21 @@ mod tests { } #[test] - fn authorization_header_wins_over_x_api_key() { + fn valid_authorization_and_x_api_key_are_rejected_before_verification() { let mut headers = HeaderMap::new(); headers.insert(header::AUTHORIZATION, "Bearer from-auth".parse().unwrap()); headers.insert(X_API_KEY, "from-x-api-key".parse().unwrap()); - let token = extract_credential(&headers).expect("token extracted"); - assert_eq!(&*token, "from-auth"); + let err = extract_credential(&headers).expect_err("ambiguous credentials rejected"); + assert!(matches!(err, AuthError::MultipleCredentials)); + } + + #[test] + fn malformed_authorization_and_valid_x_api_key_are_rejected_from_presence() { + let mut headers = HeaderMap::new(); + headers.insert(header::AUTHORIZATION, "Basic malformed".parse().unwrap()); + headers.insert(X_API_KEY, "valid-api-key".parse().unwrap()); + let err = extract_credential(&headers).expect_err("ambiguous credentials rejected"); + assert!(matches!(err, AuthError::MultipleCredentials)); } #[test] diff --git a/crates/registry-relay/src/auth/middleware.rs b/crates/registry-relay/src/auth/middleware.rs index 1d3899857..a99db745a 100644 --- a/crates/registry-relay/src/auth/middleware.rs +++ b/crates/registry-relay/src/auth/middleware.rs @@ -250,6 +250,7 @@ fn auth_error_response(error: AuthError, consultation_operation: bool) -> Respon | AuthError::KidUnknown | AuthError::AlgorithmNotAllowed | AuthError::ClientNotAllowed => (ConsultationError::InvalidCredentials, None), + AuthError::MultipleCredentials => (ConsultationError::MultipleCredentials, None), AuthError::PurposeRequired => (ConsultationError::InvalidRequest, None), AuthError::ScopeDenied { .. } | AuthError::PurposeDenied | AuthError::AdminRequired => { (ConsultationError::Denied, None) @@ -440,6 +441,12 @@ mod tests { "auth.invalid_credentials", None, ), + ( + || AuthError::MultipleCredentials, + StatusCode::BAD_REQUEST, + "auth.multiple_credentials", + None, + ), ( || AuthError::ScopeDenied { required: "configured-sensitive-scope".to_string(), diff --git a/crates/registry-relay/src/auth/oidc/provider.rs b/crates/registry-relay/src/auth/oidc/provider.rs index cf242a297..89792d3ce 100644 --- a/crates/registry-relay/src/auth/oidc/provider.rs +++ b/crates/registry-relay/src/auth/oidc/provider.rs @@ -5,7 +5,8 @@ //! //! 1. Read `Authorization: Bearer `. OIDC does not accept the //! `x-api-key` header; that header is the API-key provider's legacy -//! surface only. +//! surface only. If both headers are present, reject the request before +//! parsing either value so credential precedence cannot vary by provider. //! 2. Decode the JOSE header. Reject tokens whose `typ` is missing or not in //! the configured allowlist (defaults to `JWT` / `at+jwt`), whose `alg` //! is not in the configured allowlist (defaults to RS256/ES256/EdDSA; @@ -64,6 +65,9 @@ use super::jwks::{JwksCache, JwksFetcher}; /// HTTP authentication scheme accepted by the OIDC provider. const BEARER_SCHEME: &str = "Bearer"; +/// Legacy API-key channel whose simultaneous presence is always ambiguous. +const X_API_KEY: &str = "x-api-key"; + /// Bearer-JWT verifier built from one [`OidcConfig`] block. /// /// One instance is built at startup and held behind @@ -295,9 +299,13 @@ impl AuthProvider for OidcAuth { } fn extract_bearer(headers: &HeaderMap) -> Result, AuthError> { - let value = headers - .get(header::AUTHORIZATION) - .ok_or(AuthError::MissingCredential)?; + let authorization = headers.get(header::AUTHORIZATION); + let api_key = headers.get(X_API_KEY); + let value = match (authorization, api_key) { + (Some(_), Some(_)) => return Err(AuthError::MultipleCredentials), + (Some(value), None) => value, + (None, Some(_)) | (None, None) => return Err(AuthError::MissingCredential), + }; let raw = value.to_str().map_err(|_| AuthError::MalformedCredential)?; // RFC 7235 §2.1: auth scheme is case-insensitive. RFC 6750 §2.1 requires // exactly one SP between the scheme and the token. @@ -407,6 +415,7 @@ fn auth_error_code(err: &AuthError) -> &'static str { AuthError::MissingCredential => "auth.missing_credential", AuthError::InvalidCredential => "auth.invalid_credential", AuthError::MalformedCredential => "auth.malformed_credential", + AuthError::MultipleCredentials => "auth.multiple_credentials", AuthError::ScopeDenied { .. } => "auth.scope_denied", AuthError::PurposeRequired => "auth.purpose_required", AuthError::PurposeDenied => "auth.purpose_denied", @@ -1455,6 +1464,29 @@ mod tests { assert!(matches!(err, AuthError::MissingCredential)); } + #[test] + fn authorization_and_x_api_key_are_ambiguous_before_parsing() { + let mut headers = HeaderMap::new(); + headers.insert(header::AUTHORIZATION, "Bearer valid-token".parse().unwrap()); + headers.insert(X_API_KEY, "valid-api-key".parse().unwrap()); + + let err = extract_bearer(&headers).expect_err("two raw credential headers are ambiguous"); + assert!(matches!(err, AuthError::MultipleCredentials)); + } + + #[test] + fn non_utf8_authorization_and_x_api_key_are_ambiguous_before_parsing() { + let mut headers = HeaderMap::new(); + headers.insert( + header::AUTHORIZATION, + axum::http::HeaderValue::from_bytes(b"Bearer \xff").expect("opaque header value"), + ); + headers.insert(X_API_KEY, "valid-api-key".parse().unwrap()); + + let err = extract_bearer(&headers).expect_err("presence wins over malformed content"); + assert!(matches!(err, AuthError::MultipleCredentials)); + } + #[tokio::test] async fn audience_list_intersection_admits() { let (sk, vk) = fresh_keypair(); diff --git a/crates/registry-relay/src/error.rs b/crates/registry-relay/src/error.rs index 06879a6c3..13ddb51bd 100644 --- a/crates/registry-relay/src/error.rs +++ b/crates/registry-relay/src/error.rs @@ -110,6 +110,8 @@ pub enum AuthError { InvalidCredential, #[error("malformed credential")] MalformedCredential, + #[error("multiple credentials")] + MultipleCredentials, /// `required` is the scope name from configuration. It is operator /// visible (not a secret) but is sanitised before being placed in /// the rendered detail message. @@ -176,6 +178,8 @@ pub enum AuthError { pub enum ConsultationError { #[error("invalid consultation request")] InvalidRequest, + #[error("multiple authentication credentials")] + MultipleCredentials, #[error("consultation batch child conflicts with durable state")] Conflict, #[error("consultation contract does not match the active profile")] @@ -693,6 +697,7 @@ impl AuthError { AuthError::MissingCredential => "auth.missing_credential", AuthError::InvalidCredential => "auth.invalid_credential", AuthError::MalformedCredential => "auth.malformed_credential", + AuthError::MultipleCredentials => "auth.multiple_credentials", AuthError::ScopeDenied { .. } => "auth.scope_denied", AuthError::PurposeRequired => "auth.purpose_required", AuthError::PurposeDenied => "auth.purpose_denied", @@ -722,6 +727,7 @@ impl AuthError { | AuthError::AudienceMismatch | AuthError::KidUnknown | AuthError::AlgorithmNotAllowed => StatusCode::UNAUTHORIZED, + AuthError::MultipleCredentials => StatusCode::BAD_REQUEST, AuthError::ScopeDenied { .. } | AuthError::PurposeDenied | AuthError::AdminRequired @@ -737,6 +743,7 @@ impl AuthError { AuthError::MissingCredential => "Missing credential", AuthError::InvalidCredential => "Invalid credential", AuthError::MalformedCredential => "Malformed credential", + AuthError::MultipleCredentials => "Multiple credentials", AuthError::ScopeDenied { .. } => "Scope denied", AuthError::PurposeRequired => "Purpose header required", AuthError::PurposeDenied => "Purpose denied", @@ -763,6 +770,9 @@ impl AuthError { "credential did not match any configured key".to_string() } AuthError::MalformedCredential => "credential header was not parseable".to_string(), + AuthError::MultipleCredentials => { + "provide exactly one authentication credential".to_string() + } AuthError::ScopeDenied { required } => { let safe = sanitise_operator_string(required, MAX_SCOPE_NAME_LEN); truncate(format!("required scope: {safe}"), MAX_DETAIL_LEN) @@ -804,6 +814,7 @@ impl ConsultationError { fn code(&self) -> &'static str { match self { Self::InvalidRequest => "consultation.invalid_request", + Self::MultipleCredentials => "auth.multiple_credentials", Self::Conflict => "consultation.batch_child_conflict", Self::ContractMismatch => "consultation.contract_mismatch", Self::InvalidCredentials => "auth.invalid_credentials", @@ -816,7 +827,7 @@ impl ConsultationError { fn http_status(&self) -> StatusCode { match self { - Self::InvalidRequest => StatusCode::BAD_REQUEST, + Self::InvalidRequest | Self::MultipleCredentials => StatusCode::BAD_REQUEST, Self::Conflict | Self::ContractMismatch => StatusCode::CONFLICT, Self::InvalidCredentials => StatusCode::UNAUTHORIZED, Self::Denied => StatusCode::FORBIDDEN, @@ -829,6 +840,7 @@ impl ConsultationError { fn title(&self) -> &'static str { match self { Self::InvalidRequest => "Invalid consultation request", + Self::MultipleCredentials => "Multiple credentials", Self::Conflict => "Consultation batch child conflict", Self::ContractMismatch => "Consultation contract mismatch", Self::InvalidCredentials => "Invalid credentials", @@ -842,6 +854,7 @@ impl ConsultationError { fn detail(&self) -> &'static str { match self { Self::InvalidRequest => "the consultation request is invalid", + Self::MultipleCredentials => "provide exactly one authentication credential", Self::Conflict => "the batch child identity conflicts with a prior request", Self::ContractMismatch => "the requested contract is not active for this profile", Self::InvalidCredentials => "service authentication failed", diff --git a/crates/registry-relay/src/server.rs b/crates/registry-relay/src/server.rs index 962a121ab..8cdebff63 100644 --- a/crates/registry-relay/src/server.rs +++ b/crates/registry-relay/src/server.rs @@ -699,7 +699,8 @@ fn is_closed_consultation_error(response: &Response) -> bool { ( StatusCode::BAD_REQUEST, Some("consultation.invalid_request") - ) | (StatusCode::UNAUTHORIZED, Some("auth.invalid_credentials")) + ) | (StatusCode::BAD_REQUEST, Some("auth.multiple_credentials")) + | (StatusCode::UNAUTHORIZED, Some("auth.invalid_credentials")) | (StatusCode::FORBIDDEN, Some("consultation.denied")) | ( StatusCode::NOT_FOUND, @@ -1376,6 +1377,15 @@ mod tests { } let cases = [ + Case { + source_status: StatusCode::BAD_REQUEST, + source_code: Some("auth.multiple_credentials"), + expected_status: StatusCode::BAD_REQUEST, + expected_code: "auth.multiple_credentials", + allow: None, + retry_after: None, + expected_retry_after: None, + }, Case { source_status: StatusCode::GATEWAY_TIMEOUT, source_code: Some("internal.timeout"), @@ -1453,7 +1463,11 @@ mod tests { for case in cases { let app = Router::new() .fallback(move || async move { - let mut response = case.source_status.into_response(); + let mut response = if case.source_code == Some("auth.multiple_credentials") { + Error::from(ConsultationError::MultipleCredentials).into_response() + } else { + case.source_status.into_response() + }; if let Some(code) = case.source_code { response .extensions_mut() diff --git a/crates/registry-relay/tests/audit_record.rs b/crates/registry-relay/tests/audit_record.rs index 094b19279..a80ce71c9 100644 --- a/crates/registry-relay/tests/audit_record.rs +++ b/crates/registry-relay/tests/audit_record.rs @@ -8,13 +8,14 @@ use std::collections::BTreeSet; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use axum::body::{to_bytes, Body}; use axum::http::{Method, Request, StatusCode}; use axum::middleware::{from_fn, Next}; use axum::response::IntoResponse; -use axum::routing::get; +use axum::routing::{get, post}; use axum::Extension; use axum::Router; use registry_relay::audit::{ @@ -1383,6 +1384,322 @@ async fn middleware_captures_error_code_from_auth_short_circuit() { assert_eq!(parsed["principal_id"], Value::Null); } +#[tokio::test] +async fn ambiguous_credentials_emit_one_redacted_denial_without_handler_dispatch() { + use registry_relay::auth::api_key::{ApiKeyAuth, ApiKeyEntry}; + use registry_relay::auth::middleware::auth_layer; + use sha2::{Digest, Sha256}; + + const VALID_API_KEY: &str = "audit-ambiguity-api-key-marker"; + const MALFORMED_AUTHORIZATION: &str = "audit-ambiguity-authorization-marker"; + const PRINCIPAL_MARKER: &str = "audit-ambiguity-principal-marker"; + const SCOPE_MARKER: &str = "audit-ambiguity-scope-marker"; + + let fingerprint = format!( + "sha256:{}", + hex_lower_local(&Sha256::digest(VALID_API_KEY.as_bytes())) + ); + let provider = Arc::new(ApiKeyAuth::new(vec![ApiKeyEntry::new( + PRINCIPAL_MARKER.to_string(), + ScopeSet::from_iter([SCOPE_MARKER]), + fingerprint, + ) + .expect("test fingerprint parses")])); + let handler_dispatches = Arc::new(AtomicUsize::new(0)); + let counter = Arc::clone(&handler_dispatches); + let (sink, pipeline) = in_memory_pipeline(); + let protected = auth_layer( + Router::new().route( + "/probe", + get(move || { + let counter = Arc::clone(&counter); + async move { + counter.fetch_add(1, Ordering::SeqCst); + StatusCode::NO_CONTENT + } + }), + ), + provider, + ); + let app = Router::new() + .merge(protected) + .layer(from_fn(audit_layer)) + .layer(Extension(pipeline)); + + let response = app + .oneshot( + Request::builder() + .method(Method::GET) + .uri("/probe") + .header("authorization", format!("Basic {MALFORMED_AUTHORIZATION}")) + .header("x-api-key", VALID_API_KEY) + .body(Body::empty()) + .expect("request builds"), + ) + .await + .expect("request completes"); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + let body = to_bytes(response.into_body(), 16 * 1024) + .await + .expect("bounded response body reads"); + let problem: Value = serde_json::from_slice(&body).expect("problem body is JSON"); + assert_eq!(problem["code"], "auth.multiple_credentials"); + assert_eq!( + problem["detail"], + "provide exactly one authentication credential" + ); + let body = String::from_utf8(body.to_vec()).expect("problem body is UTF-8"); + for marker in [ + VALID_API_KEY, + MALFORMED_AUTHORIZATION, + PRINCIPAL_MARKER, + SCOPE_MARKER, + ] { + assert!( + !body.contains(marker), + "problem body leaked marker {marker}" + ); + } + + assert_eq!(handler_dispatches.load(Ordering::SeqCst), 0); + let records = sink.snapshot(); + assert_eq!(records.len(), 1); + for marker in [ + VALID_API_KEY, + MALFORMED_AUTHORIZATION, + PRINCIPAL_MARKER, + SCOPE_MARKER, + ] { + assert!( + !records[0].contains(marker), + "audit record leaked marker {marker}" + ); + } + let parsed = captured_record(&records[0]); + assert_eq!(parsed["status_code"], 400); + assert_eq!(parsed["error_code"], "auth.multiple_credentials"); + assert_eq!(parsed["principal_id"], Value::Null); + assert_eq!(parsed["auth_mode"], Value::Null); + assert_eq!(parsed["scopes_used"], serde_json::json!([])); +} + +#[tokio::test] +async fn consultation_routes_preserve_ambiguous_auth_code_without_dispatch_or_disclosure() { + use std::collections::BTreeMap; + use std::time::{Duration, SystemTime, UNIX_EPOCH}; + + use base64::engine::general_purpose::URL_SAFE_NO_PAD; + use base64::Engine; + use ed25519_dalek::pkcs8::EncodePrivateKey; + use ed25519_dalek::SigningKey; + use jsonwebtoken::{encode, Algorithm, EncodingKey, Header}; + use rand_core::OsRng; + use registry_relay::auth::middleware::auth_layer; + use registry_relay::auth::oidc::{static_fetcher, OidcAuth}; + use registry_relay::auth::AuthProvider; + use registry_relay::config::{OidcAlgorithm, OidcConfig}; + + const PROFILE_ROUTE: &str = "/v1/consultations/{profile_id}"; + const EXECUTE_ROUTE: &str = "/v1/consultations/{profile_id}/execute"; + const ISSUER: &str = "https://idp.example.test/realms/consultation"; + const AUDIENCE: &str = "registry-relay-consultation"; + const KID: &str = "consultation-ambiguity-kid"; + const VALID_API_KEY: &str = "consultation-ambiguity-api-key-marker"; + const MALFORMED_AUTHORIZATION: &str = "consultation-ambiguity-auth-marker"; + const PRINCIPAL_MARKER: &str = "consultation-ambiguity-principal-marker"; + const SCOPE_MARKER: &str = "consultation-ambiguity-scope-marker"; + + let signing_key = SigningKey::generate(&mut OsRng); + let verifying_key = signing_key.verifying_key(); + let jwks = serde_json::from_value(serde_json::json!({ + "keys": [{ + "kty": "OKP", + "crv": "Ed25519", + "use": "sig", + "alg": "EdDSA", + "kid": KID, + "x": URL_SAFE_NO_PAD.encode(verifying_key.as_bytes()), + }] + })) + .expect("test JWKS parses"); + let config = OidcConfig { + issuer: ISSUER.to_string(), + audiences: vec![AUDIENCE.to_string()], + jwks_url: None, + discovery_url: None, + allow_dev_insecure_fetch_urls: false, + allowed_algorithms: vec![OidcAlgorithm::EdDsa], + jwks_cache_ttl: Duration::from_secs(600), + leeway: Duration::from_secs(60), + scope_claim: "scope".to_string(), + scope_map: BTreeMap::new(), + scope_object_required_keys: Vec::new(), + allowed_clients: Vec::new(), + allowed_token_types: vec!["JWT".to_string(), "at+jwt".to_string()], + }; + let provider = Arc::new(OidcAuth::new(&config, static_fetcher(jwks))); + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system time is after epoch") + .as_secs(); + let mut header = Header::new(Algorithm::EdDSA); + header.kid = Some(KID.to_string()); + header.typ = Some("JWT".to_string()); + let private_key = signing_key + .to_pkcs8_der() + .expect("Ed25519 private key encodes"); + let valid_bearer = encode( + &header, + &serde_json::json!({ + "iss": ISSUER, + "aud": AUDIENCE, + "sub": PRINCIPAL_MARKER, + "scope": SCOPE_MARKER, + "iat": now, + "exp": now + 300, + }), + &EncodingKey::from_ed_der(private_key.as_bytes()), + ) + .expect("valid OIDC bearer token mints"); + + let mut single_credential_headers = axum::http::HeaderMap::new(); + single_credential_headers.insert( + axum::http::header::AUTHORIZATION, + format!("Bearer {valid_bearer}") + .parse() + .expect("bearer header parses"), + ); + let authentication = provider + .authenticate(&single_credential_headers, IpAddr::V4(Ipv4Addr::LOCALHOST)) + .await + .expect("single signed OIDC bearer authenticates"); + assert_eq!(authentication.principal_id, PRINCIPAL_MARKER); + assert!(authentication.scopes.contains(SCOPE_MARKER)); + + let handler_or_source_dispatches = Arc::new(AtomicUsize::new(0)); + let profile_dispatches = Arc::clone(&handler_or_source_dispatches); + let execute_dispatches = Arc::clone(&handler_or_source_dispatches); + let (sink, pipeline) = in_memory_pipeline(); + let protected = auth_layer( + Router::new() + .route( + PROFILE_ROUTE, + get(move || { + let counter = Arc::clone(&profile_dispatches); + async move { + counter.fetch_add(1, Ordering::SeqCst); + StatusCode::NO_CONTENT + } + }), + ) + .route( + EXECUTE_ROUTE, + post(move || { + let counter = Arc::clone(&execute_dispatches); + async move { + counter.fetch_add(1, Ordering::SeqCst); + StatusCode::NO_CONTENT + } + }), + ), + provider, + ); + let app = Router::new() + .merge(protected) + .layer(from_fn(audit_layer)) + .layer(Extension(pipeline)); + + let cases = [ + ( + Method::GET, + "/v1/consultations/profile-marker", + format!("Bearer {valid_bearer}"), + ), + ( + Method::POST, + "/v1/consultations/profile-marker/execute", + format!("Basic {MALFORMED_AUTHORIZATION}"), + ), + ]; + for (case_index, (method, uri, authorization)) in cases.into_iter().enumerate() { + let response = app + .clone() + .oneshot( + Request::builder() + .method(method) + .uri(uri) + .header("authorization", authorization) + .header("x-api-key", VALID_API_KEY) + .body(Body::empty()) + .expect("consultation request builds"), + ) + .await + .expect("consultation request completes"); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + let body = to_bytes(response.into_body(), 16 * 1024) + .await + .expect("bounded response body reads"); + let problem: Value = serde_json::from_slice(&body).expect("problem body is JSON"); + assert_eq!(problem["code"], "auth.multiple_credentials"); + assert_eq!( + problem["detail"], + "provide exactly one authentication credential" + ); + let body = String::from_utf8(body.to_vec()).expect("problem body is UTF-8"); + for marker in [ + valid_bearer.as_str(), + VALID_API_KEY, + MALFORMED_AUTHORIZATION, + PRINCIPAL_MARKER, + SCOPE_MARKER, + ] { + assert!( + !body.contains(marker), + "problem body leaked marker {marker}" + ); + } + + assert_eq!( + handler_or_source_dispatches.load(Ordering::SeqCst), + 0, + "authentication denial must precede handler and source dispatch" + ); + let records = sink.snapshot(); + assert_eq!( + records.len(), + case_index + 1, + "each denial emits exactly one audit record" + ); + let record = records.last().expect("the denial audit is captured"); + for marker in [ + valid_bearer.as_str(), + VALID_API_KEY, + MALFORMED_AUTHORIZATION, + PRINCIPAL_MARKER, + SCOPE_MARKER, + ] { + assert!( + !record.contains(marker), + "audit record leaked marker {marker}" + ); + } + let parsed = captured_record(record); + assert_eq!(parsed["status_code"], 400); + assert_eq!(parsed["error_code"], "auth.multiple_credentials"); + assert_eq!(parsed["principal_id"], Value::Null); + assert_eq!(parsed["auth_mode"], Value::Null); + assert_eq!(parsed["scopes_used"], serde_json::json!([])); + } + + assert_eq!(handler_or_source_dispatches.load(Ordering::SeqCst), 0); + let records = sink.snapshot(); + assert_eq!( + records.len(), + 2, + "each denial emits exactly one audit record" + ); +} + #[tokio::test] async fn middleware_records_jwks_unavailable_auth_failure() { use registry_relay::auth::middleware::auth_layer; diff --git a/crates/registry-relay/tests/auth_flow.rs b/crates/registry-relay/tests/auth_flow.rs index a52435c0e..290c64c07 100644 --- a/crates/registry-relay/tests/auth_flow.rs +++ b/crates/registry-relay/tests/auth_flow.rs @@ -15,6 +15,7 @@ //! by the audit track. use std::net::{IpAddr, Ipv4Addr}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use axum::body::Body; @@ -284,6 +285,87 @@ async fn x_api_key_admits_request_and_populates_principal() { assert_eq!(value["auth_mode"].as_str(), Some("api_key")); } +fn ambiguity_probe_router(dispatches: Arc) -> Router { + let handler_dispatches = Arc::clone(&dispatches); + auth_layer( + Router::new().route( + "/dispatch-probe", + get(move || { + let handler_dispatches = Arc::clone(&handler_dispatches); + async move { + handler_dispatches.fetch_add(1, Ordering::SeqCst); + StatusCode::NO_CONTENT + } + }), + ), + build_provider(), + ) +} + +#[tokio::test] +async fn authorization_and_x_api_key_are_rejected_before_valid_credentials_dispatch() { + let dispatches = Arc::new(AtomicUsize::new(0)); + let response = ambiguity_probe_router(Arc::clone(&dispatches)) + .oneshot( + Request::builder() + .uri("/dispatch-probe") + .header("Authorization", format!("Bearer {VALID_KEY}")) + .header("x-api-key", VALID_KEY) + .body(Body::empty()) + .expect("request builds"), + ) + .await + .expect("service responds"); + + let problem = assert_problem_response( + response, + StatusCode::BAD_REQUEST, + "auth.multiple_credentials", + VALID_KEY, + ) + .await; + assert_eq!( + problem["detail"], + "provide exactly one authentication credential" + ); + assert_eq!(dispatches.load(Ordering::SeqCst), 0); +} + +#[tokio::test] +async fn malformed_authorization_and_valid_x_api_key_are_rejected_before_dispatch() { + const MALFORMED_AUTHORIZATION_MARKER: &str = "malformed-authorization-marker"; + let dispatches = Arc::new(AtomicUsize::new(0)); + let response = ambiguity_probe_router(Arc::clone(&dispatches)) + .oneshot( + Request::builder() + .uri("/dispatch-probe") + .header( + "Authorization", + format!("Basic {MALFORMED_AUTHORIZATION_MARKER}"), + ) + .header("x-api-key", VALID_KEY) + .body(Body::empty()) + .expect("request builds"), + ) + .await + .expect("service responds"); + + let problem = assert_problem_response( + response, + StatusCode::BAD_REQUEST, + "auth.multiple_credentials", + VALID_KEY, + ) + .await; + let rendered = problem.to_string(); + assert!(!rendered.contains(MALFORMED_AUTHORIZATION_MARKER)); + assert_eq!( + problem["detail"], + "provide exactly one authentication credential" + ); + assert_eq!(dispatches.load(Ordering::SeqCst), 0); +} + #[tokio::test] async fn missing_scope_returns_scope_denied() { let app = router_with_provider(build_provider()); diff --git a/crates/registry-relay/tests/error_taxonomy.rs b/crates/registry-relay/tests/error_taxonomy.rs index 19707a526..b81699a79 100644 --- a/crates/registry-relay/tests/error_taxonomy.rs +++ b/crates/registry-relay/tests/error_taxonomy.rs @@ -27,6 +27,7 @@ fn all_variants() -> Vec { Error::Auth(AuthError::MissingCredential), Error::Auth(AuthError::InvalidCredential), Error::Auth(AuthError::MalformedCredential), + Error::Auth(AuthError::MultipleCredentials), Error::Auth(AuthError::ScopeDenied { required: "social_registry:rows".to_string(), }), @@ -145,6 +146,7 @@ fn expected_table() -> Vec<(&'static str, StatusCode)> { ("auth.missing_credential", StatusCode::UNAUTHORIZED), ("auth.invalid_credential", StatusCode::UNAUTHORIZED), ("auth.malformed_credential", StatusCode::UNAUTHORIZED), + ("auth.multiple_credentials", StatusCode::BAD_REQUEST), ("auth.scope_denied", StatusCode::FORBIDDEN), ("auth.purpose_required", StatusCode::BAD_REQUEST), ("auth.purpose_denied", StatusCode::FORBIDDEN),