diff --git a/authbridge/authlib/README.md b/authbridge/authlib/README.md index 76b6cecea..4576fa237 100644 --- a/authbridge/authlib/README.md +++ b/authbridge/authlib/README.md @@ -8,6 +8,7 @@ A pure Go library providing reusable building blocks for JWT validation, OAuth 2 | Package | Purpose | |---------|---------| +| `contracts/` | Tiny, stable vocabulary plugins depend on without importing each other — role constants (`RoleUser`, `RoleAssistant`, …), the `ContentSource` / `Fragment` shape for guardrail-parser interop, and claim constants (`ClaimAuthorizationHeader`, …) for `PluginCapabilities.Claims` mutex declarations. | | `bypass/` | Path pattern matcher for public endpoints (health, agent card). Any inbound gate plugin (jwt-validation, SAML, mTLS) can use it. | | `routing/` | Host-to-audience router with glob pattern matching. Used by token-exchange; future routed plugins are expected to reuse it. | | `auth/` | Composition layer: `HandleInbound` + `HandleOutbound` — used internally by the `jwt-validation` and `token-exchange` plugins. Lingering in authlib/ for now; plugin-internal in practice. | diff --git a/authbridge/authlib/contracts/claims.go b/authbridge/authlib/contracts/claims.go new file mode 100644 index 000000000..59458fa74 --- /dev/null +++ b/authbridge/authlib/contracts/claims.go @@ -0,0 +1,29 @@ +package contracts + +// Claim strings for PluginCapabilities.Claims. A claim is a semantic +// resource that exactly one plugin per chain owns; two plugins +// declaring the same claim cause plugins.Build to fail at startup. +// +// Plugin authors reference these constants instead of string literals +// so typos are compile errors and the canonical set is greppable. +// Third-party plugins that need a claim not listed here may declare +// their own arbitrary string — the framework enforces uniqueness of +// whatever it sees, not "must be from the list" — but won't benefit +// from typo safety until the claim is upstreamed here. +// +// Resist speculative additions. Each constant is a small +// hard-to-deprecate contract. Adding a new claim should be driven by +// a concrete use case where two plugins would conflict in practice. +// Upstream a new constant in a follow-up PR when a claim stabilizes, +// with a godoc paragraph explaining what the resource is and which +// in-tree plugins claim it. + +// ClaimAuthorizationHeader is the exclusive right to replace the +// outbound Authorization header. token-exchange and token-broker +// both claim this; they cannot coexist in the same outbound chain +// because the one that runs second would silently clobber the +// first's work. +// +// A future SPIFFE-exchanger or Keycloak-flavored gate plugin that +// also rewrites Authorization should declare this claim. +const ClaimAuthorizationHeader = "authorization_header" diff --git a/authbridge/authlib/pipeline/plugin.go b/authbridge/authlib/pipeline/plugin.go index f1d0137fa..cdcd71502 100644 --- a/authbridge/authlib/pipeline/plugin.go +++ b/authbridge/authlib/pipeline/plugin.go @@ -48,6 +48,53 @@ type PluginCapabilities struct { // // Deprecated: use ReadsBody. Will be removed in a future release. BodyAccess bool + + // Requires names plugins that MUST be present in the same chain + // AND appear earlier (lower index). Matches are case-sensitive + // plugin Name() strings. A missing or misordered name causes + // plugins.Build to fail at startup. + // + // Use Requires when the plugin hardcodes access to a specific + // other plugin's extension fields — e.g., a tool-allowlist plugin + // that reads pctx.Extensions.MCP.Params["name"] declares + // Requires: []string{"mcp-parser"}. If the plugin instead reads + // through pctx.ContentSources() and works against any parser, + // see RequiresAny. + Requires []string + + // RequiresAny names plugins of which AT LEAST ONE must be present + // in the same chain, and each named plugin that IS present must + // appear earlier. Missing-all-of-them or misordered-any-of-them + // causes plugins.Build to fail at startup. + // + // Use RequiresAny for protocol-agnostic plugins that read through + // pctx.ContentSources(). Example: a PII scrubber that consumes + // fragments from whatever parsers are wired in declares + // RequiresAny: []string{"a2a-parser", "mcp-parser", "inference-parser"}. + // That way a chain with no parsers fails loud instead of running + // the guardrail as silent dead code. + RequiresAny []string + + // After names plugins that, IF present in the same chain, must + // appear earlier. Unlike Requires/RequiresAny, a missing name is + // not an error — After is a soft ordering hint. Useful for + // plugins that benefit from earlier state being populated but + // degrade gracefully without it. + After []string + + // Claims declares semantic resources the plugin takes exclusive + // ownership of. Within a single chain, at most one plugin may + // declare any given claim string; two plugins with an overlapping + // claim cause plugins.Build to fail at startup. + // + // Claim strings are arbitrary but authors should prefer the + // constants in authlib/contracts/ (e.g. contracts.ClaimAuthorizationHeader) + // so typos are compile errors and the canonical set is greppable. + // Third-party plugins may declare their own strings; the framework + // enforces uniqueness of whatever it sees, not "must be from the + // list." See authlib/contracts/claims.go for the canonical + // vocabulary. + Claims []string } // Normalize applies compatibility rules to a PluginCapabilities: diff --git a/authbridge/authlib/plugins/plugins_test.go b/authbridge/authlib/plugins/plugins_test.go index 3a4cf7cb1..9dcfa5cf1 100644 --- a/authbridge/authlib/plugins/plugins_test.go +++ b/authbridge/authlib/plugins/plugins_test.go @@ -15,6 +15,7 @@ import ( // main.go uses — ensures Build("jwt-validation") / Build("token-exchange") // resolve during tests. jwtvalidation "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation" + _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenbroker" tokenexchange "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange" ) @@ -151,3 +152,43 @@ func TestBuild_ConfigureError(t *testing.T) { t.Errorf("error %q does not name the offending plugin", err) } } + +// TestBuild_TokenExchangeAndTokenBroker_ConflictingClaims exercises the +// concrete case from issue #398: configuring both token-exchange and +// token-broker on the same outbound chain is now rejected at Build +// time because they both claim ClaimAuthorizationHeader. +func TestBuild_TokenExchangeAndTokenBroker_ConflictingClaims(t *testing.T) { + // Configure both with valid per-plugin config so the + // relationship check is what fails (not some earlier Configure + // error). token-broker requires broker_url; token-exchange + // requires the keycloak_url / keycloak_realm pair. + _, err := plugins.Build([]config.PluginEntry{ + { + Name: "token-exchange", + Config: []byte(`{ + "keycloak_url": "http://keycloak.example", + "keycloak_realm": "test", + "default_policy": "passthrough", + "identity": {"type": "client-secret"} + }`), + }, + { + Name: "token-broker", + Config: []byte(`{"broker_url": "http://broker.example"}`), + }, + }) + if err == nil { + t.Fatal("expected relationship conflict error for token-exchange + token-broker") + } + msg := err.Error() + for _, want := range []string{ + "token-exchange", + "token-broker", + "authorization_header", + "configure only one", + } { + if !strings.Contains(msg, want) { + t.Errorf("error %q does not mention %q", err, want) + } + } +} diff --git a/authbridge/authlib/plugins/registry.go b/authbridge/authlib/plugins/registry.go index 8ff8c409a..e9f7f8bf6 100644 --- a/authbridge/authlib/plugins/registry.go +++ b/authbridge/authlib/plugins/registry.go @@ -3,6 +3,7 @@ package plugins import ( "fmt" "sort" + "strings" "sync" "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" @@ -123,6 +124,122 @@ func Build(entries []config.PluginEntry, opts ...pipeline.Option) (*pipeline.Pip ps = append(ps, p) policies = append(policies, e.OnError.Resolved()) } + if err := validateRelationships(ps); err != nil { + return nil, err + } opts = append(opts, pipeline.WithPolicies(policies...)) return pipeline.New(ps, opts...) } + +// validateRelationships checks every plugin's Requires / RequiresAny / +// After / Claims declarations against the chain it's about to run in. +// Collects all errors across the chain into one joined error rather +// than short-circuiting on the first — friendlier for operators +// iterating on a freshly-edited YAML. +// +// Semantics: +// +// - Requires: every named plugin must appear at a lower index in +// the chain. Missing or misordered is an error. +// - RequiresAny: at least one named plugin must appear at a lower +// index. Any named plugin that IS present must also be at a +// lower index. +// - After: if a named plugin is present, it must appear at a lower +// index. Silent if the named plugin is absent. +// - Claims: at most one plugin per unique claim string across the +// entire chain. +// +// Each rule loop uses the per-plugin Name() as the identity key. Case- +// sensitive (Go default). If a plugin name is duplicated in a chain +// (rare — requires config.PluginEntry.ID differentiation), the +// earliest-occurrence index is authoritative for position checks. +func validateRelationships(ps []pipeline.Plugin) error { + if len(ps) == 0 { + return nil + } + // Build a name->first-occurrence-index map once. + positions := make(map[string]int, len(ps)) + for i, p := range ps { + if _, seen := positions[p.Name()]; !seen { + positions[p.Name()] = i + } + } + + var errs []string + + for i, p := range ps { + caps := p.Capabilities().Normalize() + + // Requires — hard AND with ordering. + for _, req := range caps.Requires { + j, present := positions[req] + switch { + case !present: + errs = append(errs, fmt.Sprintf( + "plugin %q requires %q earlier in the chain, but %q is not configured", + p.Name(), req, req)) + case j >= i: + errs = append(errs, fmt.Sprintf( + "plugin %q requires %q earlier in the chain, but %q appears at position %d (this plugin is at %d)", + p.Name(), req, req, j, i)) + } + } + + // RequiresAny — hard OR with ordering. + if len(caps.RequiresAny) > 0 { + anyPresentAndEarlier := false + for _, req := range caps.RequiresAny { + j, present := positions[req] + if !present { + continue + } + if j >= i { + // Present but misordered — report per-offender. + errs = append(errs, fmt.Sprintf( + "plugin %q lists %q under RequiresAny; %q must appear earlier (found at position %d, this plugin is at %d)", + p.Name(), req, req, j, i)) + continue + } + anyPresentAndEarlier = true + } + if !anyPresentAndEarlier { + errs = append(errs, fmt.Sprintf( + "plugin %q requires at least one of %v earlier in the chain, but none are configured", + p.Name(), caps.RequiresAny)) + } + } + + // After — soft ordering. + for _, name := range caps.After { + j, present := positions[name] + if !present { + continue + } + if j >= i { + errs = append(errs, fmt.Sprintf( + "plugin %q declares After %q, but %q appears at position %d (this plugin is at %d); reorder so %q runs first", + p.Name(), name, name, j, i, name)) + } + } + } + + // Claims — chain-wide aggregation. + claimOwner := make(map[string]string, len(ps)) + for _, p := range ps { + caps := p.Capabilities().Normalize() + for _, claim := range caps.Claims { + if existing, taken := claimOwner[claim]; taken && existing != p.Name() { + errs = append(errs, fmt.Sprintf( + "plugins %q and %q both claim %q; configure only one of them on this chain", + existing, p.Name(), claim)) + continue + } + claimOwner[claim] = p.Name() + } + } + + if len(errs) == 0 { + return nil + } + return fmt.Errorf("plugin relationship validation failed:\n - %s", strings.Join(errs, "\n - ")) +} diff --git a/authbridge/authlib/plugins/registry_test.go b/authbridge/authlib/plugins/registry_test.go index 7d2049b1c..62d02b598 100644 --- a/authbridge/authlib/plugins/registry_test.go +++ b/authbridge/authlib/plugins/registry_test.go @@ -1,6 +1,7 @@ package plugins import ( + "context" "testing" "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" @@ -120,3 +121,324 @@ func containsSubstring(s, sub string) bool { } return false } + +// --- Plugin relationship validation tests --- + +// relPlugin is a minimal pipeline.Plugin with configurable Capabilities +// used to drive validateRelationships through its branches. Lives here +// rather than in plugintesting because it's relationship-specific — no +// body / identity / dispatch behavior to share with other tests. +type relPlugin struct { + name string + caps pipeline.PluginCapabilities +} + +func (p *relPlugin) Name() string { return p.name } +func (p *relPlugin) Capabilities() pipeline.PluginCapabilities { return p.caps } +func (p *relPlugin) OnRequest(context.Context, *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} +func (p *relPlugin) OnResponse(context.Context, *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// mkRelPlugin is the one-liner builder used across the table-driven +// tests below. Name is required; caps drives whichever relationship +// rule the test is exercising. +func mkRelPlugin(name string, caps pipeline.PluginCapabilities) *relPlugin { + return &relPlugin{name: name, caps: caps} +} + +// TestValidateRelationships_Requires exercises the hard dependency +// rule: missing named plugin → error; named plugin present but later +// → error; named plugin present and earlier → ok. +func TestValidateRelationships_Requires(t *testing.T) { + tests := []struct { + name string + plugins []pipeline.Plugin + wantErr bool + wantInMsg []string + }{ + { + name: "required plugin present and earlier — ok", + plugins: []pipeline.Plugin{ + mkRelPlugin("mcp-parser", pipeline.PluginCapabilities{}), + mkRelPlugin("tool-allowlist", pipeline.PluginCapabilities{ + Requires: []string{"mcp-parser"}, + }), + }, + wantErr: false, + }, + { + name: "required plugin missing — error names missing plugin", + plugins: []pipeline.Plugin{ + mkRelPlugin("tool-allowlist", pipeline.PluginCapabilities{ + Requires: []string{"mcp-parser"}, + }), + }, + wantErr: true, + wantInMsg: []string{"tool-allowlist", "requires", "mcp-parser", "not configured"}, + }, + { + name: "required plugin present but later — error shows positions", + plugins: []pipeline.Plugin{ + mkRelPlugin("tool-allowlist", pipeline.PluginCapabilities{ + Requires: []string{"mcp-parser"}, + }), + mkRelPlugin("mcp-parser", pipeline.PluginCapabilities{}), + }, + wantErr: true, + wantInMsg: []string{"tool-allowlist", "mcp-parser", "position 1", "this plugin is at 0"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateRelationships(tc.plugins) + if (err != nil) != tc.wantErr { + t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) + } + if err != nil { + msg := err.Error() + for _, want := range tc.wantInMsg { + if !containsSubstring(msg, want) { + t.Errorf("error %q missing %q", msg, want) + } + } + } + }) + } +} + +// TestValidateRelationships_RequiresAny covers the OR-dependency: +// at least one of the named plugins must be present + earlier. +func TestValidateRelationships_RequiresAny(t *testing.T) { + tests := []struct { + name string + plugins []pipeline.Plugin + wantErr bool + wantInMsg []string + }{ + { + name: "one of the alternatives present earlier — ok", + plugins: []pipeline.Plugin{ + mkRelPlugin("a2a-parser", pipeline.PluginCapabilities{}), + mkRelPlugin("pii-scrubber", pipeline.PluginCapabilities{ + RequiresAny: []string{"a2a-parser", "mcp-parser", "inference-parser"}, + }), + }, + wantErr: false, + }, + { + name: "multiple alternatives present earlier — ok", + plugins: []pipeline.Plugin{ + mkRelPlugin("a2a-parser", pipeline.PluginCapabilities{}), + mkRelPlugin("mcp-parser", pipeline.PluginCapabilities{}), + mkRelPlugin("pii-scrubber", pipeline.PluginCapabilities{ + RequiresAny: []string{"a2a-parser", "mcp-parser", "inference-parser"}, + }), + }, + wantErr: false, + }, + { + name: "no alternatives present — error names the set", + plugins: []pipeline.Plugin{ + mkRelPlugin("pii-scrubber", pipeline.PluginCapabilities{ + RequiresAny: []string{"a2a-parser", "mcp-parser", "inference-parser"}, + }), + }, + wantErr: true, + wantInMsg: []string{"pii-scrubber", "at least one", "none are configured"}, + }, + { + name: "alternative present but later — error per-offender", + plugins: []pipeline.Plugin{ + mkRelPlugin("pii-scrubber", pipeline.PluginCapabilities{ + RequiresAny: []string{"a2a-parser", "mcp-parser"}, + }), + mkRelPlugin("a2a-parser", pipeline.PluginCapabilities{}), + }, + wantErr: true, + wantInMsg: []string{"pii-scrubber", "RequiresAny", "a2a-parser", "must appear earlier"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateRelationships(tc.plugins) + if (err != nil) != tc.wantErr { + t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) + } + if err != nil { + msg := err.Error() + for _, want := range tc.wantInMsg { + if !containsSubstring(msg, want) { + t.Errorf("error %q missing %q", msg, want) + } + } + } + }) + } +} + +// TestValidateRelationships_After covers the soft-ordering rule: +// silent when named plugin is absent, error when present but later. +func TestValidateRelationships_After(t *testing.T) { + tests := []struct { + name string + plugins []pipeline.Plugin + wantErr bool + wantInMsg []string + }{ + { + name: "named plugin absent — no constraint", + plugins: []pipeline.Plugin{ + mkRelPlugin("request-counter", pipeline.PluginCapabilities{ + After: []string{"mcp-parser"}, + }), + }, + wantErr: false, + }, + { + name: "named plugin present earlier — ok", + plugins: []pipeline.Plugin{ + mkRelPlugin("mcp-parser", pipeline.PluginCapabilities{}), + mkRelPlugin("request-counter", pipeline.PluginCapabilities{ + After: []string{"mcp-parser"}, + }), + }, + wantErr: false, + }, + { + name: "named plugin present but later — error says reorder", + plugins: []pipeline.Plugin{ + mkRelPlugin("request-counter", pipeline.PluginCapabilities{ + After: []string{"mcp-parser"}, + }), + mkRelPlugin("mcp-parser", pipeline.PluginCapabilities{}), + }, + wantErr: true, + wantInMsg: []string{"request-counter", "After", "mcp-parser", "reorder"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateRelationships(tc.plugins) + if (err != nil) != tc.wantErr { + t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) + } + if err != nil { + msg := err.Error() + for _, want := range tc.wantInMsg { + if !containsSubstring(msg, want) { + t.Errorf("error %q missing %q", msg, want) + } + } + } + }) + } +} + +// TestValidateRelationships_Claims covers the mutual-exclusion rule: +// exactly one plugin per claim string per chain. +func TestValidateRelationships_Claims(t *testing.T) { + tests := []struct { + name string + plugins []pipeline.Plugin + wantErr bool + wantInMsg []string + }{ + { + name: "single claimant — ok", + plugins: []pipeline.Plugin{ + mkRelPlugin("token-exchange", pipeline.PluginCapabilities{ + Claims: []string{"authorization_header"}, + }), + }, + wantErr: false, + }, + { + name: "distinct claims on different plugins — ok", + plugins: []pipeline.Plugin{ + mkRelPlugin("token-exchange", pipeline.PluginCapabilities{ + Claims: []string{"authorization_header"}, + }), + mkRelPlugin("jwt-validation", pipeline.PluginCapabilities{ + Claims: []string{"identity_resolution"}, + }), + }, + wantErr: false, + }, + { + name: "two plugins claim the same string — error names both", + plugins: []pipeline.Plugin{ + mkRelPlugin("token-exchange", pipeline.PluginCapabilities{ + Claims: []string{"authorization_header"}, + }), + mkRelPlugin("token-broker", pipeline.PluginCapabilities{ + Claims: []string{"authorization_header"}, + }), + }, + wantErr: true, + wantInMsg: []string{"token-exchange", "token-broker", "authorization_header", "configure only one"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateRelationships(tc.plugins) + if (err != nil) != tc.wantErr { + t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) + } + if err != nil { + msg := err.Error() + for _, want := range tc.wantInMsg { + if !containsSubstring(msg, want) { + t.Errorf("error %q missing %q", msg, want) + } + } + } + }) + } +} + +// TestValidateRelationships_CollectsAllErrors verifies the collector +// policy: a chain with multiple problems reports them all in one +// error, rather than short-circuiting on the first. Operators iterate +// on a single YAML fix rather than a sequence of startups. +func TestValidateRelationships_CollectsAllErrors(t *testing.T) { + plugins := []pipeline.Plugin{ + mkRelPlugin("a-claims-x", pipeline.PluginCapabilities{ + Claims: []string{"x"}, + }), + mkRelPlugin("b-claims-x", pipeline.PluginCapabilities{ + Claims: []string{"x"}, // conflicts with a-claims-x + }), + mkRelPlugin("c-requires-missing", pipeline.PluginCapabilities{ + Requires: []string{"does-not-exist"}, + }), + } + err := validateRelationships(plugins) + if err == nil { + t.Fatal("expected error, got nil") + } + msg := err.Error() + for _, want := range []string{ + "a-claims-x", + "b-claims-x", + "c-requires-missing", + "does-not-exist", + } { + if !containsSubstring(msg, want) { + t.Errorf("error message should mention %q: %s", want, msg) + } + } +} + +// TestValidateRelationships_EmptyChain is a safety check that no-plugin +// chains don't panic or error — the check is vacuously true. +func TestValidateRelationships_EmptyChain(t *testing.T) { + if err := validateRelationships(nil); err != nil { + t.Errorf("empty chain should not error, got: %v", err) + } + if err := validateRelationships([]pipeline.Plugin{}); err != nil { + t.Errorf("empty chain should not error, got: %v", err) + } +} diff --git a/authbridge/authlib/plugins/tokenbroker/plugin.go b/authbridge/authlib/plugins/tokenbroker/plugin.go index d6c3454ee..348a4c089 100644 --- a/authbridge/authlib/plugins/tokenbroker/plugin.go +++ b/authbridge/authlib/plugins/tokenbroker/plugin.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/gobwas/glob" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/contracts" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenbroker/client" @@ -154,7 +155,14 @@ func NewTokenBroker() *TokenBroker { return &TokenBroker{} } func (p *TokenBroker) Name() string { return "token-broker" } func (p *TokenBroker) Capabilities() pipeline.PluginCapabilities { - return pipeline.PluginCapabilities{} + return pipeline.PluginCapabilities{ + // token-broker takes exclusive ownership of the outbound + // Authorization header — same claim as token-exchange, so + // configuring both in the same outbound chain fails at + // plugins.Build rather than letting one silently clobber + // the other. + Claims: []string{contracts.ClaimAuthorizationHeader}, + } } func (p *TokenBroker) Configure(raw json.RawMessage) error { diff --git a/authbridge/authlib/plugins/tokenexchange/plugin.go b/authbridge/authlib/plugins/tokenexchange/plugin.go index f5d80b042..439488744 100644 --- a/authbridge/authlib/plugins/tokenexchange/plugin.go +++ b/authbridge/authlib/plugins/tokenexchange/plugin.go @@ -13,6 +13,7 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/auth" "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/contracts" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/cache" @@ -211,7 +212,13 @@ func init() { func (p *TokenExchange) Name() string { return "token-exchange" } func (p *TokenExchange) Capabilities() pipeline.PluginCapabilities { - return pipeline.PluginCapabilities{} + return pipeline.PluginCapabilities{ + // token-exchange takes exclusive ownership of the outbound + // Authorization header. Any other plugin that also claims this + // resource (e.g., token-broker) will fail at plugins.Build + // time rather than silently clobbering our replaced token. + Claims: []string{contracts.ClaimAuthorizationHeader}, + } } func (p *TokenExchange) Configure(raw json.RawMessage) error { diff --git a/authbridge/demos/github-issue/demo-ui.md b/authbridge/demos/github-issue/demo-ui.md index 5d4fae668..c29200659 100644 --- a/authbridge/demos/github-issue/demo-ui.md +++ b/authbridge/demos/github-issue/demo-ui.md @@ -385,7 +385,7 @@ kubectl get pods -n team1 ``` Expected output depends on how the **kagenti-operator** feature gate -[`combinedSidecar`](https://github.com/kagenti/kagenti/blob/main/docs/authbridge-combined-sidecar.md) +[`combinedSidecar`](https://github.com/kagenti/kagenti/blob/main/docs/authbridge/deployment-guide.md) is set (cluster-wide Helm / `kagenti-feature-gates` ConfigMap — not the import UI). **Legacy separate sidecars** (`combinedSidecar: false`, default in many installs): @@ -746,7 +746,7 @@ exit Check the ext_proc logs to confirm both inbound validation and outbound token exchange are working. Envoy and authbridge log to the **`envoy-proxy`** container in legacy mode, or to **`authbridge`** when -[`combinedSidecar`](https://github.com/kagenti/kagenti/blob/main/docs/authbridge-combined-sidecar.md) +[`combinedSidecar`](https://github.com/kagenti/kagenti/blob/main/docs/authbridge/deployment-guide.md) is enabled — replace `-c envoy-proxy` with `-c authbridge` below. **Inbound validation logs:** diff --git a/authbridge/docs/framework-architecture.md b/authbridge/docs/framework-architecture.md index 31cabe360..2ef87ff38 100644 --- a/authbridge/docs/framework-architecture.md +++ b/authbridge/docs/framework-architecture.md @@ -383,7 +383,7 @@ func (p *Pipeline) Plugins() []Plugin // func (p *Pipeline) NeedsBody() bool // OR over all plugins' BodyAccess ``` -`New` validates capability wiring at startup: every `Read` must be satisfied by some earlier plugin's `Write`. +`New` validates capability wiring at startup: every `Read` must be satisfied by some earlier plugin's `Write`. `plugins.Build` additionally validates the cross-plugin relationship declarations — `Requires`, `RequiresAny`, `After`, `Claims` — before returning the pipeline to the listener. See [`plugin-reference.md` "Declaring plugin relationships"](./plugin-reference.md#declaring-plugin-relationships). ### Plugin lifecycle (`Start` / `Stop`) @@ -678,6 +678,7 @@ The plugin interface is **not** semver-stable yet (AuthBridge is pre-1.0). Chang - **Body mutation**: `PluginCapabilities.BodyAccess` split into `ReadsBody` / `WritesBody`. New `pctx.SetBody` / `pctx.SetResponseBody` helpers flip a mutation flag; all three listeners (extproc / forwardproxy / reverseproxy) propagate the rewrite to the upstream with correct `Content-Length` and cleared `Content-Encoding`. `BodyAccess` kept as deprecated alias. See §6, "Body mutation." - **Detyped framework**: `pipeline/` no longer imports plugin-specific packages. **Breaking**: `Context.Claims *validation.Claims` → `Context.Identity Identity` (interface with `Subject()`/`ClientID()`/`Scopes()`); plugins publish adapters. `Context.Route` removed (was dead code). `Invocation`'s nine jwt-validation + token-exchange specific fields (`ExpectedIssuer`, `TokenSubject`, `RouteHost`, `CacheHit`, etc.) collapsed into `Details map[string]string`; built-in plugins migrated to `Details["expected_issuer"]` etc. `SessionEvent.TargetAudience` removed (was only populated from dead `pctx.Route`). Third-party plugins get a clean diagnostic slot they can populate without framework edits. - **Single-owner packages relocated**: `authlib/validation` → `authlib/plugins/jwtvalidation/validation`. `authlib/exchange` / `authlib/cache` / `authlib/spiffe` → `authlib/plugins/tokenexchange/{exchange,cache,spiffe}`. Each plugin now lives in its own directory (`plugins/jwtvalidation/plugin.go`, `plugins/tokenexchange/plugin.go`) and self-registers via its own init(). `authlib/bypass`, `authlib/routing`, `authlib/auth` stay shared. +- **Plugin relationship declarations**: `PluginCapabilities` extended with four chain-scoped fields — `Requires` (all-must-be-earlier), `RequiresAny` (at-least-one-earlier), `After` (soft ordering), `Claims` (mutex on a semantic resource). Validated at `plugins.Build` time (startup + hot-reload); all errors per chain are collected into one report. `authlib/contracts/claims.go` ships `ClaimAuthorizationHeader` as the initial canonical claim constant. `token-exchange` and `token-broker` migrated to declare it, so configuring both on the same outbound chain now fails startup instead of silently clobbering each other's Authorization header. See [`plugin-reference.md` "Declaring plugin relationships"](./plugin-reference.md#declaring-plugin-relationships). Breaking changes will be announced in `authbridge/CHANGELOG.md` (TBD) before a 1.0 tag. @@ -694,7 +695,7 @@ Breaking changes will be announced in `authbridge/CHANGELOG.md` (TBD) before a 1 - `pipeline.go` — `Pipeline` type, `New`, `Run`, `RunResponse`, `Start`, `Stop`, `Plugins`, `NeedsBody`, `WritesBody`. - `holder.go` — `Holder`, the atomic slot listeners hold in place of a raw `*Pipeline`. -- `plugin.go` — `Plugin` interface, `PluginCapabilities` (with `ReadsBody` / `WritesBody` / deprecated `BodyAccess` + `Normalize()`), `Configurable`, `Initializer`, `Shutdowner`, `Readier`. +- `plugin.go` — `Plugin` interface, `PluginCapabilities` (with `ReadsBody` / `WritesBody` / deprecated `BodyAccess` + `Normalize()`; chain-scoped relationship fields `Requires` / `RequiresAny` / `After` / `Claims`), `Configurable`, `Initializer`, `Shutdowner`, `Readier`. - `action.go` — `Action`, `ActionType`, `Violation`, helper constructors (`Deny`, `DenyStatus`, `DenyWithDetails`, `Challenge`, `RateLimited`), `StatusFromCode`. - `context.go` — `Context`, `Direction`, `AgentIdentity`, the `pctx.Record` / `Allow` / `Skip` / `Observe` / `Modify` / `DenyAndRecord` helpers, and `pctx.SetBody` / `SetResponseBody` / `BodyMutated` / `ResponseBodyMutated` for body mutation. - `extensions.go` — `Extensions` struct, `Invocation`, `Invocations`, `InvocationAction`, named protocol extensions, `GetState` / `SetState`. diff --git a/authbridge/docs/plugin-reference.md b/authbridge/docs/plugin-reference.md index eef870890..eee3e1a02 100644 --- a/authbridge/docs/plugin-reference.md +++ b/authbridge/docs/plugin-reference.md @@ -159,6 +159,123 @@ Shadowing auth turns authentication into a suggestion — don't do this in production. Shadow-mode is for third-party guardrails being canaried; auth gates should stay on `enforce` (the default). +## Declaring plugin relationships + +`PluginCapabilities` carries four fields that let a plugin express how it +relates to other plugins in the same chain. All four are checked at +`plugins.Build` time (startup and hot-reload), and misconfigurations +fail loud before serving traffic. + +```go +type PluginCapabilities struct { + ReadsBody bool + WritesBody bool + Reads []string + Writes []string + + Requires []string // ALL must be present + earlier (hard) + RequiresAny []string // AT LEAST ONE must be present + run after it (hard) + After []string // SOFT ordering; silent if absent + Claims []string // <=1 per claim per chain (mutex) +} +``` + +| Field | All present? | At least one? | Silent if absent? | Ordering enforced? | +|---|---|---|---|---| +| `Requires` | ✓ | — | — | ✓ | +| `RequiresAny` | — | ✓ | — | ✓ | +| `After` | — | — | ✓ | ✓ | +| `Claims` | — | — | — | — (mutex) | + +All four are chain-scoped — validation runs within the inbound chain +OR within the outbound chain, independently. Plugin names are +case-sensitive and must match the `Name()` returned by the plugin. + +### `Requires` — hard dependency + +Use when your plugin hardcodes access to a specific other plugin's +extension state. Each named plugin must be present in the same chain +AND appear earlier (lower index). Missing or misordered fails startup. + +```go +// Reads pctx.Extensions.MCP.Params["name"] directly — only makes sense +// with mcp-parser ahead of it. +func (p *ToolAllowlist) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{ + Requires: []string{"mcp-parser"}, + } +} +``` + +### `RequiresAny` — hard OR + +Use when your plugin is protocol-agnostic (e.g. reads through +`pctx.ContentSources()`) but genuinely needs at least one parser +present to have anything to do. Each named plugin that IS present +must also appear earlier. Missing-all-of-them fails startup. + +```go +// PII scrubber runs against whatever parsers emit fragments; must +// have at least one or it's silent dead code. +func (p *PIIScrubber) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{ + ReadsBody: true, + RequiresAny: []string{ + "a2a-parser", "mcp-parser", "inference-parser", + }, + } +} +``` + +### `After` — soft ordering + +Use for optional ordering relationships: the plugin benefits from a +named plugin running earlier when present, but runs fine without it. +Absent named plugin is not an error. + +```go +// Adds per-protocol labels to request counts if a parser is +// present; falls back to generic labels otherwise. +func (p *RequestCounter) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{ + After: []string{"a2a-parser", "mcp-parser", "inference-parser"}, + } +} +``` + +### `Claims` — mutual exclusion + +A claim is a semantic resource that exactly one plugin per chain +owns. Two plugins declaring the same claim fail startup. + +Claim strings are arbitrary, but the well-known set lives as Go +constants in `authbridge/authlib/contracts/claims.go` — plugin +authors reference the constants instead of string literals so +typos are compile errors and the canonical set is greppable. + +```go +import "github.com/kagenti/kagenti-extensions/authbridge/authlib/contracts" + +// token-exchange and token-broker both declare this; they can't +// coexist in the same outbound chain. +func (p *TokenExchange) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{ + Claims: []string{contracts.ClaimAuthorizationHeader}, + } +} +``` + +Third-party plugins may declare arbitrary strings — the framework +enforces uniqueness of whatever it sees, not "must be from the list." +Upstream a new constant in a follow-up PR when a claim stabilizes. + +### Error collection + +When validation fails the error aggregates every violation in the +chain, not just the first one found. Operators iterating on a +freshly-edited YAML get one fix-list per startup attempt rather +than a sequence of fix-one-at-a-time restarts. + ## The Configurable interface ```go