Skip to content

feat: RFC 9728: OAuth Protected Resource Metadata discovery - #162

Merged
rsharath merged 6 commits into
mainfrom
feat/rfc-9728-protected-resource-metadata
May 26, 2026
Merged

feat: RFC 9728: OAuth Protected Resource Metadata discovery#162
rsharath merged 6 commits into
mainfrom
feat/rfc-9728-protected-resource-metadata

Conversation

@rsharath

@rsharath rsharath commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements RFC 9728 — OAuth 2.0 Protected Resource Metadata: publishes the new /.well-known/oauth-protected-resource discovery document, lands a 13-test compliance suite, and updates the README + DCR docs to describe the two-hop PRM → AS discovery chain.

RFC 9728 is the discovery substrate every standards-track agent-auth flow assumes. With this PR, a stock OAuth client hitting a 401 walks resource → PRM → AS metadata per spec without ZeroID-specific shimming.

What this PR ships

  • New endpoint /.well-known/oauth-protected-resource (internal/handler/wellknown.go) advertising:
    • resource, resource_name, authorization_servers, jwks_uri, bearer_methods_supported: ["header"]
    • dpop_bound_access_tokens_required: false (RFC 9449 §5.3)
  • tests/integration/prm_compliance_test.go — 13 tests in the *_compliance_test.go style introduced by test: RFC compliance suites for the standards table (9 RFCs, 57 tests) #154:
    • §2 required-field assertions (resource, authorization_servers, bearer_methods_supported, jwks_uri)
    • §3.2 GET-only, content-type, no empty arrays
    • §3.3 resource value matches the URL used to fetch the document
    • §5.1 WWW-Authenticate breadcrumb negative pin (currently NOT emitted; flip when the middleware change lands)
    • Cross-document consistency PRM ↔ AS metadata
    • RFC 9449 §5.1 vs §5.3 DPoP boundary — dpop_signing_alg_values_supported is AS metadata, MUST NOT leak into PRM
  • README — adds the new endpoint + the previously-undocumented SPIFFE trust bundle to the endpoint table; adds RFC 8414 + RFC 9728 rows to the standards table.
  • docs/dpop-and-dcr.md — expands the Discovery section from the one-hop AS metadata description to the two-hop PRM → AS chain.

What this PR intentionally does NOT do

Concern Why it's deferred
WWW-Authenticate: Bearer resource_metadata="…" breadcrumb in 401 responses (RFC 9728 §5.1) Touches every bearer-auth middleware site. Separate PR. Test TestRFC9728_S5_1_WWWAuthenticateResourceMetadataNotYetEmitted pins the current absence so the follow-up forces the implementer to flip the assertion.
resource_signing_alg_values_supported field Field belongs in PRM only when the resource signs its own responses. ZeroID's /oauth2/token/introspect returns plain JSON (RFC 7662), not RFC 9701 signed introspection. Pinned to NOT advertise until RFC 9701 ships in a separate PR.

Test plan

  • go build ./... clean
  • go vet ./... clean
  • Full integration suite passes locally (testcontainers + Postgres):
    • 13/13 new PRM tests pass
    • 0 regressions across the existing standards-table suite (RFC 8414, 7591/7592, 7009, 7662, 8693, 9449, JWT-SVID, SPIFFE)
  • TestRFC9728_S3_GetMethodOnly confirms POST returns 405 (router behavior, tighter than the != 200 assertion in the test)

References

rsharath added 4 commits May 25, 2026 16:59
Publishes /.well-known/oauth-protected-resource so agents that hit a 401
can chain resource → PRM → AS metadata per RFC 9728 §2/§3. This is the
discovery substrate every standards-track agent-auth draft (ID-JAG,
identity-chaining, WIMSE) assumes.

The PRM document advertises:
  - resource (REQUIRED): canonical resource URL
  - authorization_servers: pointers to AS metadata
  - jwks_uri: shared with the AS keyset (single-deployment topology)
  - bearer_methods_supported: ["header"] (RFC 6750 §2.1, no query/body)
  - resource_signing_alg_values_supported: [ES256, RS256]
  - dpop_bound_access_tokens_required: false (RFC 9449 §5.3)

RFC 9449's dpop_signing_alg_values_supported stays in AS metadata only —
§5.1 defines it for the AS, §5.3 does not include it in PRM. The new
test pins that boundary so it can't regress.

Adds prm_compliance_test.go to the standards-table suite (#154 pattern):
8 tests covering RFC 9728 §2 required fields, §3 path/content-type, the
PRM↔AS issuer chain consistency, and the RFC 9449 §5.3 DPoP boundary.
- README endpoint table: add /.well-known/oauth-protected-resource and
  /.well-known/spiffe-trust-bundle.json (previously omitted), tag the
  AS metadata entry with RFC 8414 explicitly.
- README standards table: add RFC 8414 (AS metadata) and RFC 9728 (PRM)
  rows. RFC 9728 is called out as the discovery substrate the
  standards-track agent-auth drafts (ID-JAG, identity-chaining) assume.
- docs/dpop-and-dcr.md Discovery section: expand from one-hop AS metadata
  to the two-hop PRM → AS chain a conformant client actually walks.
PR-A is RFC 9728 conformance only. The README standards-table entry, the
docs/dpop-and-dcr.md discovery section, and the prm_compliance_test.go
header each gestured at ID-JAG / identity-chaining drafts as if ZeroID
supported them — those are PR-B work and not implemented here. Rephrase
to describe what RFC 9728 actually is (the first hop of the OAuth
discovery chain) without forward-claiming draft support.
PRM changes:
  - Drop `resource_signing_alg_values_supported`. RFC 9728 §2 defines it
    as algs the resource uses *for signed responses*. ZeroID's
    /oauth2/token/introspect returns plain JSON (RFC 7662), not the JWT
    envelope from RFC 9701 — advertising signing algs overclaimed.
    Re-add when RFC 9701 signed introspection lands.

Test additions (all in the established #154 *_compliance_test.go style):
  - TestRFC9728_S2_ResourceSigningAlgValuesNotAdvertisedUntilRFC9701 —
    flipped from positive assertion to negative pin so the field can't
    silently come back.
  - TestRFC9449_S5_3_DPoPBoundAccessTokensRequiredAdvertised — pins
    the only DPoP field RFC 9449 defines for PRM.
  - TestRFC9449_S5_1_DPoPSigningAlgsAreASMetadataNotPRM — pins the
    boundary so the AS-only DPoP field can't leak into PRM.
  - TestRFC9728_S3_GetMethodOnly — pins read-only semantics (POST → 405).
  - TestRFC9728_S3_3_ResourceMatchesRequestedURL — pins the §3.3
    validation invariant that catches BaseURL drift.
  - TestRFC9728_S3_2_NoEmptyArrayValues — pins §3.2's MUST-omit rule.
  - TestRFC9728_S5_1_WWWAuthenticateResourceMetadataNotYetEmitted —
    pins current absence of the breadcrumb so the middleware follow-up
    forces the implementer to flip the assertion.

Section-number corrections in test names: §3.4 → §3.3, §3.5 → §3.2,
§5.3 → §5.1. Verified against RFC 9728's actual table of contents.

All 13 PRM tests + full existing integration suite pass.
@rsharath
rsharath requested a review from saucam May 26, 2026 01:10

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request implements RFC 9728 (OAuth 2.0 Protected Resource Metadata) by introducing the /.well-known/oauth-protected-resource endpoint, updating the relevant documentation, and adding a comprehensive integration test suite. The review feedback highlights two issues in the new test file: a missing resp.Body.Close() call that could lead to a resource leak, and an unsafe type assertion that could cause a panic if the JSON structure changes.

Comment thread tests/integration/prm_compliance_test.go
Comment thread tests/integration/prm_compliance_test.go

Copilot AI 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.

Pull request overview

Implements RFC 9728 (OAuth Protected Resource Metadata) discovery for ZeroID by publishing a new well-known PRM document, adding an integration compliance suite, and updating documentation to describe the PRM → AS metadata discovery chain.

Changes:

  • Add /.well-known/oauth-protected-resource endpoint that serves RFC 9728 Protected Resource Metadata (including RFC 9449’s PRM DPoP boolean).
  • Add a 13-test RFC 9728/RFC 9449 compliance suite to validate PRM semantics and PRM ↔ AS metadata consistency.
  • Update README and DPoP/DCR docs to document the new endpoint and the two-hop discovery flow.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
internal/handler/wellknown.go Registers and serves the RFC 9728 Protected Resource Metadata document.
tests/integration/prm_compliance_test.go Adds RFC 9728 (and related RFC 9449 PRM constraints) integration compliance tests.
README.md Documents the new well-known endpoint and adds RFC 8414/9728 to the standards table.
docs/dpop-and-dcr.md Updates discovery documentation to reflect the PRM → AS two-hop chain.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/integration/prm_compliance_test.go Outdated
…assertion

- TestRFC9728_S3_WellKnownPathIsExact: defer resp.Body.Close() — was
  leaking the response body since the test reads only headers, not via
  decode(). Matches the pattern used by TestRFC9728_S5_1_*.
- TestRFC9728_AuthorizationServersPointsAtASMetadata: use comma-ok type
  assertion when iterating authorization_servers entries — matches the
  convention from TestRFC9728_S2_AuthorizationServersListed. Bare
  s.(string) would panic instead of failing cleanly if the JSON ever
  contained a non-string entry.

Both flagged by gemini-code-assist[bot]; the type-assertion issue was
independently flagged by Copilot.
@rsharath

Copy link
Copy Markdown
Contributor Author

@Copilot — addressed in 3fbec1f. Same fix as Gemini's inline comment on line 269 (TestRFC9728_AuthorizationServersPointsAtASMetadata): switched the unchecked s.(string) to the comma-ok form with a require.True on the assertion result, matching the convention from TestRFC9728_S2_AuthorizationServersListed earlier in the same file.

@rsharath rsharath changed the title feat: RFC 9728 — OAuth Protected Resource Metadata discovery feat: RFC 9728: OAuth Protected Resource Metadata discovery May 26, 2026
@rsharath
rsharath merged commit 883d4af into main May 26, 2026
9 checks passed
@rsharath
rsharath deleted the feat/rfc-9728-protected-resource-metadata branch May 26, 2026 04:13
rsharath added a commit that referenced this pull request 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).
rsharath added a commit that referenced this pull request 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).
rsharath added a commit that referenced this pull request May 29, 2026
… 401 (#166)

* feat: RFC 9728 §5.1 — emit resource_metadata breadcrumb in WWW-Authenticate 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).

* fix: add TODO markers at out-of-scope §5.1 breadcrumb sites

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.

* fix: address PR-166 review — RFC 7230 quoting + RFC 6750 §3 bare challenge

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.

* fix: address PR-166 Copilot review — three findings

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.

* fix: missing-tenant-headers is 400 not 401, drop dead 401 branch in backchannel 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.

* test: remove obsolete §5.1 negative-pin from prm_compliance_test.go

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.

* fix: address PR-166 Copilot review batch 2 — DCR error code + comment 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).
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.

3 participants