Skip to content

Commit 249c284

Browse files
bokelleyclaude
andcommitted
fix(signing): authorized_operators[] is top-level on brand.json, re-export new types
Addresses PR #770 reviewer feedback (aao-ipr-bot, two reviews on commit d3536c2). **Blocking — authorized_operators[] location.** Per the canonical brand.json schema (House Portfolio variant in schemas/cache/3.0/brand.json), ``authorized_operators`` is a TOP-LEVEL property of the document — sibling of ``house`` / ``brands`` / ``contact`` / ``trademarks`` — and ADCP #3690 security.mdx step 3 reads it unqualified at the document root. I had misread the schema indentation and was reading from ``data["house"]["authorized_operators"]``. As written, the SDK would have silently failed closed against every conforming brand.json the TS/Go reference impls accept, AND would have created an operator-delegation bypass via cross-verifier disagreement once Stage 5 wires it into the verifier path. Fix reads from the document root and adds a defensive test asserting that operators misplaced under ``house`` are IGNORED (the wrong-location reading must fail closed). **Non-blocking — re-export new public symbols.** Reviewer noted the PR description says adopters can use the resolver today, but the new types live in submodules and aren't exported from ``adcp.signing``. The sibling ``BrandJsonJwksResolver`` IS exported via that surface, so the new types should follow the same pattern. Adds to ``__init__`` and ``__all__``: - BrandAuthorizationResolver / BrandJsonAuthorizationResolver - BrandAuthorizationResult / BrandAuthorizationReason - build_brand_json_resolvers - host_from / registrable_domain / same_registrable_domain **Not addressed in this commit** (deferring to Stage 4 PR or follow-ups; reviewer flagged as non-blocking): - BrandAuthorizationReason taxonomy overlap with spec codes — the taxonomy is intentionally finer than the spec's framework-boundary codes (e.g. ``agent_type_mismatch`` vs spec's ``agent_not_in_brand_json``); Stage 5 maps reasons to spec codes at the dispatch boundary. Confirmed with reviewer reading. - Protocol returns ``bool`` not ``BrandAuthorizationResult`` — Stage 5 consumers use ``check()`` on the concrete impl. Widening the Protocol can come with Stage 5 when the framework wire-up exposes the real ergonomic shape. - IDNA-2008 A-label canonicalization in ``host_from`` — existing codebase uses stdlib ``host.encode("idna")`` (IDNA 2003) consistently (jwks.py, ip_pinned_transport.py, revocation_fetcher.py). Migrating all of them to IDNA 2008 is a separate concern, deserves its own spec-conformance pass. - shared-fetcher test should exercise the JWKS resolver too — agreed, trivial follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d3536c2 commit 249c284

3 files changed

Lines changed: 82 additions & 19 deletions

File tree

src/adcp/signing/__init__.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,13 @@
101101
SigningDecision,
102102
operation_needs_signing,
103103
)
104+
from adcp.signing.brand_authz import (
105+
BrandAuthorizationReason,
106+
BrandAuthorizationResolver,
107+
BrandAuthorizationResult,
108+
BrandJsonAuthorizationResolver,
109+
build_brand_json_resolvers,
110+
)
104111
from adcp.signing.brand_jwks import (
105112
BrandAgentType,
106113
BrandJsonJwksResolver,
@@ -175,6 +182,11 @@
175182
REQUEST_SIGNATURE_WINDOW_INVALID,
176183
SignatureVerificationError,
177184
)
185+
from adcp.signing.etld import (
186+
host_from,
187+
registrable_domain,
188+
same_registrable_domain,
189+
)
178190
from adcp.signing.ip_pinned_transport import (
179191
AsyncIpPinnedTransport,
180192
IpPinnedTransport,
@@ -293,6 +305,10 @@ def __init__(self, *args: object, **kwargs: object) -> None:
293305
"AsyncJwksResolver",
294306
"AsyncRevocationListFetcher",
295307
"BrandAgentType",
308+
"BrandAuthorizationReason",
309+
"BrandAuthorizationResolver",
310+
"BrandAuthorizationResult",
311+
"BrandJsonAuthorizationResolver",
296312
"BrandJsonJwksResolver",
297313
"BrandJsonResolverError",
298314
"BrandJsonResolverErrorCode",
@@ -373,6 +389,7 @@ def __init__(self, *args: object, **kwargs: object) -> None:
373389
"b64url_decode",
374390
"b64url_encode",
375391
"build_async_ip_pinned_transport",
392+
"build_brand_json_resolvers",
376393
"build_capability_cache_key",
377394
"build_ip_pinned_transport",
378395
"build_signature_base",
@@ -388,15 +405,18 @@ def __init__(self, *args: object, **kwargs: object) -> None:
388405
"extract_signature_bytes",
389406
"format_signature_header",
390407
"generate_signing_keypair",
408+
"host_from",
391409
"install_signing_event_hook",
392410
"load_private_key_pem",
393411
"operation_needs_signing",
394412
"parse_signature_input_header",
395413
"pem_to_adcp_jwk",
396414
"private_key_from_jwk",
397415
"public_key_from_jwk",
416+
"registrable_domain",
398417
"resolve_agent",
399418
"resolve_and_validate_host",
419+
"same_registrable_domain",
400420
"sign_request",
401421
"sign_signature_base",
402422
"sign_standard_webhook",

src/adcp/signing/brand_authz.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -498,11 +498,18 @@ def _find_authorized_operator(
498498
domain == declared operator domain registrable domain) so an
499499
operator declared as ``wpp.com`` covers ``api.wpp.com``,
500500
``us-east.wpp.com``, etc. — same posture as eTLD+1 step 2a.
501+
502+
**Location: top-level on the brand.json document**, not nested
503+
under ``house``. Per the canonical brand.json schema (House
504+
Portfolio variant), ``authorized_operators`` is a sibling of
505+
``house`` / ``brands`` / ``contact`` / ``trademarks``, and ADCP
506+
#3690 ``security.mdx`` step 3 reads it unqualified at the document
507+
root. Reading it from ``data["house"]["authorized_operators"]``
508+
would silently fail closed against every conforming brand.json
509+
(binding_failed everywhere) and would create cross-verifier
510+
disagreement with the TS reference impl.
501511
"""
502-
house = data.get("house")
503-
if not isinstance(house, dict):
504-
return None
505-
operators = house.get("authorized_operators")
512+
operators = data.get("authorized_operators")
506513
if not isinstance(operators, list):
507514
return None
508515

tests/test_brand_authz.py

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -243,11 +243,11 @@ async def test_authz_operator_delegation_with_wildcard_brands() -> None:
243243
"agents": [
244244
{"type": "signals", "id": "s", "url": "https://wpp.com/brand/agent"},
245245
],
246-
"authorized_operators": [
247-
{"domain": "wpp.com", "brands": ["*"]},
248-
],
249246
},
250247
"brands": [{"id": "brand_one"}],
248+
"authorized_operators": [
249+
{"domain": "wpp.com", "brands": ["*"]},
250+
],
251251
}
252252
)
253253
transport = _MockTransport({"https://brand.com/.well-known/brand.json": {"body": body}})
@@ -273,11 +273,11 @@ async def test_authz_operator_delegation_scoped_brand_id_matches() -> None:
273273
"agents": [
274274
{"type": "signals", "id": "s", "url": "https://wpp.com/agent"},
275275
],
276-
"authorized_operators": [
277-
{"domain": "wpp.com", "brands": ["nike"]},
278-
],
279276
},
280277
"brands": [{"id": "nike"}, {"id": "adidas"}],
278+
"authorized_operators": [
279+
{"domain": "wpp.com", "brands": ["nike"]},
280+
],
281281
}
282282
)
283283
transport = _MockTransport({"https://brand.com/.well-known/brand.json": {"body": body}})
@@ -304,11 +304,11 @@ async def test_authz_operator_delegation_scoped_brand_id_misses() -> None:
304304
"agents": [
305305
{"type": "signals", "id": "s", "url": "https://wpp.com/agent"},
306306
],
307-
"authorized_operators": [
308-
{"domain": "wpp.com", "brands": ["nike"]},
309-
],
310307
},
311308
"brands": [{"id": "nike"}, {"id": "adidas"}],
309+
"authorized_operators": [
310+
{"domain": "wpp.com", "brands": ["nike"]},
311+
],
312312
}
313313
)
314314
transport = _MockTransport({"https://brand.com/.well-known/brand.json": {"body": body}})
@@ -337,10 +337,10 @@ async def test_authz_operator_without_wildcard_fails_unscoped_request() -> None:
337337
"agents": [
338338
{"type": "signals", "id": "s", "url": "https://wpp.com/agent"},
339339
],
340-
"authorized_operators": [
341-
{"domain": "wpp.com", "brands": ["nike"]},
342-
],
343340
},
341+
"authorized_operators": [
342+
{"domain": "wpp.com", "brands": ["nike"]},
343+
],
344344
}
345345
)
346346
transport = _MockTransport({"https://brand.com/.well-known/brand.json": {"body": body}})
@@ -368,10 +368,10 @@ async def test_authz_operator_etld1_compared_not_byte_equal() -> None:
368368
"agents": [
369369
{"type": "signals", "id": "s", "url": "https://api.wpp.com/agent"},
370370
],
371-
"authorized_operators": [
372-
{"domain": "wpp.com", "brands": ["*"]},
373-
],
374371
},
372+
"authorized_operators": [
373+
{"domain": "wpp.com", "brands": ["*"]},
374+
],
375375
}
376376
)
377377
transport = _MockTransport({"https://brand.com/.well-known/brand.json": {"body": body}})
@@ -388,6 +388,42 @@ async def test_authz_operator_etld1_compared_not_byte_equal() -> None:
388388
assert result.reason == "operator_delegation"
389389

390390

391+
@pytest.mark.asyncio
392+
async def test_authz_operator_declared_under_house_is_ignored() -> None:
393+
# Per the canonical brand.json schema, ``authorized_operators`` is
394+
# top-level on the House Portfolio variant — sibling of ``house`` /
395+
# ``brands`` / ``contact``. A document that misplaces the array
396+
# inside ``house`` MUST fail closed (the spec's reference verifier
397+
# reads top-level; honoring the nested location would create cross-
398+
# verifier disagreement and an operator-delegation bypass via the
399+
# wrong-location reading).
400+
body = _brand_json(
401+
{
402+
"house": {
403+
"agents": [
404+
{"type": "signals", "id": "s", "url": "https://wpp.com/agent"},
405+
],
406+
# Wrong location — should be ignored.
407+
"authorized_operators": [
408+
{"domain": "wpp.com", "brands": ["*"]},
409+
],
410+
},
411+
}
412+
)
413+
transport = _MockTransport({"https://brand.com/.well-known/brand.json": {"body": body}})
414+
resolver = BrandJsonAuthorizationResolver(
415+
"https://brand.com/.well-known/brand.json",
416+
_client_factory=_factory(transport),
417+
)
418+
419+
result = await resolver.check(
420+
agent_url="https://wpp.com/agent",
421+
brand_domain="brand.com",
422+
)
423+
assert result.authorized is False
424+
assert result.reason == "binding_failed"
425+
426+
391427
# ----- brand_id scopes the agents[] walk -----
392428

393429

0 commit comments

Comments
 (0)