From 823a8a3ca6b069f8a31bba3486b00706281ce4d4 Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 20:00:35 -0700 Subject: [PATCH 1/7] =?UTF-8?q?feat:=20internal/oautherror=20=E2=80=94=20c?= =?UTF-8?q?anonical=20OAuth=20error=20code=20constants=20(PR-F)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces bare OAuth error code strings ("invalid_token", "invalid_grant", "invalid_dpop_proof", etc.) scattered across handler/service emission sites with named constants from a new internal/oautherror package, grouped by RFC with spec URLs preserved as comments. Covered specs: - RFC 6749 §5.2 (token endpoint errors) - RFC 6750 §3.1 (Bearer Token error codes) - RFC 7591 §3.2.2 / RFC 7592 §2.3 (DCR errors) - RFC 9396 §5.4 (RAR authorization_details) - RFC 9449 §5 (DPoP) Constants are plain string consts (not a typed alias) so they remain assignable to the many string parameter slots existing emission sites use, without forcing conversions at every call site. Test files asserting on observed wire output are intentionally left as literal strings — they verify bytes-on-the-wire, not the internal symbol. --- internal/handler/auth_verify.go | 11 +- internal/handler/dynamic_registration.go | 33 +++--- internal/handler/oauth.go | 19 ++-- internal/oautherror/codes.go | 113 +++++++++++++++++++ internal/oautherror/oautherror_test.go | 40 +++++++ internal/service/backchannel.go | 37 ++++--- internal/service/oauth.go | 133 ++++++++++++----------- internal/service/oauth_error.go | 10 +- 8 files changed, 280 insertions(+), 116 deletions(-) create mode 100644 internal/oautherror/codes.go create mode 100644 internal/oautherror/oautherror_test.go diff --git a/internal/handler/auth_verify.go b/internal/handler/auth_verify.go index fe3c9d1..4cfe687 100644 --- a/internal/handler/auth_verify.go +++ b/internal/handler/auth_verify.go @@ -1,11 +1,14 @@ package handler import ( + "fmt" "net/http" "strings" "github.com/go-chi/chi/v5" "github.com/rs/zerolog/log" + + "github.com/highflame-ai/zeroid/internal/oautherror" ) func (a *API) registerAuthVerifyRoute(router chi.Router) { @@ -49,7 +52,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", `Bearer error="invalid_request"`) + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer error=%q`, oautherror.InvalidRequest)) http.Error(w, `{"error":"invalid_authorization_header"}`, http.StatusUnauthorized) return } @@ -57,14 +60,14 @@ func (a *API) authVerifyHandler(w http.ResponseWriter, r *http.Request) { claims, err := a.oauthSvc.Introspect(r.Context(), token) if err != nil { log.Error().Err(err).Msg("auth/verify: introspect error") - http.Error(w, `{"error":"server_error"}`, http.StatusInternalServerError) + http.Error(w, fmt.Sprintf(`{"error":%q}`, oautherror.ServerError), http.StatusInternalServerError) return } active, _ := claims["active"].(bool) if !active { - w.Header().Set("WWW-Authenticate", `Bearer error="invalid_token"`) - http.Error(w, `{"error":"invalid_token"}`, http.StatusUnauthorized) + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer error=%q`, oautherror.InvalidToken)) + 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 2f20edb..cd6fe82 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/oautherror" "github.com/highflame-ai/zeroid/internal/service" ) @@ -159,10 +160,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: "invalid_client_metadata", desc: "client already exists"}), nil + return 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: "server_error", desc: "failed to register client"}), nil + return 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 @@ -216,10 +217,10 @@ 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: "invalid_token", desc: "client no longer exists"}), nil + return 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: "server_error", desc: "failed to update client registration"}), nil + return 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 } @@ -230,7 +231,7 @@ func (a *API) dcrDeleteOp(ctx context.Context, input *DCRDeleteInput) (*DCROutpu } 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: "server_error", desc: "failed to delete client registration"}), nil + return dcrErr(&dcrError{status: http.StatusInternalServerError, code: oautherror.ServerError, desc: "failed to delete client registration"}), nil } return &DCROutput{Status: http.StatusNoContent, Body: nil}, nil } @@ -251,7 +252,7 @@ type dcrValidatedFields struct { // in. Returns the normalised fields or a *dcrError ready for dcrErr(). func validateDCRClientMetadata(clientName, scopeStr, authMethodIn string, grantTypesIn []string) (*dcrValidatedFields, *dcrError) { if clientName == "" { - return nil, &dcrError{status: http.StatusBadRequest, code: "invalid_client_metadata", desc: "client_name is required"} + return nil, &dcrError{status: http.StatusBadRequest, code: oautherror.InvalidClientMetadata, desc: "client_name is required"} } grantTypes := grantTypesIn if len(grantTypes) == 0 { @@ -259,7 +260,7 @@ func validateDCRClientMetadata(clientName, scopeStr, authMethodIn string, grantT } for _, gt := range grantTypes { if !allowedDCRGrantTypes[gt] { - return nil, &dcrError{status: http.StatusBadRequest, code: "invalid_client_metadata", desc: "unsupported grant_type: " + gt} + return nil, &dcrError{status: http.StatusBadRequest, code: oautherror.InvalidClientMetadata, desc: "unsupported grant_type: " + gt} } } authMethod := authMethodIn @@ -273,9 +274,9 @@ func validateDCRClientMetadata(clientName, scopeStr, authMethodIn string, grantT case "client_secret_post", "client_secret_basic": // accepted case "none": - return nil, &dcrError{status: http.StatusBadRequest, code: "invalid_client_metadata", desc: "token_endpoint_auth_method 'none' is not supported; this server requires client authentication"} + return nil, &dcrError{status: http.StatusBadRequest, code: oautherror.InvalidClientMetadata, desc: "token_endpoint_auth_method 'none' is not supported; this server requires client authentication"} default: - return nil, &dcrError{status: http.StatusBadRequest, code: "invalid_client_metadata", desc: "unsupported token_endpoint_auth_method: " + authMethod} + return nil, &dcrError{status: http.StatusBadRequest, code: oautherror.InvalidClientMetadata, desc: "unsupported token_endpoint_auth_method: " + authMethod} } var scopes []string if scopeStr != "" { @@ -323,7 +324,7 @@ 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: "invalid_token", desc: "Authorization header with Bearer initial access token is required"} + return nil, &dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidToken, desc: "Authorization header with Bearer initial access token is required"} } tokenStr := strings.TrimPrefix(authHeader, "Bearer ") @@ -335,7 +336,7 @@ func (a *API) validateInitialAccessToken(authHeader string) (*initialAccessToken ) if err != nil { log.Info().Err(err).Msg("DCR: initial access token rejected") - return nil, &dcrError{status: http.StatusUnauthorized, code: "invalid_token", desc: "initial access token is invalid or expired"} + return nil, &dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidToken, desc: "initial access token is invalid or expired"} } // The scopes claim may decode as []string (when jwx preserves the issuance @@ -361,7 +362,7 @@ func (a *API) validateInitialAccessToken(authHeader string) (*initialAccessToken } if !hasRegisterScope { log.Info().Msg("DCR: initial access token rejected — insufficient scope") - return nil, &dcrError{status: http.StatusForbidden, code: "insufficient_scope", desc: "initial access token must have '" + dcrClientRegisterScope + "' scope"} + return nil, &dcrError{status: http.StatusForbidden, code: oautherror.InsufficientScope, desc: "initial access token must have '" + dcrClientRegisterScope + "' scope"} } claims := &initialAccessTokenClaims{} @@ -378,7 +379,7 @@ func (a *API) validateInitialAccessToken(authHeader string) (*initialAccessToken log.Info(). Str("registered_by_sub", claims.Subject). Msg("DCR: initial access token rejected — missing tenant claims") - return nil, &dcrError{status: http.StatusForbidden, code: "invalid_token", desc: "initial access token must carry account_id and project_id claims"} + return nil, &dcrError{status: http.StatusForbidden, code: oautherror.InvalidToken, desc: "initial access token must carry account_id and project_id claims"} } return claims, nil } @@ -387,7 +388,7 @@ 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: "invalid_token", desc: "Authorization header with Bearer registration_access_token is required"} + return nil, &dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidToken, desc: "Authorization header with Bearer registration_access_token is required"} } regToken := strings.TrimPrefix(authHeader, "Bearer ") @@ -396,10 +397,10 @@ func (a *API) authorizeDCRManagement(ctx context.Context, authHeader, clientID s // 401 for genuine not-found / bad token; 500 for DB / infra failures so an // outage isn't masked as an auth rejection. if errors.Is(err, service.ErrOAuthClientNotFound) { - return nil, &dcrError{status: http.StatusUnauthorized, code: "invalid_token", desc: "invalid or unknown registration_access_token"} + return nil, &dcrError{status: http.StatusUnauthorized, code: oautherror.InvalidToken, desc: "invalid or unknown registration_access_token"} } log.Error().Err(err).Str("client_id", clientID).Msg("DCR: registration-token verification failed") - return nil, &dcrError{status: http.StatusInternalServerError, code: "server_error", desc: "failed to verify registration token"} + return nil, &dcrError{status: http.StatusInternalServerError, code: oautherror.ServerError, desc: "failed to verify registration token"} } return client, nil } diff --git a/internal/handler/oauth.go b/internal/handler/oauth.go index e8a9ac0..23024ee 100644 --- a/internal/handler/oauth.go +++ b/internal/handler/oauth.go @@ -11,6 +11,7 @@ import ( "github.com/highflame-ai/zeroid/domain" internalMiddleware "github.com/highflame-ai/zeroid/internal/middleware" + "github.com/highflame-ai/zeroid/internal/oautherror" "github.com/highflame-ai/zeroid/internal/service" ) @@ -77,7 +78,7 @@ func extractOAuthError(err error) (code, description string, status int) { return "policy_violation", err.Error(), http.StatusBadRequest } if errors.Is(err, service.ErrScopesNotAllowed) { - return "insufficient_scope", err.Error(), http.StatusBadRequest + return oautherror.InsufficientScope, err.Error(), http.StatusBadRequest } // Identity gates can fire at the chokepoint after the per-grant // check passed (TOCTOU window). Map to stable error_description @@ -87,15 +88,15 @@ func extractOAuthError(err error) (code, description string, status int) { // the literal expires_at timestamp) and produce a different string // shape than the per-grant path. if errors.Is(err, domain.ErrIdentityExpired) { - return "invalid_grant", "identity_expired", http.StatusBadRequest + return oautherror.InvalidGrant, "identity_expired", http.StatusBadRequest } if errors.Is(err, domain.ErrIdentityNotUsable) { - return "invalid_grant", "identity is suspended or deactivated", http.StatusBadRequest + return oautherror.InvalidGrant, "identity is suspended or deactivated", http.StatusBadRequest } if errors.Is(err, domain.ErrCredentialExpired) { - return "invalid_grant", "credential_expired", http.StatusBadRequest + return oautherror.InvalidGrant, "credential_expired", http.StatusBadRequest } - return "server_error", "an unexpected error occurred", http.StatusInternalServerError + return oautherror.ServerError, "an unexpected error occurred", http.StatusInternalServerError } type IntrospectInput struct { @@ -241,7 +242,7 @@ func (a *API) tokenOp(ctx context.Context, input *TokenInput) (*TokenOutput, err if a.dpopSvc == nil { return &TokenOutput{ Status: http.StatusBadRequest, - Body: oauthErrorBody{Error: "invalid_dpop_proof", ErrorDescription: "DPoP is not enabled on this deployment"}, + Body: oauthErrorBody{Error: oautherror.InvalidDPoPProof, ErrorDescription: "DPoP is not enabled on this deployment"}, }, nil } // htu must match what the client signed. Prefer the request's effective URL @@ -258,12 +259,12 @@ func (a *API) tokenOp(ctx context.Context, input *TokenInput) (*TokenOutput, err log.Error().Err(dpopErr).Msg("DPoP JTI store unavailable") return &TokenOutput{ Status: http.StatusInternalServerError, - Body: oauthErrorBody{Error: "server_error", ErrorDescription: "failed to validate DPoP proof"}, + Body: oauthErrorBody{Error: oautherror.ServerError, ErrorDescription: "failed to validate DPoP proof"}, }, nil } return &TokenOutput{ Status: http.StatusBadRequest, - Body: oauthErrorBody{Error: "invalid_dpop_proof", ErrorDescription: dpopErr.Error()}, + Body: oauthErrorBody{Error: oautherror.InvalidDPoPProof, ErrorDescription: dpopErr.Error()}, }, nil } dpopThumbprint = tp @@ -363,7 +364,7 @@ func (a *API) bcAuthorizeOp(ctx context.Context, input *BcAuthorizeInput) (*BcAu // gate trips. return &BcAuthorizeOutput{ Status: http.StatusBadRequest, - Body: oauthErrorBody{Error: "unsupported_grant_type", ErrorDescription: "CIBA is not enabled on this deployment"}, + Body: oauthErrorBody{Error: oautherror.UnsupportedGrantType, ErrorDescription: "CIBA is not enabled on this deployment"}, }, nil } out, err := a.backchannelSvc.CreateAuthRequest(ctx, service.CreateAuthRequestInput{ diff --git a/internal/oautherror/codes.go b/internal/oautherror/codes.go new file mode 100644 index 0000000..380d621 --- /dev/null +++ b/internal/oautherror/codes.go @@ -0,0 +1,113 @@ +// Package oautherror defines canonical OAuth 2.0 protocol error code constants +// for the codes ZeroID emits on the wire. +// +// Why this package exists: across the codebase these codes were originally +// scattered as bare string literals. String literals compile cleanly when +// mistyped, offer no autocomplete, and have no central source of truth tying +// each value back to the spec that defines it. This package replaces those +// bare strings with named constants grouped by RFC, so the canonical value +// for every emitted error code is verifiable against the spec. +// +// The constants are deliberately untyped string consts (not a typed alias) +// so they remain assignable to the many `string` parameter slots that +// existing emission sites already use, without forcing conversions at every +// call site. This is internal scaffolding, not a public API. +// +// Spec references: +// - RFC 6749 §5.2 (token endpoint errors): +// https://www.rfc-editor.org/rfc/rfc6749#section-5.2 +// - RFC 6750 §3.1 (Bearer Token error codes): +// https://www.rfc-editor.org/rfc/rfc6750#section-3.1 +// - RFC 7591 §3.2.2 (DCR client registration errors): +// https://www.rfc-editor.org/rfc/rfc7591#section-3.2.2 +// - RFC 7592 §2.3 (DCR management errors): +// https://www.rfc-editor.org/rfc/rfc7592#section-2.3 +// - RFC 9396 §5.4 (RAR authorization_details errors): +// https://www.rfc-editor.org/rfc/rfc9396#section-5.4 +// - RFC 9449 §5 (DPoP errors): +// https://www.rfc-editor.org/rfc/rfc9449#section-5 +package oautherror + +// ── RFC 6749 §5.2 — token-endpoint error codes ────────────────────────────── +// https://www.rfc-editor.org/rfc/rfc6749#section-5.2 +const ( + // InvalidClient indicates client authentication failed (unknown client, + // no client authentication included, or unsupported authentication method). + InvalidClient = "invalid_client" + + // InvalidGrant indicates the provided authorization grant (e.g. + // authorization code, refresh token, assertion) or refresh token is + // invalid, expired, revoked, does not match the redirection URI used in + // the authorization request, or was issued to another client. + InvalidGrant = "invalid_grant" + + // UnauthorizedClient indicates the authenticated client is not authorized + // to use this authorization grant type. + UnauthorizedClient = "unauthorized_client" + + // UnsupportedGrantType indicates the authorization grant type is not + // supported by the authorization server. + UnsupportedGrantType = "unsupported_grant_type" + + // InvalidScope indicates the requested scope is invalid, unknown, + // malformed, or exceeds the scope granted by the resource owner. + InvalidScope = "invalid_scope" + + // ServerError indicates the authorization server encountered an + // unexpected condition that prevented it from fulfilling the request + // (HTTP 500 equivalent, included in RFC 6749 §5.2 by reference from §4.1.2.1). + ServerError = "server_error" +) + +// ── RFC 6750 §3.1 — Bearer Token error codes ──────────────────────────────── +// https://www.rfc-editor.org/rfc/rfc6750#section-3.1 +const ( + // InvalidRequest indicates the request is missing a required parameter, + // includes an unsupported parameter or parameter value, repeats the same + // parameter, uses more than one method for including an access token, or + // is otherwise malformed. + InvalidRequest = "invalid_request" + + // InvalidToken indicates the access token provided is expired, revoked, + // malformed, or invalid for other reasons. + InvalidToken = "invalid_token" + + // InsufficientScope indicates the request requires higher privileges + // than provided by the access token. + InsufficientScope = "insufficient_scope" +) + +// ── RFC 7591 §3.2.2 / RFC 7592 §2.3 — Dynamic Client Registration errors ──── +// https://www.rfc-editor.org/rfc/rfc7591#section-3.2.2 +// https://www.rfc-editor.org/rfc/rfc7592#section-2.3 +const ( + // InvalidClientMetadata indicates the value of one of the client metadata + // fields is invalid and the server has rejected this request. + InvalidClientMetadata = "invalid_client_metadata" + + // InvalidRedirectURI indicates the value of one or more redirection URIs + // is invalid. + InvalidRedirectURI = "invalid_redirect_uri" + + // InvalidSoftwareStatement indicates the software statement presented is + // invalid. + InvalidSoftwareStatement = "invalid_software_statement" +) + +// ── RFC 9396 §5.4 — Rich Authorization Requests ───────────────────────────── +// https://www.rfc-editor.org/rfc/rfc9396#section-5.4 +const ( + // InvalidAuthorizationDetails indicates the authorization_details + // parameter contains an unknown authorization details type or invalid + // content. + InvalidAuthorizationDetails = "invalid_authorization_details" +) + +// ── RFC 9449 §5 — DPoP (Demonstrating Proof of Possession) ────────────────── +// https://www.rfc-editor.org/rfc/rfc9449#section-5 +const ( + // InvalidDPoPProof indicates the DPoP proof JWT is missing, malformed, + // has an invalid signature, or otherwise fails the validation rules of + // RFC 9449 §4.3. + InvalidDPoPProof = "invalid_dpop_proof" +) diff --git a/internal/oautherror/oautherror_test.go b/internal/oautherror/oautherror_test.go new file mode 100644 index 0000000..8b480d9 --- /dev/null +++ b/internal/oautherror/oautherror_test.go @@ -0,0 +1,40 @@ +package oautherror + +import "testing" + +// TestConstantsAreNonEmpty documents the package's purpose and guards against +// an accidental empty-string declaration silently shipping (e.g. `const Foo = ""` +// would compile but emit a meaningless OAuth error code on the wire). +// +// Per-constant value assertions are intentionally not included: they would +// duplicate the const declarations in codes.go and add maintenance cost +// without catching real bugs. The authoritative reference is the RFC URL +// commented above each group in codes.go. +func TestConstantsAreNonEmpty(t *testing.T) { + cases := map[string]string{ + // RFC 6749 §5.2 + "InvalidClient": InvalidClient, + "InvalidGrant": InvalidGrant, + "UnauthorizedClient": UnauthorizedClient, + "UnsupportedGrantType": UnsupportedGrantType, + "InvalidScope": InvalidScope, + "ServerError": ServerError, + // RFC 6750 §3.1 + "InvalidRequest": InvalidRequest, + "InvalidToken": InvalidToken, + "InsufficientScope": InsufficientScope, + // RFC 7591 §3.2.2 / RFC 7592 §2.3 + "InvalidClientMetadata": InvalidClientMetadata, + "InvalidRedirectURI": InvalidRedirectURI, + "InvalidSoftwareStatement": InvalidSoftwareStatement, + // RFC 9396 §5.4 + "InvalidAuthorizationDetails": InvalidAuthorizationDetails, + // RFC 9449 §5 + "InvalidDPoPProof": InvalidDPoPProof, + } + for name, val := range cases { + if val == "" { + t.Errorf("%s constant is empty; OAuth error codes must be non-empty wire-format strings", name) + } + } +} diff --git a/internal/service/backchannel.go b/internal/service/backchannel.go index 620493a..53ca721 100644 --- a/internal/service/backchannel.go +++ b/internal/service/backchannel.go @@ -16,6 +16,7 @@ import ( "github.com/rs/zerolog/log" "github.com/highflame-ai/zeroid/domain" + "github.com/highflame-ai/zeroid/internal/oautherror" "github.com/highflame-ai/zeroid/internal/store/postgres" ) @@ -308,15 +309,15 @@ type CreateAuthRequestOutput struct { // error responses without re-classification. func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAuthRequestInput) (*CreateAuthRequestOutput, error) { if in.ClientID == "" { - return nil, oauthBadRequest("invalid_request", "client_id is required for bc-authorize") + return nil, oauthBadRequest(oautherror.InvalidRequest, "client_id is required for bc-authorize") } if in.AccountID == "" || in.ProjectID == "" { - return nil, oauthBadRequest("invalid_request", "account_id and project_id are required for bc-authorize") + return nil, oauthBadRequest(oautherror.InvalidRequest, "account_id and project_id are required for bc-authorize") } if in.LoginHint == "" { // CIBA Core §7.1: at least one of login_hint / login_hint_token / id_token_hint // MUST be supplied. PR 1 supports login_hint only. - return nil, oauthBadRequest("invalid_request", "login_hint is required") + return nil, oauthBadRequest(oautherror.InvalidRequest, "login_hint is required") } // Validate client exists in the tenant scope. We don't enforce a @@ -325,7 +326,7 @@ func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAut // user's approval, not the client credential. client, err := s.oauthClientSvc.GetClientByClientID(ctx, in.ClientID) if err != nil { - return nil, oauthBadRequestCause("invalid_client", fmt.Sprintf("unknown client %s", in.ClientID), err) + return nil, oauthBadRequestCause(oautherror.InvalidClient, fmt.Sprintf("unknown client %s", in.ClientID), err) } // Determine notification mode. CIBA Core §10 makes the delivery mode a @@ -342,11 +343,11 @@ func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAut switch declared { case domain.BackchannelNotificationPing, domain.BackchannelNotificationPush: if in.ClientNotificationToken == "" { - return nil, oauthBadRequest("invalid_request", + return nil, oauthBadRequest(oautherror.InvalidRequest, fmt.Sprintf("backchannel_token_delivery_mode=%s requires client_notification_token on bc-authorize", declared)) } if client.ClientNotificationEndpoint == "" { - return nil, oauthBadRequest("invalid_request", + return nil, oauthBadRequest(oautherror.InvalidRequest, "client_notification_token requires the client to have a registered client_notification_endpoint") } // Defence-in-depth: re-validate the registered endpoint at request time. @@ -357,7 +358,7 @@ func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAut // hostname might have been DNS-rebound to point at a private IP // since registration — this catches that (GHSA-599q-j34m-33vc). if err := validateNotificationEndpoint(ctx, client.ClientNotificationEndpoint, s.cfg.AllowPrivateNotificationEndpoints); err != nil { - return nil, oauthBadRequestCause("invalid_request", "client_notification_endpoint is invalid", err) + return nil, oauthBadRequestCause(oautherror.InvalidRequest, "client_notification_endpoint is invalid", err) } notificationMode = declared notificationEndpoint = client.ClientNotificationEndpoint @@ -375,7 +376,7 @@ func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAut bindingMsg := in.BindingMessage if s.cfg.MaxBindingMessageBytes > 0 && len(bindingMsg) > s.cfg.MaxBindingMessageBytes { return nil, oauthBadRequestCause( - "invalid_request", + oautherror.InvalidRequest, fmt.Sprintf("binding_message exceeds maximum length of %d bytes", s.cfg.MaxBindingMessageBytes), fmt.Errorf("%w: length %d > max %d", ErrInvalidBindingMessage, len(bindingMsg), s.cfg.MaxBindingMessageBytes), ) @@ -460,18 +461,18 @@ type ApproveInput struct { // - access_denied: row already in a terminal state, expired, or wrong tenant func (s *BackchannelService) Approve(ctx context.Context, in ApproveInput) error { if in.AuthReqID == "" || in.AccountID == "" || in.ProjectID == "" || in.SubjectID == "" { - return oauthBadRequest("invalid_request", "auth_req_id, account_id, project_id, subject_id are required to approve") + return oauthBadRequest(oautherror.InvalidRequest, "auth_req_id, account_id, project_id, subject_id are required to approve") } row, err := s.repo.GetByAuthReqID(ctx, in.AuthReqID) if err != nil { if errors.Is(err, postgres.ErrBackchannelRequestNotFound) { - return oauthBadRequest("invalid_request", "unknown auth_req_id") + return oauthBadRequest(oautherror.InvalidRequest, "unknown auth_req_id") } return oauthServerError("failed to load backchannel auth request", err) } if row.AccountID != in.AccountID || row.ProjectID != in.ProjectID { // Don't leak existence across tenants — same opaque error as "unknown". - return oauthBadRequest("invalid_request", "unknown auth_req_id") + return oauthBadRequest(oautherror.InvalidRequest, "unknown auth_req_id") } if row.Status != domain.BackchannelStatusPending { return oauthBadRequest("access_denied", fmt.Sprintf("request is in status %q and cannot be approved", row.Status)) @@ -509,17 +510,17 @@ type DenyInput struct { // Deny transitions the request to denied. Same tenant-isolation guarantees as Approve. func (s *BackchannelService) Deny(ctx context.Context, in DenyInput) error { if in.AuthReqID == "" || in.AccountID == "" || in.ProjectID == "" { - return oauthBadRequest("invalid_request", "auth_req_id, account_id, project_id are required to deny") + return oauthBadRequest(oautherror.InvalidRequest, "auth_req_id, account_id, project_id are required to deny") } row, err := s.repo.GetByAuthReqID(ctx, in.AuthReqID) if err != nil { if errors.Is(err, postgres.ErrBackchannelRequestNotFound) { - return oauthBadRequest("invalid_request", "unknown auth_req_id") + return oauthBadRequest(oautherror.InvalidRequest, "unknown auth_req_id") } return oauthServerError("failed to load backchannel auth request", err) } if row.AccountID != in.AccountID || row.ProjectID != in.ProjectID { - return oauthBadRequest("invalid_request", "unknown auth_req_id") + return oauthBadRequest(oautherror.InvalidRequest, "unknown auth_req_id") } if row.Status != domain.BackchannelStatusPending { return oauthBadRequest("access_denied", fmt.Sprintf("request is in status %q and cannot be denied", row.Status)) @@ -592,18 +593,18 @@ type RedeemInput struct { // authorization_pending, slow_down, access_denied, expired_token, invalid_grant. func (s *BackchannelService) Redeem(ctx context.Context, in RedeemInput) (*domain.AccessToken, error) { if in.AuthReqID == "" { - return nil, oauthBadRequest("invalid_grant", "auth_req_id is required for grant_type=urn:openid:params:grant-type:ciba") + return nil, oauthBadRequest(oautherror.InvalidGrant, "auth_req_id is required for grant_type=urn:openid:params:grant-type:ciba") } row, err := s.repo.GetByAuthReqID(ctx, in.AuthReqID) if err != nil { if errors.Is(err, postgres.ErrBackchannelRequestNotFound) { - return nil, oauthBadRequest("invalid_grant", "unknown auth_req_id") + return nil, oauthBadRequest(oautherror.InvalidGrant, "unknown auth_req_id") } return nil, oauthServerError("failed to load backchannel auth request", err) } if in.ClientID != "" && row.ClientID != in.ClientID { // Mismatch means a different client is polling — refuse without leaking detail. - return nil, oauthBadRequest("invalid_grant", "auth_req_id was not issued to this client") + return nil, oauthBadRequest(oautherror.InvalidGrant, "auth_req_id was not issued to this client") } // Push mode never permits polling — the token is delivered via the @@ -646,7 +647,7 @@ func (s *BackchannelService) Redeem(ctx context.Context, in RedeemInput) (*domai return s.issueTokenForApprovedRow(ctx, row, in.DPoPKeyThumbprint) default: - return nil, oauthBadRequest("invalid_grant", fmt.Sprintf("unexpected request status %q", row.Status)) + return nil, oauthBadRequest(oautherror.InvalidGrant, fmt.Sprintf("unexpected request status %q", row.Status)) } } diff --git a/internal/service/oauth.go b/internal/service/oauth.go index 9f2acfb..9359f9c 100644 --- a/internal/service/oauth.go +++ b/internal/service/oauth.go @@ -20,6 +20,7 @@ import ( "github.com/highflame-ai/zeroid/domain" "github.com/highflame-ai/zeroid/internal/jwtalg" + "github.com/highflame-ai/zeroid/internal/oautherror" "github.com/highflame-ai/zeroid/internal/signing" "github.com/highflame-ai/zeroid/internal/store/postgres" ) @@ -198,7 +199,7 @@ func (s *OAuthService) Token(ctx context.Context, req TokenRequest) (*domain.Acc return s.refreshToken(ctx, req) case string(domain.GrantTypeCIBA): if s.backchannelSvc == nil { - return nil, oauthBadRequest("unsupported_grant_type", "CIBA is not enabled on this deployment") + return nil, oauthBadRequest(oautherror.UnsupportedGrantType, "CIBA is not enabled on this deployment") } return s.backchannelSvc.Redeem(ctx, RedeemInput{ AuthReqID: req.AuthReqID, @@ -210,13 +211,13 @@ func (s *OAuthService) Token(ctx context.Context, req TokenRequest) (*domain.Acc if handler, ok := s.customGrants[req.GrantType]; ok { return handler(ctx, req) } - return nil, oauthBadRequest("unsupported_grant_type", req.GrantType) + return nil, oauthBadRequest(oautherror.UnsupportedGrantType, req.GrantType) } } func (s *OAuthService) clientCredentials(ctx context.Context, req TokenRequest) (*domain.AccessToken, error) { if req.AccountID == "" || req.ProjectID == "" { - return nil, oauthBadRequest("invalid_request", "account_id and project_id are required for client_credentials grant") + return nil, oauthBadRequest(oautherror.InvalidRequest, "account_id and project_id are required for client_credentials grant") } // Validate client credentials against the oauth_clients table. @@ -231,7 +232,7 @@ func (s *OAuthService) clientCredentials(ctx context.Context, req TokenRequest) // Ensure client_credentials grant is permitted. allowed := slices.Contains(client.GrantTypes, "client_credentials") if !allowed { - return nil, oauthBadRequest("unauthorized_client", "client not authorized for client_credentials grant") + return nil, oauthBadRequest(oautherror.UnauthorizedClient, "client not authorized for client_credentials grant") } // Parse and intersect requested scopes with the client's allowed scopes. @@ -244,10 +245,10 @@ func (s *OAuthService) clientCredentials(ctx context.Context, req TokenRequest) return nil, oauthUnauthorized(fmt.Sprintf("no identity found for client_id %s", req.ClientID), err) } if !identity.Status.IsUsable() { - return nil, oauthBadRequest("invalid_grant", "identity is suspended or deactivated") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity is suspended or deactivated") } if identity.IsExpired() { - return nil, oauthBadRequest("invalid_grant", "identity_expired") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity_expired") } // Resolve the identity policy — the authority ceiling. Without this @@ -278,49 +279,49 @@ func (s *OAuthService) clientCredentials(ctx context.Context, req TokenRequest) // iss must equal the agent's WIMSE URI; aud must equal the issuer URL. func (s *OAuthService) jwtBearer(ctx context.Context, req TokenRequest) (*domain.AccessToken, error) { if req.Subject == "" { - return nil, oauthBadRequest("invalid_request", "subject (assertion JWT) is required for jwt_bearer grant") + return nil, oauthBadRequest(oautherror.InvalidRequest, "subject (assertion JWT) is required for jwt_bearer grant") } // Reject alg=none / HS* before any further work — JWT-SVID §3. if err := jwtalg.Validate(req.Subject); err != nil { - return nil, oauthBadRequestCause("invalid_grant", "assertion JWT uses an unsupported algorithm", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "assertion JWT uses an unsupported algorithm", err) } // Peek at the assertion without signature verification to extract the iss claim (WIMSE URI). peeked, err := jwt.ParseInsecure([]byte(req.Subject)) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "assertion JWT is malformed", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "assertion JWT is malformed", err) } wimseURI, _ := peeked.Issuer() if wimseURI == "" { - return nil, oauthBadRequest("invalid_grant", "assertion JWT missing iss claim") + return nil, oauthBadRequest(oautherror.InvalidGrant, "assertion JWT missing iss claim") } // Parse tenant from the WIMSE URI itself — no caller-supplied tenant headers needed. accountID, projectID, err := s.parseWIMSEURI(wimseURI) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "invalid WIMSE URI in assertion", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "invalid WIMSE URI in assertion", err) } // Resolve the identity by WIMSE URI, scoped to the tenant extracted above. identity, err := s.identitySvc.repo.GetByWIMSEURI(ctx, wimseURI, accountID, projectID) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", fmt.Sprintf("unknown issuer %s", wimseURI), err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, fmt.Sprintf("unknown issuer %s", wimseURI), err) } if !identity.Status.IsUsable() { - return nil, oauthBadRequest("invalid_grant", "agent identity is suspended or deactivated") + return nil, oauthBadRequest(oautherror.InvalidGrant, "agent identity is suspended or deactivated") } if identity.IsExpired() { - return nil, oauthBadRequest("invalid_grant", "identity_expired") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity_expired") } if identity.PublicKeyPEM == "" { - return nil, oauthBadRequest("invalid_grant", fmt.Sprintf("no public key registered for identity %s — register a key before using jwt_bearer", identity.ID)) + return nil, oauthBadRequest(oautherror.InvalidGrant, fmt.Sprintf("no public key registered for identity %s — register a key before using jwt_bearer", identity.ID)) } agentPubKey, err := parseECPublicKeyPEM(identity.PublicKeyPEM) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "registered public key is invalid", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "registered public key is invalid", err) } // Fully validate the assertion JWT against the agent's registered public key. @@ -330,7 +331,7 @@ func (s *OAuthService) jwtBearer(ctx context.Context, req TokenRequest) (*domain jwt.WithAudience(s.issuer), ) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "assertion JWT validation failed", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "assertion JWT validation failed", err) } // RFC 7523 §3 mandatory claims. jwx's WithValidate(true) honors them @@ -338,15 +339,15 @@ func (s *OAuthService) jwtBearer(ctx context.Context, req TokenRequest) (*domain // §3 (2): "The JWT MUST contain a 'sub' (subject) claim ..." // §3 (4): "The JWT MUST contain an 'exp' (expiration) claim ..." if _, ok := assertionToken.Subject(); !ok { - return nil, oauthBadRequest("invalid_grant", "assertion JWT missing required sub claim") + return nil, oauthBadRequest(oautherror.InvalidGrant, "assertion JWT missing required sub claim") } if _, ok := assertionToken.Expiration(); !ok { - return nil, oauthBadRequest("invalid_grant", "assertion JWT missing required exp claim") + return nil, oauthBadRequest(oautherror.InvalidGrant, "assertion JWT missing required exp claim") } // iss must match the identity's WIMSE URI. if iss, _ := assertionToken.Issuer(); iss != identity.WIMSEURI { - return nil, oauthBadRequest("invalid_grant", "iss claim does not match identity WIMSE URI") + return nil, oauthBadRequest(oautherror.InvalidGrant, "iss claim does not match identity WIMSE URI") } // Resolve the identity policy — the authority ceiling for scopes, TTL, @@ -388,7 +389,7 @@ func (s *OAuthService) jwtBearer(ctx context.Context, req TokenRequest) (*domain // public key (same requirement as jwt_bearer). func (s *OAuthService) tokenExchange(ctx context.Context, req TokenRequest) (*domain.AccessToken, error) { if req.SubjectToken == "" { - return nil, oauthBadRequest("invalid_request", "subject_token is required for token_exchange grant") + return nil, oauthBadRequest(oautherror.InvalidRequest, "subject_token is required for token_exchange grant") } // RFC 8693 defines two exchange modes: @@ -403,65 +404,65 @@ func (s *OAuthService) tokenExchange(ctx context.Context, req TokenRequest) (*do // Accept both ES256 and RS256 tokens — the library matches kid + alg from the JWKS. subjectParsed, err := s.parseToken(req.SubjectToken, true) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "subject_token validation failed", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "subject_token validation failed", err) } subjectJTI, _ := subjectParsed.JwtID() if subjectJTI == "" { - return nil, oauthBadRequest("invalid_grant", "subject_token missing jti claim") + return nil, oauthBadRequest(oautherror.InvalidGrant, "subject_token missing jti claim") } // Check that the credential has not been revoked. subjectCred, active, err := s.credentialSvc.IntrospectToken(ctx, subjectJTI) if err != nil || subjectCred == nil || !active { - return nil, oauthBadRequest("invalid_grant", "subject_token is inactive or has been revoked") + return nil, oauthBadRequest(oautherror.InvalidGrant, "subject_token is inactive or has been revoked") } // Step 2: Verify the actor_token (sub-agent's signed JWT assertion). // Reject alg=none / HS* before any further work — JWT-SVID §3. if err := jwtalg.Validate(req.ActorToken); err != nil { - return nil, oauthBadRequestCause("invalid_grant", "actor_token uses an unsupported algorithm", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "actor_token uses an unsupported algorithm", err) } actorPeeked, err := jwt.ParseInsecure([]byte(req.ActorToken)) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "actor_token is malformed", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "actor_token is malformed", err) } actorWIMSEURI, _ := actorPeeked.Issuer() if actorWIMSEURI == "" { - return nil, oauthBadRequest("invalid_grant", "actor_token missing iss claim") + return nil, oauthBadRequest(oautherror.InvalidGrant, "actor_token missing iss claim") } // Derive the tenant from the actor's WIMSE URI. accountID, projectID, err := s.parseWIMSEURI(actorWIMSEURI) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "actor_token iss is not a valid WIMSE URI", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "actor_token iss is not a valid WIMSE URI", err) } // Subject and actor must belong to the same tenant. if accountID != subjectCred.AccountID || projectID != subjectCred.ProjectID { - return nil, oauthBadRequest("invalid_grant", "subject_token and actor_token must belong to the same tenant") + return nil, oauthBadRequest(oautherror.InvalidGrant, "subject_token and actor_token must belong to the same tenant") } // Look up the actor (sub-agent) identity. actorIdentity, err := s.identitySvc.repo.GetByWIMSEURI(ctx, actorWIMSEURI, accountID, projectID) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", fmt.Sprintf("unknown actor identity %s", actorWIMSEURI), err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, fmt.Sprintf("unknown actor identity %s", actorWIMSEURI), err) } if !actorIdentity.Status.IsUsable() { - return nil, oauthBadRequest("invalid_grant", "actor identity is suspended or deactivated") + return nil, oauthBadRequest(oautherror.InvalidGrant, "actor identity is suspended or deactivated") } if actorIdentity.IsExpired() { - return nil, oauthBadRequest("invalid_grant", "identity_expired") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity_expired") } if actorIdentity.PublicKeyPEM == "" { - return nil, oauthBadRequest("invalid_grant", fmt.Sprintf("no public key registered for actor identity %s — register a key before using token_exchange", actorIdentity.ID)) + return nil, oauthBadRequest(oautherror.InvalidGrant, fmt.Sprintf("no public key registered for actor identity %s — register a key before using token_exchange", actorIdentity.ID)) } // Fully validate the actor_token against the sub-agent's registered public key. actorPubKey, err := parseECPublicKeyPEM(actorIdentity.PublicKeyPEM) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "actor's registered public key is invalid", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "actor's registered public key is invalid", err) } validatedActorToken, err := jwt.Parse([]byte(req.ActorToken), @@ -470,7 +471,7 @@ func (s *OAuthService) tokenExchange(ctx context.Context, req TokenRequest) (*do jwt.WithAudience(s.issuer), ) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "actor_token validation failed", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "actor_token validation failed", err) } // RFC 7523 §3 mandatory claims apply to the actor_token too (RFC 8693 // §1.2 inherits the JWT-bearer assertion contract). jwx's validator @@ -478,13 +479,13 @@ func (s *OAuthService) tokenExchange(ctx context.Context, req TokenRequest) (*do // §3 (2): sub REQUIRED // §3 (4): exp REQUIRED if _, ok := validatedActorToken.Subject(); !ok { - return nil, oauthBadRequest("invalid_grant", "actor_token missing required sub claim") + return nil, oauthBadRequest(oautherror.InvalidGrant, "actor_token missing required sub claim") } if _, ok := validatedActorToken.Expiration(); !ok { - return nil, oauthBadRequest("invalid_grant", "actor_token missing required exp claim") + return nil, oauthBadRequest(oautherror.InvalidGrant, "actor_token missing required exp claim") } if iss, _ := validatedActorToken.Issuer(); iss != actorIdentity.WIMSEURI { - return nil, oauthBadRequest("invalid_grant", "actor_token iss does not match actor identity WIMSE URI") + return nil, oauthBadRequest(oautherror.InvalidGrant, "actor_token iss does not match actor identity WIMSE URI") } // Step 3: Resolve the actor's identity policy — the authority ceiling @@ -529,7 +530,7 @@ func (s *OAuthService) tokenExchange(ctx context.Context, req TokenRequest) (*do } } if len(scopes) == 0 { - return nil, oauthBadRequest("invalid_scope", "requested scopes are not available for delegation") + return nil, oauthBadRequest(oautherror.InvalidScope, "requested scopes are not available for delegation") } // Step 5: Compute delegation depth (increment from orchestrator's depth). @@ -595,20 +596,20 @@ func (s *OAuthService) tokenExchange(ctx context.Context, req TokenRequest) (*do func (s *OAuthService) ExternalPrincipalExchange(ctx context.Context, req TokenRequest) (*domain.AccessToken, error) { // Step 1: Verify the caller is a trusted internal service. if s.trustedServiceValidator == nil { - return nil, oauthBadRequest("invalid_grant", "external principal exchange is not configured") + return nil, oauthBadRequest(oautherror.InvalidGrant, "external principal exchange is not configured") } serviceName, err := s.trustedServiceValidator(ctx) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "caller is not a trusted service", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "caller is not a trusted service", err) } // Step 2: Validate required fields. The trusted service is responsible for // authenticating the external principal and resolving tenant context. if req.AccountID == "" || req.ProjectID == "" { - return nil, oauthBadRequest("invalid_request", "account_id and project_id are required for external principal exchange") + return nil, oauthBadRequest(oautherror.InvalidRequest, "account_id and project_id are required for external principal exchange") } if req.UserID == "" { - return nil, oauthBadRequest("invalid_request", "user_id is required for external principal exchange") + return nil, oauthBadRequest(oautherror.InvalidRequest, "user_id is required for external principal exchange") } // Step 3: Resolve the identity for the token. @@ -623,10 +624,10 @@ func (s *OAuthService) ExternalPrincipalExchange(ctx context.Context, req TokenR return nil, fmt.Errorf("invalid_request: application_id %s not found or access denied", req.ApplicationID) } if !resolved.Status.IsUsable() { - return nil, oauthBadRequest("invalid_grant", "identity is suspended or deactivated") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity is suspended or deactivated") } if resolved.IsExpired() { - return nil, oauthBadRequest("invalid_grant", "identity_expired") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity_expired") } identity = resolved } else { @@ -697,7 +698,7 @@ func (s *OAuthService) ExternalPrincipalExchange(ctx context.Context, req TokenR // Tenant is derived from the API key record — no caller-supplied headers needed. func (s *OAuthService) apiKeyGrant(ctx context.Context, req TokenRequest) (*domain.AccessToken, error) { if req.APIKey == "" { - return nil, oauthBadRequest("invalid_request", "api_key is required for api_key grant") + return nil, oauthBadRequest(oautherror.InvalidRequest, "api_key is required for api_key grant") } if !s.jwksSvc.HasRSAKeys() { @@ -713,7 +714,7 @@ func (s *OAuthService) apiKeyGrant(ctx context.Context, req TokenRequest) (*doma // GetByKeyHash already filters on state=active and rejects keys past // their expires_at — both surface as a not-found from the caller's // perspective. No service-layer expiry check needed. - return nil, oauthBadRequestCause("invalid_grant", "invalid api key", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "invalid api key", err) } // Build a synthetic identity for the API key holder. @@ -726,9 +727,9 @@ func (s *OAuthService) apiKeyGrant(ctx context.Context, req TokenRequest) (*doma log.Warn().Str("identity_id", sk.IdentityID).Str("key_id", sk.ID).Msg("API key linked to unknown identity_id, issuing without identity") identity = nil } else if !identity.Status.IsUsable() { - return nil, oauthBadRequest("invalid_grant", "identity is suspended or deactivated") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity is suspended or deactivated") } else if identity.IsExpired() { - return nil, oauthBadRequest("invalid_grant", "identity_expired") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity_expired") } } @@ -833,7 +834,7 @@ func (s *OAuthService) apiKeyGrant(ctx context.Context, req TokenRequest) (*doma // all tokens issued from the original exchange are revoked. func (s *OAuthService) authorizationCode(ctx context.Context, req TokenRequest) (*domain.AccessToken, error) { if req.Code == "" || req.CodeVerifier == "" || req.ClientID == "" || req.RedirectURI == "" { - return nil, oauthBadRequest("invalid_request", "code, code_verifier, client_id, and redirect_uri are required") + return nil, oauthBadRequest(oautherror.InvalidRequest, "code, code_verifier, client_id, and redirect_uri are required") } if s.hmacSecret == "" { @@ -844,11 +845,11 @@ func (s *OAuthService) authorizationCode(ctx context.Context, req TokenRequest) // inside the signed JWT, not in caller-supplied headers. authCode, err := decodeAuthCodeJWT(req.Code, s.hmacSecret, s.authCodeIssuer) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "invalid authorization code", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "invalid authorization code", err) } if authCode.ClientID != req.ClientID { - return nil, oauthBadRequest("invalid_grant", "client_id mismatch") + return nil, oauthBadRequest(oautherror.InvalidGrant, "client_id mismatch") } // Look up the client in the registry — this is the authoritative check. @@ -862,15 +863,15 @@ func (s *OAuthService) authorizationCode(ctx context.Context, req TokenRequest) // Verify the client is authorised to use the authorization_code grant. grantAllowed := slices.Contains(oauthClient.GrantTypes, string(domain.GrantTypeAuthorizationCode)) if !grantAllowed { - return nil, oauthBadRequest("unauthorized_client", "client is not authorized for authorization_code grant") + return nil, oauthBadRequest(oautherror.UnauthorizedClient, "client is not authorized for authorization_code grant") } if normalizeLoopback(authCode.RedirectURI) != normalizeLoopback(req.RedirectURI) { - return nil, oauthBadRequest("invalid_grant", "redirect_uri mismatch") + return nil, oauthBadRequest(oautherror.InvalidGrant, "redirect_uri mismatch") } if !verifyCodeChallenge(req.CodeVerifier, authCode.CodeChallenge) { - return nil, oauthBadRequest("invalid_grant", "PKCE verification failed") + return nil, oauthBadRequest(oautherror.InvalidGrant, "PKCE verification failed") } // ── Single-use enforcement (RFC 6749 §4.1.2) ──────────────────────── @@ -896,7 +897,7 @@ func (s *OAuthService) authorizationCode(ctx context.Context, req TokenRequest) } if !consumed { s.revokeAuthCodeTokens(ctx, authCode.JTI) - return nil, oauthBadRequest("invalid_grant", "authorization code has already been used") + return nil, oauthBadRequest(oautherror.InvalidGrant, "authorization code has already been used") } // Determine access token TTL. @@ -931,13 +932,13 @@ func (s *OAuthService) authorizationCode(ctx context.Context, req TokenRequest) if oauthClient.IdentityID != nil && *oauthClient.IdentityID != "" { linked, err := s.identitySvc.repo.GetByID(ctx, *oauthClient.IdentityID, authCode.AccountID, authCode.ProjectID) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "oauth client linked to unknown identity", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "oauth client linked to unknown identity", err) } if !linked.Status.IsUsable() { - return nil, oauthBadRequest("invalid_grant", "identity is suspended or deactivated") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity is suspended or deactivated") } if linked.IsExpired() { - return nil, oauthBadRequest("invalid_grant", "identity_expired") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity_expired") } identity = linked policy, err := s.identitySvc.ResolveCredentialPolicy(ctx, linked) @@ -1034,7 +1035,7 @@ func (s *OAuthService) revokeAuthCodeTokens(ctx context.Context, codeJTI string) // Implements single-use rotation with family-based reuse detection. func (s *OAuthService) refreshToken(ctx context.Context, req TokenRequest) (*domain.AccessToken, error) { if req.RefreshTokenStr == "" || req.ClientID == "" { - return nil, oauthBadRequest("invalid_request", "refresh_token and client_id are required") + return nil, oauthBadRequest(oautherror.InvalidRequest, "refresh_token and client_id are required") } if s.refreshTokenSvc == nil { @@ -1061,13 +1062,13 @@ func (s *OAuthService) refreshToken(ctx context.Context, req TokenRequest) (*dom // back) so the legitimate caller's next request still works. RFC 9449 // §5 carriage: the AS rejects the request without revoking the token. if errors.Is(err, ErrDPoPBindingMismatch) { - return nil, oauthBadRequest("invalid_dpop_proof", "refresh token is DPoP-bound; the presented proof does not match the original key") + return nil, oauthBadRequest(oautherror.InvalidDPoPProof, "refresh token is DPoP-bound; the presented proof does not match the original key") } - return nil, oauthBadRequestCause("invalid_grant", "invalid or expired refresh token", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "invalid or expired refresh token", err) } if oldToken.ClientID != req.ClientID { - return nil, oauthBadRequest("invalid_grant", "client_id mismatch") + return nil, oauthBadRequest(oautherror.InvalidGrant, "client_id mismatch") } // Identity gate. The link came from the OAuth client at authorization_code @@ -1085,13 +1086,13 @@ func (s *OAuthService) refreshToken(ctx context.Context, req TokenRequest) (*dom if oldToken.IdentityID != nil && *oldToken.IdentityID != "" { linked, err := s.identitySvc.repo.GetByID(ctx, *oldToken.IdentityID, oldToken.AccountID, oldToken.ProjectID) if err != nil { - return nil, oauthBadRequestCause("invalid_grant", "refresh token references unknown identity", err) + return nil, oauthBadRequestCause(oautherror.InvalidGrant, "refresh token references unknown identity", err) } if !linked.Status.IsUsable() { - return nil, oauthBadRequest("invalid_grant", "identity is suspended or deactivated") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity is suspended or deactivated") } if linked.IsExpired() { - return nil, oauthBadRequest("invalid_grant", "identity_expired") + return nil, oauthBadRequest(oautherror.InvalidGrant, "identity_expired") } identity = linked policy, err := s.identitySvc.ResolveCredentialPolicy(ctx, linked) diff --git a/internal/service/oauth_error.go b/internal/service/oauth_error.go index ba92b8c..b6f75b5 100644 --- a/internal/service/oauth_error.go +++ b/internal/service/oauth_error.go @@ -1,6 +1,10 @@ package service -import "net/http" +import ( + "net/http" + + "github.com/highflame-ai/zeroid/internal/oautherror" +) // OAuthError is the structured error type returned by OAuthService methods. // @@ -48,10 +52,10 @@ func oauthBadRequestCause(code, description string, cause error) *OAuthError { // oauthUnauthorized returns an *OAuthError for invalid_client with HTTP 401. func oauthUnauthorized(description string, cause error) *OAuthError { - return &OAuthError{Code: "invalid_client", Description: description, HTTPStatus: http.StatusUnauthorized, err: cause} + return &OAuthError{Code: oautherror.InvalidClient, Description: description, HTTPStatus: http.StatusUnauthorized, err: cause} } // oauthServerError returns an *OAuthError for server_error with HTTP 500. func oauthServerError(description string, cause error) *OAuthError { - return &OAuthError{Code: "server_error", Description: description, HTTPStatus: http.StatusInternalServerError, err: cause} + return &OAuthError{Code: oautherror.ServerError, Description: description, HTTPStatus: http.StatusInternalServerError, err: cause} } From 547ae3fe8fc4af0aa38ae555404fd1503bb0d835 Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 20:06:36 -0700 Subject: [PATCH 2/7] =?UTF-8?q?fix:=20document=20oautherror=20package=20sc?= =?UTF-8?q?ope=20=E2=80=94=20RFC=20codes=20only,=20Highflame=20codes=20sta?= =?UTF-8?q?y=20literal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses PR-167 review (concern #1): the inconsistency where some emitted error codes go through oautherror constants (RFC-defined) while others remain bare string literals (Highflame-specific, e.g. "policy_violation" in extractOAuthError) had no in-code disambiguator. Future contributors adding new emission sites needed a signal to decide which path to use. This commit adds an explicit "Scope" paragraph to the package doc that: - States the invariant: every constant here MUST map to a clause in a published RFC. - Names the alternative: Highflame-internal codes either stay literal or move to a separate Highflame-namespaced package. No code change — just the doc comment. --- internal/oautherror/codes.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/oautherror/codes.go b/internal/oautherror/codes.go index 380d621..4fea88b 100644 --- a/internal/oautherror/codes.go +++ b/internal/oautherror/codes.go @@ -26,6 +26,15 @@ // https://www.rfc-editor.org/rfc/rfc9396#section-5.4 // - RFC 9449 §5 (DPoP errors): // https://www.rfc-editor.org/rfc/rfc9449#section-5 +// +// Scope: this package covers ONLY OAuth-spec-defined error codes. Highflame- +// or product-specific codes that aren't defined by an RFC (e.g. +// "policy_violation" in extractOAuthError) stay as bare string literals at +// their emission sites — they have no canonical RFC source for this package +// to anchor on, and adding them here would dilute the "every constant maps +// to a clause in a published RFC" invariant. Future RFC-defined codes go +// here; future Highflame-internal codes either stay literal or move to a +// separate Highflame-namespaced constants package. package oautherror // ── RFC 6749 §5.2 — token-endpoint error codes ────────────────────────────── From 7842e2a261095166e9bb1c7dbfbc20e165f63365 Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 20:19:03 -0700 Subject: [PATCH 3/7] =?UTF-8?q?fix:=20address=20PR-167=20review=20?= =?UTF-8?q?=E2=80=94=20reject=20userinfo=20in=20issuer;=20add=20CIBA=20Cor?= =?UTF-8?q?e=20=C2=A711=20codes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gemini-code-assist findings: 1. config.go validateIssuer — added u.User != nil check. Issuer URLs with embedded user-info (https://user:pass@host) would leak credentials into the JWT iss claim and every published URI. RFC 8414 §2's "URL using the https scheme" language excludes user-info by reference to RFC 3986's URI composition rules for OAuth identifiers. Rejected with a clear message naming the security concern. 2. internal/oautherror/codes.go — added the OpenID CIBA Core 1.0 §11 error codes (authorization_pending, slow_down, expired_token, access_denied) under a new RFC group. CIBA codes are spec-defined wire values just like the OAuth ones; they belong here for the same reason RFC 6750 codes belong here. 3. internal/oautherror/oautherror_test.go — added the four new constants to TestConstantsAreNonEmpty so an accidental empty-string declaration fails the existing guard. Also: swept internal/service/backchannel.go (13 emission sites) to use the new constants. Per the package's documented invariant (every constant maps to an RFC clause; emission sites use the constant), the sweep keeps the codebase consistent — adding CIBA constants without sweeping their emission sites would leave a stale-string anti-pattern. One bare string intentionally left at backchannel.go:744 — it's a code-comment reference ("Most likely an OAuthError("access_denied")"), not an emission site, so the package's invariant doesn't apply. Full integration test suite passes. --- config.go | 9 +++++++++ internal/oautherror/codes.go | 26 ++++++++++++++++++++++++ internal/oautherror/oautherror_test.go | 5 +++++ internal/service/backchannel.go | 28 +++++++++++++------------- 4 files changed, 54 insertions(+), 14 deletions(-) diff --git a/config.go b/config.go index 04e5cf9..43e890a 100644 --- a/config.go +++ b/config.go @@ -326,6 +326,15 @@ func validateIssuer(s string) error { if u.Host == "" { return fmt.Errorf("token.issuer must have a host (got %q)", s) } + if u.User != nil { + // Issuer URLs with embedded user-info (https://user:pass@host) are + // not a real OAuth deployment shape — they would leak credentials + // into JWT `iss` claims, metadata documents, and every URI the + // server publishes. RFC 8414 §2's "URL using the https scheme" + // language excludes user-info by reference to RFC 3986's URI + // composition rules for OAuth identifiers. + return fmt.Errorf("token.issuer must not contain user-info (got %q): an issuer URL with embedded credentials would leak them into the JWT iss claim and every published URI", s) + } if u.RawQuery != "" { return fmt.Errorf("token.issuer must not contain query parameters (got %q): RFC 8414 §2 requires the issuer URL to have no query component", s) } diff --git a/internal/oautherror/codes.go b/internal/oautherror/codes.go index 4fea88b..ccc20f1 100644 --- a/internal/oautherror/codes.go +++ b/internal/oautherror/codes.go @@ -120,3 +120,29 @@ const ( // RFC 9449 §4.3. InvalidDPoPProof = "invalid_dpop_proof" ) + +// ── OpenID CIBA Core 1.0 §11 — Backchannel Authentication error codes ─────── +// https://openid.net/specs/openid-client-initiated-backchannel-authentication-core-1_0.html#rfc.section.11 +const ( + // AuthorizationPending indicates the authorization request is still + // pending — the end-user has not yet completed the user interaction. + // Clients SHOULD continue polling after the interval returned in the + // bc-authorize response. + AuthorizationPending = "authorization_pending" + + // SlowDown indicates the client is polling /oauth2/token too frequently. + // The poll interval is implicitly increased; clients MUST wait at least + // the new interval before polling again. + SlowDown = "slow_down" + + // ExpiredToken indicates the auth_req_id has expired and the request is + // no longer redeemable. The client MUST start a new authentication + // request via /oauth2/bc-authorize. + ExpiredToken = "expired_token" + + // AccessDenied indicates the end-user or the authorization server denied + // the request. Also used for state-machine violations (e.g. polling a + // push-delivery auth_req_id, redeeming an already-redeemed code) where + // no other code applies. + AccessDenied = "access_denied" +) diff --git a/internal/oautherror/oautherror_test.go b/internal/oautherror/oautherror_test.go index 8b480d9..35379e2 100644 --- a/internal/oautherror/oautherror_test.go +++ b/internal/oautherror/oautherror_test.go @@ -31,6 +31,11 @@ func TestConstantsAreNonEmpty(t *testing.T) { "InvalidAuthorizationDetails": InvalidAuthorizationDetails, // RFC 9449 §5 "InvalidDPoPProof": InvalidDPoPProof, + // OpenID CIBA Core §11 + "AuthorizationPending": AuthorizationPending, + "SlowDown": SlowDown, + "ExpiredToken": ExpiredToken, + "AccessDenied": AccessDenied, } for name, val := range cases { if val == "" { diff --git a/internal/service/backchannel.go b/internal/service/backchannel.go index 53ca721..f2035ea 100644 --- a/internal/service/backchannel.go +++ b/internal/service/backchannel.go @@ -475,10 +475,10 @@ func (s *BackchannelService) Approve(ctx context.Context, in ApproveInput) error return oauthBadRequest(oautherror.InvalidRequest, "unknown auth_req_id") } if row.Status != domain.BackchannelStatusPending { - return oauthBadRequest("access_denied", fmt.Sprintf("request is in status %q and cannot be approved", row.Status)) + return oauthBadRequest(oautherror.AccessDenied, fmt.Sprintf("request is in status %q and cannot be approved", row.Status)) } if time.Now().After(row.ExpiresAt) { - return oauthBadRequest("access_denied", "request has expired") + return oauthBadRequest(oautherror.AccessDenied, "request has expired") } affected, err := s.repo.MarkApproved(ctx, in.AuthReqID, in.SubjectID, in.SubjectEmail, in.SubjectName) @@ -487,7 +487,7 @@ func (s *BackchannelService) Approve(ctx context.Context, in ApproveInput) error } if affected == 0 { // Lost a race against expiry sweep or a concurrent deny. - return oauthBadRequest("access_denied", "request could not be approved (concurrent modification or expiry)") + return oauthBadRequest(oautherror.AccessDenied, "request could not be approved (concurrent modification or expiry)") } // Re-load so callbacks see the persisted approved_subject_* fields and // the updated status. Ping/push dispatchers consume these. @@ -523,14 +523,14 @@ func (s *BackchannelService) Deny(ctx context.Context, in DenyInput) error { return oauthBadRequest(oautherror.InvalidRequest, "unknown auth_req_id") } if row.Status != domain.BackchannelStatusPending { - return oauthBadRequest("access_denied", fmt.Sprintf("request is in status %q and cannot be denied", row.Status)) + return oauthBadRequest(oautherror.AccessDenied, fmt.Sprintf("request is in status %q and cannot be denied", row.Status)) } affected, err := s.repo.MarkDenied(ctx, in.AuthReqID) if err != nil { return oauthServerError("failed to mark backchannel auth request denied", err) } if affected == 0 { - return oauthBadRequest("access_denied", "request could not be denied (concurrent modification or expiry)") + return oauthBadRequest(oautherror.AccessDenied, "request could not be denied (concurrent modification or expiry)") } persisted, err := s.repo.GetByAuthReqID(ctx, in.AuthReqID) if err == nil { @@ -611,13 +611,13 @@ func (s *BackchannelService) Redeem(ctx context.Context, in RedeemInput) (*domai // callback exactly once. Allowing both would double-deliver and break // single-use semantics. if row.NotificationMode == domain.BackchannelNotificationPush { - return nil, oauthBadRequest("access_denied", "auth_req_id is delivered via push callback; polling is not permitted") + return nil, oauthBadRequest(oautherror.AccessDenied, "auth_req_id is delivered via push callback; polling is not permitted") } now := time.Now() if now.After(row.ExpiresAt) && row.Status == domain.BackchannelStatusPending { // Race against the sweep: surface expired_token immediately. - return nil, oauthBadRequest("expired_token", "the backchannel authentication request has expired") + return nil, oauthBadRequest(oautherror.ExpiredToken, "the backchannel authentication request has expired") } switch row.Status { @@ -625,23 +625,23 @@ func (s *BackchannelService) Redeem(ctx context.Context, in RedeemInput) (*domai // Enforce the slow_down floor — clients that poll faster than // MinPollInterval get a 400 even if their previous response said they could. if row.LastPolledAt != nil && now.Sub(*row.LastPolledAt) < s.cfg.MinPollInterval { - return nil, oauthBadRequest("slow_down", "polling interval must be at least the value returned by the bc-authorize response") + return nil, oauthBadRequest(oautherror.SlowDown, "polling interval must be at least the value returned by the bc-authorize response") } if err := s.repo.TouchPoll(ctx, in.AuthReqID, now); err != nil { log.Warn().Err(err).Str("auth_req_id", in.AuthReqID).Msg("failed to record poll timestamp") } - return nil, oauthBadRequest("authorization_pending", "the user has not yet acted on the authentication request") + return nil, oauthBadRequest(oautherror.AuthorizationPending, "the user has not yet acted on the authentication request") case domain.BackchannelStatusDenied: - return nil, oauthBadRequest("access_denied", "the user denied the authentication request") + return nil, oauthBadRequest(oautherror.AccessDenied, "the user denied the authentication request") case domain.BackchannelStatusExpired: - return nil, oauthBadRequest("expired_token", "the backchannel authentication request has expired") + return nil, oauthBadRequest(oautherror.ExpiredToken, "the backchannel authentication request has expired") case domain.BackchannelStatusIssued: // A successful token was already minted for this auth_req_id. Refuse // re-redemption — auth codes / auth_req_ids are single-use. - return nil, oauthBadRequest("access_denied", "auth_req_id has already been redeemed") + return nil, oauthBadRequest(oautherror.AccessDenied, "auth_req_id has already been redeemed") case domain.BackchannelStatusApproved: return s.issueTokenForApprovedRow(ctx, row, in.DPoPKeyThumbprint) @@ -678,7 +678,7 @@ func (s *BackchannelService) issueTokenForApprovedRow(ctx context.Context, row * return nil, oauthServerError("failed to commit issuance state", mErr) } if affected == 0 { - return nil, oauthBadRequest("access_denied", "auth_req_id has already been redeemed") + return nil, oauthBadRequest(oautherror.AccessDenied, "auth_req_id has already been redeemed") } // Synthesise an identity for the approved user. CIBA Core §10.1.2 requires @@ -957,7 +957,7 @@ func (s *BackchannelService) dispatchPushDenial(ctx context.Context, row *domain return } payload := map[string]any{ - "error": "access_denied", + "error": oautherror.AccessDenied, "error_description": "the user denied the authentication request", "auth_req_id": row.AuthReqID, } From 20e91c878f91f4763bec49778ccf8b3464ca6a75 Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 21:46:56 -0700 Subject: [PATCH 4/7] fix: sweep PR-164's new bare invalid_authorization_details strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the main-merge update brought PR-164's RAR-side code in, internal/service/backchannel.go gained 3 new emission sites with bare "invalid_authorization_details" strings (lines 758, 769, 801). These are RFC 9396 §5.4 codes that ARE in the oautherror package, so per PR-167's documented invariant ("every constant maps to an RFC clause; emission sites use the constant") they should use the existing oautherror.InvalidAuthorizationDetails — leaving them as literals would be the anti-pattern this PR exists to eliminate. No new constants needed; the package already had it. Full integration test suite passes. --- internal/service/backchannel.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/backchannel.go b/internal/service/backchannel.go index f2035ea..e6ddd2f 100644 --- a/internal/service/backchannel.go +++ b/internal/service/backchannel.go @@ -755,7 +755,7 @@ func (s *BackchannelService) parseAndValidateAuthorizationDetails(raw []byte) (d if len(raw) > domain.MaxAuthorizationDetailsBytes { return nil, nil, oauthBadRequestCause( - "invalid_authorization_details", + oautherror.InvalidAuthorizationDetails, fmt.Sprintf("authorization_details exceeds %d bytes", domain.MaxAuthorizationDetailsBytes), fmt.Errorf("%w: length %d > cap %d", domain.ErrAuthorizationDetailsOversized, @@ -766,7 +766,7 @@ func (s *BackchannelService) parseAndValidateAuthorizationDetails(raw []byte) (d parsed, err := domain.ParseAuthorizationDetails(raw) if err != nil { return nil, nil, oauthBadRequestCause( - "invalid_authorization_details", + oautherror.InvalidAuthorizationDetails, "authorization_details is not a valid RFC 9396 array of typed objects", err, ) @@ -798,7 +798,7 @@ func (s *BackchannelService) parseAndValidateAuthorizationDetails(raw []byte) (d // code clients should see for any RAR-side rejection. if vErr := runRARValidator(fn, d.Raw); vErr != nil { return nil, nil, oauthBadRequestCause( - "invalid_authorization_details", + oautherror.InvalidAuthorizationDetails, fmt.Sprintf("authorization_details[%d] (type=%q): %s", i, d.Type, vErr.Error()), vErr, ) From 7a7e70ad0a8d538da740ca0a68e9a5b70bcc408a Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 21:56:03 -0700 Subject: [PATCH 5/7] fix: address self-review findings on PR-167 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two concerns flagged in the post-rebase code review: 1. internal/oautherror/codes.go — the "Scope" paragraph said emission sites use constants but didn't explicitly cover comments. Future grep results for bare error code strings in non-test code would hit the constant declarations AND any prose-style comments that quote the wire value ("returns invalid_grant on ..."), creating a false-positive "incomplete sweep" signal. Added a "Convention" paragraph that spells out the rule: emission sites = constants, comment quotes = bare strings allowed. 2. config.go validateIssuer userinfo error — the inline message was 175 characters, spilling past terminal width on operators' startup logs. The actionable signal ("must not contain user-info") was already in the first clause; the security rationale was duplicated from the comment block above. Trimmed to just the actionable signal; the rationale stays in the comment for code readers. Skipped: the CIBA Core §11 naming suggestion (CibaAccessDenied, etc.) — the current names match the RFC strings verbatim and there's no naming clash with another RFC in the codebase's actual usage today. Renaming would obscure the spec mapping for speculative benefit. --- config.go | 6 ++++-- internal/oautherror/codes.go | 7 +++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/config.go b/config.go index 43e890a..0cb58ea 100644 --- a/config.go +++ b/config.go @@ -332,8 +332,10 @@ func validateIssuer(s string) error { // into JWT `iss` claims, metadata documents, and every URI the // server publishes. RFC 8414 §2's "URL using the https scheme" // language excludes user-info by reference to RFC 3986's URI - // composition rules for OAuth identifiers. - return fmt.Errorf("token.issuer must not contain user-info (got %q): an issuer URL with embedded credentials would leak them into the JWT iss claim and every published URI", s) + // composition rules for OAuth identifiers. Error message kept + // short so it fits one terminal line; rationale lives in this + // comment for code readers. + return fmt.Errorf("token.issuer must not contain user-info (got %q)", s) } if u.RawQuery != "" { return fmt.Errorf("token.issuer must not contain query parameters (got %q): RFC 8414 §2 requires the issuer URL to have no query component", s) diff --git a/internal/oautherror/codes.go b/internal/oautherror/codes.go index ccc20f1..e00f346 100644 --- a/internal/oautherror/codes.go +++ b/internal/oautherror/codes.go @@ -35,6 +35,13 @@ // to a clause in a published RFC" invariant. Future RFC-defined codes go // here; future Highflame-internal codes either stay literal or move to a // separate Highflame-namespaced constants package. +// +// Convention: the rule covers *emission sites* only — anywhere a wire-bound +// error code is produced (a call to oauthBadRequest, a header value, a JSON +// field). Comments and doc strings that quote a code by its literal value +// for documentation purposes (e.g. "// returns invalid_grant on …") may use +// the bare string. Grepping for "invalid_grant" should hit either the +// constant declaration in this file OR a comment, not an emission site. package oautherror // ── RFC 6749 §5.2 — token-endpoint error codes ────────────────────────────── From c2aa1d44c4dbbfd66f03ffdd2123177567eecd7f Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 21:56:58 -0700 Subject: [PATCH 6/7] fix: disambiguate "separate Highflame-namespaced package" in oautherror docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original "Scope" paragraph (from 8d8945e earlier in this PR) ended with "future Highflame-internal codes either stay literal or move to a separate Highflame-namespaced constants package." Read literally that sounded like such a package exists or is being created — it doesn't, and it isn't. The clause is hypothetical guidance for *if* Highflame ever decides to formalize its product-specific codes (like "policy_violation"). Rephrased to make the hypothetical nature explicit: names the example path (internal/highflameerror), states why the separation matters (keeps oautherror's RFC-only invariant trustworthy), and ends with "No such package exists or is planned today; noted for if/when the question comes up." Pure documentation; no code change. --- internal/oautherror/codes.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/oautherror/codes.go b/internal/oautherror/codes.go index e00f346..ef3d42b 100644 --- a/internal/oautherror/codes.go +++ b/internal/oautherror/codes.go @@ -32,9 +32,15 @@ // "policy_violation" in extractOAuthError) stay as bare string literals at // their emission sites — they have no canonical RFC source for this package // to anchor on, and adding them here would dilute the "every constant maps -// to a clause in a published RFC" invariant. Future RFC-defined codes go -// here; future Highflame-internal codes either stay literal or move to a -// separate Highflame-namespaced constants package. +// to a clause in a published RFC" invariant. +// +// Future RFC-defined codes go here. Highflame-internal codes stay as +// literals; if a future need ever justifies formalizing them as constants, +// that belongs in a separate Highflame-namespaced package (e.g. +// internal/highflameerror), NOT here — keeping the "RFC-only" invariant +// stable is what makes this package's contents trustworthy for +// spec-conformance work. No such package exists or is planned today; +// noted for if/when the question comes up. // // Convention: the rule covers *emission sites* only — anywhere a wire-bound // error code is produced (a call to oauthBadRequest, a header value, a JSON From 2d81c3e3e570d284684acf4367a4e3fa1edce9bb Mon Sep 17 00:00:00 2001 From: Sharath Rajasekar Date: Mon, 25 May 2026 22:07:40 -0700 Subject: [PATCH 7/7] test: add TestValidateIssuer covering all branches incl. PR-167's userinfo check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit config.go's validateIssuer was added via PR-163 without tests, and the userinfo check this PR adds inherits that gap. Closes both before merge. Table-driven test (style matches the adjacent TestValidateWIMSEDomain) with 16 cases covering: - Valid shapes: bare host, with path, http on localhost, custom port - Empty / trailing-slash: empty string, "/" with and without path - Scheme: ftp://, missing scheme, schemeless "//host" double-slash - Host: opaque URL shape that leaves Host empty after parse - User-info: "user:pass@" and bare "user@" forms (PR-167's new check) - Query / fragment: ?key=val and #frag - Parse-failure: control character in URL (one of the few inputs that trips url.Parse — Go's url.Parse is otherwise permissive) The user-info cases are the security-relevant ones this PR added: issuer URLs with embedded credentials would leak them into the JWT iss claim, every metadata document, and every published URI. Substring assertions (not exact-equality) match the precedent and let operators read the error messages without the tests being fragile to wording tweaks. --- config_test.go | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/config_test.go b/config_test.go index bcb98b5..f995a63 100644 --- a/config_test.go +++ b/config_test.go @@ -8,6 +8,79 @@ import ( // TestValidateWIMSEDomain pins the SPIFFE §2.2 / RFC 1123 rules. The error // substring assertions exist because operators read these messages — if the // wording drifts we want the test to flag it, not just the missing reject. +// TestValidateIssuer pins RFC 8414 §2 shape constraints on Token.Issuer. +// The function is load-bearing: a malformed issuer produces silent client +// failures everywhere (discovery, JWT verification, endpoint URLs). +// Substring assertions match the test-style precedent from +// TestValidateWIMSEDomain — operators read these messages, so we pin the +// actionable substring rather than the exact text. +// +// Coverage: valid shapes (https + http for local dev, paths), and every +// rejection branch of validateIssuer (empty, trailing slash, parse failure, +// non-http(s) scheme, missing host, user-info, query, fragment). +// The user-info branch is the security-relevant case added in PR-167 — +// rejecting issuer URLs with embedded credentials prevents leaking them +// into the JWT iss claim and every published URI. +func TestValidateIssuer(t *testing.T) { + cases := []struct { + name string + input string + wantErr string // substring; "" means success + }{ + // Valid shapes. + {"https_bare_host", "https://auth.example.com", ""}, + {"https_with_path", "https://auth.example.com/v1/auth", ""}, + {"http_localhost_for_dev", "http://localhost:8899", ""}, + {"https_with_port", "https://auth.example.com:8443", ""}, + + // Empty / trailing-slash. + {"empty", "", "required"}, + {"trailing_slash", "https://auth.example.com/", "must not have a trailing slash"}, + {"trailing_slash_with_path", "https://auth.example.com/v1/", "must not have a trailing slash"}, + + // Scheme. + {"ftp_scheme", "ftp://auth.example.com", "must use http or https scheme"}, + {"missing_scheme", "auth.example.com", "must use http or https scheme"}, + {"schemeless_double_slash", "//auth.example.com", "must use http or https scheme"}, + + // Host. "https://" would naturally trip the trailing-slash check + // first, so use the opaque-URL shape that parses cleanly but + // leaves Host empty. + {"no_host_opaque", "https:foo", "must have a host"}, + + // User-info (the security-relevant check added in PR-167). + {"userinfo_with_password", "https://user:pass@auth.example.com", "must not contain user-info"}, + {"userinfo_user_only", "https://user@auth.example.com", "must not contain user-info"}, + + // Query / fragment. + {"with_query", "https://auth.example.com?foo=bar", "must not contain query parameters"}, + {"with_fragment", "https://auth.example.com#section", "must not contain a fragment"}, + + // Parse-failure paths. url.Parse is permissive in Go — most "bad" + // inputs parse cleanly as relative URLs and then fail downstream + // checks (scheme, host). A control character in the URL is one of + // the few inputs that produces a hard parse error. + {"ctl_char_fails_parse", "https://auth.example.com\x00", "must be a valid URL"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateIssuer(tc.input) + if tc.wantErr == "" { + if err != nil { + t.Fatalf("expected ok, got error: %v", err) + } + return + } + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("error %q must mention %q so operators can act on it", err.Error(), tc.wantErr) + } + }) + } +} + func TestValidateWIMSEDomain(t *testing.T) { cases := []struct { name string