From 5857bac537d81d8adf27db080faa57e0bc7735bb Mon Sep 17 00:00:00 2001 From: pnguyen44 Date: Tue, 18 Aug 2026 16:50:48 -0400 Subject: [PATCH 1/7] HYPERFLEET-1469 - feat: add tenant configuration and enforcement middleware --- CHANGELOG.md | 2 + cmd/hyperfleet-api/servecmd/api_server.go | 5 + .../servecmd/api_server_test.go | 64 ++++++ configs/config.yaml.example | 5 + configs/dev.yaml | 43 ++++ pkg/api/response/service_error.go | 27 +++ pkg/auth/auth_middleware.go | 2 +- pkg/auth/identity.go | 5 +- pkg/config/flags.go | 3 + pkg/config/loader.go | 10 + pkg/config/server.go | 4 + pkg/config/tenant.go | 98 +++++++++ pkg/config/tenant_test.go | 153 ++++++++++++++ pkg/errors/errors.go | 36 +++- pkg/errors/errors_test.go | 8 + pkg/tenant/middleware.go | 116 ++++++++++ pkg/tenant/middleware_test.go | 199 ++++++++++++++++++ 17 files changed, 767 insertions(+), 13 deletions(-) create mode 100644 cmd/hyperfleet-api/servecmd/api_server_test.go create mode 100644 pkg/api/response/service_error.go create mode 100644 pkg/config/tenant.go create mode 100644 pkg/config/tenant_test.go create mode 100644 pkg/tenant/middleware.go create mode 100644 pkg/tenant/middleware_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e631ed35..805a7574 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Tenant enforcement middleware that resolves caller tenant identity from trusted gateway-injected headers; configurable via `server.tenant` (`enabled`, `system_header`, `dimensions` with header, key, and required flag); system callers receive unscoped context, non-system callers missing required dimensions or resolving zero dimensions receive 403 problem+json +- `HYPERFLEET-AUZ-001` Permission Denied error code for tenant identity rejection responses - Grafana dashboard for API and database metrics (`charts/dashboards/hyperfleet-api.json`) — covers HTTP request rate/latency, reconciliation pending/stuck gauges, DB query duration/errors, connection pool, and build info ([#311](https://github.com/openshift-hyperfleet/hyperfleet-api/pull/311)) - JWT authentication handler using `golang-jwt/jwt/v5` and `MicahParks/keyfunc/v3` with RS256 validation, configurable issuer and audience, and JWKS key rotation support ([#120](https://github.com/openshift-hyperfleet/hyperfleet-api/pull/120)) - Hard deletion for Clusters and NodePools: resources and their adapter statuses are permanently removed from the database once all required adapters report `Finalized=True` and no child resources remain ([#119](https://github.com/openshift-hyperfleet/hyperfleet-api/pull/119)) diff --git a/cmd/hyperfleet-api/servecmd/api_server.go b/cmd/hyperfleet-api/servecmd/api_server.go index da22c6e2..55d92f4b 100644 --- a/cmd/hyperfleet-api/servecmd/api_server.go +++ b/cmd/hyperfleet-api/servecmd/api_server.go @@ -12,6 +12,7 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/middleware" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/tenant" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validators" ) @@ -66,6 +67,10 @@ func BuildAPIServer( callerIdentityMiddleware.ResolveCallerIdentity, ) } + if cfg.Server.Tenant.Enabled { + tenantResolver := tenant.NewResolver(cfg.Server.Tenant) + authMiddleware = append(authMiddleware, tenantResolver.ResolveTenant) + } registrars := []server.RouteRegistrar{ server.NewEntityRouteRegistrar(resourceService, adapterStatusService, schemaValidator), diff --git a/cmd/hyperfleet-api/servecmd/api_server_test.go b/cmd/hyperfleet-api/servecmd/api_server_test.go new file mode 100644 index 00000000..b2b2cb6a --- /dev/null +++ b/cmd/hyperfleet-api/servecmd/api_server_test.go @@ -0,0 +1,64 @@ +package servecmd + +import ( + "net/http" + "testing" + + . "github.com/onsi/gomega" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/config" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" +) + +// TestBuildAPIServer_TenantMiddlewareWiredWhenEnabled guards the composition-root +// wiring in BuildAPIServer: when cfg.Server.Tenant.Enabled is true, the tenant +// middleware must actually be appended to the auth chain and enforced. Without +// this test, a broken or dropped wiring (e.g. reordered/removed the append) +// would ship silently since pkg/tenant's own tests only exercise the middleware +// in isolation, never through BuildAPIServer. +// +// resourceService, adapterStatusService, schemaValidator, and sessionFactory +// are all nil: the tenant middleware runs in authMiddleware, ahead of +// protectedAPIMiddleware and the entity handlers, so a request rejected for a +// missing required tenant header never reaches code that would dereference them. +func TestBuildAPIServer_TenantMiddlewareWiredWhenEnabled(t *testing.T) { + RegisterTestingT(t) + registry.Reset() + t.Cleanup(registry.Reset) + registry.Register(registry.EntityDescriptor{Kind: "Channel", Plural: "channels"}) + + cfg := config.NewApplicationConfig() + cfg.Server.Host = "127.0.0.1" + cfg.Server.Port = 0 // ephemeral port + cfg.Server.JWT.Enabled = false + cfg.Server.Tenant = config.TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []config.TenantDimension{ + {Header: "X-HyperFleet-Org", Key: "org", Required: true}, + }, + } + + apiServer, err := BuildAPIServer(cfg, nil, nil, nil, nil, nil, false) + Expect(err).NotTo(HaveOccurred()) + + listener, err := apiServer.Listen() + Expect(err).NotTo(HaveOccurred()) + go apiServer.Serve(listener) + t.Cleanup(func() { _ = apiServer.Stop() }) + + baseURL := "http://" + listener.Addr().String() + + // Missing the required tenant dimension header: tenant middleware must + // reject with 403 before the request reaches any entity handler. + var resp *http.Response + Eventually(func() error { + var getErr error + resp, getErr = http.Get(baseURL + "/api/hyperfleet/v1/channels") + return getErr + }, "2s", "25ms").Should(Succeed()) + defer resp.Body.Close() + + Expect(resp.StatusCode).To(Equal(http.StatusForbidden), + "request missing a required tenant header must be rejected by the tenant middleware") +} diff --git a/configs/config.yaml.example b/configs/config.yaml.example index 8ddde292..187bb478 100644 --- a/configs/config.yaml.example +++ b/configs/config.yaml.example @@ -30,6 +30,11 @@ server: identity_claim_pattern: "" # Regex pattern to validate the identity claim value (optional) identity_header: "" # Per-issuer HTTP header for caller identity; overrides JWT claim when set (e.g. X-HyperFleet-Identity) + tenant: + enabled: false # Enable tenant enforcement middleware + system_header: "" # Header marking system callers (required when enabled=true) + dimensions: [] # Tenant dimension headers (header, key, required); at least one required=true if enabled + # Database Configuration database: dialect: postgres # Database dialect (postgres, mysql) diff --git a/configs/dev.yaml b/configs/dev.yaml index 6612fee4..652e222d 100644 --- a/configs/dev.yaml +++ b/configs/dev.yaml @@ -6,3 +6,46 @@ server: jwk_cert_file: configs/dev-jwks.json header: Authorization identity_claim: email + tenant: + enabled: true + system_header: X-HyperFleet-System + dimensions: + - header: X-HyperFleet-Org + key: org + required: true + - header: X-HyperFleet-Project + key: project + required: false + +entities: + - kind: Cluster + plural: clusters + spec_schema_name: ClusterSpec + required_adapters: [validation, dns, pullsecret, hypershift] + name_min_len: 3 + name_max_len: 53 + require_spec_schema: true + + - kind: NodePool + plural: nodepools + parent_kind: Cluster + on_parent_delete: cascade + spec_schema_name: NodePoolSpec + required_adapters: [validation, hypershift] + name_min_len: 3 + name_max_len: 15 + require_spec_schema: true + + - kind: Channel + plural: channels + spec_schema_name: ChannelSpec + + - kind: Version + plural: versions + parent_kind: Channel + on_parent_delete: restrict + spec_schema_name: VersionSpec + + - kind: WifConfig + plural: wifconfigs + spec_schema_name: WifConfigSpec diff --git a/pkg/api/response/service_error.go b/pkg/api/response/service_error.go new file mode 100644 index 00000000..83338c46 --- /dev/null +++ b/pkg/api/response/service_error.go @@ -0,0 +1,27 @@ +package response + +import ( + "context" + "net/http" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" +) + +// WriteServiceErrorResponse writes err as an RFC 9457 Problem Details response, +// resolving the trace ID from ctx (via logger.GetRequestID) and using the +// request path as the problem instance. ctx is taken explicitly rather than +// derived from r.Context() so callers can still resolve a trace ID when r is +// nil. Callers are responsible for logging the error themselves beforehand, +// since the appropriate log level (e.g. Warn vs Info vs Error) varies by caller. +func WriteServiceErrorResponse(ctx context.Context, w http.ResponseWriter, r *http.Request, err *errors.ServiceError) { + traceID, ok := logger.GetRequestID(ctx) + if !ok { + traceID = "unknown" + } + instance := "" + if r != nil { + instance = r.URL.Path + } + WriteProblemDetailsResponse(w, r, err.HTTPCode, err.AsProblemDetails(instance, traceID)) +} diff --git a/pkg/auth/auth_middleware.go b/pkg/auth/auth_middleware.go index c4c192b9..c76f9459 100755 --- a/pkg/auth/auth_middleware.go +++ b/pkg/auth/auth_middleware.go @@ -26,7 +26,7 @@ func NewCallerIdentityMiddleware() CallerIdentityMiddleware { // If an identity header is configured, it takes precedence over JWT claims. func (m *callerIdentityMiddleware) ResolveCallerIdentity(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if shouldSkipCallerIdentity(r.URL.Path) { + if ShouldSkipCallerIdentity(r.URL.Path) { next.ServeHTTP(w, r) return } diff --git a/pkg/auth/identity.go b/pkg/auth/identity.go index e7aa7bbb..50dd546a 100644 --- a/pkg/auth/identity.go +++ b/pkg/auth/identity.go @@ -84,7 +84,10 @@ func normalizeIdentity(raw string, source string) (string, error) { return value, nil } -func shouldSkipCallerIdentity(path string) bool { +// ShouldSkipCallerIdentity reports whether path bypasses caller identity +// resolution. Exported so pkg/tenant can reuse the same skip list for tenant +// enforcement instead of maintaining a second copy of these path prefixes. +func ShouldSkipCallerIdentity(path string) bool { return strings.HasPrefix(path, "/api/hyperfleet/v1/openapi") || strings.HasPrefix(path, "/api/hyperfleet/v1/errors") } diff --git a/pkg/config/flags.go b/pkg/config/flags.go index 1d8f1584..0cdc1c34 100644 --- a/pkg/config/flags.go +++ b/pkg/config/flags.go @@ -29,6 +29,9 @@ func AddServerFlags(cmd *cobra.Command) { cmd.Flags().String("server-https-key-file", defaults.TLS.KeyFile, "Path to TLS key file") cmd.Flags().Bool("server-https-enabled", defaults.TLS.Enabled, "Enable HTTPS rather than HTTP") cmd.Flags().Bool("server-jwt-enabled", defaults.JWT.Enabled, "Enable JWT authentication") + cmd.Flags().Bool("server-tenant-enabled", defaults.Tenant.Enabled, "Enable tenant enforcement middleware") + cmd.Flags().String("server-tenant-system-header", defaults.Tenant.SystemHeader, + "Trusted header identifying system callers (bypasses tenant scoping)") } // AddDatabaseFlags adds database configuration flags following standard naming diff --git a/pkg/config/loader.go b/pkg/config/loader.go index d7ef7d5d..9526814f 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -175,6 +175,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.Tenant.Validate(); valErr != nil { + return fmt.Errorf("server tenant validation failed: %w", valErr) + } if valErr := config.Health.Validate(); valErr != nil { return fmt.Errorf("health config validation failed: %w", valErr) } @@ -265,6 +268,10 @@ func (l *ConfigLoader) bindAllEnvVars() { l.bindEnv("server.jwt.enabled") // server.jwt.configs is a list of structs — loaded from YAML config only. // Viper cannot bind env vars to individual list elements. + l.bindEnv("server.tenant.enabled") + l.bindEnv("server.tenant.system_header") + // server.tenant.dimensions is a list of structs — loaded from YAML config only, + // same reason as server.jwt.configs above. // Database config l.bindEnv("database.dialect") l.bindEnv("database.host") @@ -330,6 +337,9 @@ func (l *ConfigLoader) bindFlags(cmd *cobra.Command) { l.bindPFlag("server.tls.enabled", cmd.Flags().Lookup("server-https-enabled")) l.bindPFlag("server.jwt.enabled", cmd.Flags().Lookup("server-jwt-enabled")) // server.jwt.configs: no CLI flags — per-issuer config is YAML-only + l.bindPFlag("server.tenant.enabled", cmd.Flags().Lookup("server-tenant-enabled")) + l.bindPFlag("server.tenant.system_header", cmd.Flags().Lookup("server-tenant-system-header")) + // server.tenant.dimensions: no CLI flags — per-dimension config is YAML-only // Database flags: --db-* -> database.* l.bindPFlag("database.host", cmd.Flags().Lookup("db-host")) l.bindPFlag("database.port", cmd.Flags().Lookup("db-port")) diff --git a/pkg/config/server.go b/pkg/config/server.go index 8ce8c046..976a5842 100755 --- a/pkg/config/server.go +++ b/pkg/config/server.go @@ -20,6 +20,7 @@ type ServerConfig struct { OpenAPISchemaPath string `mapstructure:"openapi_schema_path" json:"openapi_schema_path"` TLS TLSConfig `mapstructure:"tls" json:"tls" validate:"required"` JWT JWTConfig `mapstructure:"jwt" json:"jwt" validate:"required"` + Tenant TenantConfig `mapstructure:"tenant" json:"tenant" validate:"required"` Timeouts TimeoutsConfig `mapstructure:"timeouts" json:"timeouts" validate:"required"` Port int `mapstructure:"port" json:"port" validate:"required,min=1,max=65535"` } @@ -193,6 +194,9 @@ func NewServerConfig() *ServerConfig { JWT: JWTConfig{ Enabled: true, }, + Tenant: TenantConfig{ + Enabled: false, + }, } } diff --git a/pkg/config/tenant.go b/pkg/config/tenant.go new file mode 100644 index 00000000..0c5f57a4 --- /dev/null +++ b/pkg/config/tenant.go @@ -0,0 +1,98 @@ +package config + +import ( + "fmt" + "strings" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/validation" +) + +// TenantDimension maps one trusted gateway-injected header to a tenancy key. +// Required dimensions must be present for a non-system caller to be granted +// a tenant context; see TenantConfig.Validate for the at-least-one-required rule. +type TenantDimension struct { + Header string `mapstructure:"header" json:"header"` + Key string `mapstructure:"key" json:"key"` + Required bool `mapstructure:"required" json:"required"` +} + +// TenantConfig holds tenant enforcement middleware configuration. +// Identity arrives as trusted headers injected by the gateway (Envoy + Authorino); +// the API never extracts JWT claims for tenancy. +type TenantConfig struct { + SystemHeader string `mapstructure:"system_header" json:"system_header"` + Dimensions []TenantDimension `mapstructure:"dimensions" json:"dimensions"` + Enabled bool `mapstructure:"enabled" json:"enabled"` +} + +// Validate enforces the tenant configuration invariants. A caller resolving zero +// dimensions must never reach the data layer (an empty tenancy map would +// contain-match every row), so at least one dimension must be required. +func (c *TenantConfig) Validate() error { + if !c.Enabled { + return nil + } + + if c.SystemHeader == "" { + return fmt.Errorf("server.tenant.system_header is required when tenant is enabled") + } + if validation.IsForbiddenIdentityHeaderName(c.SystemHeader) { + return fmt.Errorf("server.tenant.system_header %q is not allowed", c.SystemHeader) + } + if len(c.Dimensions) == 0 { + return fmt.Errorf("server.tenant.dimensions requires at least one dimension when tenant is enabled") + } + + seenHeaders := make(map[string]bool, len(c.Dimensions)) + seenKeys := make(map[string]bool, len(c.Dimensions)) + requiredCount := 0 + + for i := range c.Dimensions { + dim := &c.Dimensions[i] + if err := c.validateDimension(i, dim, seenHeaders, seenKeys); err != nil { + return err + } + if dim.Required { + requiredCount++ + } + } + + if requiredCount == 0 { + return fmt.Errorf("server.tenant.dimensions requires at least one dimension with required: true") + } + + return nil +} + +// validateDimension checks a single dimension against the config-level rules: +// required fields, forbidden/colliding header names, and duplicate headers or keys +// across previously validated dimensions in the same config (tracked via seenHeaders/seenKeys). +func (c *TenantConfig) validateDimension( + i int, dim *TenantDimension, seenHeaders, seenKeys map[string]bool, +) error { + if dim.Header == "" { + return fmt.Errorf("server.tenant.dimensions[%d].header is required", i) + } + if dim.Key == "" { + return fmt.Errorf("server.tenant.dimensions[%d].key is required", i) + } + if validation.IsForbiddenIdentityHeaderName(dim.Header) { + return fmt.Errorf("server.tenant.dimensions[%d].header %q is not allowed", i, dim.Header) + } + if strings.EqualFold(dim.Header, c.SystemHeader) { + return fmt.Errorf("server.tenant.dimensions[%d].header %q must differ from system_header", i, dim.Header) + } + + headerKey := strings.ToLower(dim.Header) + if seenHeaders[headerKey] { + return fmt.Errorf("server.tenant.dimensions[%d].header %q is a duplicate", i, dim.Header) + } + seenHeaders[headerKey] = true + + if seenKeys[dim.Key] { + return fmt.Errorf("server.tenant.dimensions[%d].key %q is a duplicate", i, dim.Key) + } + seenKeys[dim.Key] = true + + return nil +} diff --git a/pkg/config/tenant_test.go b/pkg/config/tenant_test.go new file mode 100644 index 00000000..dca33c1f --- /dev/null +++ b/pkg/config/tenant_test.go @@ -0,0 +1,153 @@ +package config + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestTenantConfig_Validate(t *testing.T) { + RegisterTestingT(t) + + validDimension := TenantDimension{Header: "X-HyperFleet-Org", Key: "org", Required: true} + + cases := []struct { + name string + expectErr string + config TenantConfig + }{ + { + name: "disabled tenant requires nothing", + config: TenantConfig{Enabled: false}, + }, + { + name: "valid config with single required dimension passes", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{validDimension}, + }, + }, + { + name: "valid config with mixed required and optional dimensions passes", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{ + validDimension, + {Header: "X-HyperFleet-Project", Key: "project", Required: false}, + }, + }, + }, + { + name: "enabled with empty system header fails", + config: TenantConfig{ + Enabled: true, + Dimensions: []TenantDimension{validDimension}, + }, + expectErr: "system_header is required", + }, + { + name: "forbidden system header name fails", + config: TenantConfig{ + Enabled: true, + SystemHeader: "Authorization", + Dimensions: []TenantDimension{validDimension}, + }, + expectErr: "system_header \"Authorization\" is not allowed", + }, + { + name: "enabled with zero dimensions fails", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + }, + expectErr: "requires at least one dimension when tenant is enabled", + }, + { + name: "dimension missing header fails", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{{Key: "org", Required: true}}, + }, + expectErr: "dimensions[0].header is required", + }, + { + name: "dimension missing key fails", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{{Header: "X-HyperFleet-Org", Required: true}}, + }, + expectErr: "dimensions[0].key is required", + }, + { + name: "forbidden dimension header name fails", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{{Header: "Cookie", Key: "org", Required: true}}, + }, + expectErr: "dimensions[0].header \"Cookie\" is not allowed", + }, + { + name: "dimension header equal to system header fails", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{{Header: "x-hyperfleet-system", Key: "org", Required: true}}, + }, + expectErr: "must differ from system_header", + }, + { + name: "duplicate dimension headers fail", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{ + {Header: "X-HyperFleet-Org", Key: "org", Required: true}, + {Header: "x-hyperfleet-org", Key: "org2", Required: false}, + }, + }, + expectErr: "dimensions[1].header \"x-hyperfleet-org\" is a duplicate", + }, + { + name: "duplicate dimension keys fail", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{ + {Header: "X-HyperFleet-Org", Key: "org", Required: true}, + {Header: "X-HyperFleet-Project", Key: "org", Required: false}, + }, + }, + expectErr: "dimensions[1].key \"org\" is a duplicate", + }, + { + name: "zero required dimensions fails", + config: TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []TenantDimension{ + {Header: "X-HyperFleet-Org", Key: "org", Required: false}, + {Header: "X-HyperFleet-Project", Key: "project", Required: false}, + }, + }, + expectErr: "at least one dimension with required: true", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + err := tc.config.Validate() + if tc.expectErr != "" { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(tc.expectErr)) + } else { + Expect(err).NotTo(HaveOccurred()) + } + }) + } +} diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index b9c4c719..237a43df 100755 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -12,17 +12,18 @@ import ( // Error type URIs for RFC 9457 const ( - ErrorTypeBase = "https://api.hyperfleet.io/errors/" - ErrorTypeValidation = ErrorTypeBase + "validation-error" - ErrorTypeAuth = ErrorTypeBase + "authentication-error" - ErrorTypeNotFound = ErrorTypeBase + "not-found" - ErrorTypeConflict = ErrorTypeBase + "conflict" - ErrorTypeRateLimit = ErrorTypeBase + "rate-limit" - ErrorTypeInternal = ErrorTypeBase + "internal-error" - ErrorTypeService = ErrorTypeBase + "service-unavailable" - ErrorTypeBadRequest = ErrorTypeBase + "bad-request" - ErrorTypeMalformed = ErrorTypeBase + "malformed-request" - ErrorTypeNotImpl = ErrorTypeBase + "not-implemented" + ErrorTypeBase = "https://api.hyperfleet.io/errors/" + ErrorTypeValidation = ErrorTypeBase + "validation-error" + ErrorTypeAuth = ErrorTypeBase + "authentication-error" + ErrorTypePermissionDenied = ErrorTypeBase + "permission-denied" + ErrorTypeNotFound = ErrorTypeBase + "not-found" + ErrorTypeConflict = ErrorTypeBase + "conflict" + ErrorTypeRateLimit = ErrorTypeBase + "rate-limit" + ErrorTypeInternal = ErrorTypeBase + "internal-error" + ErrorTypeService = ErrorTypeBase + "service-unavailable" + ErrorTypeBadRequest = ErrorTypeBase + "bad-request" + ErrorTypeMalformed = ErrorTypeBase + "malformed-request" + ErrorTypeNotImpl = ErrorTypeBase + "not-implemented" ) // Error codes in HYPERFLEET-CAT-NUM format @@ -39,6 +40,9 @@ const ( CodeAuthInvalidCredentials = "HYPERFLEET-AUT-002" //nolint:gosec // Not actual credentials, just error code names CodeAuthExpiredToken = "HYPERFLEET-AUT-003" //nolint:gosec // Not actual credentials, just error code names + // Authorization errors (AUZ) - 403 + CodeAuthzPermissionDenied = "HYPERFLEET-AUZ-001" //nolint:gosec // Not actual credentials, just error code names + // Not Found errors (NTF) - 404 CodeNotFoundEndpoint = "HYPERFLEET-NTF-000" CodeNotFoundGeneric = "HYPERFLEET-NTF-001" @@ -109,6 +113,12 @@ var errorDefinitions = map[string]errorDefinition{ ErrorTypeAuth, "Invalid Token", "Invalid token provided", http.StatusUnauthorized, }, + // Authorization errors (AUZ) - 403 + CodeAuthzPermissionDenied: { + ErrorTypePermissionDenied, "Permission Denied", + "The caller's tenant identity does not grant access to this resource", http.StatusForbidden, + }, + // Validation errors (VAL) - 400 CodeValidationMultiple: { ErrorTypeValidation, "Validation Failed", "Multiple validation errors occurred", http.StatusBadRequest, @@ -375,3 +385,7 @@ func ServiceUnavailable(reason string, values ...interface{}) *ServiceError { func InvalidToken(reason string, values ...interface{}) *ServiceError { return New(CodeAuthExpiredToken, reason, values...) } + +func Forbidden(reason string, values ...interface{}) *ServiceError { + return New(CodeAuthzPermissionDenied, reason, values...) +} diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go index 611fab83..d64964a7 100755 --- a/pkg/errors/errors_test.go +++ b/pkg/errors/errors_test.go @@ -128,6 +128,14 @@ func TestConstructors(t *testing.T) { expectedType: ErrorTypeAuth, expectedReason: "token expired", }, + { + name: "Forbidden", + build: func() *ServiceError { return Forbidden("tenant header %q is missing", "X-HyperFleet-Org") }, + expectedCode: CodeAuthzPermissionDenied, + expectedHTTP: http.StatusForbidden, + expectedType: ErrorTypePermissionDenied, + expectedReason: `tenant header "X-HyperFleet-Org" is missing`, + }, } for _, tt := range tests { diff --git a/pkg/tenant/middleware.go b/pkg/tenant/middleware.go new file mode 100644 index 00000000..8a4bee50 --- /dev/null +++ b/pkg/tenant/middleware.go @@ -0,0 +1,116 @@ +package tenant + +import ( + "context" + "fmt" + "net/http" + "regexp" + "strings" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/response" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/config" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" +) + +// maxDimensionValueLen bounds tenant dimension header values to the RFC 1123 +// DNS label length limit, matching Kubernetes label-value and namespace-name +// conventions. dimensionValuePattern restricts values to a safe charset +// before they propagate into tenancy JSON and scoped queries. +const maxDimensionValueLen = 63 + +var dimensionValuePattern = regexp.MustCompile(`^[A-Za-z0-9._-]+$`) + +// Resolver resolves the caller's tenant context from trusted gateway-injected +// request headers and fails closed when required identity is missing. +type Resolver interface { + ResolveTenant(next http.Handler) http.Handler +} + +type resolver struct { + cfg config.TenantConfig +} + +var _ Resolver = &resolver{} + +// NewResolver returns a tenant enforcement Resolver configured from cfg. +// Callers decide whether to mount it based on cfg.Enabled; the resolver +// itself does not check it. +func NewResolver(cfg config.TenantConfig) Resolver { + return &resolver{cfg: cfg} +} + +// ResolveTenant reads the system header and configured dimension headers off +// the request. A system header value of exactly "true" (case-insensitive) +// grants an unscoped ResolvedTenant. Otherwise, every configured dimension +// header present on the request is collected into a tenancy map; a missing +// required dimension, or resolving zero dimensions, rejects the request with +// 403 before it reaches downstream handlers. +func (m *resolver) ResolveTenant(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if shouldSkipTenant(r.URL.Path) { + next.ServeHTTP(w, r) + return + } + + ctx := r.Context() + + if strings.EqualFold(strings.TrimSpace(r.Header.Get(m.cfg.SystemHeader)), "true") { + next.ServeHTTP(w, r.WithContext(WithTenant(ctx, &ResolvedTenant{System: true}))) + return + } + + dims, err := m.resolveDimensions(r) + if err != nil { + handleForbidden(ctx, w, r, "%s", err) + return + } + + next.ServeHTTP(w, r.WithContext(WithTenant(ctx, &ResolvedTenant{Dimensions: dims}))) + }) +} + +// resolveDimensions collects every configured dimension header present on the +// request into a tenancy map. It returns an error describing why resolution +// failed if a required dimension is missing, a value fails validation, or +// zero dimensions are resolved overall. +func (m *resolver) resolveDimensions(r *http.Request) (map[string]string, error) { + dims := make(map[string]string, len(m.cfg.Dimensions)) + for _, d := range m.cfg.Dimensions { + val := strings.TrimSpace(r.Header.Get(d.Header)) + if val == "" { + if d.Required { + return nil, fmt.Errorf("required tenant header %q is missing", d.Header) + } + continue + } + if len(val) > maxDimensionValueLen || !dimensionValuePattern.MatchString(val) { + return nil, fmt.Errorf("tenant header %q has an invalid value", d.Header) + } + dims[d.Key] = val + } + + if len(dims) == 0 { + return nil, fmt.Errorf("caller resolved no tenant dimensions") + } + + return dims, nil +} + +// handleForbidden writes a 403 problem+json response for a tenant resolution failure. +// reason and values are forwarded to errors.Forbidden, which formats them. +func handleForbidden( + ctx context.Context, w http.ResponseWriter, r *http.Request, reason string, values ...interface{}, +) { + err := errors.Forbidden(reason, values...) + logger.WithError(ctx, err).Info("Tenant identity rejected") + response.WriteServiceErrorResponse(ctx, w, r, err) +} + +// shouldSkipTenant delegates to auth.ShouldSkipCallerIdentity so openapi/errors +// paths bypass tenant enforcement the same way they bypass caller identity +// resolution, without maintaining a second copy of the path list. +func shouldSkipTenant(path string) bool { + return auth.ShouldSkipCallerIdentity(path) +} diff --git a/pkg/tenant/middleware_test.go b/pkg/tenant/middleware_test.go new file mode 100644 index 00000000..e135ae4d --- /dev/null +++ b/pkg/tenant/middleware_test.go @@ -0,0 +1,199 @@ +package tenant + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + . "github.com/onsi/gomega" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/config" +) + +func testConfig() config.TenantConfig { + return config.TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []config.TenantDimension{ + {Header: "X-HyperFleet-Org", Key: "org", Required: true}, + {Header: "X-HyperFleet-Project", Key: "project", Required: false}, + }, + } +} + +func TestResolver_ResolveTenant(t *testing.T) { + RegisterTestingT(t) + + var resolved *ResolvedTenant + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + resolved = FromContext(r.Context()) + w.WriteHeader(http.StatusOK) + }) + + systemCases := []struct { + headers map[string]string + name string + }{ + { + name: "system header alone", + headers: map[string]string{"X-HyperFleet-System": "true"}, + }, + { + name: "system header takes precedence over a present dimension header", + headers: map[string]string{ + "X-HyperFleet-System": "true", + "X-HyperFleet-Org": "acme", // present but must be ignored once system bypass triggers + }, + }, + { + name: "system header value is case-insensitive", + headers: map[string]string{"X-HyperFleet-System": "TRUE"}, + }, + } + for _, tc := range systemCases { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + resolved = nil + mw := NewResolver(testConfig()).ResolveTenant(next) + rr := serve(mw, "/api/hyperfleet/v1/clusters", tc.headers) + Expect(rr.Code).To(Equal(http.StatusOK)) + Expect(resolved).NotTo(BeNil()) + Expect(resolved.System).To(BeTrue()) + Expect(resolved.Dimensions).To(BeNil()) + }) + } + + resolveCases := []struct { + headers map[string]string + wantDims map[string]string + name string + }{ + { + name: "all dimensions present", + headers: map[string]string{ + "X-HyperFleet-Org": "acme", + "X-HyperFleet-Project": "platform", + }, + wantDims: map[string]string{"org": "acme", "project": "platform"}, + }, + { + name: "optional dimension omitted still resolves", + headers: map[string]string{"X-HyperFleet-Org": "acme"}, + wantDims: map[string]string{"org": "acme"}, + }, + { + name: "whitespace-only system header falls through to normal resolution", + headers: map[string]string{ + "X-HyperFleet-System": " ", + "X-HyperFleet-Org": "acme", + }, + wantDims: map[string]string{"org": "acme"}, + }, + { + name: "non-true system header value falls through to normal resolution", + headers: map[string]string{ + "X-HyperFleet-System": "false", // placeholder/truthy-looking value must not bypass scoping + "X-HyperFleet-Org": "acme", + }, + wantDims: map[string]string{"org": "acme"}, + }, + } + for _, tc := range resolveCases { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + resolved = nil + mw := NewResolver(testConfig()).ResolveTenant(next) + rr := serve(mw, "/api/hyperfleet/v1/clusters", tc.headers) + Expect(rr.Code).To(Equal(http.StatusOK)) + Expect(resolved).NotTo(BeNil()) + Expect(resolved.System).To(BeFalse()) + Expect(resolved.Dimensions).To(Equal(tc.wantDims)) + }) + } + + // Config with no required dimensions bypasses config.Validate() invariants but + // exercises the middleware's own zero-dimensions fail-closed check directly. + zeroDimsConfig := config.TenantConfig{ + Enabled: true, + SystemHeader: "X-HyperFleet-System", + Dimensions: []config.TenantDimension{ + {Header: "X-HyperFleet-Project", Key: "project", Required: false}, + }, + } + + forbiddenCases := []struct { + headers map[string]string + name string + cfg config.TenantConfig + }{ + { + name: "missing required dimension", + cfg: testConfig(), + headers: map[string]string{"X-HyperFleet-Project": "platform"}, + }, + { + name: "whitespace-only required dimension header", + cfg: testConfig(), + headers: map[string]string{"X-HyperFleet-Org": " "}, + }, + { + name: "dimension value with disallowed characters", + cfg: testConfig(), + headers: map[string]string{"X-HyperFleet-Org": "acme/