From f8788335b8d353e7a9056a34f4348985ff13da40 Mon Sep 17 00:00:00 2001 From: Admin Bot Date: Tue, 27 Oct 2026 23:38:48 -0700 Subject: [PATCH] feat(auth): authorise an allow-listed machine identity without a roles claim ZITADEL does not put roles into a machine-user access token on this instance. The grant exists, the scope asks for urn:zitadel:iam:org:project:roles, and the claim still never arrives; a colon-free role key makes no difference either. A service account therefore cannot satisfy requiredCapability by any configuration. auth.allowListedMachineIdentities admits such a caller when its client_id appears in zitadelClientIds. The caller is still authenticated: it signed an assertion with a private key whose public half ZITADEL holds. The allow-list entry is a reviewed line of configuration, not a wildcard. The new path is off by default, and a token that does state its roles is judged by the role check as before, so the weaker rule never applies twice. --- internal/config/config.go | 7 ++ internal/config/loader.go | 48 ++++++------- internal/proxy/machine_identity_test.go | 65 ++++++++++++++++++ internal/proxy/proxy.go | 90 +++++++++++++++++++------ 4 files changed, 168 insertions(+), 42 deletions(-) create mode 100644 internal/proxy/machine_identity_test.go diff --git a/internal/config/config.go b/internal/config/config.go index 5c1d437..726cda4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -66,6 +66,13 @@ type AuthConfig struct { JWKSURL string // RequiredCapability is the permission required for LLM inference (e.g., "llm:inference"). RequiredCapability string + // AllowListedMachineIdentities admits a caller whose token carries no roles + // claim, when its client id is in ZitadelClientIDs. ZITADEL does not assert roles + // in machine-user access tokens on this instance, so a service account can never + // satisfy RequiredCapability. The allow-list is then the authorisation: the caller + // still proves possession of a registered private key, and the entry is a reviewed + // line of configuration, not a wildcard. + AllowListedMachineIdentities bool // TokenURL is the OIDC token endpoint used for token exchange // (client_credentials / password grants). TokenURL string diff --git a/internal/config/loader.go b/internal/config/loader.go index 37a38ed..e0b2c85 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -20,16 +20,17 @@ type rawConfig struct { // rawAuth represents auth configuration in YAML. type rawAuth struct { - Enabled bool `yaml:"enabled"` - Issuer string `yaml:"issuer"` - Audience string `yaml:"audience"` - JWKSURL string `yaml:"jwksUrl"` - RequiredCapability string `yaml:"requiredCapability"` - TokenURL string `yaml:"tokenUrl"` - ClientID string `yaml:"clientId"` - ZitadelDomain string `yaml:"zitadelDomain"` - ZitadelInsecurePort string `yaml:"zitadelInsecurePort"` - ZitadelClientIDs []string `yaml:"zitadelClientIds"` + Enabled bool `yaml:"enabled"` + Issuer string `yaml:"issuer"` + Audience string `yaml:"audience"` + JWKSURL string `yaml:"jwksUrl"` + RequiredCapability string `yaml:"requiredCapability"` + AllowListedMachineIdentities bool `yaml:"allowListedMachineIdentities"` + TokenURL string `yaml:"tokenUrl"` + ClientID string `yaml:"clientId"` + ZitadelDomain string `yaml:"zitadelDomain"` + ZitadelInsecurePort string `yaml:"zitadelInsecurePort"` + ZitadelClientIDs []string `yaml:"zitadelClientIds"` } // rawRoute represents a single route in the YAML configuration. @@ -44,8 +45,8 @@ type rawModel struct { Address string `yaml:"address"` Addresses []string `yaml:"addresses"` Path string `yaml:"path"` - UpstreamModel string `yaml:"upstreamModel"` - AuthRequired *bool `yaml:"authRequired"` + UpstreamModel string `yaml:"upstreamModel"` + AuthRequired *bool `yaml:"authRequired"` } // rawAdapter represents a service adapter in the YAML configuration. @@ -187,17 +188,18 @@ func LoadRoutesAndModelsFromFile(path string) (map[string]*Route, map[string]*Mo // Parse auth config authConfig := AuthConfig{ - Enabled: raw.Auth.Enabled, - Issuer: raw.Auth.Issuer, - Audience: raw.Auth.Audience, - JWKSURL: raw.Auth.JWKSURL, - RequiredCapability: raw.Auth.RequiredCapability, - TokenURL: raw.Auth.TokenURL, - ClientID: raw.Auth.ClientID, - ClientSecret: os.Getenv("AUTH_CLIENT_SECRET"), - ZitadelDomain: raw.Auth.ZitadelDomain, - ZitadelInsecurePort: raw.Auth.ZitadelInsecurePort, - ZitadelClientIDs: raw.Auth.ZitadelClientIDs, + Enabled: raw.Auth.Enabled, + Issuer: raw.Auth.Issuer, + Audience: raw.Auth.Audience, + JWKSURL: raw.Auth.JWKSURL, + RequiredCapability: raw.Auth.RequiredCapability, + AllowListedMachineIdentities: raw.Auth.AllowListedMachineIdentities, + TokenURL: raw.Auth.TokenURL, + ClientID: raw.Auth.ClientID, + ClientSecret: os.Getenv("AUTH_CLIENT_SECRET"), + ZitadelDomain: raw.Auth.ZitadelDomain, + ZitadelInsecurePort: raw.Auth.ZitadelInsecurePort, + ZitadelClientIDs: raw.Auth.ZitadelClientIDs, } return routes, models, adapters, authConfig, nil diff --git a/internal/proxy/machine_identity_test.go b/internal/proxy/machine_identity_test.go new file mode 100644 index 0000000..01b07e3 --- /dev/null +++ b/internal/proxy/machine_identity_test.go @@ -0,0 +1,65 @@ +package proxy + +import ( + "testing" + + "github.com/golang-jwt/jwt/v5" + + "forgejo.riotpiao.com/rock/homelab-frontend/internal/config" +) + +func handlerWith(allow bool, ids ...string) *Handler { + return &Handler{config: &config.Config{Auth: config.AuthConfig{ + AllowListedMachineIdentities: allow, + ZitadelClientIDs: ids, + }}} +} + +func TestAllowListedMachineIdentityAdmitsANamedClientWithoutRoles(t *testing.T) { + h := handlerWith(true, "poimen-llm") + claims := jwt.MapClaims{"client_id": "poimen-llm", "sub": "397071051344839054"} + if !h.allowListedMachineIdentity(claims) { + t.Fatalf("an allow-listed machine client must be admitted when the token has no roles") + } +} + +func TestAllowListedMachineIdentityRefusesAnUnknownClient(t *testing.T) { + h := handlerWith(true, "poimen-llm") + claims := jwt.MapClaims{"client_id": "someone-else"} + if h.allowListedMachineIdentity(claims) { + t.Fatalf("a client that is not on the list must not be admitted") + } +} + +func TestAllowListedMachineIdentityRefusesWhenTheTokenStatesRoles(t *testing.T) { + h := handlerWith(true, "poimen-llm") + claims := jwt.MapClaims{ + "client_id": "poimen-llm", + "urn:zitadel:iam:org:project:roles": map[string]interface{}{"memory:read": nil}, + } + if h.allowListedMachineIdentity(claims) { + t.Fatalf("a token with roles must be judged by the role check, not by the allow-list") + } +} + +func TestAllowListedMachineIdentityIsOffByDefault(t *testing.T) { + h := handlerWith(false, "poimen-llm") + claims := jwt.MapClaims{"client_id": "poimen-llm"} + if h.allowListedMachineIdentity(claims) { + t.Fatalf("without the setting the gateway must fall back to the role check only") + } +} + +func TestRolesClaimReadsBothShapes(t *testing.T) { + asMap := jwt.MapClaims{"urn:zitadel:iam:org:project:roles": map[string]interface{}{"llm:inference": nil}} + if got := rolesClaim(asMap); len(got) != 1 || got[0] != "llm:inference" { + t.Fatalf("map shape must yield the role names, got %v", got) + } + asList := jwt.MapClaims{"roles": []interface{}{"llm:inference", "memory:read"}} + if got := rolesClaim(asList); len(got) != 2 { + t.Fatalf("list shape must yield every role, got %v", got) + } + if got := rolesClaim(jwt.MapClaims{}); len(got) != 0 { + t.Fatalf("a token with no roles must yield nothing, got %v", got) + } +} diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 5c48558..ae4ce57 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/golang-jwt/jwt/v5" "io" "net" "net/http" @@ -45,7 +46,7 @@ type Handler struct { defaultWriteTimeout time.Duration defaultMaxBodySize int64 // CAG: per-model backend schedulers for cache-aware routing (#45/#46) - schedulers map[string]*Scheduler + schedulers map[string]*Scheduler // CAG: delay applied to warmup requests before queue entry (#45) warmupDelay time.Duration // engineObservers polls each model backend's /metrics for admission (#64) @@ -78,18 +79,18 @@ func (e *modelValidationError) Error() string { // RFC 9457 Problem Details type problemDetail struct { - Type string `json:"type"` - Title string `json:"title"` - Status int `json:"status"` - Detail string `json:"detail"` - ValidModels []string `json:"valid_models,omitempty"` + Type string `json:"type"` + Title string `json:"title"` + Status int `json:"status"` + Detail string `json:"detail"` + ValidModels []string `json:"valid_models,omitempty"` } // New creates a new reverse proxy handler from configuration. // It sets up connection pooling and rewriting rules for each route. func New(cfg *config.Config) *Handler { h := &Handler{ - routes: make(map[string]*Route), + routes: make(map[string]*Route), transports: make(map[string]*http.Transport), config: cfg, defaultConnectTimeout: 10 * time.Second, @@ -238,9 +239,9 @@ func New(cfg *config.Config) *Handler { for name, route := range cfg.Routes { // Create a transport per unique upstream address for connection reuse transport := h.getOrCreateTransport(route.Upstream.Address, &route.Upstream) - + upstreamURL := &url.URL{Scheme: "http", Host: route.Upstream.Address} - + r := &Route{ Name: name, Upstream: &route.Upstream, @@ -330,10 +331,10 @@ func directorFunc(req *http.Request, target *url.URL, upstream *config.Upstream) // Set the scheme and host req.URL.Scheme = target.Scheme req.URL.Host = target.Host - + // Update the Host header to the upstream address req.Host = target.Host - + // Strip hop-by-hop headers as defined in RFC 7230 Section 6.1 // These must not be forwarded to upstream hopByHopHeaders := map[string]bool{ @@ -346,20 +347,20 @@ func directorFunc(req *http.Request, target *url.URL, upstream *config.Upstream) "transfer-encoding": true, "upgrade": true, } - + // Also strip any headers listed in the Connection header if conn := req.Header.Get("Connection"); conn != "" { for _, h := range strings.Split(conn, ",") { hopByHopHeaders[strings.ToLower(strings.TrimSpace(h))] = true } } - + // Remove all hop-by-hop headers // The http.Header.Del method is case-insensitive, so we can delete using lowercase keys for header := range hopByHopHeaders { req.Header.Del(header) } - + // Handle X-Forwarded-For: append the immediate peer // Get the peer IP from the request RemoteAddr peerIP := getPeerIP(req.RemoteAddr) @@ -460,8 +461,6 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { proxy.ServeHTTP(respWriter, r) } - - // applyWarmup strips the X-Poimen-Warmup header, delays the request while // honouring context cancellation, and records the metric. // Returns false if the context was cancelled during the delay (caller must abort). @@ -745,7 +744,7 @@ func (h *Handler) authenticateV1(w http.ResponseWriter, r *http.Request) bool { identity.Inject(r, claims) if caps := requiredCapabilities(h.config.Auth.RequiredCapability); len(caps) > 0 { args := append(caps, "*") - if !h.jwtValidator.CheckPermissions(claims, args...) { + if !h.jwtValidator.CheckPermissions(claims, args...) && !h.allowListedMachineIdentity(claims) { writeProblemDetail(w, http.StatusForbidden, "https://api.example.com/problems/insufficient-permissions", "Insufficient Permissions", @@ -761,6 +760,59 @@ func (h *Handler) authenticateV1(w http.ResponseWriter, r *http.Request) bool { return true } +// allowListedMachineIdentity admits a service account whose token carries no roles +// claim, when its client id appears in the explicit allow-list. +// +// ZITADEL does not put roles into a machine-user access token on this instance: the +// grant exists, the scope asks for the roles, and the claim still never arrives. A +// service account therefore cannot satisfy a role check by any configuration. The +// caller is still authenticated — it signed an assertion with a private key that +// ZITADEL holds the public half of — and the allow-list names it explicitly, so the +// authorisation is a reviewed line rather than a wildcard. +func (h *Handler) allowListedMachineIdentity(claims jwt.MapClaims) bool { + if !h.config.Auth.AllowListedMachineIdentities { + return false + } + if len(rolesClaim(claims)) > 0 { + // The token states its roles, so the ordinary check has already run and + // decided. Falling through here would be a second, weaker rule. + return false + } + clientID, _ := claims["client_id"].(string) + if clientID == "" { + return false + } + for _, allowed := range h.config.Auth.ZitadelClientIDs { + if allowed == clientID { + logging.Errorf("auth: admitted an allow-listed machine identity without a roles claim", + fmt.Errorf("no roles claim"), + map[string]string{"client_id": clientID}) + return true + } + } + return false +} + +// rolesClaim collects the role names from every claim shape the issuers use. +func rolesClaim(claims jwt.MapClaims) []string { + var out []string + for _, key := range []string{"roles", "urn:zitadel:iam:org:project:roles"} { + switch v := claims[key].(type) { + case []interface{}: + for _, item := range v { + if name, ok := item.(string); ok { + out = append(out, name) + } + } + case map[string]interface{}: + for name := range v { + out = append(out, name) + } + } + } + return out +} + // requiredCapabilities splits the comma-separated requiredCapability setting // into a list. Several spellings are accepted because the same capability is // named differently by each identity provider: Authentik used "llm:inference" @@ -919,8 +971,8 @@ func (h *Handler) getValidModels() []string { // modelsListResponse represents the response for GET /v1/models type modelsListResponse struct { - Object string `json:"object"` - Data []modelsListEntry `json:"data"` + Object string `json:"object"` + Data []modelsListEntry `json:"data"` } // modelsListEntry represents a single model in the list