Skip to content

feat(oauth): add secure buyer authorization helpers - #1079

Merged
bokelley merged 1 commit into
mainfrom
issue-1031-oauth
Aug 24, 2026
Merged

feat(oauth): add secure buyer authorization helpers#1079
bokelley merged 1 commit into
mainfrom
issue-1031-oauth

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Closes the buyer OAuth helper item in #1031.

What changed

  • add exact-issuer RFC 8414 discovery with path-aware well-known URLs, DNS-pinned SSRF protection, bounded streaming, and one absolute DNS+HTTP deadline
  • add pre-registered public-client authorization-code helpers with mandatory S256 PKCE, browser-session state binding, RFC 9207 issuer defenses, and RFC 8707 resource continuity
  • add an atomic pending-flow store protocol plus a fixed-capacity, TTL-aware single-process reference implementation
  • add one-shot token exchange with persisted endpoint/loopback authority, safe typed token projection, and sanitized errors
  • export the stable handwritten surface and document production shared-store/secret-persistence requirements

This deliberately accepts an authorization-server issuer URL, not an MCP resource URL; resource-to-issuer discovery remains an RFC 9728 concern. It does not expose experimental AdCP 3.2 schema types.

Validation

  • make lint typecheck-all
  • uv run pytest -q — 7,235 passed, 77 skipped, 9 deselected, 1 xfailed
  • focused OAuth/public API suite — 52 passed
  • independent code/security and protocol reviews: approved

Comment thread tests/test_oauth.py
sibling_task = asyncio.create_task(sibling())
with pytest.raises(OAuthDiscoveryError) as caught:
await discover_oauth_metadata("https://auth.example/tenant", timeout=0.05)
await sibling_task
Comment thread tests/test_oauth.py
await asyncio.sleep(0)
task.cancel()
with pytest.raises(asyncio.CancelledError):
await task

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — purely additive OAuth 2.0 helper with no blocking or medium findings.

What I checked:

  • New src/adcp/oauth.py (888 net new lines): fail-closed buyer-side OAuth authorization-code + S256 PKCE helper. Reviewer confirms security-critical paths (state compare, consume-before-I/O replay safety, DNS-pinned SSRF-guarded transports, deadline-covers-DNS timeouts, secret-safe error surfaces) are correct and adversarially tested.
  • Public surface change wired in with a matching public_api_snapshot.json update, type-check fixture, and README section — no public-API break, no type-layering violation.
  • Correct feat(oauth): conventional-commit prefix; purely additive (2009 additions, 0 deletions), so no semver-break signal required.
  • CI gates remain armed.
  • high_risk is true only because src/adcp/__init__.py was modified — but the reviewer found no medium-or-higher concerns on that file, so the modification is presumed safe (row 5 does not fire). No deletions.
  • gated_paths is false, so row 2 does not apply despite review_decision: REVIEW_REQUIRED.
  • No no-auto-approve team match.

Findings: Only two low findings (correct replay-ordering note; id_token opaqueness observation), both omitted from blocking consideration.

No rows 1–8 fired → row 9 approve.

@bokelley
bokelley merged commit d4a8df9 into main Aug 24, 2026
27 checks passed
@bokelley
bokelley deleted the issue-1031-oauth branch August 24, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant