-
Notifications
You must be signed in to change notification settings - Fork 13
Complete removal of the legacy consent KV path #903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,7 +147,7 @@ use trusted_server_core::tester_cookie::{handle_clear_tester, handle_set_tester} | |
| use crate::middleware::{AuthMiddleware, FinalizeResponseMiddleware}; | ||
| use crate::platform::{ | ||
| FastlyPlatformBackend, FastlyPlatformConfigStore, FastlyPlatformGeo, FastlyPlatformHttpClient, | ||
| FastlyPlatformSecretStore, UnavailableKvStore, open_kv_store, | ||
| FastlyPlatformSecretStore, UnavailableKvStore, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 thinking — Re-verified at The plan doc's contract item 7 scopes this out as platform infrastructure, which is a fair deferral. But I searched open and closed issues and found no follow-up filed, so nothing currently tracks it. A quick issue to either wire the slot to a real consumer or delete |
||
| }; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
|
|
@@ -237,36 +237,6 @@ fn warn_if_certificate_check_disabled(settings: &Settings) { | |
| } | ||
| } | ||
|
|
||
| /// Resolves per-request consent KV store services for routes that read consent data. | ||
| /// | ||
| /// When `settings.consent.consent_store` is configured and the named KV store cannot | ||
| /// be opened, returns `Err` so the caller can respond with 503 (fail-closed). This is | ||
| /// intentional hardening over the legacy `route_request` path, which builds | ||
| /// `runtime_services` with `UnavailableKvStore` and never opens the named consent | ||
| /// store, so it never fails closed — the `EdgeZero` path instead makes consent-dependent | ||
| /// routes unavailable rather than proceeding without consent. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error when the configured consent store cannot be opened. | ||
| pub(crate) fn runtime_services_for_consent_route( | ||
| settings: &Settings, | ||
| runtime_services: &RuntimeServices, | ||
| ) -> Result<RuntimeServices, Report<TrustedServerError>> { | ||
| let Some(store_name) = settings.consent.consent_store.as_deref() else { | ||
| return Ok(runtime_services.clone()); | ||
| }; | ||
|
|
||
| open_kv_store(store_name) | ||
| .map(|store| runtime_services.clone().with_kv_store(store)) | ||
| .map_err(|e| { | ||
| Report::new(TrustedServerError::KvStore { | ||
| store_name: store_name.to_string(), | ||
| message: e.to_string(), | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Per-request RuntimeServices | ||
| // --------------------------------------------------------------------------- | ||
|
|
@@ -675,10 +645,6 @@ async fn run_named_route( | |
| NamedRouteHandler::SetTester => handle_set_tester(&state.settings), | ||
| NamedRouteHandler::ClearTester => handle_clear_tester(&state.settings), | ||
| NamedRouteHandler::Auction => { | ||
| // The auction reads consent data, so the consent KV store must be | ||
| // available — fail closed with 503 when it is configured but | ||
| // cannot be opened, matching legacy behavior. | ||
| let consent_services = runtime_services_for_consent_route(&state.settings, services)?; | ||
| let partner_registry = PartnerRegistry::from_config(&state.settings.ec.partners)?; | ||
| let registry_ref = if partner_registry.is_empty() { | ||
| None | ||
|
|
@@ -691,7 +657,7 @@ async fn run_named_route( | |
| ec.kv_graph.as_ref(), | ||
| registry_ref, | ||
| &mut ec.ec_context, | ||
| &consent_services, | ||
| services, | ||
|
ChristianPavilonis marked this conversation as resolved.
|
||
| req, | ||
| ) | ||
| .await | ||
|
|
@@ -703,10 +669,6 @@ async fn run_named_route( | |
| if req.method() == Method::OPTIONS { | ||
| return Ok(page_bids_preflight_denied()); | ||
| } | ||
| // Like the auction, page-bids reads consent data, so the consent KV | ||
| // store must be available — fail closed with 503 when configured but | ||
| // unopenable, matching legacy. | ||
| let consent_services = runtime_services_for_consent_route(&state.settings, services)?; | ||
| let partner_registry = PartnerRegistry::from_config(&state.settings.ec.partners)?; | ||
| let registry_ref = if partner_registry.is_empty() { | ||
| None | ||
|
|
@@ -720,7 +682,7 @@ async fn run_named_route( | |
| }; | ||
| handle_page_bids( | ||
| &state.settings, | ||
| &consent_services, | ||
| services, | ||
| ec.kv_graph.as_ref(), | ||
| auction, | ||
| &mut ec.ec_context, | ||
|
|
@@ -864,57 +826,47 @@ async fn dispatch_fallback( | |
| log::warn!("EC generation failed for publisher proxy: {err:?}"); | ||
| } | ||
|
|
||
| // Publisher pages read consent data, so the consent KV store must be | ||
| // available — fail closed with 503 when it is configured but cannot | ||
| // be opened, matching legacy behavior. | ||
| match runtime_services_for_consent_route(&state.settings, services) { | ||
| Ok(publisher_services) => { | ||
| // Run the server-side auction with the configured creative- | ||
| // opportunity slots and collect dispatched bids from the lazy | ||
| // publisher body stream. `handle_publisher_request` matches the | ||
| // slots against the request path. The partner registry plus the | ||
| // EC identity-graph KV (`ec.kv_graph`) enrich the bid request with | ||
| // server-side EIDs, same as the legacy auction. | ||
| let slots = state.settings.creative_opportunity_slots(); | ||
| match PartnerRegistry::from_config(&state.settings.ec.partners) { | ||
| Ok(partner_registry) => { | ||
| let auction = AuctionDispatch { | ||
| orchestrator: &state.orchestrator, | ||
| slots, | ||
| registry: Some(&partner_registry), | ||
| }; | ||
| match handle_publisher_request( | ||
| &state.settings, | ||
| &publisher_services, | ||
| ec.kv_graph.as_ref(), | ||
| &mut ec.ec_context, | ||
| auction, | ||
| req, | ||
| EdgeCacheHeader::SurrogateControl, | ||
| // Run the server-side auction with the configured creative- | ||
| // opportunity slots and collect dispatched bids from the lazy | ||
| // publisher body stream. `handle_publisher_request` matches the | ||
| // slots against the request path. The partner registry plus the | ||
| // EC identity-graph KV (`ec.kv_graph`) enrich the bid request with | ||
| // server-side EIDs, same as the legacy auction. | ||
| let slots = state.settings.creative_opportunity_slots(); | ||
| match PartnerRegistry::from_config(&state.settings.ec.partners) { | ||
| Ok(partner_registry) => { | ||
| let auction = AuctionDispatch { | ||
| orchestrator: &state.orchestrator, | ||
| slots, | ||
| registry: Some(&partner_registry), | ||
| }; | ||
| match handle_publisher_request( | ||
| &state.settings, | ||
| services, | ||
| ec.kv_graph.as_ref(), | ||
| &mut ec.ec_context, | ||
| auction, | ||
| req, | ||
| EdgeCacheHeader::SurrogateControl, | ||
| ) | ||
| .await | ||
| { | ||
| Ok(pub_response) => { | ||
| // Origin start succeeded on the sole publisher-page | ||
| // path. Authorize orphan recovery only for real-browser | ||
| // document navigations so named routes, integration | ||
| // proxies, and filter short circuits cannot rotate an | ||
| // identity. | ||
| ec.ec_context.set_recovery_eligible(is_publisher_navigation); | ||
| publisher_response_into_streaming_response( | ||
| pub_response, | ||
| &method, | ||
| Arc::clone(&state.settings), | ||
| state.registry.as_ref(), | ||
| Arc::clone(&state.orchestrator), | ||
| services.clone(), | ||
| ) | ||
| .await | ||
| { | ||
| Ok(pub_response) => { | ||
| // Origin start succeeded on the sole publisher- | ||
| // page path: authorize orphan recovery now, and | ||
| // only for real-browser document navigations. | ||
| // Restricting it here keeps identity rotation | ||
| // within the publisher-navigation boundary — | ||
| // named routes, integration proxies, and filter | ||
| // short circuits never reach this point. | ||
| ec.ec_context.set_recovery_eligible(is_publisher_navigation); | ||
| publisher_response_into_streaming_response( | ||
| pub_response, | ||
| &method, | ||
| Arc::clone(&state.settings), | ||
| state.registry.as_ref(), | ||
| Arc::clone(&state.orchestrator), | ||
| publisher_services.clone(), | ||
| ) | ||
| .await | ||
| } | ||
| Err(e) => Err(e), | ||
| } | ||
| } | ||
| Err(e) => Err(e), | ||
| } | ||
|
|
@@ -1394,7 +1346,6 @@ mod tests { | |
|
|
||
| use error_stack::Report; | ||
| use futures::executor::block_on; | ||
| use serde_json::json; | ||
| use trusted_server_core::constants::HEADER_X_GEO_INFO_AVAILABLE; | ||
| use trusted_server_core::ec::device::DeviceSignals; | ||
| use trusted_server_core::error::TrustedServerError; | ||
|
|
@@ -1487,53 +1438,6 @@ mod tests { | |
| assert_eq!(stores.secret_store_name.as_ref(), "trusted_server_secrets"); | ||
| } | ||
|
|
||
| fn settings_with_missing_consent_store() -> Settings { | ||
| Settings::from_toml( | ||
| r#" | ||
| [[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" | ||
|
|
||
| [proxy] | ||
| allowed_domains = ["*.example", "*.example.com"] | ||
|
|
||
| [ec] | ||
| passphrase = "test-passphrase-at-least-32-bytes!!" | ||
|
|
||
| [request_signing] | ||
| enabled = false | ||
| config_store_id = "test-config-store-id" | ||
| secret_store_id = "test-secret-store-id" | ||
|
|
||
| [consent] | ||
| consent_store = "missing-consent-store" | ||
|
|
||
| [integrations.prebid] | ||
| enabled = true | ||
| external_bundle_url = "https://assets.example/prebid/trusted-prebid.js" | ||
|
|
||
| [integrations.datadome] | ||
| enabled = true | ||
|
|
||
| [auction] | ||
| enabled = true | ||
| [auction.providers.prebid] | ||
| protocol = "openrtb-2.6" | ||
| profile = "prebid-server" | ||
| endpoint = "https://test-prebid.com/openrtb2/auction" | ||
| timeout_ms = 2000 | ||
| "#, | ||
| ) | ||
| .expect("should parse EdgeZero app test settings") | ||
| } | ||
|
|
||
| fn app_state_for_settings(settings: Settings) -> Arc<AppState> { | ||
| build_state_from_settings(settings).expect("should build app state from settings") | ||
| } | ||
|
|
@@ -2574,27 +2478,6 @@ mod tests { | |
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn dispatch_auction_with_missing_consent_store_returns_503() { | ||
| let state = app_state_for_settings(settings_with_missing_consent_store()); | ||
| let router = TrustedServerApp::routes_for_state(&state); | ||
| let body = json!({ "adUnits": [] }).to_string(); | ||
| let req = request_builder() | ||
| .method(Method::POST) | ||
| .uri("/auction") | ||
| .header(header::CONTENT_TYPE, "application/json") | ||
| .body(Body::from(body)) | ||
| .expect("should build auction request"); | ||
|
|
||
| let response = route(&router, req); | ||
|
|
||
| assert_eq!( | ||
| response.status(), | ||
| StatusCode::SERVICE_UNAVAILABLE, | ||
| "auction route should fail closed when configured consent store cannot be opened" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn dispatch_unregistered_method_returns_405_at_router_level() { | ||
| // Documents the known router-level behavior for verbs outside the | ||
|
|
@@ -2626,54 +2509,6 @@ mod tests { | |
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn edgezero_missing_consent_store_breaks_only_consent_routes() { | ||
| let state = app_state_for_settings(settings_with_missing_consent_store()); | ||
| let router = TrustedServerApp::routes_for_state(&state); | ||
|
|
||
| let admin_response = route( | ||
| &router, | ||
| empty_request(Method::POST, "/_ts/admin/keys/rotate"), | ||
| ); | ||
| assert_eq!( | ||
| admin_response.status(), | ||
| StatusCode::UNAUTHORIZED, | ||
| "admin auth behavior should not depend on consent KV availability" | ||
| ); | ||
|
|
||
| let auction_request = request_builder() | ||
| .method(Method::POST) | ||
| .uri("/auction") | ||
| .body(Body::from(r#"{"adUnits":[]}"#)) | ||
| .expect("should build auction request"); | ||
| let auction_response = route(&router, auction_request); | ||
| assert_eq!( | ||
| auction_response.status(), | ||
| StatusCode::SERVICE_UNAVAILABLE, | ||
| "auction should fail closed when configured consent KV cannot be opened" | ||
| ); | ||
|
|
||
| let publisher_response = route(&router, empty_request(Method::GET, "/articles/example")); | ||
| assert_eq!( | ||
| publisher_response.status(), | ||
| StatusCode::SERVICE_UNAVAILABLE, | ||
| "publisher fallback should fail closed when configured consent KV cannot be opened" | ||
| ); | ||
|
|
||
| // Integration routes must NOT require the consent KV — runtime_services_for_consent_route | ||
| // is wired only into the publisher and auction branches of dispatch_fallback, not into | ||
| // the integration proxy branch. A missing consent store must not 503 integration routes. | ||
| let integration_response = route( | ||
| &router, | ||
| empty_request(Method::GET, "/integrations/datadome/tags.js"), | ||
| ); | ||
| assert_ne!( | ||
| integration_response.status(), | ||
| StatusCode::SERVICE_UNAVAILABLE, | ||
| "integration routes should be unaffected by a missing consent KV store" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn dispatch_fallback_asset_route_skips_ec_finalization() { | ||
| // Parity guard for the configured asset-route fallback: a GET matching a | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.