diff --git a/crates/aisix-admin/src/auth.rs b/crates/aisix-admin/src/auth.rs index 2ebfbaa7..ed3d2161 100644 --- a/crates/aisix-admin/src/auth.rs +++ b/crates/aisix-admin/src/auth.rs @@ -51,7 +51,7 @@ pub(crate) fn is_admin_authorized(headers: &axum::http::HeaderMap, admin_keys: & fn extract_bearer(headers: &axum::http::HeaderMap) -> Result { if let Some(auth) = headers.get(axum::http::header::AUTHORIZATION) { let s = auth.to_str().map_err(|_| AdminError::Unauthorized)?; - if let Some(rest) = s.strip_prefix("Bearer ") { + if let Some(rest) = aisix_core::forwarded_headers::strip_bearer_scheme(s) { let rest = rest.trim(); if rest.is_empty() { return Err(AdminError::Unauthorized); @@ -95,6 +95,21 @@ mod tests { ); } + #[test] + fn extract_bearer_matches_the_scheme_case_insensitively() { + for value in ["bearer admin-secret", "BEARER admin-secret"] { + let mut h = HeaderMap::new(); + h.insert( + axum::http::header::AUTHORIZATION, + HeaderValue::from_static(value), + ); + assert_eq!( + extract_bearer(&parts_with(h).headers).unwrap(), + "admin-secret" + ); + } + } + #[test] fn extract_bearer_accepts_x_api_key_fallback() { let mut h = HeaderMap::new(); diff --git a/crates/aisix-core/src/forwarded_headers.rs b/crates/aisix-core/src/forwarded_headers.rs index 6a576ed2..1061a4a2 100644 --- a/crates/aisix-core/src/forwarded_headers.rs +++ b/crates/aisix-core/src/forwarded_headers.rs @@ -78,6 +78,17 @@ pub const GATEWAY_HEADER_PREFIX: &str = "x-aisix-"; /// slot is refused for capture or as a request-id source. pub const GATEWAY_CREDENTIAL_HEADER: &str = "x-aisix-api-key"; +/// The credential behind a `Bearer` authentication scheme, matching the +/// scheme name case-insensitively (RFC 7235 §2.1) and the credential as +/// written. `None` when `value` does not start with `Bearer `. +pub fn strip_bearer_scheme(value: &str) -> Option<&str> { + const SCHEME: &str = "bearer "; + value + .get(..SCHEME.len()) + .filter(|scheme| scheme.eq_ignore_ascii_case(SCHEME)) + .map(|_| &value[SCHEME.len()..]) +} + /// Client headers never forwarded by a surface that REBUILDS the outbound /// message, on top of [`NON_FORWARDABLE_HEADERS`]. /// @@ -287,6 +298,23 @@ pub fn resolve_forwarded_client_headers( #[cfg(test)] mod tests { + + #[test] + fn bearer_scheme_is_case_insensitive_and_the_token_is_not() { + for v in [ + "Bearer sk-AbC", + "bearer sk-AbC", + "BEARER sk-AbC", + "bEaReR sk-AbC", + ] { + assert_eq!(strip_bearer_scheme(v), Some("sk-AbC"), "{v}"); + } + for v in ["Basic sk-AbC", "Bearer", "Bearersk-AbC", "", "Bé"] { + assert_eq!(strip_bearer_scheme(v), None, "{v}"); + } + assert_eq!(strip_bearer_scheme("Bearer "), Some("")); + } + use super::*; fn map(pairs: &[(&str, &str)]) -> HeaderMap { diff --git a/crates/aisix-proxy/src/attempt.rs b/crates/aisix-proxy/src/attempt.rs index 0d41cd46..2b1106f4 100644 --- a/crates/aisix-proxy/src/attempt.rs +++ b/crates/aisix-proxy/src/attempt.rs @@ -512,7 +512,7 @@ pub(crate) const GUARDRAIL_BLOCK_CLASS: &str = "content_filter"; /// upstream wrote. pub(crate) fn gateway_error_message(err: &ProxyError) -> String { let gateway_text = match err { - ProxyError::MissingAuth + ProxyError::MissingAuth(_) | ProxyError::MissingRouteAuthHeader(_) | ProxyError::InvalidApiKey | ProxyError::ApiKeyExpired @@ -574,7 +574,7 @@ pub(crate) fn attempt_reached_upstream(err: &ProxyError) -> bool { match err { ProxyError::Bridge(be) => be.reached_upstream(), ProxyError::ContentFiltered { .. } => true, - ProxyError::MissingAuth + ProxyError::MissingAuth(_) | ProxyError::MissingRouteAuthHeader(_) | ProxyError::InvalidApiKey | ProxyError::ApiKeyExpired diff --git a/crates/aisix-proxy/src/auth.rs b/crates/aisix-proxy/src/auth.rs index 8be607c4..dee88e52 100644 --- a/crates/aisix-proxy/src/auth.rs +++ b/crates/aisix-proxy/src/auth.rs @@ -476,6 +476,12 @@ fn deny_key( pub(crate) const GATEWAY_KEY_HEADER: &str = aisix_core::forwarded_headers::GATEWAY_CREDENTIAL_HEADER; +/// How a `MissingAuth` message spells `Authorization` (display text, not +/// the lowercase wire name) — also what it names when the request carries +/// no credential header at all. +pub(crate) const AUTHORIZATION_LABEL: &str = "Authorization"; +pub(crate) const X_API_KEY_HEADER: &str = "x-api-key"; + /// Whether the request OFFERS an inbound gateway credential at all. /// /// Deliberately wider than [`header_credential`]: a present-but-unusable @@ -488,15 +494,10 @@ pub(crate) fn credential_offered(parts: &Parts) -> bool { [ GATEWAY_KEY_HEADER, axum::http::header::AUTHORIZATION.as_str(), - "x-api-key", + X_API_KEY_HEADER, ] .iter() - .any(|name| { - parts - .headers - .get(*name) - .is_some_and(|v| v.to_str().map_or(true, |s| !s.trim().is_empty())) - }) + .any(|name| parts.headers.contains_key(*name)) } /// Whether the caller's gateway credential came from [`GATEWAY_KEY_HEADER`] @@ -509,8 +510,9 @@ pub(crate) fn admitted_via_gateway_key_header(headers: &axum::http::HeaderMap) - /// The caller's gateway credential, read from the request headers by /// precedence: [`GATEWAY_KEY_HEADER`] when the header is present, else /// `Authorization: Bearer`, else `x-api-key`. The first header present -/// decides — an empty or malformed one is `MissingAuth`, never a -/// fall-through to the next. `None` when none of the three is present. +/// decides — an empty or malformed one is `MissingAuth` naming that +/// header, never a fall-through to the next. `None` when none of the three +/// is present. pub(crate) fn header_credential( headers: &axum::http::HeaderMap, ) -> Option> { @@ -519,14 +521,18 @@ pub(crate) fn header_credential( } if let Some(auth) = headers.get(axum::http::header::AUTHORIZATION) { let Ok(s) = auth.to_str() else { - return Some(Err(ProxyError::MissingAuth)); + return Some(Err(ProxyError::MissingAuth(AUTHORIZATION_LABEL))); }; - return Some(match s.strip_prefix("Bearer ").map(str::trim) { - Some(rest) if !rest.is_empty() => Ok(rest.to_string()), - _ => Err(ProxyError::MissingAuth), - }); + return Some( + match aisix_core::forwarded_headers::strip_bearer_scheme(s).map(str::trim) { + Some(rest) if !rest.is_empty() => Ok(rest.to_string()), + _ => Err(ProxyError::MissingAuth(AUTHORIZATION_LABEL)), + }, + ); } - headers.get("x-api-key").map(raw_credential) + headers + .get(X_API_KEY_HEADER) + .map(|raw| raw_credential(raw, X_API_KEY_HEADER)) } /// The credential in [`GATEWAY_KEY_HEADER`]: the key itself, or the key @@ -534,28 +540,30 @@ pub(crate) fn header_credential( pub(crate) fn gateway_key_header_credential( raw: &axum::http::HeaderValue, ) -> Result { - let value = raw_credential(raw)?; - match value - .strip_prefix("Bearer ") - .or_else(|| value.strip_prefix("bearer ")) - .map(str::trim) - { - Some("") => Err(ProxyError::MissingAuth), + let value = raw_credential(raw, GATEWAY_KEY_HEADER)?; + match aisix_core::forwarded_headers::strip_bearer_scheme(&value).map(str::trim) { + Some("") => Err(ProxyError::MissingAuth(GATEWAY_KEY_HEADER)), Some(key) => Ok(key.to_string()), None => Ok(value), } } -fn raw_credential(raw: &axum::http::HeaderValue) -> Result { - let s = raw.to_str().map_err(|_| ProxyError::MissingAuth)?.trim(); +fn raw_credential( + raw: &axum::http::HeaderValue, + header: &'static str, +) -> Result { + let s = raw + .to_str() + .map_err(|_| ProxyError::MissingAuth(header))? + .trim(); if s.is_empty() { - return Err(ProxyError::MissingAuth); + return Err(ProxyError::MissingAuth(header)); } Ok(s.to_string()) } fn extract_bearer(parts: &Parts) -> Result { - header_credential(&parts.headers).unwrap_or(Err(ProxyError::MissingAuth)) + header_credential(&parts.headers).unwrap_or(Err(ProxyError::MissingAuth(AUTHORIZATION_LABEL))) } #[cfg(test)] @@ -784,6 +792,14 @@ mod tests { req.into_parts().0 } + fn headers(pairs: &[(&'static str, &'static str)]) -> HeaderMap { + let mut h = HeaderMap::new(); + for (name, value) in pairs { + h.insert(*name, HeaderValue::from_static(value)); + } + h + } + #[test] fn extract_bearer_happy_path() { let mut h = HeaderMap::new(); @@ -805,13 +821,6 @@ mod tests { #[test] fn the_gateway_key_header_wins_and_never_falls_through() { - let headers = |pairs: &[(&'static str, &'static str)]| { - let mut h = HeaderMap::new(); - for (name, value) in pairs { - h.insert(*name, HeaderValue::from_static(value)); - } - h - }; let both = headers(&[ ("x-aisix-api-key", "sk-gateway"), ("authorization", "Bearer upstream-token"), @@ -830,7 +839,7 @@ mod tests { ]); assert!(matches!( extract_bearer(&parts_with(empty.clone())), - Err(ProxyError::MissingAuth) + Err(ProxyError::MissingAuth(_)) )); assert!(credential_offered(&parts_with(headers(&[( "x-aisix-api-key", @@ -849,7 +858,7 @@ mod tests { let parts = parts_with(HeaderMap::new()); assert!(matches!( extract_bearer(&parts), - Err(ProxyError::MissingAuth) + Err(ProxyError::MissingAuth(_)) )); } @@ -863,7 +872,7 @@ mod tests { let parts = parts_with(h); assert!(matches!( extract_bearer(&parts), - Err(ProxyError::MissingAuth) + Err(ProxyError::MissingAuth(_)) )); } @@ -871,32 +880,61 @@ mod tests { fn credential_offered_covers_unusable_credentials() { // Absent → not offered: the anonymous path may serve these. assert!(!credential_offered(&parts_with(HeaderMap::new()))); - let mut empty = HeaderMap::new(); - empty.insert( - axum::http::header::AUTHORIZATION, - HeaderValue::from_static(" "), - ); - assert!(!credential_offered(&parts_with(empty))); - - // Present but unusable → offered: must fail, never downgrade. - for (name, value) in [ - (axum::http::header::AUTHORIZATION.as_str(), "Bearer sk-abc"), - (axum::http::header::AUTHORIZATION.as_str(), "Bearer "), - ( - axum::http::header::AUTHORIZATION.as_str(), - "Basic dXNlcjpwdw==", - ), - ("x-api-key", "sk-abc"), + + // Present but unusable → offered: must fail, never downgrade. An + // empty or whitespace-only value is one of those. + for name in [ + GATEWAY_KEY_HEADER, + axum::http::header::AUTHORIZATION.as_str(), + X_API_KEY_HEADER, ] { - let mut h = HeaderMap::new(); - h.insert(name, HeaderValue::from_str(value).unwrap()); - assert!( - credential_offered(&parts_with(h)), - "{name}: {value:?} must count as an offered credential" + for value in ["", " ", "Bearer ", "Basic dXNlcjpwdw==", "sk-abc"] { + let mut h = HeaderMap::new(); + h.insert(name, HeaderValue::from_str(value).unwrap()); + assert!( + credential_offered(&parts_with(h)), + "{name}: {value:?} must count as an offered credential" + ); + } + } + } + + #[test] + fn the_bearer_scheme_is_case_insensitive() { + for value in ["bearer sk-abc", "BEARER sk-abc", "bEaReR sk-abc"] { + assert_eq!( + extract_bearer(&parts_with(headers(&[("authorization", value)]))).unwrap(), + "sk-abc" ); + let key = headers(&[("x-aisix-api-key", value)]); + assert_eq!(extract_bearer(&parts_with(key)).unwrap(), "sk-abc"); } } + #[test] + fn missing_auth_names_the_header_that_failed() { + let message = |pairs: &[(&'static str, &'static str)]| { + extract_bearer(&parts_with(headers(pairs))) + .unwrap_err() + .to_string() + }; + let authorization = "missing or malformed Authorization header"; + assert_eq!(message(&[]), authorization); + assert_eq!(message(&[("authorization", "")]), authorization); + assert_eq!(message(&[("authorization", "Basic abc")]), authorization); + assert_eq!( + message(&[ + ("x-aisix-api-key", ""), + ("authorization", "Bearer sk-valid") + ]), + "missing or malformed x-aisix-api-key header" + ); + assert_eq!( + message(&[("x-api-key", " ")]), + "missing or malformed x-api-key header" + ); + } + #[test] fn extract_bearer_rejects_empty_bearer() { let mut h = HeaderMap::new(); @@ -907,7 +945,7 @@ mod tests { let parts = parts_with(h); assert!(matches!( extract_bearer(&parts), - Err(ProxyError::MissingAuth) + Err(ProxyError::MissingAuth(_)) )); } } diff --git a/crates/aisix-proxy/src/error.rs b/crates/aisix-proxy/src/error.rs index 4b3a3337..98480bea 100644 --- a/crates/aisix-proxy/src/error.rs +++ b/crates/aisix-proxy/src/error.rs @@ -163,8 +163,10 @@ impl ErrorEnvelope { #[derive(Debug, thiserror::Error)] pub enum ProxyError { - #[error("missing or malformed Authorization header")] - MissingAuth, + /// No usable gateway credential. Carries the credential header that + /// failed, so the message points at the header to fix. + #[error("missing or malformed {0} header")] + MissingAuth(&'static str), /// A `header_key` passthrough route saw no gateway key in its configured /// header. Named separately from [`ProxyError::MissingAuth`] because on /// these routes `Authorization` deliberately carries the caller's own @@ -458,7 +460,7 @@ impl ProxyError { pub fn status(&self) -> StatusCode { match self { ProxyError::MissingRouteAuthHeader(_) - | ProxyError::MissingAuth + | ProxyError::MissingAuth(_) | ProxyError::InvalidApiKey | ProxyError::ApiKeyExpired | ProxyError::ApiKeyDisabled @@ -503,7 +505,7 @@ impl ProxyError { pub fn kind(&self) -> &'static str { match self { ProxyError::MissingRouteAuthHeader(_) - | ProxyError::MissingAuth + | ProxyError::MissingAuth(_) | ProxyError::InvalidApiKey | ProxyError::ApiKeyExpired | ProxyError::ApiKeyDisabled @@ -784,7 +786,7 @@ impl ProxyError { /// fault, not a credential judgment. fn auth_challenge(&self) -> Option { match self { - ProxyError::MissingAuth => Some(AuthChallenge::MissingCredentials), + ProxyError::MissingAuth(_) => Some(AuthChallenge::MissingCredentials), ProxyError::InvalidApiKey | ProxyError::ApiKeyExpired | ProxyError::ApiKeyDisabled @@ -1186,7 +1188,7 @@ mod tests { #[test] fn missing_auth_maps_to_401_invalid_api_key() { - let e = ProxyError::MissingAuth; + let e = ProxyError::MissingAuth("Authorization"); assert_eq!(e.status(), StatusCode::UNAUTHORIZED); assert_eq!(e.kind(), "invalid_api_key"); } @@ -1213,7 +1215,7 @@ mod tests { #[test] fn auth_challenge_classification_covers_exactly_the_curable_failures() { assert!(matches!( - ProxyError::MissingAuth.auth_challenge(), + ProxyError::MissingAuth("Authorization").auth_challenge(), Some(AuthChallenge::MissingCredentials) )); for e in [ @@ -1265,7 +1267,7 @@ mod tests { #[tokio::test] async fn into_response_attaches_the_challenge_extension() { - let resp = ProxyError::MissingAuth.into_response(); + let resp = ProxyError::MissingAuth("Authorization").into_response(); assert!(matches!( resp.extensions().get::(), Some(AuthChallenge::MissingCredentials) @@ -1482,7 +1484,7 @@ mod tests { #[tokio::test] async fn anthropic_envelope_401_maps_to_authentication_error() { - let err = ProxyError::MissingAuth; + let err = ProxyError::MissingAuth("Authorization"); let resp = err.into_anthropic_response(); assert_anthropic_envelope(resp, StatusCode::UNAUTHORIZED, "authentication_error").await; } diff --git a/crates/aisix-proxy/src/mcp.rs b/crates/aisix-proxy/src/mcp.rs index 37d17b4a..057177cd 100644 --- a/crates/aisix-proxy/src/mcp.rs +++ b/crates/aisix-proxy/src/mcp.rs @@ -2504,12 +2504,21 @@ mod tests { let router = router_with(snapshot_with_anonymous( serde_json::json!({ "servers": ["docs"], "aggregate_entry": true }), )); - for bad in ["Bearer sk-not-a-real-key", "Basic dXNlcjpwdw==", "Bearer "] { + let mut offered: Vec<(&str, &str)> = + ["Bearer sk-not-a-real-key", "Basic dXNlcjpwdw==", "Bearer "] + .into_iter() + .map(|bad| ("authorization", bad)) + .collect(); + // An empty credential header is an offered one too, whichever slot. + for name in ["x-aisix-api-key", "authorization", "x-api-key"] { + offered.push((name, "")); + offered.push((name, " ")); + } + for (name, bad) in offered { let mut request = from_ip(initialize_request(None), "10.1.2.3"); - request.headers_mut().insert( - axum::http::header::AUTHORIZATION, - axum::http::HeaderValue::from_str(bad).unwrap(), - ); + request + .headers_mut() + .insert(name, axum::http::HeaderValue::from_str(bad).unwrap()); let response = router .clone() .oneshot(request) @@ -2518,7 +2527,7 @@ mod tests { assert_eq!( response.status(), StatusCode::UNAUTHORIZED, - "{bad:?} must be rejected, not served anonymously" + "{name}: {bad:?} must be rejected, not served anonymously" ); } } diff --git a/crates/aisix-proxy/src/passthrough_route.rs b/crates/aisix-proxy/src/passthrough_route.rs index 8ca7b851..2d05ceda 100644 --- a/crates/aisix-proxy/src/passthrough_route.rs +++ b/crates/aisix-proxy/src/passthrough_route.rs @@ -1290,14 +1290,11 @@ async fn authenticate( }; match route.auth_mode { PassthroughAuthMode::GatewayKey => { - // The dedicated gateway-key header, when present, is the only - // one read (and an empty one is not a fall-through). - let token = match headers.get(crate::auth::GATEWAY_KEY_HEADER) { - Some(raw) => crate::auth::gateway_key_header_credential(raw)?, - None => bearer_of(headers.get(header::AUTHORIZATION)) - .or_else(|| raw_of(headers.get("x-api-key"))) - .ok_or(ProxyError::MissingAuth)?, - }; + // Read exactly as `/v1/*` reads it: the first credential header + // present decides, never a fall-through to the next. + let token = crate::auth::header_credential(headers).unwrap_or(Err( + ProxyError::MissingAuth(crate::auth::AUTHORIZATION_LABEL), + ))?; crate::auth::authenticate_token(state, &token, ctx).await } PassthroughAuthMode::HeaderKey => { @@ -1305,7 +1302,12 @@ async fn authenticate( let token = headers .get(name) .and_then(|v| v.to_str().ok()) - .map(|v| v.strip_prefix("Bearer ").unwrap_or(v).trim().to_string()) + .map(|v| { + aisix_core::forwarded_headers::strip_bearer_scheme(v) + .unwrap_or(v) + .trim() + .to_string() + }) .filter(|v| !v.is_empty()) .ok_or_else(|| ProxyError::MissingRouteAuthHeader(name.to_string()))?; crate::auth::authenticate_token(state, &token, ctx).await @@ -1340,20 +1342,6 @@ async fn authenticate( } } -fn bearer_of(v: Option<&HeaderValue>) -> Option { - let s = v?.to_str().ok()?; - let token = s - .strip_prefix("Bearer ") - .or_else(|| s.strip_prefix("bearer "))?; - let token = token.trim(); - (!token.is_empty()).then(|| token.to_string()) -} - -fn raw_of(v: Option<&HeaderValue>) -> Option { - let s = v?.to_str().ok()?.trim(); - (!s.is_empty()).then(|| s.to_string()) -} - /// Route-level source allowlist: unset means unrestricted (the schema /// forces a non-empty list for `anonymous` routes). fn source_allowed(route: &PassthroughRoute, source_ip: &str) -> bool { @@ -10760,6 +10748,105 @@ data: {"type":"error","error":{"type":"server_error","code":null,"message":"nest assert_eq!(usage(br#"{"code":"InvalidParameter","message":"x"}"#), None); } + /// A `gateway_key` route reads the credential exactly as `/v1/*` does: + /// the first credential header present decides, an empty or unusable + /// one is a 401 naming it, and the Bearer scheme is case-insensitive. + #[tokio::test] + async fn gateway_key_route_reads_the_credential_like_v1() { + let upstream = MockServer::builder().start().await; + Mock::given(wm_method("GET")) + .and(wm_path("/v1/models")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({"ok": true}))) + .mount(&upstream) + .await; + let snap = AisixSnapshot::new(); + snap.provider_keys + .insert(provider_key_entry("http://unused")); + snap.apikeys.insert(apikey_entry("sk-caller", Some(&["*"]))); + snap.passthrough_routes + .insert(inject_route(&upstream.uri())); + let app = build_app(snap); + + let send = |pairs: &'static [(&'static str, &'static str)]| { + let app = app.clone(); + async move { + let mut req = Request::builder() + .method("GET") + .uri("/passthrough/openai/v1/models"); + for (name, value) in pairs { + req = req.header(*name, *value); + } + let resp = app + .oneshot(req.body(axum::body::Body::empty()).unwrap()) + .await + .unwrap(); + let status = resp.status(); + let body: serde_json::Value = + serde_json::from_slice(&to_bytes(resp.into_body(), 65536).await.unwrap()) + .unwrap_or_default(); + ( + status, + body["error"]["message"].as_str().unwrap_or("").to_string(), + ) + } + }; + + assert_eq!(send(&[("x-api-key", "sk-caller")]).await.0, StatusCode::OK); + // Only the scheme is case-insensitive. + for pairs in [ + &[("authorization", "bearer sk-caller")], + &[("authorization", "BEARER sk-caller")], + ] { + assert_eq!(send(pairs).await.0, StatusCode::OK, "{pairs:?}"); + } + assert_eq!( + send(&[("authorization", "Bearer SK-CALLER")]).await.0, + StatusCode::UNAUTHORIZED + ); + // A present but unusable Authorization never falls through either. + for pairs in [ + &[ + ("authorization", "Basic dXNlcjpwdw=="), + ("x-api-key", "sk-caller"), + ], + &[("authorization", "Bearer "), ("x-api-key", "sk-caller")], + ] { + assert_eq!( + send(pairs).await, + ( + StatusCode::UNAUTHORIZED, + "missing or malformed Authorization header".to_string() + ), + "{pairs:?}" + ); + } + assert_eq!( + send(&[("authorization", ""), ("x-api-key", "sk-caller")]).await, + ( + StatusCode::UNAUTHORIZED, + "missing or malformed Authorization header".to_string() + ) + ); + assert_eq!( + send(&[("x-api-key", " ")]).await, + ( + StatusCode::UNAUTHORIZED, + "missing or malformed x-api-key header".to_string() + ) + ); + assert_eq!( + send(&[ + ("x-aisix-api-key", ""), + ("authorization", "Bearer sk-caller") + ]) + .await, + ( + StatusCode::UNAUTHORIZED, + "missing or malformed x-aisix-api-key header".to_string() + ) + ); + } + #[tokio::test] async fn inject_strips_caller_credentials_even_with_empty_strip_headers() { // A ProviderKey whose strip_headers is explicitly EMPTY: the diff --git a/crates/aisix-proxy/src/realtime.rs b/crates/aisix-proxy/src/realtime.rs index 2d45c029..61167e55 100644 --- a/crates/aisix-proxy/src/realtime.rs +++ b/crates/aisix-proxy/src/realtime.rs @@ -581,7 +581,7 @@ async fn authenticate( } } } - Err(ProxyError::MissingAuth) + Err(ProxyError::MissingAuth(crate::auth::AUTHORIZATION_LABEL)) } fn to_ws_scheme(url: &str) -> Result { diff --git a/tests/e2e/src/cases/mcp-anonymous-access-e2e.test.ts b/tests/e2e/src/cases/mcp-anonymous-access-e2e.test.ts index 2df56e3c..9b1d499a 100644 --- a/tests/e2e/src/cases/mcp-anonymous-access-e2e.test.ts +++ b/tests/e2e/src/cases/mcp-anonymous-access-e2e.test.ts @@ -27,7 +27,8 @@ import { // principal's own grant is `*`; // - an unlisted-but-registered server is indistinguishable from an unknown // one (401, never 404); -// - a credential that is offered and fails NEVER downgrades to anonymous; +// - a credential that is offered and fails NEVER downgrades to anonymous, +// and an empty credential header is an offered one; // - a valid credential keeps its own identity and wider reach on the same // entries. @@ -36,6 +37,8 @@ const ANON_PRINCIPAL_SECRET = `sk-anon-e2e-principal-${randomUUID()}`; const sha256 = (s: string) => createHash("sha256").update(s).digest("hex"); +type Credential = string | Record | undefined; + interface RpcReply { status: number; json?: { @@ -57,17 +60,22 @@ describe("mcp anonymous access e2e", () => { let seed: SeedClient; const settingsId = randomUUID(); - /** POST a JSON-RPC body; `token` omitted means no credential at all. */ + /** + * POST a JSON-RPC body. A string `token` rides `Authorization: Bearer`, an + * object is sent as the credential headers verbatim, and `undefined` means + * no credential at all. + */ const post = async ( path: string, - token: string | undefined, + token: Credential, body: unknown, ): Promise => { const headers: Record = { "content-type": "application/json", accept: "application/json, text/event-stream", }; - if (token !== undefined) headers.authorization = `Bearer ${token}`; + if (typeof token === "string") headers.authorization = `Bearer ${token}`; + else if (token) Object.assign(headers, token); const res = await fetch(`${app!.proxyUrl}${path}`, { method: "POST", headers, @@ -85,7 +93,7 @@ describe("mcp anonymous access e2e", () => { const initialize = async ( path: string, - token: string | undefined, + token: Credential, ): Promise => { const init = await post(path, token, { jsonrpc: "2.0", @@ -108,7 +116,7 @@ describe("mcp anonymous access e2e", () => { const listToolNames = async ( path: string, - token: string | undefined, + token: Credential, ): Promise<{ status: number; names?: string[] }> => { const status = await initialize(path, token); if (status !== 200) return { status }; @@ -264,6 +272,72 @@ describe("mcp anonymous access e2e", () => { } }); + test("an empty credential header is offered, never anonymous", async (ctx) => { + if (!etcdReachable || !app) return ctx.skip(); + + // A caller that sent a credential header, even an empty one, tried to + // authenticate: serving it as the anonymous principal would hand it a + // grant it did not ask for. + // (fetch trims header values, and HTTP drops a field's surrounding + // whitespace anyway, so a whitespace-only value arrives as this one.) + for (const name of ["x-aisix-api-key", "authorization", "x-api-key"]) { + for (const path of ["/mcp/docs", "/mcp"]) { + expect( + await initialize(path, { [name]: "" }), + `${path} with an empty ${name}`, + ).toBe(401); + } + } + }); + + test("an empty x-aisix-api-key is a 401 naming it, despite a valid Authorization", async (ctx) => { + if (!etcdReachable || !app) return ctx.skip(); + + const creds = { "x-aisix-api-key": "", authorization: `Bearer ${KEY_WILD}` }; + for (const res of [ + await fetch(`${app.proxyUrl}/mcp/docs`, { + method: "POST", + headers: { + ...creds, + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "ping" }), + }), + await fetch(`${app.proxyUrl}/v1/chat/completions`, { + method: "POST", + headers: { ...creds, "content-type": "application/json" }, + body: JSON.stringify({ + model: "any", + messages: [{ role: "user", content: "hi" }], + }), + }), + ]) { + expect(res.status).toBe(401); + const body = (await res.json()) as { error?: { message?: string } }; + expect(body.error?.message).toContain("x-aisix-api-key"); + expect(body.error?.message).not.toContain("Authorization"); + } + }); + + test("a valid x-aisix-api-key authenticates as its own key", async (ctx) => { + if (!etcdReachable || !app) return ctx.skip(); + + const listed = await listToolNames("/mcp", { "x-aisix-api-key": KEY_WILD }); + expect(listed.status).toBe(200); + // The key's own reach, not the anonymous principal's capped one. + expect(listed.names).toEqual([ + "docs__echo", + "docs__reverse", + "kb__echo", + "kb__reverse", + ]); + const models = await fetch(`${app.proxyUrl}/v1/models`, { + headers: { "x-aisix-api-key": KEY_WILD }, + }); + expect(models.status).toBe(200); + }); + test("a valid credential keeps its own identity and reach", async (ctx) => { if (!etcdReachable || !app) return ctx.skip(); diff --git a/tests/e2e/src/cases/passthrough-route-e2e.test.ts b/tests/e2e/src/cases/passthrough-route-e2e.test.ts index 8847b16f..cc2a5dfa 100644 --- a/tests/e2e/src/cases/passthrough-route-e2e.test.ts +++ b/tests/e2e/src/cases/passthrough-route-e2e.test.ts @@ -48,6 +48,8 @@ const CALLER_PLAINTEXT = "sk-ptr-e2e-caller"; const CALLER_KEY_HASH = createHash("sha256") .update(CALLER_PLAINTEXT) .digest("hex"); +const CRED_PLAINTEXT = "sk-ptr-e2e-cred"; +const CRED_KEY_HASH = createHash("sha256").update(CRED_PLAINTEXT).digest("hex"); const STREAM_LIMITED_PLAINTEXT = "sk-ptr-e2e-stream-limit"; const STREAM_LIMITED_KEY_HASH = createHash("sha256") .update(STREAM_LIMITED_PLAINTEXT) @@ -216,6 +218,102 @@ describe("passthrough-route e2e: explicit routes, BYO credentials, unclaimed pat }); }); + test("gateway_key credential reads like /v1: case-insensitive Bearer, no fall-through", async (ctx) => { + if (!etcdReachable || !app || !seed) { + ctx.skip(); + return; + } + + const upstream = await startOpenAiUpstream({ + nonStreamBody: { + id: "chatcmpl-ptr-cred", + object: "chat.completion", + created: 0, + model: "gpt-4o-mini", + choices: [ + { + index: 0, + message: { role: "assistant", content: "ok" }, + finish_reason: "stop", + }, + ], + usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }, + }, + }); + upstreams.push(upstream); + const pk = await seed.createProviderKey({ + display_name: "ptr-cred-pk", + secret: "sk-mock", + api_base: `${upstream.baseUrl}/v1`, + }); + await seed.createModel({ + display_name: "ptr-cred-model", + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: pk.id, + }); + await seed.createPassthroughRoute({ + name: "ptr-cred-tunnel", + path_prefix: "/passthrough/ptr-cred", + target_url: `${upstream.baseUrl}/v1`, + provider_key_id: pk.id, + }); + // Seeded last: its authenticating implies the model and route landed. + await seed.createApiKey({ + key_hash: CRED_KEY_HASH, + allowed_models: ["*"], + allowed_routes: ["*"], + }); + const probe = new ProxyClient(app.proxyUrl, CRED_PLAINTEXT); + await waitConfigPropagation(async () => (await probe.listModels()).status === 200); + + const chat = (path: string, headers: Record) => + fetch(`${app!.proxyUrl}${path}`, { + method: "POST", + headers: { ...headers, "content-type": "application/json" }, + body: JSON.stringify({ + model: "ptr-cred-model", + messages: [{ role: "user", content: "hi" }], + }), + }); + const ROUTE = "/passthrough/ptr-cred/chat/completions"; + const V1 = "/v1/chat/completions"; + + // RFC 7235: the auth scheme name is case-insensitive; the key is not. + for (const path of [ROUTE, V1]) { + for (const scheme of ["Bearer", "bearer", "BEARER"]) { + const res = await chat(path, { authorization: `${scheme} ${CRED_PLAINTEXT}` }); + expect(res.status, `${path} with ${scheme}`).toBe(200); + await res.text(); + } + const upper = await chat(path, { + authorization: `Bearer ${CRED_PLAINTEXT.toUpperCase()}`, + }); + expect(upper.status, `${path} with an upper-cased key`).toBe(401); + await upper.text(); + } + + // A present but unusable Authorization is a 401 naming it, never a + // fall-through to a valid x-api-key — on the route as on /v1. + for (const path of [ROUTE, V1]) { + for (const authorization of ["Basic dXNlcjpwdw==", "Bearer"]) { + const res = await chat(path, { authorization, "x-api-key": CRED_PLAINTEXT }); + expect(res.status, `${path} with ${authorization}`).toBe(401); + const body = (await res.json()) as { error?: { message?: string } }; + expect(body.error?.message).toBe("missing or malformed Authorization header"); + } + } + + // The migration path: the gateway key in x-aisix-api-key leaves + // Authorization to whatever the caller needs it for. + const migrated = await chat(ROUTE, { + "x-aisix-api-key": CRED_PLAINTEXT, + authorization: "Basic dXNlcjpwdw==", + }); + expect(migrated.status).toBe(200); + await migrated.text(); + }); + test("anthropic inject shape: x-api-key + anthropic-version, no Bearer", async (ctx) => { if (!etcdReachable || !app || !seed) { ctx.skip();