From c2876edd47a3941308ae77eee7917195c1e081e4 Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 19:58:56 -0700 Subject: [PATCH 1/7] =?UTF-8?q?feat:=20RFC=209728=20=C2=A75.1=20=E2=80=94?= =?UTF-8?q?=20emit=20resource=5Fmetadata=20breadcrumb=20in=20WWW-Authentic?= =?UTF-8?q?ate=20on=20401?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- internal/handler/auth_verify.go | 13 +- internal/handler/dynamic_registration.go | 46 +++-- internal/handler/routes.go | 8 + internal/middleware/agent_auth.go | 33 +++- internal/middleware/www_authenticate.go | 42 +++++ server.go | 4 + tests/integration/auth_verify_test.go | 20 +- .../www_authenticate_compliance_test.go | 173 ++++++++++++++++++ 8 files changed, 308 insertions(+), 31 deletions(-) create mode 100644 internal/middleware/www_authenticate.go create mode 100644 tests/integration/www_authenticate_compliance_test.go diff --git a/internal/handler/auth_verify.go b/internal/handler/auth_verify.go index 4cfe6877..045d152d 100644 --- a/internal/handler/auth_verify.go +++ b/internal/handler/auth_verify.go @@ -8,6 +8,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/rs/zerolog/log" + "github.com/highflame-ai/zeroid/internal/middleware" "github.com/highflame-ai/zeroid/internal/oautherror" ) @@ -42,9 +43,15 @@ func (a *API) registerAuthVerifyRoute(router chi.Router) { // copy_headers X-Forwarded-User X-Zeroid-Identity-Type X-Zeroid-Trust-Level X-Zeroid-Account-ID X-Zeroid-Project-ID // } func (a *API) authVerifyHandler(w http.ResponseWriter, r *http.Request) { + prm := a.prmURL() + authHeader := r.Header.Get("Authorization") if authHeader == "" { - w.Header().Set("WWW-Authenticate", `Bearer error="missing_token"`) + // RFC 6750 §3.1 — "missing_token" is not in the standard enum, + // but pre-PR usage shipped this string; preserved for client + // compatibility. The RFC 9728 §5.1 breadcrumb is the additive + // improvement. + w.Header().Set("WWW-Authenticate", middleware.WWWAuthenticate("missing_token", "", prm)) http.Error(w, `{"error":"missing_token"}`, http.StatusUnauthorized) return } @@ -52,7 +59,7 @@ func (a *API) authVerifyHandler(w http.ResponseWriter, r *http.Request) { token, ok := strings.CutPrefix(authHeader, "Bearer ") token = strings.TrimSpace(token) if !ok || token == "" { - w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer error=%q`, oautherror.InvalidRequest)) + w.Header().Set("WWW-Authenticate", middleware.WWWAuthenticate(oautherror.InvalidRequest, "", prm)) http.Error(w, `{"error":"invalid_authorization_header"}`, http.StatusUnauthorized) return } @@ -66,7 +73,7 @@ func (a *API) authVerifyHandler(w http.ResponseWriter, r *http.Request) { active, _ := claims["active"].(bool) if !active { - w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer error=%q`, oautherror.InvalidToken)) + w.Header().Set("WWW-Authenticate", middleware.WWWAuthenticate(oautherror.InvalidToken, "", prm)) http.Error(w, fmt.Sprintf(`{"error":%q}`, oautherror.InvalidToken), http.StatusUnauthorized) return } diff --git a/internal/handler/dynamic_registration.go b/internal/handler/dynamic_registration.go index cd6fe82e..eff83c9d 100644 --- a/internal/handler/dynamic_registration.go +++ b/internal/handler/dynamic_registration.go @@ -11,6 +11,7 @@ import ( "github.com/rs/zerolog/log" "github.com/highflame-ai/zeroid/domain" + "github.com/highflame-ai/zeroid/internal/middleware" "github.com/highflame-ai/zeroid/internal/oautherror" "github.com/highflame-ai/zeroid/internal/service" ) @@ -57,9 +58,15 @@ type DCRRegisterInput struct { // DCROutput is the polymorphic response body. RFC 7591/7592 success bodies are // dynamic-shape; error bodies are oauthErrorBody. +// +// WWWAuthenticate, when set, is emitted as the WWW-Authenticate response +// header. Populated by `dcrErr` for 401 responses so the RFC 9728 §5.1 +// resource_metadata breadcrumb reaches clients that hit a DCR auth failure +// the same way it reaches clients that hit a generic bearer-auth failure. type DCROutput struct { - Status int - Body any + Status int + WWWAuthenticate string `header:"WWW-Authenticate"` + Body any } // DCRGetInput / DCRUpdateInput / DCRDeleteInput share the same auth shape: @@ -140,12 +147,12 @@ func (a *API) registerDynamicRegistrationRoutes(api huma.API) { func (a *API) dcrRegisterOp(ctx context.Context, input *DCRRegisterInput) (*DCROutput, error) { iatClaims, err := a.validateInitialAccessToken(input.Authorization) if err != nil { - return dcrErr(err), nil + return a.dcrErr(err), nil } v, err := validateDCRClientMetadata(input.Body.ClientName, input.Body.Scope, input.Body.TokenEndpointAuthMethod, input.Body.GrantTypes) if err != nil { - return dcrErr(err), nil + return a.dcrErr(err), nil } client, plainSecret, plainRegToken, regErr := a.oauthClientSvc.DynamicRegisterClient(ctx, service.DynamicRegisterClientRequest{ @@ -160,10 +167,10 @@ func (a *API) dcrRegisterOp(ctx context.Context, input *DCRRegisterInput) (*DCRO }) if regErr != nil { if errors.Is(regErr, service.ErrOAuthClientAlreadyExists) { - return dcrErr(&dcrError{status: http.StatusConflict, code: oautherror.InvalidClientMetadata, desc: "client already exists"}), nil + return a.dcrErr(&dcrError{status: http.StatusConflict, code: oautherror.InvalidClientMetadata, desc: "client already exists"}), nil } log.Error().Err(regErr).Msg("dynamic client registration failed") - return dcrErr(&dcrError{status: http.StatusInternalServerError, code: oautherror.ServerError, desc: "failed to register client"}), nil + return a.dcrErr(&dcrError{status: http.StatusInternalServerError, code: oautherror.ServerError, desc: "failed to register client"}), nil } // Audit log: who minted what. registered_by_* claims are derived from the @@ -187,19 +194,19 @@ func (a *API) dcrRegisterOp(ctx context.Context, input *DCRRegisterInput) (*DCRO func (a *API) dcrGetOp(ctx context.Context, input *DCRGetInput) (*DCROutput, error) { cl, err := a.authorizeDCRManagement(ctx, input.Authorization, input.ClientID) if err != nil { - return dcrErr(err), nil + return a.dcrErr(err), nil } return &DCROutput{Status: http.StatusOK, Body: a.dcrClientResponse(cl)}, nil } func (a *API) dcrUpdateOp(ctx context.Context, input *DCRUpdateInput) (*DCROutput, error) { if _, err := a.authorizeDCRManagement(ctx, input.Authorization, input.ClientID); err != nil { - return dcrErr(err), nil + return a.dcrErr(err), nil } v, err := validateDCRClientMetadata(input.Body.ClientName, input.Body.Scope, input.Body.TokenEndpointAuthMethod, input.Body.GrantTypes) if err != nil { - return dcrErr(err), nil + return a.dcrErr(err), nil } updated, updateErr := a.oauthClientSvc.UpdateDynamicClient(ctx, input.ClientID, service.DynamicRegisterClientRequest{ @@ -217,21 +224,21 @@ func (a *API) dcrUpdateOp(ctx context.Context, input *DCRUpdateInput) (*DCROutpu // have been deleted in between. Map ErrOAuthClientNotFound to the // same 401 the auth path uses; other errors are infra failures. if errors.Is(updateErr, service.ErrOAuthClientNotFound) { - return dcrErr(&dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidToken, desc: "client no longer exists"}), nil + return a.dcrErr(&dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidToken, desc: "client no longer exists"}), nil } log.Error().Err(updateErr).Str("client_id", input.ClientID).Msg("dynamic client update failed") - return dcrErr(&dcrError{status: http.StatusInternalServerError, code: oautherror.ServerError, desc: "failed to update client registration"}), nil + return a.dcrErr(&dcrError{status: http.StatusInternalServerError, code: oautherror.ServerError, desc: "failed to update client registration"}), nil } return &DCROutput{Status: http.StatusOK, Body: a.dcrClientResponse(updated)}, nil } func (a *API) dcrDeleteOp(ctx context.Context, input *DCRDeleteInput) (*DCROutput, error) { if _, err := a.authorizeDCRManagement(ctx, input.Authorization, input.ClientID); err != nil { - return dcrErr(err), nil + return a.dcrErr(err), nil } if err := a.oauthClientSvc.DeleteDynamicClient(ctx, input.ClientID); err != nil { log.Error().Err(err).Str("client_id", input.ClientID).Msg("dynamic client delete failed") - return dcrErr(&dcrError{status: http.StatusInternalServerError, code: oautherror.ServerError, desc: "failed to delete client registration"}), nil + return a.dcrErr(&dcrError{status: http.StatusInternalServerError, code: oautherror.ServerError, desc: "failed to delete client registration"}), nil } return &DCROutput{Status: http.StatusNoContent, Body: nil}, nil } @@ -300,8 +307,17 @@ type dcrError struct { func (e *dcrError) Error() string { return e.code + ": " + e.desc } -func dcrErr(de *dcrError) *DCROutput { - return &DCROutput{Status: de.status, Body: oauthErrorBody{Error: de.code, ErrorDescription: de.desc}} +func (a *API) dcrErr(de *dcrError) *DCROutput { + out := &DCROutput{Status: de.status, Body: oauthErrorBody{Error: de.code, ErrorDescription: de.desc}} + // RFC 9728 §5.1 — on 401 from any Bearer-protected path, the + // resource_metadata breadcrumb points cold-start clients at the PRM + // document so they can discover the AS without prior knowledge. The + // error code and description echo what the body already carries so a + // client that only inspects headers gets the same signal. + if de.status == http.StatusUnauthorized { + out.WWWAuthenticate = middleware.WWWAuthenticate(de.code, de.desc, a.prmURL()) + } + return out } // initialAccessTokenClaims captures the tenant-relevant claims of a successfully diff --git a/internal/handler/routes.go b/internal/handler/routes.go index 63388503..1fcfe455 100644 --- a/internal/handler/routes.go +++ b/internal/handler/routes.go @@ -107,6 +107,14 @@ func NewHumaAPI(router chi.Router) huma.API { return humachi.New(router, config) } +// prmURL returns the absolute URL of this server's RFC 9728 Protected +// Resource Metadata document. Centralized so the value emitted in +// WWW-Authenticate breadcrumbs (RFC 9728 §5.1) stays in lockstep with the +// PRM endpoint's registered path. +func (a *API) prmURL() string { + return a.issuer + "/.well-known/oauth-protected-resource" +} + // RegisterPublic registers endpoints that require no authentication: // health, well-known, OAuth2 endpoints (token, revoke), and forward-auth verify. // The /oauth2/register endpoints (RFC 7591/7592) live here too — they enforce diff --git a/internal/middleware/agent_auth.go b/internal/middleware/agent_auth.go index 7bc086fd..25abd903 100644 --- a/internal/middleware/agent_auth.go +++ b/internal/middleware/agent_auth.go @@ -35,6 +35,13 @@ type AgentAuthConfig struct { PublicKey *ecdsa.PublicKey // Issuer is the expected iss claim value. Issuer string + // ResourceMetadataURL is the absolute URL of this server's RFC 9728 + // Protected Resource Metadata document. Emitted in the WWW-Authenticate + // header on every 401 so cold-start clients can chain resource → PRM → + // AS metadata per RFC 9728 §5.1. Empty disables the breadcrumb (e.g. + // for legacy deployments that haven't migrated to issuer-anchored + // discovery). + ResourceMetadataURL string } // AgentAuthMiddleware validates ES256 Bearer tokens issued by ZeroID and injects agent claims into context. @@ -44,7 +51,10 @@ func AgentAuthMiddleware(cfg AgentAuthConfig) func(http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { authHeader := r.Header.Get("Authorization") if !strings.HasPrefix(authHeader, "Bearer ") { - writeAgentAuthError(w, http.StatusUnauthorized, "missing or invalid Authorization header") + // RFC 6750 §3.1: "invalid_request" — request lacks the + // Authorization header or it is malformed. RFC 9728 §5.1 + // breadcrumb points the cold-start client at PRM. + writeAgentAuthError(w, "invalid_request", "missing or invalid Authorization header", cfg.ResourceMetadataURL) return } tokenStr := strings.TrimPrefix(authHeader, "Bearer ") @@ -52,7 +62,7 @@ func AgentAuthMiddleware(cfg AgentAuthConfig) func(http.Handler) http.Handler { // Reject alg=none / HS* before any further work — JWT-SVID §3. if err := jwtalg.Validate(tokenStr); err != nil { log.Warn().Err(err).Str("path", r.URL.Path).Msg("Agent JWT rejected: bad alg") - writeAgentAuthError(w, http.StatusUnauthorized, "invalid or expired token") + writeAgentAuthError(w, "invalid_token", "invalid or expired token", cfg.ResourceMetadataURL) return } @@ -63,14 +73,14 @@ func AgentAuthMiddleware(cfg AgentAuthConfig) func(http.Handler) http.Handler { ) if err != nil { log.Warn().Err(err).Str("path", r.URL.Path).Msg("Agent JWT validation failed") - writeAgentAuthError(w, http.StatusUnauthorized, "invalid or expired token") + writeAgentAuthError(w, "invalid_token", "invalid or expired token", cfg.ResourceMetadataURL) return } claims := extractAgentClaims(parsed) if claims.AccountID == "" || claims.ProjectID == "" { - writeAgentAuthError(w, http.StatusUnauthorized, "token missing required tenant claims") + writeAgentAuthError(w, "invalid_token", "token missing required tenant claims", cfg.ResourceMetadataURL) return } @@ -132,12 +142,21 @@ func extractAgentClaims(token jwt.Token) AgentClaims { return claims } -func writeAgentAuthError(w http.ResponseWriter, status int, message string) { +// writeAgentAuthError emits a 401 response with an RFC 6750 §3 challenge in +// the WWW-Authenticate header. The errorCode value follows RFC 6750 §3.1 +// ("invalid_request", "invalid_token", "insufficient_scope"); the message is +// surfaced both in the header (error_description) and in the JSON body for +// callers that don't inspect headers. +// +// When resourceMetadataURL is non-empty, RFC 9728 §5.1's resource_metadata +// parameter is appended so cold-start clients can discover the PRM document. +func writeAgentAuthError(w http.ResponseWriter, errorCode, message, resourceMetadataURL string) { + w.Header().Set("WWW-Authenticate", WWWAuthenticate(errorCode, message, resourceMetadataURL)) w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) + w.WriteHeader(http.StatusUnauthorized) _ = json.NewEncoder(w).Encode(map[string]any{ "error": map[string]any{ - "code": status, + "code": http.StatusUnauthorized, "message": message, }, }) diff --git a/internal/middleware/www_authenticate.go b/internal/middleware/www_authenticate.go new file mode 100644 index 00000000..6d08c6d9 --- /dev/null +++ b/internal/middleware/www_authenticate.go @@ -0,0 +1,42 @@ +package middleware + +import ( + "fmt" + "strings" +) + +// WWWAuthenticate builds a Bearer WWW-Authenticate challenge value combining +// RFC 6750 §3 (Bearer error codes) with the RFC 9728 §5.1 resource_metadata +// breadcrumb. +// +// errorCode and errorDesc are RFC 6750 §3.1 error semantics. errorCode SHOULD +// be one of "invalid_request", "invalid_token", or "insufficient_scope". An +// empty errorCode emits a bare "Bearer" challenge — appropriate when no +// authentication has been attempted yet (initial 401 to an unauthenticated +// client). +// +// resourceMetadataURL is the absolute URL of the protected resource metadata +// document (typically "{issuer}/.well-known/oauth-protected-resource"). When +// non-empty, it is appended as the RFC 9728 §5.1 `resource_metadata` +// parameter so cold-start clients can chain resource → PRM → AS metadata +// without prior knowledge. +// +// All parameter values are double-quoted per RFC 7235 §2.1 (quoted-string). +// Callers are responsible for ensuring values are well-formed; this function +// does not URL-encode or escape. +func WWWAuthenticate(errorCode, errorDesc, resourceMetadataURL string) string { + var params []string + if errorCode != "" { + params = append(params, fmt.Sprintf(`error=%q`, errorCode)) + } + if errorDesc != "" { + params = append(params, fmt.Sprintf(`error_description=%q`, errorDesc)) + } + if resourceMetadataURL != "" { + params = append(params, fmt.Sprintf(`resource_metadata=%q`, resourceMetadataURL)) + } + if len(params) == 0 { + return "Bearer" + } + return "Bearer " + strings.Join(params, ", ") +} diff --git a/server.go b/server.go index 9cb0b3e4..a8e02cfd 100644 --- a/server.go +++ b/server.go @@ -316,6 +316,10 @@ func NewServer(cfg Config) (*Server, error) { agentAuthCfg := internalMiddleware.AgentAuthConfig{ PublicKey: jwksSvc.PublicKey(), Issuer: cfg.Token.Issuer, + // RFC 9728 §5.1 breadcrumb on 401s — points cold-start + // clients at the PRM document so they can chain + // resource → PRM → AS metadata without prior knowledge. + ResourceMetadataURL: cfg.Token.Issuer + "/.well-known/oauth-protected-resource", } r.Use(internalMiddleware.AgentAuthMiddleware(agentAuthCfg)) diff --git a/tests/integration/auth_verify_test.go b/tests/integration/auth_verify_test.go index dfc0dfcc..e9df089c 100644 --- a/tests/integration/auth_verify_test.go +++ b/tests/integration/auth_verify_test.go @@ -24,15 +24,23 @@ func issueAPIKeyToken(t *testing.T, externalID string) string { return decode(t, resp)["access_token"].(string) } +// 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() + `"` +} + // TestAuthVerify_MissingAuthorizationHeader checks that a request with no // Authorization header is rejected with 401 and the correct WWW-Authenticate -// challenge. +// challenge (RFC 6750 §3 error + RFC 9728 §5.1 resource_metadata breadcrumb). func TestAuthVerify_MissingAuthorizationHeader(t *testing.T) { resp := get(t, "/oauth2/token/verify", nil) defer func() { _ = resp.Body.Close() }() assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) - assert.Equal(t, `Bearer error="missing_token"`, resp.Header.Get("WWW-Authenticate")) + assert.Equal(t, expectedWWWAuth("missing_token"), resp.Header.Get("WWW-Authenticate")) } // TestAuthVerify_WrongScheme checks that a non-Bearer Authorization scheme @@ -44,7 +52,7 @@ func TestAuthVerify_WrongScheme(t *testing.T) { defer func() { _ = resp.Body.Close() }() assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) - assert.Equal(t, `Bearer error="invalid_request"`, resp.Header.Get("WWW-Authenticate")) + assert.Equal(t, expectedWWWAuth("invalid_request"), resp.Header.Get("WWW-Authenticate")) } // TestAuthVerify_EmptyBearerToken checks that "Bearer " followed by only @@ -56,7 +64,7 @@ func TestAuthVerify_EmptyBearerToken(t *testing.T) { defer func() { _ = resp.Body.Close() }() assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) - assert.Equal(t, `Bearer error="invalid_request"`, resp.Header.Get("WWW-Authenticate")) + assert.Equal(t, expectedWWWAuth("invalid_request"), resp.Header.Get("WWW-Authenticate")) } // TestAuthVerify_InvalidToken checks that a well-formed but unrecognised token @@ -68,7 +76,7 @@ func TestAuthVerify_InvalidToken(t *testing.T) { defer func() { _ = resp.Body.Close() }() assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) - assert.Equal(t, `Bearer error="invalid_token"`, resp.Header.Get("WWW-Authenticate")) + assert.Equal(t, expectedWWWAuth("invalid_token"), resp.Header.Get("WWW-Authenticate")) } // TestAuthVerify_ValidToken checks that a valid JWT issued by ZeroID is @@ -110,7 +118,7 @@ func TestAuthVerify_RevokedToken(t *testing.T) { defer func() { _ = resp.Body.Close() }() assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) - assert.Equal(t, `Bearer error="invalid_token"`, resp.Header.Get("WWW-Authenticate")) + assert.Equal(t, expectedWWWAuth("invalid_token"), resp.Header.Get("WWW-Authenticate")) } // TestAuthVerify_ResponseBodyOnSuccess checks that the 200 response body is diff --git a/tests/integration/www_authenticate_compliance_test.go b/tests/integration/www_authenticate_compliance_test.go new file mode 100644 index 00000000..ae248469 --- /dev/null +++ b/tests/integration/www_authenticate_compliance_test.go @@ -0,0 +1,173 @@ +// RFC 9728 §5.1 (WWW-Authenticate Response) compliance suite. +// +// See COMPLIANCE.md for the conventions this file follows. +// +// RFC 9728 §5.1 defines the `resource_metadata` parameter — carried in the +// WWW-Authenticate header that a Bearer-protected resource returns on 401 — +// as "The URL of the protected resource metadata." This is the breadcrumb +// that lets a cold-start client (one that didn't already know the well-known +// URL) chain resource → PRM → AS metadata per spec. +// +// PR-162's PRM endpoint shipped a negative-pin test asserting the breadcrumb +// was NOT yet emitted. This PR flips that path: every 401 from a Bearer- +// protected ZeroID endpoint MUST now include the parameter. + +package integration_test + +import ( + "net/http" + "regexp" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// prmURL returns the URL the breadcrumb should point at, derived from the +// test harness's testIssuer (which serves as both iss claim and endpoint +// URL prefix per RFC 8414 §3). +func prmURL() string { + return testIssuer + "/.well-known/oauth-protected-resource" +} + +// resourceMetadataParamRE extracts the `resource_metadata=""` parameter +// from a WWW-Authenticate header value. Matches the RFC 7235 §2.1 +// quoted-string shape: the parameter name, "=", and a double-quoted value. +var resourceMetadataParamRE = regexp.MustCompile(`resource_metadata="([^"]+)"`) + +// extractResourceMetadata returns the URL inside the resource_metadata +// parameter, or empty string if the parameter is absent. +func extractResourceMetadata(wwwAuthenticate string) string { + m := resourceMetadataParamRE.FindStringSubmatch(wwwAuthenticate) + if len(m) < 2 { + return "" + } + return m[1] +} + +// ── RFC 9728 §5.1 — bearer-auth middleware path ───────────────────────────── + +// agentAuthProtectedPath is a POST endpoint mounted inside the agent-auth +// middleware sub-group — POSTing to it exercises the middleware's 401 +// emission path. The agent-auth group lives under the admin path prefix +// (adminPath("...")) so the full URL is "/api/v1/proof/generate". +func agentAuthProtectedPath() string { return adminPath("/proof/generate") } + +func TestRFC9728_S5_1_AgentAuthMiddleware_EmitsBreadcrumbOnMissingAuth(t *testing.T) { + // Probe an agent-auth-protected endpoint with no Authorization header + // — the AgentAuthMiddleware MUST return 401 with the RFC 9728 §5.1 + // resource_metadata parameter so a cold-start client can discover PRM. + resp := post(t, agentAuthProtectedPath(), map[string]any{}, nil) + defer resp.Body.Close() + require.Equal(t, http.StatusUnauthorized, resp.StatusCode, + "bearer-protected endpoint with no auth MUST 401") + + wwwAuth := resp.Header.Get("WWW-Authenticate") + require.NotEmpty(t, wwwAuth, + "401 MUST include a WWW-Authenticate challenge (RFC 6750 §3 / RFC 9728 §5.1)") + + url := extractResourceMetadata(wwwAuth) + require.NotEmpty(t, url, + "WWW-Authenticate MUST include the resource_metadata parameter (RFC 9728 §5.1); got %q", wwwAuth) + assert.Equal(t, prmURL(), url, + "resource_metadata MUST point at {issuer}/.well-known/oauth-protected-resource") +} + +func TestRFC9728_S5_1_AgentAuthMiddleware_EmitsBreadcrumbOnInvalidToken(t *testing.T) { + // Same path but with a bogus Bearer token — must still get the + // breadcrumb. The error code is "invalid_token" per RFC 6750 §3.1. + resp := post(t, agentAuthProtectedPath(), map[string]any{}, map[string]string{ + "Authorization": "Bearer not-a-real-token", + }) + defer resp.Body.Close() + require.Equal(t, http.StatusUnauthorized, resp.StatusCode) + + wwwAuth := resp.Header.Get("WWW-Authenticate") + require.NotEmpty(t, wwwAuth) + + assert.Equal(t, prmURL(), extractResourceMetadata(wwwAuth), + "resource_metadata MUST point at PRM URL on invalid-token 401 as well") + + // RFC 6750 §3.1: error parameter MUST be one of invalid_request, + // invalid_token, insufficient_scope on a 401. + assert.Contains(t, wwwAuth, `error="invalid_token"`, + "WWW-Authenticate MUST advertise error=invalid_token for a bogus bearer (RFC 6750 §3.1)") +} + +// ── RFC 9728 §5.1 — DCR auth path ─────────────────────────────────────────── + +func TestRFC9728_S5_1_DCR_EmitsBreadcrumbOnInvalidToken(t *testing.T) { + // DCR with a bogus initial access token — handler reaches dcrErr and + // emits the breadcrumb via DCROutput.WWWAuthenticate. (We don't probe + // the "no Authorization header" path because huma rejects it with 422 + // at input validation time — required:"true" on the Authorization + // field — before the handler runs, so the breadcrumb has no + // emission site to inject from.) + resp := post(t, "/oauth2/register", map[string]any{ + "client_name": "test-bogus-iat", + }, map[string]string{ + "Authorization": "Bearer not-a-real-iat", + }) + defer resp.Body.Close() + require.Equal(t, http.StatusUnauthorized, resp.StatusCode, + "DCR with bogus initial access token MUST 401") + + wwwAuth := resp.Header.Get("WWW-Authenticate") + require.NotEmpty(t, wwwAuth, + "DCR 401 MUST include WWW-Authenticate with resource_metadata") + assert.Equal(t, prmURL(), extractResourceMetadata(wwwAuth), + "DCR resource_metadata MUST point at PRM URL") +} + +// ── RFC 9728 §5.1 — forward-auth verify path ───────────────────────────────── + +func TestRFC9728_S5_1_AuthVerify_EmitsBreadcrumbOnMissingAuth(t *testing.T) { + // GET /oauth2/token/verify is the reverse-proxy forward-auth endpoint. + // It emits its own WWW-Authenticate on 401; PR-E adds the + // resource_metadata parameter to each of its 3 emission sites. + resp := get(t, "/oauth2/token/verify", nil) + defer resp.Body.Close() + require.Equal(t, http.StatusUnauthorized, resp.StatusCode) + + wwwAuth := resp.Header.Get("WWW-Authenticate") + require.NotEmpty(t, wwwAuth) + assert.Equal(t, prmURL(), extractResourceMetadata(wwwAuth), + "forward-auth verify MUST include resource_metadata on 401") +} + +// ── RFC 6750 §3 — challenge shape stays well-formed ────────────────────────── + +func TestRFC6750_S3_ChallengeShape_BearerSchemeFirst(t *testing.T) { + // RFC 6750 §3: "The WWW-Authenticate response header field uses the + // framework defined by HTTP/1.1, Section 4.1 [RFC7235] as follows: + // challenge = 'Bearer' [ 1*SP 1#auth-param ]" + // Verify our breadcrumb-augmented challenge still starts with "Bearer" + // followed by a space and parameters, i.e. it's parseable by stock + // OAuth clients that key off the scheme. + resp := post(t, agentAuthProtectedPath(), map[string]any{}, nil) + defer resp.Body.Close() + wwwAuth := resp.Header.Get("WWW-Authenticate") + require.NotEmpty(t, wwwAuth) + assert.True(t, strings.HasPrefix(wwwAuth, "Bearer "), + `challenge MUST start with "Bearer " (RFC 6750 §3); got %q`, wwwAuth) +} + +// ── PRM endpoint reachability via the breadcrumb ──────────────────────────── + +func TestRFC9728_S5_1_BreadcrumbURLShapeIsWellFormed(t *testing.T) { + // The breadcrumb URL must be parseable and follow RFC 9728 §3's + // well-known anchoring. We assert URL shape here, not that it resolves + // — PR-E is branched off PR-D, which is independent of PR-A's PRM + // endpoint. When this branch rebases against a main that has PR-A + // merged, a separate test can extend this to fetch the URL and + // confirm 200. + resp := post(t, agentAuthProtectedPath(), map[string]any{}, nil) + defer resp.Body.Close() + url := extractResourceMetadata(resp.Header.Get("WWW-Authenticate")) + require.NotEmpty(t, url) + assert.True(t, strings.HasPrefix(url, testIssuer), + "resource_metadata MUST live under the issuer URL (RFC 9728 §3 well-known anchoring)") + assert.True(t, strings.HasSuffix(url, "/.well-known/oauth-protected-resource"), + "resource_metadata MUST end with /.well-known/oauth-protected-resource (RFC 9728 §3)") +} From 86a34e1d4fc7b870f333495360760a36b5fcac50 Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 20:08:10 -0700 Subject: [PATCH 2/7] =?UTF-8?q?fix:=20add=20TODO=20markers=20at=20out-of-s?= =?UTF-8?q?cope=20=C2=A75.1=20breadcrumb=20sites?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/handler/oauth.go | 9 +++++++++ internal/handler/signal.go | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/internal/handler/oauth.go b/internal/handler/oauth.go index 23024ee8..753c869f 100644 --- a/internal/handler/oauth.go +++ b/internal/handler/oauth.go @@ -467,6 +467,15 @@ func (a *API) bcDenyOp(ctx context.Context, input *BcDenyInput) (*BcDenyOutput, // admin error. The admin endpoints are not OAuth token endpoints, so the // RFC 6749 §5.2 error_code/error_description envelope would be misleading // here; we use plain HTTP semantics instead. +// +// TODO(RFC 9728 §5.1): the 401 branch below should emit the +// resource_metadata breadcrumb on its WWW-Authenticate header, matching +// the bearer-auth, DCR, and forward-auth paths PR #166 covered. +// huma.Error401Unauthorized doesn't accept response headers, so this +// needs a deeper huma error-injection pattern (e.g. a custom error type +// implementing huma.StatusError + a Headers() method, or a response +// hook). Deferred until that pattern is established — tracked in #165 +// follow-up. func mapBackchannelAdminError(err error) error { var oauthErr *service.OAuthError if errors.As(err, &oauthErr) { diff --git a/internal/handler/signal.go b/internal/handler/signal.go index 7194cf6d..80a6ecca 100644 --- a/internal/handler/signal.go +++ b/internal/handler/signal.go @@ -144,6 +144,13 @@ func (a *API) listSignalsOp(ctx context.Context, input *SignalListInput) (*Signa func (a *API) streamSignalsHandler(w http.ResponseWriter, r *http.Request) { tenant, err := internalMiddleware.GetTenant(r.Context()) if err != nil { + // TODO(RFC 9728 §5.1): emit the resource_metadata breadcrumb in the + // WWW-Authenticate header on this 401, alongside the rest of the + // Bearer-protected paths covered by PR #166. Deferred because this + // endpoint's 401 path is "missing admin tenant context" (different + // failure class from a Bearer-auth failure), not the cold-start + // discovery case the breadcrumb is most valuable for. See #165 + // follow-up. respondWithError(w, http.StatusUnauthorized, domain.ErrCodeUnauthorized, "missing tenant context") return } From 407abd11f6aae33ed566fd89a4250501ca96247b Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 20:15:07 -0700 Subject: [PATCH 3/7] =?UTF-8?q?fix:=20address=20PR-166=20review=20?= =?UTF-8?q?=E2=80=94=20RFC=207230=20quoting=20+=20RFC=206750=20=C2=A73=20b?= =?UTF-8?q?are=20challenge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/middleware/agent_auth.go | 18 +++++++--- internal/middleware/www_authenticate.go | 44 ++++++++++++++++++------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/internal/middleware/agent_auth.go b/internal/middleware/agent_auth.go index 25abd903..011c2283 100644 --- a/internal/middleware/agent_auth.go +++ b/internal/middleware/agent_auth.go @@ -50,11 +50,21 @@ func AgentAuthMiddleware(cfg AgentAuthConfig) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { authHeader := r.Header.Get("Authorization") + if authHeader == "" { + // RFC 6750 §3: "If the request lacks any authentication + // information, the resource server SHOULD NOT include an + // error code or other error information." Emit a bare + // Bearer challenge — the RFC 9728 §5.1 resource_metadata + // breadcrumb still gets attached (the SHOULD-NOT clause + // scopes to error info, not discovery hints), so a + // cold-start client can still find PRM. + writeAgentAuthError(w, "", "", cfg.ResourceMetadataURL) + return + } if !strings.HasPrefix(authHeader, "Bearer ") { - // RFC 6750 §3.1: "invalid_request" — request lacks the - // Authorization header or it is malformed. RFC 9728 §5.1 - // breadcrumb points the cold-start client at PRM. - writeAgentAuthError(w, "invalid_request", "missing or invalid Authorization header", cfg.ResourceMetadataURL) + // Credentials WERE sent, just not in a recognized scheme — + // RFC 6750 §3.1 error_code applies here. + writeAgentAuthError(w, "invalid_request", "Authorization header must use the Bearer scheme", cfg.ResourceMetadataURL) return } tokenStr := strings.TrimPrefix(authHeader, "Bearer ") diff --git a/internal/middleware/www_authenticate.go b/internal/middleware/www_authenticate.go index 6d08c6d9..e777a564 100644 --- a/internal/middleware/www_authenticate.go +++ b/internal/middleware/www_authenticate.go @@ -1,7 +1,6 @@ package middleware import ( - "fmt" "strings" ) @@ -12,31 +11,52 @@ import ( // errorCode and errorDesc are RFC 6750 §3.1 error semantics. errorCode SHOULD // be one of "invalid_request", "invalid_token", or "insufficient_scope". An // empty errorCode emits a bare "Bearer" challenge — appropriate when no -// authentication has been attempted yet (initial 401 to an unauthenticated -// client). +// authentication has been attempted yet (RFC 6750 §3: "If the request lacks +// any authentication information, the resource server SHOULD NOT include an +// error code or other error information"). When errorCode is empty, errorDesc +// is dropped as well — error_description without error_code is meaningless and +// also violates the SHOULD-NOT-include-error-information guidance. // // resourceMetadataURL is the absolute URL of the protected resource metadata // document (typically "{issuer}/.well-known/oauth-protected-resource"). When // non-empty, it is appended as the RFC 9728 §5.1 `resource_metadata` // parameter so cold-start clients can chain resource → PRM → AS metadata -// without prior knowledge. +// without prior knowledge. The breadcrumb is independent of error info, so +// it appears in both bare challenges (missing credentials) and decorated +// challenges (invalid credentials). // -// All parameter values are double-quoted per RFC 7235 §2.1 (quoted-string). -// Callers are responsible for ensuring values are well-formed; this function -// does not URL-encode or escape. +// All parameter values are double-quoted per RFC 7235 §2.1 quoted-string +// rules — only " and \ are escaped. We do NOT use fmt's %q verb here because +// %q applies Go-specific escaping (e.g. \uXXXX for non-ASCII, \n for +// newlines) which produces strings that are not valid HTTP quoted-string per +// RFC 7230 §3.2.6. For ASCII-only inputs the two are identical, but the +// custom quote() helper stays correct if a future caller passes a +// URL containing non-ASCII (punycode, IDN, etc.) or a description with a +// literal newline that the caller forgot to strip. func WWWAuthenticate(errorCode, errorDesc, resourceMetadataURL string) string { var params []string if errorCode != "" { - params = append(params, fmt.Sprintf(`error=%q`, errorCode)) - } - if errorDesc != "" { - params = append(params, fmt.Sprintf(`error_description=%q`, errorDesc)) + params = append(params, "error="+httpQuotedString(errorCode)) + if errorDesc != "" { + params = append(params, "error_description="+httpQuotedString(errorDesc)) + } } if resourceMetadataURL != "" { - params = append(params, fmt.Sprintf(`resource_metadata=%q`, resourceMetadataURL)) + params = append(params, "resource_metadata="+httpQuotedString(resourceMetadataURL)) } if len(params) == 0 { return "Bearer" } return "Bearer " + strings.Join(params, ", ") } + +// httpQuotedString wraps s in double quotes and escapes only the two +// characters that RFC 7230 §3.2.6 quoted-string requires escaping (backslash +// and double-quote). All other octets — including UTF-8 — pass through +// unchanged, matching the RFC's allowed character set (obs-text covers any +// %x80-FF byte). +func httpQuotedString(s string) string { + s = strings.ReplaceAll(s, `\`, `\\`) + s = strings.ReplaceAll(s, `"`, `\"`) + return `"` + s + `"` +} From fefd8e910e77f17dec652cded2c32ea9f976a6c0 Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 20:48:29 -0700 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20address=20PR-166=20Copilot=20review?= =?UTF-8?q?=20=E2=80=94=20three=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/middleware/agent_auth.go | 48 ++++++++++++++++++------- internal/middleware/www_authenticate.go | 48 +++++++++++++++++++------ 2 files changed, 74 insertions(+), 22 deletions(-) diff --git a/internal/middleware/agent_auth.go b/internal/middleware/agent_auth.go index 011c2283..1082b75e 100644 --- a/internal/middleware/agent_auth.go +++ b/internal/middleware/agent_auth.go @@ -58,21 +58,37 @@ func AgentAuthMiddleware(cfg AgentAuthConfig) func(http.Handler) http.Handler { // breadcrumb still gets attached (the SHOULD-NOT clause // scopes to error info, not discovery hints), so a // cold-start client can still find PRM. - writeAgentAuthError(w, "", "", cfg.ResourceMetadataURL) + // + // The JSON body still carries a human-readable message + // (`bodyMessage`) so a developer reading the response gets + // actionable signal — the SHOULD-NOT-include-error-info + // guidance is about the WWW-Authenticate header, not the + // response body which is a ZeroID-internal convention. + writeAgentAuthError(w, "", "", "Authorization header is required", cfg.ResourceMetadataURL) return } if !strings.HasPrefix(authHeader, "Bearer ") { // Credentials WERE sent, just not in a recognized scheme — // RFC 6750 §3.1 error_code applies here. - writeAgentAuthError(w, "invalid_request", "Authorization header must use the Bearer scheme", cfg.ResourceMetadataURL) + writeAgentAuthError(w, "invalid_request", "Authorization header must use the Bearer scheme", "Authorization header must use the Bearer scheme", cfg.ResourceMetadataURL) return } tokenStr := strings.TrimPrefix(authHeader, "Bearer ") + // `Bearer ` with no token after the prefix is a malformed + // request, not a token-validation failure — there is no token + // to validate. RFC 6750 §3.1 invalid_request applies; short- + // circuiting before jwtalg.Validate also avoids an unnecessary + // JWS parse on input that can never succeed. + if tokenStr == "" { + writeAgentAuthError(w, "invalid_request", "Authorization header carries an empty Bearer token", "Authorization header carries an empty Bearer token", cfg.ResourceMetadataURL) + return + } + // Reject alg=none / HS* before any further work — JWT-SVID §3. if err := jwtalg.Validate(tokenStr); err != nil { log.Warn().Err(err).Str("path", r.URL.Path).Msg("Agent JWT rejected: bad alg") - writeAgentAuthError(w, "invalid_token", "invalid or expired token", cfg.ResourceMetadataURL) + writeAgentAuthError(w, "invalid_token", "invalid or expired token", "invalid or expired token", cfg.ResourceMetadataURL) return } @@ -83,14 +99,14 @@ func AgentAuthMiddleware(cfg AgentAuthConfig) func(http.Handler) http.Handler { ) if err != nil { log.Warn().Err(err).Str("path", r.URL.Path).Msg("Agent JWT validation failed") - writeAgentAuthError(w, "invalid_token", "invalid or expired token", cfg.ResourceMetadataURL) + writeAgentAuthError(w, "invalid_token", "invalid or expired token", "invalid or expired token", cfg.ResourceMetadataURL) return } claims := extractAgentClaims(parsed) if claims.AccountID == "" || claims.ProjectID == "" { - writeAgentAuthError(w, "invalid_token", "token missing required tenant claims", cfg.ResourceMetadataURL) + writeAgentAuthError(w, "invalid_token", "token missing required tenant claims", "token missing required tenant claims", cfg.ResourceMetadataURL) return } @@ -153,21 +169,29 @@ func extractAgentClaims(token jwt.Token) AgentClaims { } // writeAgentAuthError emits a 401 response with an RFC 6750 §3 challenge in -// the WWW-Authenticate header. The errorCode value follows RFC 6750 §3.1 -// ("invalid_request", "invalid_token", "insufficient_scope"); the message is -// surfaced both in the header (error_description) and in the JSON body for -// callers that don't inspect headers. +// the WWW-Authenticate header. +// +// - errorCode — RFC 6750 §3.1 value ("invalid_request", "invalid_token", +// "insufficient_scope"). Empty when the request lacks any auth info, per +// RFC 6750 §3 SHOULD-NOT-emit-error-info. +// - headerMessage — error_description value in the header. Same RFC 6750 §3 +// constraint as errorCode: empty when the request lacked auth info. +// - bodyMessage — human-readable message for the JSON response body. +// This is ZeroID's internal convention, NOT subject to the RFC 6750 §3 +// SHOULD-NOT clause (which scopes to the header). Always populate this so +// a developer reading the response gets actionable signal even when the +// header is intentionally bare. // // When resourceMetadataURL is non-empty, RFC 9728 §5.1's resource_metadata // parameter is appended so cold-start clients can discover the PRM document. -func writeAgentAuthError(w http.ResponseWriter, errorCode, message, resourceMetadataURL string) { - w.Header().Set("WWW-Authenticate", WWWAuthenticate(errorCode, message, resourceMetadataURL)) +func writeAgentAuthError(w http.ResponseWriter, errorCode, headerMessage, bodyMessage, resourceMetadataURL string) { + w.Header().Set("WWW-Authenticate", WWWAuthenticate(errorCode, headerMessage, resourceMetadataURL)) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusUnauthorized) _ = json.NewEncoder(w).Encode(map[string]any{ "error": map[string]any{ "code": http.StatusUnauthorized, - "message": message, + "message": bodyMessage, }, }) } diff --git a/internal/middleware/www_authenticate.go b/internal/middleware/www_authenticate.go index e777a564..cf2c77dd 100644 --- a/internal/middleware/www_authenticate.go +++ b/internal/middleware/www_authenticate.go @@ -26,13 +26,19 @@ import ( // challenges (invalid credentials). // // All parameter values are double-quoted per RFC 7235 §2.1 quoted-string -// rules — only " and \ are escaped. We do NOT use fmt's %q verb here because -// %q applies Go-specific escaping (e.g. \uXXXX for non-ASCII, \n for -// newlines) which produces strings that are not valid HTTP quoted-string per -// RFC 7230 §3.2.6. For ASCII-only inputs the two are identical, but the -// custom quote() helper stays correct if a future caller passes a -// URL containing non-ASCII (punycode, IDN, etc.) or a description with a -// literal newline that the caller forgot to strip. +// rules — only " and \ are escaped, with HTAB, CR, and LF stripped. We do +// NOT use fmt's %q verb here because %q applies Go-specific escaping (e.g. +// \uXXXX for non-ASCII) which produces strings that are not valid HTTP +// quoted-string per RFC 7230 §3.2.6. For ASCII-only inputs the two are +// nearly identical, but the custom helper stays correct if a future caller +// passes a URL containing non-ASCII (punycode, IDN, etc.). +// +// httpQuotedString defensively strips CTL characters (0x00-0x1F, 0x7F) +// other than HTAB before quoting. CTLs in a header value would be rejected +// by Go's net/http at write time, and CR/LF specifically would enable +// response-splitting attacks if any caller fed user-controlled input +// without sanitizing first. Callers SHOULD still pre-validate; the strip +// is defense-in-depth. func WWWAuthenticate(errorCode, errorDesc, resourceMetadataURL string) string { var params []string if errorCode != "" { @@ -52,11 +58,33 @@ func WWWAuthenticate(errorCode, errorDesc, resourceMetadataURL string) string { // httpQuotedString wraps s in double quotes and escapes only the two // characters that RFC 7230 §3.2.6 quoted-string requires escaping (backslash -// and double-quote). All other octets — including UTF-8 — pass through -// unchanged, matching the RFC's allowed character set (obs-text covers any -// %x80-FF byte). +// and double-quote). CTL characters (0x00-0x1F and 0x7F) other than HTAB are +// stripped — they're disallowed in HTTP header field values, Go's net/http +// rejects them at write time, and CR/LF specifically would enable response- +// splitting attacks if user-controlled input ever reached this helper. All +// other octets — including UTF-8 — pass through unchanged, matching the RFC's +// allowed character set (obs-text covers any %x80-FF byte). func httpQuotedString(s string) string { + s = stripCTL(s) s = strings.ReplaceAll(s, `\`, `\\`) s = strings.ReplaceAll(s, `"`, `\"`) return `"` + s + `"` } + +// stripCTL removes CTL characters per RFC 7230 §3.2.6's exclusion: control +// characters (%x00-1F / %x7F) are forbidden in quoted-string values, except +// for HTAB (%x09) which is explicitly permitted in obs-text. The strip is +// intentionally lossy — a header value with a stray newline is broken +// whether we strip it or emit it; stripping is the safer of the two +// failure modes. +func stripCTL(s string) string { + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); i++ { + c := s[i] + if c == '\t' || (c >= 0x20 && c != 0x7F) { + b.WriteByte(c) + } + } + return b.String() +} From 1ace4c7bb0058d7ec5232d37428b19ad14920dad Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 21:32:29 -0700 Subject: [PATCH 5/7] fix: missing-tenant-headers is 400 not 401, drop dead 401 branch in backchannel admin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/handler/oauth.go | 28 ++++++++++++++++------------ internal/handler/signal.go | 16 ++++++++-------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/internal/handler/oauth.go b/internal/handler/oauth.go index 753c869f..4fb563f9 100644 --- a/internal/handler/oauth.go +++ b/internal/handler/oauth.go @@ -411,7 +411,12 @@ func (a *API) bcApproveOp(ctx context.Context, input *BcApproveInput) (*BcApprov } tenant, err := internalMiddleware.GetTenant(ctx) if err != nil { - return nil, huma.Error401Unauthorized("missing tenant context") + // Missing X-Account-ID / X-Project-ID is a request-formedness + // failure (admin endpoints have no built-in auth — the deployer's + // edge service is responsible for setting routing headers after + // its own auth check). 400, not 401: there are no credentials in + // play at this layer. + return nil, huma.Error400BadRequest("missing X-Account-ID or X-Project-ID header") } if err := a.backchannelSvc.Approve(ctx, service.ApproveInput{ AuthReqID: input.AuthReqID, @@ -448,7 +453,8 @@ func (a *API) bcDenyOp(ctx context.Context, input *BcDenyInput) (*BcDenyOutput, } tenant, err := internalMiddleware.GetTenant(ctx) if err != nil { - return nil, huma.Error401Unauthorized("missing tenant context") + // See bcApproveOp comment — same 400-not-401 rationale. + return nil, huma.Error400BadRequest("missing X-Account-ID or X-Project-ID header") } if err := a.backchannelSvc.Deny(ctx, service.DenyInput{ AuthReqID: input.AuthReqID, @@ -468,22 +474,20 @@ func (a *API) bcDenyOp(ctx context.Context, input *BcDenyInput) (*BcDenyOutput, // RFC 6749 §5.2 error_code/error_description envelope would be misleading // here; we use plain HTTP semantics instead. // -// TODO(RFC 9728 §5.1): the 401 branch below should emit the -// resource_metadata breadcrumb on its WWW-Authenticate header, matching -// the bearer-auth, DCR, and forward-auth paths PR #166 covered. -// huma.Error401Unauthorized doesn't accept response headers, so this -// needs a deeper huma error-injection pattern (e.g. a custom error type -// implementing huma.StatusError + a Headers() method, or a response -// hook). Deferred until that pattern is established — tracked in #165 -// follow-up. +// The backchannel service produces only 400 and 500 OAuthErrors — there's +// no auth surface inside the service (admin auth happens at the handler / +// edge layer, see bcApproveOp / bcDenyOp). 401 would be returned only if +// the service started producing invalid_client errors directly, which it +// doesn't today; if that ever changes, add a case here AND consider +// whether the failure is really an OAuth client-auth failure (RFC 9728 +// §5.1 breadcrumb applies) or just a misuse of OAuthError shape (it +// doesn't apply). func mapBackchannelAdminError(err error) error { var oauthErr *service.OAuthError if errors.As(err, &oauthErr) { switch oauthErr.HTTPStatus { case http.StatusBadRequest: return huma.Error400BadRequest(oauthErr.Description) - case http.StatusUnauthorized: - return huma.Error401Unauthorized(oauthErr.Description) case http.StatusInternalServerError: return huma.Error500InternalServerError(oauthErr.Description) } diff --git a/internal/handler/signal.go b/internal/handler/signal.go index 80a6ecca..a4f3abaa 100644 --- a/internal/handler/signal.go +++ b/internal/handler/signal.go @@ -144,14 +144,14 @@ func (a *API) listSignalsOp(ctx context.Context, input *SignalListInput) (*Signa func (a *API) streamSignalsHandler(w http.ResponseWriter, r *http.Request) { tenant, err := internalMiddleware.GetTenant(r.Context()) if err != nil { - // TODO(RFC 9728 §5.1): emit the resource_metadata breadcrumb in the - // WWW-Authenticate header on this 401, alongside the rest of the - // Bearer-protected paths covered by PR #166. Deferred because this - // endpoint's 401 path is "missing admin tenant context" (different - // failure class from a Bearer-auth failure), not the cold-start - // discovery case the breadcrumb is most valuable for. See #165 - // follow-up. - respondWithError(w, http.StatusUnauthorized, domain.ErrCodeUnauthorized, "missing tenant context") + // Missing X-Account-ID / X-Project-ID is a request-formedness + // failure — the caller (typically the operator's edge service + // after its own auth check) did not include the required routing + // headers. This is 400 Bad Request, not 401: there are no + // credentials in play at this layer (ZeroID admin endpoints have + // no built-in auth; the deployer wraps them via AdminAuthMiddleware + // or the network), so RFC 6750 §3 / RFC 9728 §5.1 don't apply. + respondWithError(w, http.StatusBadRequest, domain.ErrCodeBadRequest, "missing X-Account-ID or X-Project-ID header") return } From fe3cd5a1f2fdcb4cabedb62388274c97dfdf819e Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 22:20:04 -0700 Subject: [PATCH 6/7] =?UTF-8?q?test:=20remove=20obsolete=20=C2=A75.1=20neg?= =?UTF-8?q?ative-pin=20from=20prm=5Fcompliance=5Ftest.go?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/integration/prm_compliance_test.go | 41 +++++------------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/tests/integration/prm_compliance_test.go b/tests/integration/prm_compliance_test.go index a65dc991..9ea41e58 100644 --- a/tests/integration/prm_compliance_test.go +++ b/tests/integration/prm_compliance_test.go @@ -209,39 +209,14 @@ func TestRFC9728_S3_2_NoEmptyArrayValues(t *testing.T) { } } -// ── RFC 9728 §5.1 — WWW-Authenticate Response ─────────────────────────────── - -func TestRFC9728_S5_1_WWWAuthenticateResourceMetadataNotYetEmitted(t *testing.T) { - // RFC 9728 §5.1 (WWW-Authenticate Response) defines the - // `resource_metadata` parameter as "The URL of the protected - // resource metadata," carried in the WWW-Authenticate header that a - // resource server returns on 401. - // - // This is the breadcrumb that lets a cold agent discover PRM from a - // 401 without prior knowledge. ZeroID's bearer-auth middleware does - // not currently emit this parameter — that change has wider blast - // radius (touches every 401 emission site) and lands as a follow-up. - // - // Pinning the current state explicitly: - // - If a future change wires the parameter, this test fails and the - // implementer must flip the assertion to verify the breadcrumb - // matches the well-known URL. - // - If the parameter is added inconsistently (some 401s emit it, - // others don't), the failing test localizes the gap. - // - // Probe with an obviously-invalid bearer on a protected endpoint; we - // just need *a* 401 from the bearer-auth middleware path. - resp := get(t, adminPath("/identities"), map[string]string{ - "Authorization": "Bearer not-a-real-token", - }) - defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode, - "protected endpoint with bogus token MUST 401") - - wwwAuth := resp.Header.Get("WWW-Authenticate") - assert.NotContains(t, wwwAuth, "resource_metadata=", - "RFC 9728 §5.1 breadcrumb not yet emitted — flip this assertion when the middleware change lands") -} +// RFC 9728 §5.1 (WWW-Authenticate Response) — positive coverage lives in +// tests/integration/www_authenticate_compliance_test.go, which probes the +// AgentAuthMiddleware, DCR, and forward-auth paths that actually emit the +// breadcrumb. A prior negative-pin test ("…NotYetEmitted") lived here as a +// placeholder for the deferred middleware work; it probed +// /api/v1/identities, which is admin-only (no bearer-auth middleware) and +// so couldn't actually exercise the §5.1 emission. Removed once PR-166 +// shipped the real implementation and its compliance file. // ── Cross-document consistency ────────────────────────────────────────────── From a0bd6e12d671464ad8027b875819d65424b827f0 Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Thu, 28 May 2026 22:22:15 -0700 Subject: [PATCH 7/7] =?UTF-8?q?fix:=20address=20PR-166=20Copilot=20review?= =?UTF-8?q?=20batch=202=20=E2=80=94=20DCR=20error=20code=20+=20comment=20a?= =?UTF-8?q?ccuracy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- internal/handler/dynamic_registration.go | 12 ++++++++++-- internal/middleware/www_authenticate.go | 3 ++- tests/integration/auth_verify_test.go | 9 ++++++--- tests/integration/prm_compliance_test.go | 8 ++++---- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/internal/handler/dynamic_registration.go b/internal/handler/dynamic_registration.go index eff83c9d..42370d48 100644 --- a/internal/handler/dynamic_registration.go +++ b/internal/handler/dynamic_registration.go @@ -340,7 +340,12 @@ type initialAccessTokenClaims struct { // Returns the extracted tenant claims on success or a *dcrError on failure. func (a *API) validateInitialAccessToken(authHeader string) (*initialAccessTokenClaims, *dcrError) { if !strings.HasPrefix(authHeader, "Bearer ") { - return nil, &dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidToken, desc: "Authorization header with Bearer initial access token is required"} + // RFC 6750 §3.1 — a missing/non-Bearer scheme is a malformed request, + // not a bad token, so the WWW-Authenticate challenge `error` (which + // dcrErr mirrors from this code) must be invalid_request. invalid_token + // is reserved for a syntactically valid Bearer credential that fails + // validation (see the jwt.Parse failure path below). + return nil, &dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidRequest, desc: "Authorization header with Bearer initial access token is required"} } tokenStr := strings.TrimPrefix(authHeader, "Bearer ") @@ -404,7 +409,10 @@ func (a *API) validateInitialAccessToken(authHeader string) (*initialAccessToken // Authorization header against the stored bcrypt hash for the path's client_id. func (a *API) authorizeDCRManagement(ctx context.Context, authHeader, clientID string) (*domain.OAuthClient, *dcrError) { if !strings.HasPrefix(authHeader, "Bearer ") { - return nil, &dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidToken, desc: "Authorization header with Bearer registration_access_token is required"} + // RFC 6750 §3.1 — missing/non-Bearer scheme is a malformed request + // (invalid_request), not a rejected credential (invalid_token). The + // real-token-but-wrong/unknown case below correctly stays invalid_token. + return nil, &dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidRequest, desc: "Authorization header with Bearer registration_access_token is required"} } regToken := strings.TrimPrefix(authHeader, "Bearer ") diff --git a/internal/middleware/www_authenticate.go b/internal/middleware/www_authenticate.go index cf2c77dd..c6731bc8 100644 --- a/internal/middleware/www_authenticate.go +++ b/internal/middleware/www_authenticate.go @@ -26,7 +26,8 @@ import ( // challenges (invalid credentials). // // All parameter values are double-quoted per RFC 7235 §2.1 quoted-string -// rules — only " and \ are escaped, with HTAB, CR, and LF stripped. We do +// rules — only " and \ are escaped, and all CTL characters except HTAB are +// stripped (HTAB is preserved as RFC 7230 §3.2.6 obs-text permits it). We do // NOT use fmt's %q verb here because %q applies Go-specific escaping (e.g. // \uXXXX for non-ASCII) which produces strings that are not valid HTTP // quoted-string per RFC 7230 §3.2.6. For ASCII-only inputs the two are diff --git a/tests/integration/auth_verify_test.go b/tests/integration/auth_verify_test.go index e9df089c..3024ff05 100644 --- a/tests/integration/auth_verify_test.go +++ b/tests/integration/auth_verify_test.go @@ -25,9 +25,12 @@ func issueAPIKeyToken(t *testing.T, externalID string) string { } // 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. +// is expected to emit for a given Bearer error code string. Note this is not +// always an RFC 6750-defined code: the forward-auth path intentionally ships +// the non-standard "missing_token" string for client compatibility (see +// auth_verify.go). Every 401 from a Bearer-protected path also 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() + `"` } diff --git a/tests/integration/prm_compliance_test.go b/tests/integration/prm_compliance_test.go index 9ea41e58..56468a1c 100644 --- a/tests/integration/prm_compliance_test.go +++ b/tests/integration/prm_compliance_test.go @@ -209,14 +209,14 @@ func TestRFC9728_S3_2_NoEmptyArrayValues(t *testing.T) { } } -// RFC 9728 §5.1 (WWW-Authenticate Response) — positive coverage lives in +// RFC 9728 §5.1 (WWW-Authenticate Response) — positive coverage now lives in // tests/integration/www_authenticate_compliance_test.go, which probes the // AgentAuthMiddleware, DCR, and forward-auth paths that actually emit the // breadcrumb. A prior negative-pin test ("…NotYetEmitted") lived here as a -// placeholder for the deferred middleware work; it probed +// placeholder for the then-deferred middleware work; it probed // /api/v1/identities, which is admin-only (no bearer-auth middleware) and -// so couldn't actually exercise the §5.1 emission. Removed once PR-166 -// shipped the real implementation and its compliance file. +// so couldn't actually exercise the §5.1 emission. It was removed when the +// real implementation and its dedicated compliance file landed. // ── Cross-document consistency ──────────────────────────────────────────────