From dcbdaa5e70900abac5913143fa70fbee1c812d39 Mon Sep 17 00:00:00 2001 From: Angel Marin Date: Thu, 28 May 2026 14:26:47 +0200 Subject: [PATCH 1/3] HYPERLFEET-1134 - feat: add configurable caller identity for audit attribution --- charts/Chart.yaml | 2 +- charts/templates/NOTES.txt | 25 +++ charts/templates/configmap.yaml | 5 + charts/values.yaml | 6 + .../environments/e_integration_testing.go | 5 + cmd/hyperfleet-api/environments/types.go | 2 +- cmd/hyperfleet-api/server/routes.go | 28 ++- configs/config.yaml.example | 6 + docs/authentication.md | 37 ++++ docs/config.md | 9 + pkg/auth/auth_middleware.go | 59 ++++-- pkg/auth/auth_middleware_mock.go | 16 -- pkg/auth/context.go | 67 +++++++ pkg/auth/context_test.go | 72 ++++++++ pkg/auth/identity.go | 62 +++++++ pkg/auth/identity_test.go | 169 ++++++++++++++++++ pkg/config/flags.go | 15 ++ pkg/config/loader.go | 9 + pkg/config/loader_test.go | 1 + pkg/config/logging.go | 1 + pkg/config/server.go | 59 ++++-- pkg/config/server_test.go | 46 ++++- pkg/services/cluster.go | 4 +- pkg/services/node_pool.go | 4 +- pkg/services/util.go | 8 + pkg/validation/identity_header.go | 19 ++ pkg/validation/identity_header_test.go | 13 ++ plugins/clusters/plugin.go | 5 +- plugins/nodePools/plugin.go | 5 +- test/helper.go | 15 ++ test/integration/caller_identity_test.go | 80 +++++++++ test/integration/integration_test.go | 6 + 32 files changed, 786 insertions(+), 74 deletions(-) delete mode 100755 pkg/auth/auth_middleware_mock.go create mode 100644 pkg/auth/context_test.go create mode 100644 pkg/auth/identity.go create mode 100644 pkg/auth/identity_test.go create mode 100644 pkg/validation/identity_header.go create mode 100644 pkg/validation/identity_header_test.go create mode 100644 test/integration/caller_identity_test.go diff --git a/charts/Chart.yaml b/charts/Chart.yaml index ab46db97..50903267 100644 --- a/charts/Chart.yaml +++ b/charts/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: hyperfleet-api description: HyperFleet API - Cluster Lifecycle Management Service type: application -version: 1.0.0 +version: 1.1.0 appVersion: "0.0.0-dev" maintainers: - name: HyperFleet Team diff --git a/charts/templates/NOTES.txt b/charts/templates/NOTES.txt index e7747aa4..f145d702 100644 --- a/charts/templates/NOTES.txt +++ b/charts/templates/NOTES.txt @@ -17,5 +17,30 @@ Validation schema validation is ENABLED. The API will fail to start if the schema is missing or invalid. {{- end }} +Caller identity (audit attribution): + config.server.jwt.identity_claim: {{ .Values.config.server.jwt.identity_claim | default "email" | quote }} + config.server.identity_header.enabled: {{ .Values.config.server.identity_header.enabled }} (chart default: false) + config.server.identity_header.name: {{ .Values.config.server.identity_header.name | default "X-HyperFleet-Identity" | quote }} + +When identity_header.enabled is true and a request includes a non-empty +{{ .Values.config.server.identity_header.name | default "X-HyperFleet-Identity" }} header, +that value overrides the JWT claim for audit fields (created_by, updated_by). +JWT authentication behavior is unchanged when config.server.jwt.enabled is true. +Only trusted gateways should set the identity header in production. + +Override in values.yaml: + config: + server: + jwt: + identity_claim: preferred_username + identity_header: + enabled: true + name: X-HyperFleet-Identity + +Or at install/upgrade: + --set config.server.jwt.identity_claim=preferred_username + --set config.server.identity_header.enabled=true + --set config.server.identity_header.name=X-HyperFleet-Identity + Documentation: https://github.com/openshift-hyperfleet/hyperfleet-api/blob/main/docs/deployment.md diff --git a/charts/templates/configmap.yaml b/charts/templates/configmap.yaml index 7b37626a..d1767371 100644 --- a/charts/templates/configmap.yaml +++ b/charts/templates/configmap.yaml @@ -30,6 +30,11 @@ data: enabled: {{ .Values.config.server.jwt.enabled }} issuer_url: {{ .Values.config.server.jwt.issuer_url | quote }} audience: {{ .Values.config.server.jwt.audience | quote }} + identity_claim: {{ .Values.config.server.jwt.identity_claim | quote }} + + identity_header: + enabled: {{ .Values.config.server.identity_header.enabled }} + name: {{ .Values.config.server.identity_header.name | quote }} jwk: cert_file: {{ .Values.config.server.jwk.cert_file | quote }} diff --git a/charts/values.yaml b/charts/values.yaml index c5501547..a6283e0d 100644 --- a/charts/values.yaml +++ b/charts/values.yaml @@ -52,6 +52,11 @@ config: enabled: false issuer_url: "" audience: "" + identity_claim: email + + identity_header: + enabled: false + name: X-HyperFleet-Identity jwk: cert_file: "" @@ -104,6 +109,7 @@ config: - Cookie - X-Auth-Token - X-Forwarded-Authorization + - X-HyperFleet-Identity fields: - password - secret diff --git a/cmd/hyperfleet-api/environments/e_integration_testing.go b/cmd/hyperfleet-api/environments/e_integration_testing.go index 765c41d0..7afc3c84 100755 --- a/cmd/hyperfleet-api/environments/e_integration_testing.go +++ b/cmd/hyperfleet-api/environments/e_integration_testing.go @@ -38,6 +38,11 @@ func (e *integrationTestingEnvImpl) OverrideConfig(c *config.ApplicationConfig) c.Database.SSL.Mode = SSLModeDisable } + // Avoid clashing with a local dev server on default ports + c.Server.Port = 8777 + c.Metrics.Port = 19090 + c.Health.Port = 18080 + return nil } diff --git a/cmd/hyperfleet-api/environments/types.go b/cmd/hyperfleet-api/environments/types.go index cf267930..2e26e154 100755 --- a/cmd/hyperfleet-api/environments/types.go +++ b/cmd/hyperfleet-api/environments/types.go @@ -38,7 +38,7 @@ type Database struct { } type Handlers struct { - AuthMiddleware auth.JWTMiddleware + CallerIdentityMiddleware auth.CallerIdentityMiddleware } type Services struct { diff --git a/cmd/hyperfleet-api/server/routes.go b/cmd/hyperfleet-api/server/routes.go index 4a35bbdd..c4c5117e 100755 --- a/cmd/hyperfleet-api/server/routes.go +++ b/cmd/hyperfleet-api/server/routes.go @@ -25,7 +25,6 @@ type ServicesInterface interface { type RouteRegistrationFunc func( apiV1Router *mux.Router, services ServicesInterface, - authMiddleware auth.JWTMiddleware, ) var routeRegistry = make(map[string]RouteRegistrationFunc) @@ -40,10 +39,9 @@ func RegisterRoutes(name string, registrationFunc RouteRegistrationFunc) { func LoadDiscoveredRoutes( apiV1Router *mux.Router, services ServicesInterface, - authMiddleware auth.JWTMiddleware, ) { for name, registrationFunc := range routeRegistry { - registrationFunc(apiV1Router, services, authMiddleware) + registrationFunc(apiV1Router, services) _ = name // prevent unused variable warning } } @@ -53,17 +51,6 @@ func (s *apiServer) routes(tracingEnabled bool) *mux.Router { metadataHandler := handlers.NewMetadataHandler() - var authMiddleware auth.JWTMiddleware - authMiddleware = &auth.MiddlewareMock{} - if env().Config.Server.JWT.Enabled { - var err error - authMiddleware, err = auth.NewAuthMiddleware() - check(err, "Unable to create auth middleware") - } - if authMiddleware == nil { - check(fmt.Errorf("auth middleware is nil"), "Unable to create auth middleware: missing middleware") - } - // mainRouter is top level "/" mainRouter := mux.NewRouter() mainRouter.NotFoundHandler = http.HandlerFunc(api.SendNotFound) @@ -99,8 +86,19 @@ func (s *apiServer) routes(tracingEnabled bool) *mux.Router { err = registerAPIMiddleware(apiV1Router) check(err, "Failed to initialize API middleware") + if env().Config.Server.JWT.Enabled || env().Config.Server.IdentityHeader.Enabled { + callerIdentityMW, mwErr := auth.NewCallerIdentityMiddleware(auth.CallerIdentityConfig{ + JWTEnabled: env().Config.Server.JWT.Enabled, + JWTIdentityClaim: env().Config.Server.JWT.IdentityClaim, + HeaderEnabled: env().Config.Server.IdentityHeader.Enabled, + HeaderName: env().Config.Server.IdentityHeader.Name, + }) + check(mwErr, "Unable to create caller identity middleware") + apiV1Router.Use(callerIdentityMW.ResolveCallerIdentity) + } + // Auto-discovered routes (no manual editing needed) - LoadDiscoveredRoutes(apiV1Router, services, authMiddleware) + LoadDiscoveredRoutes(apiV1Router, services) return mainRouter } diff --git a/configs/config.yaml.example b/configs/config.yaml.example index b8247d17..af30027c 100644 --- a/configs/config.yaml.example +++ b/configs/config.yaml.example @@ -21,6 +21,11 @@ server: enabled: true # Enable JWT authentication issuer_url: "" # JWT issuer URL (required when jwt.enabled=true) audience: "" # JWT audience claim (optional) + identity_claim: email # JWT claim used as request identity for audit (e.g. email, preferred_username, sub) + + identity_header: + enabled: false # Use HTTP header as caller identity (overrides JWT claim when set) + name: X-HyperFleet-Identity # Header name (must be set by a trusted gateway in production) jwk: cert_file: "" # JWK certificate file path @@ -67,6 +72,7 @@ logging: - Cookie - X-Auth-Token - X-Forwarded-Authorization + - X-HyperFleet-Identity fields: # Sensitive JSON fields to mask - password diff --git a/docs/authentication.md b/docs/authentication.md index a39b6fb1..8956b2ef 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -69,6 +69,43 @@ curl -H "Authorization: Bearer ${TOKEN}" \ http://localhost:8000/api/hyperfleet/v1/clusters ``` +## Caller identity for audit + +Authentication (JWT validation) and caller identity (audit attribution) are separate: + +| Layer | Component | Responsibility | +|-------|-----------|----------------| +| Outer | `JWTHandler` | Validates `Authorization: Bearer` token | +| Inner | `ResolveCallerIdentity` middleware | Resolves who is recorded as the actor (`CreatedBy`, force-delete logs) | + +### JWT claim + +Configure which JWT claim is used when no identity header is present: + +```yaml +server: + jwt: + identity_claim: email # or preferred_username, sub, etc. +``` + +### HTTP identity header (optional) + +When enabled, a trusted gateway can set the caller identity via HTTP header. **If the header is present and non-empty, it overrides the JWT claim** for audit fields. JWT validation is still required when `jwt.enabled=true`. + +```yaml +server: + identity_header: + enabled: true + name: X-HyperFleet-Identity +``` + +**Security:** Clients must not be able to set this header directly. Configure your ingress/gateway to strip the header from external requests and set it from the authenticated upstream user. + +```bash +export HYPERFLEET_SERVER_IDENTITY_HEADER_ENABLED=true +export HYPERFLEET_SERVER_IDENTITY_HEADER_NAME=X-HyperFleet-Identity +``` + ## Configuration ### Environment Variables diff --git a/docs/config.md b/docs/config.md index aff0003a..5be9871c 100644 --- a/docs/config.md +++ b/docs/config.md @@ -257,6 +257,9 @@ HTTP server settings for the API endpoint. | `server.jwt.enabled` | bool | `true` | Enable JWT authentication | | `server.jwt.issuer_url` | string | `""` | Expected JWT issuer URL for token validation (required when JWT is enabled) | | `server.jwt.audience` | string | `""` | Expected JWT audience claim (optional) | +| `server.jwt.identity_claim` | string | `email` | JWT claim used as request identity for audit (e.g. `email`, `preferred_username`, `sub`) | +| `server.identity_header.enabled` | bool | `false` | Enable HTTP header as caller identity source for audit | +| `server.identity_header.name` | string | `X-HyperFleet-Identity` | Header name; when set and non-empty, overrides JWT claim for audit attribution | | `server.jwk.cert_file` | string | `""` | JWK certificate file path (optional) | | `server.jwk.cert_url` | string | `""` | JWK certificate URL (required when JWT is enabled and cert_file is not set) | @@ -351,6 +354,9 @@ Complete table of all configuration properties, their environment variables, and | `server.jwt.enabled` | `HYPERFLEET_SERVER_JWT_ENABLED` | bool | `true` | | `server.jwt.issuer_url` | `HYPERFLEET_SERVER_JWT_ISSUER_URL` | string | `""` | | `server.jwt.audience` | `HYPERFLEET_SERVER_JWT_AUDIENCE` | string | `""` | +| `server.jwt.identity_claim` | `HYPERFLEET_SERVER_JWT_IDENTITY_CLAIM` | string | `email` | +| `server.identity_header.enabled` | `HYPERFLEET_SERVER_IDENTITY_HEADER_ENABLED` | bool | `false` | +| `server.identity_header.name` | `HYPERFLEET_SERVER_IDENTITY_HEADER_NAME` | string | `X-HyperFleet-Identity` | | `server.jwk.cert_file` | `HYPERFLEET_SERVER_JWK_CERT_FILE` | string | `""` | | `server.jwk.cert_url` | `HYPERFLEET_SERVER_JWK_CERT_URL` | string | `""` | | **Database** | | | | @@ -413,6 +419,9 @@ All CLI flags and their corresponding configuration paths. | `--server-jwt-enabled` | `server.jwt.enabled` | bool | | `--server-jwt-issuer-url` | `server.jwt.issuer_url` | string | | `--server-jwt-audience` | `server.jwt.audience` | string | +| `--server-jwt-identity-claim` | `server.jwt.identity_claim` | string | +| `--server-identity-header-enabled` | `server.identity_header.enabled` | bool | +| `--server-identity-header-name` | `server.identity_header.name` | string | | `--server-jwk-cert-file` | `server.jwk.cert_file` | string | | `--server-jwk-cert-url` | `server.jwk.cert_url` | string | | **Database** | | | diff --git a/pkg/auth/auth_middleware.go b/pkg/auth/auth_middleware.go index 47302300..39335805 100755 --- a/pkg/auth/auth_middleware.go +++ b/pkg/auth/auth_middleware.go @@ -5,37 +5,68 @@ import ( "net/http" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validation" ) -type JWTMiddleware interface { - AuthenticateAccountJWT(next http.Handler) http.Handler +// CallerIdentityMiddleware resolves and attaches the caller identity used for audit fields. +type CallerIdentityMiddleware interface { + ResolveCallerIdentity(next http.Handler) http.Handler } -type Middleware struct{} +type callerIdentityMiddleware struct { + cfg CallerIdentityConfig +} -var _ JWTMiddleware = &Middleware{} +var _ CallerIdentityMiddleware = &callerIdentityMiddleware{} -func NewAuthMiddleware() (*Middleware, error) { - middleware := Middleware{} - return &middleware, nil +func NewCallerIdentityMiddleware(cfg CallerIdentityConfig) (CallerIdentityMiddleware, error) { + if cfg.JWTIdentityClaim == "" { + cfg.JWTIdentityClaim = DefaultJWTIdentityClaim + } + if cfg.HeaderEnabled { + if cfg.HeaderName == "" { + return nil, fmt.Errorf("identity header name is required when identity header is enabled") + } + if validation.IsForbiddenIdentityHeaderName(cfg.HeaderName) { + return nil, fmt.Errorf("identity header name %q is not allowed", cfg.HeaderName) + } + } + return &callerIdentityMiddleware{cfg: cfg}, nil } -// AuthenticateAccountJWT Middleware handler to validate JWT tokens and authenticate users -func (a *Middleware) AuthenticateAccountJWT(next http.Handler) http.Handler { +// ResolveCallerIdentity attaches the resolved caller identity to the request context. +// JWT validation is performed by JWTHandler; this middleware only resolves attribution. +func (m *callerIdentityMiddleware) ResolveCallerIdentity(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if shouldSkipCallerIdentity(r.URL.Path) { + next.ServeHTTP(w, r) + return + } + ctx := r.Context() - payload, err := GetAuthPayload(r) + identity, err := CallerIdentityFromRequest(ctx, r, m.cfg) if err != nil { handleError( ctx, w, r, errors.CodeAuthNoCredentials, - fmt.Sprintf("Unable to get payload details from JWT token: %s", err), + fmt.Sprintf("Unable to resolve caller identity: %s", err), ) return } - // Append the username to the request context - ctx = SetUsernameContext(ctx, payload.Username) - *r = *r.WithContext(ctx) + if identity != "" { + ctx = SetUsernameContext(ctx, identity) + r = r.WithContext(ctx) + next.ServeHTTP(w, r) + return + } + + if m.cfg.JWTEnabled { + handleError( + ctx, w, r, errors.CodeAuthNoCredentials, + "Unable to resolve caller identity from JWT token or identity header", + ) + return + } next.ServeHTTP(w, r) }) diff --git a/pkg/auth/auth_middleware_mock.go b/pkg/auth/auth_middleware_mock.go deleted file mode 100755 index 5deef8e6..00000000 --- a/pkg/auth/auth_middleware_mock.go +++ /dev/null @@ -1,16 +0,0 @@ -package auth - -import ( - "net/http" -) - -type MiddlewareMock struct{} - -var _ JWTMiddleware = &MiddlewareMock{} - -func (a *MiddlewareMock) AuthenticateAccountJWT(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // TODO need to append a username to the request context - next.ServeHTTP(w, r) - }) -} diff --git a/pkg/auth/context.go b/pkg/auth/context.go index 2fbcf409..fc466174 100755 --- a/pkg/auth/context.go +++ b/pkg/auth/context.go @@ -14,6 +14,9 @@ type contextKey string const ( ContextUsernameKey contextKey = "username" ContextJWTTokenKey contextKey = "jwt_token" + + // DefaultJWTIdentityClaim is used when server.jwt.identity_claim is unset. + DefaultJWTIdentityClaim = "email" ) // Payload defines the structure of the JWT payload we expect @@ -117,6 +120,70 @@ func GetAuthPayloadFromContext(ctx context.Context) (*Payload, error) { return payload, nil } +// GetIdentityFromContext returns the configured JWT claim value used as the request identity. +func GetIdentityFromContext(ctx context.Context, identityClaim string) (string, error) { + if identityClaim == "" { + identityClaim = DefaultJWTIdentityClaim + } + + userToken := GetJWTTokenFromContext(ctx) + if userToken == nil { + return "", fmt.Errorf("JWT token in context is nil, unauthorized") + } + + claims, ok := userToken.Claims.(jwt.MapClaims) + if !ok { + return "", fmt.Errorf("unable to parse JWT token claims: unexpected type %T", userToken.Claims) + } + + if identity, ok := stringClaim(claims, identityClaim); ok { + return identity, nil + } + + payload, err := GetAuthPayloadFromContext(ctx) + if err != nil { + return "", fmt.Errorf("identity claim %q not found: %w", identityClaim, err) + } + + switch identityClaim { + case "email": + if payload.Email != "" { + return payload.Email, nil + } + case "username", "preferred_username": + if payload.Username != "" { + return payload.Username, nil + } + case "first_name", "given_name": + if payload.FirstName != "" { + return payload.FirstName, nil + } + case "last_name", "family_name": + if payload.LastName != "" { + return payload.LastName, nil + } + case "clientId": + if payload.ClientID != "" { + return payload.ClientID, nil + } + case "iss": + if payload.Issuer != "" { + return payload.Issuer, nil + } + } + + return "", fmt.Errorf("identity claim %q not found or empty", identityClaim) +} + +func stringClaim(claims jwt.MapClaims, key string) (string, bool) { + val, ok := claims[key] + if !ok { + return "", false + } + s, ok := val.(string) + return s, ok && s != "" +} + func GetAuthPayload(r *http.Request) (*Payload, error) { return GetAuthPayloadFromContext(r.Context()) } diff --git a/pkg/auth/context_test.go b/pkg/auth/context_test.go new file mode 100644 index 00000000..790aed47 --- /dev/null +++ b/pkg/auth/context_test.go @@ -0,0 +1,72 @@ +package auth + +import ( + "context" + "testing" + + "github.com/golang-jwt/jwt/v5" + . "github.com/onsi/gomega" +) + +func TestGetIdentityFromContext(t *testing.T) { + tests := []struct { + name string + claims jwt.MapClaims + identityField string + want string + wantErr bool + errSubstring string + }{ + { + name: "reads configured claim directly", + claims: jwt.MapClaims{ + "email": "user@example.com", + "sub": "subject-id", + }, + identityField: "sub", + want: "subject-id", + }, + { + name: "defaults to email when field is empty", + claims: jwt.MapClaims{"email": "user@example.com"}, + identityField: "", + want: "user@example.com", + }, + { + name: "falls back to preferred_username via payload normalization", + claims: jwt.MapClaims{"preferred_username": "jdoe"}, + identityField: "preferred_username", + want: "jdoe", + }, + { + name: "returns error when claim is missing", + claims: jwt.MapClaims{"email": "user@example.com"}, + identityField: "missing_claim", + wantErr: true, + errSubstring: "missing_claim", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + ctx := contextWithClaims(tc.claims) + + identity, err := GetIdentityFromContext(ctx, tc.identityField) + if tc.wantErr { + Expect(err).To(HaveOccurred()) + if tc.errSubstring != "" { + Expect(err.Error()).To(ContainSubstring(tc.errSubstring)) + } + return + } + Expect(err).NotTo(HaveOccurred()) + Expect(identity).To(Equal(tc.want)) + }) + } +} + +func contextWithClaims(claims jwt.MapClaims) context.Context { + token := &jwt.Token{Claims: claims} + return SetJWTTokenContext(context.Background(), token) +} diff --git a/pkg/auth/identity.go b/pkg/auth/identity.go new file mode 100644 index 00000000..1a29be5c --- /dev/null +++ b/pkg/auth/identity.go @@ -0,0 +1,62 @@ +package auth + +import ( + "context" + "fmt" + "net/http" + "strings" +) + +const maxCallerIdentityLen = 256 + +// CallerIdentityConfig controls how the caller identity is resolved for audit fields. +type CallerIdentityConfig struct { + JWTIdentityClaim string + HeaderName string + JWTEnabled bool + HeaderEnabled bool +} + +// CallerIdentityFromRequest resolves the caller identity with header-primary precedence. +// When the identity header is set and non-empty, it overrides the JWT claim. +func CallerIdentityFromRequest(ctx context.Context, r *http.Request, cfg CallerIdentityConfig) (string, error) { + if cfg.HeaderEnabled { + raw := r.Header.Get(cfg.HeaderName) + if raw != "" { + identity, err := normalizeIdentityHeaderValue(raw) + if err != nil { + return "", err + } + if identity != "" { + return identity, nil + } + } + } + + if cfg.JWTEnabled { + return GetIdentityFromContext(ctx, cfg.JWTIdentityClaim) + } + + return "", nil +} + +func normalizeIdentityHeaderValue(raw string) (string, error) { + value := strings.TrimSpace(raw) + if value == "" { + return "", nil + } + if len(value) > maxCallerIdentityLen { + return "", fmt.Errorf("identity header value exceeds maximum length %d", maxCallerIdentityLen) + } + for _, r := range value { + if r < 0x20 || r == 0x7f { + return "", fmt.Errorf("identity header value contains invalid characters") + } + } + return value, nil +} + +func shouldSkipCallerIdentity(path string) bool { + return strings.HasPrefix(path, "/api/hyperfleet/v1/openapi") || + strings.HasPrefix(path, "/api/hyperfleet/v1/errors") +} diff --git a/pkg/auth/identity_test.go b/pkg/auth/identity_test.go new file mode 100644 index 00000000..9d314037 --- /dev/null +++ b/pkg/auth/identity_test.go @@ -0,0 +1,169 @@ +package auth + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/golang-jwt/jwt/v5" + . "github.com/onsi/gomega" +) + +func TestCallerIdentityFromRequest(t *testing.T) { + tests := []struct { + name string + claims jwt.MapClaims + setHeader bool + headerValue string + cfg CallerIdentityConfig + want string + wantErr bool + }{ + { + name: "header overrides JWT claim", + claims: jwt.MapClaims{"email": "jwt@example.com"}, + setHeader: true, + headerValue: "gateway-user@example.com", + cfg: CallerIdentityConfig{ + JWTEnabled: true, + JWTIdentityClaim: "email", + HeaderEnabled: true, + HeaderName: "X-HyperFleet-Identity", + }, + want: "gateway-user@example.com", + }, + { + name: "falls back to JWT when header absent", + claims: jwt.MapClaims{"email": "jwt@example.com"}, + cfg: CallerIdentityConfig{ + JWTEnabled: true, + JWTIdentityClaim: "email", + HeaderEnabled: true, + HeaderName: "X-HyperFleet-Identity", + }, + want: "jwt@example.com", + }, + { + name: "rejects invalid header value", + setHeader: true, + headerValue: "bad\x00value", + cfg: CallerIdentityConfig{ + HeaderEnabled: true, + HeaderName: "X-HyperFleet-Identity", + }, + wantErr: true, + }, + { + name: "rejects header value exceeding max length", + setHeader: true, + headerValue: strings.Repeat("a", maxCallerIdentityLen+1), + cfg: CallerIdentityConfig{ + HeaderEnabled: true, + HeaderName: "X-HyperFleet-Identity", + }, + wantErr: true, + }, + { + name: "header only when JWT disabled", + setHeader: true, + headerValue: "dev-user", + cfg: CallerIdentityConfig{ + JWTEnabled: false, + HeaderEnabled: true, + HeaderName: "X-HyperFleet-Identity", + }, + want: "dev-user", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + r := httptest.NewRequest(http.MethodGet, "/api/hyperfleet/v1/clusters", nil) + if tc.claims != nil { + r = r.WithContext(contextWithClaims(tc.claims)) + } + if tc.setHeader { + headerName := tc.cfg.HeaderName + if headerName == "" { + headerName = "X-HyperFleet-Identity" + } + r.Header.Set(headerName, tc.headerValue) + } + + identity, err := CallerIdentityFromRequest(r.Context(), r, tc.cfg) + if tc.wantErr { + Expect(err).To(HaveOccurred()) + return + } + Expect(err).NotTo(HaveOccurred()) + Expect(identity).To(Equal(tc.want)) + }) + } +} + +func TestNewCallerIdentityMiddleware(t *testing.T) { + RegisterTestingT(t) + + t.Run("rejects forbidden header name when enabled", func(t *testing.T) { + RegisterTestingT(t) + _, err := NewCallerIdentityMiddleware(CallerIdentityConfig{ + HeaderEnabled: true, + HeaderName: "Authorization", + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not allowed")) + }) + + t.Run("returns middleware when header validation passes", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{ + HeaderEnabled: true, + HeaderName: "X-HyperFleet-Identity", + }) + Expect(err).NotTo(HaveOccurred()) + Expect(mw).NotTo(BeNil()) + }) +} + +func TestResolveCallerIdentityMiddleware(t *testing.T) { + RegisterTestingT(t) + + t.Run("skips openapi paths", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{JWTEnabled: true, JWTIdentityClaim: "email"}) + Expect(err).NotTo(HaveOccurred()) + + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + Expect(GetUsernameFromContext(r.Context())).To(BeEmpty()) + }) + + r := httptest.NewRequest(http.MethodGet, "/api/hyperfleet/v1/openapi", nil) + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(called).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + t.Run("returns 401 when JWT enabled and identity missing", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{JWTEnabled: true, JWTIdentityClaim: "email"}) + Expect(err).NotTo(HaveOccurred()) + + nextCalled := false + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + nextCalled = true + }) + + r := httptest.NewRequest(http.MethodGet, "/api/hyperfleet/v1/clusters", nil) + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + Expect(nextCalled).To(BeFalse()) + }) +} diff --git a/pkg/config/flags.go b/pkg/config/flags.go index 8c30c9a9..d1d3ba45 100644 --- a/pkg/config/flags.go +++ b/pkg/config/flags.go @@ -31,6 +31,21 @@ func AddServerFlags(cmd *cobra.Command) { cmd.Flags().Bool("server-jwt-enabled", defaults.JWT.Enabled, "Enable JWT authentication") cmd.Flags().String("server-jwt-issuer-url", defaults.JWT.IssuerURL, "Expected JWT issuer URL for token validation") cmd.Flags().String("server-jwt-audience", defaults.JWT.Audience, "Expected JWT audience (optional)") + cmd.Flags().String( + "server-jwt-identity-claim", + defaults.JWT.IdentityClaim, + "JWT claim used as request identity for audit", + ) + cmd.Flags().Bool( + "server-identity-header-enabled", + defaults.IdentityHeader.Enabled, + "Enable HTTP header as caller identity source for audit", + ) + cmd.Flags().String( + "server-identity-header-name", + defaults.IdentityHeader.Name, + "HTTP header name for caller identity (overrides JWT claim when set)", + ) cmd.Flags().String("server-jwk-cert-file", defaults.JWK.CertFile, "JWK certificate file path") cmd.Flags().String("server-jwk-cert-url", defaults.JWK.CertURL, "JWK certificate URL") } diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 2b1daca3..3a98eb40 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -179,6 +179,9 @@ func (l *ConfigLoader) validateConfig(config *ApplicationConfig) error { if valErr := config.Server.JWT.Validate(); valErr != nil { return fmt.Errorf("server JWT validation failed: %w", valErr) } + if valErr := config.Server.IdentityHeader.Validate(); valErr != nil { + return fmt.Errorf("server identity header validation failed: %w", valErr) + } if config.Server.JWT.Enabled && config.Server.JWK.CertFile == "" && config.Server.JWK.CertURL == "" { @@ -315,6 +318,9 @@ func (l *ConfigLoader) bindAllEnvVars() { l.bindEnv("server.jwt.enabled") l.bindEnv("server.jwt.issuer_url") l.bindEnv("server.jwt.audience") + l.bindEnv("server.jwt.identity_claim") + l.bindEnv("server.identity_header.enabled") + l.bindEnv("server.identity_header.name") l.bindEnv("server.jwk.cert_file") l.bindEnv("server.jwk.cert_url") // Database config @@ -384,6 +390,9 @@ func (l *ConfigLoader) bindFlags(cmd *cobra.Command) { l.bindPFlag("server.jwt.enabled", cmd.Flags().Lookup("server-jwt-enabled")) l.bindPFlag("server.jwt.issuer_url", cmd.Flags().Lookup("server-jwt-issuer-url")) l.bindPFlag("server.jwt.audience", cmd.Flags().Lookup("server-jwt-audience")) + l.bindPFlag("server.jwt.identity_claim", cmd.Flags().Lookup("server-jwt-identity-claim")) + l.bindPFlag("server.identity_header.enabled", cmd.Flags().Lookup("server-identity-header-enabled")) + l.bindPFlag("server.identity_header.name", cmd.Flags().Lookup("server-identity-header-name")) l.bindPFlag("server.jwk.cert_file", cmd.Flags().Lookup("server-jwk-cert-file")) l.bindPFlag("server.jwk.cert_url", cmd.Flags().Lookup("server-jwk-cert-url")) // Database flags: --db-* -> database.* diff --git a/pkg/config/loader_test.go b/pkg/config/loader_test.go index f3a1130b..eec2fd27 100644 --- a/pkg/config/loader_test.go +++ b/pkg/config/loader_test.go @@ -348,6 +348,7 @@ func TestConfigLoader_DefaultValues(t *testing.T) { Expect(cfg.Server.Timeouts.Write.Seconds()).To(Equal(float64(30)), "Default write timeout") Expect(cfg.Server.TLS.Enabled).To(BeFalse(), "Default TLS disabled") Expect(cfg.Server.JWT.Enabled).To(BeTrue(), "Default JWT enabled") + Expect(cfg.Server.JWT.IdentityClaim).To(Equal("email"), "Default JWT identity claim") Expect(cfg.Database.Dialect).To(Equal("postgres"), "Default database dialect") Expect(cfg.Database.Port).To(Equal(5432), "Default database port") Expect(cfg.Logging.Level).To(Equal("info"), "Default log level") diff --git a/pkg/config/logging.go b/pkg/config/logging.go index dad1b188..d6e77c18 100644 --- a/pkg/config/logging.go +++ b/pkg/config/logging.go @@ -46,6 +46,7 @@ func NewLoggingConfig() *LoggingConfig { "Cookie", "X-Auth-Token", "X-Forwarded-Authorization", + "X-HyperFleet-Identity", }, Fields: []string{ "password", diff --git a/pkg/config/server.go b/pkg/config/server.go index fd15f428..5d29376e 100755 --- a/pkg/config/server.go +++ b/pkg/config/server.go @@ -5,19 +5,22 @@ import ( "net" "strconv" "time" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validation" ) // ServerConfig holds HTTP/HTTPS server configuration // Follows HyperFleet Configuration Standard type ServerConfig struct { - Hostname string `mapstructure:"hostname" json:"hostname" validate:"omitempty,hostname|ip"` - Host string `mapstructure:"host" json:"host" validate:"required,hostname|ip"` - OpenAPISchemaPath string `mapstructure:"openapi_schema_path" json:"openapi_schema_path"` - JWK JWKConfig `mapstructure:"jwk" json:"jwk" validate:"required"` - TLS TLSConfig `mapstructure:"tls" json:"tls" validate:"required"` - JWT JWTConfig `mapstructure:"jwt" json:"jwt" validate:"required"` - Timeouts TimeoutsConfig `mapstructure:"timeouts" json:"timeouts" validate:"required"` - Port int `mapstructure:"port" json:"port" validate:"required,min=1,max=65535"` + Hostname string `mapstructure:"hostname" json:"hostname" validate:"omitempty,hostname|ip"` + Host string `mapstructure:"host" json:"host" validate:"required,hostname|ip"` + OpenAPISchemaPath string `mapstructure:"openapi_schema_path" json:"openapi_schema_path"` + JWK JWKConfig `mapstructure:"jwk" json:"jwk" validate:"required"` + TLS TLSConfig `mapstructure:"tls" json:"tls" validate:"required"` + JWT JWTConfig `mapstructure:"jwt" json:"jwt" validate:"required"` + IdentityHeader IdentityHeaderConfig `mapstructure:"identity_header" json:"identity_header" validate:"required"` + Timeouts TimeoutsConfig `mapstructure:"timeouts" json:"timeouts" validate:"required"` + Port int `mapstructure:"port" json:"port" validate:"required,min=1,max=65535"` } // TimeoutsConfig holds HTTP timeout configuration @@ -61,9 +64,10 @@ func (c *TLSConfig) Validate() error { // JWTConfig holds JWT authentication configuration type JWTConfig struct { - IssuerURL string `mapstructure:"issuer_url" json:"issuer_url" validate:"omitempty,url"` - Audience string `mapstructure:"audience" json:"audience"` - Enabled bool `mapstructure:"enabled" json:"enabled"` + IssuerURL string `mapstructure:"issuer_url" json:"issuer_url" validate:"omitempty,url"` + Audience string `mapstructure:"audience" json:"audience"` + IdentityClaim string `mapstructure:"identity_claim" json:"identity_claim"` + Enabled bool `mapstructure:"enabled" json:"enabled"` } func (c *JWTConfig) Validate() error { @@ -73,6 +77,28 @@ func (c *JWTConfig) Validate() error { if c.IssuerURL == "" { return fmt.Errorf("server.jwt.issuer_url is required when jwt is enabled") } + if c.IdentityClaim == "" { + return fmt.Errorf("server.jwt.identity_claim is required when jwt is enabled") + } + return nil +} + +// IdentityHeaderConfig holds HTTP header-based caller identity configuration. +type IdentityHeaderConfig struct { + Name string `mapstructure:"name" json:"name"` + Enabled bool `mapstructure:"enabled" json:"enabled"` +} + +func (c *IdentityHeaderConfig) Validate() error { + if !c.Enabled { + return nil + } + if c.Name == "" { + return fmt.Errorf("server.identity_header.name is required when identity_header is enabled") + } + if validation.IsForbiddenIdentityHeaderName(c.Name) { + return fmt.Errorf("server.identity_header.name %q is not allowed", c.Name) + } return nil } @@ -100,9 +126,14 @@ func NewServerConfig() *ServerConfig { KeyFile: "", }, JWT: JWTConfig{ - Enabled: true, - IssuerURL: "", - Audience: "", + Enabled: true, + IssuerURL: "", + Audience: "", + IdentityClaim: "email", + }, + IdentityHeader: IdentityHeaderConfig{ + Enabled: false, + Name: "X-HyperFleet-Identity", }, JWK: JWKConfig{ CertFile: "", diff --git a/pkg/config/server_test.go b/pkg/config/server_test.go index dd7eb521..1fb61eb4 100644 --- a/pkg/config/server_test.go +++ b/pkg/config/server_test.go @@ -25,7 +25,51 @@ func TestJWTConfig_Validate(t *testing.T) { t.Run("enabled JWT with issuer URL passes", func(t *testing.T) { RegisterTestingT(t) - cfg := JWTConfig{Enabled: true, IssuerURL: "https://sso.example.com/auth/realms/test"} + cfg := JWTConfig{ + Enabled: true, + IssuerURL: "https://sso.example.com/auth/realms/test", + IdentityClaim: "email", + } + Expect(cfg.Validate()).To(Succeed()) + }) + + t.Run("enabled JWT without identity claim fails", func(t *testing.T) { + RegisterTestingT(t) + cfg := JWTConfig{Enabled: true, IssuerURL: "https://sso.example.com/auth/realms/test", IdentityClaim: ""} + err := cfg.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("identity_claim")) + }) +} + +func TestIdentityHeaderConfig_Validate(t *testing.T) { + RegisterTestingT(t) + + t.Run("disabled identity header requires nothing", func(t *testing.T) { + RegisterTestingT(t) + cfg := IdentityHeaderConfig{Enabled: false} + Expect(cfg.Validate()).To(Succeed()) + }) + + t.Run("enabled identity header without name fails", func(t *testing.T) { + RegisterTestingT(t) + cfg := IdentityHeaderConfig{Enabled: true, Name: ""} + err := cfg.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("identity_header.name")) + }) + + t.Run("forbidden header name fails", func(t *testing.T) { + RegisterTestingT(t) + cfg := IdentityHeaderConfig{Enabled: true, Name: "Authorization"} + err := cfg.Validate() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not allowed")) + }) + + t.Run("enabled identity header with valid name passes", func(t *testing.T) { + RegisterTestingT(t) + cfg := IdentityHeaderConfig{Enabled: true, Name: "X-HyperFleet-Identity"} Expect(cfg.Validate()).To(Succeed()) }) } diff --git a/pkg/services/cluster.go b/pkg/services/cluster.go index d3a0f4e0..a43c8f54 100644 --- a/pkg/services/cluster.go +++ b/pkg/services/cluster.go @@ -71,10 +71,10 @@ func (s *sqlClusterService) Get(ctx context.Context, id string) (*api.Cluster, * func (s *sqlClusterService) Create(ctx context.Context, cluster *api.Cluster) (*api.Cluster, *errors.ServiceError) { if cluster.CreatedBy == "" { - cluster.CreatedBy = defaultSystemUser + cluster.CreatedBy = actorFromContext(ctx) } if cluster.UpdatedBy == "" { - cluster.UpdatedBy = defaultSystemUser + cluster.UpdatedBy = actorFromContext(ctx) } if cluster.Generation == 0 { cluster.Generation = 1 diff --git a/pkg/services/node_pool.go b/pkg/services/node_pool.go index a2d77c3c..5e8bc98a 100644 --- a/pkg/services/node_pool.go +++ b/pkg/services/node_pool.go @@ -81,10 +81,10 @@ func (s *sqlNodePoolService) Get(ctx context.Context, id string) (*api.NodePool, func (s *sqlNodePoolService) Create(ctx context.Context, nodePool *api.NodePool) (*api.NodePool, *errors.ServiceError) { if nodePool.CreatedBy == "" { - nodePool.CreatedBy = defaultSystemUser + nodePool.CreatedBy = actorFromContext(ctx) } if nodePool.UpdatedBy == "" { - nodePool.UpdatedBy = defaultSystemUser + nodePool.UpdatedBy = actorFromContext(ctx) } if nodePool.Generation == 0 { nodePool.Generation = 1 diff --git a/pkg/services/util.go b/pkg/services/util.go index 88281495..02194cbc 100755 --- a/pkg/services/util.go +++ b/pkg/services/util.go @@ -10,6 +10,7 @@ import ( "gorm.io/gorm" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/db" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" @@ -113,3 +114,10 @@ func buildAdapterSummaries(ctx context.Context, statuses api.AdapterStatusList) } return summaries } + +func actorFromContext(ctx context.Context) string { + if caller := auth.GetUsernameFromContext(ctx); caller != "" { + return caller + } + return defaultSystemUser +} diff --git a/pkg/validation/identity_header.go b/pkg/validation/identity_header.go new file mode 100644 index 00000000..b077f5f3 --- /dev/null +++ b/pkg/validation/identity_header.go @@ -0,0 +1,19 @@ +package validation + +import "net/http" + +var forbiddenIdentityHeaderNames = map[string]struct{}{ + "Authorization": {}, + "Cookie": {}, + "Set-Cookie": {}, + "X-Api-Key": {}, + "X-Auth-Token": {}, + "X-Forwarded-Authorization": {}, + "Proxy-Authorization": {}, +} + +// IsForbiddenIdentityHeaderName reports whether name must not be used as the caller identity header. +func IsForbiddenIdentityHeaderName(name string) bool { + _, ok := forbiddenIdentityHeaderNames[http.CanonicalHeaderKey(name)] + return ok +} diff --git a/pkg/validation/identity_header_test.go b/pkg/validation/identity_header_test.go new file mode 100644 index 00000000..cb9e0167 --- /dev/null +++ b/pkg/validation/identity_header_test.go @@ -0,0 +1,13 @@ +package validation + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestIsForbiddenIdentityHeaderName(t *testing.T) { + RegisterTestingT(t) + Expect(IsForbiddenIdentityHeaderName("Authorization")).To(BeTrue()) + Expect(IsForbiddenIdentityHeaderName("X-HyperFleet-Identity")).To(BeFalse()) +} diff --git a/plugins/clusters/plugin.go b/plugins/clusters/plugin.go index 2375d048..08bdf411 100644 --- a/plugins/clusters/plugin.go +++ b/plugins/clusters/plugin.go @@ -9,7 +9,6 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/server" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/presenters" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/dao" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" @@ -52,7 +51,7 @@ func init() { }) // Routes registration - server.RegisterRoutes("clusters", func(apiV1Router *mux.Router, services server.ServicesInterface, authMiddleware auth.JWTMiddleware) { + server.RegisterRoutes("clusters", func(apiV1Router *mux.Router, services server.ServicesInterface) { envServices := services.(*environments.Services) clusterHandler := handlers.NewClusterHandler(Service(envServices), generic.Service(envServices)) @@ -85,8 +84,6 @@ func init() { nodepoolStatusHandler := handlers.NewNodePoolStatusHandler(adapterStatus.Service(envServices), nodePools.Service(envServices)) clustersRouter.HandleFunc("/{id}/nodepools/{nodepool_id}/statuses", nodepoolStatusHandler.List).Methods(http.MethodGet) clustersRouter.HandleFunc("/{id}/nodepools/{nodepool_id}/statuses", nodepoolStatusHandler.Create).Methods(http.MethodPut) - - clustersRouter.Use(authMiddleware.AuthenticateAccountJWT) }) // REMOVED: Controller registration - Sentinel handles orchestration diff --git a/plugins/nodePools/plugin.go b/plugins/nodePools/plugin.go index 1d73af97..ca122375 100644 --- a/plugins/nodePools/plugin.go +++ b/plugins/nodePools/plugin.go @@ -9,7 +9,6 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/server" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/presenters" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/dao" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" @@ -50,7 +49,7 @@ func init() { }) // Routes registration - server.RegisterRoutes("nodePools", func(apiV1Router *mux.Router, services server.ServicesInterface, authMiddleware auth.JWTMiddleware) { + server.RegisterRoutes("nodePools", func(apiV1Router *mux.Router, services server.ServicesInterface) { envServices := services.(*environments.Services) nodePoolHandler := handlers.NewNodePoolHandler(Service(envServices), generic.Service(envServices)) @@ -58,8 +57,6 @@ func init() { // GET /api/hyperfleet/v1/nodepools - List all nodepools nodePoolsRouter := apiV1Router.PathPrefix("/nodepools").Subrouter() nodePoolsRouter.HandleFunc("", nodePoolHandler.List).Methods(http.MethodGet) - - nodePoolsRouter.Use(authMiddleware.AuthenticateAccountJWT) }) // REMOVED: Controller registration - Sentinel handles orchestration diff --git a/test/helper.go b/test/helper.go index 5aabea99..32c8b8ae 100755 --- a/test/helper.go +++ b/test/helper.go @@ -369,6 +369,21 @@ func WithAuthToken(ctx context.Context) openapi.RequestEditorFn { } } +// WithIdentityHeader returns a RequestEditorFn that sets the caller identity header. +func WithIdentityHeader(headerName, headerValue string) openapi.RequestEditorFn { + return func(_ context.Context, req *http.Request) error { + if headerName != "" && headerValue != "" { + req.Header.Set(headerName, headerValue) + } + return nil + } +} + +// IdentityHeaderName returns the configured identity header name for integration tests. +func IdentityHeaderName() string { + return environments.Environment().Config.Server.IdentityHeader.Name +} + func (helper *Helper) StartJWKCertServerMock() (teardown func() error) { jwkURL, teardown = mocks.NewJWKCertServerMock(helper.T, helper.JWTCA, jwkKID, jwkAlg) helper.Env().Config.Server.JWK.CertURL = jwkURL diff --git a/test/integration/caller_identity_test.go b/test/integration/caller_identity_test.go new file mode 100644 index 00000000..fbeb1c68 --- /dev/null +++ b/test/integration/caller_identity_test.go @@ -0,0 +1,80 @@ +package integration + +import ( + "fmt" + "net/http" + "strings" + "testing" + + . "github.com/onsi/gomega" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/util" + "github.com/openshift-hyperfleet/hyperfleet-api/test" +) + +func shortClusterName(prefix, id string) string { + suffix := strings.ReplaceAll(id, "-", "") + if len(suffix) > 12 { + suffix = suffix[:12] + } + return fmt.Sprintf("%s-%s", prefix, suffix) +} + +func TestCallerIdentityCreate(t *testing.T) { + cases := []struct { + name string + namePrefix string + email string + setHeader bool + headerActor string + }{ + { + name: "header present overrides JWT", + namePrefix: "ci-hdr", + email: "jwt-user@example.com", + setHeader: true, + headerActor: "gateway-user@example.com", + }, + { + name: "header absent uses JWT claim", + namePrefix: "ci-jwt", + email: "jwt-only-user@example.com", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h, client := test.RegisterIntegration(t) + + account := h.NewAccount(h.NewID(), "Test User", tc.email) + ctx := h.NewAuthenticatedContext(account) + + wantCreatedBy := account.Email + if tc.setHeader { + wantCreatedBy = tc.headerActor + } + + clusterInput := openapi.ClusterCreateRequest{ + Kind: util.PtrString("Cluster"), + Name: shortClusterName(tc.namePrefix, h.NewID()), + Spec: map[string]interface{}{"test": "spec"}, + } + + opts := []openapi.RequestEditorFn{test.WithAuthToken(ctx)} + if tc.setHeader { + opts = append(opts, test.WithIdentityHeader(test.IdentityHeaderName(), tc.headerActor)) + } + + resp, err := client.PostClusterWithResponse( + ctx, + openapi.PostClusterJSONRequestBody(clusterInput), + opts..., + ) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.StatusCode()).To(Equal(http.StatusCreated)) + Expect(resp.JSON201).NotTo(BeNil()) + Expect(string(resp.JSON201.CreatedBy)).To(Equal(wantCreatedBy)) + }) + } +} diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index d14578a6..e6042643 100755 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -34,6 +34,12 @@ func TestMain(m *testing.M) { if os.Getenv("HYPERFLEET_SERVER_JWK_CERT_URL") == "" { _ = os.Setenv("HYPERFLEET_SERVER_JWK_CERT_URL", "https://test-idp.example.com/certs") } + if os.Getenv("HYPERFLEET_SERVER_IDENTITY_HEADER_ENABLED") == "" { + _ = os.Setenv("HYPERFLEET_SERVER_IDENTITY_HEADER_ENABLED", "true") + } + if os.Getenv("HYPERFLEET_SERVER_IDENTITY_HEADER_NAME") == "" { + _ = os.Setenv("HYPERFLEET_SERVER_IDENTITY_HEADER_NAME", "X-HyperFleet-Identity") + } // Set OpenAPI schema path for integration tests if not already set // This enables schema validation middleware during tests From 71ece57ed2a6575bedf7c6a608ae468a7b77573e Mon Sep 17 00:00:00 2001 From: Angel Marin Date: Fri, 29 May 2026 14:49:09 +0200 Subject: [PATCH 2/3] HYPERFLEET-1134 - chore: reorder test struct fields for alignment --- pkg/auth/context_test.go | 4 +- pkg/auth/identity_test.go | 6 +- pkg/services/cluster.go | 1 + pkg/services/node_pool.go | 1 + test/integration/caller_identity_test.go | 81 +++++++++++++++++++++++- 5 files changed, 87 insertions(+), 6 deletions(-) diff --git a/pkg/auth/context_test.go b/pkg/auth/context_test.go index 790aed47..0ea9a0da 100644 --- a/pkg/auth/context_test.go +++ b/pkg/auth/context_test.go @@ -10,12 +10,12 @@ import ( func TestGetIdentityFromContext(t *testing.T) { tests := []struct { - name string claims jwt.MapClaims + name string identityField string want string - wantErr bool errSubstring string + wantErr bool }{ { name: "reads configured claim directly", diff --git a/pkg/auth/identity_test.go b/pkg/auth/identity_test.go index 9d314037..f1ce94a8 100644 --- a/pkg/auth/identity_test.go +++ b/pkg/auth/identity_test.go @@ -12,12 +12,12 @@ import ( func TestCallerIdentityFromRequest(t *testing.T) { tests := []struct { - name string claims jwt.MapClaims - setHeader bool + name string headerValue string - cfg CallerIdentityConfig want string + cfg CallerIdentityConfig + setHeader bool wantErr bool }{ { diff --git a/pkg/services/cluster.go b/pkg/services/cluster.go index a43c8f54..28577a84 100644 --- a/pkg/services/cluster.go +++ b/pkg/services/cluster.go @@ -117,6 +117,7 @@ func (s *sqlClusterService) Patch( } cluster.IncrementGeneration() + cluster.UpdatedBy = actorFromContext(ctx) if saveErr := s.clusterDao.Save(ctx, cluster); saveErr != nil { return nil, handleUpdateError(api.ResourceTypeCluster, saveErr) diff --git a/pkg/services/node_pool.go b/pkg/services/node_pool.go index 5e8bc98a..ffcec684 100644 --- a/pkg/services/node_pool.go +++ b/pkg/services/node_pool.go @@ -127,6 +127,7 @@ func (s *sqlNodePoolService) Patch( } nodePool.IncrementGeneration() + nodePool.UpdatedBy = actorFromContext(ctx) if saveErr := s.nodePoolDao.Save(ctx, nodePool); saveErr != nil { return nil, handleUpdateError(api.ResourceTypeNodePool, saveErr) diff --git a/test/integration/caller_identity_test.go b/test/integration/caller_identity_test.go index fbeb1c68..8d09952e 100644 --- a/test/integration/caller_identity_test.go +++ b/test/integration/caller_identity_test.go @@ -26,8 +26,8 @@ func TestCallerIdentityCreate(t *testing.T) { name string namePrefix string email string - setHeader bool headerActor string + setHeader bool }{ { name: "header present overrides JWT", @@ -78,3 +78,82 @@ func TestCallerIdentityCreate(t *testing.T) { }) } } + +func TestCallerIdentityPatch(t *testing.T) { + cases := []struct { + name string + namePrefix string + createEmail string + patchEmail string + headerActor string + wantUpdatedBy string + setHeader bool + }{ + { + name: "patch with header updates updated_by", + namePrefix: "ci-patch-hdr", + createEmail: "creator@example.com", + patchEmail: "creator@example.com", + setHeader: true, + headerActor: "updater@example.com", + wantUpdatedBy: "updater@example.com", + }, + { + name: "patch without header uses JWT identity", + namePrefix: "ci-patch-jwt", + createEmail: "creator@example.com", + patchEmail: "patcher@example.com", + wantUpdatedBy: "patcher@example.com", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h, client := test.RegisterIntegration(t) + + // Create cluster with one identity. + createAccount := h.NewAccount(h.NewID(), "Creator", tc.createEmail) + createCtx := h.NewAuthenticatedContext(createAccount) + + clusterInput := openapi.ClusterCreateRequest{ + Kind: util.PtrString("Cluster"), + Name: shortClusterName(tc.namePrefix, h.NewID()), + Spec: map[string]interface{}{"test": "spec"}, + } + + createResp, err := client.PostClusterWithResponse( + createCtx, + openapi.PostClusterJSONRequestBody(clusterInput), + test.WithAuthToken(createCtx), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(createResp.StatusCode()).To(Equal(http.StatusCreated)) + clusterID := createResp.JSON201.Id + + // Patch cluster with a different identity. + patchAccount := h.NewAccount(h.NewID(), "Patcher", tc.patchEmail) + patchCtx := h.NewAuthenticatedContext(patchAccount) + + newSpec := openapi.ClusterSpec{"test": "patched"} + patchBody := openapi.PatchClusterByIdJSONRequestBody{Spec: &newSpec} + + opts := []openapi.RequestEditorFn{test.WithAuthToken(patchCtx)} + if tc.setHeader { + opts = append(opts, test.WithIdentityHeader(test.IdentityHeaderName(), tc.headerActor)) + } + + patchResp, err := client.PatchClusterByIdWithResponse( + patchCtx, + *clusterID, + patchBody, + opts..., + ) + Expect(err).NotTo(HaveOccurred()) + Expect(patchResp.StatusCode()).To(Equal(http.StatusOK)) + Expect(patchResp.JSON200).NotTo(BeNil()) + Expect(string(patchResp.JSON200.UpdatedBy)).To(Equal(tc.wantUpdatedBy)) + // created_by must remain unchanged. + Expect(string(patchResp.JSON200.CreatedBy)).To(Equal(tc.createEmail)) + }) + } +} From cd6a57bd0f624398d12c16c84b50e445c7aa0887 Mon Sep 17 00:00:00 2001 From: Angel Marin Date: Mon, 1 Jun 2026 12:41:28 +0200 Subject: [PATCH 3/3] HYPERFLEET-1134 - feat: identity mandatory if config set --- charts/templates/NOTES.txt | 21 +- charts/templates/configmap.yaml | 4 +- charts/values.yaml | 4 +- .../environments/e_integration_testing.go | 5 - cmd/hyperfleet-api/environments/types.go | 5 +- cmd/hyperfleet-api/server/routes.go | 15 +- configs/config.yaml.example | 5 +- docs/authentication.md | 119 ++++++++- docs/config.md | 9 +- pkg/auth/auth_middleware.go | 26 +- pkg/auth/identity.go | 35 ++- pkg/auth/identity_test.go | 252 ++++++++++++++++-- pkg/config/flags.go | 11 +- pkg/config/loader.go | 8 +- pkg/config/logging.go | 1 - pkg/config/server.go | 40 +-- pkg/config/server_test.go | 26 +- pkg/services/cluster.go | 4 +- pkg/services/node_pool.go | 2 +- plugins/channels/plugin.go | 9 +- plugins/versions/plugin.go | 9 +- test/helper.go | 2 +- test/integration/caller_identity_test.go | 152 +++++++++++ test/integration/clusters_test.go | 4 +- test/integration/integration_test.go | 7 +- test/integration/node_pools_test.go | 2 +- 26 files changed, 585 insertions(+), 192 deletions(-) diff --git a/charts/templates/NOTES.txt b/charts/templates/NOTES.txt index f145d702..185568c7 100644 --- a/charts/templates/NOTES.txt +++ b/charts/templates/NOTES.txt @@ -18,14 +18,12 @@ Validation schema validation is ENABLED. {{- end }} Caller identity (audit attribution): - config.server.jwt.identity_claim: {{ .Values.config.server.jwt.identity_claim | default "email" | quote }} - config.server.identity_header.enabled: {{ .Values.config.server.identity_header.enabled }} (chart default: false) - config.server.identity_header.name: {{ .Values.config.server.identity_header.name | default "X-HyperFleet-Identity" | quote }} - -When identity_header.enabled is true and a request includes a non-empty -{{ .Values.config.server.identity_header.name | default "X-HyperFleet-Identity" }} header, -that value overrides the JWT claim for audit fields (created_by, updated_by). -JWT authentication behavior is unchanged when config.server.jwt.enabled is true. + config.server.jwt.identity_claim: {{ .Values.config.server.jwt.identity_claim | default "" | quote }} + config.server.identity_header: {{ .Values.config.server.identity_header | default "" | quote }} + +When identity_header is set and a request includes a non-empty header with that name, +the header value overrides the JWT claim for audit fields (created_by, updated_by). +When identity_claim is set, the named JWT claim is used as the caller identity. Only trusted gateways should set the identity header in production. Override in values.yaml: @@ -33,14 +31,11 @@ Override in values.yaml: server: jwt: identity_claim: preferred_username - identity_header: - enabled: true - name: X-HyperFleet-Identity + identity_header: X-HyperFleet-Identity Or at install/upgrade: --set config.server.jwt.identity_claim=preferred_username - --set config.server.identity_header.enabled=true - --set config.server.identity_header.name=X-HyperFleet-Identity + --set config.server.identity_header=X-HyperFleet-Identity Documentation: https://github.com/openshift-hyperfleet/hyperfleet-api/blob/main/docs/deployment.md diff --git a/charts/templates/configmap.yaml b/charts/templates/configmap.yaml index d1767371..7b18ce13 100644 --- a/charts/templates/configmap.yaml +++ b/charts/templates/configmap.yaml @@ -32,9 +32,7 @@ data: audience: {{ .Values.config.server.jwt.audience | quote }} identity_claim: {{ .Values.config.server.jwt.identity_claim | quote }} - identity_header: - enabled: {{ .Values.config.server.identity_header.enabled }} - name: {{ .Values.config.server.identity_header.name | quote }} + identity_header: {{ .Values.config.server.identity_header | quote }} jwk: cert_file: {{ .Values.config.server.jwk.cert_file | quote }} diff --git a/charts/values.yaml b/charts/values.yaml index a6283e0d..755a4862 100644 --- a/charts/values.yaml +++ b/charts/values.yaml @@ -54,9 +54,7 @@ config: audience: "" identity_claim: email - identity_header: - enabled: false - name: X-HyperFleet-Identity + identity_header: "" jwk: cert_file: "" diff --git a/cmd/hyperfleet-api/environments/e_integration_testing.go b/cmd/hyperfleet-api/environments/e_integration_testing.go index 7afc3c84..765c41d0 100755 --- a/cmd/hyperfleet-api/environments/e_integration_testing.go +++ b/cmd/hyperfleet-api/environments/e_integration_testing.go @@ -38,11 +38,6 @@ func (e *integrationTestingEnvImpl) OverrideConfig(c *config.ApplicationConfig) c.Database.SSL.Mode = SSLModeDisable } - // Avoid clashing with a local dev server on default ports - c.Server.Port = 8777 - c.Metrics.Port = 19090 - c.Health.Port = 18080 - return nil } diff --git a/cmd/hyperfleet-api/environments/types.go b/cmd/hyperfleet-api/environments/types.go index 2e26e154..35e71396 100755 --- a/cmd/hyperfleet-api/environments/types.go +++ b/cmd/hyperfleet-api/environments/types.go @@ -3,7 +3,6 @@ package environments import ( "sync" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/config" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/db" ) @@ -37,9 +36,7 @@ type Database struct { SessionFactory db.SessionFactory } -type Handlers struct { - CallerIdentityMiddleware auth.CallerIdentityMiddleware -} +type Handlers struct{} type Services struct { serviceRegistry map[string]interface{} diff --git a/cmd/hyperfleet-api/server/routes.go b/cmd/hyperfleet-api/server/routes.go index c4c5117e..7f961e5a 100755 --- a/cmd/hyperfleet-api/server/routes.go +++ b/cmd/hyperfleet-api/server/routes.go @@ -86,13 +86,14 @@ func (s *apiServer) routes(tracingEnabled bool) *mux.Router { err = registerAPIMiddleware(apiV1Router) check(err, "Failed to initialize API middleware") - if env().Config.Server.JWT.Enabled || env().Config.Server.IdentityHeader.Enabled { - callerIdentityMW, mwErr := auth.NewCallerIdentityMiddleware(auth.CallerIdentityConfig{ - JWTEnabled: env().Config.Server.JWT.Enabled, - JWTIdentityClaim: env().Config.Server.JWT.IdentityClaim, - HeaderEnabled: env().Config.Server.IdentityHeader.Enabled, - HeaderName: env().Config.Server.IdentityHeader.Name, - }) + identityCfg := auth.CallerIdentityConfig{ + HeaderName: env().Config.Server.IdentityHeader, + } + if env().Config.Server.JWT.Enabled && env().Config.Server.JWT.IdentityClaim != "" { + identityCfg.JWTIdentityClaim = env().Config.Server.JWT.IdentityClaim + } + if identityCfg.JWTIdentityClaim != "" || identityCfg.HeaderName != "" { + callerIdentityMW, mwErr := auth.NewCallerIdentityMiddleware(identityCfg) check(mwErr, "Unable to create caller identity middleware") apiV1Router.Use(callerIdentityMW.ResolveCallerIdentity) } diff --git a/configs/config.yaml.example b/configs/config.yaml.example index af30027c..a1266703 100644 --- a/configs/config.yaml.example +++ b/configs/config.yaml.example @@ -23,9 +23,7 @@ server: audience: "" # JWT audience claim (optional) identity_claim: email # JWT claim used as request identity for audit (e.g. email, preferred_username, sub) - identity_header: - enabled: false # Use HTTP header as caller identity (overrides JWT claim when set) - name: X-HyperFleet-Identity # Header name (must be set by a trusted gateway in production) + identity_header: "" # HTTP header name for caller identity; leave empty to disable (e.g. X-HyperFleet-Identity) jwk: cert_file: "" # JWK certificate file path @@ -72,7 +70,6 @@ logging: - Cookie - X-Auth-Token - X-Forwarded-Authorization - - X-HyperFleet-Identity fields: # Sensitive JSON fields to mask - password diff --git a/docs/authentication.md b/docs/authentication.md index 8956b2ef..6fbc50d5 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -4,10 +4,11 @@ This document describes authentication mechanisms for the HyperFleet API. ## Overview -HyperFleet API supports two authentication modes: +HyperFleet API supports the following authentication modes: -1. **Development Mode (No Auth)**: For local development and testing -2. **Production Mode (JWT Auth)**: JWT-based authentication with configurable issuer +1. **Development Mode (No Auth)**: For local development and testing without authentication +2. **Development with JWT (Google Cloud)**: Local development with real JWT validation using Google identity tokens +3. **Production Mode (JWT Auth)**: JWT-based authentication with configurable issuer ## Development Mode (No Auth) @@ -30,8 +31,99 @@ export HYPERFLEET_SERVER_JWT_ENABLED=false ./bin/hyperfleet-api serve ``` +### Caller identity in development mode + +When JWT is disabled and no `identity_header` is configured, caller identity resolution is inactive. Audit fields (`created_by`, `updated_by`, `deleted_by`) fall back to `system@hyperfleet.local`. + +To get proper caller attribution without JWT, configure an identity header: + +```bash +./bin/hyperfleet-api serve \ + --server-jwt-enabled=false \ + --server-identity-header=X-HyperFleet-Identity +``` + +Then pass the header in requests: + +```bash +# Create with attribution +curl -X POST http://localhost:8000/api/hyperfleet/v1/clusters \ + -H "Content-Type: application/json" \ + -H "X-HyperFleet-Identity: dev-user@local" \ + -d '{"kind":"Cluster","name":"my-cluster","spec":{}}' + +# Read requests work without the header +curl http://localhost:8000/api/hyperfleet/v1/clusters | jq +``` + +When `identity_header` or `identity_claim` is configured, mutating requests (POST, PATCH, PUT, DELETE) that cannot resolve a caller identity are rejected with `401 Unauthorized`. Read requests (GET, LIST) are allowed without identity. + **Important**: Never disable authentication in production environments. +## Development with JWT (Google Cloud) + +For local development with real JWT validation, you can use Google Cloud identity tokens. This gives you proper authentication and caller identity without deploying a dedicated identity provider. + +### Prerequisites + +- [Google Cloud SDK](https://cloud.google.com/sdk/docs/install) installed +- Authenticated with `gcloud auth login` + +### Start the server + +```bash +./bin/hyperfleet-api serve \ + --server-jwt-enabled=true \ + --server-jwt-issuer-url="https://accounts.google.com" \ + --server-jwk-cert-url="https://www.googleapis.com/oauth2/v3/certs" \ + --server-jwt-audience="32555940559.apps.googleusercontent.com" \ + --server-jwt-identity-claim="email" \ + --server-identity-header=X-HyperFleet-Identity \ + --db-host localhost --db-port 5432 --db-name hyperfleet --db-username hyperfleet +``` + +The audience `32555940559.apps.googleusercontent.com` is the default gcloud CLI OAuth client ID. It matches the `aud` claim in tokens generated by `gcloud auth print-identity-token`. + +### Generate a token and make requests + +```bash +# Generate an identity token (valid for ~1 hour) +TOKEN=$(gcloud auth print-identity-token) + +# List clusters +curl -H "Authorization: Bearer $TOKEN" \ + http://localhost:8000/api/hyperfleet/v1/clusters | jq + +# Create a cluster (created_by will be your Google email) +curl -X POST http://localhost:8000/api/hyperfleet/v1/clusters \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{"kind":"Cluster","name":"my-cluster","spec":{}}' + +# Override identity via header (header takes precedence over JWT) +curl -X POST http://localhost:8000/api/hyperfleet/v1/clusters \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -H "X-HyperFleet-Identity: gateway-user@corp.com" \ + -d '{"kind":"Cluster","name":"my-cluster-2","spec":{}}' +``` + +### How it works + +Google identity tokens are standard OIDC JWTs signed by Google's keys. The server validates them like any other JWT: + +1. Fetches Google's public keys from the `jwk_cert_url` +2. Verifies the RS256 signature +3. Checks `iss` matches `https://accounts.google.com` +4. Checks `aud` matches the configured audience +5. Extracts the `email` claim for caller identity + +You can inspect your token with: + +```bash +gcloud auth print-identity-token | cut -d. -f2 | base64 -d 2>/dev/null | jq +``` + ## Production Mode (JWT Auth) Production deployments use JWT-based authentication with a configurable issuer. @@ -71,16 +163,20 @@ curl -H "Authorization: Bearer ${TOKEN}" \ ## Caller identity for audit -Authentication (JWT validation) and caller identity (audit attribution) are separate: +Authentication (JWT validation) and caller identity (audit attribution) are separate concerns. Identity resolution is enabled by setting `identity_claim` (in the JWT config) and/or `identity_header`. When neither is set, no identity middleware is registered and audit fields fall back to `system@hyperfleet.local`. | Layer | Component | Responsibility | |-------|-----------|----------------| | Outer | `JWTHandler` | Validates `Authorization: Bearer` token | -| Inner | `ResolveCallerIdentity` middleware | Resolves who is recorded as the actor (`CreatedBy`, force-delete logs) | +| Inner | `ResolveCallerIdentity` middleware | Resolves who is recorded as the actor | + +The resolved identity is written to `created_by` on create, `updated_by` on update, and `deleted_by` on delete. Precedence: identity header > JWT claim. + +When identity resolution is configured, mutating requests (POST, PATCH, PUT, DELETE) that cannot resolve a caller identity are rejected with `401 Unauthorized`. Read requests (GET, LIST) are allowed without identity. ### JWT claim -Configure which JWT claim is used when no identity header is present: +Configure which JWT claim is used as the caller identity: ```yaml server: @@ -90,22 +186,21 @@ server: ### HTTP identity header (optional) -When enabled, a trusted gateway can set the caller identity via HTTP header. **If the header is present and non-empty, it overrides the JWT claim** for audit fields. JWT validation is still required when `jwt.enabled=true`. +When set, a trusted gateway can set the caller identity via HTTP header. **If the header is present and non-empty, it overrides the JWT claim** for audit fields. JWT validation is still required when `jwt.enabled=true`. ```yaml server: - identity_header: - enabled: true - name: X-HyperFleet-Identity + identity_header: X-HyperFleet-Identity ``` **Security:** Clients must not be able to set this header directly. Configure your ingress/gateway to strip the header from external requests and set it from the authenticated upstream user. ```bash -export HYPERFLEET_SERVER_IDENTITY_HEADER_ENABLED=true -export HYPERFLEET_SERVER_IDENTITY_HEADER_NAME=X-HyperFleet-Identity +export HYPERFLEET_SERVER_IDENTITY_HEADER=X-HyperFleet-Identity ``` +Identity values from both sources are validated: trimmed of whitespace, limited to 256 characters, and rejected if they contain control characters. + ## Configuration ### Environment Variables diff --git a/docs/config.md b/docs/config.md index 5be9871c..88b5416d 100644 --- a/docs/config.md +++ b/docs/config.md @@ -258,8 +258,7 @@ HTTP server settings for the API endpoint. | `server.jwt.issuer_url` | string | `""` | Expected JWT issuer URL for token validation (required when JWT is enabled) | | `server.jwt.audience` | string | `""` | Expected JWT audience claim (optional) | | `server.jwt.identity_claim` | string | `email` | JWT claim used as request identity for audit (e.g. `email`, `preferred_username`, `sub`) | -| `server.identity_header.enabled` | bool | `false` | Enable HTTP header as caller identity source for audit | -| `server.identity_header.name` | string | `X-HyperFleet-Identity` | Header name; when set and non-empty, overrides JWT claim for audit attribution | +| `server.identity_header` | string | `""` | HTTP header name for caller identity; when set and non-empty, overrides JWT claim for audit attribution | | `server.jwk.cert_file` | string | `""` | JWK certificate file path (optional) | | `server.jwk.cert_url` | string | `""` | JWK certificate URL (required when JWT is enabled and cert_file is not set) | @@ -355,8 +354,7 @@ Complete table of all configuration properties, their environment variables, and | `server.jwt.issuer_url` | `HYPERFLEET_SERVER_JWT_ISSUER_URL` | string | `""` | | `server.jwt.audience` | `HYPERFLEET_SERVER_JWT_AUDIENCE` | string | `""` | | `server.jwt.identity_claim` | `HYPERFLEET_SERVER_JWT_IDENTITY_CLAIM` | string | `email` | -| `server.identity_header.enabled` | `HYPERFLEET_SERVER_IDENTITY_HEADER_ENABLED` | bool | `false` | -| `server.identity_header.name` | `HYPERFLEET_SERVER_IDENTITY_HEADER_NAME` | string | `X-HyperFleet-Identity` | +| `server.identity_header` | `HYPERFLEET_SERVER_IDENTITY_HEADER` | string | `""` | | `server.jwk.cert_file` | `HYPERFLEET_SERVER_JWK_CERT_FILE` | string | `""` | | `server.jwk.cert_url` | `HYPERFLEET_SERVER_JWK_CERT_URL` | string | `""` | | **Database** | | | | @@ -420,8 +418,7 @@ All CLI flags and their corresponding configuration paths. | `--server-jwt-issuer-url` | `server.jwt.issuer_url` | string | | `--server-jwt-audience` | `server.jwt.audience` | string | | `--server-jwt-identity-claim` | `server.jwt.identity_claim` | string | -| `--server-identity-header-enabled` | `server.identity_header.enabled` | bool | -| `--server-identity-header-name` | `server.identity_header.name` | string | +| `--server-identity-header` | `server.identity_header` | string | | `--server-jwk-cert-file` | `server.jwk.cert_file` | string | | `--server-jwk-cert-url` | `server.jwk.cert_url` | string | | **Database** | | | diff --git a/pkg/auth/auth_middleware.go b/pkg/auth/auth_middleware.go index 39335805..0e2d2338 100755 --- a/pkg/auth/auth_middleware.go +++ b/pkg/auth/auth_middleware.go @@ -20,13 +20,7 @@ type callerIdentityMiddleware struct { var _ CallerIdentityMiddleware = &callerIdentityMiddleware{} func NewCallerIdentityMiddleware(cfg CallerIdentityConfig) (CallerIdentityMiddleware, error) { - if cfg.JWTIdentityClaim == "" { - cfg.JWTIdentityClaim = DefaultJWTIdentityClaim - } - if cfg.HeaderEnabled { - if cfg.HeaderName == "" { - return nil, fmt.Errorf("identity header name is required when identity header is enabled") - } + if cfg.HeaderName != "" { if validation.IsForbiddenIdentityHeaderName(cfg.HeaderName) { return nil, fmt.Errorf("identity header name %q is not allowed", cfg.HeaderName) } @@ -45,13 +39,6 @@ func (m *callerIdentityMiddleware) ResolveCallerIdentity(next http.Handler) http ctx := r.Context() identity, err := CallerIdentityFromRequest(ctx, r, m.cfg) - if err != nil { - handleError( - ctx, w, r, errors.CodeAuthNoCredentials, - fmt.Sprintf("Unable to resolve caller identity: %s", err), - ) - return - } if identity != "" { ctx = SetUsernameContext(ctx, identity) @@ -60,11 +47,12 @@ func (m *callerIdentityMiddleware) ResolveCallerIdentity(next http.Handler) http return } - if m.cfg.JWTEnabled { - handleError( - ctx, w, r, errors.CodeAuthNoCredentials, - "Unable to resolve caller identity from JWT token or identity header", - ) + if isMutatingMethod(r.Method) { + msg := "Caller identity is required for mutating requests but could not be resolved" + if err != nil { + msg = fmt.Sprintf("Unable to resolve caller identity: %s", err) + } + handleError(ctx, w, r, errors.CodeAuthNoCredentials, msg) return } diff --git a/pkg/auth/identity.go b/pkg/auth/identity.go index 1a29be5c..d5a17140 100644 --- a/pkg/auth/identity.go +++ b/pkg/auth/identity.go @@ -10,20 +10,23 @@ import ( const maxCallerIdentityLen = 256 // CallerIdentityConfig controls how the caller identity is resolved for audit fields. +// Identity resolution is enabled by setting the relevant fields: +// - HeaderName: when non-empty, the named HTTP header is checked first +// - JWTIdentityClaim: when non-empty, the JWT claim is used as fallback (or primary when no header is configured) type CallerIdentityConfig struct { JWTIdentityClaim string HeaderName string - JWTEnabled bool - HeaderEnabled bool } // CallerIdentityFromRequest resolves the caller identity with header-primary precedence. -// When the identity header is set and non-empty, it overrides the JWT claim. +// When the identity header is configured and present, it overrides the JWT claim. +// Both header and JWT identity values are normalized: trimmed, length-checked, and +// validated for control characters before being accepted. func CallerIdentityFromRequest(ctx context.Context, r *http.Request, cfg CallerIdentityConfig) (string, error) { - if cfg.HeaderEnabled { + if cfg.HeaderName != "" { raw := r.Header.Get(cfg.HeaderName) if raw != "" { - identity, err := normalizeIdentityHeaderValue(raw) + identity, err := normalizeIdentity(raw, "identity header") if err != nil { return "", err } @@ -33,24 +36,31 @@ func CallerIdentityFromRequest(ctx context.Context, r *http.Request, cfg CallerI } } - if cfg.JWTEnabled { - return GetIdentityFromContext(ctx, cfg.JWTIdentityClaim) + if cfg.JWTIdentityClaim != "" { + raw, err := GetIdentityFromContext(ctx, cfg.JWTIdentityClaim) + if err != nil { + return "", err + } + return normalizeIdentity(raw, fmt.Sprintf("JWT claim %q", cfg.JWTIdentityClaim)) } return "", nil } -func normalizeIdentityHeaderValue(raw string) (string, error) { +// normalizeIdentity trims, length-checks, and validates a caller identity value +// regardless of source (HTTP header or JWT claim). The source parameter is used +// in error messages to distinguish the origin. +func normalizeIdentity(raw string, source string) (string, error) { value := strings.TrimSpace(raw) if value == "" { return "", nil } if len(value) > maxCallerIdentityLen { - return "", fmt.Errorf("identity header value exceeds maximum length %d", maxCallerIdentityLen) + return "", fmt.Errorf("%s value exceeds maximum length %d", source, maxCallerIdentityLen) } for _, r := range value { if r < 0x20 || r == 0x7f { - return "", fmt.Errorf("identity header value contains invalid characters") + return "", fmt.Errorf("%s value contains invalid characters", source) } } return value, nil @@ -60,3 +70,8 @@ func shouldSkipCallerIdentity(path string) bool { return strings.HasPrefix(path, "/api/hyperfleet/v1/openapi") || strings.HasPrefix(path, "/api/hyperfleet/v1/errors") } + +func isMutatingMethod(method string) bool { + return method == http.MethodPost || method == http.MethodPatch || method == http.MethodDelete || + method == http.MethodPut +} diff --git a/pkg/auth/identity_test.go b/pkg/auth/identity_test.go index f1ce94a8..9d6c7a7c 100644 --- a/pkg/auth/identity_test.go +++ b/pkg/auth/identity_test.go @@ -26,9 +26,7 @@ func TestCallerIdentityFromRequest(t *testing.T) { setHeader: true, headerValue: "gateway-user@example.com", cfg: CallerIdentityConfig{ - JWTEnabled: true, JWTIdentityClaim: "email", - HeaderEnabled: true, HeaderName: "X-HyperFleet-Identity", }, want: "gateway-user@example.com", @@ -37,9 +35,7 @@ func TestCallerIdentityFromRequest(t *testing.T) { name: "falls back to JWT when header absent", claims: jwt.MapClaims{"email": "jwt@example.com"}, cfg: CallerIdentityConfig{ - JWTEnabled: true, JWTIdentityClaim: "email", - HeaderEnabled: true, HeaderName: "X-HyperFleet-Identity", }, want: "jwt@example.com", @@ -49,8 +45,7 @@ func TestCallerIdentityFromRequest(t *testing.T) { setHeader: true, headerValue: "bad\x00value", cfg: CallerIdentityConfig{ - HeaderEnabled: true, - HeaderName: "X-HyperFleet-Identity", + HeaderName: "X-HyperFleet-Identity", }, wantErr: true, }, @@ -59,22 +54,72 @@ func TestCallerIdentityFromRequest(t *testing.T) { setHeader: true, headerValue: strings.Repeat("a", maxCallerIdentityLen+1), cfg: CallerIdentityConfig{ - HeaderEnabled: true, - HeaderName: "X-HyperFleet-Identity", + HeaderName: "X-HyperFleet-Identity", }, wantErr: true, }, { - name: "header only when JWT disabled", + name: "header only without JWT claim", setHeader: true, headerValue: "dev-user", cfg: CallerIdentityConfig{ - JWTEnabled: false, - HeaderEnabled: true, - HeaderName: "X-HyperFleet-Identity", + HeaderName: "X-HyperFleet-Identity", }, want: "dev-user", }, + { + name: "no resolution when nothing configured", + claims: jwt.MapClaims{"email": "jwt@example.com"}, + cfg: CallerIdentityConfig{}, + want: "", + }, + { + name: "empty header falls back to JWT", + claims: jwt.MapClaims{"email": "jwt@example.com"}, + setHeader: true, + headerValue: "", + cfg: CallerIdentityConfig{ + JWTIdentityClaim: "email", + HeaderName: "X-HyperFleet-Identity", + }, + want: "jwt@example.com", + }, + { + name: "whitespace-only header falls back to JWT", + claims: jwt.MapClaims{"email": "jwt@example.com"}, + setHeader: true, + headerValue: " ", + cfg: CallerIdentityConfig{ + JWTIdentityClaim: "email", + HeaderName: "X-HyperFleet-Identity", + }, + want: "jwt@example.com", + }, + { + name: "header at exact max length accepted", + setHeader: true, + headerValue: strings.Repeat("a", maxCallerIdentityLen), + cfg: CallerIdentityConfig{ + HeaderName: "X-HyperFleet-Identity", + }, + want: strings.Repeat("a", maxCallerIdentityLen), + }, + { + name: "rejects oversized JWT claim value", + claims: jwt.MapClaims{"email": strings.Repeat("x", maxCallerIdentityLen+1)}, + cfg: CallerIdentityConfig{ + JWTIdentityClaim: "email", + }, + wantErr: true, + }, + { + name: "trims whitespace from JWT claim value", + claims: jwt.MapClaims{"email": " user@example.com "}, + cfg: CallerIdentityConfig{ + JWTIdentityClaim: "email", + }, + want: "user@example.com", + }, } for _, tc := range tests { @@ -106,11 +151,10 @@ func TestCallerIdentityFromRequest(t *testing.T) { func TestNewCallerIdentityMiddleware(t *testing.T) { RegisterTestingT(t) - t.Run("rejects forbidden header name when enabled", func(t *testing.T) { + t.Run("rejects forbidden header name", func(t *testing.T) { RegisterTestingT(t) _, err := NewCallerIdentityMiddleware(CallerIdentityConfig{ - HeaderEnabled: true, - HeaderName: "Authorization", + HeaderName: "Authorization", }) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("not allowed")) @@ -119,12 +163,18 @@ func TestNewCallerIdentityMiddleware(t *testing.T) { t.Run("returns middleware when header validation passes", func(t *testing.T) { RegisterTestingT(t) mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{ - HeaderEnabled: true, - HeaderName: "X-HyperFleet-Identity", + HeaderName: "X-HyperFleet-Identity", }) Expect(err).NotTo(HaveOccurred()) Expect(mw).NotTo(BeNil()) }) + + t.Run("returns middleware with no config", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{}) + Expect(err).NotTo(HaveOccurred()) + Expect(mw).NotTo(BeNil()) + }) } func TestResolveCallerIdentityMiddleware(t *testing.T) { @@ -132,7 +182,7 @@ func TestResolveCallerIdentityMiddleware(t *testing.T) { t.Run("skips openapi paths", func(t *testing.T) { RegisterTestingT(t) - mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{JWTEnabled: true, JWTIdentityClaim: "email"}) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{JWTIdentityClaim: "email"}) Expect(err).NotTo(HaveOccurred()) called := false @@ -149,9 +199,28 @@ func TestResolveCallerIdentityMiddleware(t *testing.T) { Expect(w.Code).To(Equal(http.StatusOK)) }) - t.Run("returns 401 when JWT enabled and identity missing", func(t *testing.T) { + t.Run("allows GET without identity", func(t *testing.T) { RegisterTestingT(t) - mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{JWTEnabled: true, JWTIdentityClaim: "email"}) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{JWTIdentityClaim: "email"}) + Expect(err).NotTo(HaveOccurred()) + + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + Expect(GetUsernameFromContext(r.Context())).To(BeEmpty()) + }) + + r := httptest.NewRequest(http.MethodGet, "/api/hyperfleet/v1/clusters", nil) + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(called).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + t.Run("returns 401 on POST without identity", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{JWTIdentityClaim: "email"}) Expect(err).NotTo(HaveOccurred()) nextCalled := false @@ -159,11 +228,152 @@ func TestResolveCallerIdentityMiddleware(t *testing.T) { nextCalled = true }) - r := httptest.NewRequest(http.MethodGet, "/api/hyperfleet/v1/clusters", nil) + r := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", nil) + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + Expect(nextCalled).To(BeFalse()) + }) + + t.Run("returns 401 on PATCH without identity", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{HeaderName: "X-HyperFleet-Identity"}) + Expect(err).NotTo(HaveOccurred()) + + nextCalled := false + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + nextCalled = true + }) + + r := httptest.NewRequest(http.MethodPatch, "/api/hyperfleet/v1/clusters/123", nil) w := httptest.NewRecorder() mw.ResolveCallerIdentity(next).ServeHTTP(w, r) Expect(w.Code).To(Equal(http.StatusUnauthorized)) Expect(nextCalled).To(BeFalse()) }) + + t.Run("returns 401 on DELETE without identity", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{HeaderName: "X-HyperFleet-Identity"}) + Expect(err).NotTo(HaveOccurred()) + + nextCalled := false + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + nextCalled = true + }) + + r := httptest.NewRequest(http.MethodDelete, "/api/hyperfleet/v1/clusters/123", nil) + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + Expect(nextCalled).To(BeFalse()) + }) + + t.Run("allows POST with identity from header", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{HeaderName: "X-HyperFleet-Identity"}) + Expect(err).NotTo(HaveOccurred()) + + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + Expect(GetUsernameFromContext(r.Context())).To(Equal("user@example.com")) + }) + + r := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", nil) + r.Header.Set("X-HyperFleet-Identity", "user@example.com") + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(called).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + t.Run("returns 401 on PUT without identity", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{HeaderName: "X-HyperFleet-Identity"}) + Expect(err).NotTo(HaveOccurred()) + + nextCalled := false + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + nextCalled = true + }) + + r := httptest.NewRequest(http.MethodPut, "/api/hyperfleet/v1/clusters/123", nil) + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + Expect(nextCalled).To(BeFalse()) + }) + + t.Run("returns 401 on POST with oversized header", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{HeaderName: "X-HyperFleet-Identity"}) + Expect(err).NotTo(HaveOccurred()) + + nextCalled := false + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + nextCalled = true + }) + + r := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", nil) + r.Header.Set("X-HyperFleet-Identity", strings.Repeat("a", maxCallerIdentityLen+1)) + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + Expect(nextCalled).To(BeFalse()) + }) + + t.Run("POST with empty header falls back to JWT", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{ + JWTIdentityClaim: "email", + HeaderName: "X-HyperFleet-Identity", + }) + Expect(err).NotTo(HaveOccurred()) + + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + Expect(GetUsernameFromContext(r.Context())).To(Equal("jwt@example.com")) + }) + + r := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", nil) + r = r.WithContext(contextWithClaims(jwt.MapClaims{"email": "jwt@example.com"})) + r.Header.Set("X-HyperFleet-Identity", "") + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(called).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + t.Run("header identity takes precedence over JWT on POST", func(t *testing.T) { + RegisterTestingT(t) + mw, err := NewCallerIdentityMiddleware(CallerIdentityConfig{ + JWTIdentityClaim: "email", + HeaderName: "X-HyperFleet-Identity", + }) + Expect(err).NotTo(HaveOccurred()) + + called := false + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + Expect(GetUsernameFromContext(r.Context())).To(Equal("header@gateway.com")) + }) + + r := httptest.NewRequest(http.MethodPost, "/api/hyperfleet/v1/clusters", nil) + r = r.WithContext(contextWithClaims(jwt.MapClaims{"email": "jwt@example.com"})) + r.Header.Set("X-HyperFleet-Identity", "header@gateway.com") + w := httptest.NewRecorder() + mw.ResolveCallerIdentity(next).ServeHTTP(w, r) + + Expect(called).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusOK)) + }) } diff --git a/pkg/config/flags.go b/pkg/config/flags.go index d1d3ba45..0bb4cef1 100644 --- a/pkg/config/flags.go +++ b/pkg/config/flags.go @@ -36,15 +36,10 @@ func AddServerFlags(cmd *cobra.Command) { defaults.JWT.IdentityClaim, "JWT claim used as request identity for audit", ) - cmd.Flags().Bool( - "server-identity-header-enabled", - defaults.IdentityHeader.Enabled, - "Enable HTTP header as caller identity source for audit", - ) cmd.Flags().String( - "server-identity-header-name", - defaults.IdentityHeader.Name, - "HTTP header name for caller identity (overrides JWT claim when set)", + "server-identity-header", + defaults.IdentityHeader, + "HTTP header name for caller identity (overrides JWT claim when set); leave empty to disable", ) cmd.Flags().String("server-jwk-cert-file", defaults.JWK.CertFile, "JWK certificate file path") cmd.Flags().String("server-jwk-cert-url", defaults.JWK.CertURL, "JWK certificate URL") diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 3a98eb40..0d173822 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -179,7 +179,7 @@ func (l *ConfigLoader) validateConfig(config *ApplicationConfig) error { if valErr := config.Server.JWT.Validate(); valErr != nil { return fmt.Errorf("server JWT validation failed: %w", valErr) } - if valErr := config.Server.IdentityHeader.Validate(); valErr != nil { + if valErr := config.Server.ValidateIdentityHeader(); valErr != nil { return fmt.Errorf("server identity header validation failed: %w", valErr) } if config.Server.JWT.Enabled && @@ -319,8 +319,7 @@ func (l *ConfigLoader) bindAllEnvVars() { l.bindEnv("server.jwt.issuer_url") l.bindEnv("server.jwt.audience") l.bindEnv("server.jwt.identity_claim") - l.bindEnv("server.identity_header.enabled") - l.bindEnv("server.identity_header.name") + l.bindEnv("server.identity_header") l.bindEnv("server.jwk.cert_file") l.bindEnv("server.jwk.cert_url") // Database config @@ -391,8 +390,7 @@ func (l *ConfigLoader) bindFlags(cmd *cobra.Command) { l.bindPFlag("server.jwt.issuer_url", cmd.Flags().Lookup("server-jwt-issuer-url")) l.bindPFlag("server.jwt.audience", cmd.Flags().Lookup("server-jwt-audience")) l.bindPFlag("server.jwt.identity_claim", cmd.Flags().Lookup("server-jwt-identity-claim")) - l.bindPFlag("server.identity_header.enabled", cmd.Flags().Lookup("server-identity-header-enabled")) - l.bindPFlag("server.identity_header.name", cmd.Flags().Lookup("server-identity-header-name")) + l.bindPFlag("server.identity_header", cmd.Flags().Lookup("server-identity-header")) l.bindPFlag("server.jwk.cert_file", cmd.Flags().Lookup("server-jwk-cert-file")) l.bindPFlag("server.jwk.cert_url", cmd.Flags().Lookup("server-jwk-cert-url")) // Database flags: --db-* -> database.* diff --git a/pkg/config/logging.go b/pkg/config/logging.go index d6e77c18..dad1b188 100644 --- a/pkg/config/logging.go +++ b/pkg/config/logging.go @@ -46,7 +46,6 @@ func NewLoggingConfig() *LoggingConfig { "Cookie", "X-Auth-Token", "X-Forwarded-Authorization", - "X-HyperFleet-Identity", }, Fields: []string{ "password", diff --git a/pkg/config/server.go b/pkg/config/server.go index 5d29376e..45a1aa08 100755 --- a/pkg/config/server.go +++ b/pkg/config/server.go @@ -12,15 +12,15 @@ import ( // ServerConfig holds HTTP/HTTPS server configuration // Follows HyperFleet Configuration Standard type ServerConfig struct { - Hostname string `mapstructure:"hostname" json:"hostname" validate:"omitempty,hostname|ip"` - Host string `mapstructure:"host" json:"host" validate:"required,hostname|ip"` - OpenAPISchemaPath string `mapstructure:"openapi_schema_path" json:"openapi_schema_path"` - JWK JWKConfig `mapstructure:"jwk" json:"jwk" validate:"required"` - TLS TLSConfig `mapstructure:"tls" json:"tls" validate:"required"` - JWT JWTConfig `mapstructure:"jwt" json:"jwt" validate:"required"` - IdentityHeader IdentityHeaderConfig `mapstructure:"identity_header" json:"identity_header" validate:"required"` - Timeouts TimeoutsConfig `mapstructure:"timeouts" json:"timeouts" validate:"required"` - Port int `mapstructure:"port" json:"port" validate:"required,min=1,max=65535"` + Hostname string `mapstructure:"hostname" json:"hostname" validate:"omitempty,hostname|ip"` + Host string `mapstructure:"host" json:"host" validate:"required,hostname|ip"` + OpenAPISchemaPath string `mapstructure:"openapi_schema_path" json:"openapi_schema_path"` + IdentityHeader string `mapstructure:"identity_header" json:"identity_header"` + JWK JWKConfig `mapstructure:"jwk" json:"jwk" validate:"required"` + TLS TLSConfig `mapstructure:"tls" json:"tls" validate:"required"` + JWT JWTConfig `mapstructure:"jwt" json:"jwt" validate:"required"` + Timeouts TimeoutsConfig `mapstructure:"timeouts" json:"timeouts" validate:"required"` + Port int `mapstructure:"port" json:"port" validate:"required,min=1,max=65535"` } // TimeoutsConfig holds HTTP timeout configuration @@ -83,21 +83,13 @@ func (c *JWTConfig) Validate() error { return nil } -// IdentityHeaderConfig holds HTTP header-based caller identity configuration. -type IdentityHeaderConfig struct { - Name string `mapstructure:"name" json:"name"` - Enabled bool `mapstructure:"enabled" json:"enabled"` -} - -func (c *IdentityHeaderConfig) Validate() error { - if !c.Enabled { +// ValidateIdentityHeader validates the identity header name if set. +func (s *ServerConfig) ValidateIdentityHeader() error { + if s.IdentityHeader == "" { return nil } - if c.Name == "" { - return fmt.Errorf("server.identity_header.name is required when identity_header is enabled") - } - if validation.IsForbiddenIdentityHeaderName(c.Name) { - return fmt.Errorf("server.identity_header.name %q is not allowed", c.Name) + if validation.IsForbiddenIdentityHeaderName(s.IdentityHeader) { + return fmt.Errorf("server.identity_header %q is not allowed", s.IdentityHeader) } return nil } @@ -131,10 +123,6 @@ func NewServerConfig() *ServerConfig { Audience: "", IdentityClaim: "email", }, - IdentityHeader: IdentityHeaderConfig{ - Enabled: false, - Name: "X-HyperFleet-Identity", - }, JWK: JWKConfig{ CertFile: "", CertURL: "", diff --git a/pkg/config/server_test.go b/pkg/config/server_test.go index 1fb61eb4..dabb1e6b 100644 --- a/pkg/config/server_test.go +++ b/pkg/config/server_test.go @@ -42,35 +42,27 @@ func TestJWTConfig_Validate(t *testing.T) { }) } -func TestIdentityHeaderConfig_Validate(t *testing.T) { +func TestServerConfig_ValidateIdentityHeader(t *testing.T) { RegisterTestingT(t) - t.Run("disabled identity header requires nothing", func(t *testing.T) { + t.Run("empty identity header requires nothing", func(t *testing.T) { RegisterTestingT(t) - cfg := IdentityHeaderConfig{Enabled: false} - Expect(cfg.Validate()).To(Succeed()) - }) - - t.Run("enabled identity header without name fails", func(t *testing.T) { - RegisterTestingT(t) - cfg := IdentityHeaderConfig{Enabled: true, Name: ""} - err := cfg.Validate() - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("identity_header.name")) + cfg := &ServerConfig{} + Expect(cfg.ValidateIdentityHeader()).To(Succeed()) }) t.Run("forbidden header name fails", func(t *testing.T) { RegisterTestingT(t) - cfg := IdentityHeaderConfig{Enabled: true, Name: "Authorization"} - err := cfg.Validate() + cfg := &ServerConfig{IdentityHeader: "Authorization"} + err := cfg.ValidateIdentityHeader() Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("not allowed")) }) - t.Run("enabled identity header with valid name passes", func(t *testing.T) { + t.Run("valid header name passes", func(t *testing.T) { RegisterTestingT(t) - cfg := IdentityHeaderConfig{Enabled: true, Name: "X-HyperFleet-Identity"} - Expect(cfg.Validate()).To(Succeed()) + cfg := &ServerConfig{IdentityHeader: "X-HyperFleet-Identity"} + Expect(cfg.ValidateIdentityHeader()).To(Succeed()) }) } diff --git a/pkg/services/cluster.go b/pkg/services/cluster.go index 28577a84..2d72e81f 100644 --- a/pkg/services/cluster.go +++ b/pkg/services/cluster.go @@ -131,7 +131,7 @@ func (s *sqlClusterService) Patch( } // SoftDelete marks a cluster for deletion by setting DeletedTime and -// DeletedBy to the current time and system@hyperfleet.local. +// DeletedBy to the current time and the caller identity from context. // If already marked, it returns the cluster unchanged. Cascades the deletion timestamp to all child nodepools. // Actual removal is handled by adapters detecting the new generation and triggering hard deletion asynchronously. func (s *sqlClusterService) SoftDelete(ctx context.Context, id string) (*api.Cluster, *errors.ServiceError) { @@ -146,7 +146,7 @@ func (s *sqlClusterService) SoftDelete(ctx context.Context, id string) (*api.Clu } deletedTime := time.Now().UTC().Truncate(time.Microsecond) - deletedBy := defaultSystemUser + deletedBy := actorFromContext(ctx) cluster.MarkDeleted(deletedBy, deletedTime) cluster.IncrementGeneration() diff --git a/pkg/services/node_pool.go b/pkg/services/node_pool.go index ffcec684..6c43845b 100644 --- a/pkg/services/node_pool.go +++ b/pkg/services/node_pool.go @@ -187,7 +187,7 @@ func (s *sqlNodePoolService) SoftDelete(ctx context.Context, nodePoolID string) } t := time.Now().UTC().Truncate(time.Microsecond) - deletedBy := defaultSystemUser + deletedBy := actorFromContext(ctx) nodePool.MarkDeleted(deletedBy, t) nodePool.IncrementGeneration() diff --git a/plugins/channels/plugin.go b/plugins/channels/plugin.go index 450301b3..681f2e84 100644 --- a/plugins/channels/plugin.go +++ b/plugins/channels/plugin.go @@ -7,7 +7,6 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/environments" "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/server" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" @@ -27,11 +26,7 @@ func init() { SearchDisallowedFields: []string{"spec"}, }) - server.RegisterRoutes(pluralChannels, func( - apiV1Router *mux.Router, - svc server.ServicesInterface, - authMiddleware auth.JWTMiddleware, - ) { + server.RegisterRoutes(pluralChannels, func(apiV1Router *mux.Router, svc server.ServicesInterface) { envServices := svc.(*environments.Services) descriptor := registry.MustGet(kindChannel) h := handlers.NewResourceHandler( @@ -45,7 +40,5 @@ func init() { r.HandleFunc("/{id}", h.Get).Methods(http.MethodGet) r.HandleFunc("/{id}", h.Patch).Methods(http.MethodPatch) r.HandleFunc("/{id}", h.Delete).Methods(http.MethodDelete) - - r.Use(authMiddleware.AuthenticateAccountJWT) }) } diff --git a/plugins/versions/plugin.go b/plugins/versions/plugin.go index 7edfd8ed..c3852ee7 100644 --- a/plugins/versions/plugin.go +++ b/plugins/versions/plugin.go @@ -7,7 +7,6 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/environments" "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/server" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" @@ -29,11 +28,7 @@ func init() { SearchDisallowedFields: []string{"spec"}, }) - server.RegisterRoutes(pluralVersions, func( - apiV1Router *mux.Router, - svc server.ServicesInterface, - authMiddleware auth.JWTMiddleware, - ) { + server.RegisterRoutes(pluralVersions, func(apiV1Router *mux.Router, svc server.ServicesInterface) { envServices := svc.(*environments.Services) channelDescriptor := registry.MustGet("Channel") descriptor := registry.MustGet(kindVersion) @@ -48,7 +43,5 @@ func init() { r.HandleFunc("/{id}", h.GetByOwner).Methods(http.MethodGet) r.HandleFunc("/{id}", h.PatchByOwner).Methods(http.MethodPatch) r.HandleFunc("/{id}", h.DeleteByOwner).Methods(http.MethodDelete) - - r.Use(authMiddleware.AuthenticateAccountJWT) }) } diff --git a/test/helper.go b/test/helper.go index 32c8b8ae..f78c67a1 100755 --- a/test/helper.go +++ b/test/helper.go @@ -381,7 +381,7 @@ func WithIdentityHeader(headerName, headerValue string) openapi.RequestEditorFn // IdentityHeaderName returns the configured identity header name for integration tests. func IdentityHeaderName() string { - return environments.Environment().Config.Server.IdentityHeader.Name + return environments.Environment().Config.Server.IdentityHeader } func (helper *Helper) StartJWKCertServerMock() (teardown func() error) { diff --git a/test/integration/caller_identity_test.go b/test/integration/caller_identity_test.go index 8d09952e..c1905e40 100644 --- a/test/integration/caller_identity_test.go +++ b/test/integration/caller_identity_test.go @@ -157,3 +157,155 @@ func TestCallerIdentityPatch(t *testing.T) { }) } } + +func TestCallerIdentityMultiplePatches(t *testing.T) { + RegisterTestingT(t) + + h, client := test.RegisterIntegration(t) + + // Create cluster as user-a. + accountA := h.NewAccount(h.NewID(), "User A", "user-a@example.com") + ctxA := h.NewAuthenticatedContext(accountA) + + clusterInput := openapi.ClusterCreateRequest{ + Kind: util.PtrString("Cluster"), + Name: shortClusterName("ci-multi", h.NewID()), + Spec: map[string]interface{}{"version": "1"}, + } + createResp, err := client.PostClusterWithResponse( + ctxA, openapi.PostClusterJSONRequestBody(clusterInput), test.WithAuthToken(ctxA), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(createResp.StatusCode()).To(Equal(http.StatusCreated)) + clusterID := *createResp.JSON201.Id + + Expect(string(createResp.JSON201.CreatedBy)).To(Equal("user-a@example.com")) + Expect(string(createResp.JSON201.UpdatedBy)).To(Equal("user-a@example.com")) + Expect(createResp.JSON201.Generation).To(Equal(int32(1))) + + // Patch 1: user-b via JWT. + accountB := h.NewAccount(h.NewID(), "User B", "user-b@example.com") + ctxB := h.NewAuthenticatedContext(accountB) + + spec2 := openapi.ClusterSpec{"version": "2"} + patch1, err := client.PatchClusterByIdWithResponse( + ctxB, clusterID, + openapi.PatchClusterByIdJSONRequestBody{Spec: &spec2}, + test.WithAuthToken(ctxB), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(patch1.StatusCode()).To(Equal(http.StatusOK)) + + Expect(string(patch1.JSON200.CreatedBy)).To(Equal("user-a@example.com"), "created_by must never change") + Expect(string(patch1.JSON200.UpdatedBy)).To(Equal("user-b@example.com")) + Expect(patch1.JSON200.Generation).To(Equal(int32(2))) + + // Patch 2: user-c via identity header. + spec3 := openapi.ClusterSpec{"version": "3"} + patch2, err := client.PatchClusterByIdWithResponse( + ctxA, clusterID, + openapi.PatchClusterByIdJSONRequestBody{Spec: &spec3}, + test.WithAuthToken(ctxA), + test.WithIdentityHeader(test.IdentityHeaderName(), "user-c@gateway.com"), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(patch2.StatusCode()).To(Equal(http.StatusOK)) + + Expect(string(patch2.JSON200.CreatedBy)).To(Equal("user-a@example.com"), "created_by must never change") + Expect(string(patch2.JSON200.UpdatedBy)).To(Equal("user-c@gateway.com")) + Expect(patch2.JSON200.Generation).To(Equal(int32(3))) + + // GET confirms persisted state. + getResp, err := client.GetClusterByIdWithResponse(ctxA, clusterID, nil, test.WithAuthToken(ctxA)) + Expect(err).NotTo(HaveOccurred()) + Expect(getResp.StatusCode()).To(Equal(http.StatusOK)) + Expect(string(getResp.JSON200.CreatedBy)).To(Equal("user-a@example.com")) + Expect(string(getResp.JSON200.UpdatedBy)).To(Equal("user-c@gateway.com")) + Expect(getResp.JSON200.Generation).To(Equal(int32(3))) +} + +func TestCallerIdentityDelete(t *testing.T) { + RegisterTestingT(t) + + h, client := test.RegisterIntegration(t) + + // Create cluster with identity header. + account := h.NewAccount(h.NewID(), "Creator", "creator@example.com") + ctx := h.NewAuthenticatedContext(account) + + clusterInput := openapi.ClusterCreateRequest{ + Kind: util.PtrString("Cluster"), + Name: shortClusterName("ci-del", h.NewID()), + Spec: map[string]interface{}{"test": "spec"}, + } + createResp, err := client.PostClusterWithResponse( + ctx, openapi.PostClusterJSONRequestBody(clusterInput), + test.WithAuthToken(ctx), + test.WithIdentityHeader(test.IdentityHeaderName(), "header-creator@corp.com"), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(createResp.StatusCode()).To(Equal(http.StatusCreated)) + clusterID := *createResp.JSON201.Id + Expect(string(createResp.JSON201.CreatedBy)).To(Equal("header-creator@corp.com")) + + // Soft-delete the cluster. + delResp, err := client.DeleteClusterByIdWithResponse(ctx, clusterID, test.WithAuthToken(ctx)) + Expect(err).NotTo(HaveOccurred()) + Expect(delResp.StatusCode()).To(Equal(http.StatusAccepted)) + Expect(delResp.JSON202).NotTo(BeNil()) + Expect(delResp.JSON202.DeletedTime).NotTo(BeNil()) + Expect(delResp.JSON202.DeletedBy).NotTo(BeNil()) + // deleted_by reflects the caller identity. + Expect(string(*delResp.JSON202.DeletedBy)).To(Equal("creator@example.com")) + // created_by is preserved. + Expect(string(delResp.JSON202.CreatedBy)).To(Equal("header-creator@corp.com")) +} + +func TestCallerIdentityEmptyHeaderFallback(t *testing.T) { + RegisterTestingT(t) + + h, client := test.RegisterIntegration(t) + + account := h.NewAccount(h.NewID(), "JWT User", "jwt-fallback@example.com") + ctx := h.NewAuthenticatedContext(account) + + // Create with an empty identity header — should fall back to JWT claim. + clusterInput := openapi.ClusterCreateRequest{ + Kind: util.PtrString("Cluster"), + Name: shortClusterName("ci-empty", h.NewID()), + Spec: map[string]interface{}{"test": "spec"}, + } + resp, err := client.PostClusterWithResponse( + ctx, openapi.PostClusterJSONRequestBody(clusterInput), + test.WithAuthToken(ctx), + test.WithIdentityHeader(test.IdentityHeaderName(), ""), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.StatusCode()).To(Equal(http.StatusCreated)) + Expect(string(resp.JSON201.CreatedBy)).To(Equal("jwt-fallback@example.com")) + Expect(string(resp.JSON201.UpdatedBy)).To(Equal("jwt-fallback@example.com")) +} + +func TestCallerIdentityOversizedHeader(t *testing.T) { + RegisterTestingT(t) + + h, client := test.RegisterIntegration(t) + + account := h.NewAccount(h.NewID(), "Test User", "user@example.com") + ctx := h.NewAuthenticatedContext(account) + + // Create with an oversized identity header (>256 chars) — should be rejected. + clusterInput := openapi.ClusterCreateRequest{ + Kind: util.PtrString("Cluster"), + Name: shortClusterName("ci-long", h.NewID()), + Spec: map[string]interface{}{"test": "spec"}, + } + oversized := strings.Repeat("a", 257) + resp, err := client.PostClusterWithResponse( + ctx, openapi.PostClusterJSONRequestBody(clusterInput), + test.WithAuthToken(ctx), + test.WithIdentityHeader(test.IdentityHeaderName(), oversized), + ) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.StatusCode()).To(Equal(http.StatusUnauthorized)) +} diff --git a/test/integration/clusters_test.go b/test/integration/clusters_test.go index 30f6b59a..e6b7c114 100644 --- a/test/integration/clusters_test.go +++ b/test/integration/clusters_test.go @@ -958,7 +958,7 @@ func TestClusterSoftDelete(t *testing.T) { Expect(*resp.JSON202.Id).To(Equal(cluster.ID)) Expect(resp.JSON202.DeletedTime).NotTo(BeNil()) Expect(resp.JSON202.DeletedBy).NotTo(BeNil()) - Expect(string(*resp.JSON202.DeletedBy)).To(Equal("system@hyperfleet.local")) + Expect(string(*resp.JSON202.DeletedBy)).To(Equal(account.Email)) }) t.Run("given a cluster with child nodepools, when deleted, then nodepools are cascade soft-deleted in DB", func(t *testing.T) { //nolint:lll @@ -986,7 +986,7 @@ func TestClusterSoftDelete(t *testing.T) { Expect(err).NotTo(HaveOccurred()) Expect(delResp.StatusCode()).To(Equal(http.StatusAccepted)) Expect(delResp.JSON202.DeletedTime).NotTo(BeNil()) - Expect(string(*delResp.JSON202.DeletedBy)).To(Equal("system@hyperfleet.local")) + Expect(string(*delResp.JSON202.DeletedBy)).To(Equal(account.Email)) // Verify cascade via direct DB query dbSession := h.DBFactory.New(ctx) var nodePool api.NodePool diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index e6042643..f392d089 100755 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -34,11 +34,8 @@ func TestMain(m *testing.M) { if os.Getenv("HYPERFLEET_SERVER_JWK_CERT_URL") == "" { _ = os.Setenv("HYPERFLEET_SERVER_JWK_CERT_URL", "https://test-idp.example.com/certs") } - if os.Getenv("HYPERFLEET_SERVER_IDENTITY_HEADER_ENABLED") == "" { - _ = os.Setenv("HYPERFLEET_SERVER_IDENTITY_HEADER_ENABLED", "true") - } - if os.Getenv("HYPERFLEET_SERVER_IDENTITY_HEADER_NAME") == "" { - _ = os.Setenv("HYPERFLEET_SERVER_IDENTITY_HEADER_NAME", "X-HyperFleet-Identity") + if os.Getenv("HYPERFLEET_SERVER_IDENTITY_HEADER") == "" { + _ = os.Setenv("HYPERFLEET_SERVER_IDENTITY_HEADER", "X-HyperFleet-Identity") } // Set OpenAPI schema path for integration tests if not already set diff --git a/test/integration/node_pools_test.go b/test/integration/node_pools_test.go index 81a87514..48271ace 100644 --- a/test/integration/node_pools_test.go +++ b/test/integration/node_pools_test.go @@ -601,7 +601,7 @@ func TestNodePoolSoftDelete(t *testing.T) { Expect(*resp.JSON202.Id).To(Equal(nodePoolID)) Expect(resp.JSON202.DeletedTime).NotTo(BeNil()) Expect(resp.JSON202.DeletedBy).NotTo(BeNil()) - Expect(string(*resp.JSON202.DeletedBy)).To(Equal("system@hyperfleet.local")) + Expect(string(*resp.JSON202.DeletedBy)).To(Equal(account.Email)) }) t.Run("given a nodepool with Ready=True, when deleted, then generation increments and Ready becomes False", func(t *testing.T) { //nolint:lll