feat: RFC 9728 §5.1: WWW-Authenticate resource_metadata breadcrumb on 401 - #166
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request consolidates the BaseURL and Issuer configuration settings into a single Issuer field, enforces RFC 8414 §2 URL shape constraints on the issuer, and implements RFC 9728 §5.1 compliant WWW-Authenticate challenges that include the resource_metadata parameter pointing to the Protected Resource Metadata (PRM) document on 401 responses. The review feedback highlights two important RFC compliance improvements: first, replacing Go's %q formatting in WWWAuthenticate with a custom HTTP-compliant quoting helper to avoid Go-specific escaping while ensuring error_description is only appended when an errorCode is present; second, returning a bare challenge (no error code) when the Authorization header is completely missing, as required by RFC 6750, by distinguishing between missing and malformed headers in AgentAuthMiddleware.
rsharath
changed the title
rsharath
marked this pull request as ready for review
May 26, 2026 03:26
There was a problem hiding this comment.
Pull request overview
Adds RFC 9728 §5.1 resource_metadata breadcrumbs to WWW-Authenticate on 401 responses across ZeroID’s Bearer-protected surfaces, enabling cold-start clients to discover the PRM document and then AS metadata per spec. The PR also completes the ongoing “Issuer is the single URL anchor” migration by removing BaseURL usage and tightening issuer configuration validation.
Changes:
- Centralizes Bearer
WWW-Authenticateformatting (quoting/ordering) and appendsresource_metadatawhere applicable. - Emits the breadcrumb on 401s from agent-auth middleware, forward-auth verify, and DCR (via response headers).
- Updates config/runtime wiring and integration/sdk tests for Issuer-only URL anchoring and new breadcrumb expectations.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/middleware/www_authenticate.go |
New helper to build Bearer challenges and append RFC 9728 resource_metadata. |
internal/middleware/agent_auth.go |
Adds ResourceMetadataURL config and uses the helper to emit consistent challenges on 401. |
internal/handler/auth_verify.go |
Switches forward-auth verify 401 emissions to the shared helper with breadcrumb. |
internal/handler/dynamic_registration.go |
Adds a WWW-Authenticate header field to huma output and emits breadcrumb on DCR 401s. |
internal/handler/routes.go |
Removes baseURL from API and adds prmURL() helper for breadcrumb URL construction. |
internal/handler/wellknown.go |
Migrates advertised endpoints to be issuer-anchored (no baseURL). |
internal/handler/oauth.go |
Updates DPoP htu fallback to use a.issuer instead of a.baseURL. |
internal/handler/signal.go |
Adds a TODO note about future breadcrumb emission for an SSE 401 path. |
server.go |
Wires issuer-based PRM URL into agent-auth middleware config and updates NewAPI call signature. |
config.go |
Validates issuer URL shape, rejects removed keys (token.base_url, ZEROID_BASE_URL), updates defaults/env mapping. |
zeroid.yaml |
Updates sample config to issuer-only and aligns local-dev issuer with make run. |
docs/dpop-and-dcr.md |
Updates documentation references from BaseURL to Issuer. |
tests/integration/www_authenticate_compliance_test.go |
New integration compliance suite validating breadcrumb presence/shape. |
tests/integration/auth_verify_test.go |
Updates exact-match assertions to include resource_metadata. |
tests/integration/discovery_compliance_test.go |
Updates wording to reflect Issuer (vs BaseURL) misconfiguration. |
tests/integration/helpers_test.go |
Removes BaseURL from test config wiring. |
tests/sdk/test_sdk_smoke.py |
Updates SDK smoke env vars to issuer-only URL anchor. |
tests/sdk/sdk-smoke.test.ts |
Same issuer-only env var updates for TS SDK smoke. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rsharath
force-pushed
the
feat/rfc-9728-www-authenticate-breadcrumb
branch
from
May 26, 2026 05:20
3a8dee6 to
96e706c
Compare
saucam
approved these changes
May 26, 2026
…ticate on 401 Completes the RFC 9728 surface that PR-A (#162) deferred. Every 401 from a Bearer-protected endpoint now carries the discovery breadcrumb so a cold-start client that hit the endpoint without prior knowledge can chain resource → PRM → AS metadata per spec. What the breadcrumb looks like: WWW-Authenticate: Bearer error="invalid_token", error_description="...", resource_metadata="{Issuer}/.well-known/oauth-protected-resource" Implementation: - New helper `internal/middleware/www_authenticate.go` — composes a RFC 6750 §3 Bearer challenge with RFC 9728 §5.1 resource_metadata. Centralizes the param-quoting and ordering so emission sites stay one-liners. - `internal/middleware/agent_auth.go` — adds ResourceMetadataURL to AgentAuthConfig; writeAgentAuthError signature reshaped to take an RFC 6750 error code + description and emit the WWW-Authenticate header on every 401. The 4 emission sites now use proper RFC 6750 error codes (invalid_request for missing/malformed auth header, invalid_token for everything past that). - `internal/handler/auth_verify.go` — the forward-auth endpoint already emitted WWW-Authenticate; converted its 3 sites to use the helper so the breadcrumb is appended consistently. - `internal/handler/dynamic_registration.go` — DCR's dcrErr path flows through huma's DCROutput; added a header:"WWW-Authenticate" field on DCROutput, populated by dcrErr when status is 401. dcrErr became a method on *API so it can reach a.prmURL(). - `internal/handler/routes.go` — new a.prmURL() helper centralizes the breadcrumb URL construction; used by auth_verify and dcrErr. - `server.go` — wires ResourceMetadataURL into AgentAuthConfig. Tests: - New `tests/integration/www_authenticate_compliance_test.go` — 7 tests pinning the §5.1 breadcrumb across the three covered paths (agent-auth middleware, DCR, forward-auth verify), the RFC 6750 §3 challenge shape (Bearer-first), and the breadcrumb URL well-formedness. - Updated `tests/integration/auth_verify_test.go` — the 5 existing WWW-Authenticate exact-match assertions tightened to the post-PR-E shape via a shared `expectedWWWAuth(errorCode)` helper. Same contract, now includes the resource_metadata parameter. Out of scope for this PR (will follow up): - `internal/handler/signal.go` SSE 401 — admin endpoint, missing-tenant context error, not a Bearer-auth path. - `internal/handler/oauth.go` mapBackchannelAdminError — wraps via huma.Error401Unauthorized which doesn't accept response headers; needs a deeper huma error-injection pattern. - PR-A's `TestRFC9728_S5_1_WWWAuthenticateResourceMetadataNotYetEmitted` pin test does not exist on this branch (PR-E is branched off PR-D, not PR-A). On rebase against a main that has #162 merged, that test flips from `NotYetEmitted` (negative pin) to `Emitted` (positive). Sequencing: depends on #162 (RFC 9728 PRM) and #163 (eliminate Token.BaseURL) merging first. Opening as draft. Full integration test suite — 100+ tests — passes locally. Refs: closes #165 (RFC 9728 §5.1 breadcrumb on 401).
Addresses PR-166 review (concern #3): the two 401-emission sites this PR intentionally skipped (signal.go SSE missing-tenant, oauth.go mapBackchannelAdminError) were only documented in the PR description, not in the code itself. A future contributor reading those handlers wouldn't know to look at PR-166. Adds inline TODO comments at each site naming RFC 9728 §5.1, the follow-up issue (#165), and the specific reason for the deferral: - signal.go — admin SSE 401 is a missing-tenant failure, not a Bearer-auth failure; different failure class from the cold-start discovery case the breadcrumb is most valuable for. - oauth.go mapBackchannelAdminError — huma.Error401Unauthorized doesn't accept response headers, so the breadcrumb needs a deeper huma error-injection pattern (custom error type with Headers() method or response hook) before this site can emit cleanly. No behavior change; pure documentation.
…lenge
Two gemini-code-assist findings on the WWW-Authenticate path:
1. `WWWAuthenticate` helper used `fmt.Sprintf("%q", …)` to wrap parameter
values. `%q` applies Go-specific escaping (\\uXXXX for non-ASCII, \\n
for newlines) which is NOT valid RFC 7230 §3.2.6 HTTP quoted-string
(only \\ and " require escaping; obs-text covers any %x80-FF byte).
For our ASCII-only inputs (RFC-defined error codes, ASCII PRM URLs)
the bytes on the wire were identical, but the helper would emit
invalid HTTP if a future caller ever passed a non-ASCII URL (IDN,
punycode) or a description containing a literal newline.
Replaces %q with a dedicated `httpQuotedString` helper that
escapes only the two RFC 7230 §3.2.6 mandatory characters.
2. `WWWAuthenticate` emitted `error_description` even when `errorCode`
was empty. RFC 6750 §3 SHOULD-NOT-emit-error-info applies to the
whole error info block, not just the code — an `error_description`
with no `error` field is meaningless. Now drops `error_description`
when `errorCode` is empty.
3. `AgentAuthMiddleware` previously returned `invalid_request` for both
missing-Authorization-header AND wrong-scheme. RFC 6750 §3 SHOULD-NOT
guidance scopes specifically to the "request lacks any authentication
information" case — a missing header. Now splits:
- missing header → bare Bearer challenge (no error code or
description), plus the RFC 9728 §5.1 resource_metadata
breadcrumb (discovery hint is not error info).
- present but non-Bearer scheme → `invalid_request` with
description, plus breadcrumb.
The breadcrumb attaches in both cases — RFC 9728 §5.1 doesn't gate it
on the presence of error info, and a cold-start client benefits from
the discovery hint regardless of whether they sent credentials.
Full integration test suite passes; no test updates required because
the existing tests don't assert on the missing-auth `error=…` value.
Copilot caught three real issues on commit 9f5d36b (the merge of main into this branch): 1. agent_auth.go:76 — "Bearer " with an empty token fell through to jwtalg.Validate(""), reported as invalid_token. Per RFC 6750 §3.1 that's a malformed request (no token to validate), not a token- validation failure. Now short-circuits with invalid_request before the JWS parse — avoids the wasted work and emits the right code. 2. agent_auth.go:63 — on the missing-Authorization path we called writeAgentAuthError(w, "", "", ...) which made the JSON body `{"error":{"code":401,"message":""}}` — empty message looked like an accidental regression. The RFC 6750 §3 SHOULD-NOT-include-error- info clause scopes to the WWW-Authenticate HEADER, not the response body, so the body can still carry a useful message. Refactor: writeAgentAuthError signature is now (w, errorCode, headerMessage, bodyMessage, prmURL). Header obeys RFC 6750 §3 (bare on missing-creds); body always carries actionable text. Each call site supplies both — the missing-creds site sends ("", "", "Authorization header is required", prm). 3. www_authenticate.go:36 — httpQuotedString didn't guard against CTL characters. CR/LF in a header value is unsafe (response-splitting) and Go's net/http rejects them at write time. Added stripCTL pass that removes %x00-1F and %x7F (except HTAB, which RFC 7230 §3.2.6's obs-text permits). The strip is defense-in-depth: callers SHOULD still pre-validate, but a stray newline never reaches the wire. Also updated the docstring — my previous comment example claimed "literal newline" was a use case the helper handled, which was misleading. It now describes the strip behavior accurately. Full integration test suite passes; no test updates required.
…ackchannel admin
Three sites returned 401 when X-Account-ID / X-Project-ID were missing:
- internal/handler/signal.go:147 (streamSignalsHandler)
- internal/handler/oauth.go:413 (bcApproveOp)
- internal/handler/oauth.go:450 (bcDenyOp)
This is a category error. ZeroID's admin endpoints have NO built-in
authentication (TenantContextMiddleware:27 documents this explicitly:
"protected at the network layer ... authentication is the operator's
responsibility"). Missing routing headers is a request-formedness failure
— a misuse of the API by the caller (typically the operator's edge
service after its own auth check) — not an authentication failure. 401
implies "your credentials were rejected"; there are no credentials in
play at this layer at all.
Now returns 400 with a specific message ("missing X-Account-ID or
X-Project-ID header") so a developer hitting the error knows exactly
what to fix.
Also drops the now-dead 401 case from mapBackchannelAdminError. The
backchannel service produces only 400 and 500 OAuthErrors; the 401
branch never fires today. Replaced the previous TODO (which posited a
deferred RFC 9728 §5.1 breadcrumb effort) with a comment explaining
the service-side constraint and what to consider if the service ever
starts producing 401 OAuthErrors.
Knock-on effect: removes the §5.1 breadcrumb follow-up I had documented
in issue #165 for the mapBackchannelAdminError path. The follow-up was
based on the assumption these 401s were legitimate Bearer-auth failures
(RFC 9728 §5.1 applies). They aren't. The fix is to make the status
code honest, which moots the breadcrumb question entirely.
Full integration test suite passes.
PR-162 added TestRFC9728_S5_1_WWWAuthenticateResourceMetadataNotYetEmitted as a placeholder asserting "breadcrumb not yet emitted — flip this when the middleware change lands." PR-166's middleware change has now landed. The flip wasn't a literal NotContains → Contains swap because the placeholder was probing /api/v1/identities — an admin-only endpoint that doesn't go through AgentAuthMiddleware and so doesn't emit WWW-Authenticate at all. PR-166's new file tests/integration/www_authenticate_compliance_test.go probes /api/v1/proof/generate (agent-auth-protected) and has the correctly- scoped positive assertions: - TestRFC9728_S5_1_AgentAuthMiddleware_EmitsBreadcrumbOnMissingAuth - TestRFC9728_S5_1_AgentAuthMiddleware_EmitsBreadcrumbOnInvalidToken - TestRFC9728_S5_1_DCR_EmitsBreadcrumbOnInvalidToken - TestRFC9728_S5_1_AuthVerify_EmitsBreadcrumbOnMissingAuth - TestRFC9728_S5_1_BreadcrumbURLShapeIsWellFormed - TestRFC6750_S3_ChallengeShape_BearerSchemeFirst Replaced the obsolete test with a short comment block pointing future readers at the new compliance file so the history of the placeholder is preserved for context.
rsharath
force-pushed
the
feat/rfc-9728-www-authenticate-breadcrumb
branch
from
May 26, 2026 07:06
96e706c to
fe3cd5a
Compare
Comment on lines
+27
to
+33
| // expectedWWWAuth builds the WWW-Authenticate value the forward-auth endpoint | ||
| // is expected to emit for a given RFC 6750 error code. After PR-E, every 401 | ||
| // from a Bearer-protected path adds the RFC 9728 §5.1 resource_metadata | ||
| // parameter pointing at this server's PRM document. | ||
| func expectedWWWAuth(errorCode string) string { | ||
| return `Bearer error="` + errorCode + `", resource_metadata="` + prmURL() + `"` | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Reworded — `expectedWWWAuth` is now documented as taking "a Bearer error code string" with an explicit note that the forward-auth path intentionally ships the non-standard `missing_token` string for client compatibility, so it no longer implies RFC 6750 defines it.
… accuracy Copilot's second review pass on PR #166 flagged six items. One real code fix and three comment/docstring corrections; two were already-intentional 401->400 changes (documented in the PR description, no code change). 1. dynamic_registration.go — DCR's missing/non-Bearer-scheme auth path returned error="invalid_token" in the WWW-Authenticate challenge. Per RFC 6750 §3.1 a malformed/missing scheme is invalid_request, not a rejected credential. validateInitialAccessToken and authorizeDCRManagement now return oautherror.InvalidRequest for the scheme branch; invalid_token stays on the jwt.Parse-failure and unknown-registration-token paths. This matches the missing/wrong/bad split agent_auth.go already uses. Existing compliance tests assert invalid_token only for valid-scheme-bad-token, so they remain correct. 2. www_authenticate.go — docstring claimed "HTAB, CR, and LF stripped", but stripCTL preserves HTAB. Now reads "all CTL characters except HTAB are stripped". 3. auth_verify_test.go — expectedWWWAuth doc said "RFC 6750 error code" but it's also called with the non-standard missing_token string. Loosened to "Bearer error code string" with a note on the intentional non-standard use. 4. prm_compliance_test.go — reworded the placeholder-removal comment to be present-tense and PR-number-agnostic. go build ./... and go vet ./internal/... clean (GOEXPERIMENT=jsonv2).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #165. Completes RFC 9728 by adding the §5.1
resource_metadataparameter to theWWW-Authenticateheader on every 401 from a Bearer-protected ZeroID endpoint. PR-A (#162) shipped the PRM document itself (§2/§3); this PR adds the cold-start discovery breadcrumb that points clients at it.Without this PR, a stock OAuth client that hits a 401 has no signpost to the PRM document — it has to know the well-known URL out-of-band. With it, the client follows
resource_metadata="…"→ PRM → AS metadata per spec.The breadcrumb shape
erroranderror_descriptionstay RFC 6750 §3.1-correct (invalid_request/invalid_token/insufficient_scope). The breadcrumb is purely additive.Implementation
internal/middleware/www_authenticate.go"/\escaped) + CTL-strip (all CTLs except HTAB) for defense-in-depth against response-splitting.internal/middleware/agent_auth.goResourceMetadataURLtoAgentAuthConfig; reshapeswriteAgentAuthErrorto emit the breadcrumb on every 401; each emission site uses the right RFC 6750 error code (missing creds → bare challenge; wrong scheme →invalid_request; bad token →invalid_token).internal/handler/auth_verify.goWWW-Authenticate— converts its 3 sites to use the new helper.internal/handler/dynamic_registration.goDCROutput. AddsWWWAuthenticatefield withheader:"WWW-Authenticate"tag;dcrErr(now a method on*API) populates it on 401. Missing/non-Bearer-scheme auth now returnsinvalid_request(RFC 6750 §3.1 — malformed request), notinvalid_token(which stays reserved for a syntactically valid credential that fails validation).internal/handler/routes.goa.prmURL()helper centralizes the breadcrumb URL construction.server.goToken.Issuer + "/.well-known/oauth-protected-resource"intoAgentAuthConfig.ResourceMetadataURL.What's covered
/oauth2/token/verify)/oauth2/registerand/oauth2/register/{client_id})Status-code correction (folded in during review)
The two admin paths originally listed here as "out of scope for the breadcrumb" were re-examined and fixed rather than deferred. Both returned 401 when a routing header (
X-Account-ID/X-Project-ID) was missing, which is a category error: ZeroID's admin endpoints have no built-in authentication (it's the operator's responsibility at the network layer perTenantContextMiddleware), so a missing routing header is a request-formedness failure (400), not an authentication failure (401).internal/handler/signal.go(streamSignalsHandler) — missing-tenant SSE response 401 → 400 with a specific message.internal/handler/oauth.go(bcApproveOp,bcDenyOp) — missing-tenant response 401 → 400; also drops the now-dead 401 branch frommapBackchannelAdminError(the backchannel service only ever produces 400/500, so the case never fired; it now falls through to 500, fail-closed).Because these paths no longer emit 401, the RFC 9728 §5.1 breadcrumb question is moot for them — there is no Bearer-auth 401 to decorate. This supersedes the earlier "follow-up work" note and the corresponding part of #165.
Tests
tests/integration/www_authenticate_compliance_test.go— 7 tests:tests/integration/auth_verify_test.go— existing exact-match assertions tightened to the post-PR-E shape via a sharedexpectedWWWAuth(errorCode)helper.prm_compliance_test.go's obsolete…NotYetEmittednegative-pin (the placeholder probed an admin endpoint that never went through bearer-auth); positive coverage now lives in the dedicated compliance file.Full integration suite (~100 tests) passes.
References