Skip to content
11 changes: 11 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,17 @@ func validateIssuer(s string) error {
if u.Host == "" {
return fmt.Errorf("token.issuer must have a host (got %q)", s)
}
Comment thread
rsharath marked this conversation as resolved.
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. 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)
}
Expand Down
73 changes: 73 additions & 0 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions internal/handler/auth_verify.go
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -49,22 +52,22 @@ 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
}

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
}

Expand Down
33 changes: 17 additions & 16 deletions internal/handler/dynamic_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -251,15 +252,15 @@ 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 {
grantTypes = []string{"client_credentials"}
}
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
Expand All @@ -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 != "" {
Expand Down Expand Up @@ -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 ")

Expand All @@ -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
Expand All @@ -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{}
Expand All @@ -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
}
Expand All @@ -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 ")

Expand All @@ -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
}
Expand Down
19 changes: 10 additions & 9 deletions internal/handler/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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{
Expand Down
Loading