feat: eliminate Token.BaseURL — Token.Issuer is the single URL anchor (BREAKING) by rsharath · Pull Request #163 · highflame-ai/zeroid

rsharath · 2026-05-26T01:17:36Z

Summary

Removes the `Token.BaseURL` config field and the `ZEROID_BASE_URL` env var. `Token.Issuer` now serves the three roles RFC 8414 §3 collapses into one URL: JWT `iss` claim, discovery anchor, and URL prefix for advertised endpoints.

This is the cleanup [PR-D] from the standards-conformance series. PR-A (#162) lands RFC 9728 PRM; this PR removes the custom field that made the prior conformance gap possible.

Why this is breaking

`Token.BaseURL` was Highflame-introduced — no OAuth/OpenID spec defines or requires it. The split between Issuer (JWT iss) and BaseURL (URL prefix) let deployments configure `Issuer ≠ BaseURL`, where RFC 8414 §3's discovery URL constructed from Issuer would not reach the server's actual endpoints. That's exactly the conformance gap dev1 has today (see highflame-cloud#1929).

Every major OAuth server uses one URL anchor:

Server	Single URL knob
Keycloak	Frontend URL
Ory Hydra	`urls.self.issuer`
Spring Security OAuth	`issuer-uri`
Okta / Auth0 / WorkOS (hosted)	implicit tenant URL = iss

ZeroID now matches that convention.

What changed

`config.go` — `TokenConfig.BaseURL` field removed. Validate() requires `Token.Issuer` non-empty + no trailing slash (RFC 8414 §3 trailing-slash rule). New `rejectRemovedKeys()` helper fails ZeroID startup with a clear migration message if a deployer still has `token.base_url` (YAML) or `ZEROID_BASE_URL` (env) set.
`internal/handler/routes.go` + `server.go` — `NewAPI` constructor no longer takes `baseURL`; the `baseURL` field on the `API` struct is gone.
`internal/handler/wellknown.go` — 8 `a.baseURL` uses (AS metadata: `token_endpoint`, `jwks_uri`, `introspection_endpoint`, `revocation_endpoint`, `registration_endpoint`, `backchannel_authentication_endpoint`, plus the `backchannel_user_code_parameter_supported` block) → `a.issuer`.
`internal/handler/dynamic_registration.go` — RFC 7592 `registration_client_uri` uses `a.issuer`.
`internal/handler/oauth.go` — DPoP `htu` fallback uses `a.issuer` (production path still goes through `RequestURLMiddleware.EffectiveRequestURL`, unchanged).
`tests/integration/helpers_test.go` — drop the redundant `BaseURL: testIssuer` field initialization. The test harness was already setting them equal.
`zeroid.yaml` sample config — `base_url` removed, `issuer` doc-comment updated to describe its three-fold role.
`docs/dpop-and-dcr.md` — one reference to `cfg.Token.BaseURL` updated.

Migration (for deployments still using `Token.BaseURL`)

```yaml

Before

token:
issuer: https://highflame.ai
base_url: https://auth.example.com/v1 # the URL clients actually hit

After

token:
issuer: https://auth.example.com/v1 # the URL clients actually hit
# (== what base_url was)
```

If you previously relied on `Issuer ≠ BaseURL`, you have an RFC 8414 §3 conformance gap independent of this PR — see highflame-cloud#1929 for the dev1 deployment fix.

ZeroID will refuse to start while `token.base_url` or `ZEROID_BASE_URL` is still set, with a message pointing at this migration.

Test plan

`go build ./...` clean
`go vet ./...` clean
Full unit + integration suite passes (`go test ./...`)
- RFC compliance suites for 7009, 7517, 7591/7592, 7662, 8414, 8693, 9449, SPIFFE/JWT-SVID, CIBA, PKCE — all green
- Identity lifecycle, refresh tokens, signing credentials, signals, DCR SSRF guards, DPoP replay — all green
- 0 regressions
Confirmed deprecated-key rejection: removed in code; startup fails fast with the migration message when `ZEROID_BASE_URL` or `token.base_url` is still set

Sequencing

PR	Status	What it does
#162 (PR-A)	Open	Adds RFC 9728 PRM endpoint + tests
this PR	Open	Removes `Token.BaseURL`
highflame-cloud#1929	Open	Deployment-side: move dev1 to a dedicated subdomain

This PR has a trivial conflict with #162 in `internal/handler/wellknown.go` (both touch the file, both substitute `a.baseURL → a.issuer`). Whichever lands first, the other rebases with a one-line resolution.

References

RFC 8414 §3 — Obtaining Authorization Server Metadata
[Standards-adoption deliberation memory](this PR follows the four-question filter agreed in the standards-discipline discussion: real RFC, no vendor branding, topology fit, no breaking-change surprises)

BREAKING: removes the token.base_url config field and the ZEROID_BASE_URL env var. Token.Issuer now serves three roles per RFC 8414 §3: 1. The JWT iss claim on issued tokens. 2. The discovery anchor — clients construct {Issuer}/.well-known/oauth-authorization-server per RFC 8414 §3. 3. The URL prefix for every endpoint advertised in AS metadata, PRM, and RFC 7592 registration_client_uri. Why: Token.BaseURL was a Highflame-introduced custom field with no spec basis. The split between Issuer (JWT iss claim) and BaseURL (URL prefix for advertised endpoints) let deployments accidentally configure Issuer ≠ BaseURL, where RFC 8414 §3's discovery URL constructed from Issuer would not reach the server's actual endpoints. The split also introduced confusion (which field do I set?) and concealed a class of RFC 8414 non-conformance. Every major OAuth server in the ecosystem uses one URL anchor (Keycloak Frontend URL, Hydra urls.self.issuer, Spring issuer-uri, Okta/Auth0/WorkOS implicit tenant URL). ZeroID now matches that convention. What changed: - config.go — TokenConfig.BaseURL removed; Validate() now requires Token.Issuer non-empty + no trailing slash. rejectRemovedKeys() fails fast on token.base_url / ZEROID_BASE_URL with migration text. - internal/handler/routes.go, server.go — API constructor no longer takes baseURL. - internal/handler/wellknown.go — all 8 a.baseURL uses → a.issuer (AS metadata token_endpoint / jwks_uri / introspection_endpoint / revocation_endpoint / registration_endpoint / backchannel_*). - internal/handler/dynamic_registration.go — RFC 7592 registration_client_uri uses a.issuer. - internal/handler/oauth.go — DPoP htu fallback uses a.issuer (production path still uses RequestURLMiddleware's effective URL). - tests/integration/helpers_test.go — drop BaseURL: testIssuer line. - zeroid.yaml sample config — drop base_url, add doc on Issuer's three-fold role. - docs/dpop-and-dcr.md — update one reference to cfg.Token.BaseURL. Migration (deployments using token.base_url / ZEROID_BASE_URL today): - Set token.issuer (or ZEROID_ISSUER) to whatever you had in token.base_url. This will typically be the full public URL clients actually hit — including any reverse-proxy path prefix. - Remove token.base_url / ZEROID_BASE_URL from your config; ZeroID refuses to start while either is set, with the migration message. - If your deployment previously relied on Issuer ≠ BaseURL (the JWT iss claim being different from the URL prefix), you have an RFC 8414 §3 conformance gap regardless of this PR. See highflame-cloud#1929 for the dev1 deployment alignment. Full integration test suite (RFC 7009/7517/7591/7592/7662/8414/8693/9449, SPIFFE/JWT-SVID, CIBA, DCR, DPoP, PKCE, identity lifecycle, signing credentials, signals, refresh tokens) — green. Zero regressions. Refs: highflame-cloud#1929 (dev1 hosting-pattern alignment).

gemini-code-assist

Code Review

This pull request consolidates configuration by removing the redundant token.base_url setting and using token.issuer as the single canonical URL across all handlers, metadata endpoints, and tests. It also introduces a migration check (rejectRemovedKeys) to fail loudly if deployers attempt to use the deprecated configuration keys. The review feedback suggests strengthening the validation of token.issuer in config.go by parsing it as a URL to ensure it has a valid scheme, host, and no query parameters or fragments.

The previous default was https://highflame.ai — the marketing site, not where ZeroID actually serves. With Token.BaseURL gone, the single Issuer knob doubles as the URL clients hit (RFC 8414 §3 discovery anchor + endpoint URL prefix), so the default needs to point at the real hosted service. Self-hosted deployments still override via ZEROID_ISSUER or token.issuer in YAML. Local dev still picks up http://localhost:8899 from the bundled zeroid.yaml.

Aligns the sample config with the code default so both point at Highflame's hosted ZeroID URL. Local dev on localhost:8899 needs to override via ZEROID_ISSUER=http://localhost:8899 — the in-file comment now spells that out. This keeps the bundled YAML usable as a copy-paste starting point for production deployments instead of being local-dev specific.

…moke tests Two follow-ups to PR-163: 1. SDK smoke tests were setting ZEROID_BASE_URL alongside ZEROID_ISSUER. The new rejectRemovedKeys() check (correctly) refuses startup when ZEROID_BASE_URL is present, which is exactly the strict-fail behavior the deprecation is meant to enforce. The tests were one of the un-migrated deployers it caught. Migrating them: - Drop ZEROID_BASE_URL from the env block. - Switch ZEROID_ISSUER from a brand-y \"https://zeroid.test\" to the actual reachable URL (http://localhost:{port} / baseUrl) so it serves all three roles per RFC 8414 §3. Affects tests/sdk/test_sdk_smoke.py and tests/sdk/sdk-smoke.test.ts. 2. validateIssuer extracted from Validate() and made strict per RFC 8414 §2 (flagged by gemini-code-assist[bot] on PR-163): - Must parse as a URL. - Scheme must be http or https. - Must have a host. - No query parameters (RFC 8414 §2). - No fragment (RFC 8414 §2). - (Existing checks: non-empty, no trailing slash.) Misconfigurations that previously produced invalid metadata endpoints at runtime now fail fast at startup with a precise message naming the bad component. Full integration suite passes locally.

Per Yash's review on PR-163: the file's own header declares "Sample Configuration for Local Development," so the issuer in it should match where `make run` actually serves (localhost:8899), not a production URL. Pointing it at auth.highflame.ai meant `make run` minted tokens whose iss claim and metadata-advertised endpoints didn't reach the local server — coherent only after an extra ZEROID_ISSUER env override that every new contributor had to discover. Layering after this change: - Code default (loadDefaults): https://auth.highflame.ai — the reference hosted URL, what an unconfigured ZeroID names. - Bundled zeroid.yaml: http://localhost:8899 — for `make run`, coherent out of the box for local dev. - Prod deploy: ZEROID_ISSUER env via Helm/Terraform — overrides both. Matches the convention in Hydra, Keycloak, Spring Security OAuth where the sample config ships localhost and production sets the real URL.

rsharath · 2026-05-26T02:19:11Z

@saucam — good catch. Reverted in 914a714. The bundled zeroid.yaml now ships with issuer: "http://localhost:8899" so make run produces a coherent local-dev experience without the env-var workaround.

Layering after this change:

Code default (loadDefaults): https://auth.highflame.ai — what an unconfigured ZeroID names (reference to the hosted instance).
Bundled zeroid.yaml: http://localhost:8899 — what make run uses; matches where the local server actually serves.
Production: ZEROID_ISSUER env var via Helm/Terraform overrides both.

Matches the Hydra / Spring Security OAuth convention where the sample config ships localhost and production sets the real URL.

The "Update branch" merge from main (commit 74e680b) brought in PR-162's new protectedResourceMetadataOp, which uses a.baseURL on lines 180 and 183. PR-163's whole purpose is to eliminate that field — so the merge left an unresolved build break: internal/handler/wellknown.go:180:33: a.baseURL undefined (type *API has no field or method baseURL) internal/handler/wellknown.go:183:33: a.baseURL undefined ... Failed all 6 substantive CI checks (build, lint, integration, docker, both SDK smokes). Swapped both references to a.issuer — same value post-PR-D (Issuer is now the single URL knob per RFC 8414 §3), so the PRM document advertises the same URL it would have if PR-162 had been written on top of PR-D from the start. Also updated the §2 doc comment that still referenced "baseURL" by name. Full integration test suite passes locally.

…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).

… 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).

gemini-code-assist Bot reviewed May 26, 2026

View reviewed changes

Comment thread config.go Outdated

rsharath added 2 commits May 25, 2026 18:20

saucam approved these changes May 26, 2026

View reviewed changes

rsharath added 2 commits May 25, 2026 18:37

This was referenced May 26, 2026

feat: RFC 9728 §5.1 — emit resource_metadata parameter in WWW-Authenticate on 401 #165

Closed

feat: RFC 9728 §5.1: WWW-Authenticate resource_metadata breadcrumb on 401 #166

Merged

feat: Canonical OAuth error code constants #167

Merged

rsharath added 3 commits May 25, 2026 20:30

Merge branch 'main' into feat/eliminate-token-baseurl

a4e3671

Merge branch 'main' into feat/eliminate-token-baseurl

74e680b

rsharath merged commit 1b8f4d7 into main May 26, 2026
9 checks passed

Navigation Menu

Uh oh!

feat: eliminate Token.BaseURL — Token.Issuer is the single URL anchor (BREAKING) - #163

feat: eliminate Token.BaseURL — Token.Issuer is the single URL anchor (BREAKING)#163
rsharath merged 8 commits into
mainfrom
feat/eliminate-token-baseurl

rsharath commented May 26, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

rsharath commented May 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

rsharath commented May 26, 2026

Summary

Why this is breaking

What changed

Migration (for deployments still using `Token.BaseURL`)

Before

After

Test plan

Sequencing

References

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

rsharath commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rsharath commented May 26, 2026 •

edited

Loading