Skip to content

Commit e3bb4b9

Browse files
committed
Merge the Edge Cookie review follow-ups through split/2
Brings upd/split/2-device-geo at 37eadae, which carries the split/1 review follow-ups (0980b73, 0e7f7eb, fb1bc28, cb6f717 and 1b88e42), into split/3-permissions. Two blocks conflicted. - The build_pull_sync_context doc keeps this branch's statement that pull sync needs the sharing permission pair, and adds the follow-up's case where no provider this deployment reads owns the identifier. - In handle_publisher_request the forwarded identifier keeps this branch's ec_sharing_allowed() filter, and the follow-up's active_kv_key line goes in front of it, so the navigation preload reads and compares the row under the canonical key. One test needed a change without a textual conflict. The follow-up's auction_endpoint_loads_the_row_under_the_canonical_key built its context with make_ec_context(Jurisdiction::NonRegulated, ...), and on this branch that helper takes the permission gate as a bool. The test now uses this branch's make_non_regulated_ec_context, which opens the gate in a non-regulated jurisdiction. The other new tests build their context with EcContext::new_for_test, which on this branch grants storage and personalized-ad selection, so they pass the sharing gate unchanged. Tests. The merged tree passes a native all-targets check, 2,851 core tests, the Axum, Cloudflare and Spin adapter tests, the permission signal crate tests, and the Fastly, Cloudflare and Spin wasm checks.
2 parents be0ed2d + 37eadae commit e3bb4b9

9 files changed

Lines changed: 939 additions & 154 deletions

File tree

crates/trusted-server-core/src/auction/endpoints.rs

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,13 @@ pub async fn handle_auction(
305305
// EC and both KV and partner stores are available. Gate the read on a
306306
// present registry: without one, `resolve_auction_eids` yields no
307307
// server-side EIDs, so the snapshot would be an unused billable KV read.
308+
// The row is read under the owning provider's canonical form of the
309+
// identifier, the key it is stored under, rather than under the identifier
310+
// as issued.
308311
let auction_kv_snapshot = match (kv, ec_id.as_deref(), registry) {
309-
(Some(graph), Some(ec_id), Some(_)) => graph.load_snapshot(ec_id),
312+
(Some(graph), Some(_), Some(_)) => ec_context
313+
.ec_kv_key()
314+
.map_or(EcKvSnapshot::NotRead, |kv_key| graph.load_snapshot(&kv_key)),
310315
_ => EcKvSnapshot::NotRead,
311316
};
312317
// Hand the loaded row to the request context so response finalization —
@@ -452,7 +457,13 @@ pub(crate) fn resolve_auction_eids(
452457

453458
let ec_id = ec_context.ec_value()?;
454459

455-
let Some(entry) = snapshot.entry_for(ec_id) else {
460+
// Callers read the snapshot under the identity-graph key, the owning
461+
// provider's canonical form of the identifier, so the entry is looked up
462+
// under that key rather than under the identifier as issued.
463+
let Some(entry) = ec_context
464+
.kv_key_for(ec_id)
465+
.and_then(|kv_key| snapshot.entry_for(&kv_key))
466+
else {
456467
return Some(Vec::new());
457468
};
458469

@@ -625,6 +636,7 @@ mod tests {
625636
use crate::auction::types::{AuctionRequest, AuctionResponse};
626637
use crate::consent::jurisdiction::Jurisdiction;
627638
use crate::consent::types::ConsentContext;
639+
use crate::ec::tests::{CANONICAL_COOKIE_VALUE, CANONICAL_KV_KEY, CanonicalizingProvider};
628640
use crate::error::IntoHttpResponse as _;
629641
use crate::openrtb::Uid;
630642
use crate::platform::test_support::{
@@ -809,6 +821,84 @@ mod tests {
809821
);
810822
}
811823

824+
#[tokio::test]
825+
async fn auction_endpoint_loads_the_row_under_the_canonical_key() {
826+
// The identity graph stores a row under the owning provider's
827+
// canonical form of the identifier. Loaded and resolved under the
828+
// identifier as issued, a provider whose canonical form differs from
829+
// the cookie value found no row, so the auction carried no server-side
830+
// EIDs and the context kept a snapshot bound to the wrong key.
831+
let settings = create_test_settings();
832+
let had_eids = Arc::new(std::sync::Mutex::new(None));
833+
let mut orchestrator = AuctionOrchestrator::new(AuctionConfig {
834+
enabled: true,
835+
providers: AuctionConfig::legacy_provider_map(&["eid_capturing_provider"]),
836+
timeout_ms: 2000,
837+
mediator: None,
838+
..Default::default()
839+
});
840+
orchestrator.register_provider(Arc::new(EidCapturingProvider {
841+
had_eids: Arc::clone(&had_eids),
842+
}));
843+
let registry = PartnerRegistry::from_config(&[counting_test_partner("ssp.example.com")])
844+
.expect("should build partner registry");
845+
let graph = KvIdentityGraph::in_memory("canonical-auction-store");
846+
graph
847+
.create(
848+
CANONICAL_KV_KEY,
849+
&crate::ec::kv_types::KvEntry::minimal(
850+
"ssp.example.com",
851+
"partner-uid-123",
852+
1_741_824_000,
853+
),
854+
)
855+
.expect("should seed the row under the canonical key");
856+
let mut ec_context = make_non_regulated_ec_context(Some(CANONICAL_COOKIE_VALUE))
857+
.with_provider_for_test(Arc::new(CanonicalizingProvider));
858+
let req = Request::builder()
859+
.method("POST")
860+
.uri("https://test-publisher.com/auction")
861+
.body(EdgeBody::from(
862+
serde_json::to_vec(&json!({
863+
"adUnits": [
864+
{
865+
"code": "div-gpt-ad-1",
866+
"mediaTypes": { "banner": { "sizes": [[300, 250]] } }
867+
}
868+
]
869+
}))
870+
.expect("should serialize body"),
871+
))
872+
.expect("should build auction request");
873+
874+
// The capturing provider records whether the request carried EIDs and
875+
// then fails its launch, which is all this test needs. The request
876+
// carries no client EIDs, so any EID it records came from the graph.
877+
let _ = handle_auction(
878+
&settings,
879+
&orchestrator,
880+
Some(&graph),
881+
Some(&registry),
882+
&mut ec_context,
883+
&noop_services(),
884+
req,
885+
)
886+
.await;
887+
888+
assert!(
889+
ec_context
890+
.kv_snapshot()
891+
.entry_for(CANONICAL_KV_KEY)
892+
.is_some(),
893+
"the endpoint should load the row stored under the canonical key"
894+
);
895+
assert_eq!(
896+
*had_eids.lock().expect("should lock captured eids"),
897+
Some(true),
898+
"the auction should carry the canonical row's partner ID as an EID"
899+
);
900+
}
901+
812902
/// Provider that fails the test if it is ever contacted. Used to prove the
813903
/// `/auction` consent gate short-circuits before any outbound bid request.
814904
struct PanicOnBidProvider;

crates/trusted-server-core/src/ec/admin.rs

Lines changed: 132 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,11 @@ pub fn deny_admin_diagnostic_fallback(req: &Request<EdgeBody>) -> Option<Respons
197197
/// Successful admin EC lookup payload.
198198
#[derive(Debug, Serialize)]
199199
struct AdminEcLookupResponse {
200-
/// The EC ID that was looked up.
200+
/// The EC ID as requested, from the path or the `ts-ec` cookie.
201201
ec_id: String,
202+
/// The identity-graph key the entry was read from, being the canonical form
203+
/// of `ec_id` that [`AcceptedProviders::canonical_kv_key`] returns.
204+
kv_key: String,
202205
/// Platform KV store name the entry was read from.
203206
store: String,
204207
/// Store generation marker for the entry.
@@ -283,22 +286,33 @@ pub fn handle_admin_ec_lookup(
283286
return Ok(admin_ec_lookup_not_supported());
284287
};
285288

286-
let ec_id = match requested_ec_id(req, &AcceptedProviders::active(provider)) {
287-
Ok(ec_id) => ec_id,
289+
let requested = match requested_ec_id(req, &AcceptedProviders::active(provider)) {
290+
Ok(requested) => requested,
288291
Err(response) => return Ok(*response),
289292
};
290293

291-
let Some(lookup) = kv.lookup_raw(&ec_id)? else {
292-
log::info!("Admin EC lookup: no entry for '{}'", log_id(&ec_id));
294+
// Read the row under the owning provider's canonical form of the
295+
// identifier, the key the row is stored under, rather than under the
296+
// identifier as requested. The two differ whenever the canonical form is
297+
// not the requested string itself, for example for a built-in HMAC
298+
// identifier requested with its hash in uppercase.
299+
let Some(lookup) = kv.lookup_raw(&requested.kv_key)? else {
300+
log::info!(
301+
"Admin EC lookup: no entry for '{}'",
302+
log_id(&requested.ec_id)
303+
);
293304
return Ok(json_error(
294305
StatusCode::NOT_FOUND,
295306
"EC entry not found (KV reads are eventually consistent; a very \
296307
recent entry may not be visible yet)",
297308
));
298309
};
299310

300-
log::info!("Admin EC lookup: returning entry for '{}'", log_id(&ec_id));
301-
let payload = build_lookup_response(registry, kv.store_name(), ec_id, &lookup);
311+
log::info!(
312+
"Admin EC lookup: returning entry for '{}'",
313+
log_id(&requested.ec_id)
314+
);
315+
let payload = build_lookup_response(registry, kv.store_name(), requested, &lookup);
302316
let body =
303317
serde_json::to_string(&payload).change_context(TrustedServerError::Configuration {
304318
message: "failed to serialize admin EC lookup response".to_owned(),
@@ -341,19 +355,34 @@ fn cookie_ec_id(req: &Request<EdgeBody>) -> Result<String, Box<Response<EdgeBody
341355
})
342356
}
343357

344-
/// Resolves the EC ID to look up from the path or the `ts-ec` cookie.
358+
/// An EC ID resolved for lookup, with the identity-graph key its row is stored
359+
/// under.
360+
#[derive(Debug)]
361+
struct RequestedEcId {
362+
/// The EC ID as requested, from the path or the `ts-ec` cookie.
363+
ec_id: String,
364+
/// The canonical form of `ec_id` that
365+
/// [`AcceptedProviders::canonical_kv_key`] returns, which the row is stored
366+
/// under.
367+
kv_key: String,
368+
}
369+
370+
/// Resolves the EC ID to look up from the path or the `ts-ec` cookie, with the
371+
/// identity-graph key its row is stored under.
345372
///
346373
/// The identifier is validated in two parts: the global cookie bounds, then
347374
/// the provider that owns its `{code}~` prefix, so an operator can look up an
348375
/// identifier created by whichever provider this deployment reads rather than
349-
/// only a built-in HMAC one.
376+
/// only a built-in HMAC one. The same check supplies the key, the canonical
377+
/// form of the identifier that [`AcceptedProviders::canonical_kv_key`]
378+
/// returns.
350379
///
351380
/// Returns the (boxed) error response to send directly when no valid ID is
352381
/// available.
353382
fn requested_ec_id(
354383
req: &Request<EdgeBody>,
355384
accepted_providers: &AcceptedProviders<'_>,
356-
) -> Result<String, Box<Response<EdgeBody>>> {
385+
) -> Result<RequestedEcId, Box<Response<EdgeBody>>> {
357386
let remainder = req
358387
.uri()
359388
.path()
@@ -367,16 +396,16 @@ fn requested_ec_id(
367396
remainder.to_owned()
368397
};
369398

370-
if !accepted_providers.accepts(&ec_id) {
399+
let Some(kv_key) = accepted_providers.canonical_kv_key(&ec_id) else {
371400
return Err(Box::new(json_error(
372401
StatusCode::BAD_REQUEST,
373402
"invalid EC ID: not an identifier any provider this deployment reads \
374403
issued (the built-in HMAC provider issues hmac~{64hex}.{6alnum} and \
375404
still reads the bare legacy form)",
376405
)));
377-
}
406+
};
378407

379-
Ok(ec_id)
408+
Ok(RequestedEcId { ec_id, kv_key })
380409
}
381410

382411
/// Builds the success payload from a raw KV lookup.
@@ -386,11 +415,12 @@ fn requested_ec_id(
386415
fn build_lookup_response(
387416
registry: &PartnerRegistry,
388417
store_name: &str,
389-
ec_id: String,
418+
requested: RequestedEcId,
390419
lookup: &EcKvLookup,
391420
) -> AdminEcLookupResponse {
392421
let mut payload = AdminEcLookupResponse {
393-
ec_id,
422+
ec_id: requested.ec_id,
423+
kv_key: requested.kv_key,
394424
store: store_name.to_owned(),
395425
generation: lookup.generation,
396426
tombstone: None,
@@ -694,6 +724,7 @@ mod tests {
694724
use crate::ec::kv_backend::test_support::InMemoryEcKv;
695725
use crate::ec::kv_backend::{EcKvStore as _, EcKvWrite, EcKvWriteMode};
696726
use crate::ec::kv_types::KvPartnerId;
727+
use crate::ec::tests::{CANONICAL_COOKIE_VALUE, CANONICAL_KV_KEY, CanonicalizingProvider};
697728
use crate::redacted::Redacted;
698729
use crate::settings::EcPartner;
699730

@@ -1576,10 +1607,17 @@ mod tests {
15761607
let coded = format!("hmac~{}", test_ec_id());
15771608
let request = request_with_method(http::Method::GET, &format!("/_ts/admin/ec/{coded}"));
15781609

1579-
let ec_id = requested_ec_id(&request, &AcceptedProviders::active(None))
1610+
let requested = requested_ec_id(&request, &AcceptedProviders::active(None))
15801611
.unwrap_or_else(|_| panic!("should accept a coded HMAC identifier in the path"));
15811612

1582-
assert_eq!(ec_id, coded, "should look up the identifier as given");
1613+
assert_eq!(
1614+
requested.ec_id, coded,
1615+
"should report the identifier as given"
1616+
);
1617+
assert_eq!(
1618+
requested.kv_key, coded,
1619+
"a lowercase HMAC identifier should be its own identity-graph key"
1620+
);
15831621
}
15841622

15851623
#[test]
@@ -1592,9 +1630,12 @@ mod tests {
15921630

15931631
let opaque = "t0op~Opaque_Value_MixedCase";
15941632
let request = request_with_method(http::Method::GET, &format!("/_ts/admin/ec/{opaque}"));
1595-
let ec_id = requested_ec_id(&request, &accepted)
1633+
let requested = requested_ec_id(&request, &accepted)
15961634
.unwrap_or_else(|_| panic!("should accept the active provider's identifier"));
1597-
assert_eq!(ec_id, opaque, "should look up the identifier as given");
1635+
assert_eq!(
1636+
requested.ec_id, opaque,
1637+
"should report the identifier as given"
1638+
);
15981639

15991640
// A code no configured provider reads stays a 400, even in the built-in
16001641
// HMAC shape, so one deployment cannot inspect another's identifiers.
@@ -1608,4 +1649,76 @@ mod tests {
16081649
"an unread provider code should be a 400"
16091650
);
16101651
}
1652+
1653+
#[test]
1654+
fn ec_lookup_reads_the_row_under_the_canonical_key() {
1655+
// The identity graph stores a row under the owning provider's
1656+
// canonical form of the identifier. Read under the identifier as
1657+
// requested, the lookup answered 404 for a row that exists whenever a
1658+
// provider's canonical form differs from the cookie value.
1659+
let kv = kv_with_entry(CANONICAL_KV_KEY, &sample_entry());
1660+
let req =
1661+
get_request_with_cookie("/_ts/admin/ec", &format!("ts-ec={CANONICAL_COOKIE_VALUE}"));
1662+
1663+
let response = handle_admin_ec_lookup(
1664+
Some(&kv),
1665+
&test_registry(),
1666+
Some(&CanonicalizingProvider),
1667+
&req,
1668+
)
1669+
.expect("should handle lookup");
1670+
1671+
assert_eq!(
1672+
response.status(),
1673+
StatusCode::OK,
1674+
"the cookie value should find the row stored under the canonical key"
1675+
);
1676+
let json = response_json(response);
1677+
assert_eq!(
1678+
json["ec_id"], CANONICAL_COOKIE_VALUE,
1679+
"should report the identifier as requested"
1680+
);
1681+
assert_eq!(
1682+
json["kv_key"], CANONICAL_KV_KEY,
1683+
"should report the key the entry was read from"
1684+
);
1685+
assert_eq!(
1686+
json["entry"]["ids"]["bidstream.example"]["uid"], "uid-live",
1687+
"should return the entry stored under the canonical key"
1688+
);
1689+
}
1690+
1691+
#[test]
1692+
fn ec_lookup_given_an_uppercase_hmac_hash_reads_the_lowercase_row() {
1693+
// The built-in HMAC provider issues lowercase hex, and its canonical
1694+
// form lowercases the hash, so its row key is the identifier it issued.
1695+
// An operator who pastes that identifier with the hash in uppercase is
1696+
// still asking for the same row.
1697+
let ec_id = format!("hmac~{}", test_ec_id());
1698+
let kv = kv_with_entry(&ec_id, &sample_entry());
1699+
let uppercase = format!("hmac~{}.abc123", "A".repeat(64));
1700+
let provider = crate::ec::tests::hmac_provider();
1701+
let req = get_request(&format!("/_ts/admin/ec/{uppercase}"));
1702+
1703+
let response =
1704+
handle_admin_ec_lookup(Some(&kv), &test_registry(), Some(provider.as_ref()), &req)
1705+
.expect("should handle lookup");
1706+
1707+
assert_eq!(
1708+
response.status(),
1709+
StatusCode::OK,
1710+
"an uppercase hash should find the row stored under the lowercase key"
1711+
);
1712+
let json = response_json(response);
1713+
assert_eq!(
1714+
json["ec_id"],
1715+
uppercase.as_str(),
1716+
"should report the identifier as requested"
1717+
);
1718+
assert_eq!(
1719+
json["kv_key"],
1720+
ec_id.as_str(),
1721+
"should report the lowercase key the entry was read from"
1722+
);
1723+
}
16111724
}

0 commit comments

Comments
 (0)