diff --git a/authbridge/CLAUDE.md b/authbridge/CLAUDE.md index 362d3d0b4..a9506c403 100644 --- a/authbridge/CLAUDE.md +++ b/authbridge/CLAUDE.md @@ -111,25 +111,24 @@ with protocol-specific listeners in `cmd/authbridge/listener/`: - Routes file is loaded once at startup; restart the pod to pick up changes **Configuration loading:** -- YAML config with `${ENV_VAR}` expansion, mode presets, and startup validation -- Supports `keycloak_url` + `keycloak_realm` derivation for operator compatibility -- Waits up to 60s for credential files from client-registration -- Reads `CLIENT_ID` from `/shared/client-id.txt` (file) or `CLIENT_ID` env var (fallback) -- Reads `CLIENT_SECRET` from `/shared/client-secret.txt` (file) or `CLIENT_SECRET` env var (fallback) -- `TOKEN_URL`: explicit env var, or auto-derived from `KEYCLOAK_URL` + `KEYCLOAK_REALM` (i.e. `{KEYCLOAK_URL}/realms/{KEYCLOAK_REALM}/protocol/openid-connect/token`) -- `ISSUER`: explicit env var, or auto-derived from `KEYCLOAK_URL` + `KEYCLOAK_REALM` (i.e. `{KEYCLOAK_URL}/realms/{KEYCLOAK_REALM}`) -- Inbound audience validation uses `CLIENT_ID` (from `/shared/client-id.txt` or `CLIENT_ID` env var) -- automatic, per-agent -- Outbound route config from `/etc/authproxy/routes.yaml` (default; override with `ROUTES_CONFIG_PATH` env var in standalone deployments). Target audience and scopes are configured per-route only. -- Default outbound policy from `DEFAULT_OUTBOUND_POLICY` env var: `"passthrough"` (default) or `"exchange"` -- JWKS URL is derived from TOKEN_URL: replaces `/token` suffix with `/certs` +- YAML config with `${ENV_VAR}` expansion, mode presets, and startup validation. +- Plugin settings are local to each plugin under `pipeline.*.plugins[].config`; the runtime YAML itself only carries `mode`, `listener`, `session`, `stats`, and the pipeline composition. See [`cmd/authbridge/README.md`](cmd/authbridge/README.md) for the per-mode YAML shape and [`authlib/plugins/CONVENTIONS.md`](authlib/plugins/CONVENTIONS.md) for the per-plugin decode pattern. +- The operator-supplied env vars (`KEYCLOAK_URL`, `KEYCLOAK_REALM`, `TOKEN_URL`, `ISSUER`, `DEFAULT_OUTBOUND_POLICY`, `CLIENT_ID`) are consumed by the default `authbridge-combined.yaml` via `${VAR}` expansion — they land inside the appropriate plugin's `config:` block rather than a top-level section. +- `jwt-validation` derives `jwks_url` from `issuer` when omitted (appends `/protocol/openid-connect/certs`). +- `token-exchange` derives `token_url` from `keycloak_url + keycloak_realm` when omitted (Keycloak convention). +- Credential files: `client-registration` writes `/shared/client-id.txt` and `/shared/client-secret.txt`; `spiffe-helper` writes `/opt/jwt_svid.token`. `jwt-validation` reads the audience from `/shared/client-id.txt` via `audience_file`; `token-exchange` reads client credentials via `client_id_file` / `client_secret_file` / `jwt_svid_path`. Each plugin attempts a synchronous read at Configure time and falls back to a background poll from its `Init` goroutine if the file isn't yet readable. +- Outbound route config: `token-exchange` reads `/etc/authproxy/routes.yaml` by default (path is per-plugin, configured via `routes.file` in its config block); inline rules can be declared under `routes.rules`. +- Outbound `default_policy`: `passthrough` (default) or `exchange`, configured per-plugin (no top-level `DEFAULT_OUTBOUND_POLICY` field anymore; the env var is still expanded into the plugin config by `authbridge-combined.yaml`). **Key library packages (authlib/):** -- `authlib/validation/` -- JWKS-backed JWT verifier -- `authlib/exchange/` -- RFC 8693 token exchange client +- `authlib/validation/` -- JWKS-backed JWT verifier (used internally by `jwt-validation` plugin) +- `authlib/exchange/` -- RFC 8693 token exchange client (used internally by `token-exchange` plugin) - `authlib/cache/` -- SHA-256 keyed token cache -- `authlib/routing/` -- Host-to-audience route resolver -- `authlib/auth/` -- `HandleInbound` + `HandleOutbound` composition -- `authlib/config/` -- Mode presets, YAML config, validation +- `authlib/routing/` -- Host-to-audience route resolver (used internally by `token-exchange` plugin) +- `authlib/auth/` -- `HandleInbound` + `HandleOutbound` composition; each plugin instance constructs its own `auth.Auth` from its own local config +- `authlib/config/` -- Mode presets, YAML config loader, credential-file waiters, top-level (mode + listener + session) validation +- `authlib/pipeline/` -- Plugin interface + lifecycle (`Configurable`, `Initializer`, `Shutdowner`); see [`authlib/pipeline/README.md`](authlib/pipeline/README.md) +- `authlib/plugins/` -- The concrete plugins + registry; see [`authlib/plugins/CONVENTIONS.md`](authlib/plugins/CONVENTIONS.md) for the per-plugin config convention ### init-iptables.sh @@ -396,7 +395,7 @@ Set `session.enabled: false` in the runtime config to turn off the store (and im ## Gotchas and Known Issues -1. **Credential file race condition**: Authbridge waits up to 60s for `/shared/client-id.txt` and `/shared/client-secret.txt`. If client-registration takes longer (e.g., Keycloak slow to start), authbridge will fall back to env vars which may be empty. +1. **Credential file race condition**: Each plugin that reads a credential file (jwt-validation's `audience_file`, token-exchange's `client_id_file` / `client_secret_file` / `jwt_svid_path`) tries a synchronous read at Configure time and, on miss, spawns an Init goroutine that polls indefinitely — emitting a WARN every ~60s while the file is still missing. OnRequest returns 503 until the file arrives. If the file never shows up (wrong path, missing volume mount), the pod stays unready for outbound traffic; follow the WARN lines to the misconfigured path. 2. **ISSUER vs TOKEN_URL**: `ISSUER` must be the Keycloak **frontend URL** (what appears in the `iss` claim of tokens), while `TOKEN_URL` is the **internal service URL**. These are often different in Kubernetes (e.g., `http://keycloak.localtest.me:8080` vs `http://keycloak-service.keycloak.svc:8080`). diff --git a/authbridge/authlib/README.md b/authbridge/authlib/README.md index d8c16ca3c..699a7838b 100644 --- a/authbridge/authlib/README.md +++ b/authbridge/authlib/README.md @@ -12,32 +12,39 @@ A pure Go library providing reusable building blocks for JWT validation, OAuth 2 | `bypass/` | Path pattern matcher for public endpoints (health, agent card) | | `spiffe/` | SPIFFE credential sources (file-based JWT-SVID) | | `routing/` | Host-to-audience router with glob pattern matching | -| `auth/` | Composition layer: `HandleInbound` + `HandleOutbound` | -| `config/` | YAML config, mode presets, startup validation, URL derivation | -| `pipeline/` | Plugin pipeline — see [pipeline/README.md](./pipeline/README.md) for the full interface spec and bridge-plugin integration guide | +| `auth/` | Composition layer: `HandleInbound` + `HandleOutbound` — used internally by `jwt-validation` and `token-exchange` plugins | +| `config/` | YAML config loader, mode presets, credential-file waiters, top-level validation | +| `pipeline/` | Plugin pipeline + lifecycle (`Configurable`, `Initializer`, `Shutdowner`) — see [pipeline/README.md](./pipeline/README.md) | | `session/` | In-memory session store + `SessionSummary` aggregation, backing the `:9094` API | | `sessionapi/` | HTTP API (`/v1/sessions`, `/v1/events` SSE, `/v1/pipeline`) exposing the session store | -| `plugins/` | Built-in protocol parsers: `a2a-parser`, `mcp-parser`, `inference-parser`, `jwt-validation` | +| `plugins/` | Built-in plugins: `jwt-validation`, `token-exchange`, `a2a-parser`, `mcp-parser`, `inference-parser`. See [plugins/CONVENTIONS.md](./plugins/CONVENTIONS.md) for the per-plugin config convention | | `observe/` | OTEL + metrics helpers | ## Usage +Plugins own their own configuration and construct any `auth.Auth` they need internally from their per-plugin config (see [plugins/CONVENTIONS.md](./plugins/CONVENTIONS.md)). Host processes (cmd/authbridge) just load the YAML, call `plugins.Build`, run the pipelines, and let each plugin handle its own dependencies. + ```go import ( - "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" ) -// Load and resolve config cfg, _ := config.Load("config.yaml") -resolved, _ := config.Resolve(ctx, cfg) -handler := auth.New(*resolved) +config.ApplyPreset(cfg) +_ = config.Validate(cfg) // mode + listener combo only; plugins self-validate + +inbound, _ := plugins.Build(cfg.Pipeline.Inbound.Plugins) +outbound, _ := plugins.Build(cfg.Pipeline.Outbound.Plugins) -// Handle requests -inResult := handler.HandleInbound(ctx, authHeader, path, "") -outResult := handler.HandleOutbound(ctx, authHeader, host) +_ = inbound.Start(ctx) // invokes Init on plugins that implement pipeline.Initializer +_ = outbound.Start(ctx) + +// Listeners drive the pipelines — see cmd/authbridge/listener/* ``` +For direct use of the `auth/` composition layer (outside of plugins), see `auth/auth.go` — `auth.New(cfg)` takes an `auth.Config` containing the specific building blocks a caller needs. + ## Go Module ``` diff --git a/authbridge/authlib/auth/auth.go b/authbridge/authlib/auth/auth.go index f87866759..a5cc6d39b 100644 --- a/authbridge/authlib/auth/auth.go +++ b/authbridge/authlib/auth/auth.go @@ -521,6 +521,41 @@ func NewStats() *Stats { } } +// MergeStats returns a new Stats whose counters are the sum of all +// srcs. Each src's mutex is taken independently; the returned Stats +// has its own storage and no relationship to the sources. Safe to +// call concurrently — sources are only read under their own locks. +// +// Used by the /stats aggregator to fold per-plugin counters into a +// single response per HTTP request without leaking plugin-local +// Stats instances into the presentation layer. +func MergeStats(srcs ...*Stats) *Stats { + out := NewStats() + for _, s := range srcs { + if s == nil { + continue + } + s.mu.Lock() + for k, v := range s.inboundApprovals { + out.inboundApprovals[k] += v + } + for k, v := range s.inboundDenials { + out.inboundDenials[k] += v + } + for k, v := range s.outboundApprovals { + out.outboundApprovals[k] += v + } + for k, v := range s.outboundDenials { + out.outboundDenials[k] += v + } + for k, v := range s.outboundReplaceTokens { + out.outboundReplaceTokens[k] += v + } + s.mu.Unlock() + } + return out +} + // IncInboundApprove records a new approval (for statistics) func (a *Auth) IncInboundApprove(reason InboundApprovalReason) { a.Stats.mu.Lock() diff --git a/authbridge/authlib/auth/auth_test.go b/authbridge/authlib/auth/auth_test.go index a6ce48ae7..7e14e6ee3 100644 --- a/authbridge/authlib/auth/auth_test.go +++ b/authbridge/authlib/auth/auth_test.go @@ -453,6 +453,56 @@ func TestIncOutboundReplaceToken(t *testing.T) { } } +// MergeStats sums counters from multiple Stats objects into a fresh +// one. Each source's mutex is taken independently — no deadlock even +// if sources are being mutated concurrently. +func TestMergeStats_SumsCounters(t *testing.T) { + a := New(Config{}) + a.IncInboundApprove(APPROVE_AUTHORIZED) + a.IncInboundApprove(APPROVE_AUTHORIZED) + a.IncInboundDeny(DENY_NO_HEADER) + + b := New(Config{}) + b.IncInboundApprove(APPROVE_AUTHORIZED) // +1, total 3 + b.IncInboundApprove(APPROVE_PASSTHROUGH) + b.IncOutboundApprove(OUTBOUND_PASSTHROUGH) + + merged := MergeStats(a.Stats, b.Stats) + if got, want := merged.inboundApprovals[APPROVE_AUTHORIZED], 3; got != want { + t.Errorf("APPROVE_AUTHORIZED = %d, want %d", got, want) + } + if got, want := merged.inboundApprovals[APPROVE_PASSTHROUGH], 1; got != want { + t.Errorf("APPROVE_PASSTHROUGH = %d, want %d", got, want) + } + if got, want := merged.inboundDenials[DENY_NO_HEADER], 1; got != want { + t.Errorf("DENY_NO_HEADER = %d, want %d", got, want) + } + if got, want := merged.outboundApprovals[OUTBOUND_PASSTHROUGH], 1; got != want { + t.Errorf("OUTBOUND_PASSTHROUGH = %d, want %d", got, want) + } + + // Merging must not mutate source counters — the aggregator runs + // on every /stats probe, and we can't have each probe double- + // counting. + if got, want := a.Stats.inboundApprovals[APPROVE_AUTHORIZED], 2; got != want { + t.Errorf("source a mutated: APPROVE_AUTHORIZED = %d, want %d", got, want) + } +} + +func TestMergeStats_TolerantOfNilSources(t *testing.T) { + merged := MergeStats(nil, nil) + if merged == nil { + t.Fatal("MergeStats(nil, nil) returned nil") + } +} + +func TestMergeStats_Empty(t *testing.T) { + merged := MergeStats() + if merged == nil { + t.Fatal("MergeStats() returned nil") + } +} + func TestStatsMarshalJSON_Empty(t *testing.T) { s := NewStats() data, err := json.Marshal(s) diff --git a/authbridge/authlib/config/config.go b/authbridge/authlib/config/config.go index 5d98d7a0b..c13a39bb3 100644 --- a/authbridge/authlib/config/config.go +++ b/authbridge/authlib/config/config.go @@ -3,6 +3,7 @@ package config import ( + "encoding/json" "fmt" "os" @@ -10,14 +11,15 @@ import ( ) // Config is the top-level AuthBridge configuration. +// +// Plugin-specific settings (inbound JWT validation, outbound token +// exchange, identity, bypass paths, routes) live inside their +// respective entries under Pipeline.* now — see the plugin README at +// authbridge/authlib/plugins/CONVENTIONS.md for how each plugin +// declares its own config schema and defaults. type Config struct { Mode string `yaml:"mode" json:"mode"` // "envoy-sidecar", "waypoint", "proxy-sidecar" - Inbound InboundConfig `yaml:"inbound" json:"inbound"` - Outbound OutboundConfig `yaml:"outbound" json:"outbound"` - Identity IdentityConfig `yaml:"identity" json:"identity"` Listener ListenerConfig `yaml:"listener" json:"listener"` - Bypass BypassConfig `yaml:"bypass" json:"bypass"` - Routes RoutesConfig `yaml:"routes" json:"routes"` Pipeline PipelineConfig `yaml:"pipeline" json:"pipeline"` Session SessionConfig `yaml:"session" json:"session"` Stats StatsConfig `yaml:"stats" json:"stats"` @@ -48,9 +50,10 @@ func (s SessionConfig) SessionEnabled() bool { return *s.Enabled } -// PipelineConfig holds the plugin pipeline composition. -// If omitted (empty), default pipelines are used: -// inbound=[jwt-validation], outbound=[token-exchange]. +// PipelineConfig holds the plugin pipeline composition. Required: +// the runtime YAML must populate both inbound and outbound lists, or +// plugins.Build will produce empty pipelines and the listener will +// have nothing to invoke. There are no implicit defaults. type PipelineConfig struct { Inbound PipelineStageConfig `yaml:"inbound" json:"inbound"` Outbound PipelineStageConfig `yaml:"outbound" json:"outbound"` @@ -58,34 +61,121 @@ type PipelineConfig struct { // PipelineStageConfig lists the plugins for a pipeline stage in execution order. type PipelineStageConfig struct { - Plugins []string `yaml:"plugins" json:"plugins"` + Plugins []PluginEntry `yaml:"plugins" json:"plugins"` } -// InboundConfig holds JWT validation settings. -type InboundConfig struct { - JWKSURL string `yaml:"jwks_url" json:"jwks_url"` - Issuer string `yaml:"issuer" json:"issuer"` +// PluginEntry names a plugin and optionally carries per-instance config. +// +// The YAML accepts both the bare-name form ("jwt-validation") and the +// full form ({name, id, config}). The short form keeps existing pipeline +// configs parsing unchanged; the long form is what plugins that +// implement pipeline.Configurable actually need. See +// authbridge/authlib/plugins/CONVENTIONS.md for the convention plugins +// follow when decoding Config. +// +// Config is captured as a raw subtree via json.RawMessage so the plugin +// can do its own DisallowUnknownFields decode against a typed struct — +// the framework does not interpret it. +type PluginEntry struct { + Name string `yaml:"name" json:"name"` + ID string `yaml:"id,omitempty" json:"id,omitempty"` + Config json.RawMessage `yaml:"-" json:"config,omitempty"` } -// OutboundConfig holds token exchange settings. -type OutboundConfig struct { - TokenURL string `yaml:"token_url" json:"token_url"` - KeycloakURL string `yaml:"keycloak_url" json:"keycloak_url"` // alternative: derives token_url and issuer - KeycloakRealm string `yaml:"keycloak_realm" json:"keycloak_realm"` // used with keycloak_url - DefaultPolicy string `yaml:"default_policy" json:"default_policy"` // "exchange" or "passthrough" +// UnmarshalYAML accepts either a bare string or a map. The string form +// is equivalent to {name: } with no config. +func (p *PluginEntry) UnmarshalYAML(node *yaml.Node) error { + switch node.Kind { + case yaml.ScalarNode: + p.Name = node.Value + return nil + case yaml.MappingNode: + // Walk the mapping's Content pairs directly so we can preserve + // the config subtree as raw bytes. yaml.v3's struct decode into + // a *yaml.Node field produces nil in this version; iterating + // Content is the reliable path. + for i := 0; i+1 < len(node.Content); i += 2 { + key, val := node.Content[i], node.Content[i+1] + if key.Kind != yaml.ScalarNode { + return fmt.Errorf("plugin entry: non-scalar key %q", key.Value) + } + switch key.Value { + case "name": + if err := val.Decode(&p.Name); err != nil { + return fmt.Errorf("plugin entry name: %w", err) + } + case "id": + if err := val.Decode(&p.ID); err != nil { + return fmt.Errorf("plugin entry id: %w", err) + } + case "config": + // Explicit `config: null` (or `config:` with no value) + // decodes to a null-tagged scalar node. Normalize to + // nil here — otherwise yamlNodeToJSON would emit the + // literal bytes "null" and the Build-time "plugin does + // not accept configuration" gate would fire + // spuriously on non-Configurable plugins that a user + // explicitly declared with a null config block. + if val.Kind == yaml.ScalarNode && val.Tag == "!!null" { + p.Config = nil + continue + } + raw, err := yamlNodeToJSON(val) + if err != nil { + return fmt.Errorf("plugin %q config: %w", p.Name, err) + } + p.Config = raw + default: + return fmt.Errorf("plugin entry: unknown field %q", key.Value) + } + } + return nil + default: + return fmt.Errorf("plugin entry: expected string or map, got kind %d", node.Kind) + } } -// IdentityConfig holds agent identity and credentials. -type IdentityConfig struct { - Type string `yaml:"type" json:"type"` // "spiffe", "client-secret", "k8s-sa" - ClientID string `yaml:"client_id" json:"client_id"` - ClientSecret string `yaml:"client_secret" json:"client_secret"` - ClientIDFile string `yaml:"client_id_file" json:"client_id_file"` // alternative: read client_id from file - ClientSecretFile string `yaml:"client_secret_file" json:"client_secret_file"` // alternative: read client_secret from file - SocketPath string `yaml:"socket_path" json:"socket_path"` // SPIFFE Workload API - JWTSVIDPath string `yaml:"jwt_svid_path" json:"jwt_svid_path"` // file-based SPIFFE - JWTAudience []string `yaml:"jwt_audience" json:"jwt_audience"` // SPIFFE JWT audience - CredentialWaitTimeout string `yaml:"credential_wait_timeout" json:"credential_wait_timeout"` // initial fast-poll duration, e.g. "120s" +// yamlNodeToJSON converts a YAML node to JSON bytes by round-tripping +// through a generic Go value. Sufficient for config sub-trees, which +// only contain scalars, sequences, and maps. +func yamlNodeToJSON(n *yaml.Node) ([]byte, error) { + var v any + if err := n.Decode(&v); err != nil { + return nil, err + } + return json.Marshal(normalizeYAMLMaps(v)) +} + +// normalizeYAMLMaps converts map[any]any (which yaml.v3 can produce when +// decoding into an untyped `any`) into map[string]any so json.Marshal +// accepts it. YAML allows non-string keys but config files never use them. +func normalizeYAMLMaps(v any) any { + switch x := v.(type) { + case map[any]any: + out := make(map[string]any, len(x)) + for k, val := range x { + ks, ok := k.(string) + if !ok { + ks = fmt.Sprintf("%v", k) + } + out[ks] = normalizeYAMLMaps(val) + } + return out + case map[string]any: + out := make(map[string]any, len(x)) + for k, val := range x { + out[k] = normalizeYAMLMaps(val) + } + return out + case []any: + out := make([]any, len(x)) + for i, val := range x { + out[i] = normalizeYAMLMaps(val) + } + return out + default: + return v + } } // ListenerConfig holds per-mode listener addresses. @@ -102,30 +192,6 @@ type ListenerConfig struct { SessionAPIAddr string `yaml:"session_api_addr" json:"session_api_addr"` } -// BypassConfig holds path patterns that skip inbound JWT validation. -type BypassConfig struct { - InboundPaths []string `yaml:"inbound_paths" json:"inbound_paths"` -} - -// RoutesConfig holds outbound routing rules. -type RoutesConfig struct { - File string `yaml:"file" json:"file"` // path to routes YAML file - Rules []RouteConfig `yaml:"rules" json:"rules"` // inline rules (alternative to file) -} - -// RouteConfig is the YAML representation of an outbound route. -// Supports both legacy `passthrough: true` and new `action: passthrough` formats. -// Note that the JSON doesn't omitempty because we want to make it obvious -// to human readers which fields are empty. -type RouteConfig struct { - Host string `yaml:"host" json:"host"` - TargetAudience string `yaml:"target_audience,omitempty" json:"target_audience"` - TokenScopes string `yaml:"token_scopes,omitempty" json:"token_scopes"` - TokenURL string `yaml:"token_url,omitempty" json:"token_url"` - Passthrough bool `yaml:"passthrough,omitempty" json:"passthrough"` // legacy format - Action string `yaml:"action,omitempty" json:"action"` // "exchange" or "passthrough" -} - // StatsConfig represents the configuration for reporting config and statistics type StatsConfig struct { StatsAddress string `yaml:"address" json:"address"` // for example, ":9093" diff --git a/authbridge/authlib/config/config_test.go b/authbridge/authlib/config/config_test.go index 058388de2..5599aff5c 100644 --- a/authbridge/authlib/config/config_test.go +++ b/authbridge/authlib/config/config_test.go @@ -1,8 +1,11 @@ package config import ( + "context" + "encoding/json" "os" "path/filepath" + "strings" "testing" "time" ) @@ -12,67 +15,44 @@ import ( func TestApplyPreset_EnvoySidecar(t *testing.T) { cfg := &Config{Mode: ModeEnvoySidecar} ApplyPreset(cfg) - if cfg.Identity.Type != "spiffe" { - t.Errorf("identity.type = %q, want spiffe", cfg.Identity.Type) - } - if cfg.Outbound.DefaultPolicy != "passthrough" { - t.Errorf("default_policy = %q, want passthrough", cfg.Outbound.DefaultPolicy) - } if cfg.Listener.ExtProcAddr != ":9090" { t.Errorf("ext_proc_addr = %q, want :9090", cfg.Listener.ExtProcAddr) } - if len(cfg.Bypass.InboundPaths) == 0 { - t.Error("expected default bypass paths") + if cfg.Listener.SessionAPIAddr != ":9094" { + t.Errorf("session_api_addr = %q, want :9094", cfg.Listener.SessionAPIAddr) } } func TestApplyPreset_Waypoint(t *testing.T) { cfg := &Config{Mode: ModeWaypoint} ApplyPreset(cfg) - if cfg.Identity.Type != "client-secret" { - t.Errorf("identity.type = %q, want client-secret", cfg.Identity.Type) + if cfg.Listener.ExtAuthzAddr != ":9090" { + t.Errorf("ext_authz_addr = %q, want :9090", cfg.Listener.ExtAuthzAddr) } - if cfg.Outbound.DefaultPolicy != "exchange" { - t.Errorf("default_policy = %q, want exchange", cfg.Outbound.DefaultPolicy) + if cfg.Listener.ForwardProxyAddr != ":8080" { + t.Errorf("forward_proxy_addr = %q, want :8080", cfg.Listener.ForwardProxyAddr) } } func TestApplyPreset_ProxySidecar(t *testing.T) { cfg := &Config{Mode: ModeProxySidecar} ApplyPreset(cfg) - if cfg.Identity.Type != "spiffe" { - t.Errorf("identity.type = %q, want spiffe", cfg.Identity.Type) - } if cfg.Listener.ReverseProxyAddr != ":8080" { t.Errorf("reverse_proxy_addr = %q, want :8080", cfg.Listener.ReverseProxyAddr) } + if cfg.Listener.ForwardProxyAddr != ":8081" { + t.Errorf("forward_proxy_addr = %q, want :8081", cfg.Listener.ForwardProxyAddr) + } } func TestApplyPreset_UserOverride(t *testing.T) { cfg := &Config{ - Mode: ModeEnvoySidecar, - Identity: IdentityConfig{Type: "client-secret"}, // user override + Mode: ModeEnvoySidecar, + Listener: ListenerConfig{ExtProcAddr: ":19090"}, // user override } ApplyPreset(cfg) - if cfg.Identity.Type != "client-secret" { - t.Errorf("user override lost: identity.type = %q", cfg.Identity.Type) - } -} - -func TestNoTokenPolicyForMode(t *testing.T) { - tests := []struct { - mode string - want string - }{ - {ModeEnvoySidecar, "client-credentials"}, - {ModeWaypoint, "allow"}, - {ModeProxySidecar, "deny"}, - {"unknown", "deny"}, - } - for _, tt := range tests { - if got := NoTokenPolicyForMode(tt.mode); got != tt.want { - t.Errorf("NoTokenPolicyForMode(%q) = %q, want %q", tt.mode, got, tt.want) - } + if cfg.Listener.ExtProcAddr != ":19090" { + t.Errorf("user override lost: ext_proc_addr = %q", cfg.Listener.ExtProcAddr) } } @@ -92,73 +72,45 @@ func TestValidate_InvalidMode(t *testing.T) { } } -func TestValidate_MissingRequired(t *testing.T) { - base := Config{ - Mode: ModeWaypoint, - Identity: IdentityConfig{Type: "client-secret", ClientID: "c", ClientSecret: "s"}, - } - - // Missing issuer - cfg := base - cfg.Inbound = InboundConfig{JWKSURL: "http://jwks"} - cfg.Outbound = OutboundConfig{TokenURL: "http://token"} - if err := Validate(&cfg); err == nil { - t.Error("expected error for missing issuer") - } - - // Missing jwks_url - cfg = base - cfg.Inbound = InboundConfig{Issuer: "http://issuer"} - cfg.Outbound = OutboundConfig{TokenURL: "http://token"} - if err := Validate(&cfg); err == nil { - t.Error("expected error for missing jwks_url") - } - - // Missing token_url - cfg = base - cfg.Inbound = InboundConfig{JWKSURL: "http://jwks", Issuer: "http://issuer"} - if err := Validate(&cfg); err == nil { - t.Error("expected error for missing token_url") - } -} - -func TestValidate_SpiffeIdentityRequiresPath(t *testing.T) { - cfg := validEnvoySidecarConfig() - cfg.Identity = IdentityConfig{Type: "spiffe"} - if err := Validate(cfg); err == nil { - t.Error("expected error for spiffe without path") - } -} - func TestValidate_InvalidListenerCombo(t *testing.T) { - cfg := validEnvoySidecarConfig() - cfg.Listener.ReverseProxyAddr = ":8080" // invalid for envoy-sidecar + cfg := &Config{ + Mode: ModeEnvoySidecar, + Listener: ListenerConfig{ReverseProxyAddr: ":8080"}, // invalid for envoy-sidecar + } if err := Validate(cfg); err == nil { t.Error("expected error for envoy-sidecar + reverse_proxy_addr") } } func TestValidate_WaypointRejectsExtProc(t *testing.T) { - cfg := validWaypointConfig() - cfg.Listener.ExtProcAddr = ":9090" + cfg := &Config{ + Mode: ModeWaypoint, + Listener: ListenerConfig{ExtProcAddr: ":9090"}, + } if err := Validate(cfg); err == nil { t.Error("expected error for waypoint + ext_proc_addr") } } func TestValidate_ProxySidecarRequiresBackend(t *testing.T) { - cfg := validProxySidecarConfig() - cfg.Listener.ReverseProxyBackend = "" + cfg := &Config{Mode: ModeProxySidecar} if err := Validate(cfg); err == nil { t.Error("expected error for proxy-sidecar without backend") } } -func TestValidate_ValidConfig(t *testing.T) { +func TestValidate_ValidConfigs(t *testing.T) { + withPipeline := func(c *Config) *Config { + c.Pipeline = PipelineConfig{ + Inbound: PipelineStageConfig{Plugins: []PluginEntry{{Name: "jwt-validation"}}}, + Outbound: PipelineStageConfig{Plugins: []PluginEntry{{Name: "token-exchange"}}}, + } + return c + } for _, cfg := range []*Config{ - validEnvoySidecarConfig(), - validWaypointConfig(), - validProxySidecarConfig(), + withPipeline(&Config{Mode: ModeEnvoySidecar}), + withPipeline(&Config{Mode: ModeWaypoint}), + withPipeline(&Config{Mode: ModeProxySidecar, Listener: ListenerConfig{ReverseProxyBackend: "http://upstream"}}), } { if err := Validate(cfg); err != nil { t.Errorf("unexpected error for mode %s: %v", cfg.Mode, err) @@ -166,27 +118,97 @@ func TestValidate_ValidConfig(t *testing.T) { } } +// TestValidate_EmptyPipelineRejected guards the "open proxy" failure +// mode: before this check, an operator who kept the pre-migration +// top-level schema (inbound:, outbound:, identity:, bypass:, routes:) +// in their ConfigMap would upgrade the image, land on a config where +// yaml.v3 silently dropped the unknown top-level keys, end up with an +// empty Pipeline, and boot successfully. Listeners would then run +// pipelines with zero plugins — every request would pass through +// without JWT validation or token exchange. +// +// Empty pipelines being a configuration error forces the operator to +// either migrate to the new shape or leave the old image tagged. The +// error message names the offending section and points at the new +// schema. +func TestValidate_EmptyPipelineRejected(t *testing.T) { + cfg := &Config{Mode: ModeEnvoySidecar} + err := Validate(cfg) + if err == nil { + t.Fatal("expected error for empty pipeline") + } + if !strings.Contains(err.Error(), "pipeline.inbound.plugins is empty") { + t.Errorf("error does not name the offending section: %q", err) + } +} + +func TestValidate_EmptyOutboundPipelineRejected(t *testing.T) { + cfg := &Config{ + Mode: ModeEnvoySidecar, + Pipeline: PipelineConfig{ + Inbound: PipelineStageConfig{Plugins: []PluginEntry{{Name: "jwt-validation"}}}, + // Outbound intentionally empty + }, + } + err := Validate(cfg) + if err == nil { + t.Fatal("expected error for empty outbound pipeline") + } + if !strings.Contains(err.Error(), "pipeline.outbound.plugins is empty") { + t.Errorf("error does not name the offending section: %q", err) + } +} + +// PluginEntry's UnmarshalYAML treats `config: null` as no config at +// all. A literal null must not be forwarded to Configurable-gate as +// four bytes "null" — that would spuriously reject non-Configurable +// plugins that the operator explicitly authored with a null config +// (e.g. to emphasize "no settings"). +func TestPluginEntry_NullConfigIsTreatedAsUnset(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + content := `mode: envoy-sidecar +pipeline: + inbound: + plugins: + - name: jwt-validation + - name: a2a-parser + config: null + outbound: + plugins: + - token-exchange +` + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) + } + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + entries := cfg.Pipeline.Inbound.Plugins + if len(entries) != 2 { + t.Fatalf("entries len = %d, want 2", len(entries)) + } + if len(entries[1].Config) != 0 { + t.Errorf("a2a-parser Config = %q, want empty after config: null normalization", + entries[1].Config) + } +} + // --- Load Tests --- func TestLoad(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "config.yaml") content := `mode: waypoint -inbound: - jwks_url: "${TEST_JWKS_URL}" - issuer: "http://issuer" -outbound: - token_url: "http://token" -identity: - type: client-secret - client_id: "svc" - client_secret: "secret" +listener: + ext_authz_addr: "${TEST_ADDR}" ` if err := os.WriteFile(path, []byte(content), 0600); err != nil { t.Fatal(err) } - os.Setenv("TEST_JWKS_URL", "http://expanded-jwks") - defer os.Unsetenv("TEST_JWKS_URL") + os.Setenv("TEST_ADDR", ":19090") + defer os.Unsetenv("TEST_ADDR") cfg, err := Load(path) if err != nil { @@ -195,167 +217,222 @@ identity: if cfg.Mode != ModeWaypoint { t.Errorf("mode = %q, want waypoint", cfg.Mode) } - if cfg.Inbound.JWKSURL != "http://expanded-jwks" { - t.Errorf("jwks_url = %q, want expanded value", cfg.Inbound.JWKSURL) + if cfg.Listener.ExtAuthzAddr != ":19090" { + t.Errorf("ext_authz_addr = %q, want expanded value", cfg.Listener.ExtAuthzAddr) } } -// --- RouteConfig backwards compat --- +// --- PluginEntry YAML parsing --- -func TestRouteConfig_LegacyPassthrough(t *testing.T) { - rc := RouteConfig{Host: "internal", Passthrough: true} - // Simulate what resolve.go does - action := rc.Action - if action == "" && rc.Passthrough { - action = "passthrough" +// Pipeline configs continue to accept bare plugin names. Bare names +// mean "this plugin with no config," which is the right behavior for +// parsers (a2a-parser / mcp-parser / inference-parser) — they don't +// implement Configurable and have nothing to configure. +func TestPluginEntry_BareStringForm(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + content := `mode: envoy-sidecar +pipeline: + inbound: + plugins: + - jwt-validation + - a2a-parser + outbound: + plugins: + - token-exchange +` + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) } - if action != "passthrough" { - t.Errorf("expected passthrough, got %q", action) + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) } -} - -// --- Keycloak URL derivation --- - -func TestDeriveKeycloakURLs(t *testing.T) { - cfg := &Config{ - Mode: ModeWaypoint, - Outbound: OutboundConfig{KeycloakURL: "http://keycloak:8080", KeycloakRealm: "kagenti"}, - Identity: IdentityConfig{Type: "client-secret", ClientID: "svc", ClientSecret: "secret"}, + inbound := cfg.Pipeline.Inbound.Plugins + if len(inbound) != 2 { + t.Fatalf("inbound len = %d, want 2", len(inbound)) } - deriveKeycloakURLs(cfg) - if cfg.Outbound.TokenURL != "http://keycloak:8080/realms/kagenti/protocol/openid-connect/token" { - t.Errorf("token_url = %q", cfg.Outbound.TokenURL) + if inbound[0].Name != "jwt-validation" || len(inbound[0].Config) != 0 { + t.Errorf("inbound[0] = %+v, want {jwt-validation, nil config}", inbound[0]) } - if cfg.Inbound.Issuer != "http://keycloak:8080/realms/kagenti" { - t.Errorf("issuer = %q", cfg.Inbound.Issuer) + if inbound[1].Name != "a2a-parser" || len(inbound[1].Config) != 0 { + t.Errorf("inbound[1] = %+v, want {a2a-parser, nil config}", inbound[1]) } } -func TestDeriveKeycloakURLs_ExplicitTakesPrecedence(t *testing.T) { - cfg := &Config{ - Inbound: InboundConfig{Issuer: "http://explicit-issuer"}, - Outbound: OutboundConfig{TokenURL: "http://explicit-token", KeycloakURL: "http://keycloak:8080", KeycloakRealm: "kagenti"}, +// The richer form captures config as a raw JSON subtree. The framework +// doesn't interpret it; the plugin decodes against its own typed +// struct. Assert the bytes round-trip the operator's YAML faithfully +// (scalars preserved, nested maps intact) because that's the contract +// plugins rely on for DisallowUnknownFields decoding. +func TestPluginEntry_FullForm(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + content := `mode: envoy-sidecar +pipeline: + inbound: + plugins: + - name: jwt-validation + id: jwt-validation + config: + issuer: "http://keycloak.example/realms/kagenti" + bypass_paths: + - /healthz + - /.well-known/* + nested: + count: 3 + enabled: true +` + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) } - deriveKeycloakURLs(cfg) - if cfg.Outbound.TokenURL != "http://explicit-token" { - t.Errorf("explicit token_url should not be overridden, got %q", cfg.Outbound.TokenURL) + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) } - if cfg.Inbound.Issuer != "http://explicit-issuer" { - t.Errorf("explicit issuer should not be overridden, got %q", cfg.Inbound.Issuer) + entries := cfg.Pipeline.Inbound.Plugins + if len(entries) != 1 { + t.Fatalf("entries len = %d, want 1", len(entries)) } -} - -// --- JWKS URL derivation --- - -func TestDeriveJWKSURL(t *testing.T) { - cfg := &Config{ - Outbound: OutboundConfig{TokenURL: "http://keycloak:8080/realms/kagenti/protocol/openid-connect/token"}, + e := entries[0] + if e.Name != "jwt-validation" || e.ID != "jwt-validation" { + t.Errorf("name/id = %q/%q, want jwt-validation/jwt-validation", e.Name, e.ID) } - deriveJWKSURL(cfg) - if cfg.Inbound.JWKSURL != "http://keycloak:8080/realms/kagenti/protocol/openid-connect/certs" { - t.Errorf("jwks_url = %q", cfg.Inbound.JWKSURL) + var decoded map[string]any + if err := json.Unmarshal(e.Config, &decoded); err != nil { + t.Fatalf("config JSON parse: %v\nbytes: %s", err, e.Config) } -} - -func TestDeriveJWKSURL_ExplicitTakesPrecedence(t *testing.T) { - cfg := &Config{ - Inbound: InboundConfig{JWKSURL: "http://explicit-jwks"}, - Outbound: OutboundConfig{TokenURL: "http://keycloak:8080/realms/kagenti/protocol/openid-connect/token"}, + if decoded["issuer"] != "http://keycloak.example/realms/kagenti" { + t.Errorf("issuer round-trip lost: %v", decoded["issuer"]) } - deriveJWKSURL(cfg) - if cfg.Inbound.JWKSURL != "http://explicit-jwks" { - t.Errorf("explicit jwks_url should not be overridden, got %q", cfg.Inbound.JWKSURL) + paths, ok := decoded["bypass_paths"].([]any) + if !ok || len(paths) != 2 { + t.Errorf("bypass_paths = %v, want 2-element list", decoded["bypass_paths"]) + } + nested, ok := decoded["nested"].(map[string]any) + if !ok { + t.Fatalf("nested lost shape: %v", decoded["nested"]) + } + if got, want := nested["count"], float64(3); got != want { + t.Errorf("nested.count = %v, want 3", got) + } + if nested["enabled"] != true { + t.Errorf("nested.enabled = %v, want true", nested["enabled"]) } } -// --- Credential file validation --- - -func TestValidate_ClientIDFile(t *testing.T) { - cfg := &Config{ - Mode: ModeWaypoint, - Inbound: InboundConfig{JWKSURL: "http://jwks", Issuer: "http://issuer"}, - Outbound: OutboundConfig{TokenURL: "http://token"}, - Identity: IdentityConfig{Type: "client-secret", ClientIDFile: "/shared/client-id.txt", ClientSecretFile: "/shared/client-secret.txt"}, +// ID stays empty when omitted; callers default to Name themselves (at +// Build time, which this test does not exercise). +func TestPluginEntry_IDOmittedStaysEmpty(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + content := `mode: envoy-sidecar +pipeline: + inbound: + plugins: + - name: jwt-validation +` + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) + } + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) } - // Should pass validation — file paths accepted as alternatives - if err := Validate(cfg); err != nil { - t.Errorf("unexpected error: %v", err) + e := cfg.Pipeline.Inbound.Plugins[0] + if e.ID != "" { + t.Errorf("omitted id = %q, want empty (defaulting happens at Build)", e.ID) } } -// --- Credential file reading --- +// --- Credential file helpers --- -func TestWaitAndReadFile(t *testing.T) { +func TestReadCredentialFile(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "client-id.txt") - if err := os.WriteFile(path, []byte(" my-agent \n"), 0600); err != nil { + if err := os.WriteFile(path, []byte(" my-client-id\n"), 0600); err != nil { t.Fatal(err) } - var dest string - if err := waitAndReadFile(path, &dest, 5*time.Second); err != nil { - t.Fatalf("unexpected error: %v", err) + v, err := ReadCredentialFile(path) + if err != nil { + t.Fatalf("ReadCredentialFile: %v", err) } - if dest != "my-agent" { - t.Errorf("got %q, want trimmed value", dest) + if v != "my-client-id" { + t.Errorf("got %q, want %q (trimmed)", v, "my-client-id") } } -func TestWaitForFile_Timeout(t *testing.T) { - err := waitForFile("/nonexistent/file", 1*time.Second) +func TestReadCredentialFile_Missing(t *testing.T) { + _, err := ReadCredentialFile(filepath.Join(t.TempDir(), "does-not-exist")) if err == nil { - t.Error("expected timeout error") + t.Error("expected error for missing file") } } -// --- Helpers --- - -func validEnvoySidecarConfig() *Config { - return &Config{ - Mode: ModeEnvoySidecar, - Inbound: InboundConfig{JWKSURL: "http://jwks", Issuer: "http://issuer"}, - Outbound: OutboundConfig{TokenURL: "http://token"}, - Identity: IdentityConfig{Type: "spiffe", JWTSVIDPath: "/opt/svid.token", ClientID: "agent"}, +func TestReadCredentialFile_Empty(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "empty.txt") + if err := os.WriteFile(path, []byte{}, 0600); err != nil { + t.Fatal(err) + } + _, err := ReadCredentialFile(path) + if err == nil { + t.Error("expected error for empty file") } } -func validWaypointConfig() *Config { - return &Config{ - Mode: ModeWaypoint, - Inbound: InboundConfig{JWKSURL: "http://jwks", Issuer: "http://issuer"}, - Outbound: OutboundConfig{TokenURL: "http://token"}, - Identity: IdentityConfig{Type: "client-secret", ClientID: "svc", ClientSecret: "secret"}, +func TestWaitForCredentialFile_ContextCancel(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + _, err := WaitForCredentialFile(ctx, filepath.Join(t.TempDir(), "never")) + if err == nil { + t.Error("expected error when ctx cancels before file appears") } } -func validProxySidecarConfig() *Config { - return &Config{ - Mode: ModeProxySidecar, - Inbound: InboundConfig{JWKSURL: "http://jwks", Issuer: "http://issuer"}, - Outbound: OutboundConfig{TokenURL: "http://token"}, - Identity: IdentityConfig{Type: "spiffe", JWTSVIDPath: "/opt/svid.token", ClientID: "agent"}, - Listener: ListenerConfig{ReverseProxyBackend: "http://localhost:8081"}, +// TestWaitForCredentialFile_HeartbeatFires verifies that the +// heartbeat log path is reachable while the file is absent. The actual +// slog output isn't captured (stdlib slog has no test hook without a +// handler swap) — this test just ensures the heartbeat branch in the +// select loop is wired up, by lowering the interval and letting ctx +// time out after a heartbeat has fired. +func TestWaitForCredentialFile_HeartbeatFires(t *testing.T) { + orig := heartbeatInterval + heartbeatInterval = 50 * time.Millisecond + defer func() { heartbeatInterval = orig }() + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + + // 200ms is enough for at least one heartbeat tick even under CI + // load. The assertion below is indirect: if the heartbeat branch + // panicked (missing slog import, nil deref on the ticker, etc.), + // the goroutine would crash and the test harness would surface it. + _, err := WaitForCredentialFile(ctx, filepath.Join(t.TempDir(), "never")) + if err == nil { + t.Error("expected error when ctx cancels before file appears") } } +// --- SessionConfig tri-state --- + func TestSessionConfig_SessionEnabled(t *testing.T) { - t.Run("unset defaults to enabled", func(t *testing.T) { - var s SessionConfig - if !s.SessionEnabled() { - t.Error("unset Enabled should default to true") - } - }) - t.Run("explicit true", func(t *testing.T) { - b := true - s := SessionConfig{Enabled: &b} - if !s.SessionEnabled() { - t.Error("explicit true should be true") - } - }) - t.Run("explicit false disables", func(t *testing.T) { - b := false - s := SessionConfig{Enabled: &b} - if s.SessionEnabled() { - t.Error("explicit false should opt out") - } - }) + trueP := func(b bool) *bool { return &b } + + tests := []struct { + name string + cfg SessionConfig + want bool + }{ + {"unset defaults to true", SessionConfig{Enabled: nil}, true}, + {"explicit true", SessionConfig{Enabled: trueP(true)}, true}, + {"explicit false", SessionConfig{Enabled: trueP(false)}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.cfg.SessionEnabled(); got != tt.want { + t.Errorf("got %v, want %v", got, tt.want) + } + }) + } } diff --git a/authbridge/authlib/config/presets.go b/authbridge/authlib/config/presets.go index fe7017441..b1d10b346 100644 --- a/authbridge/authlib/config/presets.go +++ b/authbridge/authlib/config/presets.go @@ -1,58 +1,28 @@ package config -import ( - "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/bypass" -) - -// ApplyPreset fills in mode-specific defaults for settings the user didn't specify. -// Locked settings (which listeners to start) are enforced by Phase 3 listeners, -// not here — this only sets defaults for user-overridable settings. +// ApplyPreset fills in mode-specific defaults for listener addresses. +// Plugin-specific defaults live inside each plugin's Configure (see +// authbridge/authlib/plugins/CONVENTIONS.md); the runtime config no +// longer shapes plugin behavior. func ApplyPreset(cfg *Config) { switch cfg.Mode { case ModeEnvoySidecar: - setDefault(&cfg.Identity.Type, "spiffe") - setDefault(&cfg.Outbound.DefaultPolicy, "passthrough") setDefault(&cfg.Listener.ExtProcAddr, ":9090") case ModeWaypoint: - setDefault(&cfg.Identity.Type, "client-secret") - setDefault(&cfg.Outbound.DefaultPolicy, "exchange") setDefault(&cfg.Listener.ExtAuthzAddr, ":9090") setDefault(&cfg.Listener.ForwardProxyAddr, ":8080") case ModeProxySidecar: - setDefault(&cfg.Identity.Type, "spiffe") - setDefault(&cfg.Outbound.DefaultPolicy, "passthrough") setDefault(&cfg.Listener.ReverseProxyAddr, ":8080") setDefault(&cfg.Listener.ForwardProxyAddr, ":8081") } - // Session events API is default-on for every mode. Operators who want - // to turn it off can disable session tracking entirely via - // session.enabled: false — main.go skips the API server when the store - // itself is nil. + // Session events API is default-on for every mode. Operators who + // want to turn it off can disable session tracking entirely via + // session.enabled: false — main.go skips the API server when the + // store itself is nil. setDefault(&cfg.Listener.SessionAPIAddr, ":9094") - - // All modes share the same default bypass paths - if len(cfg.Bypass.InboundPaths) == 0 { - cfg.Bypass.InboundPaths = bypass.DefaultPatterns - } -} - -// NoTokenPolicyForMode returns the no-token outbound policy for a mode. -// This is locked per mode — users cannot override it. -func NoTokenPolicyForMode(mode string) string { - switch mode { - case ModeEnvoySidecar: - return auth.NoTokenPolicyClientCredentials - case ModeWaypoint: - return auth.NoTokenPolicyAllow - case ModeProxySidecar: - return auth.NoTokenPolicyDeny - default: - return auth.NoTokenPolicyDeny - } } func setDefault(field *string, value string) { diff --git a/authbridge/authlib/config/resolve.go b/authbridge/authlib/config/resolve.go index 61a4baba8..c49d469c5 100644 --- a/authbridge/authlib/config/resolve.go +++ b/authbridge/authlib/config/resolve.go @@ -1,3 +1,10 @@ +// Package config's resolve.go previously constructed a shared +// auth.Config + auth.Auth for all plugins to share. With per-plugin +// configuration, that responsibility moved into each plugin's Configure +// (see authbridge/authlib/plugins/CONVENTIONS.md), and this file is now +// just the shared credential-file waiters that multiple plugins need +// when they share a file path (e.g. /shared/client-id.txt used by both +// jwt-validation's audience_file and token-exchange's client_id_file). package config import ( @@ -7,268 +14,64 @@ import ( "os" "strings" "time" - - "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/bypass" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/cache" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/exchange" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/spiffe" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) -// Resolve applies presets, validates, and instantiates all authlib components -// from the configuration. Returns a fully wired auth.Config ready for auth.New(). -func Resolve(_ context.Context, cfg *Config) (*auth.Config, error) { - ApplyPreset(cfg) - - // Derive URLs from KEYCLOAK_URL + KEYCLOAK_REALM when explicit values are missing - deriveKeycloakURLs(cfg) - - // Derive JWKS URL from TOKEN_URL when not explicitly set (Keycloak convention) - deriveJWKSURL(cfg) - - // Credential files are NOT resolved here — caller handles them - // asynchronously via ResolveCredentialFiles() after the listener starts. - // This avoids blocking gRPC startup for up to 2 minutes. - - if err := Validate(cfg); err != nil { - return nil, fmt.Errorf("config validation: %w", err) - } - if err := ValidateOutboundPolicy(cfg.Outbound.DefaultPolicy); err != nil { - return nil, err - } - - // Bypass matcher - matcher, err := bypass.NewMatcher(cfg.Bypass.InboundPaths) +// ReadCredentialFile performs a one-shot read of a credential file, +// returning its whitespace-trimmed contents. Used by plugins from +// Configure to opportunistically pick up values that client-registration +// has already written; when it returns an error, the plugin should fall +// back to WaitForCredentialFile from Init to wait for the file. +func ReadCredentialFile(path string) (string, error) { + info, err := os.Stat(path) if err != nil { - return nil, fmt.Errorf("bypass patterns: %w", err) - } - - // JWT verifier — lazy initialization defers the JWKS HTTP fetch to the - // first Verify() call, so the gRPC listener can start immediately. - verifier := validation.NewLazyJWKSVerifier(cfg.Inbound.JWKSURL, cfg.Inbound.Issuer) - - // Client auth for token exchange - clientAuth, err := ResolveClientAuth(cfg) - if err != nil { - return nil, fmt.Errorf("client auth: %w", err) - } - - exchanger := exchange.NewClient(cfg.Outbound.TokenURL, clientAuth) - - // Router - router, err := resolveRouter(cfg) - if err != nil { - return nil, fmt.Errorf("router: %w", err) - } - - result := &auth.Config{ - Verifier: verifier, - Exchanger: exchanger, - Cache: cache.New(), - Bypass: matcher, - Router: router, - Identity: auth.IdentityConfig{ - ClientID: cfg.Identity.ClientID, - Audience: cfg.Identity.ClientID, // inbound audience defaults to client ID - }, - NoTokenPolicy: NoTokenPolicyForMode(cfg.Mode), - } - - // Waypoint mode: derive audience from destination hostname when no route matches - if cfg.Mode == ModeWaypoint { - result.AudienceDeriver = routing.ServiceNameFromHost - } - - return result, nil -} - -// deriveKeycloakURLs derives TOKEN_URL and ISSUER from KEYCLOAK_URL + KEYCLOAK_REALM. -// Explicit values always take precedence. -func deriveKeycloakURLs(cfg *Config) { - keycloakURL := strings.TrimRight(cfg.Outbound.KeycloakURL, "/") - realm := cfg.Outbound.KeycloakRealm - if keycloakURL == "" || realm == "" { - return - } - base := keycloakURL + "/realms/" + realm - if cfg.Outbound.TokenURL == "" { - cfg.Outbound.TokenURL = base + "/protocol/openid-connect/token" - slog.Info("derived token_url from keycloak_url + keycloak_realm", - "token_url", cfg.Outbound.TokenURL) - } - if cfg.Inbound.Issuer == "" { - cfg.Inbound.Issuer = base - slog.Info("derived issuer from keycloak_url + keycloak_realm", - "issuer", cfg.Inbound.Issuer) - } -} - -// deriveJWKSURL derives the JWKS endpoint from TOKEN_URL using Keycloak's convention: -// .../protocol/openid-connect/token → .../protocol/openid-connect/certs -// Uses suffix-based replacement to avoid modifying hostnames containing "token". -func deriveJWKSURL(cfg *Config) { - if cfg.Inbound.JWKSURL != "" || cfg.Outbound.TokenURL == "" { - return - } - if strings.HasSuffix(cfg.Outbound.TokenURL, "/token") { - cfg.Inbound.JWKSURL = strings.TrimSuffix(cfg.Outbound.TokenURL, "/token") + "/certs" - slog.Info("derived jwks_url from token_url", "jwks_url", cfg.Inbound.JWKSURL) - } -} - -// ResolveCredentialFiles polls for credential files and signals readiness. -// It never gives up: after initialTimeout it switches to exponential backoff. -// Call this after the server listener starts to avoid blocking startup. -func ResolveCredentialFiles(cfg *Config, initialTimeout time.Duration) <-chan struct{} { - ready := make(chan struct{}) - - needClientID := cfg.Identity.ClientIDFile != "" - needClientSecret := cfg.Identity.ClientSecretFile != "" - needSVID := cfg.Identity.Type == "spiffe" && cfg.Identity.JWTSVIDPath != "" - - if !needClientID && !needClientSecret && !needSVID { - close(ready) - return ready + return "", err } - - go func() { - // Phase 1: fast poll (2s interval) up to initialTimeout - if resolveAllCredentials(cfg, needClientID, needClientSecret, needSVID, initialTimeout) { - close(ready) - return - } - slog.Warn("credential files not available after initial timeout, continuing in background", - "timeout", initialTimeout) - - // Phase 2: exponential backoff, never gives up - backoff := 5 * time.Second - const maxBackoff = 60 * time.Second - for { - time.Sleep(backoff) - if resolveAllCredentials(cfg, needClientID, needClientSecret, needSVID, 0) { - close(ready) - slog.Info("credential files loaded after extended wait") - return - } - if backoff < maxBackoff { - backoff = min(backoff*2, maxBackoff) - } - } - }() - return ready -} - -// resolveAllCredentials attempts to read all needed credential files. -// With timeout=0 it does a single non-blocking check. -// Returns true only when ALL needed files have been successfully loaded. -func resolveAllCredentials(cfg *Config, needClientID, needClientSecret, needSVID bool, timeout time.Duration) bool { - allOK := true - - if needClientID { - if err := waitAndReadFile(cfg.Identity.ClientIDFile, &cfg.Identity.ClientID, timeout); err != nil { - allOK = false - } - } - if needClientSecret { - if err := waitAndReadFile(cfg.Identity.ClientSecretFile, &cfg.Identity.ClientSecret, timeout); err != nil { - allOK = false - } - } - if needSVID { - if err := waitForFile(cfg.Identity.JWTSVIDPath, timeout); err != nil { - allOK = false - } - } - return allOK -} - -// waitAndReadFile polls for a file to exist, then reads its content into dest. -func waitAndReadFile(path string, dest *string, timeout time.Duration) error { - if err := waitForFile(path, timeout); err != nil { - return err + if info.Size() == 0 { + return "", fmt.Errorf("file %s is empty", path) } - content, err := os.ReadFile(path) + b, err := os.ReadFile(path) if err != nil { - return fmt.Errorf("reading %s: %w", path, err) + return "", err } - *dest = strings.TrimSpace(string(content)) - slog.Info("loaded credential from file", "path", path) - return nil + return strings.TrimSpace(string(b)), nil } -// waitForFile polls for a file to exist, returning nil when found or error on timeout. -// With timeout=0, performs a single non-blocking check. -func waitForFile(path string, timeout time.Duration) error { - if info, err := os.Stat(path); err == nil && info.Size() > 0 { - return nil - } - if timeout <= 0 { - return fmt.Errorf("file not ready: %s", path) - } - deadline := time.Now().Add(timeout) - for time.Now().Before(deadline) { - time.Sleep(2 * time.Second) - if info, err := os.Stat(path); err == nil && info.Size() > 0 { - return nil - } - } - return fmt.Errorf("timeout waiting for %s (%v)", path, timeout) -} - -// ResolveClientAuth creates the appropriate client authentication from config. -// Exported so main.go can rebuild it after credential files are loaded. -func ResolveClientAuth(cfg *Config) (exchange.ClientAuth, error) { - switch cfg.Identity.Type { - case "spiffe": - if cfg.Identity.JWTSVIDPath != "" { - source := spiffe.NewFileJWTSource(cfg.Identity.JWTSVIDPath) - return &exchange.JWTAssertionAuth{ - ClientID: cfg.Identity.ClientID, - AssertionType: "urn:ietf:params:oauth:client-assertion-type:jwt-spiffe", - TokenSource: source.FetchToken, - }, nil - } - return nil, fmt.Errorf("spiffe identity requires jwt_svid_path (Workload API not yet supported)") - - case "client-secret": - return &exchange.ClientSecretAuth{ - ClientID: cfg.Identity.ClientID, - ClientSecret: cfg.Identity.ClientSecret, - }, nil - - default: - return nil, fmt.Errorf("unsupported identity type %q for client auth", cfg.Identity.Type) - } -} - -func resolveRouter(cfg *Config) (*routing.Router, error) { - var rules []routing.Route - - // Load from file if specified - if cfg.Routes.File != "" { - fileRoutes, err := routing.LoadRoutes(cfg.Routes.File) - if err != nil { - return nil, err +// heartbeatInterval is how often WaitForCredentialFile emits a WARN +// while still waiting. Chosen so that a misconfigured volume mount +// (e.g., ConfigMap name typo) is loud enough to notice during an +// operational incident, but not so spammy that it drowns out real +// signal. Overridable at package scope for tests. +var heartbeatInterval = 60 * time.Second + +// WaitForCredentialFile blocks until the file is readable with non-zero +// length, or until ctx is cancelled. Plugins call this from Init (via a +// goroutine) to wait out the race with client-registration's secret +// provisioning. +// +// Polls at 2s intervals — fast enough for human-observable boot times, +// slow enough that a pod full of plugins isn't hammering the kubelet. +// Emits a WARN every heartbeatInterval while the file is still absent +// so operators can spot wrong paths / missing volume mounts in +// `kubectl logs` during an incident, rather than discovering them +// after chasing silent 503s from traffic. +func WaitForCredentialFile(ctx context.Context, path string) (string, error) { + ticker := time.NewTicker(2 * time.Second) + defer ticker.Stop() + heartbeat := time.NewTicker(heartbeatInterval) + defer heartbeat.Stop() + start := time.Now() + for { + if v, err := ReadCredentialFile(path); err == nil { + return v, nil } - rules = append(rules, fileRoutes...) - } - - // Add inline rules, converting from RouteConfig to routing.Route - for _, rc := range cfg.Routes.Rules { - action := rc.Action - if action == "" && rc.Passthrough { - action = "passthrough" // backwards compatibility + select { + case <-ctx.Done(): + return "", ctx.Err() + case <-heartbeat.C: + slog.Warn("credential file still not available", + "path", path, + "waited", time.Since(start).Round(time.Second)) + case <-ticker.C: } - rules = append(rules, routing.Route{ - Host: rc.Host, - Audience: rc.TargetAudience, - Scopes: rc.TokenScopes, - TokenEndpoint: rc.TokenURL, - Action: action, - }) } - - return routing.NewRouter(cfg.Outbound.DefaultPolicy, rules) } diff --git a/authbridge/authlib/config/validate.go b/authbridge/authlib/config/validate.go index 3a68e0196..3c8e9cba5 100644 --- a/authbridge/authlib/config/validate.go +++ b/authbridge/authlib/config/validate.go @@ -1,15 +1,23 @@ package config -import ( - "fmt" - "log/slog" -) +import "fmt" -// Validate checks the configuration for errors and warnings. -// Returns an error for invalid configurations that would fail at runtime. -// Logs warnings for unusual-but-valid combinations. +// Validate checks the top-level runtime config: mode, listener combo, +// and that the pipeline composition is populated. Plugin-specific +// validation (issuer, token URL, identity type) lives inside each +// plugin's Configure and runs at pipeline build time. +// +// Empty pipelines are rejected. Under the per-plugin config shape, +// a valid runtime config always names at least one inbound plugin +// (jwt-validation) and one outbound plugin (token-exchange). Silently +// accepting empty pipelines caused the whole point of authbridge to +// disappear — inbound traffic passing without JWT validation, outbound +// passing without token exchange. Operators upgrading from the old +// top-level-block schema ("inbound:", "outbound:", etc.) whose YAML +// does not yet include a pipeline section fail loudly here rather +// than shipping an open proxy. See the schema migration note in +// cmd/authbridge/README.md. func Validate(cfg *Config) error { - // Mode is required switch cfg.Mode { case ModeEnvoySidecar, ModeWaypoint, ModeProxySidecar: // valid @@ -18,57 +26,23 @@ func Validate(cfg *Config) error { default: return fmt.Errorf("unknown mode %q (valid: envoy-sidecar, waypoint, proxy-sidecar)", cfg.Mode) } - - // Required fields - if cfg.Inbound.Issuer == "" { - return fmt.Errorf("inbound.issuer is required") - } - if cfg.Inbound.JWKSURL == "" { - return fmt.Errorf("inbound.jwks_url is required") - } - if cfg.Outbound.TokenURL == "" { - // token_url may have been derived from keycloak_url + keycloak_realm in Resolve() - return fmt.Errorf("outbound.token_url is required (or set keycloak_url + keycloak_realm)") - } - - // Identity validation - if err := validateIdentity(cfg); err != nil { - return err - } - - // Mode-specific listener validation if err := validateListeners(cfg); err != nil { return err } - - // Warnings for unusual combinations - warnUnusual(cfg) - - return nil + return validatePipeline(cfg) } -func validateIdentity(cfg *Config) error { - switch cfg.Identity.Type { - case "spiffe": - if cfg.Identity.SocketPath == "" && cfg.Identity.JWTSVIDPath == "" { - return fmt.Errorf("identity.type=spiffe requires socket_path or jwt_svid_path") - } - if cfg.Identity.ClientID == "" && cfg.Identity.ClientIDFile == "" { - return fmt.Errorf("identity.type=spiffe requires client_id or client_id_file") - } - case "client-secret": - if cfg.Identity.ClientID == "" && cfg.Identity.ClientIDFile == "" { - return fmt.Errorf("identity.type=client-secret requires client_id or client_id_file") - } - if cfg.Identity.ClientSecret == "" && cfg.Identity.ClientSecretFile == "" { - return fmt.Errorf("identity.type=client-secret requires client_secret or client_secret_file") - } - case "k8s-sa": - // Future: validate service account token path - case "": - return fmt.Errorf("identity.type is required") - default: - return fmt.Errorf("unknown identity.type %q (valid: spiffe, client-secret, k8s-sa)", cfg.Identity.Type) +func validatePipeline(cfg *Config) error { + if len(cfg.Pipeline.Inbound.Plugins) == 0 { + return fmt.Errorf("pipeline.inbound.plugins is empty; specify at least one plugin " + + "(typically jwt-validation) — see cmd/authbridge/README.md. " + + "If you see this after an upgrade, your config.yaml is using the old top-level shape " + + "(inbound:, outbound:, identity:, bypass:, routes:) — move those settings under " + + "pipeline.*.plugins[].config") + } + if len(cfg.Pipeline.Outbound.Plugins) == 0 { + return fmt.Errorf("pipeline.outbound.plugins is empty; specify at least one plugin " + + "(typically token-exchange) — see cmd/authbridge/README.md") } return nil } @@ -102,28 +76,3 @@ func validateListeners(cfg *Config) error { } return nil } - -func warnUnusual(cfg *Config) { - warnings := []string{} - - if cfg.Mode == ModeEnvoySidecar && cfg.Identity.Type == "client-secret" { - warnings = append(warnings, "envoy-sidecar with client-secret identity is unusual (typically uses spiffe)") - } - if cfg.Mode == ModeWaypoint && cfg.Identity.Type == "spiffe" { - warnings = append(warnings, "waypoint with spiffe identity is unusual (typically uses client-secret)") - } - - for _, w := range warnings { - slog.Warn(w) - } -} - -// ValidateOutboundPolicy checks that default_policy is a valid value. -func ValidateOutboundPolicy(policy string) error { - switch policy { - case "exchange", "passthrough", "": - return nil - default: - return fmt.Errorf("unknown outbound.default_policy %q (valid: exchange, passthrough)", policy) - } -} diff --git a/authbridge/authlib/observe/statserver.go b/authbridge/authlib/observe/statserver.go index cae95310d..41623b0a4 100644 --- a/authbridge/authlib/observe/statserver.go +++ b/authbridge/authlib/observe/statserver.go @@ -16,11 +16,21 @@ type StatServer struct { server *http.Server } -func NewStatServer(addr string, config *config.Config, stats *auth.Stats) *StatServer { +// StatsProvider returns a fresh *auth.Stats per /stats request. The +// host typically implements this by calling auth.MergeStats over the +// per-plugin stats collected from each pipeline — see +// plugins.CollectStats. Called per HTTP request, so implementations +// should be cheap (a few map copies). +type StatsProvider func() *auth.Stats + +// NewStatServer builds the stat HTTP server. The statsProvider is +// invoked per /stats request so per-plugin counters can be aggregated +// live rather than captured at process start. +func NewStatServer(addr string, config *config.Config, statsProvider StatsProvider) *StatServer { mux := http.NewServeMux() mux.HandleFunc("/config", handleConfigFactory(config)) - mux.HandleFunc("/stats", handleStatsFactory(stats)) + mux.HandleFunc("/stats", handleStatsFactory(statsProvider)) mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/html; charset=utf-8") @@ -48,41 +58,31 @@ func NewStatServer(addr string, config *config.Config, stats *auth.Stats) *StatS func handleConfigFactory(cfg *config.Config) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - // Rather than outputting the entire config, - // customize the output to redact the client secret. - err := json.NewEncoder(w).Encode(config.Config{ - Mode: cfg.Mode, - Inbound: cfg.Inbound, - Outbound: cfg.Outbound, - Identity: config.IdentityConfig{ - Type: cfg.Identity.Type, - // We report the ClientID unredacted. In Kagenti, the ID will be something like - // "spiffe://localtest.me/ns/team1/sa/my-weather-service-with-authbridge" - // Although a brute force attack is possible, showing the ClientID here does - // not introduce new security concerns, as an attacker can already construct - // the ClientID from the pod's namespace and name, available in the UI. - ClientID: cfg.Identity.ClientID, - ClientSecret: "*redacted*", - ClientIDFile: "*redacted*", - ClientSecretFile: "*redacted*", - SocketPath: "*redacted*", - JWTSVIDPath: "*redacted*", - JWTAudience: cfg.Identity.JWTAudience, - }, - Listener: cfg.Listener, - Bypass: cfg.Bypass, - Routes: cfg.Routes, - Stats: cfg.Stats, - }) + // Plugin config subtrees are captured verbatim as json.RawMessage + // by the PluginEntry unmarshaler. Operators shouldn't put + // secrets in the runtime config — the per-plugin convention is + // to reference a file path instead (client_secret_file, etc.) — + // so we render the config as-is. If a plugin ever needs to + // suppress a known-sensitive field here, it can be added to a + // redaction pass in a follow-up. + err := json.NewEncoder(w).Encode(cfg) if err != nil { slog.Default().Info("Failed to send configuration", "err", err) } } } -func handleStatsFactory(stats *auth.Stats) func(http.ResponseWriter, *http.Request) { +func handleStatsFactory(provider StatsProvider) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") + // Provider returns a freshly-merged *auth.Stats. Nil means + // "no source plugins" — render an empty object rather than + // failing, so the endpoint shape is stable even on pipelines + // that register no stats sources. + stats := provider() + if stats == nil { + stats = auth.NewStats() + } err := json.NewEncoder(w).Encode(stats) if err != nil { slog.Default().Info("Failed to send stats", "err", err) diff --git a/authbridge/authlib/observe/statserver_test.go b/authbridge/authlib/observe/statserver_test.go index 828dca617..7a2718846 100644 --- a/authbridge/authlib/observe/statserver_test.go +++ b/authbridge/authlib/observe/statserver_test.go @@ -15,37 +15,11 @@ import ( func newTestConfig() *config.Config { return &config.Config{ Mode: "proxy-sidecar", - Inbound: config.InboundConfig{ - JWKSURL: "https://keycloak.example.com/certs", - Issuer: "https://keycloak.example.com/realms/test", - }, - Outbound: config.OutboundConfig{ - TokenURL: "https://keycloak.example.com/token", - DefaultPolicy: "passthrough", - }, - Identity: config.IdentityConfig{ - Type: "client-secret", - ClientID: "my-agent", - ClientSecret: "super-secret-value", - ClientIDFile: "/shared/client-id.txt", - ClientSecretFile: "/shared/client-secret.txt", - SocketPath: "/run/spire/sockets/agent.sock", - JWTSVIDPath: "/opt/jwt_svid.token", - JWTAudience: []string{"my-audience"}, - }, Listener: config.ListenerConfig{ ForwardProxyAddr: ":15123", ReverseProxyAddr: ":15124", ReverseProxyBackend: "http://localhost:8080", }, - Bypass: config.BypassConfig{ - InboundPaths: []string{"/health", "/ready"}, - }, - Routes: config.RoutesConfig{ - Rules: []config.RouteConfig{ - {Host: "target-svc", TargetAudience: "target", TokenScopes: "openid"}, - }, - }, Stats: config.StatsConfig{ StatsAddress: ":9093", }, @@ -53,7 +27,11 @@ func newTestConfig() *config.Config { } func serveMux(cfg *config.Config, stats *auth.Stats) http.Handler { - s := NewStatServer(":0", cfg, stats) + // Provider closes over a fixed Stats so the tests can control + // what /stats returns. Production callers pass a provider that + // merges per-plugin Stats at request time; see main.go's + // statsProvider closure. + s := NewStatServer(":0", cfg, func() *auth.Stats { return stats }) return s.server.Handler } @@ -79,7 +57,13 @@ func TestRootHandler(t *testing.T) { } } -func TestConfigEndpointRedactsSensitiveFields(t *testing.T) { +// The /config endpoint rendering today emits the runtime config +// verbatim. Per-plugin config subtrees are opaque json.RawMessage +// blobs; operators are expected to keep secrets out of the runtime +// YAML (reference file paths like client_secret_file instead). A +// future redaction pass can walk known-sensitive plugin fields, but +// is not in this PR's scope. +func TestConfigEndpointShape(t *testing.T) { cfg := newTestConfig() handler := serveMux(cfg, auth.NewStats()) @@ -99,52 +83,9 @@ func TestConfigEndpointRedactsSensitiveFields(t *testing.T) { if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { t.Fatalf("decode config response: %v", err) } - - if got.Identity.ClientSecret != "*redacted*" { - t.Errorf("ClientSecret = %q, want *redacted*", got.Identity.ClientSecret) - } - if got.Identity.ClientIDFile != "*redacted*" { - t.Errorf("ClientIDFile = %q, want *redacted*", got.Identity.ClientIDFile) - } - if got.Identity.ClientSecretFile != "*redacted*" { - t.Errorf("ClientSecretFile = %q, want *redacted*", got.Identity.ClientSecretFile) - } - if got.Identity.SocketPath != "*redacted*" { - t.Errorf("SocketPath = %q, want *redacted*", got.Identity.SocketPath) - } - if got.Identity.JWTSVIDPath != "*redacted*" { - t.Errorf("JWTSVIDPath = %q, want *redacted*", got.Identity.JWTSVIDPath) - } -} - -func TestConfigEndpointPreservesNonSensitiveFields(t *testing.T) { - cfg := newTestConfig() - handler := serveMux(cfg, auth.NewStats()) - - req := httptest.NewRequest(http.MethodGet, "/config", nil) - w := httptest.NewRecorder() - handler.ServeHTTP(w, req) - - var got config.Config - if err := json.NewDecoder(w.Result().Body).Decode(&got); err != nil { - t.Fatalf("decode: %v", err) - } - if got.Mode != cfg.Mode { t.Errorf("Mode = %q, want %q", got.Mode, cfg.Mode) } - if got.Identity.Type != cfg.Identity.Type { - t.Errorf("Identity.Type = %q, want %q", got.Identity.Type, cfg.Identity.Type) - } - if got.Identity.ClientID != cfg.Identity.ClientID { - t.Errorf("Identity.ClientID = %q, want %q", got.Identity.ClientID, cfg.Identity.ClientID) - } - if got.Inbound.Issuer != cfg.Inbound.Issuer { - t.Errorf("Inbound.Issuer = %q, want %q", got.Inbound.Issuer, cfg.Inbound.Issuer) - } - if got.Outbound.DefaultPolicy != cfg.Outbound.DefaultPolicy { - t.Errorf("Outbound.DefaultPolicy = %q, want %q", got.Outbound.DefaultPolicy, cfg.Outbound.DefaultPolicy) - } if got.Stats.StatsAddress != cfg.Stats.StatsAddress { t.Errorf("Stats.StatsAddress = %q, want %q", got.Stats.StatsAddress, cfg.Stats.StatsAddress) } @@ -178,13 +119,10 @@ func TestStatsEndpointEmptyStats(t *testing.T) { } } -func TestStatsEndpointWithCounts(t *testing.T) { +func TestStatsEndpointValidJSON(t *testing.T) { stats := auth.NewStats() handler := serveMux(newTestConfig(), stats) - // Stats are exported from auth package — the Stats fields are unexported, - // but we can exercise the JSON output by marshalling through the endpoint. - // First, verify we get valid JSON back even from a fresh Stats object. req := httptest.NewRequest(http.MethodGet, "/stats", nil) w := httptest.NewRecorder() handler.ServeHTTP(w, req) @@ -196,72 +134,15 @@ func TestStatsEndpointWithCounts(t *testing.T) { } func TestNewStatServerSetsAddr(t *testing.T) { - s := NewStatServer(":9093", newTestConfig(), auth.NewStats()) + s := NewStatServer(":9093", newTestConfig(), func() *auth.Stats { return auth.NewStats() }) if s.server.Addr != ":9093" { t.Errorf("server.Addr = %q, want :9093", s.server.Addr) } } func TestNewStatServerCustomAddr(t *testing.T) { - s := NewStatServer("127.0.0.1:8888", newTestConfig(), auth.NewStats()) + s := NewStatServer("127.0.0.1:8888", newTestConfig(), func() *auth.Stats { return auth.NewStats() }) if s.server.Addr != "127.0.0.1:8888" { t.Errorf("server.Addr = %q, want 127.0.0.1:8888", s.server.Addr) } } - -func TestConfigEndpointPreservesRoutes(t *testing.T) { - cfg := newTestConfig() - handler := serveMux(cfg, auth.NewStats()) - - req := httptest.NewRequest(http.MethodGet, "/config", nil) - w := httptest.NewRecorder() - handler.ServeHTTP(w, req) - - var got config.Config - if err := json.NewDecoder(w.Result().Body).Decode(&got); err != nil { - t.Fatalf("decode: %v", err) - } - - if len(got.Routes.Rules) != 1 { - t.Fatalf("Routes.Rules length = %d, want 1", len(got.Routes.Rules)) - } - if got.Routes.Rules[0].Host != "target-svc" { - t.Errorf("Routes.Rules[0].Host = %q, want target-svc", got.Routes.Rules[0].Host) - } -} - -func TestConfigEndpointPreservesBypass(t *testing.T) { - cfg := newTestConfig() - handler := serveMux(cfg, auth.NewStats()) - - req := httptest.NewRequest(http.MethodGet, "/config", nil) - w := httptest.NewRecorder() - handler.ServeHTTP(w, req) - - var got config.Config - if err := json.NewDecoder(w.Result().Body).Decode(&got); err != nil { - t.Fatalf("decode: %v", err) - } - - if len(got.Bypass.InboundPaths) != 2 { - t.Fatalf("Bypass.InboundPaths length = %d, want 2", len(got.Bypass.InboundPaths)) - } -} - -func TestConfigEndpointPreservesJWTAudience(t *testing.T) { - cfg := newTestConfig() - handler := serveMux(cfg, auth.NewStats()) - - req := httptest.NewRequest(http.MethodGet, "/config", nil) - w := httptest.NewRecorder() - handler.ServeHTTP(w, req) - - var got config.Config - if err := json.NewDecoder(w.Result().Body).Decode(&got); err != nil { - t.Fatalf("decode: %v", err) - } - - if len(got.Identity.JWTAudience) != 1 || got.Identity.JWTAudience[0] != "my-audience" { - t.Errorf("Identity.JWTAudience = %v, want [my-audience]", got.Identity.JWTAudience) - } -} diff --git a/authbridge/authlib/pipeline/configurable.go b/authbridge/authlib/pipeline/configurable.go new file mode 100644 index 000000000..f34ffc86e --- /dev/null +++ b/authbridge/authlib/pipeline/configurable.go @@ -0,0 +1,25 @@ +package pipeline + +import "encoding/json" + +// Configurable is an optional interface plugins implement when they need +// per-instance configuration. The pipeline builder calls Configure exactly +// once per instance, during pipeline construction, before Start. Plugins +// that don't need config simply omit this interface; the builder skips +// them. +// +// The contract is deliberately narrow: +// - The raw argument is the plugin's own config subtree from the runtime +// YAML, as json.RawMessage — the framework does not interpret it. +// - Plugins decode with DisallowUnknownFields so stale or misspelled keys +// are rejected loudly at startup rather than silently ignored. +// - Plugins apply their own defaults and run their own validation, then +// construct any internal state needed at request time. +// - An error from Configure aborts pipeline construction and takes the +// process down; do not partial-initialize and return nil. +// +// See authbridge/authlib/plugins/CONVENTIONS.md for the recommended shape +// of per-plugin config and a worked example. +type Configurable interface { + Configure(raw json.RawMessage) error +} diff --git a/authbridge/authlib/pipeline/configurable_test.go b/authbridge/authlib/pipeline/configurable_test.go new file mode 100644 index 000000000..14fe9f973 --- /dev/null +++ b/authbridge/authlib/pipeline/configurable_test.go @@ -0,0 +1,86 @@ +package pipeline + +import ( + "context" + "encoding/json" + "errors" + "testing" +) + +// configurablePlugin is a stubPlugin that also implements Configurable so +// tests can verify Configure is invoked and its error is surfaced. +type configurablePlugin struct { + stubPlugin + configureCalled bool + receivedRaw json.RawMessage + returnErr error +} + +func (c *configurablePlugin) Configure(raw json.RawMessage) error { + c.configureCalled = true + c.receivedRaw = raw + return c.returnErr +} + +func TestConfigurable_InterfaceIsOptional(t *testing.T) { + // A plugin that doesn't implement Configurable must not be rejected. + // This is the whole reason Configurable exists as an interface rather + // than a method on Plugin — parsers and other config-free plugins + // shouldn't need a boilerplate Configure stub. + p := &stubPlugin{name: "no-config"} + + // Type assertion that callers (Build) will use. + if _, ok := any(p).(Configurable); ok { + t.Error("stubPlugin must NOT implement Configurable; otherwise this test is vacuous") + } +} + +func TestConfigurable_ErrorPropagates(t *testing.T) { + // A Configure error is surfaced to the caller. Pipeline + // construction should not silently continue when a plugin refuses + // its config — Build-level coverage that the error actually + // aborts a pipeline build lives in + // plugins_test.go:TestBuild_ConfigureError. + p := &configurablePlugin{ + stubPlugin: stubPlugin{name: "bad"}, + returnErr: errors.New("bad config"), + } + + err := p.Configure(json.RawMessage(`{"x":1}`)) + if err == nil { + t.Fatal("expected error from Configure, got nil") + } + if !p.configureCalled { + t.Error("Configure was never invoked") + } +} + +func TestConfigurable_RawIsPassedThrough(t *testing.T) { + // The framework must not interpret or re-marshal the raw config — the + // plugin receives the exact bytes from its config sub-tree. This lets + // plugins do their own DisallowUnknownFields decode without the + // framework swallowing stray keys. + p := &configurablePlugin{stubPlugin: stubPlugin{name: "ok"}} + in := json.RawMessage(`{"issuer":"http://example","extra":"keep"}`) + + if err := p.Configure(in); err != nil { + t.Fatalf("Configure: %v", err) + } + if string(p.receivedRaw) != string(in) { + t.Errorf("raw = %s, want %s", p.receivedRaw, in) + } +} + +// The Plugin interface itself is unchanged; a plugin can implement +// Configurable alongside the usual hooks without any adapter wiring. +func TestConfigurable_DoesNotAlterPluginSurface(t *testing.T) { + p := &configurablePlugin{stubPlugin: stubPlugin{name: "x"}} + // Plugin surface still works. + if got := p.Name(); got != "x" { + t.Errorf("Name() = %q, want x", got) + } + action := p.OnRequest(context.Background(), &Context{}) + if action.Type != Continue { + t.Errorf("OnRequest = %v, want Continue", action.Type) + } +} diff --git a/authbridge/authlib/pipeline/pipeline.go b/authbridge/authlib/pipeline/pipeline.go index fc164df21..f7e4ea1f4 100644 --- a/authbridge/authlib/pipeline/pipeline.go +++ b/authbridge/authlib/pipeline/pipeline.go @@ -130,6 +130,39 @@ func (p *Pipeline) Plugins() []Plugin { return out } +// Ready reports whether every plugin implementing pipeline.Readier +// currently reports ready. Plugins without Readier are considered +// always-ready (no deferred state). Called per /readyz probe, so the +// implementation is one cheap type-assert + bool read per plugin. +func (p *Pipeline) Ready() bool { + for _, plugin := range p.plugins { + r, ok := plugin.(Readier) + if !ok { + continue + } + if !r.Ready() { + return false + } + } + return true +} + +// NotReadyPlugin returns the first plugin whose Ready() returned +// false, or "" when the pipeline is ready. Used by /readyz to +// produce a helpful error body. +func (p *Pipeline) NotReadyPlugin() string { + for _, plugin := range p.plugins { + r, ok := plugin.(Readier) + if !ok { + continue + } + if !r.Ready() { + return plugin.Name() + } + } + return "" +} + // NeedsBody returns true if any plugin in the pipeline declares BodyAccess. func (p *Pipeline) NeedsBody() bool { for _, plugin := range p.plugins { @@ -145,23 +178,49 @@ func (p *Pipeline) NeedsBody() bool { // on error, later plugins are not initialized. Plugins without Init are // silently skipped. // +// If Init fails on plugin N, Start invokes Shutdown on plugins +// [0..N-1] (those whose Init succeeded) in reverse order before +// returning the error. This cleans up any background goroutines the +// earlier plugins spawned, so the plugin chain doesn't leak when a +// downstream peer rejects its config at boot. Shutdown errors during +// unwind are logged but do not mask the original Init failure. +// // Callers should invoke Start after Pipeline construction (pipeline.New) // and before the listener accepts traffic. Safe to call at most once per // Pipeline — plugins may assume Init runs exactly once per process. func (p *Pipeline) Start(ctx context.Context) error { - for _, plugin := range p.plugins { + for i, plugin := range p.plugins { init, ok := plugin.(Initializer) if !ok { continue } slog.Debug("pipeline: initializing plugin", "plugin", plugin.Name()) if err := init.Init(ctx); err != nil { + p.unwindStart(ctx, i) return fmt.Errorf("plugin %q Init: %w", plugin.Name(), err) } } return nil } +// unwindStart invokes Shutdown on plugins [0..failedIdx-1] in reverse +// order after a Start failure at index failedIdx. Best-effort — errors +// are logged. +func (p *Pipeline) unwindStart(ctx context.Context, failedIdx int) { + for i := failedIdx - 1; i >= 0; i-- { + sh, ok := p.plugins[i].(Shutdowner) + if !ok { + continue + } + slog.Debug("pipeline: unwinding plugin after Start failure", + "plugin", p.plugins[i].Name()) + if err := sh.Shutdown(ctx); err != nil { + slog.Warn("pipeline: plugin Shutdown during Start unwind returned error", + "plugin", p.plugins[i].Name(), "error", err) + } + } +} + // Stop invokes Shutdown on every plugin that implements the Shutdowner // interface, in reverse declaration order (LIFO). Errors are logged but // do not stop the sequence — every Shutdowner is given a chance to flush. diff --git a/authbridge/authlib/pipeline/pipeline_test.go b/authbridge/authlib/pipeline/pipeline_test.go index 92ea88542..cacb4e0bd 100644 --- a/authbridge/authlib/pipeline/pipeline_test.go +++ b/authbridge/authlib/pipeline/pipeline_test.go @@ -496,6 +496,122 @@ func TestPipelineStart_StopsOnInitError(t *testing.T) { } } +// When a plugin's Init fails, plugins earlier in the chain whose Init +// already succeeded get their Shutdown called in reverse order. This +// prevents the common failure mode of "plugin A's Init spawns a +// background goroutine, plugin B's Init rejects its config, process +// exits via log.Fatalf, plugin A's goroutine is orphaned until the +// process dies." Not a correctness bug for the production flow (exit +// cleans up everything), but matters for embedded / multi-tenant +// callers. +func TestPipelineStart_UnwindsSuccessfulInitsOnFailure(t *testing.T) { + var shutdownOrder []string + a := &lifecyclePlugin{ + stubPlugin: stubPlugin{name: "a"}, + onShutdown: func(context.Context) { shutdownOrder = append(shutdownOrder, "a") }, + } + b := &lifecyclePlugin{ + stubPlugin: stubPlugin{name: "b"}, + onShutdown: func(context.Context) { shutdownOrder = append(shutdownOrder, "b") }, + } + // c fails its Init — a and b should be shut down in reverse order. + c := &lifecyclePlugin{ + stubPlugin: stubPlugin{name: "c"}, + initErr: errors.New("config rejected"), + onShutdown: func(context.Context) { shutdownOrder = append(shutdownOrder, "c") }, + } + // d is never Init'd, must not be Shutdown either. + d := &lifecyclePlugin{ + stubPlugin: stubPlugin{name: "d"}, + onShutdown: func(context.Context) { shutdownOrder = append(shutdownOrder, "d") }, + } + + p, err := New([]Plugin{a, b, c, d}) + if err != nil { + t.Fatalf("New: %v", err) + } + if err := p.Start(context.Background()); err == nil { + t.Fatal("expected Start to return an error") + } + // Expect [b, a]: reverse order of [a, b]. c's Init failed, so c is + // not unwound (it never succeeded). d was never touched. + if got, want := shutdownOrder, []string{"b", "a"}; !equalStrings(got, want) { + t.Errorf("shutdown order = %v, want %v", got, want) + } +} + +// readyPlugin implements Readier so Pipeline.Ready / NotReadyPlugin +// can be exercised directly. Plugins without Readier are treated as +// always-ready, and this fixture proves both paths. +type readyPlugin struct { + stubPlugin + ready bool +} + +func (p *readyPlugin) Ready() bool { return p.ready } + +func TestPipelineReady_AllReadiersTrue(t *testing.T) { + p, err := New([]Plugin{ + &readyPlugin{stubPlugin: stubPlugin{name: "a"}, ready: true}, + &stubPlugin{name: "no-opinion"}, + &readyPlugin{stubPlugin: stubPlugin{name: "b"}, ready: true}, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + if !p.Ready() { + t.Error("expected pipeline to be ready when all Readiers return true") + } + if name := p.NotReadyPlugin(); name != "" { + t.Errorf("NotReadyPlugin() = %q, want empty", name) + } +} + +func TestPipelineReady_OneFalseBlocks(t *testing.T) { + p, err := New([]Plugin{ + &readyPlugin{stubPlugin: stubPlugin{name: "a"}, ready: true}, + &readyPlugin{stubPlugin: stubPlugin{name: "b"}, ready: false}, + &readyPlugin{stubPlugin: stubPlugin{name: "c"}, ready: true}, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + if p.Ready() { + t.Error("expected pipeline to be not-ready when one Readier returns false") + } + if got, want := p.NotReadyPlugin(), "b"; got != want { + t.Errorf("NotReadyPlugin() = %q, want %q", got, want) + } +} + +// Pipelines containing only non-Readier plugins (parsers, stubs) are +// always-ready. This is the behavior Ready()-free plugins rely on — +// they must not block the pipeline's /readyz. +func TestPipelineReady_NoReadiersMeansReady(t *testing.T) { + p, err := New([]Plugin{ + &stubPlugin{name: "a"}, + &stubPlugin{name: "b"}, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + if !p.Ready() { + t.Error("pipeline with no Readiers should be ready") + } +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + // Shutdown is called in reverse declaration order (LIFO), so plugins // that depend on earlier plugins' resources can still use them while // cleaning up. diff --git a/authbridge/authlib/pipeline/plugin.go b/authbridge/authlib/pipeline/plugin.go index ca8146ef4..82fcbc223 100644 --- a/authbridge/authlib/pipeline/plugin.go +++ b/authbridge/authlib/pipeline/plugin.go @@ -50,3 +50,21 @@ type Initializer interface { type Shutdowner interface { Shutdown(ctx context.Context) error } + +// Readier is an optional interface a plugin may implement when it has +// deferred initialization that matters to a /readyz probe. The host +// ANDs Ready() across all implementers to decide whether the pipeline +// is ready to serve traffic. A plugin whose Configure succeeded but +// whose Init is still waiting (e.g. for a credential file to be +// written by client-registration) returns false — the kubelet keeps +// traffic off the pod until Init completes. +// +// Plugins without deferred state don't implement this interface and +// are treated as always-ready. Pipeline.Ready() returns true when +// every Readier-implementing plugin returns true. +// +// Ready is expected to be cheap (pointer read / atomic load). The +// /readyz handler calls it on every probe (~10s cadence from kubelet). +type Readier interface { + Ready() bool +} diff --git a/authbridge/authlib/plugins/CONVENTIONS.md b/authbridge/authlib/plugins/CONVENTIONS.md new file mode 100644 index 000000000..83efe5988 --- /dev/null +++ b/authbridge/authlib/plugins/CONVENTIONS.md @@ -0,0 +1,282 @@ +# Plugin Config Conventions + +How plugins under `authbridge/authlib/plugins/` receive, validate, and +apply their configuration. Everything here is convention — the framework +only requires `pipeline.Configurable` if the plugin has any config at all. +The rest of this document exists so that the sixth and tenth plugin +don't each invent their own style. + +## Scope + +- What the YAML entry for a plugin looks like. +- How a plugin decodes that YAML into a typed config struct. +- How a plugin applies defaults and runs validation. +- What the framework does and doesn't do on your behalf. +- A template you can copy for a new plugin. + +## YAML entry shape + +Each plugin appears in the pipeline as either a bare name or a full entry: + +```yaml +pipeline: + inbound: + plugins: + - a2a-parser # bare name — no config + - name: jwt-validation + id: jwt-validation # optional; defaults to name + config: + issuer: "http://keycloak..." + audience_file: "/shared/client-id.txt" + bypass_paths: + - "/healthz" +``` + +- **`name`** — required. Must match a key in the plugin registry. +- **`id`** — optional. Defaults to `name`. Lets two instances of the same + plugin coexist with different config (not yet exercised, but the shape + is reserved). +- **`config`** — optional. Arbitrary YAML sub-tree owned by the plugin. + The framework does not interpret it; it's captured as `json.RawMessage` + and handed to `Configure`. + +## The Configurable interface + +```go +type Configurable interface { + Configure(raw json.RawMessage) error +} +``` + +The framework calls `Configure` exactly once per plugin instance, during +pipeline construction, before `Start`. Plugins without config don't +implement this interface — the builder type-asserts and skips them. + +If a plugin **does not** implement `Configurable` but the YAML entry +has a non-empty `config:` block, the builder fails with a clear +`"plugin %q does not accept configuration"` error. This catches +misconfigurations (typo in plugin name, leftover config after a +refactor) at startup. + +## The four-step Configure pattern + +Every Configurable plugin follows the same shape: + +```go +func (p *Plugin) Configure(raw json.RawMessage) error { + var c pluginConfig + if len(raw) > 0 { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() // 1. strict decode + if err := dec.Decode(&c); err != nil { + return fmt.Errorf("plugin config: %w", err) + } + } + c.applyDefaults() // 2. fill in defaults + if err := c.validate(); err != nil { // 3. validate + return fmt.Errorf("plugin config: %w", err) + } + // 4. construct internal state + p.verifier = newVerifier(c.Issuer, c.JWKSURL) + p.bypass = bypass.New(c.BypassPaths) + return nil +} +``` + +### 1. Strict decode (`DisallowUnknownFields`) + +Always. A stale or misspelled key is a mistake, not a preference. Loud +failure at startup beats a silent wrong default at request time. + +### 2. `applyDefaults()` + +Fills zero-value fields with sensible defaults and derives computed +fields. Keep it pure — no I/O, no file reads — so it can be unit-tested +with the config struct alone. + +```go +func (c *pluginConfig) applyDefaults() { + if c.DefaultPolicy == "" { + c.DefaultPolicy = "passthrough" + } + if c.JWKSURL == "" && c.Issuer != "" { + c.JWKSURL = c.Issuer + "/protocol/openid-connect/certs" + } +} +``` + +When you need to distinguish "unset" from "explicitly set to zero" — +typically for booleans — use `*bool` / `*int` in the struct and convert +to plain values after `applyDefaults`. `SessionConfig.Enabled` in +`authlib/config` is the reference pattern. + +### 3. `validate()` + +Rejects configurations the plugin cannot operate with. Run validation +**after** `applyDefaults` so derived fields are in place. + +```go +func (c *pluginConfig) validate() error { + if c.Issuer == "" { + return errors.New("issuer is required") + } + if c.DefaultPolicy != "passthrough" && c.DefaultPolicy != "exchange" { + return fmt.Errorf("default_policy must be passthrough or exchange, got %q", c.DefaultPolicy) + } + return nil +} +``` + +Return errors phrased for an operator reading a pod log, not a developer +reading a stack trace. + +### 4. Construct internal state + +This is the only step allowed to do I/O (read credential files, open +connections, etc.). Everything the plugin needs at request time should +be materialized here, not lazily on first `OnRequest` — lazy init +hides config errors until traffic arrives. + +## File-sourced values + +Several plugins accept either an inline value or a file path for the +same datum (e.g. `client_secret` vs `client_secret_file`). The +convention: + +- Both fields live in the config struct; the file variant has the + `_file` suffix. +- `applyDefaults` does not read the file. +- `validate` requires exactly one to be set. +- Internal state construction calls the file-read helper from + `authlib/config` (not a new one), which tolerates transient absence + during pod boot (client-registration may still be writing). + +## What Configure MUST NOT do + +- **Block forever.** Configure runs before traffic starts; the process + is still holding the startup deadline. Use bounded waits with + timeouts, not unbounded blocking reads. +- **Start background goroutines.** Use `Init(ctx)` from the + `pipeline.Initializer` interface for that — it runs after Configure + and has a process context you can key your goroutine's lifetime to. +- **Mutate global state.** Plugins run in a single process today, but + the config → runtime mapping must stay per-instance. Two instances + of the same plugin with different config must not clobber each other. +- **Persist the raw bytes.** Decode into your typed struct and drop + the `json.RawMessage`. Holding it leaks the original YAML, which + may contain secrets, into any log that dumps the plugin for + debugging. + +## Testing + +Each Configurable plugin ships three kinds of tests: + +1. **Config round-trip.** Given a YAML snippet, does Configure produce + the expected internal state? Exercise defaults-applied and defaults- + rejected paths explicitly. +2. **Validation failures.** One test per validation error path — name + a missing-required field, a malformed value, a conflicting pair. + Assert the error message names the bad field. +3. **Behavior integration.** The existing `OnRequest` / `OnResponse` + tests, but wired through Configure rather than hand-built internal + state. This is what keeps the config layer and the plugin behavior + honest about each other. + +## Template + +Copy this into a new plugin file as the starting point. Replace +`myPlugin` with your plugin's identifier. + +```go +package plugins + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" +) + +// myPluginConfig is the plugin's private config schema. Fields are JSON- +// tagged so Configure can DisallowUnknownFields against operator-supplied +// YAML (YAML → JSON round-trip preserves key names). +type myPluginConfig struct { + SomeKnob string `json:"some_knob"` + SomePaths []string `json:"some_paths"` + // ... +} + +func (c *myPluginConfig) applyDefaults() { + if c.SomeKnob == "" { + c.SomeKnob = "default-value" + } +} + +func (c *myPluginConfig) validate() error { + if c.SomeKnob == "" { + return errors.New("some_knob is required") + } + return nil +} + +type MyPlugin struct { + // internal state populated by Configure +} + +func (p *MyPlugin) Configure(raw json.RawMessage) error { + var c myPluginConfig + if len(raw) > 0 { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&c); err != nil { + return fmt.Errorf("my-plugin config: %w", err) + } + } + c.applyDefaults() + if err := c.validate(); err != nil { + return fmt.Errorf("my-plugin config: %w", err) + } + // construct internal state from c + return nil +} + +func (p *MyPlugin) Name() string { return "my-plugin" } +func (p *MyPlugin) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{} } +func (p *MyPlugin) OnRequest(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} +func (p *MyPlugin) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} +``` + +## Strictness asymmetry: plugin config vs. runtime top-level + +The plugin-level config inside each `plugins[].config` subtree is +**strict** — `DisallowUnknownFields` is part of the Configure +convention, so a typo or a stale key fails the plugin at boot. + +The runtime YAML's **top-level** keys (`mode`, `listener`, `pipeline`, +`session`, `stats`) are **forgiving**: unknown top-level keys are +silently ignored by the YAML decoder. This is deliberate forward- +compat — adding a new top-level section (say, `observability:`) in a +future release must not break older binaries reading a newer config. + +The obvious gap — an operator keeping the pre-migration top-level +schema (`inbound:`, `outbound:`, `identity:`, `bypass:`, `routes:`) +would have their config silently accepted with those keys dropped — +is closed by `config.Validate`, which errors when either pipeline +list is empty. The error message names the likely cause so the +operator is pointed at the migration, not left wondering why +authentication isn't happening. + +## Cross-references + +- `authbridge/authlib/pipeline/configurable.go` — the interface. +- `authbridge/authlib/pipeline/README.md` — how plugins compose and + run; Configure's place in the lifecycle. +- `authbridge/authlib/config/config.go` — `PluginEntry` YAML shape and + parsing. +- `authbridge/authlib/plugins/registry.go` — how Build calls Configure. diff --git a/authbridge/authlib/plugins/defaults.go b/authbridge/authlib/plugins/defaults.go deleted file mode 100644 index 7467f62d7..000000000 --- a/authbridge/authlib/plugins/defaults.go +++ /dev/null @@ -1,27 +0,0 @@ -package plugins - -import ( - "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" -) - -var ( - DefaultInboundPlugins = []string{"jwt-validation"} - DefaultOutboundPlugins = []string{"token-exchange"} -) - -func DefaultInboundPipeline(a *auth.Auth) (*pipeline.Pipeline, error) { - return Build(DefaultInboundPlugins, a) -} - -func DefaultOutboundPipeline(a *auth.Auth) (*pipeline.Pipeline, error) { - return Build(DefaultOutboundPlugins, a) -} - -// WaypointInboundPipeline creates an inbound pipeline for waypoint mode -// where audience is derived per-request from the destination host. -func WaypointInboundPipeline(a *auth.Auth) (*pipeline.Pipeline, error) { - jwtPlugin := NewJWTValidation(a, WithAudienceDeriver(routing.ServiceNameFromHost)) - return pipeline.New([]pipeline.Plugin{jwtPlugin}) -} diff --git a/authbridge/authlib/plugins/jwtvalidation.go b/authbridge/authlib/plugins/jwtvalidation.go index 51eb72f3c..5dc909c77 100644 --- a/authbridge/authlib/plugins/jwtvalidation.go +++ b/authbridge/authlib/plugins/jwtvalidation.go @@ -1,52 +1,263 @@ package plugins import ( + "bytes" "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "strings" + "sync/atomic" "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/bypass" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) -// JWTValidation is a pipeline plugin that validates inbound JWTs. -// It delegates to auth.HandleInbound and populates pctx.Claims on success. -type JWTValidation struct { - auth *auth.Auth - audienceDeriver func(string) string -} +// jwtValidationConfig is the plugin's local config schema. See +// authlib/plugins/CONVENTIONS.md for the decode → applyDefaults → +// validate pattern. +type jwtValidationConfig struct { + // Issuer is the JWT `iss` claim expected on inbound tokens. + Issuer string `json:"issuer"` + + // JWKSURL points at the JWKS endpoint used to verify signatures. + // When empty, derived from Issuer using Keycloak's convention + // (/protocol/openid-connect/certs). + JWKSURL string `json:"jwks_url"` -// JWTValidationOption configures the JWTValidation plugin. -type JWTValidationOption func(*JWTValidation) + // Audience is the literal audience value expected on inbound + // tokens. One of {Audience, AudienceFile, AudienceMode:"per-host"} + // is required. + Audience string `json:"audience"` -// WithAudienceDeriver sets a function that derives the expected JWT audience -// from the request host. Used in waypoint mode where audience varies per-request. -func WithAudienceDeriver(f func(string) string) JWTValidationOption { - return func(j *JWTValidation) { j.audienceDeriver = f } + // AudienceFile reads the expected audience from a file. Used + // together with client-registration's /shared/client-id.txt. The + // file may not exist at Configure time; a background poll started + // by Init waits for it and updates the plugin when it appears. + // + // Note: an empty-string value is treated as "unset" — applyDefaults + // will fill in /shared/client-id.txt. To opt out of any file poll, + // supply an explicit Audience instead; the file default only kicks + // in when both Audience and AudienceFile are empty. + AudienceFile string `json:"audience_file"` + + // AudienceMode chooses how the expected audience is resolved: + // "static" (default) uses Audience/AudienceFile; "per-host" derives + // it from pctx.Host via routing.ServiceNameFromHost (waypoint mode). + AudienceMode string `json:"audience_mode"` + + // BypassPaths are URL path globs (see authlib/bypass) that skip + // validation entirely. + BypassPaths []string `json:"bypass_paths"` } -func NewJWTValidation(a *auth.Auth, opts ...JWTValidationOption) *JWTValidation { - p := &JWTValidation{auth: a} - for _, opt := range opts { - opt(p) +func (c *jwtValidationConfig) applyDefaults() { + if c.JWKSURL == "" && c.Issuer != "" { + c.JWKSURL = strings.TrimRight(c.Issuer, "/") + "/protocol/openid-connect/certs" + } + if c.AudienceMode == "" { + c.AudienceMode = "static" + } + // When neither Audience nor AudienceFile is set, fall back to the + // Kagenti convention: client-registration writes the agent's client + // ID (which doubles as the inbound audience) to this path. + // Deployments that don't run client-registration should set + // Audience explicitly — the Configure-time read is best-effort and + // Init's poll will give up silently if ctx is cancelled. + if c.AudienceMode == "static" && c.Audience == "" && c.AudienceFile == "" { + c.AudienceFile = "/shared/client-id.txt" + } + if len(c.BypassPaths) == 0 { + c.BypassPaths = bypass.DefaultPatterns + } +} + +func (c *jwtValidationConfig) validate() error { + if c.Issuer == "" { + return errors.New("issuer is required") + } + if c.JWKSURL == "" { + return errors.New("jwks_url could not be derived; set it explicitly") } - return p + switch c.AudienceMode { + case "static": + // applyDefaults guarantees AudienceFile is set whenever both + // Audience and AudienceFile arrived empty, so no check here — + // the plugin will always have either a literal audience or a + // file path to poll. + case "per-host": + // Audience derived at request time from pctx.Host — nothing to check. + default: + return fmt.Errorf("audience_mode must be static or per-host, got %q", c.AudienceMode) + } + return nil } +// JWTValidation validates inbound JWTs. Internal state is built during +// Configure and later updated by Init's background audience-file poller +// via auth.UpdateIdentity, which is atomic with respect to in-flight +// requests. +type JWTValidation struct { + cfg jwtValidationConfig + inner *auth.Auth + audienceDeriver func(string) string + + // bgCancel stops the background audience-file poller started by + // Init. It's created with context.Background() (not Init's ctx) so + // the poller's lifetime is the plugin's lifetime, not Start's + // 60-second budget — otherwise a slow client-registration during + // pod boot would orphan the plugin after the initCtx deadline. + // + // Held in an atomic.Pointer so a future caller can invoke Shutdown + // from a goroutine other than the one that ran Init without racing + // the Init assignment. Today the pipeline serializes Start / Stop, + // so the lock-free guarantee is future-proofing rather than a + // correctness fix for current callers. + bgCancel atomic.Pointer[context.CancelFunc] +} + +// NewJWTValidation constructs an unconfigured plugin. Configure must be +// called before the pipeline accepts traffic. +func NewJWTValidation() *JWTValidation { return &JWTValidation{} } + func (p *JWTValidation) Name() string { return "jwt-validation" } func (p *JWTValidation) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{Writes: []string{"security"}} } +// Configure decodes the plugin's config subtree, applies defaults, +// validates, and constructs the internal auth handler. If AudienceFile +// is set but the file isn't yet readable (client-registration still +// provisioning during pod boot), the handler is created with an empty +// audience and Init's goroutine fills it in when the file appears. +func (p *JWTValidation) Configure(raw json.RawMessage) error { + var c jwtValidationConfig + if len(raw) > 0 { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&c); err != nil { + return fmt.Errorf("jwt-validation config: %w", err) + } + } + // Capture whether audience_file arrived explicitly so the boot-time + // WARN can distinguish "operator pointed at the wrong path" from + // "defaulted to the Kagenti convention and you might not have + // noticed." applyDefaults fills AudienceFile in when both audience + // and audience_file are empty, erasing the signal. + audienceFileExplicit := c.AudienceFile != "" + c.applyDefaults() + if err := c.validate(); err != nil { + return fmt.Errorf("jwt-validation config: %w", err) + } + p.cfg = c + + if c.AudienceMode == "per-host" { + p.audienceDeriver = routing.ServiceNameFromHost + } + + audience := c.Audience + if audience == "" && c.AudienceFile != "" { + if v, err := config.ReadCredentialFile(c.AudienceFile); err == nil { + audience = v + } else { + // Boot-time visibility: operators see this in `kubectl logs` + // of the initial pod instead of chasing 503s from traffic + // that arrived before Init's poll filled the audience in. + // When the path was defaulted (not written in the YAML), + // spell that out so non-Kagenti deployers don't wonder why + // the plugin is asking for /shared/client-id.txt. + if audienceFileExplicit { + slog.Warn("jwt-validation: audience_file not yet readable; Init will poll in background", + "path", c.AudienceFile, "error", err) + } else { + slog.Warn("jwt-validation: audience_file defaulted to Kagenti convention and not yet readable; "+ + "Init will poll in background. Set audience (literal value) or audience_file (explicit path) "+ + "if you are not running under Kagenti.", + "path", c.AudienceFile, "error", err) + } + } + } + + matcher, err := bypass.NewMatcher(c.BypassPaths) + if err != nil { + return fmt.Errorf("jwt-validation bypass patterns: %w", err) + } + verifier := validation.NewLazyJWKSVerifier(c.JWKSURL, c.Issuer) + p.inner = auth.New(auth.Config{ + Verifier: verifier, + Bypass: matcher, + Identity: auth.IdentityConfig{Audience: audience}, + }) + return nil +} + +// Init starts a background poll for AudienceFile when the file wasn't +// readable during Configure. +// +// The ctx passed to Init bounds synchronous initialization — not +// long-running watchers. The poller is spawned with a process-lifetime +// context (context.Background() + a cancel func stored in bgCancel) +// so Pipeline.Start's 60s init budget doesn't kill it. Shutdown +// cancels the watcher when the process is shutting down. +func (p *JWTValidation) Init(_ context.Context) error { + if p.cfg.AudienceFile == "" || p.cfg.Audience != "" || p.inner.Ready() { + return nil + } + // Defensive guard: pipeline.Start contract says Init runs exactly + // once per process, but a double-call would otherwise leak the + // first goroutine (the first cancel func would be dropped on the + // floor when we replaced bgCancel). + if p.bgCancel.Load() != nil { + return nil + } + bgCtx, cancel := context.WithCancel(context.Background()) + p.bgCancel.Store(&cancel) + go func() { + v, err := config.WaitForCredentialFile(bgCtx, p.cfg.AudienceFile) + if err != nil { + // Only reached when Shutdown cancels bgCtx — the file-wait + // doesn't have a deadline of its own. Log at Debug so clean + // shutdowns don't spam the log. + slog.Debug("jwt-validation: audience_file wait stopped", + "path", p.cfg.AudienceFile, "error", err) + return + } + p.inner.UpdateIdentity(auth.IdentityConfig{Audience: v}, nil) + slog.Info("jwt-validation: audience loaded from file", + "path", p.cfg.AudienceFile, "audience", v) + }() + return nil +} + +// Shutdown cancels the background audience-file poller if one was +// started by Init. Called by Pipeline.Stop during process shutdown. +// Safe to call more than once — the atomic swap makes the second call +// a no-op. +func (p *JWTValidation) Shutdown(_ context.Context) error { + if cancel := p.bgCancel.Swap(nil); cancel != nil { + (*cancel)() + } + return nil +} + func (p *JWTValidation) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { + if p.inner == nil { + return pipeline.DenyStatus(503, "upstream.unreachable", "jwt-validation not configured") + } authHeader := pctx.Headers.Get("Authorization") path := pctx.Path - var audience string if p.audienceDeriver != nil { audience = p.audienceDeriver(pctx.Host) } - result := p.auth.HandleInbound(ctx, authHeader, path, audience) + result := p.inner.HandleInbound(ctx, authHeader, path, audience) if result.Action == auth.ActionDeny { // result.DenyReason carries the specific failure (missing header, // audience mismatch, expired, etc.). Pick a code whose default @@ -66,3 +277,38 @@ func (p *JWTValidation) OnRequest(ctx context.Context, pctx *pipeline.Context) p func (p *JWTValidation) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { return pipeline.Action{Type: pipeline.Continue} } + +// Ready reports whether the plugin can validate traffic. auth.Auth's +// Ready() returns true once Identity.Audience is non-empty, which the +// plugin's synchronous-load path in Configure or the async poll in +// Init (via auth.UpdateIdentity) arranges. Per-host mode skips the +// audience check because the audience is derived from pctx.Host per +// request rather than loaded up front. +func (p *JWTValidation) Ready() bool { + if p.inner == nil { + return false + } + if p.cfg.AudienceMode == "per-host" { + return true + } + return p.inner.Ready() +} + +// Stats returns the plugin's counter store for the /stats aggregator +// (see plugins.CollectStats). Returns nil when Configure hasn't run +// yet — aggregation code tolerates nils. +func (p *JWTValidation) Stats() *auth.Stats { + if p.inner == nil { + return nil + } + return p.inner.Stats +} + +// Compile-time interface checks. +var ( + _ pipeline.Configurable = (*JWTValidation)(nil) + _ pipeline.Initializer = (*JWTValidation)(nil) + _ pipeline.Shutdowner = (*JWTValidation)(nil) + _ pipeline.Readier = (*JWTValidation)(nil) + _ StatsSource = (*JWTValidation)(nil) +) diff --git a/authbridge/authlib/plugins/plugins_test.go b/authbridge/authlib/plugins/plugins_test.go index d35d79c83..7df77ba15 100644 --- a/authbridge/authlib/plugins/plugins_test.go +++ b/authbridge/authlib/plugins/plugins_test.go @@ -5,134 +5,418 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" + "strings" "testing" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/cache" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/exchange" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) -type mockVerifier struct { - claims *validation.Claims - err error +// TestAuthbridgeCombinedYAML_Loads asserts that the in-repo default +// config consumed by the combined sidecar image +// (authbridge/authproxy/authbridge-combined.yaml) parses, env-expands, +// and produces working pipelines. Since that YAML leans on per-plugin +// defaults for file paths and bypass patterns, a future rename of any +// default constant would silently break the shipped image unless this +// test fails. It's cheaper to fail in CI than in a running pod. +func TestAuthbridgeCombinedYAML_Loads(t *testing.T) { + // The canonical file path is relative to this test file — + // plugins_test.go lives in authlib/plugins/, the YAML in + // authproxy/. Go up two directories, across into authproxy/. + yamlPath := filepath.Join("..", "..", "authproxy", "authbridge-combined.yaml") + if _, err := os.Stat(yamlPath); err != nil { + t.Skipf("authbridge-combined.yaml not found (repo layout changed?): %v", err) + } + + envs := map[string]string{ + "ISSUER": "http://keycloak.localtest.me:8080/realms/kagenti", + "KEYCLOAK_URL": "http://keycloak-service.keycloak.svc:8080", + "KEYCLOAK_REALM": "kagenti", + "DEFAULT_OUTBOUND_POLICY": "passthrough", + "TOKEN_URL": "", // intentionally empty: the plugin should derive from keycloak_url + realm + } + for k, v := range envs { + t.Setenv(k, v) + } + + cfg, err := config.Load(yamlPath) + if err != nil { + t.Fatalf("Load(%s): %v", yamlPath, err) + } + if cfg.Mode != config.ModeEnvoySidecar { + t.Errorf("mode = %q, want %q", cfg.Mode, config.ModeEnvoySidecar) + } + if err := config.Validate(cfg); err != nil { + t.Errorf("Validate: %v", err) + } + + // Build both pipelines. Any plugin whose Configure rejects the + // env-expanded config subtree (e.g. because a default path moved + // but the YAML still relies on it) fails the build here. + if _, err := Build(cfg.Pipeline.Inbound.Plugins); err != nil { + t.Errorf("Build inbound: %v", err) + } + if _, err := Build(cfg.Pipeline.Outbound.Plugins); err != nil { + t.Errorf("Build outbound: %v", err) + } } -func (m *mockVerifier) Verify(_ context.Context, _ string, _ string) (*validation.Claims, error) { - return m.claims, m.err +// --- JWTValidation: Configure --- + +func TestJWTValidation_Configure_MissingIssuer(t *testing.T) { + p := NewJWTValidation() + err := p.Configure([]byte(`{}`)) + if err == nil { + t.Fatal("expected error for missing issuer") + } } -// --- JWTValidation Tests --- +func TestJWTValidation_Configure_UnknownField(t *testing.T) { + p := NewJWTValidation() + err := p.Configure([]byte(`{"issuer":"http://ex","audience":"a","not_a_field":"x"}`)) + if err == nil { + t.Fatal("expected error for unknown field; DisallowUnknownFields should reject") + } +} -func TestJWTValidation_ValidToken(t *testing.T) { - a := auth.New(auth.Config{ - Verifier: &mockVerifier{claims: &validation.Claims{Subject: "user-1", ClientID: "agent"}}, - Identity: auth.IdentityConfig{Audience: "my-agent"}, - }) - plugin := NewJWTValidation(a) +// Legacy test obsolete: applyDefaults now sets audience_file to +// /shared/client-id.txt when neither audience nor audience_file is +// supplied, so this scenario no longer reaches validate(). The +// replacement test is TestJWTValidation_Configure_DefaultAudienceFile. - pctx := &pipeline.Context{ - Direction: pipeline.Inbound, - Path: "/api/test", - Headers: http.Header{"Authorization": []string{"Bearer valid-token"}}, +func TestJWTValidation_Configure_PerHost(t *testing.T) { + p := NewJWTValidation() + // per-host mode does not require an audience field. + err := p.Configure([]byte(`{"issuer":"http://ex","audience_mode":"per-host"}`)) + if err != nil { + t.Fatalf("per-host mode should not require audience: %v", err) } - action := plugin.OnRequest(context.Background(), pctx) - if action.Type != pipeline.Continue { - t.Errorf("got %v, want Continue", action.Type) + if p.audienceDeriver == nil { + t.Error("per-host mode should set audienceDeriver") } - if pctx.Claims == nil { - t.Fatal("expected pctx.Claims to be populated") +} + +// When neither audience nor audience_file is supplied, the plugin +// defaults audience_file to /shared/client-id.txt (the Kagenti +// client-registration convention). Omitting both in the YAML must not +// fail validation — the file read is best-effort with a background +// fallback poll. +func TestJWTValidation_Configure_DefaultAudienceFile(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://ex"}`)); err != nil { + t.Fatalf("Configure with defaults: %v", err) } - if pctx.Claims.Subject != "user-1" { - t.Errorf("subject = %q, want user-1", pctx.Claims.Subject) + if p.cfg.AudienceFile != "/shared/client-id.txt" { + t.Errorf("AudienceFile = %q, want /shared/client-id.txt", p.cfg.AudienceFile) } } -func TestJWTValidation_InvalidToken(t *testing.T) { - a := auth.New(auth.Config{ - Verifier: &mockVerifier{err: errorf("token expired")}, - Identity: auth.IdentityConfig{Audience: "my-agent"}, - }) - plugin := NewJWTValidation(a) +// bypass_paths defaults to bypass.DefaultPatterns so health / .well-known +// endpoints don't reject every JWT-less probe from kubelet. +func TestJWTValidation_Configure_DefaultBypassPaths(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://ex","audience":"a"}`)); err != nil { + t.Fatalf("Configure: %v", err) + } + if len(p.cfg.BypassPaths) == 0 { + t.Fatal("expected default bypass paths") + } +} - pctx := &pipeline.Context{ - Direction: pipeline.Inbound, - Path: "/api/test", - Headers: http.Header{"Authorization": []string{"Bearer bad-token"}}, +// Inline audience suppresses the audience_file default: operators who +// supply a literal audience must not also get a surprise file read. +func TestJWTValidation_Configure_InlineAudienceSuppressesFileDefault(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://ex","audience":"literal"}`)); err != nil { + t.Fatalf("Configure: %v", err) } - action := plugin.OnRequest(context.Background(), pctx) - if action.Type != pipeline.Reject { - t.Fatalf("got %v, want Reject", action.Type) + if p.cfg.AudienceFile != "" { + t.Errorf("AudienceFile = %q, want empty (inline audience should suppress default)", p.cfg.AudienceFile) } - status, _, _ := action.Violation.Render(); if status != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", status) +} + +func TestJWTValidation_Configure_DefaultsJWKSFromIssuer(t *testing.T) { + p := NewJWTValidation() + err := p.Configure([]byte(`{"issuer":"http://keycloak/realms/kagenti","audience":"a"}`)) + if err != nil { + t.Fatalf("Configure: %v", err) + } + // The derived JWKS URL is applied during Configure — we can't + // inspect it directly because it's buried inside the verifier, but + // if the inner auth handler is nil we know Configure bailed. + if p.inner == nil { + t.Fatal("Configure produced no inner auth handler") } } -func TestJWTValidation_MissingHeader(t *testing.T) { - a := auth.New(auth.Config{ - Verifier: &mockVerifier{claims: &validation.Claims{Subject: "user"}}, - Identity: auth.IdentityConfig{Audience: "my-agent"}, - }) - plugin := NewJWTValidation(a) +func TestJWTValidation_Configure_AudienceFromFile(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "aud") + if err := os.WriteFile(f, []byte("my-agent"), 0600); err != nil { + t.Fatal(err) + } + p := NewJWTValidation() + raw := []byte(`{"issuer":"http://ex","audience_file":"` + f + `"}`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if !p.inner.Ready() { + t.Error("expected inner.Ready() == true after synchronous audience load") + } +} - pctx := &pipeline.Context{ - Direction: pipeline.Inbound, - Path: "/api/test", - Headers: http.Header{}, +// --- JWTValidation: Ready --- + +// Synchronous audience load → plugin reports ready immediately after +// Configure. The /readyz probe sees this on the kubelet's first check. +func TestJWTValidation_Ready_AfterSyncLoad(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "aud") + if err := os.WriteFile(f, []byte("my-agent"), 0600); err != nil { + t.Fatal(err) + } + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://ex","audience_file":"` + f + `"}`)); err != nil { + t.Fatalf("Configure: %v", err) + } + if !p.Ready() { + t.Error("expected Ready() == true after synchronous audience_file load") + } +} + +// Missing audience_file → plugin not ready until Init's poller flips +// it. Without per-plugin Ready(), /readyz would return 200 and the +// pod would get traffic that immediately 503s. +func TestJWTValidation_Ready_PendingWithoutFile(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://ex","audience_file":"/does/not/exist"}`)); err != nil { + t.Fatalf("Configure: %v", err) + } + if p.Ready() { + t.Error("expected Ready() == false when audience_file is missing") } - action := plugin.OnRequest(context.Background(), pctx) +} + +// Per-host mode derives audience per request; no deferred state. +// Must be always-ready so waypoint deployments don't stay unready. +func TestJWTValidation_Ready_PerHostAlwaysReady(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://ex","audience_mode":"per-host"}`)); err != nil { + t.Fatalf("Configure: %v", err) + } + if !p.Ready() { + t.Error("expected Ready() == true in per-host mode") + } +} + +// --- JWTValidation: OnRequest --- + +func TestJWTValidation_OnRequest_NotConfigured(t *testing.T) { + p := NewJWTValidation() + action := p.OnRequest(context.Background(), &pipeline.Context{Headers: http.Header{}}) if action.Type != pipeline.Reject { - t.Fatalf("got %v, want Reject", action.Type) + t.Errorf("got %v, want Reject for unconfigured plugin", action.Type) } - status, _, _ := action.Violation.Render(); if status != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", status) +} + +// --- TokenExchange: Configure --- + +func TestTokenExchange_Configure_MissingTokenURL(t *testing.T) { + p := NewTokenExchange() + err := p.Configure([]byte(`{"identity":{"type":"client-secret","client_id":"c","client_secret":"s"}}`)) + if err == nil { + t.Fatal("expected error for missing token_url") } } -func TestJWTValidation_WithAudienceDeriver(t *testing.T) { - var receivedAudience string - verifier := &mockVerifier{claims: &validation.Claims{Subject: "user"}} - a := auth.New(auth.Config{ - Verifier: &captureAudienceVerifier{inner: verifier, captured: &receivedAudience}, - Identity: auth.IdentityConfig{Audience: "default-aud"}, - }) - plugin := NewJWTValidation(a, WithAudienceDeriver(func(host string) string { - return "derived-from-" + host - })) +func TestTokenExchange_Configure_DerivesTokenURL(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "keycloak_url":"http://keycloak:8080", + "keycloak_realm":"kagenti", + "identity":{"type":"client-secret","client_id":"c","client_secret":"s"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + want := "http://keycloak:8080/realms/kagenti/protocol/openid-connect/token" + if p.cfg.TokenURL != want { + t.Errorf("token_url = %q, want %q", p.cfg.TokenURL, want) + } +} - pctx := &pipeline.Context{ - Direction: pipeline.Inbound, - Host: "target-svc", - Path: "/api", - Headers: http.Header{"Authorization": []string{"Bearer token"}}, +// Identity file paths default to Kagenti conventions when the operator +// doesn't supply them. Inline values suppress the default. +func TestTokenExchange_Configure_DefaultIdentityPaths_SPIFFE(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://t", + "identity":{"type":"spiffe"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) } - action := plugin.OnRequest(context.Background(), pctx) - if action.Type != pipeline.Continue { - t.Fatalf("got %v, want Continue", action.Type) + if p.cfg.Identity.ClientIDFile != "/shared/client-id.txt" { + t.Errorf("ClientIDFile = %q, want /shared/client-id.txt", p.cfg.Identity.ClientIDFile) } - if receivedAudience != "derived-from-target-svc" { - t.Errorf("audience = %q, want derived-from-target-svc", receivedAudience) + if p.cfg.Identity.JWTSVIDPath != "/opt/jwt_svid.token" { + t.Errorf("JWTSVIDPath = %q, want /opt/jwt_svid.token", p.cfg.Identity.JWTSVIDPath) } } -// --- TokenExchange Tests --- +func TestTokenExchange_Configure_DefaultIdentityPaths_ClientSecret(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://t", + "identity":{"type":"client-secret"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if p.cfg.Identity.ClientIDFile != "/shared/client-id.txt" { + t.Errorf("ClientIDFile = %q, want /shared/client-id.txt", p.cfg.Identity.ClientIDFile) + } + if p.cfg.Identity.ClientSecretFile != "/shared/client-secret.txt" { + t.Errorf("ClientSecretFile = %q, want /shared/client-secret.txt", p.cfg.Identity.ClientSecretFile) + } +} -func TestTokenExchange_Passthrough(t *testing.T) { - router, _ := routing.NewRouter("passthrough", []routing.Route{}) - a := auth.New(auth.Config{Router: router}) - plugin := NewTokenExchange(a) +// Inline identity values must suppress the file defaults, otherwise an +// operator who writes inline credentials could be silently overridden +// by a pre-existing file on the mount point. +func TestTokenExchange_Configure_InlineIdentitySuppressesFileDefaults(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://t", + "identity":{"type":"client-secret","client_id":"c","client_secret":"s"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if p.cfg.Identity.ClientIDFile != "" { + t.Errorf("ClientIDFile = %q, want empty", p.cfg.Identity.ClientIDFile) + } + if p.cfg.Identity.ClientSecretFile != "" { + t.Errorf("ClientSecretFile = %q, want empty", p.cfg.Identity.ClientSecretFile) + } +} + +func TestTokenExchange_Configure_DefaultRoutesFile(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://t", + "identity":{"type":"client-secret","client_id":"c","client_secret":"s"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if p.cfg.Routes.File != "/etc/authproxy/routes.yaml" { + t.Errorf("Routes.File = %q, want /etc/authproxy/routes.yaml", p.cfg.Routes.File) + } +} + +func TestTokenExchange_Configure_DefaultsPassthrough(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://token", + "identity":{"type":"client-secret","client_id":"c","client_secret":"s"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if p.cfg.DefaultPolicy != "passthrough" { + t.Errorf("default_policy = %q, want passthrough", p.cfg.DefaultPolicy) + } +} + +func TestTokenExchange_Configure_InvalidDefaultPolicy(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://token", + "default_policy":"nope", + "identity":{"type":"client-secret","client_id":"c","client_secret":"s"} + }`) + if err := p.Configure(raw); err == nil { + t.Fatal("expected error for invalid default_policy") + } +} + +// Identity type is still required — defaulting covers the *paths* to +// credential files, not the choice between SPIFFE and client-secret. +// Unknown types fall through to the default error branch. +func TestTokenExchange_Configure_IdentityValidation(t *testing.T) { + cases := []struct { + name string + raw string + }{ + {"type missing", `{"token_url":"http://t"}`}, + {"type unknown", `{"token_url":"http://t","identity":{"type":"whatever"}}`}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + p := NewTokenExchange() + if err := p.Configure([]byte(c.raw)); err == nil { + t.Error("expected error, got nil") + } + }) + } +} + +// --- TokenExchange: Ready --- + +func TestTokenExchange_Ready_InlineCredentials(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://t", + "identity":{"type":"client-secret","client_id":"c","client_secret":"s"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if !p.Ready() { + t.Error("expected Ready() == true with inline credentials") + } +} + +// Default /shared/* paths don't exist in the test environment, so +// Configure's sync load fails and Ready stays false. pollCredentials +// would flip it later; this test doesn't call Init. +func TestTokenExchange_Ready_PendingWithoutCredentials(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://t", + "identity":{"type":"client-secret"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if p.Ready() { + t.Error("expected Ready() == false when defaulted credential files don't exist") + } +} +// --- TokenExchange: OnRequest (end-to-end through Configure) --- + +func TestTokenExchange_Passthrough(t *testing.T) { + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://unused", + "default_policy":"passthrough", + "identity":{"type":"client-secret","client_id":"c","client_secret":"s"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } pctx := &pipeline.Context{ Direction: pipeline.Outbound, Host: "some-host", Headers: http.Header{"Authorization": []string{"Bearer user-token"}}, } - action := plugin.OnRequest(context.Background(), pctx) + action := p.OnRequest(context.Background(), pctx) if action.Type != pipeline.Continue { - t.Errorf("got %v, want Continue", action.Type) + t.Fatalf("got %v, want Continue", action.Type) } if pctx.Headers.Get("Authorization") != "Bearer user-token" { t.Error("headers should not be modified for passthrough") @@ -150,23 +434,21 @@ func TestTokenExchange_ExchangeSuccess(t *testing.T) { })) defer exchangeSrv.Close() - router, _ := routing.NewRouter("exchange", []routing.Route{}) - exchanger := exchange.NewClient(exchangeSrv.URL, &exchange.ClientSecretAuth{ - ClientID: "agent", ClientSecret: "secret", - }) - a := auth.New(auth.Config{ - Router: router, - Exchanger: exchanger, - Cache: cache.New(), - }) - plugin := NewTokenExchange(a) - + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"` + exchangeSrv.URL + `", + "default_policy":"exchange", + "identity":{"type":"client-secret","client_id":"agent","client_secret":"secret"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } pctx := &pipeline.Context{ Direction: pipeline.Outbound, Host: "target-svc", Headers: http.Header{"Authorization": []string{"Bearer user-token"}}, } - action := plugin.OnRequest(context.Background(), pctx) + action := p.OnRequest(context.Background(), pctx) if action.Type != pipeline.Continue { t.Fatalf("got %v, want Continue", action.Type) } @@ -182,59 +464,100 @@ func TestTokenExchange_ExchangeFailure(t *testing.T) { })) defer exchangeSrv.Close() - router, _ := routing.NewRouter("exchange", []routing.Route{}) - exchanger := exchange.NewClient(exchangeSrv.URL, &exchange.ClientSecretAuth{ - ClientID: "agent", ClientSecret: "secret", - }) - a := auth.New(auth.Config{ - Router: router, - Exchanger: exchanger, - }) - plugin := NewTokenExchange(a) - + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"` + exchangeSrv.URL + `", + "default_policy":"exchange", + "identity":{"type":"client-secret","client_id":"agent","client_secret":"secret"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } pctx := &pipeline.Context{ Direction: pipeline.Outbound, Host: "target-svc", Headers: http.Header{"Authorization": []string{"Bearer user-token"}}, } - action := plugin.OnRequest(context.Background(), pctx) + action := p.OnRequest(context.Background(), pctx) if action.Type != pipeline.Reject { t.Fatalf("got %v, want Reject", action.Type) } - status, _, _ := action.Violation.Render(); if status != http.StatusServiceUnavailable { + status, _, _ := action.Violation.Render() + if status != http.StatusServiceUnavailable { t.Errorf("status = %d, want 503", status) } } func TestTokenExchange_NoToken_Deny(t *testing.T) { - router, _ := routing.NewRouter("exchange", []routing.Route{}) - a := auth.New(auth.Config{ - Router: router, - NoTokenPolicy: auth.NoTokenPolicyDeny, - }) - plugin := NewTokenExchange(a) - + p := NewTokenExchange() + raw := []byte(`{ + "token_url":"http://unused", + "default_policy":"exchange", + "no_token_policy":"deny", + "identity":{"type":"client-secret","client_id":"c","client_secret":"s"} + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } pctx := &pipeline.Context{ Direction: pipeline.Outbound, Host: "target-svc", Headers: http.Header{}, } - action := plugin.OnRequest(context.Background(), pctx) + action := p.OnRequest(context.Background(), pctx) if action.Type != pipeline.Reject { t.Fatalf("got %v, want Reject", action.Type) } - status, _, _ := action.Violation.Render(); if status != http.StatusUnauthorized { + status, _, _ := action.Violation.Render() + if status != http.StatusUnauthorized { t.Errorf("status = %d, want 401", status) } } -// --- Registry/Build Tests --- +// --- Stats aggregation --- + +// CollectStats walks a pipeline and returns *auth.Stats from each +// plugin implementing StatsSource. Non-Configurable plugins (parsers) +// don't implement StatsSource, so they're skipped; the slice length +// reflects only plugins with observable counters. +func TestCollectStats_CollectsOnlyStatsSources(t *testing.T) { + jwt := NewJWTValidation() + if err := jwt.Configure([]byte(`{"issuer":"http://ex","audience":"a"}`)); err != nil { + t.Fatalf("jwt Configure: %v", err) + } + tok := NewTokenExchange() + if err := tok.Configure([]byte(`{"token_url":"http://t","identity":{"type":"client-secret","client_id":"c","client_secret":"s"}}`)); err != nil { + t.Fatalf("tok Configure: %v", err) + } + // a2a-parser does not implement StatsSource. + p, err := pipeline.New([]pipeline.Plugin{jwt, NewA2AParser(), tok}) + if err != nil { + t.Fatalf("New: %v", err) + } + got := CollectStats(p) + if len(got) != 2 { + t.Errorf("len(CollectStats) = %d, want 2 (jwt + tok, parser skipped)", len(got)) + } +} + +// Nil pipeline must not panic — callers often cons up a statsProvider +// closure that references both inbound and outbound pipelines, and +// one could legitimately be nil in a degenerate config. +func TestCollectStats_NilPipeline(t *testing.T) { + if got := CollectStats(nil); got != nil { + t.Errorf("CollectStats(nil) = %v, want nil", got) + } +} + +// --- Registry / Build --- func TestBuild_ValidNames(t *testing.T) { - a := auth.New(auth.Config{}) - p, err := Build([]string{"jwt-validation", "token-exchange"}, a) + p, err := Build([]config.PluginEntry{ + {Name: "a2a-parser"}, + {Name: "mcp-parser"}, + }) if err != nil { - t.Fatalf("Build returned error: %v", err) + t.Fatalf("Build: %v", err) } if p == nil { t.Fatal("expected non-nil pipeline") @@ -242,18 +565,16 @@ func TestBuild_ValidNames(t *testing.T) { } func TestBuild_UnknownName(t *testing.T) { - a := auth.New(auth.Config{}) - _, err := Build([]string{"nonexistent-plugin"}, a) + _, err := Build([]config.PluginEntry{{Name: "nonexistent-plugin"}}) if err == nil { t.Fatal("expected error for unknown plugin name") } } func TestBuild_EmptyList(t *testing.T) { - a := auth.New(auth.Config{}) - p, err := Build([]string{}, a) + p, err := Build([]config.PluginEntry{}) if err != nil { - t.Fatalf("Build returned error: %v", err) + t.Fatalf("Build: %v", err) } action := p.Run(context.Background(), &pipeline.Context{Headers: http.Header{}}) if action.Type != pipeline.Continue { @@ -261,43 +582,35 @@ func TestBuild_EmptyList(t *testing.T) { } } -func TestDefaultInboundPipeline(t *testing.T) { - a := auth.New(auth.Config{}) - p, err := DefaultInboundPipeline(a) - if err != nil { - t.Fatalf("DefaultInboundPipeline returned error: %v", err) +// A config: block on a plugin that doesn't implement Configurable is a +// startup error. Silent acceptance would hide typos (wrong plugin name) +// and stale config across refactors. +func TestBuild_ConfigForNonConfigurablePlugin(t *testing.T) { + _, err := Build([]config.PluginEntry{ + {Name: "a2a-parser", Config: []byte(`{"unused":true}`)}, + }) + if err == nil { + t.Fatal("expected error for config on non-Configurable plugin") } - if p == nil { - t.Fatal("expected non-nil pipeline") + // Error text is operator-facing contract — a future refactor that + // changes it must update this assertion intentionally. + if !strings.Contains(err.Error(), "does not accept configuration") { + t.Errorf("error %q does not match the operator-facing contract "+ + `"%q does not accept configuration"`, err, "a2a-parser") } } -func TestDefaultOutboundPipeline(t *testing.T) { - a := auth.New(auth.Config{}) - p, err := DefaultOutboundPipeline(a) - if err != nil { - t.Fatalf("DefaultOutboundPipeline returned error: %v", err) +// Configure errors surface through Build with the offending plugin's +// name so startup logs identify the broken entry without the operator +// having to read every plugin's error wording. +func TestBuild_ConfigureError(t *testing.T) { + _, err := Build([]config.PluginEntry{ + {Name: "jwt-validation", Config: []byte(`{}`)}, // missing issuer + }) + if err == nil { + t.Fatal("expected error for invalid jwt-validation config") } - if p == nil { - t.Fatal("expected non-nil pipeline") + if !strings.Contains(err.Error(), "jwt-validation") { + t.Errorf("error %q does not name the offending plugin", err) } } - -// --- Helpers --- - -type errString string - -func errorf(s string) error { return errString(s) } - -func (e errString) Error() string { return string(e) } - -// captureAudienceVerifier wraps a verifier and captures the audience parameter. -type captureAudienceVerifier struct { - inner *mockVerifier - captured *string -} - -func (v *captureAudienceVerifier) Verify(ctx context.Context, token string, audience string) (*validation.Claims, error) { - *v.captured = audience - return v.inner.Verify(ctx, token, audience) -} diff --git a/authbridge/authlib/plugins/plugintesting/plugintesting.go b/authbridge/authlib/plugins/plugintesting/plugintesting.go new file mode 100644 index 000000000..20b600ec9 --- /dev/null +++ b/authbridge/authlib/plugins/plugintesting/plugintesting.go @@ -0,0 +1,116 @@ +// Package plugintesting exposes stub plugin adapters for listener +// tests that need to inject pre-built *auth.Auth instances (for +// example, with a mock verifier or a mock exchanger) without going +// through the real plugins' Configure path. +// +// Production code must not import this package. It lives in a +// dedicated sub-package — rather than in authbridge/authlib/plugins — +// so that it can't be pulled into the production binary, and can't +// accidentally become an alternate public constructor API for +// jwt-validation / token-exchange. Plugins build their own auth.Auth +// from their own local config in production; this package exists +// only to keep the listener-test surface small. +package plugintesting + +import ( + "context" + "net/http" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" +) + +// JWTValidationStub mimics the jwt-validation plugin's OnRequest +// behavior but takes a pre-built *auth.Auth directly. Used by +// listener tests to assert listener-level behavior (reject vs. +// continue, header handling, body buffering) without standing up a +// full Configure-driven plugin. +// +// The Name() value matches the production plugin so session events +// and pipeline introspection show identical output in tests and +// production. +type JWTValidationStub struct { + inner *auth.Auth + audienceFromHost bool +} + +// NewJWTValidation wraps a pre-built auth handler as a pipeline +// plugin equivalent to jwt-validation. audienceFromHost=true +// mirrors the production audience_mode:per-host path, deriving the +// expected audience from pctx.Host at request time. +func NewJWTValidation(a *auth.Auth, audienceFromHost bool) *JWTValidationStub { + return &JWTValidationStub{inner: a, audienceFromHost: audienceFromHost} +} + +func (p *JWTValidationStub) Name() string { return "jwt-validation" } + +func (p *JWTValidationStub) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{Writes: []string{"security"}} +} + +func (p *JWTValidationStub) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { + authHeader := pctx.Headers.Get("Authorization") + var audience string + if p.audienceFromHost { + audience = routing.ServiceNameFromHost(pctx.Host) + } + result := p.inner.HandleInbound(ctx, authHeader, pctx.Path, audience) + if result.Action == auth.ActionDeny { + code := "auth.unauthorized" + if result.DenyStatus == http.StatusServiceUnavailable { + code = "upstream.unreachable" + } + return pipeline.DenyStatus(result.DenyStatus, code, result.DenyReason) + } + pctx.Claims = result.Claims + return pipeline.Action{Type: pipeline.Continue} +} + +func (p *JWTValidationStub) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// TokenExchangeStub mimics the token-exchange plugin's OnRequest +// behavior but takes a pre-built *auth.Auth directly. See +// JWTValidationStub's docstring for the rationale. +type TokenExchangeStub struct { + inner *auth.Auth +} + +func NewTokenExchange(a *auth.Auth) *TokenExchangeStub { + return &TokenExchangeStub{inner: a} +} + +func (p *TokenExchangeStub) Name() string { return "token-exchange" } + +func (p *TokenExchangeStub) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{} +} + +func (p *TokenExchangeStub) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { + authHeader := pctx.Headers.Get("Authorization") + result := p.inner.HandleOutbound(ctx, authHeader, pctx.Host) + switch result.Action { + case auth.ActionDeny: + code := "upstream.token-exchange-failed" + if result.DenyStatus == http.StatusForbidden { + code = "policy.forbidden" + } + return pipeline.DenyStatus(result.DenyStatus, code, result.DenyReason) + case auth.ActionReplaceToken: + pctx.Headers.Set("Authorization", "Bearer "+result.Token) + } + return pipeline.Action{Type: pipeline.Continue} +} + +func (p *TokenExchangeStub) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// BuildPipeline wraps a slice of plugins in a pipeline.Pipeline. +// Equivalent to pipeline.New but lives here so listener tests don't +// need to import the pipeline package just for construction. +func BuildPipeline(plugins []pipeline.Plugin, opts ...pipeline.Option) (*pipeline.Pipeline, error) { + return pipeline.New(plugins, opts...) +} diff --git a/authbridge/authlib/plugins/registry.go b/authbridge/authlib/plugins/registry.go index 141940892..a79059c31 100644 --- a/authbridge/authlib/plugins/registry.go +++ b/authbridge/authlib/plugins/registry.go @@ -3,31 +3,49 @@ package plugins import ( "fmt" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" ) -// PluginFactory creates a pipeline plugin from an auth.Auth instance. -type PluginFactory func(a *auth.Auth) pipeline.Plugin +// PluginFactory returns a fresh plugin instance. Plugins take no +// construction arguments — they receive their configuration through +// pipeline.Configurable.Configure during Build, and any external +// dependencies (JWKS cache, HTTP client, etc.) are built from that +// local config inside Configure. +type PluginFactory func() pipeline.Plugin var registry = map[string]PluginFactory{ - "jwt-validation": func(a *auth.Auth) pipeline.Plugin { return NewJWTValidation(a) }, - "token-exchange": func(a *auth.Auth) pipeline.Plugin { return NewTokenExchange(a) }, - "mcp-parser": func(_ *auth.Auth) pipeline.Plugin { return NewMCPParser() }, - "a2a-parser": func(_ *auth.Auth) pipeline.Plugin { return NewA2AParser() }, - "inference-parser": func(_ *auth.Auth) pipeline.Plugin { return NewInferenceParser() }, + "jwt-validation": func() pipeline.Plugin { return NewJWTValidation() }, + "token-exchange": func() pipeline.Plugin { return NewTokenExchange() }, + "mcp-parser": func() pipeline.Plugin { return NewMCPParser() }, + "a2a-parser": func() pipeline.Plugin { return NewA2AParser() }, + "inference-parser": func() pipeline.Plugin { return NewInferenceParser() }, } -// Build constructs a pipeline from an ordered list of plugin names. -// Returns an error if any name is not in the registry. -func Build(names []string, a *auth.Auth, opts ...pipeline.Option) (*pipeline.Pipeline, error) { - ps := make([]pipeline.Plugin, 0, len(names)) - for _, name := range names { - factory, ok := registry[name] +// Build constructs a pipeline from an ordered list of plugin entries. +// For every plugin that implements pipeline.Configurable, Build calls +// Configure with the entry's Config bytes (nil when omitted). Passing +// config to a plugin that doesn't implement Configurable is rejected so +// stale or misplaced config blocks fail at startup instead of being +// silently ignored. +// +// Unknown plugin names fail fast. +func Build(entries []config.PluginEntry, opts ...pipeline.Option) (*pipeline.Pipeline, error) { + ps := make([]pipeline.Plugin, 0, len(entries)) + for _, e := range entries { + factory, ok := registry[e.Name] if !ok { - return nil, fmt.Errorf("unknown plugin %q", name) + return nil, fmt.Errorf("unknown plugin %q", e.Name) } - ps = append(ps, factory(a)) + p := factory() + if c, ok := p.(pipeline.Configurable); ok { + if err := c.Configure(e.Config); err != nil { + return nil, fmt.Errorf("configure %q: %w", e.Name, err) + } + } else if len(e.Config) > 0 { + return nil, fmt.Errorf("plugin %q does not accept configuration", e.Name) + } + ps = append(ps, p) } return pipeline.New(ps, opts...) } diff --git a/authbridge/authlib/plugins/stats.go b/authbridge/authlib/plugins/stats.go new file mode 100644 index 000000000..86fb1694c --- /dev/null +++ b/authbridge/authlib/plugins/stats.go @@ -0,0 +1,43 @@ +package plugins + +import ( + "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" +) + +// StatsSource is an optional interface a plugin implements when it +// maintains observability counters worth exposing on the /stats +// endpoint. Lives in the plugins package (rather than pipeline) +// because auth.Stats is the shared counter type and the pipeline +// package has no reason to depend on auth. +// +// Plugins that don't report stats don't implement this interface; +// the host skips them during aggregation. +type StatsSource interface { + Stats() *auth.Stats +} + +// CollectStats walks a pipeline and returns every *auth.Stats +// exposed by a StatsSource-implementing plugin. Order matches plugin +// declaration order in the pipeline, which keeps the aggregated +// output deterministic. +// +// Returns nil (not an empty slice) when no plugins implement +// StatsSource, so callers can pass the result straight to +// auth.MergeStats without guarding against zero-length. +func CollectStats(p *pipeline.Pipeline) []*auth.Stats { + if p == nil { + return nil + } + var out []*auth.Stats + for _, plugin := range p.Plugins() { + src, ok := plugin.(StatsSource) + if !ok { + continue + } + if s := src.Stats(); s != nil { + out = append(out, s) + } + } + return out +} diff --git a/authbridge/authlib/plugins/tokenexchange.go b/authbridge/authlib/plugins/tokenexchange.go index 70778fe78..887e09938 100644 --- a/authbridge/authlib/plugins/tokenexchange.go +++ b/authbridge/authlib/plugins/tokenexchange.go @@ -1,34 +1,468 @@ package plugins import ( + "bytes" "context" + "encoding/json" + "errors" + "fmt" + "log/slog" "net/http" + "strings" + "sync/atomic" "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/spiffe" ) -// TokenExchange is a pipeline plugin that performs outbound token exchange. -// It delegates to auth.HandleOutbound and mutates pctx.Headers on token replacement. -type TokenExchange struct { - auth *auth.Auth +// tokenExchangeConfig is the plugin's local config schema. See +// authlib/plugins/CONVENTIONS.md for the pattern. +type tokenExchangeConfig struct { + // TokenURL is the OAuth token endpoint. Explicit value wins; else + // derived from KeycloakURL + KeycloakRealm using Keycloak's + // convention. + TokenURL string `json:"token_url"` + + // KeycloakURL and KeycloakRealm are a convenience for deriving + // TokenURL when the operator prefers to supply Keycloak base + realm + // rather than the full token endpoint. + KeycloakURL string `json:"keycloak_url"` + KeycloakRealm string `json:"keycloak_realm"` + + // DefaultPolicy is applied when a request's host matches no route: + // "passthrough" (default) forwards the request unchanged; + // "exchange" attempts a client-credentials exchange with an empty + // audience (usually fails — kept for rare use cases where the IdP + // allows it). + DefaultPolicy string `json:"default_policy"` + + // NoTokenPolicy controls how the plugin handles outbound requests + // that arrive without a bearer token: "client-credentials" does an + // unprompted client_credentials exchange; "allow" forwards + // unchanged; "deny" rejects. Default: "deny" in all modes. + // Operators who need a different behavior (e.g. waypoint's historic + // "allow" default) must set it explicitly per plugin entry. + NoTokenPolicy string `json:"no_token_policy"` + + // Identity carries client credentials used for token exchange. + Identity tokenExchangeIdentity `json:"identity"` + + // Routes drives host-to-audience matching. A host that matches no + // route falls through to DefaultPolicy. + Routes tokenExchangeRoutes `json:"routes"` + + // AudienceFromHost — when true, requests with no matching route use + // routing.ServiceNameFromHost(host) as the target audience. Used in + // waypoint mode. + AudienceFromHost bool `json:"audience_from_host"` +} + +type tokenExchangeIdentity struct { + // Type is one of "spiffe" or "client-secret". + Type string `json:"type"` + + // ClientID identifies the client in Keycloak. Explicit value wins; + // else read from ClientIDFile at Configure time (or by Init if the + // file isn't yet available). + ClientID string `json:"client_id"` + ClientIDFile string `json:"client_id_file"` + + // ClientSecret / ClientSecretFile are the client-secret credentials + // (type=client-secret). + ClientSecret string `json:"client_secret"` + ClientSecretFile string `json:"client_secret_file"` + + // JWTSVIDPath is the file path where spiffe-helper writes the + // JWT-SVID (type=spiffe). + JWTSVIDPath string `json:"jwt_svid_path"` } -func NewTokenExchange(a *auth.Auth) *TokenExchange { - return &TokenExchange{auth: a} +type tokenExchangeRoutes struct { + // File is an optional path to a routes.yaml file (see + // authlib/routing.LoadRoutes). + File string `json:"file"` + + // Rules are inline route entries; combined with routes loaded from + // File. + Rules []tokenExchangeRoute `json:"rules"` +} + +type tokenExchangeRoute struct { + Host string `json:"host"` + TargetAudience string `json:"target_audience"` + TokenScopes string `json:"token_scopes"` + TokenURL string `json:"token_url"` + Action string `json:"action"` + // Passthrough is accepted for backwards compatibility with the + // pre-migration routes.yaml shape (Action:"passthrough" is preferred). + Passthrough bool `json:"passthrough"` } +func (c *tokenExchangeConfig) applyDefaults() { + if c.TokenURL == "" && c.KeycloakURL != "" && c.KeycloakRealm != "" { + base := strings.TrimRight(c.KeycloakURL, "/") + "/realms/" + c.KeycloakRealm + c.TokenURL = base + "/protocol/openid-connect/token" + } + if c.DefaultPolicy == "" { + c.DefaultPolicy = "passthrough" + } + if c.NoTokenPolicy == "" { + c.NoTokenPolicy = auth.NoTokenPolicyDeny + } + // Kagenti file-system conventions for credential sources. Each + // default kicks in only when the matching inline value is also + // empty, so operators who supply inline credentials are never + // surprised by a file read. + // + // The route file default is safe because routing.LoadRoutes + // returns (nil, nil) when the file doesn't exist — missing routes + // means "no inline rules," which is the correct behavior for + // deployments without a mounted authproxy-routes ConfigMap. + if c.Routes.File == "" { + c.Routes.File = "/etc/authproxy/routes.yaml" + } + switch c.Identity.Type { + case "spiffe": + if c.Identity.ClientID == "" && c.Identity.ClientIDFile == "" { + c.Identity.ClientIDFile = "/shared/client-id.txt" + } + if c.Identity.JWTSVIDPath == "" { + c.Identity.JWTSVIDPath = "/opt/jwt_svid.token" + } + case "client-secret": + if c.Identity.ClientID == "" && c.Identity.ClientIDFile == "" { + c.Identity.ClientIDFile = "/shared/client-id.txt" + } + if c.Identity.ClientSecret == "" && c.Identity.ClientSecretFile == "" { + c.Identity.ClientSecretFile = "/shared/client-secret.txt" + } + } +} + +func (c *tokenExchangeConfig) validate() error { + if c.TokenURL == "" { + return errors.New("token_url is required (or set keycloak_url + keycloak_realm)") + } + switch c.DefaultPolicy { + case "exchange", "passthrough": + default: + return fmt.Errorf("default_policy must be exchange or passthrough, got %q", c.DefaultPolicy) + } + switch c.NoTokenPolicy { + case auth.NoTokenPolicyAllow, auth.NoTokenPolicyDeny, auth.NoTokenPolicyClientCredentials: + default: + return fmt.Errorf("no_token_policy must be allow, deny, or client-credentials, got %q", c.NoTokenPolicy) + } + switch c.Identity.Type { + case "spiffe", "client-secret": + // applyDefaults fills the identity file paths when the + // matching inline values are empty, so no per-field check + // here — the Configure path always ends up with a reachable + // credential source. Configure's best-effort read logs a + // WARN if the file isn't yet readable at boot, and Init's + // watcher retries in the background. + case "": + return errors.New("identity.type is required (spiffe or client-secret)") + default: + return fmt.Errorf("unknown identity.type %q", c.Identity.Type) + } + return nil +} + +// TokenExchange performs outbound token exchange. Configure builds the +// internal exchanger / router / auth handler; Init polls for credential +// files that weren't available at Configure time and swaps them in via +// auth.UpdateIdentity. +// +// cfg is immutable after Configure returns. Background goroutines +// read credential values into locals and feed them through +// auth.UpdateIdentity rather than mutating cfg, so OnRequest callers +// can safely read p.cfg without synchronization. +type TokenExchange struct { + cfg tokenExchangeConfig + inner *auth.Auth + + // bgCancel stops the background credential-file poller started by + // Init. See JWTValidation.bgCancel for why this can't be tied to + // Init's ctx, and for why it lives in an atomic.Pointer. + bgCancel atomic.Pointer[context.CancelFunc] + + // ready flips true when credentials are available — either because + // the synchronous read in Configure succeeded, because the operator + // supplied inline credentials, or because pollCredentials finished. + // auth.Auth.Ready() checks Identity.Audience which token-exchange + // doesn't set (it uses ClientID), so we track readiness locally. + ready atomic.Bool +} + +// NewTokenExchange constructs an unconfigured plugin. +func NewTokenExchange() *TokenExchange { return &TokenExchange{} } + func (p *TokenExchange) Name() string { return "token-exchange" } func (p *TokenExchange) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{} } +func (p *TokenExchange) Configure(raw json.RawMessage) error { + var c tokenExchangeConfig + if len(raw) > 0 { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&c); err != nil { + return fmt.Errorf("token-exchange config: %w", err) + } + } + // Track whether NoTokenPolicy arrived explicitly so we can warn + // waypoint-ish operators whose pre-migration deployments relied on + // the old mode-dependent default (waypoint=allow). applyDefaults + // fills in "deny" for everyone; without the explicit-set signal we + // can't tell the two cases apart at WARN time. + noTokenPolicyExplicit := c.NoTokenPolicy != "" + c.applyDefaults() + if err := c.validate(); err != nil { + return fmt.Errorf("token-exchange config: %w", err) + } + // The old NoTokenPolicyForMode defaulted to "client-credentials" for + // envoy-sidecar, "allow" for waypoint, and "deny" for proxy-sidecar. + // The new uniform default is "deny". Warn whenever no_token_policy + // was defaulted so operators relying on either of the old + // mode-specific behaviors find out at boot rather than via a + // traffic regression. + if !noTokenPolicyExplicit { + slog.Warn("token-exchange: no_token_policy defaulted to \"deny\"; " + + "prior defaults were mode-specific (envoy-sidecar: client-credentials, waypoint: allow, proxy-sidecar: deny). " + + "Set no_token_policy explicitly (allow | deny | client-credentials) to silence this warning and pin the behavior.") + } + + // Everything below runs against the local `c`, never `p.cfg`, so a + // partially-constructed failure leaves the plugin in its zero + // state. The final p.cfg / p.inner assignments happen only after + // all fallible construction has succeeded. + // + // Best-effort synchronous credential load. Missing files are + // tolerated; Init will retry. Log a boot-time WARN for each file + // that isn't yet readable so operators notice misconfiguration + // (wrong path, missing ConfigMap mount) in `kubectl logs` of the + // initial pod rather than discovering it later via 503s. + if c.Identity.ClientID == "" && c.Identity.ClientIDFile != "" { + if v, err := config.ReadCredentialFile(c.Identity.ClientIDFile); err == nil { + c.Identity.ClientID = v + } else { + slog.Warn("token-exchange: client_id_file not yet readable; Init will poll in background", + "path", c.Identity.ClientIDFile, "error", err) + } + } + if c.Identity.ClientSecret == "" && c.Identity.ClientSecretFile != "" { + if v, err := config.ReadCredentialFile(c.Identity.ClientSecretFile); err == nil { + c.Identity.ClientSecret = v + } else { + slog.Warn("token-exchange: client_secret_file not yet readable; Init will poll in background", + "path", c.Identity.ClientSecretFile, "error", err) + } + } + + clientAuth, err := buildClientAuthFrom(c.Identity.Type, + c.Identity.ClientID, c.Identity.ClientSecret, c.Identity.JWTSVIDPath) + if err != nil { + return fmt.Errorf("token-exchange: %w", err) + } + + exchanger := exchange.NewClient(c.TokenURL, clientAuth) + + router, err := buildRouterFrom(c.DefaultPolicy, c.Routes) + if err != nil { + return fmt.Errorf("token-exchange routes: %w", err) + } + + authCfg := auth.Config{ + Verifier: nil, // token-exchange doesn't validate inbound + Exchanger: exchanger, + Cache: cache.New(), + Router: router, + Identity: auth.IdentityConfig{ClientID: c.Identity.ClientID}, + NoTokenPolicy: c.NoTokenPolicy, + } + if c.AudienceFromHost { + authCfg.AudienceDeriver = routing.ServiceNameFromHost + } + // Commit. p.cfg + p.inner become visible atomically (from the + // caller's point of view — Configure itself is serialized by + // pipeline.Build). + p.cfg = c + p.inner = auth.New(authCfg) + + // Readiness: the synchronous credential load in Configure may have + // populated ClientID already. For SPIFFE identity we also need the + // SVID path to exist on disk (spiffe-helper has to have written it); + // the jwt-svid source re-reads on each exchange, so existence at + // this moment is a reasonable proxy. If the poll path is going to + // run, ready stays false until pollCredentials flips it. + if credentialsAreReady(c.Identity) { + p.ready.Store(true) + } + return nil +} + +// credentialsAreReady returns true iff the identity has everything it +// needs to do an exchange right now. Keeping this as a pure function +// lets pollCredentials and Configure share the predicate. +func credentialsAreReady(id tokenExchangeIdentity) bool { + if id.ClientID == "" { + return false + } + switch id.Type { + case "client-secret": + return id.ClientSecret != "" + case "spiffe": + // JWT-SVID source reads the file lazily; existence of ClientID + // is the signal that client-registration has completed. + return true + } + return false +} + +// buildClientAuthFrom constructs an exchange.ClientAuth from explicit +// args. Used both by Configure (against the local `c`, before p.cfg is +// assigned) and by pollCredentials (which reads its credential values +// from goroutine locals, not from the immutable p.cfg). Pure function +// — no reads from the receiver. +func buildClientAuthFrom(identityType, clientID, clientSecret, jwtSVIDPath string) (exchange.ClientAuth, error) { + switch identityType { + case "spiffe": + source := spiffe.NewFileJWTSource(jwtSVIDPath) + return &exchange.JWTAssertionAuth{ + ClientID: clientID, + AssertionType: "urn:ietf:params:oauth:client-assertion-type:jwt-spiffe", + TokenSource: source.FetchToken, + }, nil + case "client-secret": + return &exchange.ClientSecretAuth{ + ClientID: clientID, + ClientSecret: clientSecret, + }, nil + default: + return nil, fmt.Errorf("unknown identity.type %q", identityType) + } +} + +// buildRouterFrom is pure — no reads from the receiver. Used from +// Configure before p.cfg is assigned, so a build failure leaves the +// plugin in its zero state. +func buildRouterFrom(defaultPolicy string, routes tokenExchangeRoutes) (*routing.Router, error) { + var rules []routing.Route + if routes.File != "" { + fileRoutes, err := routing.LoadRoutes(routes.File) + if err != nil { + return nil, err + } + rules = append(rules, fileRoutes...) + } + for _, rc := range routes.Rules { + action := rc.Action + if action == "" && rc.Passthrough { + action = "passthrough" + } + rules = append(rules, routing.Route{ + Host: rc.Host, + Audience: rc.TargetAudience, + Scopes: rc.TokenScopes, + TokenEndpoint: rc.TokenURL, + Action: action, + }) + } + return routing.NewRouter(defaultPolicy, rules) +} + +// Init polls for credential files that weren't available during +// Configure. When both client_id and client_secret (or jwt_svid) become +// available, it builds a fresh client-auth and calls UpdateIdentity so +// in-flight exchanges pick up the new credentials. +// +// Init's ctx bounds synchronous init only; the poller runs on a +// process-lifetime context (see bgCancel) so Pipeline.Start's 60s +// budget doesn't kill it. Shutdown cancels the poller. +func (p *TokenExchange) Init(_ context.Context) error { + needID := p.cfg.Identity.ClientID == "" && p.cfg.Identity.ClientIDFile != "" + needSecret := p.cfg.Identity.ClientSecret == "" && p.cfg.Identity.ClientSecretFile != "" + if !needID && !needSecret { + return nil + } + // Defensive guard; see JWTValidation.Init for rationale. + if p.bgCancel.Load() != nil { + return nil + } + bgCtx, cancel := context.WithCancel(context.Background()) + p.bgCancel.Store(&cancel) + go p.pollCredentials(bgCtx, needID, needSecret) + return nil +} + +// pollCredentials reads credential files into local variables and +// applies them via auth.UpdateIdentity. It does not mutate p.cfg — +// keeping cfg immutable after Configure lets OnRequest and future +// readers access p.cfg without synchronization. +func (p *TokenExchange) pollCredentials(ctx context.Context, needID, needSecret bool) { + clientID := p.cfg.Identity.ClientID + clientSecret := p.cfg.Identity.ClientSecret + if needID { + v, err := config.WaitForCredentialFile(ctx, p.cfg.Identity.ClientIDFile) + if err != nil { + slog.Debug("token-exchange: client_id_file wait stopped", + "path", p.cfg.Identity.ClientIDFile, "error", err) + return + } + clientID = v + } + if needSecret { + v, err := config.WaitForCredentialFile(ctx, p.cfg.Identity.ClientSecretFile) + if err != nil { + slog.Debug("token-exchange: client_secret_file wait stopped", + "path", p.cfg.Identity.ClientSecretFile, "error", err) + return + } + clientSecret = v + } + clientAuth, err := buildClientAuthFrom(p.cfg.Identity.Type, clientID, clientSecret, p.cfg.Identity.JWTSVIDPath) + if err != nil { + slog.Warn("token-exchange: failed to rebuild client auth after credential load", "error", err) + return + } + p.inner.UpdateIdentity( + auth.IdentityConfig{ClientID: clientID}, + clientAuth, + ) + p.ready.Store(true) + // Deliberately log no client_id: some operators treat OAuth client + // IDs as sensitive (they appear in access logs, JWT sub claims, + // etc.). The signal here — credentials have loaded — doesn't need + // the identifier. + slog.Info("token-exchange: credentials loaded") +} + +// Shutdown cancels the background credential-file poller if one was +// started by Init. Called by Pipeline.Stop during process shutdown. +// Safe to call more than once. +func (p *TokenExchange) Shutdown(_ context.Context) error { + if cancel := p.bgCancel.Swap(nil); cancel != nil { + (*cancel)() + } + return nil +} + func (p *TokenExchange) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { + if p.inner == nil { + return pipeline.DenyStatus(503, "upstream.unreachable", "token-exchange not configured") + } authHeader := pctx.Headers.Get("Authorization") host := pctx.Host - result := p.auth.HandleOutbound(ctx, authHeader, host) + result := p.inner.HandleOutbound(ctx, authHeader, host) switch result.Action { case auth.ActionDeny: // Outbound denials almost always come from failed token exchange @@ -49,3 +483,32 @@ func (p *TokenExchange) OnRequest(ctx context.Context, pctx *pipeline.Context) p func (p *TokenExchange) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { return pipeline.Action{Type: pipeline.Continue} } + +// Ready reports whether client credentials are available for +// exchange. Flips true either at Configure time (if the synchronous +// credential read succeeded or the operator supplied inline values) +// or at pollCredentials success. Used by the pipeline-level /readyz +// aggregator so the kubelet holds traffic off the pod until +// credentials land. +func (p *TokenExchange) Ready() bool { + return p.ready.Load() +} + +// Stats returns the plugin's counter store for the /stats aggregator +// (see plugins.CollectStats). Returns nil when Configure hasn't run +// yet — aggregation code tolerates nils. +func (p *TokenExchange) Stats() *auth.Stats { + if p.inner == nil { + return nil + } + return p.inner.Stats +} + +// Compile-time interface checks. +var ( + _ pipeline.Configurable = (*TokenExchange)(nil) + _ pipeline.Initializer = (*TokenExchange)(nil) + _ pipeline.Shutdowner = (*TokenExchange)(nil) + _ pipeline.Readier = (*TokenExchange)(nil) + _ StatsSource = (*TokenExchange)(nil) +) diff --git a/authbridge/authproxy/authbridge-combined.yaml b/authbridge/authproxy/authbridge-combined.yaml index cee902baa..db889f8e6 100644 --- a/authbridge/authproxy/authbridge-combined.yaml +++ b/authbridge/authproxy/authbridge-combined.yaml @@ -1,23 +1,30 @@ # Default config for the combined AuthBridge sidecar image. # Env vars are expanded at startup by the unified binary's config loader. # These env vars match the operator's ConfigMap contract (authbridge-config). +# +# Plugin settings live under pipeline.*.plugins[].config; see +# authbridge/authlib/plugins/CONVENTIONS.md for the per-plugin schema. +# Fields omitted below fall back to plugin defaults — notably: +# jwt-validation: audience_file=/shared/client-id.txt, bypass_paths +# = .well-known/* + health/ready/live probes, +# jwks_url derived from issuer +# token-exchange: identity file paths under /shared/, jwt_svid_path +# =/opt/jwt_svid.token, routes.file +# =/etc/authproxy/routes.yaml mode: envoy-sidecar -inbound: - issuer: "${ISSUER}" -outbound: - token_url: "${TOKEN_URL}" - keycloak_url: "${KEYCLOAK_URL}" - keycloak_realm: "${KEYCLOAK_REALM}" - default_policy: "${DEFAULT_OUTBOUND_POLICY}" -identity: - client_id: "${CLIENT_ID}" - client_id_file: "/shared/client-id.txt" - client_secret_file: "/shared/client-secret.txt" -bypass: - inbound_paths: - - "/.well-known/*" - - "/healthz" - - "/readyz" - - "/livez" -routes: - file: "/etc/authproxy/routes.yaml" + +pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "${ISSUER}" + outbound: + plugins: + - name: token-exchange + config: + keycloak_url: "${KEYCLOAK_URL}" + keycloak_realm: "${KEYCLOAK_REALM}" + default_policy: "${DEFAULT_OUTBOUND_POLICY}" + identity: + type: "client-secret" diff --git a/authbridge/cmd/authbridge/README.md b/authbridge/cmd/authbridge/README.md index 6acd366a1..5300df0e8 100644 --- a/authbridge/cmd/authbridge/README.md +++ b/authbridge/cmd/authbridge/README.md @@ -77,54 +77,117 @@ The `--mode` flag can also be set in the YAML config. The flag overrides the con YAML with `${ENV_VAR}` expansion. Undefined env vars are preserved as-is (not expanded to empty). +The runtime config is intentionally thin — it covers the mode, the listener addresses, session tracking, and the plugin pipeline. Everything a plugin needs (issuer, token URL, credentials, routes, bypass paths) lives under its own `config:` block inside the pipeline entry. See [`authlib/plugins/CONVENTIONS.md`](../../authlib/plugins/CONVENTIONS.md) for the per-plugin decode / defaults / validate convention. + ### envoy-sidecar mode Drop-in replacement for `envoy-with-processor`. Used as a sidecar alongside Envoy in each agent pod. +Minimum viable config — every Kagenti-convention default applies: + ```yaml mode: envoy-sidecar -inbound: - jwks_url: "${JWKS_URL}" # or derived from token_url - issuer: "${ISSUER}" # or derived from keycloak_url + keycloak_realm -outbound: - token_url: "${TOKEN_URL}" # or derived from keycloak_url + keycloak_realm - keycloak_url: "${KEYCLOAK_URL}" # alternative to explicit token_url - keycloak_realm: "${KEYCLOAK_REALM}" # used with keycloak_url - default_policy: "passthrough" # passthrough (default) or exchange -identity: - type: spiffe # spiffe or client-secret - client_id: "${CLIENT_ID}" # or use client_id_file - client_id_file: "/shared/client-id.txt" # read from file (waits up to 60s) - client_secret_file: "/shared/client-secret.txt" - jwt_svid_path: "/opt/jwt_svid.token" # for SPIFFE JWT-SVID auth -bypass: - inbound_paths: # defaults: /.well-known/*, /healthz, /readyz, /livez - - "/.well-known/*" - - "/healthz" -routes: - file: "/etc/authproxy/routes.yaml" # load routes from file - rules: # or inline - - host: "target-service.**" - target_audience: "target" - token_scopes: "openid target-aud" + +pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "${ISSUER}" + + outbound: + plugins: + - name: token-exchange + config: + keycloak_url: "${KEYCLOAK_URL}" + keycloak_realm: "${KEYCLOAK_REALM}" + identity: + type: spiffe # spiffe or client-secret +``` + +Defaults filled in by the plugins (override any by setting them explicitly): + +| Plugin | Default | Value | +|---|---|---| +| jwt-validation | `jwks_url` | `/protocol/openid-connect/certs` | +| jwt-validation | `audience_file` | `/shared/client-id.txt` (client-registration convention) | +| jwt-validation | `bypass_paths` | `/.well-known/*`, `/healthz`, `/readyz`, `/livez` | +| jwt-validation | `audience_mode` | `static` | +| token-exchange | `token_url` | `/realms//protocol/openid-connect/token` | +| token-exchange | `default_policy` | `passthrough` | +| token-exchange | `no_token_policy` | `deny` | +| token-exchange | `routes.file` | `/etc/authproxy/routes.yaml` | +| token-exchange | `identity.client_id_file` | `/shared/client-id.txt` (both types) | +| token-exchange | `identity.client_secret_file` | `/shared/client-secret.txt` (client-secret type) | +| token-exchange | `identity.jwt_svid_path` | `/opt/jwt_svid.token` (spiffe type) | + +Full form — every field spelled out, for deployments that diverge from defaults: + +```yaml +mode: envoy-sidecar + +pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "${ISSUER}" + jwks_url: "${JWKS_URL}" # optional; derived from issuer + audience_file: "/shared/client-id.txt" # audience written by client-registration + bypass_paths: + - "/.well-known/*" + - "/healthz" + - "/readyz" + - "/livez" + + outbound: + plugins: + - name: token-exchange + config: + token_url: "${TOKEN_URL}" # or derived from keycloak_url + keycloak_realm + keycloak_url: "${KEYCLOAK_URL}" + keycloak_realm: "${KEYCLOAK_REALM}" + default_policy: "passthrough" # passthrough (default) or exchange + no_token_policy: "deny" # deny (default), allow, or client-credentials + identity: + type: spiffe # spiffe or client-secret + client_id_file: "/shared/client-id.txt" + client_secret_file: "/shared/client-secret.txt" + jwt_svid_path: "/opt/jwt_svid.token" + routes: + file: "/etc/authproxy/routes.yaml" # loaded when present, ignored when absent + rules: # inline; merged with file + - host: "target-service.**" + target_audience: "target" + token_scopes: "openid target-aud" ``` ### waypoint mode -Shared service for Istio ambient mesh. Derives audience from destination hostname automatically. +Shared service for Istio ambient mesh. `jwt-validation` derives audience from destination hostname when `audience_mode: per-host` is set. ```yaml mode: waypoint -inbound: - issuer: "${ISSUER}" -outbound: - keycloak_url: "${KEYCLOAK_URL}" - keycloak_realm: "${KEYCLOAK_REALM}" - default_policy: "exchange" -identity: - type: client-secret - client_id: "token-exchange-service" - client_secret: "${CLIENT_SECRET}" + +pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "${ISSUER}" + audience_mode: per-host # derive from pctx.Host per request + + outbound: + plugins: + - name: token-exchange + config: + keycloak_url: "${KEYCLOAK_URL}" + keycloak_realm: "${KEYCLOAK_REALM}" + default_policy: "exchange" + identity: + type: client-secret + client_id: "token-exchange-service" + client_secret: "${CLIENT_SECRET}" ``` ### proxy-sidecar mode @@ -133,34 +196,58 @@ Sidecar without Envoy. Reverse proxy validates inbound, forward proxy exchanges ```yaml mode: proxy-sidecar + listener: reverse_proxy_backend: "http://localhost:8081" -inbound: - issuer: "${ISSUER}" -outbound: - keycloak_url: "${KEYCLOAK_URL}" - keycloak_realm: "${KEYCLOAK_REALM}" -identity: - type: spiffe - client_id: "${CLIENT_ID}" - jwt_svid_path: "/opt/jwt_svid.token" + +pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "${ISSUER}" + audience_file: "/shared/client-id.txt" + + outbound: + plugins: + - name: token-exchange + config: + keycloak_url: "${KEYCLOAK_URL}" + keycloak_realm: "${KEYCLOAK_REALM}" + identity: + type: spiffe + client_id_file: "/shared/client-id.txt" + jwt_svid_path: "/opt/jwt_svid.token" +``` + +### Bare-name plugin entries + +Parsers and any other plugin that doesn't take configuration can appear as a bare name: + +```yaml +pipeline: + outbound: + plugins: + - name: token-exchange + config: { ... } + - mcp-parser # equivalent to { name: mcp-parser } + - inference-parser ``` -## URL Derivation +### URL Derivation -When explicit URLs are not set, they are derived automatically: +Each plugin derives missing URLs from what you supply: | Missing field | Derived from | Example | |---|---|---| -| `token_url` | `keycloak_url` + `keycloak_realm` | `http://keycloak:8080/realms/kagenti/protocol/openid-connect/token` | -| `issuer` | `keycloak_url` + `keycloak_realm` | `http://keycloak:8080/realms/kagenti` | -| `jwks_url` | `token_url` | `.../openid-connect/token` becomes `.../openid-connect/certs` | +| `jwt-validation.jwks_url` | `jwt-validation.issuer` | `/protocol/openid-connect/certs` | +| `token-exchange.token_url` | `token-exchange.keycloak_url` + `keycloak_realm` | `http://keycloak:8080/realms/kagenti/protocol/openid-connect/token` | Explicit values always take precedence over derived values. -## Credential File Waiting +### Credential File Waiting -When `client_id_file`, `client_secret_file`, or `jwt_svid_path` are configured, the binary polls for the file to exist (up to 60 seconds) before starting. This handles the startup race with client-registration and spiffe-helper sidecars. +When `audience_file` (jwt-validation) or `client_id_file` / `client_secret_file` / `jwt_svid_path` (token-exchange) are configured, the plugin first attempts a synchronous read at Configure time. If the file isn't readable yet (the common case during pod boot while client-registration is still provisioning), the plugin's `Init` spawns a background poll; once the file appears, `auth.UpdateIdentity` swaps the credentials into the live handler atomically. OnRequest returns 503 for traffic that arrives before credentials land. ## Logging diff --git a/authbridge/cmd/authbridge/listener/extauthz/server_test.go b/authbridge/cmd/authbridge/listener/extauthz/server_test.go index d51697cac..3fd7a39e8 100644 --- a/authbridge/cmd/authbridge/listener/extauthz/server_test.go +++ b/authbridge/cmd/authbridge/listener/extauthz/server_test.go @@ -14,7 +14,8 @@ import ( authpkg "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" "github.com/kagenti/kagenti-extensions/authbridge/authlib/cache" "github.com/kagenti/kagenti-extensions/authbridge/authlib/exchange" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/plugintesting" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) @@ -33,11 +34,11 @@ func (m *mockVerifier) Verify(_ context.Context, _ string, audience string) (*va func serverFromAuth(t *testing.T, a *authpkg.Auth) *Server { t.Helper() // ext_authz is waypoint mode — audience derived from host - inbound, err := plugins.WaypointInboundPipeline(a) + inbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{plugintesting.NewJWTValidation(a, true)}) if err != nil { t.Fatalf("building inbound pipeline: %v", err) } - outbound, err := plugins.DefaultOutboundPipeline(a) + outbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{plugintesting.NewTokenExchange(a)}) if err != nil { t.Fatalf("building outbound pipeline: %v", err) } diff --git a/authbridge/cmd/authbridge/listener/extproc/server_test.go b/authbridge/cmd/authbridge/listener/extproc/server_test.go index 3ea6257dc..903f21dd3 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server_test.go +++ b/authbridge/cmd/authbridge/listener/extproc/server_test.go @@ -19,7 +19,7 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/cache" "github.com/kagenti/kagenti-extensions/authbridge/authlib/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/plugintesting" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" "github.com/kagenti/kagenti-extensions/authbridge/authlib/session" "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" @@ -64,11 +64,16 @@ func (v *mockVerifier) Verify(_ context.Context, _ string, _ string) (*validatio func serverFromAuth(t *testing.T, a *auth.Auth) *Server { t.Helper() - inbound, err := plugins.DefaultInboundPipeline(a) + // Plugins build their own auth.Auth from local config in + // production. Tests inject a pre-built *auth.Auth via + // NewJWTValidationForTest / NewTokenExchangeForTest so the + // listener-level assertions don't have to care about the plugin's + // internal construction path. + inbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{plugintesting.NewJWTValidation(a, false)}) if err != nil { t.Fatalf("building inbound pipeline: %v", err) } - outbound, err := plugins.DefaultOutboundPipeline(a) + outbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{plugintesting.NewTokenExchange(a)}) if err != nil { t.Fatalf("building outbound pipeline: %v", err) } @@ -384,7 +389,7 @@ func TestExtProc_BodyBuffering_Inbound(t *testing.T) { t.Fatal(err) } - outbound, err := plugins.DefaultOutboundPipeline(auth.New(auth.Config{})) + outbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{plugintesting.NewTokenExchange(auth.New(auth.Config{}))}) if err != nil { t.Fatal(err) } @@ -447,10 +452,12 @@ func TestExtProc_BodyBuffering_Outbound(t *testing.T) { t.Fatal(err) } - inbound, err := plugins.DefaultInboundPipeline(auth.New(auth.Config{ - Verifier: &mockVerifier{claims: &validation.Claims{Subject: "user"}}, - Identity: auth.IdentityConfig{Audience: "test"}, - })) + inbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{ + plugintesting.NewJWTValidation(auth.New(auth.Config{ + Verifier: &mockVerifier{claims: &validation.Claims{Subject: "user"}}, + Identity: auth.IdentityConfig{Audience: "test"}, + }), false), + }) if err != nil { t.Fatal(err) } @@ -499,7 +506,7 @@ func TestExtProc_BodyTooLarge(t *testing.T) { t.Fatal(err) } - outbound, err := plugins.DefaultOutboundPipeline(auth.New(auth.Config{})) + outbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{plugintesting.NewTokenExchange(auth.New(auth.Config{}))}) if err != nil { t.Fatal(err) } diff --git a/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go b/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go index 0bf448859..547684da8 100644 --- a/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go +++ b/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go @@ -13,7 +13,7 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/cache" "github.com/kagenti/kagenti-extensions/authbridge/authlib/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/plugintesting" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) @@ -29,7 +29,7 @@ func (m *mockVerifier) Verify(_ context.Context, _ string, _ string) (*validatio func outboundPipelineFromAuth(t *testing.T, a *auth.Auth) *pipeline.Pipeline { t.Helper() - p, err := plugins.DefaultOutboundPipeline(a) + p, err := plugintesting.BuildPipeline([]pipeline.Plugin{plugintesting.NewTokenExchange(a)}) if err != nil { t.Fatalf("building outbound pipeline: %v", err) } diff --git a/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go b/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go index b90daedaf..6fbd25c77 100644 --- a/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go +++ b/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go @@ -11,7 +11,7 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" "github.com/kagenti/kagenti-extensions/authbridge/authlib/bypass" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/plugintesting" "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) @@ -26,7 +26,7 @@ func (m *mockVerifier) Verify(_ context.Context, _ string, _ string) (*validatio func inboundPipelineFromAuth(t *testing.T, a *auth.Auth) *pipeline.Pipeline { t.Helper() - p, err := plugins.DefaultInboundPipeline(a) + p, err := plugintesting.BuildPipeline([]pipeline.Plugin{plugintesting.NewJWTValidation(a, false)}) if err != nil { t.Fatalf("building inbound pipeline: %v", err) } diff --git a/authbridge/cmd/authbridge/main.go b/authbridge/cmd/authbridge/main.go index 1a3a3909b..8a128afa7 100644 --- a/authbridge/cmd/authbridge/main.go +++ b/authbridge/cmd/authbridge/main.go @@ -86,9 +86,6 @@ func main() { log.Fatal("--config is required") } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - // Load config cfg, err := config.Load(*configPath) if err != nil { @@ -97,22 +94,20 @@ func main() { if *mode != "" { cfg.Mode = *mode // flag overrides YAML } - - // Resolve config into auth dependencies. - // Credential files and JWKS are resolved lazily — the gRPC listener - // starts immediately so Envoy can connect without waiting. - resolved, err := config.Resolve(ctx, cfg) - if err != nil { - log.Fatalf("resolving config: %v", err) + config.ApplyPreset(cfg) + if err := config.Validate(cfg); err != nil { + log.Fatalf("config validation: %v", err) } - handler := auth.New(*resolved) - // Build pipelines from config (falls back to defaults if pipeline section is omitted) - inboundPipeline, err := buildInboundPipeline(cfg, handler) + // Build pipelines from config. Each plugin that implements + // pipeline.Configurable receives its own config subtree via + // Configure and constructs any internal state (JWKS verifier, + // token-exchange client, router) from it. No shared auth handler. + inboundPipeline, err := plugins.Build(cfg.Pipeline.Inbound.Plugins) if err != nil { log.Fatalf("building inbound pipeline: %v", err) } - outboundPipeline, err := buildOutboundPipeline(cfg, handler) + outboundPipeline, err := plugins.Build(cfg.Pipeline.Outbound.Plugins) if err != nil { log.Fatalf("building outbound pipeline: %v", err) } @@ -189,7 +184,17 @@ func main() { log.Fatalf("unhandled mode %q", cfg.Mode) } - statSrv := startStatServer(cfg, handler) + // /stats aggregates per-plugin counters at request time. Each + // plugin that implements plugins.StatsSource contributes its own + // *auth.Stats; the provider merges them into a single response + // per HTTP request. Freshly-computed every call, so the numbers + // reflect traffic up to the moment of the curl. + statsProvider := func() *auth.Stats { + sources := plugins.CollectStats(inboundPipeline) + sources = append(sources, plugins.CollectStats(outboundPipeline)...) + return auth.MergeStats(sources...) + } + statSrv := startStatServer(cfg, statsProvider) // Session events API (optional; only when session tracking is on). // The API has no authentication — bind only on in-cluster addresses and @@ -214,44 +219,31 @@ func main() { slog.Info("authbridge starting", "mode", cfg.Mode, "logLevel", logLevel.Level().String()) - // Parse configurable credential wait timeout (default 120s). - credentialTimeout := 120 * time.Second - if t := cfg.Identity.CredentialWaitTimeout; t != "" { - if d, err := time.ParseDuration(t); err == nil { - credentialTimeout = d - } - } - - // Resolve credentials in background — doesn't block the listener. - // Never gives up: after initialTimeout switches to exponential backoff. - credReady := config.ResolveCredentialFiles(cfg, credentialTimeout) - - // Update handler identity once credentials are available. - go func() { - <-credReady - clientAuth, err := config.ResolveClientAuth(cfg) - if err != nil { - slog.Warn("failed to resolve client auth after credential load", "error", err) - return - } - handler.UpdateIdentity(auth.IdentityConfig{ - ClientID: cfg.Identity.ClientID, - Audience: cfg.Identity.ClientID, - }, clientAuth) - }() - - // Health/readiness server — reflects credential readiness for K8s probes. + // Health/readiness endpoints. /healthz is liveness (always OK); + // /readyz ANDs pipeline.Ready() across inbound and outbound. A + // plugin implementing pipeline.Readier — currently jwt-validation + // and token-exchange — can return false while its deferred Init + // goroutine is still waiting on credential files. The kubelet + // holds traffic off the pod until all plugins are ready, so the + // 503-from-OnRequest window at pod boot is closed. go func() { mux := http.NewServeMux() mux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }) mux.HandleFunc("/readyz", func(w http.ResponseWriter, r *http.Request) { - if handler.Ready() { - w.WriteHeader(http.StatusOK) - } else { - http.Error(w, "credentials pending", http.StatusServiceUnavailable) + // Report the first not-ready plugin by name so operators + // can diagnose from `kubectl describe pod` without + // tailing container logs. + if name := inboundPipeline.NotReadyPlugin(); name != "" { + http.Error(w, "inbound plugin not ready: "+name, http.StatusServiceUnavailable) + return + } + if name := outboundPipeline.NotReadyPlugin(); name != "" { + http.Error(w, "outbound plugin not ready: "+name, http.StatusServiceUnavailable) + return } + w.WriteHeader(http.StatusOK) }) slog.Info("health server listening", "addr", ":9091") if err := http.ListenAndServe(":9091", mux); err != nil { @@ -298,23 +290,6 @@ func main() { } } -func buildInboundPipeline(cfg *config.Config, handler *auth.Auth) (*pipeline.Pipeline, error) { - if len(cfg.Pipeline.Inbound.Plugins) > 0 { - return plugins.Build(cfg.Pipeline.Inbound.Plugins, handler) - } - if cfg.Mode == config.ModeWaypoint { - return plugins.WaypointInboundPipeline(handler) - } - return plugins.DefaultInboundPipeline(handler) -} - -func buildOutboundPipeline(cfg *config.Config, handler *auth.Auth) (*pipeline.Pipeline, error) { - if len(cfg.Pipeline.Outbound.Plugins) > 0 { - return plugins.Build(cfg.Pipeline.Outbound.Plugins, handler) - } - return plugins.DefaultOutboundPipeline(handler) -} - func startGRPCExtProc(inbound, outbound *pipeline.Pipeline, sessions *session.Store, addr string) *grpc.Server { srv := grpc.NewServer() extprocv3.RegisterExternalProcessorServer(srv, &extproc.Server{ @@ -375,8 +350,8 @@ func startHTTPServer(name string, handler http.Handler, addr string) *http.Serve return srv } -func startStatServer(config *config.Config, handler *auth.Auth) *observe.StatServer { - srv := observe.NewStatServer(config.Stats.StatsAddress, config, handler.Stats) +func startStatServer(config *config.Config, provider observe.StatsProvider) *observe.StatServer { + srv := observe.NewStatServer(config.Stats.StatsAddress, config, provider) go func() { slog.Info("stat server listening", "addr", config.Stats.StatsAddress) if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed { diff --git a/authbridge/demos/mcp-parser/README.md b/authbridge/demos/mcp-parser/README.md index e33f3b814..8a833f133 100644 --- a/authbridge/demos/mcp-parser/README.md +++ b/authbridge/demos/mcp-parser/README.md @@ -77,37 +77,33 @@ metadata: data: config.yaml: | mode: envoy-sidecar - inbound: - issuer: "http://keycloak.localtest.me:8080/realms/kagenti" - outbound: - keycloak_url: "http://keycloak-service.keycloak.svc:8080" - keycloak_realm: "kagenti" - default_policy: "passthrough" - identity: - type: "spiffe" - client_id_file: "/shared/client-id.txt" - client_secret_file: "/shared/client-secret.txt" - jwt_svid_path: "/opt/jwt_svid.token" - bypass: - inbound_paths: - - "/.well-known/*" - - "/healthz" - - "/readyz" - - "/livez" pipeline: inbound: plugins: - - jwt-validation + - name: jwt-validation + config: + issuer: "http://keycloak.localtest.me:8080/realms/kagenti" outbound: plugins: - - token-exchange + - name: token-exchange + config: + keycloak_url: "http://keycloak-service.keycloak.svc:8080" + keycloak_realm: "kagenti" + identity: + type: "spiffe" - mcp-parser EOF ``` -> **Note**: The `pipeline` section is the only addition. All other fields -> should match your existing `authbridge-runtime-config`. If you use -> `client-secret` identity type instead of `spiffe`, adjust accordingly. +> **Note**: Per-plugin config is the only supported shape. Top-level +> `inbound:` / `outbound:` / `identity:` / `bypass:` blocks are no +> longer accepted. Defaults (`audience_file=/shared/client-id.txt`, +> `bypass_paths`=common probes, `jwt_svid_path=/opt/jwt_svid.token`, +> `client_id_file=/shared/client-id.txt`, `default_policy=passthrough`, +> `routes.file=/etc/authproxy/routes.yaml`) kick in for anything you +> omit. For `client-secret` identity type, swap `type: spiffe` to +> `type: client-secret` — the `client_secret_file` default activates +> automatically. The `mcp-parser` is placed **after** `token-exchange` in the outbound pipeline. This means: