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..42370d48 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 @@ -324,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 ") @@ -388,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/handler/oauth.go b/internal/handler/oauth.go index 92e56df4..683e5844 100644 --- a/internal/handler/oauth.go +++ b/internal/handler/oauth.go @@ -413,7 +413,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, @@ -450,7 +455,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, @@ -469,14 +475,21 @@ 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. +// +// 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/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/handler/signal.go b/internal/handler/signal.go index 7194cf6d..a4f3abaa 100644 --- a/internal/handler/signal.go +++ b/internal/handler/signal.go @@ -144,7 +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 { - 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 } diff --git a/internal/middleware/agent_auth.go b/internal/middleware/agent_auth.go index 7bc086fd..1082b75e 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. @@ -43,16 +50,45 @@ 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. + // + // 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 ") { - writeAgentAuthError(w, http.StatusUnauthorized, "missing or invalid Authorization header") + // 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", "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, http.StatusUnauthorized, "invalid or expired token") + writeAgentAuthError(w, "invalid_token", "invalid or expired token", "invalid or expired token", cfg.ResourceMetadataURL) return } @@ -63,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, http.StatusUnauthorized, "invalid or expired token") + 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, http.StatusUnauthorized, "token missing required tenant claims") + writeAgentAuthError(w, "invalid_token", "token missing required tenant claims", "token missing required tenant claims", cfg.ResourceMetadataURL) return } @@ -132,13 +168,30 @@ 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. +// +// - 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, headerMessage, bodyMessage, resourceMetadataURL string) { + w.Header().Set("WWW-Authenticate", WWWAuthenticate(errorCode, headerMessage, 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, - "message": message, + "code": http.StatusUnauthorized, + "message": bodyMessage, }, }) } diff --git a/internal/middleware/www_authenticate.go b/internal/middleware/www_authenticate.go new file mode 100644 index 00000000..c6731bc8 --- /dev/null +++ b/internal/middleware/www_authenticate.go @@ -0,0 +1,91 @@ +package middleware + +import ( + "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 (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. 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 +// 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 +// 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 != "" { + params = append(params, "error="+httpQuotedString(errorCode)) + if errorDesc != "" { + params = append(params, "error_description="+httpQuotedString(errorDesc)) + } + } + if 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). 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() +} diff --git a/server.go b/server.go index 296208cd..46fe2768 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..3024ff05 100644 --- a/tests/integration/auth_verify_test.go +++ b/tests/integration/auth_verify_test.go @@ -24,15 +24,26 @@ 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 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() + `"` +} + // 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 +55,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 +67,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 +79,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 +121,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/prm_compliance_test.go b/tests/integration/prm_compliance_test.go index a65dc991..56468a1c 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 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 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. It was removed when the +// real implementation and its dedicated compliance file landed. // ── Cross-document consistency ────────────────────────────────────────────── 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)") +}