diff --git a/crates/trusted-server-adapter-axum/src/middleware.rs b/crates/trusted-server-adapter-axum/src/middleware.rs index 9f00f7614..fd11d7728 100644 --- a/crates/trusted-server-adapter-axum/src/middleware.rs +++ b/crates/trusted-server-adapter-axum/src/middleware.rs @@ -104,8 +104,8 @@ impl AuthMiddleware { #[async_trait(?Send)] impl Middleware for AuthMiddleware { - async fn handle(&self, ctx: RequestContext, next: Next<'_>) -> Result { - match enforce_basic_auth(&self.settings, ctx.request()) { + async fn handle(&self, mut ctx: RequestContext, next: Next<'_>) -> Result { + match enforce_basic_auth(&self.settings, ctx.request_mut()) { Ok(Some(response)) => return Ok(response), Ok(None) => {} Err(report) => { diff --git a/crates/trusted-server-adapter-cloudflare/src/middleware.rs b/crates/trusted-server-adapter-cloudflare/src/middleware.rs index f2b3374c4..14efed56a 100644 --- a/crates/trusted-server-adapter-cloudflare/src/middleware.rs +++ b/crates/trusted-server-adapter-cloudflare/src/middleware.rs @@ -112,8 +112,8 @@ impl AuthMiddleware { #[async_trait(?Send)] impl Middleware for AuthMiddleware { - async fn handle(&self, ctx: RequestContext, next: Next<'_>) -> Result { - match enforce_basic_auth(&self.settings, ctx.request()) { + async fn handle(&self, mut ctx: RequestContext, next: Next<'_>) -> Result { + match enforce_basic_auth(&self.settings, ctx.request_mut()) { Ok(Some(response)) => return Ok(response), Ok(None) => {} Err(report) => { diff --git a/crates/trusted-server-adapter-fastly/src/app.rs b/crates/trusted-server-adapter-fastly/src/app.rs index 5e78a87ed..06a0a155f 100644 --- a/crates/trusted-server-adapter-fastly/src/app.rs +++ b/crates/trusted-server-adapter-fastly/src/app.rs @@ -1300,12 +1300,16 @@ impl Hooks for TrustedServerApp { #[cfg(test)] mod tests { - use std::sync::Arc; + use std::collections::HashMap; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::{Arc, Mutex}; + use std::time::Duration; use super::{ - AppState, NAMED_ROUTES, NamedRouteHandler, PAGE_BIDS_LEGACY_PATH, PAGE_BIDS_PATH, - TrustedServerApp, build_per_request_services, build_state_from_settings, - startup_error_router, + AppState, AuctionDispatch, EcContext, EdgeCacheHeader, HandlerFuture, NAMED_ROUTES, + NamedRouteHandler, PAGE_BIDS_LEGACY_PATH, PAGE_BIDS_PATH, TrustedServerApp, + build_per_request_services, build_state_from_settings, handle_publisher_request, + publisher_response_into_streaming_response, startup_error_router, }; use base64::Engine as _; use bytes::Bytes; @@ -1316,7 +1320,6 @@ mod tests { use edgezero_core::params::PathParams; use edgezero_core::router::RouterService; use std::net::{IpAddr, Ipv4Addr}; - use std::sync::Mutex; use error_stack::Report; use futures::executor::block_on; @@ -1331,7 +1334,9 @@ mod tests { use trusted_server_core::platform::{ ClientInfo, PlatformBackend, PlatformBackendSpec, PlatformError, PlatformHttpClient, PlatformHttpRequest, PlatformKvStore, PlatformPendingRequest, PlatformResponse, - PlatformSelectResult, RuntimeServices, + PlatformSelectResult, PlatformTemplateCache, PlatformTemplateCacheReservation, + RuntimeServices, TemplateCacheError, TemplateCacheKey, TemplateCacheLookup, + TemplateCacheMiss, TemplateCacheReservation, TemplateEntry, TemplateMetadata, }; use trusted_server_core::settings::Settings; @@ -2611,6 +2616,296 @@ mod tests { .build() } + #[derive(Default)] + struct DispatchTemplateCache { + entries: Arc>>, + } + + struct DispatchTemplateReservation { + entries: Arc>>, + key: TemplateCacheKey, + } + + impl PlatformTemplateCacheReservation for DispatchTemplateReservation { + fn insert( + self: Box, + metadata: &TemplateMetadata, + body: Vec, + _max_age: Duration, + ) -> Result<(), TemplateCacheError> { + self.entries.lock().expect("should lock entries").insert( + self.key.to_cache_key(), + TemplateEntry { + metadata: metadata.clone(), + body, + }, + ); + Ok(()) + } + + fn cancel(self: Box) -> Result<(), TemplateCacheError> { + Ok(()) + } + } + + #[async_trait::async_trait(?Send)] + impl PlatformTemplateCache for DispatchTemplateCache { + async fn lookup_or_reserve( + &self, + key: &TemplateCacheKey, + ) -> Result { + if let Some(entry) = self + .entries + .lock() + .expect("should lock entries") + .get(&key.to_cache_key()) + .cloned() + { + return Ok(TemplateCacheLookup::Hit(entry)); + } + Ok(TemplateCacheLookup::Reserved( + TemplateCacheReservation::new(Box::new(DispatchTemplateReservation { + entries: Arc::clone(&self.entries), + key: key.clone(), + })), + )) + } + + async fn get(&self, key: &TemplateCacheKey) -> Result { + self.entries + .lock() + .expect("should lock entries") + .get(&key.to_cache_key()) + .cloned() + .ok_or(TemplateCacheMiss::NotFound) + } + + async fn put( + &self, + key: &TemplateCacheKey, + metadata: &TemplateMetadata, + body: Vec, + _max_age: Duration, + ) -> Result<(), TemplateCacheError> { + self.entries.lock().expect("should lock entries").insert( + key.to_cache_key(), + TemplateEntry { + metadata: metadata.clone(), + body, + }, + ); + Ok(()) + } + + async fn purge_url(&self, key: &TemplateCacheKey) -> Result<(), TemplateCacheError> { + self.entries + .lock() + .expect("should lock entries") + .remove(&key.to_cache_key()); + Ok(()) + } + + async fn purge_all(&self) -> Result<(), TemplateCacheError> { + self.entries.lock().expect("should lock entries").clear(); + Ok(()) + } + } + + #[derive(Default)] + struct DispatchOriginClient { + calls: AtomicUsize, + } + + #[async_trait::async_trait(?Send)] + impl PlatformHttpClient for DispatchOriginClient { + async fn send( + &self, + _request: PlatformHttpRequest, + ) -> Result> { + self.calls.fetch_add(1, Ordering::Relaxed); + let response = edgezero_core::http::response_builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "text/html; charset=utf-8") + .header(header::CACHE_CONTROL, "public, max-age=300") + .body(Body::from( + b"origin".as_ref(), + )) + .map_err(|_| Report::new(PlatformError::HttpClient))?; + Ok(PlatformResponse::new(response)) + } + + async fn send_async( + &self, + _request: PlatformHttpRequest, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + + async fn select( + &self, + _pending_requests: Vec, + ) -> Result> { + Err(Report::new(PlatformError::Unsupported)) + } + } + + #[test] + fn dispatch_edge_authenticated_esi_request_stores_then_hits_template() { + let settings = Arc::new( + Settings::from_toml( + r#" + [[handlers]] + path = "^/secure" + username = "user" + password = "pass" + + [[handlers]] + path = "^/_ts/admin" + username = "admin" + password = "admin-pass" + + [publisher] + domain = "test-publisher.com" + cookie_domain = ".test-publisher.com" + origin_url = "https://origin.test-publisher.com" + proxy_secret = "unit-test-proxy-secret" + + [ec] + passphrase = "test-secret-key-32-bytes-minimum" + + [auction] + enabled = true + providers = [] + + [creative_opportunities] + gam_network_id = "99999" + assembly_mode = "esi" + + [[creative_opportunities.slot]] + id = "test-slot" + page_patterns = ["/secure/article"] + formats = [{ width = 728, height = 90 }] + "#, + ) + .expect("should parse dispatch cache settings"), + ); + let cache = Arc::new(DispatchTemplateCache::default()); + let origin = Arc::new(DispatchOriginClient::default()); + let services = RuntimeServices::builder() + .config_store(Arc::new(crate::platform::FastlyPlatformConfigStore)) + .secret_store(Arc::new(crate::platform::FastlyPlatformSecretStore)) + .kv_store(Arc::new(NoopKvStore) as Arc) + .template_cache(Arc::clone(&cache) as Arc) + .template_assembler(Arc::new(crate::esi_assembly::FastlyTemplateAssembler)) + .backend(Arc::new(FixedBackend)) + .http_client(Arc::clone(&origin) as Arc) + .geo(Arc::new(crate::platform::FastlyPlatformGeo)) + .client_info(ClientInfo::default()) + .build(); + let registry = Arc::new( + IntegrationRegistry::new(&settings).expect("should build integration registry"), + ); + let orchestrator = Arc::new( + trusted_server_core::auction::build_orchestrator(&settings) + .expect("should build auction orchestrator"), + ); + + let handler = { + let settings = Arc::clone(&settings); + let services = services.clone(); + let registry = Arc::clone(®istry); + let orchestrator = Arc::clone(&orchestrator); + move |ctx: RequestContext| { + let settings = Arc::clone(&settings); + let services = services.clone(); + let registry = Arc::clone(®istry); + let orchestrator = Arc::clone(&orchestrator); + Box::pin(async move { + let request = ctx.into_request(); + let method = request.method().clone(); + let mut ec_context = + match EcContext::read_from_request(&settings, &request, &services) { + Ok(context) => context, + Err(report) => return Ok(super::http_error(&report)), + }; + let response = match handle_publisher_request( + &settings, + &services, + None, + &mut ec_context, + AuctionDispatch { + orchestrator: &orchestrator, + slots: settings.creative_opportunity_slots(), + registry: None, + }, + request, + EdgeCacheHeader::SurrogateControl, + ) + .await + { + Ok(response) => response, + Err(report) => return Ok(super::http_error(&report)), + }; + match publisher_response_into_streaming_response( + response, + &method, + Arc::clone(&settings), + ®istry, + orchestrator, + services, + ) + .await + { + Ok(response) => Ok(response), + Err(report) => Ok(super::http_error(&report)), + } + }) as HandlerFuture + } + }; + let router = RouterService::builder() + .middleware(crate::middleware::AuthMiddleware::new(Arc::clone( + &settings, + ))) + .route("/secure/article", Method::GET, handler) + .build(); + let authorized_request = || { + request_builder() + .method(Method::GET) + .uri("https://test-publisher.com/secure/article") + .header(header::HOST, "test-publisher.com") + .header(header::AUTHORIZATION, "Basic dXNlcjpwYXNz") + .header("sec-fetch-dest", "document") + .header("sec-fetch-mode", "navigate") + .body(Body::empty()) + .expect("should build authorized navigation") + }; + + let cold = route(&router, authorized_request()); + assert_eq!( + cold.headers() + .get("x-ts-template-cache") + .and_then(|value| value.to_str().ok()), + Some("miss-stored") + ); + block_on(cold.into_body().into_bytes_bounded(1024 * 1024)) + .expect("should drain cold response"); + + let warm = route(&router, authorized_request()); + assert_eq!( + warm.headers() + .get("x-ts-template-cache") + .and_then(|value| value.to_str().ok()), + Some("hit") + ); + block_on(warm.into_body().into_bytes_bounded(1024 * 1024)) + .expect("should drain warm response"); + assert_eq!( + origin.calls.load(Ordering::Relaxed), + 1, + "the warm dispatch must not fetch the publisher origin" + ); + } + #[test] fn dispatch_asset_fallback_streams_origin_body_without_buffering() { // Regression guard for the EdgeZero asset streaming cutover: a successful diff --git a/crates/trusted-server-adapter-fastly/src/middleware.rs b/crates/trusted-server-adapter-fastly/src/middleware.rs index 8329c46b1..283f16255 100644 --- a/crates/trusted-server-adapter-fastly/src/middleware.rs +++ b/crates/trusted-server-adapter-fastly/src/middleware.rs @@ -127,8 +127,8 @@ impl AuthMiddleware { #[async_trait(?Send)] impl Middleware for AuthMiddleware { - async fn handle(&self, ctx: RequestContext, next: Next<'_>) -> Result { - match enforce_basic_auth(&self.settings, ctx.request()) { + async fn handle(&self, mut ctx: RequestContext, next: Next<'_>) -> Result { + match enforce_basic_auth(&self.settings, ctx.request_mut()) { Ok(Some(response)) => return Ok(response), Ok(None) => {} Err(report) => { diff --git a/crates/trusted-server-adapter-spin/src/middleware.rs b/crates/trusted-server-adapter-spin/src/middleware.rs index d3005d510..d7a09987a 100644 --- a/crates/trusted-server-adapter-spin/src/middleware.rs +++ b/crates/trusted-server-adapter-spin/src/middleware.rs @@ -105,8 +105,8 @@ impl AuthMiddleware { #[async_trait(?Send)] impl Middleware for AuthMiddleware { - async fn handle(&self, ctx: RequestContext, next: Next<'_>) -> Result { - match enforce_basic_auth(&self.settings, ctx.request()) { + async fn handle(&self, mut ctx: RequestContext, next: Next<'_>) -> Result { + match enforce_basic_auth(&self.settings, ctx.request_mut()) { Ok(Some(response)) => return Ok(response), Ok(None) => {} Err(report) => { diff --git a/crates/trusted-server-core/src/auth.rs b/crates/trusted-server-core/src/auth.rs index 6c92d042d..f5e45bbd3 100644 --- a/crates/trusted-server-core/src/auth.rs +++ b/crates/trusted-server-core/src/auth.rs @@ -11,6 +11,38 @@ use crate::settings::Settings; const BASIC_AUTH_REALM: &str = r#"Basic realm="Trusted Server""#; +/// Marks the single `Authorization` value Trusted Server validated. +/// +/// The shared template cache may exempt this value from its normal authorization +/// bypass. [`enforce_basic_auth`] clears any existing marker before checking and +/// inserts a digest-bound marker only after successful authentication. +#[derive(Debug, Clone)] +pub(crate) struct EdgeTerminatedAuthorization([u8; 32]); + +impl EdgeTerminatedAuthorization { + fn digest(value: &[u8]) -> [u8; 32] { + Sha256::digest(value).into() + } + + pub(crate) fn matches(&self, headers: &http::HeaderMap) -> bool { + let mut values = headers.get_all(header::AUTHORIZATION).iter(); + let Some(value) = values.next() else { + return false; + }; + values.next().is_none() && self.0 == Self::digest(value.as_bytes()) + } + + /// Builds the marker without performing a credential check. + /// + /// Test-only. Production code obtains this marker exclusively by passing + /// [`enforce_basic_auth`], which is what makes it meaningful. + #[cfg(test)] + #[must_use] + pub(crate) fn for_test(value: &str) -> Self { + Self(Self::digest(value.as_bytes())) + } +} + /// Enforces HTTP Basic authentication for configured handler paths. /// /// Returns `Ok(None)` when the request does not target a protected handler or @@ -24,14 +56,36 @@ const BASIC_AUTH_REALM: &str = r#"Basic realm="Trusted Server""#; /// the reserved admin namespace fail closed if no handler matches, providing /// defense in depth for malformed and parameterized paths. /// +/// # Request mutation +/// +/// Takes `req` mutably because it owns [`EdgeTerminatedAuthorization`]. Any +/// inherited marker is cleared on entry, and a fresh one is inserted only on the +/// success path, so the marker present after this call always describes this +/// call's own decision. Nothing else about the request is touched — in +/// particular the `Authorization` header is left in place and still reaches the +/// publisher origin. +/// +/// That last point is a stated assumption: a credential this edge terminates is +/// treated as reader-neutral, which holds unless the origin *also* authenticates +/// on the same header. An origin that does so declares `Vary: Authorization`, +/// which the template-cache store refuses as an uncovered `Vary` name. An origin +/// that varies on `Authorization` without declaring it would defeat any HTTP +/// cache, and is out of scope here. +/// /// # Errors /// /// Returns an error when handler configuration is invalid, such as an /// un-compilable path regex. pub fn enforce_basic_auth( settings: &Settings, - req: &Request, + req: &mut Request, ) -> Result>, Report> { + // Cleared before any early return so no inherited marker can survive a call + // that did not itself validate a credential. Without this, a request marked + // upstream and then routed to an unprotected path would keep an assertion + // nothing checked. + req.extensions_mut().remove::(); + let path = req.uri().path(); let Some(handler) = settings.handler_for_path(path)? else { if Settings::is_admin_path(path) { @@ -42,7 +96,7 @@ pub fn enforce_basic_auth( return Ok(None); }; - let Some((username, password)) = extract_credentials(req) else { + let Some((username, password, authorization_digest)) = extract_credentials(req) else { return Ok(Some(unauthorized_response())); }; @@ -59,6 +113,10 @@ pub fn enforce_basic_auth( .ct_eq(&Sha256::digest(password.as_bytes())); if bool::from(username_match & password_match) { + // Record that TS itself consumed this credential, so the shared template + // cache can distinguish it from a credential meant for the origin. + req.extensions_mut() + .insert(EdgeTerminatedAuthorization(authorization_digest)); Ok(None) } else { log::warn!("Basic auth failed for path: {}", req.uri().path()); @@ -66,11 +124,14 @@ pub fn enforce_basic_auth( } } -fn extract_credentials(req: &Request) -> Option<(String, String)> { - let header_value = req - .headers() - .get(header::AUTHORIZATION) - .and_then(|value| value.to_str().ok())?; +fn extract_credentials(req: &Request) -> Option<(String, String, [u8; 32])> { + let mut header_values = req.headers().get_all(header::AUTHORIZATION).iter(); + let header_value = header_values.next()?; + if header_values.next().is_some() { + return None; + } + let authorization_digest = EdgeTerminatedAuthorization::digest(header_value.as_bytes()); + let header_value = header_value.to_str().ok()?; let mut parts = header_value.splitn(2, ' '); let scheme = parts.next()?.trim(); @@ -90,7 +151,7 @@ fn extract_credentials(req: &Request) -> Option<(String, String)> { let username = credentials_parts.next()?.to_owned(); let password = credentials_parts.next()?.to_owned(); - Some((username, password)) + Some((username, password, authorization_digest)) } fn unauthorized_response() -> Response { @@ -134,9 +195,9 @@ mod tests { let settings = create_test_settings(); for path in ["/_ts/admin%2Fec", "/_ts/admin%2fec"] { - let req = build_request(Method::GET, &format!("https://example.com{path}")); + let mut req = build_request(Method::GET, &format!("https://example.com{path}")); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .unwrap_or_else(|| panic!("should challenge {path}")); @@ -148,13 +209,159 @@ mod tests { } } + #[test] + fn valid_credentials_mark_the_request_as_edge_terminated() { + let settings = create_test_settings(); + let mut req = build_request(Method::GET, "https://example.com/secure"); + let encoded = STANDARD.encode("user:pass"); + set_authorization(&mut req, &format!("Basic {encoded}")); + + assert!( + enforce_basic_auth(&settings, &mut req) + .expect("should evaluate auth") + .is_none(), + "valid credentials should be admitted" + ); + let marker = req + .extensions() + .get::() + .expect("should mark a credential this edge consumed"); + assert!( + marker.matches(req.headers()), + "the marker should match the unchanged validated authorization" + ); + + req.headers_mut().insert( + header::AUTHORIZATION, + HeaderValue::from_static("Bearer publisher-origin-credential"), + ); + let marker = req + .extensions() + .get::() + .expect("should retain the marker after an unrelated mutation"); + assert!( + !marker.matches(req.headers()), + "the marker must not match a replacement authorization value" + ); + } + + #[test] + fn repeated_authorization_values_are_rejected_without_a_marker() { + let settings = create_test_settings(); + let mut req = build_request(Method::GET, "https://example.com/secure"); + let encoded = STANDARD.encode("user:pass"); + set_authorization(&mut req, &format!("Basic {encoded}")); + req.headers_mut().append( + header::AUTHORIZATION, + HeaderValue::from_static("Bearer publisher-origin-credential"), + ); + + let response = enforce_basic_auth(&settings, &mut req) + .expect("should evaluate auth") + .expect("should challenge an ambiguous credential"); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!( + req.extensions() + .get::() + .is_none(), + "a repeated authorization field must remain pass-through rather than being marked safe" + ); + } + + #[test] + fn an_inherited_marker_is_cleared_on_an_unprotected_path() { + // The marker asserts "this edge already checked a credential". A request + // routed to a path no handler protects was never checked here, so a marker + // it arrived with must not survive to grant shared-template eligibility. + let settings = create_test_settings(); + let mut req = build_request(Method::GET, "https://example.com/open"); + set_authorization(&mut req, "Basic dXNlcjpwYXNz"); + req.extensions_mut() + .insert(EdgeTerminatedAuthorization::for_test("Basic dXNlcjpwYXNz")); + + assert!( + enforce_basic_auth(&settings, &mut req) + .expect("should evaluate auth") + .is_none(), + "an unprotected path should not challenge" + ); + assert!( + req.extensions() + .get::() + .is_none(), + "a marker no credential check produced must not survive this call" + ); + } + + #[test] + fn an_inherited_marker_is_cleared_when_credentials_are_rejected() { + let settings = create_test_settings(); + let mut req = build_request(Method::GET, "https://example.com/secure"); + let encoded = STANDARD.encode("user:wrong-pass"); + set_authorization(&mut req, &format!("Basic {encoded}")); + req.extensions_mut() + .insert(EdgeTerminatedAuthorization::for_test( + "Basic dXNlcjp3cm9uZy1wYXNz", + )); + + let response = enforce_basic_auth(&settings, &mut req) + .expect("should evaluate auth") + .expect("should challenge"); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!( + req.extensions() + .get::() + .is_none(), + "a failed check must strip an inherited marker rather than honour it" + ); + } + + #[test] + fn a_non_protected_path_leaves_authorization_unmarked() { + let settings = create_test_settings(); + let mut req = build_request(Method::GET, "https://example.com/open"); + set_authorization(&mut req, "Basic dXNlcjpwYXNz"); + + assert!( + enforce_basic_auth(&settings, &mut req) + .expect("should evaluate auth") + .is_none(), + "an unprotected path should not challenge" + ); + assert!( + req.extensions() + .get::() + .is_none(), + "a credential no handler consumed is pass-through and must stay disqualifying" + ); + } + + #[test] + fn rejected_credentials_leave_the_request_unmarked() { + let settings = create_test_settings(); + let mut req = build_request(Method::GET, "https://example.com/secure"); + let encoded = STANDARD.encode("user:wrong-pass"); + set_authorization(&mut req, &format!("Basic {encoded}")); + + let response = enforce_basic_auth(&settings, &mut req) + .expect("should evaluate auth") + .expect("should challenge"); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!( + req.extensions() + .get::() + .is_none(), + "a failed credential must never be marked as terminated" + ); + } + #[test] fn no_challenge_for_non_protected_path() { let settings = create_test_settings(); - let req = build_request(Method::GET, "https://example.com/open"); + let mut req = build_request(Method::GET, "https://example.com/open"); assert!( - enforce_basic_auth(&settings, &req) + enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .is_none() ); @@ -163,9 +370,9 @@ mod tests { #[test] fn challenge_when_missing_credentials() { let settings = create_test_settings(); - let req = build_request(Method::GET, "https://example.com/secure"); + let mut req = build_request(Method::GET, "https://example.com/secure"); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .expect("should challenge"); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); @@ -184,7 +391,7 @@ mod tests { set_authorization(&mut req, &format!("Basic {token}")); assert!( - enforce_basic_auth(&settings, &req) + enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .is_none() ); @@ -197,7 +404,7 @@ mod tests { let token = STANDARD.encode("wrong:wrong"); set_authorization(&mut req, &format!("Basic {token}")); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .expect("should challenge"); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); @@ -211,7 +418,7 @@ mod tests { let token = STANDARD.encode("wrong-user:pass"); set_authorization(&mut req, &format!("Basic {token}")); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .expect("should challenge"); assert_eq!( @@ -228,7 +435,7 @@ mod tests { let token = STANDARD.encode("user:wrong-pass"); set_authorization(&mut req, &format!("Basic {token}")); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .expect("should challenge"); assert_eq!( @@ -244,7 +451,7 @@ mod tests { let mut req = build_request(Method::GET, "https://example.com/secure"); set_authorization(&mut req, "Bearer token"); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .expect("should challenge"); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); @@ -269,7 +476,7 @@ mod tests { set_authorization(&mut req, &format!("Basic {token}")); assert!( - enforce_basic_auth(&settings, &req) + enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .is_none(), "should allow admin path with correct credentials" @@ -283,7 +490,7 @@ mod tests { let token = STANDARD.encode("admin:wrong"); set_authorization(&mut req, &format!("Basic {token}")); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .expect("should challenge admin path with wrong credentials"); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); @@ -312,9 +519,9 @@ mod tests { "https://example.com/_ts/page-bids?path=/article", "https://example.com/_ts/api/v1/identify", ] { - let req = build_request(Method::GET, path); + let mut req = build_request(Method::GET, path); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .unwrap_or_else(|| panic!("should challenge {path} under a `^/_ts` handler")); assert_eq!( @@ -328,9 +535,9 @@ mod tests { #[test] fn challenge_admin_path_with_missing_credentials() { let settings = create_test_settings(); - let req = build_request(Method::POST, "https://example.com/_ts/admin/keys/rotate"); + let mut req = build_request(Method::POST, "https://example.com/_ts/admin/keys/rotate"); - let response = enforce_basic_auth(&settings, &req) + let response = enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .expect("should challenge admin path with missing credentials"); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); @@ -354,12 +561,12 @@ mod tests { let settings: Settings = toml::from_str(&config).expect("should deserialize settings without finalization"); let ec_id = format!("{}.abc123", "a".repeat(64)); - let req = build_request( + let mut req = build_request( Method::GET, &format!("https://example.com/_ts/admin/ec/{ec_id}"), ); - let error = enforce_basic_auth(&settings, &req) + let error = enforce_basic_auth(&settings, &mut req) .expect_err("should fail closed without a matching admin handler"); assert!( error.to_string().contains("no configured handler"), @@ -375,10 +582,10 @@ mod tests { ); let settings: Settings = toml::from_str(&config).expect("should deserialize settings without finalization"); - let req = build_request(Method::GET, "https://example.com/_ts/administrator"); + let mut req = build_request(Method::GET, "https://example.com/_ts/administrator"); assert!( - enforce_basic_auth(&settings, &req) + enforce_basic_auth(&settings, &mut req) .expect("should evaluate auth") .is_none(), "should not classify a similar prefix as the admin namespace" diff --git a/crates/trusted-server-core/src/creative_opportunities.rs b/crates/trusted-server-core/src/creative_opportunities.rs index e83fef9cb..ab272e4da 100644 --- a/crates/trusted-server-core/src/creative_opportunities.rs +++ b/crates/trusted-server-core/src/creative_opportunities.rs @@ -313,10 +313,10 @@ pub struct CreativeOpportunitiesConfig { /// /// **Unset or empty means no operator-stated header is covered, so any origin /// `Vary` other than structurally covered `Accept-Encoding` disqualifies the - /// response.** `Cookie` may never be configured: a per-cookie object violates the - /// reader-neutral template contract. This fail-closed default prevents a deployment - /// that has not stated what its origin varies on from gaining a shared cache by - /// omission. + /// response.** `Cookie` and `Authorization` may never be configured: their values + /// are not reader-neutral template dimensions. This fail-closed default prevents a + /// deployment that has not stated what its origin varies on from gaining a shared + /// cache by omission. /// /// Spike-only. Same `Option` + `skip_serializing_if` reasoning as `assembly_mode`. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -479,6 +479,15 @@ impl CreativeOpportunitiesConfig { .to_string(), ); } + if names + .iter() + .any(|name| name.eq_ignore_ascii_case("authorization")) + { + return Err( + "template_cache_vary must not include Authorization; shared templates are keyed on the edge-terminated credential decision, not the header value" + .to_string(), + ); + } } // A network ID is required only when a slot renders the default @@ -2107,6 +2116,20 @@ mod tests { .validate_runtime() .expect_err("per-cookie templates violate the reader-neutral shared-template contract"); assert!(err.contains("Cookie"), "unexpected error: {err}"); + + for name in ["Authorization", "aUtHoRiZaTiOn"] { + let authorization_key: CreativeOpportunitiesConfig = toml::from_str(&format!( + r#" + gam_network_id = "99999" + template_cache_vary = ["{name}"] + "#, + )) + .expect("shape should deserialize before runtime validation"); + let err = authorization_key + .validate_runtime() + .expect_err("authorization must not enter shared-template cache keys"); + assert!(err.contains("Authorization"), "unexpected error: {err}"); + } } #[test] diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index b0d63b82a..40bb7ea27 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -4340,10 +4340,18 @@ pub async fn handle_publisher_request( } ); - // Recorded before the request is consumed by the origin send: the template cache gate - // below needs it, and an authorized response must never become a shared - // template. - let request_had_authorization = req.headers().contains_key(header::AUTHORIZATION); + // Capture cache eligibility before the origin send consumes the request. One + // unchanged marked Authorization value passed edge auth; unmarked, replaced, + // or repeated values remain pass-through and bypass sharing. + let authorization_value_count = req.headers().get_all(header::AUTHORIZATION).iter().count(); + let authorization_disqualifies = match authorization_value_count { + 0 => false, + 1 => req + .extensions() + .get::() + .is_none_or(|marker| !marker.matches(req.headers())), + _ => true, + }; let request_had_cookie = req.headers().contains_key(header::COOKIE); // Whether carrying a cookie is itself disqualifying. Computed once and used for both // the lookup and the store, so the two cannot drift apart. @@ -4389,7 +4397,7 @@ pub async fn handle_publisher_request( let request_can_use_shared_template = method_is_cacheable && matches!(assembly_mode, AssemblyMode::Esi) && !request_host.is_empty() - && !request_had_authorization + && !authorization_disqualifies && !cookie_disqualifies && !request_requires_origin && reader_supports_assembly; @@ -4635,7 +4643,7 @@ pub async fn handle_publisher_request( let mut template_cache_key = template_cache_reservation.and_then(|reservation| { match template_cache_ttl( assembly_mode, - request_had_authorization, + authorization_disqualifies, cookie_disqualifies, response.status(), &gate_content_type, @@ -5722,7 +5730,7 @@ impl TemplateCachePolicy { #[cfg(test)] pub(crate) fn template_cache_bypass_reason( mode: AssemblyMode, - request_had_authorization: bool, + authorization_disqualifies: bool, cookie_disqualifies: bool, status: StatusCode, content_type: &str, @@ -5732,7 +5740,7 @@ pub(crate) fn template_cache_bypass_reason( let policy = TemplateCachePolicy::for_test(key_vary, Duration::from_secs(60)); template_cache_ttl( mode, - request_had_authorization, + authorization_disqualifies, cookie_disqualifies, status, content_type, @@ -5988,7 +5996,7 @@ fn replayable_policy_headers( fn template_cache_ttl( mode: AssemblyMode, - request_had_authorization: bool, + authorization_disqualifies: bool, cookie_disqualifies: bool, status: StatusCode, content_type: &str, @@ -5998,7 +6006,7 @@ fn template_cache_ttl( if matches!(mode, AssemblyMode::Inline) { return Err(TemplateCacheBypassReason::InlineMode); } - if request_had_authorization { + if authorization_disqualifies { return Err(TemplateCacheBypassReason::AuthorizedRequest); } if cookie_disqualifies { @@ -8973,6 +8981,178 @@ mod tests { ); } + #[tokio::test] + async fn a_pass_through_authorization_never_reaches_the_shared_template() { + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let settings = Arc::new(settings_with_mode("esi")); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + queue_shareable_html(&stub); + + let mut request = navigation_request(); + request.headers_mut().insert( + header::AUTHORIZATION, + HeaderValue::from_static("Basic dXNlcjpwYXNz"), + ); + + let response = run(&settings, &services, request).await; + assert_eq!( + response + .headers() + .get(HEADER_X_TS_TEMPLATE_CACHE) + .and_then(|value| value.to_str().ok()), + Some("bypass-request"), + "a credential TS did not terminate is bound for the origin, so its response must never be shared" + ); + assert_eq!( + cache.lookups.lock().expect("should lock lookups").len(), + 0, + "an unterminated authorization must bypass before the lookup, not after it" + ); + } + + #[tokio::test] + async fn repeated_authorization_never_reaches_the_shared_template() { + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let settings = Arc::new(settings_with_mode("esi")); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + queue_shareable_html(&stub); + + let mut request = navigation_request(); + request.headers_mut().insert( + header::AUTHORIZATION, + HeaderValue::from_static("Basic dXNlcjpwYXNz"), + ); + request.headers_mut().append( + header::AUTHORIZATION, + HeaderValue::from_static("Bearer publisher-origin-credential"), + ); + request + .extensions_mut() + .insert(crate::auth::EdgeTerminatedAuthorization::for_test( + "Basic dXNlcjpwYXNz", + )); + + let response = run(&settings, &services, request).await; + assert_eq!( + response + .headers() + .get(HEADER_X_TS_TEMPLATE_CACHE) + .and_then(|value| value.to_str().ok()), + Some("bypass-request"), + "an ambiguous authorization must remain bound for the origin even if a marker is present" + ); + assert_eq!( + cache.lookups.lock().expect("should lock lookups").len(), + 0, + "repeated authorization must bypass before the shared template lookup" + ); + } + + #[tokio::test] + async fn a_replaced_edge_terminated_authorization_bypasses_the_shared_template() { + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let mut settings = settings_with_mode("esi"); + settings.handlers[0] = toml::from_str( + r#" + path = "^/article" + username = "user" + password = "pass" + "#, + ) + .expect("should parse article auth handler"); + let settings = Arc::new(settings); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + queue_shareable_html(&stub); + + let mut request = navigation_request(); + request.headers_mut().insert( + header::AUTHORIZATION, + HeaderValue::from_static("Basic dXNlcjpwYXNz"), + ); + assert!( + crate::auth::enforce_basic_auth(&settings, &mut request) + .expect("should evaluate auth") + .is_none(), + "the original credential should pass edge auth" + ); + request.headers_mut().insert( + header::AUTHORIZATION, + HeaderValue::from_static("Bearer publisher-origin-credential"), + ); + + let response = run(&settings, &services, request).await; + assert_eq!( + response + .headers() + .get(HEADER_X_TS_TEMPLATE_CACHE) + .and_then(|value| value.to_str().ok()), + Some("bypass-request"), + "a replacement credential was not validated at the edge and must bypass sharing" + ); + assert_eq!( + cache.lookups.lock().expect("should lock lookups").len(), + 0, + "a replaced authorization must bypass before the shared template lookup" + ); + } + + #[tokio::test] + async fn an_edge_terminated_authorization_still_shares_its_template() { + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let settings = Arc::new(settings_with_mode("esi")); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + + // One origin response for two requests: the warm read is asserted by the + // fixture running dry, exactly as in the unauthenticated case. + queue_shareable_html(&stub); + + let authorized_navigation = || { + let mut request = navigation_request(); + request.headers_mut().insert( + header::AUTHORIZATION, + HeaderValue::from_static("Basic dXNlcjpwYXNz"), + ); + request.extensions_mut().insert( + crate::auth::EdgeTerminatedAuthorization::for_test("Basic dXNlcjpwYXNz"), + ); + request + }; + + let cold = run(&settings, &services, authorized_navigation()).await; + assert_eq!( + cold.headers() + .get(HEADER_X_TS_TEMPLATE_CACHE) + .and_then(|value| value.to_str().ok()), + Some("miss-stored"), + "a credential this edge terminated must be allowed to fill the shared template" + ); + let first = body_of(cold).await; + + let warm = run(&settings, &services, authorized_navigation()).await; + assert_eq!( + warm.headers() + .get(HEADER_X_TS_TEMPLATE_CACHE) + .and_then(|value| value.to_str().ok()), + Some("hit"), + "the second gated request must read the template the first one stored" + ); + let second = body_of(warm).await; + + assert_eq!( + stub.recorded_request_uris().len(), + 1, + "the warm gated request must not fetch the origin" + ); + assert_eq!( + second, first, + "the gated warm response must be byte-identical to the stored template" + ); + } + #[tokio::test] async fn only_the_cold_miss_uses_the_platform_assembler() { let stub = Arc::new(StubHttpClient::new()); diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 33afb0feb..068518aee 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -753,6 +753,11 @@ from ID-shaped samples: a pattern such as matcher (`^/_ts/admin`, or `^/_ts/admin/ec/` alongside the other admin patterns). +Handler expressions match the raw URI path, while a publisher origin may decode +percent-encoded aliases before routing. For a whole-site staging gate, use +`path = "^/"`; do not rely on a decoded-path prefix such as `^/secure` to protect +equivalent origin paths. + Startup also fails when any handler — admin or not — uses a placeholder or well-known weak password (`changeme`, `password`, `admin`, or a `replace-with-…` template value). Handler selection is first-match-wins, so a @@ -1706,9 +1711,10 @@ The cache fails closed. A template is stored only for a `GET` with a processable `200 text/html` origin response, a supported content encoding, and explicit positive shared freshness. `private`, `no-store`, `no-cache`, exhausted or malformed freshness, `Set-Cookie`, `Vary: *`, `Vary: Cookie`, uncovered `Vary` -names, response-bound CSP nonces, authorization, diagnostics sessions, range or -conditional requests, positive or malformed request `max-age`, `min-fresh`, and -unsupported CDN-specific cache policy fields all bypass the template cache. Fastly +names, response-bound CSP nonces, pass-through or ambiguous authorization, +diagnostics sessions, range or conditional requests, positive or malformed +request `max-age`, `min-fresh`, and unsupported CDN-specific cache policy fields +all bypass the template cache. Fastly `Surrogate-Control` is the narrow exception: the template cache accepts exactly one positive `max-age` plus optional valid `stale-while-revalidate` and `stale-if-error` delta-seconds. Restrictive, duplicated, malformed, or unknown directives fail @@ -1725,6 +1731,14 @@ response and runs a new per-reader auction. Explicit `no-cache`, `no-store`, positive or malformed request `max-age`, range, and conditional requests still bypass the template cache. Check `X-TS-Template-Cache: hit` to verify template reuse. +Authorization has one narrow exception. A request carrying exactly the same +single Basic credential that Trusted Server just validated at the edge may share +a template; pass-through, repeated, appended, or replaced values still bypass. +Trusted Server does not remove the validated header, so it remains forwarded to +the publisher origin. If the origin uses that credential to select response +content, it must declare `Vary: Authorization`; that response is deliberately not +stored as a shared template. + `template_cache_vary` is necessary because lookup occurs before the origin can return `Vary`. Presence, empty values, repeated raw field values, host/scheme, origin identity, complete template-shaping settings, TSJS content, and schema @@ -1733,13 +1747,14 @@ not: the stored template is decoded identity and the assembled result is encoded for each reader with `Vary: Accept-Encoding`. This assumes the origin's `Accept-Encoding` variants differ only by HTTP content coding, as normal compression negotiation does. Do not enable ESI for an origin that changes the -document's meaning based on `Accept-Encoding`. Never put `Cookie` in -`template_cache_vary`; startup rejects it because a per-cookie object is not a -reader-neutral template. With `origin_is_cookie_independent = false` (the safe -default), all cookie-bearing requests bypass. With it set to `true`, an origin -`Vary: Cookie` still overrides the assertion and refuses storage. -Every other name the origin emits in `Vary` must appear in the configured list; -an uncovered name safely refuses template storage. +document's meaning based on `Accept-Encoding`. Never put `Cookie` or +`Authorization` in `template_cache_vary`; startup rejects both because raw cookie +or credential values are not reader-neutral template dimensions. With +`origin_is_cookie_independent = false` (the safe default), all cookie-bearing +requests bypass. With it set to `true`, an origin `Vary: Cookie` still overrides +the assertion and refuses storage. Every other name the origin emits in `Vary` +must appear in the configured list; an uncovered name safely refuses template +storage. For a canary, inspect `X-TS-Template-Cache`. Its bounded values are `hit`, `miss-stored`, `miss-store-error`, `miss-reserved`, `bypass-request`,