From d68ab24704fd273e7175e765ba1ca88d820e5b89 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sun, 10 May 2026 08:52:44 -0400 Subject: [PATCH 1/6] refactor(pipeline): Delete dead pctx.Route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pctx.Route was defined on Context but never written by any plugin. The only reader — listener's routeAudience() helper in extproc — always returned "" because pctx.Route was always nil. The route decision already surfaces through token-exchange's auth.OutboundResult and the Invocation stream; the dead field on Context was a vestige of an earlier composition shape. Delete: - Route *routing.ResolvedRoute field on pipeline.Context - routing import from authlib/pipeline/context.go - routeAudience() helper in cmd/authbridge/listener/extproc/server.go - Two TargetAudience: routeAudience(pctx) field assignments (now zero-valued, same observable result) - TestRouteAudience (tested dead code) - Route: &routing.ResolvedRoute{...} in TestRecordOutboundResponseSession (was setting state nothing reads); test simplified to CapturesHostAndDuration. Net: -28 LOC. One fewer plugin-specific package imported by the pipeline framework. SessionEvent.TargetAudience is now always zero in the live events; the field itself stays for now (wire-schema removal is part of the Invocation-field collapse in a follow-on commit on this branch). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/pipeline/context.go | 4 +-- .../cmd/authbridge/listener/extproc/server.go | 17 ++--------- .../listener/extproc/server_test.go | 30 +------------------ 3 files changed, 5 insertions(+), 46 deletions(-) diff --git a/authbridge/authlib/pipeline/context.go b/authbridge/authlib/pipeline/context.go index 7e7c3b185..9662985a2 100644 --- a/authbridge/authlib/pipeline/context.go +++ b/authbridge/authlib/pipeline/context.go @@ -8,7 +8,6 @@ import ( "net/http" "time" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) @@ -79,8 +78,7 @@ type Context struct { Agent *AgentIdentity Claims *validation.Claims // nil before jwt-validation runs - Route *routing.ResolvedRoute - Session *SessionView // nil unless session tracking is enabled + Session *SessionView // nil unless session tracking is enabled // Response-phase fields (populated by listener before RunResponse). // ResponseBody may be nil even during response phase if no plugin declared BodyAccess. diff --git a/authbridge/cmd/authbridge/listener/extproc/server.go b/authbridge/cmd/authbridge/listener/extproc/server.go index dd9d444c7..63352dfe3 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server.go +++ b/authbridge/cmd/authbridge/listener/extproc/server.go @@ -373,9 +373,8 @@ func (s *Server) recordOutboundResponseSession(pctx *pipeline.Context) { Identity: snapshotIdentity(pctx), StatusCode: pctx.StatusCode, Error: deriveError(pctx), - Host: pctx.Host, - TargetAudience: routeAudience(pctx), - Duration: durationSince(pctx.StartedAt), + Host: pctx.Host, + Duration: durationSince(pctx.StartedAt), } // Auth / Plugins alone qualify for recording; matches the widened // gate in recordInboundSession so outbound denials and plugin-public @@ -407,15 +406,6 @@ func snapshotIdentity(pctx *pipeline.Context) *pipeline.EventIdentity { return id } -// routeAudience returns the resolved OAuth audience for an outbound request, -// or "" when no route matched (passthrough) or the event is inbound. -func routeAudience(pctx *pipeline.Context) string { - if pctx.Route == nil || !pctx.Route.Matched { - return "" - } - return pctx.Route.Audience -} - // durationSince returns the elapsed time since StartedAt, or 0 when StartedAt // is zero (pctx constructed without wall-clock stamping, e.g. in unit tests). func durationSince(start time.Time) time.Duration { @@ -476,8 +466,7 @@ func (s *Server) recordOutboundSession(pctx *pipeline.Context) { Invocations: snapshotInvocations(pctx.Extensions.Invocations, pipeline.InvocationPhaseRequest), Plugins: plugins, Identity: snapshotIdentity(pctx), - Host: pctx.Host, - TargetAudience: routeAudience(pctx), + Host: pctx.Host, } if ev.MCP != nil || ev.Inference != nil || ev.Invocations != nil || plugins != nil { s.Sessions.Append(sid, ev) diff --git a/authbridge/cmd/authbridge/listener/extproc/server_test.go b/authbridge/cmd/authbridge/listener/extproc/server_test.go index 3c72a3ab5..b1251dfd0 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server_test.go +++ b/authbridge/cmd/authbridge/listener/extproc/server_test.go @@ -1162,27 +1162,6 @@ func TestRecordOutboundResponseSession_CapturesStatusAndError(t *testing.T) { } } -func TestRouteAudience(t *testing.T) { - cases := []struct { - name string - pctx *pipeline.Context - want string - }{ - {"nil route", &pipeline.Context{}, ""}, - {"unmatched route falls through to passthrough", - &pipeline.Context{Route: &routing.ResolvedRoute{Matched: false, Audience: "ignored"}}, ""}, - {"matched route surfaces audience", - &pipeline.Context{Route: &routing.ResolvedRoute{Matched: true, Audience: "github-tool"}}, "github-tool"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - if got := routeAudience(tc.pctx); got != tc.want { - t.Errorf("got %q, want %q", got, tc.want) - } - }) - } -} - func TestDurationSince(t *testing.T) { if d := durationSince(time.Time{}); d != 0 { t.Errorf("zero StartedAt should yield 0, got %v", d) @@ -1193,7 +1172,7 @@ func TestDurationSince(t *testing.T) { } } -func TestRecordOutboundResponseSession_CapturesHostAndRoute(t *testing.T) { +func TestRecordOutboundResponseSession_CapturesHostAndDuration(t *testing.T) { store := session.New(5*time.Minute, 100, 0) defer store.Close() store.Append(session.DefaultSessionID, pipeline.SessionEvent{}) @@ -1204,10 +1183,6 @@ func TestRecordOutboundResponseSession_CapturesHostAndRoute(t *testing.T) { StartedAt: start, Host: "github-tool-mcp", StatusCode: 200, - Route: &routing.ResolvedRoute{ - Matched: true, - Audience: "github-tool", - }, Extensions: pipeline.Extensions{ MCP: &pipeline.MCPExtension{Method: "tools/call"}, }, @@ -1219,9 +1194,6 @@ func TestRecordOutboundResponseSession_CapturesHostAndRoute(t *testing.T) { if resp.Host != "github-tool-mcp" { t.Errorf("Host = %q, want github-tool-mcp", resp.Host) } - if resp.TargetAudience != "github-tool" { - t.Errorf("TargetAudience = %q, want github-tool", resp.TargetAudience) - } if resp.Duration < 25*time.Millisecond { t.Errorf("Duration = %v, want >= 25ms", resp.Duration) } From f0a877fb6983a016d37dc5750224e027ddef6d95 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sun, 10 May 2026 08:56:23 -0400 Subject: [PATCH 2/6] refactor(pipeline): Detype pctx.Claims to Identity interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pipeline.Context previously imported authlib/validation to type its Claims field as *validation.Claims — leaking jwt-validation's output type into the framework. Any third-party auth plugin (SAML, mTLS, custom header-auth) had no clean slot for subject identity; they'd either have to shoehorn their data into validation.Claims semantically, or publish into Extensions.Custom and lose the typed Identity-on-event surface that the listener's snapshotIdentity depends on. This commit: 1. Introduces pipeline.Identity as a three-method interface: Subject(), ClientID(), Scopes(). Empty-string / nil returns are valid — a SPIFFE authenticator with no "clientID" just returns "". Framework code reads via the interface only. 2. Renames Context.Claims → Context.Identity, changes the type from *validation.Claims → Identity. Breaking source change; any external plugin reading pctx.Claims.Subject now reads pctx.Identity.Subject(). 3. Deletes the authlib/validation import from pipeline/context.go. pipeline/ no longer imports any plugin-specific package. 4. Adds a claimsIdentity adapter in the plugins package (colocated with jwt-validation) that wraps *validation.Claims. jwt-validation sets pctx.Identity = claimsIdentity{c: result.Claims}. 5. Adds a testIdentity adapter in plugintesting so the JWT stub used by listener tests continues to populate pctx.Identity without reaching into validation.Claims through the framework. 6. Adds a stubIdentity in extproc's server_test so TestSnapshotIdentity and the recordOutboundResponseSession_CapturesHostAndDuration test set pctx.Identity directly without depending on validation.Claims. Invariant check: $ grep -rn 'authlib/(validation|routing)' authbridge/authlib/pipeline/ (no output — pipeline/ is now free of plugin-specific imports) Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/pipeline/context.go | 24 ++++++-- authbridge/authlib/plugins/jwtvalidation.go | 7 ++- .../authlib/plugins/jwtvalidation_identity.go | 34 +++++++++++ .../plugins/plugintesting/plugintesting.go | 21 ++++++- .../cmd/authbridge/listener/extproc/server.go | 16 +++-- .../listener/extproc/server_test.go | 59 +++++++++++-------- 6 files changed, 123 insertions(+), 38 deletions(-) create mode 100644 authbridge/authlib/plugins/jwtvalidation_identity.go diff --git a/authbridge/authlib/pipeline/context.go b/authbridge/authlib/pipeline/context.go index 9662985a2..abdc4cf00 100644 --- a/authbridge/authlib/pipeline/context.go +++ b/authbridge/authlib/pipeline/context.go @@ -7,10 +7,24 @@ import ( "log/slog" "net/http" "time" - - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) +// Identity carries the subject identity established by whichever auth +// plugin ran — jwt-validation, SAML, mTLS, custom. Populated by the +// auth plugin via pctx.Identity = . Listener reads via these +// methods — not via concrete-type assertion — so plugins can contribute +// any identity shape without the framework caring. +// +// Returning empty-string / nil from any method is valid (e.g., a +// SPIFFE-SVID authenticator may have no "ClientID" concept). Consumers +// that need plugin-specific fields type-assert to a richer interface +// or to the plugin's known concrete type. +type Identity interface { + Subject() string // stable subject ID (sub claim / SPIFFE ID / email) + ClientID() string // registering-client ID, if applicable + Scopes() []string // granted scopes / roles +} + // Direction indicates whether a request is inbound (caller → this agent) or // outbound (this agent → target service). type Direction int @@ -76,9 +90,9 @@ type Context struct { // compute SessionEvent.Duration without walking the event history. StartedAt time.Time - Agent *AgentIdentity - Claims *validation.Claims // nil before jwt-validation runs - Session *SessionView // nil unless session tracking is enabled + Agent *AgentIdentity + Identity Identity // nil before an auth plugin runs + Session *SessionView // nil unless session tracking is enabled // Response-phase fields (populated by listener before RunResponse). // ResponseBody may be nil even during response phase if no plugin declared BodyAccess. diff --git a/authbridge/authlib/plugins/jwtvalidation.go b/authbridge/authlib/plugins/jwtvalidation.go index ee9cb9606..fb6a6905f 100644 --- a/authbridge/authlib/plugins/jwtvalidation.go +++ b/authbridge/authlib/plugins/jwtvalidation.go @@ -333,9 +333,10 @@ func (p *JWTValidation) OnRequest(ctx context.Context, pctx *pipeline.Context) p } // ActionAllow with Claims = authorized. Surface what the plugin - // VERIFIED in the token — diverges from the top-level Identity - // snapshot if later plugins re-annotate pctx.Claims. - pctx.Claims = result.Claims + // VERIFIED in the token via the pipeline.Identity interface. + // Plugins that don't run jwt-validation (SAML, mTLS, custom) + // publish their own adapter; listeners read through the interface. + pctx.Identity = claimsIdentity{c: result.Claims} pctx.Record(pipeline.Invocation{ Action: pipeline.ActionAllow, Reason: auth.APPROVE_AUTHORIZED.String(), diff --git a/authbridge/authlib/plugins/jwtvalidation_identity.go b/authbridge/authlib/plugins/jwtvalidation_identity.go new file mode 100644 index 000000000..f19fe1cf8 --- /dev/null +++ b/authbridge/authlib/plugins/jwtvalidation_identity.go @@ -0,0 +1,34 @@ +package plugins + +import "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" + +// claimsIdentity adapts a *validation.Claims to the pipeline.Identity +// interface that Context exposes. Kept in the plugin so the pipeline +// package doesn't import any validation-specific types. +// +// A nil *validation.Claims would cause NPEs on the accessor methods, +// so jwt-validation only wraps non-nil Claims. +type claimsIdentity struct { + c *validation.Claims +} + +func (i claimsIdentity) Subject() string { + if i.c == nil { + return "" + } + return i.c.Subject +} + +func (i claimsIdentity) ClientID() string { + if i.c == nil { + return "" + } + return i.c.ClientID +} + +func (i claimsIdentity) Scopes() []string { + if i.c == nil { + return nil + } + return i.c.Scopes +} diff --git a/authbridge/authlib/plugins/plugintesting/plugintesting.go b/authbridge/authlib/plugins/plugintesting/plugintesting.go index 20b600ec9..5f846e5d2 100644 --- a/authbridge/authlib/plugins/plugintesting/plugintesting.go +++ b/authbridge/authlib/plugins/plugintesting/plugintesting.go @@ -63,10 +63,29 @@ func (p *JWTValidationStub) OnRequest(ctx context.Context, pctx *pipeline.Contex } return pipeline.DenyStatus(result.DenyStatus, code, result.DenyReason) } - pctx.Claims = result.Claims + if result.Claims != nil { + pctx.Identity = testIdentity{ + subject: result.Claims.Subject, + clientID: result.Claims.ClientID, + scopes: result.Claims.Scopes, + } + } return pipeline.Action{Type: pipeline.Continue} } +// testIdentity is a minimal pipeline.Identity adapter scoped to this +// test helper. Kept separate from the jwt-validation plugin's own +// adapter so plugintesting stays self-contained and doesn't reach into +// a package-internal implementation. +type testIdentity struct { + subject, clientID string + scopes []string +} + +func (i testIdentity) Subject() string { return i.subject } +func (i testIdentity) ClientID() string { return i.clientID } +func (i testIdentity) Scopes() []string { return i.scopes } + func (p *JWTValidationStub) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { return pipeline.Action{Type: pipeline.Continue} } diff --git a/authbridge/cmd/authbridge/listener/extproc/server.go b/authbridge/cmd/authbridge/listener/extproc/server.go index 63352dfe3..8ff8ffe59 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server.go +++ b/authbridge/cmd/authbridge/listener/extproc/server.go @@ -389,15 +389,19 @@ func (s *Server) recordOutboundResponseSession(pctx *pipeline.Context) { // stays valid after pctx is discarded. Returns nil when no identity information // is available (e.g., jwt-validation didn't run on this path). func snapshotIdentity(pctx *pipeline.Context) *pipeline.EventIdentity { - if pctx.Claims == nil && pctx.Agent == nil { + // Read identity via the pipeline.Identity interface. Whichever auth + // plugin ran (jwt-validation today; SAML / mTLS / custom later) + // published an adapter onto pctx.Identity — we don't need to know + // its concrete type to snapshot the subject/client/scopes. + if pctx.Identity == nil && pctx.Agent == nil { return nil } id := &pipeline.EventIdentity{} - if pctx.Claims != nil { - id.Subject = pctx.Claims.Subject - id.ClientID = pctx.Claims.ClientID - if len(pctx.Claims.Scopes) > 0 { - id.Scopes = append([]string(nil), pctx.Claims.Scopes...) + if pctx.Identity != nil { + id.Subject = pctx.Identity.Subject() + id.ClientID = pctx.Identity.ClientID() + if scopes := pctx.Identity.Scopes(); len(scopes) > 0 { + id.Scopes = append([]string(nil), scopes...) } } if pctx.Agent != nil { diff --git a/authbridge/cmd/authbridge/listener/extproc/server_test.go b/authbridge/cmd/authbridge/listener/extproc/server_test.go index b1251dfd0..1fd4d10e8 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server_test.go +++ b/authbridge/cmd/authbridge/listener/extproc/server_test.go @@ -62,6 +62,19 @@ func (v *mockVerifier) Verify(_ context.Context, _ string, _ string) (*validatio return v.claims, v.err } +// stubIdentity is a minimal pipeline.Identity for tests that construct +// pctx directly (without running jwt-validation). A handful of session- +// recording tests rely on "pctx has a known identity" to verify the +// snapshot path. +type stubIdentity struct { + subject, clientID string + scopes []string +} + +func (s stubIdentity) Subject() string { return s.subject } +func (s stubIdentity) ClientID() string { return s.clientID } +func (s stubIdentity) Scopes() []string { return s.scopes } + func serverFromAuth(t *testing.T, a *auth.Auth) *Server { t.Helper() // Plugins build their own auth.Auth from local config in @@ -1019,17 +1032,17 @@ func TestRecordResponseSessions_NilStore(t *testing.T) { func TestSnapshotIdentity(t *testing.T) { cases := []struct { - name string - claims *validation.Claims - agent *pipeline.AgentIdentity - want *pipeline.EventIdentity + name string + identity pipeline.Identity + agent *pipeline.AgentIdentity + want *pipeline.EventIdentity }{ { - name: "claims and agent both set", - claims: &validation.Claims{ - Subject: "alice", - ClientID: "kagenti-ui", - Scopes: []string{"openid", "weather-read"}, + name: "identity and agent both set", + identity: stubIdentity{ + subject: "alice", + clientID: "kagenti-ui", + scopes: []string{"openid", "weather-read"}, }, agent: &pipeline.AgentIdentity{WorkloadID: "spiffe://localtest.me/ns/team1/sa/weather-agent"}, want: &pipeline.EventIdentity{ @@ -1040,22 +1053,22 @@ func TestSnapshotIdentity(t *testing.T) { }, }, { - name: "only agent (outbound, no JWT validation)", - claims: nil, - agent: &pipeline.AgentIdentity{WorkloadID: "spiffe://localtest.me/ns/team1/sa/weather-agent"}, - want: &pipeline.EventIdentity{AgentID: "spiffe://localtest.me/ns/team1/sa/weather-agent"}, + name: "only agent (outbound, no JWT validation)", + identity: nil, + agent: &pipeline.AgentIdentity{WorkloadID: "spiffe://localtest.me/ns/team1/sa/weather-agent"}, + want: &pipeline.EventIdentity{AgentID: "spiffe://localtest.me/ns/team1/sa/weather-agent"}, }, { - name: "neither set", - claims: nil, - agent: nil, - want: nil, + name: "neither set", + identity: nil, + agent: nil, + want: nil, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - got := snapshotIdentity(&pipeline.Context{Claims: tc.claims, Agent: tc.agent}) + got := snapshotIdentity(&pipeline.Context{Identity: tc.identity, Agent: tc.agent}) if tc.want == nil { if got != nil { t.Errorf("got %+v, want nil", got) @@ -1076,10 +1089,10 @@ func TestSnapshotIdentity(t *testing.T) { } func TestSnapshotIdentity_ScopesDeepCopy(t *testing.T) { - // Mutating the claims slice after snapshot must not mutate the event. - claims := &validation.Claims{Subject: "alice", Scopes: []string{"a", "b"}} - id := snapshotIdentity(&pipeline.Context{Claims: claims}) - claims.Scopes[0] = "x" + // Mutating the source slice after snapshot must not mutate the event. + scopes := []string{"a", "b"} + id := snapshotIdentity(&pipeline.Context{Identity: stubIdentity{subject: "alice", scopes: scopes}}) + scopes[0] = "x" if id.Scopes[0] != "a" { t.Errorf("snapshot scopes aliased original: got %v", id.Scopes) } @@ -1139,7 +1152,7 @@ func TestRecordOutboundResponseSession_CapturesStatusAndError(t *testing.T) { pctx := &pipeline.Context{ StatusCode: 503, - Claims: &validation.Claims{Subject: "alice"}, + Identity: stubIdentity{subject: "alice"}, Extensions: pipeline.Extensions{ MCP: &pipeline.MCPExtension{Method: "tools/call"}, }, From dcda457f45fe82a6feb9c1136fb8167c6dc8bc42 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sun, 10 May 2026 09:02:00 -0400 Subject: [PATCH 3/6] refactor(pipeline): Collapse Invocation diagnostic fields into Details map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pipeline.Invocation previously carried 9 jwt-validation + token-exchange- specific diagnostic fields (ExpectedIssuer, ExpectedAudience, TokenSubject, TokenAudience, TokenScopes, RouteMatched, RouteHost, TargetAudience, RequestedScopes, CacheHit). Third-party plugins inherited those slots whether they wanted them or not, and had no typed home for their own domain-specific context (rate-limiter tokens- remaining, DLP match-counts, broker consent-state, etc.). Replace with Details map[string]string: plugin-specific context, opaque to the framework, rendered as key=value rows by abctl. Conventions: - snake_case keys scoped to the plugin's semantic domain - booleans as "true"/"false" (boolStr helper) - []string as space-joined (matches OAuth scope convention) - NEVER put raw tokens, signatures, or credentials in values Migrations in this commit: jwt-validation Invocation fields → Details: expected_issuer, expected_audience (deny branch) token_subject, token_audience, token_scopes (allow) token-exchange Invocation fields → Details: route_matched, route_host, target_audience, requested_scopes, cache_hit SessionEvent.TargetAudience: removed. Only populator was the routeAudience() helper that went away with D1; operators read target audience via the Invocation Details map now. authlib/session/store.go: drops the "aud" log attribute that was reading the now-removed SessionEvent field. authlib/cmd/abctl/tui/events_pane.go: filter-text search now iterates inv.Details keys+values for substring match. detail_pane.go needs no change — it renders the session event as pretty-printed JSON and the new "details" field shows up inline with everything else. Test migrations: ~30 assertions across pipeline/session_test, plugins/plugins_test, sessionapi/server_test, extproc/server_test rewritten from `inv.ExpectedIssuer == "foo"` to `inv.Details["expected_issuer"] == "foo"`. Mechanical. Wire-breaking: the session API response shape changes. No versioning exists today so no compat shim. Only known consumer is abctl (in-tree on this branch). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/pipeline/extensions.go | 30 +++++----- authbridge/authlib/pipeline/session.go | 21 ++----- authbridge/authlib/pipeline/session_test.go | 39 +++++++------ authbridge/authlib/plugins/jwtvalidation.go | 22 +++++--- authbridge/authlib/plugins/plugins_test.go | 32 +++++------ authbridge/authlib/plugins/tokenexchange.go | 55 +++++++++++-------- authbridge/authlib/session/store.go | 3 - authbridge/authlib/sessionapi/server_test.go | 12 ++-- authbridge/cmd/abctl/tui/events_pane.go | 13 +++-- .../listener/extproc/server_test.go | 14 +++-- 10 files changed, 127 insertions(+), 114 deletions(-) diff --git a/authbridge/authlib/pipeline/extensions.go b/authbridge/authlib/pipeline/extensions.go index 5e82bff47..73d526e0c 100644 --- a/authbridge/authlib/pipeline/extensions.go +++ b/authbridge/authlib/pipeline/extensions.go @@ -279,20 +279,22 @@ type Invocation struct { // when the plugin has no path context. Path string `json:"path,omitempty"` - // Auth-gate context, populated when applicable. - ExpectedIssuer string `json:"expectedIssuer,omitempty"` - ExpectedAudience string `json:"expectedAudience,omitempty"` - TokenSubject string `json:"tokenSubject,omitempty"` - TokenAudience []string `json:"tokenAudience,omitempty"` - TokenScopes []string `json:"tokenScopes,omitempty"` - - // Outbound routing context. RouteMatched=true means a route rule - // explicitly applied; false means the default policy caught it. - RouteMatched bool `json:"routeMatched,omitempty"` - RouteHost string `json:"routeHost,omitempty"` - TargetAudience string `json:"targetAudience,omitempty"` - RequestedScopes []string `json:"requestedScopes,omitempty"` - CacheHit bool `json:"cacheHit,omitempty"` + // Details carries plugin-specific context as a flat string→string + // map. Opaque to the framework; abctl renders it as key=value rows + // in the invocation detail pane. Suggested convention: snake_case + // keys scoped to the plugin's semantic domain. Built-in plugins + // use keys like expected_issuer, token_subject, route_host, + // target_audience, cache_hit. Third-party plugins define their own. + // + // Stringify booleans as "true"/"false" and []string as space- + // joined (matching OAuth scope conventions). Keep values short + // enough for a detail pane — bulky diagnostics belong in the + // Extensions.Custom escape-hatch event. + // + // NEVER put raw tokens, signatures, or client credentials here. + // The session API has no auth on it — only safe-to-log data + // belongs in Invocation.Details. + Details map[string]string `json:"details,omitempty"` } // DelegationExtension tracks the token delegation chain across hops. diff --git a/authbridge/authlib/pipeline/session.go b/authbridge/authlib/pipeline/session.go index 1868a5e17..6d4de9ead 100644 --- a/authbridge/authlib/pipeline/session.go +++ b/authbridge/authlib/pipeline/session.go @@ -125,12 +125,6 @@ type SessionEvent struct { // landed on. Empty when the listener didn't populate pctx.Host. Host string - // TargetAudience is the OAuth audience the outbound request was routed - // to, or empty when no route matched (passthrough) or the event is - // inbound. Useful for policy plugins that care which target scope a - // call was made against, independent of host (which can be a glob). - TargetAudience string - // Duration is the wall-clock time from request entry into the listener // to response recording. Zero on request-phase events. On response // events it's computed as now - matching-request.At. @@ -158,9 +152,8 @@ type sessionEventWire struct { Identity *EventIdentity `json:"identity,omitempty"` StatusCode int `json:"statusCode,omitempty"` Error *EventError `json:"error,omitempty"` - Host string `json:"host,omitempty"` - TargetAudience string `json:"targetAudience,omitempty"` - DurationMs int64 `json:"durationMs,omitempty"` + Host string `json:"host,omitempty"` + DurationMs int64 `json:"durationMs,omitempty"` } func (e SessionEvent) MarshalJSON() ([]byte, error) { @@ -177,9 +170,8 @@ func (e SessionEvent) MarshalJSON() ([]byte, error) { Identity: e.Identity, StatusCode: e.StatusCode, Error: e.Error, - Host: e.Host, - TargetAudience: e.TargetAudience, - DurationMs: e.Duration.Milliseconds(), + Host: e.Host, + DurationMs: e.Duration.Milliseconds(), }) } @@ -204,9 +196,8 @@ func (e *SessionEvent) UnmarshalJSON(data []byte) error { Identity: w.Identity, StatusCode: w.StatusCode, Error: w.Error, - Host: w.Host, - TargetAudience: w.TargetAudience, - Duration: time.Duration(w.DurationMs) * time.Millisecond, + Host: w.Host, + Duration: time.Duration(w.DurationMs) * time.Millisecond, } return nil } diff --git a/authbridge/authlib/pipeline/session_test.go b/authbridge/authlib/pipeline/session_test.go index 9663f88e7..2680a33f7 100644 --- a/authbridge/authlib/pipeline/session_test.go +++ b/authbridge/authlib/pipeline/session_test.go @@ -76,10 +76,9 @@ func TestSessionEvent_JSONRoundTrip(t *testing.T) { ToolCalls: []InferenceToolCall{{ID: "c1", Name: "get_weather", Arguments: `{"city":"NYC"}`}}, Completion: "Hello, world!", FinishReason: "stop", TotalTokens: 17, }, - Identity: &EventIdentity{Subject: "alice", ClientID: "agent-1"}, - StatusCode: 200, - Error: &EventError{Kind: "upstream", Message: "timeout"}, - TargetAudience: "github-tool", + Identity: &EventIdentity{Subject: "alice", ClientID: "agent-1"}, + StatusCode: 200, + Error: &EventError{Kind: "upstream", Message: "timeout"}, } first, err := json.Marshal(orig) @@ -107,7 +106,7 @@ func TestSessionEvent_MarshalJSON_OmitsEmpty(t *testing.T) { } s := string(data) - for _, field := range []string{"a2a", "mcp", "inference", "auth", "plugins", "identity", "statusCode", "error", "host", "targetAudience", "durationMs"} { + for _, field := range []string{"a2a", "mcp", "inference", "auth", "plugins", "identity", "statusCode", "error", "host", "durationMs"} { if strings.Contains(s, `"`+field+`":`) { t.Errorf("expected %q omitted when zero: %s", field, s) } @@ -150,21 +149,25 @@ func TestSessionEvent_Invocations_JSONRoundTrip(t *testing.T) { Phase: SessionDenied, Invocations: &Invocations{ Inbound: []Invocation{{ - Plugin: "jwt-validation", - Action: ActionDeny, - Reason: "jwt_failed", - ExpectedIssuer: "http://keycloak.localtest.me:8080/realms/kagenti", - ExpectedAudience: "spiffe://localtest.me/ns/team1/sa/weather-tool", + Plugin: "jwt-validation", + Action: ActionDeny, + Reason: "jwt_failed", + Details: map[string]string{ + "expected_issuer": "http://keycloak.localtest.me:8080/realms/kagenti", + "expected_audience": "spiffe://localtest.me/ns/team1/sa/weather-tool", + }, }}, Outbound: []Invocation{{ - Plugin: "token-exchange", - Action: ActionModify, - Reason: "cache_hit", - RouteMatched: true, - RouteHost: "weather-tool-mcp", - TargetAudience: "spiffe://localtest.me/ns/team1/sa/weather-tool", - RequestedScopes: []string{"openid", "weather-aud"}, - CacheHit: true, + Plugin: "token-exchange", + Action: ActionModify, + Reason: "cache_hit", + Details: map[string]string{ + "route_matched": "true", + "route_host": "weather-tool-mcp", + "target_audience": "spiffe://localtest.me/ns/team1/sa/weather-tool", + "requested_scopes": "openid weather-aud", + "cache_hit": "true", + }, }}, }, } diff --git a/authbridge/authlib/plugins/jwtvalidation.go b/authbridge/authlib/plugins/jwtvalidation.go index fb6a6905f..54abf99ef 100644 --- a/authbridge/authlib/plugins/jwtvalidation.go +++ b/authbridge/authlib/plugins/jwtvalidation.go @@ -306,10 +306,12 @@ func (p *JWTValidation) OnRequest(ctx context.Context, pctx *pipeline.Context) p // ExpectedIssuer / ExpectedAudience diagnostic fields that the // one-liner DenyAndRecord doesn't accept. pctx.Record(pipeline.Invocation{ - Action: pipeline.ActionDeny, - Reason: result.DenyReasonCode.String(), - ExpectedIssuer: p.cfg.Issuer, - ExpectedAudience: audience, + Action: pipeline.ActionDeny, + Reason: result.DenyReasonCode.String(), + Details: map[string]string{ + "expected_issuer": p.cfg.Issuer, + "expected_audience": audience, + }, }) // result.DenyReason carries the specific failure (missing header, // audience mismatch, expired, etc.). Pick a code whose default @@ -338,11 +340,13 @@ func (p *JWTValidation) OnRequest(ctx context.Context, pctx *pipeline.Context) p // publish their own adapter; listeners read through the interface. pctx.Identity = claimsIdentity{c: result.Claims} pctx.Record(pipeline.Invocation{ - Action: pipeline.ActionAllow, - Reason: auth.APPROVE_AUTHORIZED.String(), - TokenSubject: result.Claims.Subject, - TokenAudience: result.Claims.Audience, - TokenScopes: result.Claims.Scopes, + Action: pipeline.ActionAllow, + Reason: auth.APPROVE_AUTHORIZED.String(), + Details: map[string]string{ + "token_subject": result.Claims.Subject, + "token_audience": strings.Join(result.Claims.Audience, ","), + "token_scopes": strings.Join(result.Claims.Scopes, " "), + }, }) return pipeline.Action{Type: pipeline.Continue} } diff --git a/authbridge/authlib/plugins/plugins_test.go b/authbridge/authlib/plugins/plugins_test.go index 0f29efd6c..2051d7316 100644 --- a/authbridge/authlib/plugins/plugins_test.go +++ b/authbridge/authlib/plugins/plugins_test.go @@ -412,8 +412,8 @@ func TestJWTValidation_OnRequest_PopulatesAuth_Deny_NoHeader(t *testing.T) { if got.Reason != "no_header" { t.Errorf("Reason = %q, want no_header", got.Reason) } - if got.ExpectedIssuer != "http://issuer.example" { - t.Errorf("ExpectedIssuer = %q, want http://issuer.example", got.ExpectedIssuer) + if got.Details["expected_issuer"] != "http://issuer.example" { + t.Errorf("expected_issuer = %q, want http://issuer.example", got.Details["expected_issuer"]) } } @@ -444,14 +444,14 @@ func TestJWTValidation_OnRequest_PopulatesAuth_Allow(t *testing.T) { if got.Action != pipeline.ActionAllow || got.Reason != "authorized" { t.Errorf("got Action=%q Reason=%q, want allow/authorized", got.Action, got.Reason) } - if got.TokenSubject != "alice" { - t.Errorf("TokenSubject = %q, want alice", got.TokenSubject) + if got.Details["token_subject"] != "alice" { + t.Errorf("token_subject = %q, want alice", got.Details["token_subject"]) } - if len(got.TokenScopes) != 2 || got.TokenScopes[0] != "openid" { - t.Errorf("TokenScopes = %v, want [openid write]", got.TokenScopes) + if got.Details["token_scopes"] != "openid write" { + t.Errorf("token_scopes = %q, want \"openid write\"", got.Details["token_scopes"]) } - if len(got.TokenAudience) != 1 || got.TokenAudience[0] != "agent-aud" { - t.Errorf("TokenAudience = %v, want [agent-aud]", got.TokenAudience) + if got.Details["token_audience"] != "agent-aud" { + t.Errorf("token_audience = %q, want agent-aud", got.Details["token_audience"]) } } @@ -667,11 +667,11 @@ func TestTokenExchange_Passthrough(t *testing.T) { if ob.Plugin != "token-exchange" || ob.Action != pipeline.ActionSkip { t.Errorf("entry = (%q, %q), want (token-exchange, skip)", ob.Plugin, ob.Action) } - if ob.RouteHost != "some-host" { - t.Errorf("RouteHost = %q, want some-host", ob.RouteHost) + if ob.Details["route_host"] != "some-host" { + t.Errorf("route_host = %q, want some-host", ob.Details["route_host"]) } - if ob.RouteMatched { - t.Error("RouteMatched should be false on default-policy passthrough") + if ob.Details["route_matched"] != "false" { + t.Errorf("route_matched = %q, want false on default-policy passthrough", ob.Details["route_matched"]) } } @@ -718,11 +718,11 @@ func TestTokenExchange_ExchangeSuccess(t *testing.T) { if got.Plugin != "token-exchange" || got.Action != pipeline.ActionModify { t.Errorf("got Plugin=%q Action=%q, want token-exchange/modify", got.Plugin, got.Action) } - if got.RouteHost != "target-svc" { - t.Errorf("RouteHost = %q, want target-svc", got.RouteHost) + if got.Details["route_host"] != "target-svc" { + t.Errorf("route_host = %q, want target-svc", got.Details["route_host"]) } - if got.CacheHit { - t.Error("CacheHit = true on first exchange; should be false") + if got.Details["cache_hit"] != "false" { + t.Errorf("cache_hit = %q, want false on first exchange", got.Details["cache_hit"]) } } diff --git a/authbridge/authlib/plugins/tokenexchange.go b/authbridge/authlib/plugins/tokenexchange.go index 57e7c4dcf..fdd30080a 100644 --- a/authbridge/authlib/plugins/tokenexchange.go +++ b/authbridge/authlib/plugins/tokenexchange.go @@ -477,12 +477,14 @@ func (p *TokenExchange) OnRequest(ctx context.Context, pctx *pipeline.Context) p switch result.Action { case auth.ActionDeny: pctx.Record(pipeline.Invocation{ - Action: pipeline.ActionDeny, - Reason: result.DenyReasonCode.String(), - RouteMatched: result.RouteMatched, - RouteHost: host, - TargetAudience: result.TargetAudience, - RequestedScopes: splitScopes(result.RequestedScopes), + Action: pipeline.ActionDeny, + Reason: result.DenyReasonCode.String(), + Details: map[string]string{ + "route_matched": boolStr(result.RouteMatched), + "route_host": host, + "target_audience": result.TargetAudience, + "requested_scopes": result.RequestedScopes, + }, }) // Outbound denials almost always come from failed token exchange // at the IdP (upstream unreachable, bad credentials, audience @@ -500,13 +502,15 @@ func (p *TokenExchange) OnRequest(ctx context.Context, pctx *pipeline.Context) p reason = "cache_hit" } pctx.Record(pipeline.Invocation{ - Action: pipeline.ActionModify, - Reason: reason, - RouteMatched: true, - RouteHost: host, - TargetAudience: result.TargetAudience, - RequestedScopes: splitScopes(result.RequestedScopes), - CacheHit: result.CacheHit, + Action: pipeline.ActionModify, + Reason: reason, + Details: map[string]string{ + "route_matched": "true", + "route_host": host, + "target_audience": result.TargetAudience, + "requested_scopes": result.RequestedScopes, + "cache_hit": boolStr(result.CacheHit), + }, }) default: // ActionAllow / unroutable host / default-policy=passthrough all @@ -518,23 +522,26 @@ func (p *TokenExchange) OnRequest(ctx context.Context, pctx *pipeline.Context) p reason = "route_passthrough" } pctx.Record(pipeline.Invocation{ - Action: pipeline.ActionSkip, - Reason: reason, - RouteMatched: result.RouteMatched, - RouteHost: host, + Action: pipeline.ActionSkip, + Reason: reason, + Details: map[string]string{ + "route_matched": boolStr(result.RouteMatched), + "route_host": host, + }, }) } return pipeline.Action{Type: pipeline.Continue} } -// splitScopes turns a space-separated scope string into []string. Returns -// nil for the empty string so the JSON omitempty tag drops the field -// entirely rather than emitting "[]". -func splitScopes(s string) []string { - if s == "" { - return nil +// boolStr renders a boolean as "true" / "false" for Invocation.Details. +// Kept as a small helper rather than inlining so both the deny and +// modify branches use the same string form and abctl's filter +// matching is predictable. +func boolStr(b bool) string { + if b { + return "true" } - return strings.Fields(s) + return "false" } func (p *TokenExchange) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { diff --git a/authbridge/authlib/session/store.go b/authbridge/authlib/session/store.go index bcc9fcd2c..d81d610fd 100644 --- a/authbridge/authlib/session/store.go +++ b/authbridge/authlib/session/store.go @@ -421,9 +421,6 @@ func logAppended(sessionID string, e *pipeline.SessionEvent) { if e.Host != "" { attrs = append(attrs, "host", e.Host) } - if e.TargetAudience != "" { - attrs = append(attrs, "aud", e.TargetAudience) - } if e.StatusCode != 0 { attrs = append(attrs, "status", e.StatusCode) } diff --git a/authbridge/authlib/sessionapi/server_test.go b/authbridge/authlib/sessionapi/server_test.go index 2d73ea9f6..e40276116 100644 --- a/authbridge/authlib/sessionapi/server_test.go +++ b/authbridge/authlib/sessionapi/server_test.go @@ -446,11 +446,13 @@ func TestHandleGet_SerializesInvocations(t *testing.T) { Phase: pipeline.SessionDenied, Invocations: &pipeline.Invocations{ Inbound: []pipeline.Invocation{{ - Plugin: "jwt-validation", - Action: pipeline.ActionDeny, - Reason: "jwt_failed", - ExpectedIssuer: "http://issuer.example", - ExpectedAudience: "agent-aud", + Plugin: "jwt-validation", + Action: pipeline.ActionDeny, + Reason: "jwt_failed", + Details: map[string]string{ + "expected_issuer": "http://issuer.example", + "expected_audience": "agent-aud", + }, }}, }, }) diff --git a/authbridge/cmd/abctl/tui/events_pane.go b/authbridge/cmd/abctl/tui/events_pane.go index 42ab7f443..f97586ef2 100644 --- a/authbridge/cmd/abctl/tui/events_pane.go +++ b/authbridge/cmd/abctl/tui/events_pane.go @@ -431,12 +431,17 @@ func matchInvocationRow(r invocationRow, q string) bool { } e := r.event - hay := []string{e.Host, e.TargetAudience, eventMethod(*e)} + hay := []string{e.Host, eventMethod(*e)} if r.inv != nil { hay = append(hay, - r.inv.Plugin, string(r.inv.Action), r.inv.Reason, r.inv.Path, - r.inv.ExpectedIssuer, r.inv.ExpectedAudience, r.inv.TokenSubject, - r.inv.RouteHost, r.inv.TargetAudience) + r.inv.Plugin, string(r.inv.Action), r.inv.Reason, r.inv.Path) + // Plugin-specific diagnostic context — iterate keys + values so + // filter text matches on e.g. "target_audience" / the target + // audience value without the UI having to know which keys + // each plugin writes. + for k, v := range r.inv.Details { + hay = append(hay, k, v) + } } if e.Identity != nil { hay = append(hay, e.Identity.Subject, e.Identity.ClientID) diff --git a/authbridge/cmd/authbridge/listener/extproc/server_test.go b/authbridge/cmd/authbridge/listener/extproc/server_test.go index 1fd4d10e8..512d55a3c 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server_test.go +++ b/authbridge/cmd/authbridge/listener/extproc/server_test.go @@ -1264,12 +1264,14 @@ func TestRecordInboundReject_EmitsDeniedPhase(t *testing.T) { Extensions: pipeline.Extensions{ Invocations: &pipeline.Invocations{ Inbound: []pipeline.Invocation{{ - Plugin: "jwt-validation", - Phase: pipeline.InvocationPhaseRequest, - Action: pipeline.ActionDeny, - Reason: "jwt_failed", - ExpectedIssuer: "http://issuer.example", - ExpectedAudience: "agent-aud", + Plugin: "jwt-validation", + Phase: pipeline.InvocationPhaseRequest, + Action: pipeline.ActionDeny, + Reason: "jwt_failed", + Details: map[string]string{ + "expected_issuer": "http://issuer.example", + "expected_audience": "agent-aud", + }, }}, }, }, From 3729a52cf959e4add1b2a07d9591c255367c1a94 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sun, 10 May 2026 09:10:07 -0400 Subject: [PATCH 4/6] refactor(authlib): Move single-owner packages into their owning plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four authlib packages had exactly one in-tree importer each. Moving them under the plugin that owns them makes the authlib/ layout honest about what's shared vs plugin-local, and models the conventional "plugin owns its internals" pattern for out-of-tree plugins. Moves (git mv, history-preserving): authlib/validation → authlib/plugins/jwtvalidation/validation authlib/exchange → authlib/plugins/tokenexchange/exchange authlib/cache → authlib/plugins/tokenexchange/cache authlib/spiffe → authlib/plugins/tokenexchange/spiffe Each plugin also converts from a flat file in authlib/plugins/ to its own directory (own package): authlib/plugins/jwtvalidation.go → authlib/plugins/jwtvalidation/plugin.go authlib/plugins/jwtvalidation_identity.go → .../jwtvalidation/identity.go authlib/plugins/tokenexchange.go → authlib/plugins/tokenexchange/plugin.go Plugin packages now import plugins.RegisterPlugin / plugins.StatsSource from the parent plugins package explicitly. main.go adds side-effect imports for the two moved plugins so their init() runs — same pattern an external plugin would use. Kept as shared building blocks: authlib/bypass — any future inbound gate plugin wants path bypass (healthz, readyz, .well-known). Clear multi-plugin future, even though jwt-validation is the only current importer. authlib/routing — two in-tree users (jwt-validation, token-exchange). External token-broker (PR #391) rolled its own router because authlib/routing.ResolvedRoute has token-exchange-shaped fields — separate cleanup deferred as a followup (make the routing mechanism reusable without the schema). authlib/auth — composition layer both gate plugins wrap via auth.Auth. Lingers in authlib/ for now even though it's effectively plugin-internal; natural refactor candidate once a third gate plugin emerges or when auth's Stats type is replaced by a plugin-generic metrics interface. Test-file reorganization: authlib/plugins/plugins_test.go split into three: plugins_test.go (package plugins_test): YAML loader, Stats, Build — cross-plugin integration tests, black-box imports of jwtvalidation and tokenexchange via side-effect. plugins/jwtvalidation/plugin_test.go (package jwtvalidation): all JWT Configure/Ready/OnRequest tests + invokeOnRequest helper + mockJWTVerifier + newTestJWTValidation. plugins/tokenexchange/plugin_test.go (package tokenexchange): all TokenExchange tests + invokeOnRequest helper. Tests that accessed unexported plugin fields (p.cfg, p.inner, p.audienceDeriver) naturally moved into the plugin's own package where those fields are in-package. Invariant checks: $ grep -rn 'authlib/(validation|exchange|cache|spiffe)' authbridge/authlib/pipeline/ (empty — pipeline/ still free of plugin-specific imports) $ ls authbridge/authlib/ | grep -E '^(validation|exchange|cache|spiffe)$' (empty — old package directories removed) Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/auth/auth.go | 6 +- authbridge/authlib/auth/auth_test.go | 6 +- authbridge/authlib/auth/result.go | 2 +- .../identity.go} | 4 +- .../plugin.go} | 9 +- .../plugins/jwtvalidation/plugin_test.go | 320 +++++++ .../jwtvalidation}/validation/jwks.go | 0 .../jwtvalidation}/validation/jwks_test.go | 0 .../jwtvalidation}/validation/lazy.go | 0 .../jwtvalidation}/validation/verifier.go | 0 authbridge/authlib/plugins/plugins_test.go | 819 +----------------- .../tokenexchange}/cache/cache.go | 0 .../tokenexchange}/cache/cache_test.go | 0 .../tokenexchange}/exchange/auth.go | 0 .../tokenexchange}/exchange/client.go | 0 .../tokenexchange}/exchange/client_test.go | 0 .../tokenexchange}/exchange/errors.go | 0 .../plugin.go} | 13 +- .../plugins/tokenexchange/plugin_test.go | 329 +++++++ .../tokenexchange}/spiffe/file.go | 0 .../tokenexchange}/spiffe/file_test.go | 0 .../tokenexchange}/spiffe/source.go | 0 .../listener/extauthz/server_test.go | 6 +- .../listener/extproc/server_test.go | 6 +- .../listener/forwardproxy/server_test.go | 6 +- .../listener/reverseproxy/server_test.go | 2 +- authbridge/cmd/authbridge/main.go | 8 + 27 files changed, 725 insertions(+), 811 deletions(-) rename authbridge/authlib/plugins/{jwtvalidation_identity.go => jwtvalidation/identity.go} (92%) rename authbridge/authlib/plugins/{jwtvalidation.go => jwtvalidation/plugin.go} (97%) create mode 100644 authbridge/authlib/plugins/jwtvalidation/plugin_test.go rename authbridge/authlib/{ => plugins/jwtvalidation}/validation/jwks.go (100%) rename authbridge/authlib/{ => plugins/jwtvalidation}/validation/jwks_test.go (100%) rename authbridge/authlib/{ => plugins/jwtvalidation}/validation/lazy.go (100%) rename authbridge/authlib/{ => plugins/jwtvalidation}/validation/verifier.go (100%) rename authbridge/authlib/{ => plugins/tokenexchange}/cache/cache.go (100%) rename authbridge/authlib/{ => plugins/tokenexchange}/cache/cache_test.go (100%) rename authbridge/authlib/{ => plugins/tokenexchange}/exchange/auth.go (100%) rename authbridge/authlib/{ => plugins/tokenexchange}/exchange/client.go (100%) rename authbridge/authlib/{ => plugins/tokenexchange}/exchange/client_test.go (100%) rename authbridge/authlib/{ => plugins/tokenexchange}/exchange/errors.go (100%) rename authbridge/authlib/plugins/{tokenexchange.go => tokenexchange/plugin.go} (97%) create mode 100644 authbridge/authlib/plugins/tokenexchange/plugin_test.go rename authbridge/authlib/{ => plugins/tokenexchange}/spiffe/file.go (100%) rename authbridge/authlib/{ => plugins/tokenexchange}/spiffe/file_test.go (100%) rename authbridge/authlib/{ => plugins/tokenexchange}/spiffe/source.go (100%) diff --git a/authbridge/authlib/auth/auth.go b/authbridge/authlib/auth/auth.go index 23e4ba5b5..49fcc45a5 100644 --- a/authbridge/authlib/auth/auth.go +++ b/authbridge/authlib/auth/auth.go @@ -11,10 +11,10 @@ import ( "time" "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/plugins/tokenexchange/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) // IdentityConfig holds the agent's identity for audience validation and token exchange. diff --git a/authbridge/authlib/auth/auth_test.go b/authbridge/authlib/auth/auth_test.go index c48ee13e0..134e737c0 100644 --- a/authbridge/authlib/auth/auth_test.go +++ b/authbridge/authlib/auth/auth_test.go @@ -11,10 +11,10 @@ import ( "time" "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/plugins/tokenexchange/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) // mockVerifier captures the audience arg and returns configured claims/error. diff --git a/authbridge/authlib/auth/result.go b/authbridge/authlib/auth/result.go index 77c77865a..673351572 100644 --- a/authbridge/authlib/auth/result.go +++ b/authbridge/authlib/auth/result.go @@ -3,7 +3,7 @@ package auth import ( - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) // Inbound actions. diff --git a/authbridge/authlib/plugins/jwtvalidation_identity.go b/authbridge/authlib/plugins/jwtvalidation/identity.go similarity index 92% rename from authbridge/authlib/plugins/jwtvalidation_identity.go rename to authbridge/authlib/plugins/jwtvalidation/identity.go index f19fe1cf8..da46ef326 100644 --- a/authbridge/authlib/plugins/jwtvalidation_identity.go +++ b/authbridge/authlib/plugins/jwtvalidation/identity.go @@ -1,6 +1,6 @@ -package plugins +package jwtvalidation -import "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" +import "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" // claimsIdentity adapts a *validation.Claims to the pipeline.Identity // interface that Context exposes. Kept in the plugin so the pipeline diff --git a/authbridge/authlib/plugins/jwtvalidation.go b/authbridge/authlib/plugins/jwtvalidation/plugin.go similarity index 97% rename from authbridge/authlib/plugins/jwtvalidation.go rename to authbridge/authlib/plugins/jwtvalidation/plugin.go index 54abf99ef..08740aafa 100644 --- a/authbridge/authlib/plugins/jwtvalidation.go +++ b/authbridge/authlib/plugins/jwtvalidation/plugin.go @@ -1,4 +1,4 @@ -package plugins +package jwtvalidation import ( "bytes" @@ -14,8 +14,9 @@ import ( "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/plugins" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" ) // jwtValidationConfig is the plugin's local config schema. See @@ -161,7 +162,7 @@ type JWTValidation struct { func NewJWTValidation() *JWTValidation { return &JWTValidation{} } func init() { - RegisterPlugin("jwt-validation", func() pipeline.Plugin { return NewJWTValidation() }) + plugins.RegisterPlugin("jwt-validation", func() pipeline.Plugin { return NewJWTValidation() }) } func (p *JWTValidation) Name() string { return "jwt-validation" } @@ -387,5 +388,5 @@ var ( _ pipeline.Initializer = (*JWTValidation)(nil) _ pipeline.Shutdowner = (*JWTValidation)(nil) _ pipeline.Readier = (*JWTValidation)(nil) - _ StatsSource = (*JWTValidation)(nil) + _ plugins.StatsSource = (*JWTValidation)(nil) ) diff --git a/authbridge/authlib/plugins/jwtvalidation/plugin_test.go b/authbridge/authlib/plugins/jwtvalidation/plugin_test.go new file mode 100644 index 000000000..ef75e5c8f --- /dev/null +++ b/authbridge/authlib/plugins/jwtvalidation/plugin_test.go @@ -0,0 +1,320 @@ +package jwtvalidation + +import ( + "context" + "net/http" + "os" + "path/filepath" + "testing" + + "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/jwtvalidation/validation" +) + +// invokeOnRequest mirrors what Pipeline.Run does around each plugin +// dispatch: set the current-plugin / current-phase attribution fields +// on pctx so pctx.Record / Allow / Skip / Observe / Modify fill in +// Plugin and Phase correctly. +func invokeOnRequest(p pipeline.Plugin, pctx *pipeline.Context) pipeline.Action { + pctx.SetCurrentPlugin(p.Name(), pipeline.InvocationPhaseRequest) + defer pctx.ClearCurrentPlugin() + return p.OnRequest(context.Background(), pctx) +} + +// --- Configure --- + +func TestJWTValidation_Configure_MissingIssuer(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{}`)); err == nil { + t.Fatal("expected error for missing issuer") + } +} + +func TestJWTValidation_Configure_UnknownField(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://ex","audience":"a","not_a_field":"x"}`)); err == nil { + t.Fatal("expected error for unknown field; DisallowUnknownFields should reject") + } +} + +func TestJWTValidation_Configure_PerHost(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://ex","audience_mode":"per-host"}`)); err != nil { + t.Fatalf("per-host mode should not require audience: %v", err) + } + if p.audienceDeriver == nil { + t.Error("per-host mode should set audienceDeriver") + } +} + +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 p.cfg.AudienceFile != "/shared/client-id.txt" { + t.Errorf("AudienceFile = %q, want /shared/client-id.txt", p.cfg.AudienceFile) + } +} + +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") + } +} + +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) + } + if p.cfg.AudienceFile != "" { + t.Errorf("AudienceFile = %q, want empty (inline audience should suppress default)", p.cfg.AudienceFile) + } +} + +func TestJWTValidation_Configure_DefaultsJWKSFromIssuer(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(`{"issuer":"http://keycloak/realms/kagenti","audience":"a"}`)); err != nil { + t.Fatalf("Configure: %v", err) + } + if got, want := p.cfg.JWKSURL, "http://keycloak/realms/kagenti/protocol/openid-connect/certs"; got != want { + t.Errorf("JWKSURL = %q, want %q", got, want) + } + if p.inner == nil { + t.Fatal("Configure produced no inner auth handler") + } +} + +func TestJWTValidation_Configure_DerivesJWKSFromInternalKeycloakURL(t *testing.T) { + p := NewJWTValidation() + raw := []byte(`{ + "issuer": "http://keycloak.localtest.me:8080/realms/kagenti", + "keycloak_url": "http://keycloak-service.keycloak.svc:8080", + "keycloak_realm": "kagenti", + "audience": "a" + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + want := "http://keycloak-service.keycloak.svc:8080/realms/kagenti/protocol/openid-connect/certs" + if got := p.cfg.JWKSURL; got != want { + t.Errorf("JWKSURL = %q, want %q (internal URL from keycloak_url+realm, not issuer)", got, want) + } +} + +func TestJWTValidation_Configure_ExplicitJWKSURLWins(t *testing.T) { + p := NewJWTValidation() + raw := []byte(`{ + "issuer": "http://keycloak.public:8080/realms/kagenti", + "jwks_url": "http://custom-jwks-proxy.example/keys", + "keycloak_url": "http://keycloak-internal:8080", + "keycloak_realm": "kagenti", + "audience": "a" + }`) + if err := p.Configure(raw); err != nil { + t.Fatalf("Configure: %v", err) + } + if got, want := p.cfg.JWKSURL, "http://custom-jwks-proxy.example/keys"; got != want { + t.Errorf("JWKSURL = %q, want %q", got, want) + } +} + +func TestJWTValidation_Configure_PartialKeycloakConfigFallsThroughToIssuer(t *testing.T) { + cases := []struct{ name, raw string }{ + {"keycloak_url without realm", `{"issuer":"http://keycloak/realms/kagenti","keycloak_url":"http://internal:8080","audience":"a"}`}, + {"keycloak_realm without url", `{"issuer":"http://keycloak/realms/kagenti","keycloak_realm":"kagenti","audience":"a"}`}, + } + want := "http://keycloak/realms/kagenti/protocol/openid-connect/certs" + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p := NewJWTValidation() + if err := p.Configure([]byte(tc.raw)); err != nil { + t.Fatalf("Configure: %v", err) + } + if got := p.cfg.JWKSURL; got != want { + t.Errorf("JWKSURL = %q, want %q", got, want) + } + }) + } +} + +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() + if err := p.Configure([]byte(`{"issuer":"http://ex","audience_file":"` + f + `"}`)); err != nil { + t.Fatalf("Configure: %v", err) + } + if !p.inner.Ready() { + t.Error("expected inner.Ready() == true after synchronous audience load") + } +} + +// --- Ready --- + +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") + } +} + +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") + } +} + +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") + } +} + +// --- OnRequest --- + +func TestJWTValidation_OnRequest_NotConfigured(t *testing.T) { + p := NewJWTValidation() + action := invokeOnRequest(p, &pipeline.Context{Headers: http.Header{}}) + if action.Type != pipeline.Reject { + t.Errorf("got %v, want Reject for unconfigured plugin", action.Type) + } +} + +// mockJWTVerifier lets the tests below dictate what the inner validator +// returns without standing up an httptest JWKS server. +type mockJWTVerifier struct { + claims *validation.Claims + err error +} + +func (m *mockJWTVerifier) Verify(_ context.Context, _, _ string) (*validation.Claims, error) { + return m.claims, m.err +} + +// newTestJWTValidation constructs a JWTValidation plugin without calling +// Configure — skips file I/O and lets each test wire a tailored inner. +func newTestJWTValidation(t *testing.T, issuer string, inner *auth.Auth) *JWTValidation { + t.Helper() + p := NewJWTValidation() + p.cfg.Issuer = issuer + p.inner = inner + return p +} + +func TestJWTValidation_OnRequest_PopulatesAuth_Bypass(t *testing.T) { + matcher, _ := bypass.NewMatcher(bypass.DefaultPatterns) + inner := auth.New(auth.Config{ + Bypass: matcher, + Verifier: &mockJWTVerifier{claims: &validation.Claims{Subject: "s"}}, + Identity: auth.IdentityConfig{Audience: "agent-aud"}, + }) + p := newTestJWTValidation(t, "http://issuer", inner) + + pctx := &pipeline.Context{Headers: http.Header{}, Path: "/healthz"} + action := invokeOnRequest(p, pctx) + if action.Type != pipeline.Continue { + t.Fatalf("bypass should Continue, got %v", action.Type) + } + if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Inbound) != 1 { + t.Fatalf("expected one Invocations.Inbound entry, got %+v", pctx.Extensions.Invocations) + } + got := pctx.Extensions.Invocations.Inbound[0] + if got.Plugin != "jwt-validation" { + t.Errorf("Plugin = %q, want jwt-validation", got.Plugin) + } + if got.Action != pipeline.ActionSkip || got.Reason != "path_bypass" { + t.Errorf("got Action=%q Reason=%q, want skip/path_bypass", got.Action, got.Reason) + } +} + +func TestJWTValidation_OnRequest_PopulatesAuth_Deny_NoHeader(t *testing.T) { + inner := auth.New(auth.Config{ + Verifier: &mockJWTVerifier{}, + Identity: auth.IdentityConfig{Audience: "agent-aud"}, + }) + p := newTestJWTValidation(t, "http://issuer.example", inner) + + pctx := &pipeline.Context{Headers: http.Header{}, Path: "/api/call"} + action := invokeOnRequest(p, pctx) + if action.Type != pipeline.Reject { + t.Fatalf("expected Reject on missing auth header, got %v", action.Type) + } + if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Inbound) != 1 { + t.Fatalf("expected one entry, got %+v", pctx.Extensions.Invocations) + } + got := pctx.Extensions.Invocations.Inbound[0] + if got.Action != pipeline.ActionDeny { + t.Errorf("Action = %q, want deny", got.Action) + } + if got.Reason != "no_header" { + t.Errorf("Reason = %q, want no_header", got.Reason) + } + if got.Details["expected_issuer"] != "http://issuer.example" { + t.Errorf("expected_issuer = %q, want http://issuer.example", got.Details["expected_issuer"]) + } +} + +func TestJWTValidation_OnRequest_PopulatesAuth_Allow(t *testing.T) { + claims := &validation.Claims{ + Subject: "alice", + Issuer: "http://issuer.example", + Audience: []string{"agent-aud"}, + ClientID: "caller", + Scopes: []string{"openid", "write"}, + } + inner := auth.New(auth.Config{ + Verifier: &mockJWTVerifier{claims: claims}, + Identity: auth.IdentityConfig{Audience: "agent-aud"}, + }) + p := newTestJWTValidation(t, "http://issuer.example", inner) + + pctx := &pipeline.Context{Headers: http.Header{}, Path: "/api/call"} + pctx.Headers.Set("Authorization", "Bearer tok") + action := invokeOnRequest(p, pctx) + if action.Type != pipeline.Continue { + t.Fatalf("expected Continue, got %v (violation=%+v)", action.Type, action.Violation) + } + if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Inbound) != 1 { + t.Fatalf("expected one entry, got %+v", pctx.Extensions.Invocations) + } + got := pctx.Extensions.Invocations.Inbound[0] + if got.Action != pipeline.ActionAllow || got.Reason != "authorized" { + t.Errorf("got Action=%q Reason=%q, want allow/authorized", got.Action, got.Reason) + } + if got.Details["token_subject"] != "alice" { + t.Errorf("token_subject = %q, want alice", got.Details["token_subject"]) + } + if got.Details["token_scopes"] != "openid write" { + t.Errorf("token_scopes = %q, want \"openid write\"", got.Details["token_scopes"]) + } + if got.Details["token_audience"] != "agent-aud" { + t.Errorf("token_audience = %q, want agent-aud", got.Details["token_audience"]) + } +} diff --git a/authbridge/authlib/validation/jwks.go b/authbridge/authlib/plugins/jwtvalidation/validation/jwks.go similarity index 100% rename from authbridge/authlib/validation/jwks.go rename to authbridge/authlib/plugins/jwtvalidation/validation/jwks.go diff --git a/authbridge/authlib/validation/jwks_test.go b/authbridge/authlib/plugins/jwtvalidation/validation/jwks_test.go similarity index 100% rename from authbridge/authlib/validation/jwks_test.go rename to authbridge/authlib/plugins/jwtvalidation/validation/jwks_test.go diff --git a/authbridge/authlib/validation/lazy.go b/authbridge/authlib/plugins/jwtvalidation/validation/lazy.go similarity index 100% rename from authbridge/authlib/validation/lazy.go rename to authbridge/authlib/plugins/jwtvalidation/validation/lazy.go diff --git a/authbridge/authlib/validation/verifier.go b/authbridge/authlib/plugins/jwtvalidation/validation/verifier.go similarity index 100% rename from authbridge/authlib/validation/verifier.go rename to authbridge/authlib/plugins/jwtvalidation/validation/verifier.go diff --git a/authbridge/authlib/plugins/plugins_test.go b/authbridge/authlib/plugins/plugins_test.go index 2051d7316..3a4cf7cb1 100644 --- a/authbridge/authlib/plugins/plugins_test.go +++ b/authbridge/authlib/plugins/plugins_test.go @@ -1,53 +1,28 @@ -package plugins +package plugins_test import ( "context" - "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/bypass" "github.com/kagenti/kagenti-extensions/authbridge/authlib/config" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" + // Side-effect imports register the bundled plugins. Same pattern + // main.go uses — ensures Build("jwt-validation") / Build("token-exchange") + // resolve during tests. + jwtvalidation "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation" + tokenexchange "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange" ) -// invokeOnRequest mirrors what Pipeline.Run does around each plugin -// dispatch: set the current-plugin / current-phase attribution fields -// on pctx so pctx.Record / Allow / Skip / Observe / Modify fill in -// Plugin and Phase correctly. Tests that call plugin.OnRequest directly -// (bypassing Pipeline.Run) need this wrapper to exercise the same code -// path as production. Without it, Invocations would land with empty -// Plugin and Phase fields. -func invokeOnRequest(p pipeline.Plugin, pctx *pipeline.Context) pipeline.Action { - pctx.SetCurrentPlugin(p.Name(), pipeline.InvocationPhaseRequest) - defer pctx.ClearCurrentPlugin() - return p.OnRequest(context.Background(), pctx) -} - -// invokeOnResponse is the response-phase twin of invokeOnRequest. -func invokeOnResponse(p pipeline.Plugin, pctx *pipeline.Context) pipeline.Action { - pctx.SetCurrentPlugin(p.Name(), pipeline.InvocationPhaseResponse) - defer pctx.ClearCurrentPlugin() - return p.OnResponse(context.Background(), pctx) -} - // 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. +// config consumed by the combined sidecar image parses and produces +// working pipelines. A future rename of any plugin default constant +// that silently breaks the shipped image fails this test first. 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) @@ -58,7 +33,7 @@ func TestAuthbridgeCombinedYAML_Loads(t *testing.T) { "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 + "TOKEN_URL": "", } for k, v := range envs { t.Setenv(k, v) @@ -74,759 +49,49 @@ func TestAuthbridgeCombinedYAML_Loads(t *testing.T) { 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 { + if _, err := plugins.Build(cfg.Pipeline.Inbound.Plugins); err != nil { t.Errorf("Build inbound: %v", err) } - if _, err := Build(cfg.Pipeline.Outbound.Plugins); err != nil { + if _, err := plugins.Build(cfg.Pipeline.Outbound.Plugins); err != nil { t.Errorf("Build outbound: %v", 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") - } -} - -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") - } -} - -// 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. - -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) - } - if p.audienceDeriver == nil { - t.Error("per-host mode should set audienceDeriver") - } -} - -// 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 p.cfg.AudienceFile != "/shared/client-id.txt" { - t.Errorf("AudienceFile = %q, want /shared/client-id.txt", p.cfg.AudienceFile) - } -} - -// 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") - } -} - -// 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) - } - if p.cfg.AudienceFile != "" { - t.Errorf("AudienceFile = %q, want empty (inline audience should suppress default)", p.cfg.AudienceFile) - } -} - -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) - } - // Issuer-derived fallback; verify the exact URL so a future refactor - // of the priority chain can't silently fall through to a 404 path. - if got, want := p.cfg.JWKSURL, "http://keycloak/realms/kagenti/protocol/openid-connect/certs"; got != want { - t.Errorf("JWKSURL = %q, want %q", got, want) - } - if p.inner == nil { - t.Fatal("Configure produced no inner auth handler") - } -} - -// Split-horizon case: operator supplies a public `issuer` (for iss-claim -// matching) and internal `keycloak_url` + `keycloak_realm` (for reachable -// JWKS fetching). The derivation prefers the internal URL — deriving from -// issuer here would send the request into the public hostname which, in -// Kagenti's Kind/OpenShift setups, resolves to 127.0.0.1 and fails -// "connection refused" (see authbridge CLAUDE.md gotcha #2). -func TestJWTValidation_Configure_DerivesJWKSFromInternalKeycloakURL(t *testing.T) { - p := NewJWTValidation() - raw := []byte(`{ - "issuer": "http://keycloak.localtest.me:8080/realms/kagenti", - "keycloak_url": "http://keycloak-service.keycloak.svc:8080", - "keycloak_realm": "kagenti", - "audience": "a" - }`) - if err := p.Configure(raw); err != nil { - t.Fatalf("Configure: %v", err) - } - want := "http://keycloak-service.keycloak.svc:8080/realms/kagenti/protocol/openid-connect/certs" - if got := p.cfg.JWKSURL; got != want { - t.Errorf("JWKSURL = %q, want %q (internal URL from keycloak_url+realm, not issuer)", got, want) - } -} - -// Explicit jwks_url beats every derivation source. Operators who know -// exactly which endpoint to hit (e.g. a custom JWKS proxy) must not have -// it silently overwritten by the keycloak_url/realm derivation. -func TestJWTValidation_Configure_ExplicitJWKSURLWins(t *testing.T) { - p := NewJWTValidation() - raw := []byte(`{ - "issuer": "http://keycloak.public:8080/realms/kagenti", - "jwks_url": "http://custom-jwks-proxy.example/keys", - "keycloak_url": "http://keycloak-internal:8080", - "keycloak_realm": "kagenti", - "audience": "a" - }`) - if err := p.Configure(raw); err != nil { - t.Fatalf("Configure: %v", err) - } - if got, want := p.cfg.JWKSURL, "http://custom-jwks-proxy.example/keys"; got != want { - t.Errorf("JWKSURL = %q, want %q (explicit jwks_url must win over derivations)", got, want) - } -} - -// Only one of keycloak_url/keycloak_realm is supplied — derivation can't -// complete (Keycloak path needs both). Fall through to issuer-derivation -// rather than crashing or leaving JWKSURL empty. -// -// Covered both directions to catch a future refactor that ANDs the check -// differently (e.g. a short-circuit that treats empty realm as default -// would silently build a bogus URL). -func TestJWTValidation_Configure_PartialKeycloakConfigFallsThroughToIssuer(t *testing.T) { - cases := []struct { - name, raw string - }{ - { - name: "keycloak_url without realm", - raw: `{ - "issuer": "http://keycloak/realms/kagenti", - "keycloak_url": "http://internal:8080", - "audience": "a" - }`, - }, - { - name: "keycloak_realm without url", - raw: `{ - "issuer": "http://keycloak/realms/kagenti", - "keycloak_realm": "kagenti", - "audience": "a" - }`, - }, - } - want := "http://keycloak/realms/kagenti/protocol/openid-connect/certs" - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - p := NewJWTValidation() - if err := p.Configure([]byte(tc.raw)); err != nil { - t.Fatalf("Configure: %v", err) - } - if got := p.cfg.JWKSURL; got != want { - t.Errorf("JWKSURL = %q, want %q (partial keycloak_* must not short-circuit issuer-derivation)", got, want) - } - }) - } -} - -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") - } -} - -// --- 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") - } -} - -// 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 := invokeOnRequest(p, &pipeline.Context{Headers: http.Header{}}) - if action.Type != pipeline.Reject { - t.Errorf("got %v, want Reject for unconfigured plugin", action.Type) - } -} - -// --- JWTValidation: Auth extension population --- -// -// These tests verify jwt-validation surfaces its decision on -// pctx.Extensions.Invocations.Inbound so the listener can record a -// SessionEvent reflecting allow/deny/bypass. Plumbed through a -// mockVerifier injected into p.inner (instead of spinning up a real -// JWKS server) — keeps the test focused on plugin behavior, not crypto. - -// mockJWTVerifier lets the tests below dictate what the inner validator -// returns without standing up an httptest JWKS server. It implements the -// validation.Verifier interface. -type mockJWTVerifier struct { - claims *validation.Claims - err error -} - -func (m *mockJWTVerifier) Verify(_ context.Context, _, _ string) (*validation.Claims, error) { - return m.claims, m.err -} - -// newTestJWTValidation constructs a JWTValidation plugin without calling -// Configure — skips file I/O (audience_file polling, bypass pattern -// compile via config) and lets each test wire a tailored inner auth.Auth. -func newTestJWTValidation(t *testing.T, issuer string, inner *auth.Auth) *JWTValidation { - t.Helper() - p := NewJWTValidation() - p.cfg.Issuer = issuer - p.inner = inner - return p -} - -func TestJWTValidation_OnRequest_PopulatesAuth_Bypass(t *testing.T) { - matcher, _ := bypass.NewMatcher(bypass.DefaultPatterns) - inner := auth.New(auth.Config{ - Bypass: matcher, - Verifier: &mockJWTVerifier{claims: &validation.Claims{Subject: "s"}}, - Identity: auth.IdentityConfig{Audience: "agent-aud"}, - }) - p := newTestJWTValidation(t, "http://issuer", inner) - - pctx := &pipeline.Context{Headers: http.Header{}, Path: "/healthz"} - action := invokeOnRequest(p, pctx) - if action.Type != pipeline.Continue { - t.Fatalf("bypass should Continue, got %v", action.Type) - } - if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Inbound) != 1 { - t.Fatalf("expected one Auth.Inbound entry, got %+v", pctx.Extensions.Invocations) - } - got := pctx.Extensions.Invocations.Inbound[0] - if got.Plugin != "jwt-validation" { - t.Errorf("Plugin = %q, want jwt-validation", got.Plugin) - } - if got.Action != pipeline.ActionSkip || got.Reason != "path_bypass" { - t.Errorf("got Action=%q Reason=%q, want skip/path_bypass", got.Action, got.Reason) - } -} - -func TestJWTValidation_OnRequest_PopulatesAuth_Deny_NoHeader(t *testing.T) { - inner := auth.New(auth.Config{ - Verifier: &mockJWTVerifier{}, - Identity: auth.IdentityConfig{Audience: "agent-aud"}, - }) - p := newTestJWTValidation(t, "http://issuer.example", inner) - - pctx := &pipeline.Context{Headers: http.Header{}, Path: "/api/call"} - action := invokeOnRequest(p, pctx) - if action.Type != pipeline.Reject { - t.Fatalf("expected Reject on missing auth header, got %v", action.Type) - } - if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Inbound) != 1 { - t.Fatalf("expected one Auth.Inbound entry, got %+v", pctx.Extensions.Invocations) - } - got := pctx.Extensions.Invocations.Inbound[0] - if got.Action != pipeline.ActionDeny { - t.Errorf("Action = %q, want deny", got.Action) - } - // Reason comes from InboundDenialReason.String() so consumers can - // filter on a machine-stable code without parsing English. - if got.Reason != "no_header" { - t.Errorf("Reason = %q, want no_header", got.Reason) - } - if got.Details["expected_issuer"] != "http://issuer.example" { - t.Errorf("expected_issuer = %q, want http://issuer.example", got.Details["expected_issuer"]) - } -} - -func TestJWTValidation_OnRequest_PopulatesAuth_Allow(t *testing.T) { - claims := &validation.Claims{ - Subject: "alice", - Issuer: "http://issuer.example", - Audience: []string{"agent-aud"}, - ClientID: "caller", - Scopes: []string{"openid", "write"}, - } - inner := auth.New(auth.Config{ - Verifier: &mockJWTVerifier{claims: claims}, - Identity: auth.IdentityConfig{Audience: "agent-aud"}, - }) - p := newTestJWTValidation(t, "http://issuer.example", inner) - - pctx := &pipeline.Context{Headers: http.Header{}, Path: "/api/call"} - pctx.Headers.Set("Authorization", "Bearer tok") - action := invokeOnRequest(p, pctx) - if action.Type != pipeline.Continue { - t.Fatalf("expected Continue, got %v (violation=%+v)", action.Type, action.Violation) - } - if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Inbound) != 1 { - t.Fatalf("expected one Auth.Inbound entry, got %+v", pctx.Extensions.Invocations) - } - got := pctx.Extensions.Invocations.Inbound[0] - if got.Action != pipeline.ActionAllow || got.Reason != "authorized" { - t.Errorf("got Action=%q Reason=%q, want allow/authorized", got.Action, got.Reason) - } - if got.Details["token_subject"] != "alice" { - t.Errorf("token_subject = %q, want alice", got.Details["token_subject"]) - } - if got.Details["token_scopes"] != "openid write" { - t.Errorf("token_scopes = %q, want \"openid write\"", got.Details["token_scopes"]) - } - if got.Details["token_audience"] != "agent-aud" { - t.Errorf("token_audience = %q, want agent-aud", got.Details["token_audience"]) - } -} - -// --- 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 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) - } -} - -// 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) - } - 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.JWTSVIDPath != "/opt/jwt_svid.token" { - t.Errorf("JWTSVIDPath = %q, want /opt/jwt_svid.token", p.cfg.Identity.JWTSVIDPath) - } -} - -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) - } -} - -// 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 := invokeOnRequest(p, pctx) - if action.Type != pipeline.Continue { - 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") - } - // Passthrough populates Auth.Outbound with Action="passthrough" — - // symmetric with jwt-validation's bypass recording so operators can - // see every outbound host the pod talks to in the session stream. - // RouteHost carries the target so they can spot unexpected egress - // without hunting through slog lines. - if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Outbound) != 1 { - t.Fatalf("expected one Auth.Outbound entry, got %+v", pctx.Extensions.Invocations) - } - ob := pctx.Extensions.Invocations.Outbound[0] - if ob.Plugin != "token-exchange" || ob.Action != pipeline.ActionSkip { - t.Errorf("entry = (%q, %q), want (token-exchange, skip)", ob.Plugin, ob.Action) - } - if ob.Details["route_host"] != "some-host" { - t.Errorf("route_host = %q, want some-host", ob.Details["route_host"]) - } - if ob.Details["route_matched"] != "false" { - t.Errorf("route_matched = %q, want false on default-policy passthrough", ob.Details["route_matched"]) - } -} - -func TestTokenExchange_ExchangeSuccess(t *testing.T) { - exchangeSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ - "access_token": "new-token", - "token_type": "Bearer", - "expires_in": 300, - }) - })) - defer exchangeSrv.Close() - - 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 := invokeOnRequest(p, pctx) - if action.Type != pipeline.Continue { - t.Fatalf("got %v, want Continue", action.Type) - } - if pctx.Headers.Get("Authorization") != "Bearer new-token" { - t.Errorf("token = %q, want Bearer new-token", pctx.Headers.Get("Authorization")) - } - // Auth extension must surface the exchange action so it flows into - // SessionEvent.Auth.Outbound once the listener records. Empty route - // (TargetAudience, RequestedScopes) is OK here — this test doesn't - // configure routes, it uses default_policy=exchange. - if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Outbound) != 1 { - t.Fatalf("expected one Auth.Outbound entry, got %+v", pctx.Extensions.Invocations) - } - got := pctx.Extensions.Invocations.Outbound[0] - if got.Plugin != "token-exchange" || got.Action != pipeline.ActionModify { - t.Errorf("got Plugin=%q Action=%q, want token-exchange/modify", got.Plugin, got.Action) - } - if got.Details["route_host"] != "target-svc" { - t.Errorf("route_host = %q, want target-svc", got.Details["route_host"]) - } - if got.Details["cache_hit"] != "false" { - t.Errorf("cache_hit = %q, want false on first exchange", got.Details["cache_hit"]) - } -} - -func TestTokenExchange_ExchangeFailure(t *testing.T) { - exchangeSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(`{"error":"invalid_grant"}`)) - })) - defer exchangeSrv.Close() - - 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 := invokeOnRequest(p, pctx) - if action.Type != pipeline.Reject { - t.Fatalf("got %v, want Reject", action.Type) - } - status, _, _ := action.Violation.Render() - if status != http.StatusServiceUnavailable { - t.Errorf("status = %d, want 503", status) - } - // Deny branch must surface a "denied" Auth.Outbound entry with the - // machine-stable reason — matches what the listener needs to emit a - // SessionDenied event on outbound exchange failure. - if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Outbound) != 1 { - t.Fatalf("expected one Auth.Outbound entry, got %+v", pctx.Extensions.Invocations) - } - got := pctx.Extensions.Invocations.Outbound[0] - if got.Action != pipeline.ActionDeny { - t.Errorf("Action = %q, want deny", got.Action) - } - if got.Reason != "token_exchange_failed" { - t.Errorf("Reason = %q, want token_exchange_failed (from OutboundDenialReason.String)", got.Reason) - } -} - -func TestTokenExchange_NoToken_Deny(t *testing.T) { - 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 := invokeOnRequest(p, pctx) - if action.Type != pipeline.Reject { - t.Fatalf("got %v, want Reject", action.Type) - } - status, _, _ := action.Violation.Render() - if status != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", status) - } -} - // --- 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() + jwt := jwtvalidation.NewJWTValidation() if err := jwt.Configure([]byte(`{"issuer":"http://ex","audience":"a"}`)); err != nil { t.Fatalf("jwt Configure: %v", err) } - tok := NewTokenExchange() + tok := tokenexchange.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}) + // Need at least one non-StatsSource plugin to prove the filter works; + // Build a pipeline with a2a-parser (registered by side-effect import + // of plugins package's self-registering parsers). + entries := []config.PluginEntry{ + {Name: "a2a-parser"}, + } + withParser, err := plugins.Build(entries) + if err != nil { + t.Fatalf("Build(a2a-parser): %v", err) + } + // Stitch jwt + a2a-parser + tok into a test pipeline via pipeline.New + // directly (bypassing the registry for this artificial combo). + p, err := pipeline.New(append([]pipeline.Plugin{jwt}, append(withParser.Plugins(), tok)...)) if err != nil { t.Fatalf("New: %v", err) } - got := CollectStats(p) + got := plugins.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 { + if got := plugins.CollectStats(nil); got != nil { t.Errorf("CollectStats(nil) = %v, want nil", got) } } @@ -834,7 +99,7 @@ func TestCollectStats_NilPipeline(t *testing.T) { // --- Registry / Build --- func TestBuild_ValidNames(t *testing.T) { - p, err := Build([]config.PluginEntry{ + p, err := plugins.Build([]config.PluginEntry{ {Name: "a2a-parser"}, {Name: "mcp-parser"}, }) @@ -847,14 +112,13 @@ func TestBuild_ValidNames(t *testing.T) { } func TestBuild_UnknownName(t *testing.T) { - _, err := Build([]config.PluginEntry{{Name: "nonexistent-plugin"}}) - if err == nil { + if _, err := plugins.Build([]config.PluginEntry{{Name: "nonexistent-plugin"}}); err == nil { t.Fatal("expected error for unknown plugin name") } } func TestBuild_EmptyList(t *testing.T) { - p, err := Build([]config.PluginEntry{}) + p, err := plugins.Build([]config.PluginEntry{}) if err != nil { t.Fatalf("Build: %v", err) } @@ -864,30 +128,21 @@ func TestBuild_EmptyList(t *testing.T) { } } -// 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{ + _, err := plugins.Build([]config.PluginEntry{ {Name: "a2a-parser", Config: []byte(`{"unused":true}`)}, }) if err == nil { t.Fatal("expected error for config on non-Configurable plugin") } - // 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") + t.Errorf("error %q does not match contract", 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 + _, err := plugins.Build([]config.PluginEntry{ + {Name: "jwt-validation", Config: []byte(`{}`)}, }) if err == nil { t.Fatal("expected error for invalid jwt-validation config") diff --git a/authbridge/authlib/cache/cache.go b/authbridge/authlib/plugins/tokenexchange/cache/cache.go similarity index 100% rename from authbridge/authlib/cache/cache.go rename to authbridge/authlib/plugins/tokenexchange/cache/cache.go diff --git a/authbridge/authlib/cache/cache_test.go b/authbridge/authlib/plugins/tokenexchange/cache/cache_test.go similarity index 100% rename from authbridge/authlib/cache/cache_test.go rename to authbridge/authlib/plugins/tokenexchange/cache/cache_test.go diff --git a/authbridge/authlib/exchange/auth.go b/authbridge/authlib/plugins/tokenexchange/exchange/auth.go similarity index 100% rename from authbridge/authlib/exchange/auth.go rename to authbridge/authlib/plugins/tokenexchange/exchange/auth.go diff --git a/authbridge/authlib/exchange/client.go b/authbridge/authlib/plugins/tokenexchange/exchange/client.go similarity index 100% rename from authbridge/authlib/exchange/client.go rename to authbridge/authlib/plugins/tokenexchange/exchange/client.go diff --git a/authbridge/authlib/exchange/client_test.go b/authbridge/authlib/plugins/tokenexchange/exchange/client_test.go similarity index 100% rename from authbridge/authlib/exchange/client_test.go rename to authbridge/authlib/plugins/tokenexchange/exchange/client_test.go diff --git a/authbridge/authlib/exchange/errors.go b/authbridge/authlib/plugins/tokenexchange/exchange/errors.go similarity index 100% rename from authbridge/authlib/exchange/errors.go rename to authbridge/authlib/plugins/tokenexchange/exchange/errors.go diff --git a/authbridge/authlib/plugins/tokenexchange.go b/authbridge/authlib/plugins/tokenexchange/plugin.go similarity index 97% rename from authbridge/authlib/plugins/tokenexchange.go rename to authbridge/authlib/plugins/tokenexchange/plugin.go index fdd30080a..f5d80b042 100644 --- a/authbridge/authlib/plugins/tokenexchange.go +++ b/authbridge/authlib/plugins/tokenexchange/plugin.go @@ -1,4 +1,4 @@ -package plugins +package tokenexchange import ( "bytes" @@ -12,12 +12,13 @@ import ( "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/plugins" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/spiffe" "github.com/kagenti/kagenti-extensions/authbridge/authlib/routing" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/spiffe" ) // tokenExchangeConfig is the plugin's local config schema. See @@ -204,7 +205,7 @@ type TokenExchange struct { func NewTokenExchange() *TokenExchange { return &TokenExchange{} } func init() { - RegisterPlugin("token-exchange", func() pipeline.Plugin { return NewTokenExchange() }) + plugins.RegisterPlugin("token-exchange", func() pipeline.Plugin { return NewTokenExchange() }) } func (p *TokenExchange) Name() string { return "token-exchange" } @@ -574,5 +575,5 @@ var ( _ pipeline.Initializer = (*TokenExchange)(nil) _ pipeline.Shutdowner = (*TokenExchange)(nil) _ pipeline.Readier = (*TokenExchange)(nil) - _ StatsSource = (*TokenExchange)(nil) + _ plugins.StatsSource = (*TokenExchange)(nil) ) diff --git a/authbridge/authlib/plugins/tokenexchange/plugin_test.go b/authbridge/authlib/plugins/tokenexchange/plugin_test.go new file mode 100644 index 000000000..edaae0926 --- /dev/null +++ b/authbridge/authlib/plugins/tokenexchange/plugin_test.go @@ -0,0 +1,329 @@ +package tokenexchange + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" +) + +func invokeOnRequest(p pipeline.Plugin, pctx *pipeline.Context) pipeline.Action { + pctx.SetCurrentPlugin(p.Name(), pipeline.InvocationPhaseRequest) + defer pctx.ClearCurrentPlugin() + return p.OnRequest(context.Background(), pctx) +} + +// --- Configure --- + +func TestTokenExchange_Configure_MissingTokenURL(t *testing.T) { + p := NewTokenExchange() + if err := p.Configure([]byte(`{"identity":{"type":"client-secret","client_id":"c","client_secret":"s"}}`)); err == nil { + t.Fatal("expected error for missing token_url") + } +} + +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) + } +} + +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) + } + 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.JWTSVIDPath != "/opt/jwt_svid.token" { + t.Errorf("JWTSVIDPath = %q, want /opt/jwt_svid.token", p.cfg.Identity.JWTSVIDPath) + } +} + +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_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") + } +} + +func TestTokenExchange_Configure_IdentityValidation(t *testing.T) { + cases := []struct{ name, 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") + } + }) + } +} + +// --- 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") + } +} + +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") + } +} + +// --- OnRequest --- + +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 := invokeOnRequest(p, pctx) + if action.Type != pipeline.Continue { + 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") + } + if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Outbound) != 1 { + t.Fatalf("expected one Outbound entry, got %+v", pctx.Extensions.Invocations) + } + ob := pctx.Extensions.Invocations.Outbound[0] + if ob.Plugin != "token-exchange" || ob.Action != pipeline.ActionSkip { + t.Errorf("entry = (%q, %q), want (token-exchange, skip)", ob.Plugin, ob.Action) + } + if ob.Details["route_host"] != "some-host" { + t.Errorf("route_host = %q, want some-host", ob.Details["route_host"]) + } + if ob.Details["route_matched"] != "false" { + t.Errorf("route_matched = %q, want false on default-policy passthrough", ob.Details["route_matched"]) + } +} + +func TestTokenExchange_ExchangeSuccess(t *testing.T) { + exchangeSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "access_token": "new-token", "token_type": "Bearer", "expires_in": 300, + }) + })) + defer exchangeSrv.Close() + + 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 := invokeOnRequest(p, pctx) + if action.Type != pipeline.Continue { + t.Fatalf("got %v, want Continue", action.Type) + } + if pctx.Headers.Get("Authorization") != "Bearer new-token" { + t.Errorf("token = %q, want Bearer new-token", pctx.Headers.Get("Authorization")) + } + if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Outbound) != 1 { + t.Fatalf("expected one entry, got %+v", pctx.Extensions.Invocations) + } + got := pctx.Extensions.Invocations.Outbound[0] + if got.Plugin != "token-exchange" || got.Action != pipeline.ActionModify { + t.Errorf("got Plugin=%q Action=%q, want token-exchange/modify", got.Plugin, got.Action) + } + if got.Details["route_host"] != "target-svc" { + t.Errorf("route_host = %q, want target-svc", got.Details["route_host"]) + } + if got.Details["cache_hit"] != "false" { + t.Errorf("cache_hit = %q, want false on first exchange", got.Details["cache_hit"]) + } +} + +func TestTokenExchange_ExchangeFailure(t *testing.T) { + exchangeSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error":"invalid_grant"}`)) + })) + defer exchangeSrv.Close() + + 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 := invokeOnRequest(p, pctx) + if action.Type != pipeline.Reject { + t.Fatalf("got %v, want Reject", action.Type) + } + status, _, _ := action.Violation.Render() + if status != http.StatusServiceUnavailable { + t.Errorf("status = %d, want 503", status) + } + if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Outbound) != 1 { + t.Fatalf("expected one entry, got %+v", pctx.Extensions.Invocations) + } + got := pctx.Extensions.Invocations.Outbound[0] + if got.Action != pipeline.ActionDeny { + t.Errorf("Action = %q, want deny", got.Action) + } + if got.Reason != "token_exchange_failed" { + t.Errorf("Reason = %q, want token_exchange_failed", got.Reason) + } +} + +func TestTokenExchange_NoToken_Deny(t *testing.T) { + 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 := invokeOnRequest(p, pctx) + if action.Type != pipeline.Reject { + t.Fatalf("got %v, want Reject", action.Type) + } + status, _, _ := action.Violation.Render() + if status != http.StatusUnauthorized { + t.Errorf("status = %d, want 401", status) + } +} diff --git a/authbridge/authlib/spiffe/file.go b/authbridge/authlib/plugins/tokenexchange/spiffe/file.go similarity index 100% rename from authbridge/authlib/spiffe/file.go rename to authbridge/authlib/plugins/tokenexchange/spiffe/file.go diff --git a/authbridge/authlib/spiffe/file_test.go b/authbridge/authlib/plugins/tokenexchange/spiffe/file_test.go similarity index 100% rename from authbridge/authlib/spiffe/file_test.go rename to authbridge/authlib/plugins/tokenexchange/spiffe/file_test.go diff --git a/authbridge/authlib/spiffe/source.go b/authbridge/authlib/plugins/tokenexchange/spiffe/source.go similarity index 100% rename from authbridge/authlib/spiffe/source.go rename to authbridge/authlib/plugins/tokenexchange/spiffe/source.go diff --git a/authbridge/cmd/authbridge/listener/extauthz/server_test.go b/authbridge/cmd/authbridge/listener/extauthz/server_test.go index 514f3bfba..d332b83ba 100644 --- a/authbridge/cmd/authbridge/listener/extauthz/server_test.go +++ b/authbridge/cmd/authbridge/listener/extauthz/server_test.go @@ -12,12 +12,12 @@ import ( "google.golang.org/grpc/codes" 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/tokenexchange/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "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" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) type mockVerifier struct { diff --git a/authbridge/cmd/authbridge/listener/extproc/server_test.go b/authbridge/cmd/authbridge/listener/extproc/server_test.go index 512d55a3c..a4b015976 100644 --- a/authbridge/cmd/authbridge/listener/extproc/server_test.go +++ b/authbridge/cmd/authbridge/listener/extproc/server_test.go @@ -16,13 +16,13 @@ import ( "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/plugins/tokenexchange/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "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/session" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) // mockStream implements ExternalProcessor_ProcessServer for testing. diff --git a/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go b/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go index 0331a1c12..2fe6ec382 100644 --- a/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go +++ b/authbridge/cmd/authbridge/listener/forwardproxy/server_test.go @@ -11,12 +11,12 @@ import ( "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/plugins/tokenexchange/cache" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange/exchange" "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" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) type mockVerifier struct { diff --git a/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go b/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go index 27095d192..9447312f3 100644 --- a/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go +++ b/authbridge/cmd/authbridge/listener/reverseproxy/server_test.go @@ -13,7 +13,7 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/bypass" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/plugintesting" - "github.com/kagenti/kagenti-extensions/authbridge/authlib/validation" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation/validation" ) type mockVerifier struct { diff --git a/authbridge/cmd/authbridge/main.go b/authbridge/cmd/authbridge/main.go index 3ffe6bd89..7831aaec3 100644 --- a/authbridge/cmd/authbridge/main.go +++ b/authbridge/cmd/authbridge/main.go @@ -30,6 +30,14 @@ import ( "github.com/kagenti/kagenti-extensions/authbridge/authlib/observe" "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" + // Bundled plugins register themselves via init() on import. These + // two moved from flat files in plugins/ to their own subpackages + // (they own private subtrees for validation / exchange / cache / + // spiffe); we opt into them here. External plugins follow the + // same pattern — drop one line in here (or a plugins_extra.go) to + // include them in this binary's build. + _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation" + _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange" "github.com/kagenti/kagenti-extensions/authbridge/authlib/reloader" "github.com/kagenti/kagenti-extensions/authbridge/authlib/session" "github.com/kagenti/kagenti-extensions/authbridge/authlib/sessionapi" From bbfba9dc9221b4febbb455ccc11b7d722c8117ff Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sun, 10 May 2026 09:12:22 -0400 Subject: [PATCH 5/6] docs: Refresh for Identity interface, Invocation.Details, package moves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - framework-architecture.md §3 Context: Identity field + interface replaces Claims; Route removed. - framework-architecture.md §4 Invocation: Details map replaces the 9 typed diagnostic fields. - framework-architecture.md §7 SessionEvent: TargetAudience removed. - framework-architecture.md §12 Versioning: new changelog entries for the detyping and the authlib reorganization. - plugin-reference.md Invocation field reference: Details map + the suggested key conventions used by built-in plugins. - plugin-tutorial.md Step 2 Record example: uses Details map. - authlib/README.md package table: split into "shared building blocks" vs "plugins and their internals"; notes the relocation. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/README.md | 27 ++++++--- authbridge/docs/framework-architecture.md | 70 ++++++++++++----------- authbridge/docs/plugin-reference.md | 56 +++++++++--------- authbridge/docs/plugin-tutorial.md | 13 +++-- 4 files changed, 92 insertions(+), 74 deletions(-) diff --git a/authbridge/authlib/README.md b/authbridge/authlib/README.md index ece4ce7e5..76b6cecea 100644 --- a/authbridge/authlib/README.md +++ b/authbridge/authlib/README.md @@ -4,21 +4,30 @@ A pure Go library providing reusable building blocks for JWT validation, OAuth 2 ## Packages +**Shared building blocks** (used by ≥2 plugins or the framework): + | Package | Purpose | |---------|---------| -| `validation/` | JWKS-backed JWT verifier (`lestrrat-go/jwx`) with required audience parameter | -| `exchange/` | RFC 8693 token exchange + client credentials grant with pluggable auth | -| `cache/` | SHA-256 keyed token cache with TTL eviction | -| `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` — used internally by `jwt-validation` and `token-exchange` plugins | +| `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. | | `config/` | YAML config loader, mode presets, credential-file waiters, top-level validation | | `pipeline/` | Plugin pipeline + lifecycle (`Configurable`, `Initializer`, `Shutdowner`) — see [docs/framework-architecture.md](../docs/framework-architecture.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 plugins: `jwt-validation`, `token-exchange`, `a2a-parser`, `mcp-parser`, `inference-parser`. See [docs/plugin-reference.md](../docs/plugin-reference.md) for the per-plugin config convention | -| `observe/` | OTEL + metrics helpers | +| `reloader/` | fsnotify-based config hot-reload — atomic pipeline swap on ConfigMap change | +| `observe/` | `/stats`, `/config`, `/reload/status` HTTP server | + +**Plugins and their internals**: + +| Package | Purpose | +|---------|---------| +| `plugins/` | Registry + parser plugins (a2a-parser, mcp-parser, inference-parser) + shared `Build` + `StatsSource` contract | +| `plugins/jwtvalidation/` | The `jwt-validation` plugin. Owns `plugins/jwtvalidation/validation/` (JWKS-backed JWT verifier). | +| `plugins/tokenexchange/` | The `token-exchange` plugin. Owns `plugins/tokenexchange/exchange/` (RFC 8693 client), `plugins/tokenexchange/cache/` (token TTL cache), `plugins/tokenexchange/spiffe/` (JWT-SVID file source). | +| `plugins/plugintesting/` | Test helpers — stubs of jwt-validation / token-exchange that skip file IO, for listener-level tests. | + +Packages that used to live at `authlib/validation`, `authlib/exchange`, `authlib/cache`, `authlib/spiffe` moved under their owning plugin. They had no reuse outside that plugin; keeping them at `authlib/` top-level implied wider usefulness than reality. New plugins should follow the same pattern: if the package is plugin-internal, colocate it under `plugins//`. ## Usage diff --git a/authbridge/docs/framework-architecture.md b/authbridge/docs/framework-architecture.md index af5add840..6cb1e11a1 100644 --- a/authbridge/docs/framework-architecture.md +++ b/authbridge/docs/framework-architecture.md @@ -113,10 +113,9 @@ type Context struct { Body []byte // nil unless a plugin declared BodyAccess: true StartedAt time.Time // listener wall-clock at request entry - Agent *AgentIdentity // this workload's SPIFFE / Keycloak identity - Claims *validation.Claims // inbound caller's JWT claims after jwt-validation - Route *routing.ResolvedRoute // outbound: resolved audience / token scopes - Session *SessionView // read-only view of the session bucket + Agent *AgentIdentity // this workload's SPIFFE / Keycloak identity + Identity Identity // caller identity after an auth plugin runs (interface) + Session *SessionView // read-only view of the session bucket // Response-phase fields (populated by listener before RunResponse) StatusCode int @@ -132,8 +131,8 @@ type Context struct { - Plugins **write** fields they declared in `Capabilities.Writes`. By convention each extension slot has exactly one writer (the parser plugin). - Plugins read `pctx.Body` / `pctx.ResponseBody` only if they declared `ReadsBody: true` (or the deprecated `BodyAccess: true`). - Plugins mutate body content via `pctx.SetBody(newBytes)` / `pctx.SetResponseBody(newBytes)`, and only if they declared `WritesBody: true`. Direct assignment (`pctx.Body = ...`) compiles but bypasses listener propagation and misses the Invocation + body-mutation event emission — see §6, "Body mutation." -- `Claims` is populated by `jwt-validation` and is read-only afterward. -- `Agent`, `Route`, `Session` are populated by the listener before `Run`. Plugins treat them as read-only. +- `Identity` is populated by whichever auth plugin ran (jwt-validation ships a `claimsIdentity` adapter around `validation.Claims`; a SAML / mTLS / custom plugin publishes its own adapter). The framework reads it through the `Identity` interface (`Subject()` / `ClientID()` / `Scopes()`) so no plugin-specific type leaks into `pipeline/`. +- `Agent`, `Session` are populated by the listener before `Run`. Plugins treat them as read-only. - `ResponseBody` appears between `Run` and `RunResponse` — plugins must not read it in `OnRequest`. **Framework-owned attribution.** `pipeline.Run` / `RunResponse` stamp the currently-dispatching plugin's name and phase onto unexported fields of `pctx` around each plugin call. These drive the `pctx.Record` family of helpers so Invocation entries are auto-attributed without plugin-side ceremony. Plugins can't set them directly (unexported); exported `SetCurrentPlugin` / `ClearCurrentPlugin` exist for test harnesses that invoke plugins outside a `Pipeline.Run` dispatch loop. @@ -146,9 +145,12 @@ pctx.Skip("path_bypass") // plugin ran but didn't act pctx.Observe("matched_tools/call") // parser extracted data pctx.Modify("token_replaced") // plugin mutated the message pctx.Record(pipeline.Invocation{ // full form with diagnostic fields - Action: pipeline.ActionDeny, - Reason: "jwt_failed", - ExpectedIssuer: issuer, + Action: pipeline.ActionDeny, + Reason: "jwt_failed", + Details: map[string]string{ + "expected_issuer": issuer, + "expected_audience": audience, + }, }) return pctx.DenyAndRecord(reason, code, message) // emit + reject in one call ``` @@ -181,20 +183,19 @@ Every plugin that runs on a pipeline pass appends at least one `Invocation` to t ```go type Invocation struct { - Plugin string // plugin.Name(); framework-filled - Action InvocationAction // 5-value: allow | deny | skip | modify | observe - Phase InvocationPhase // "request" | "response"; framework-filled - Reason string // machine-stable code, e.g. "path_bypass" - Path string // request path; framework-filled - - // Optional diagnostic fields (populated selectively): - ExpectedIssuer, ExpectedAudience string - TokenSubject string - TokenAudience, TokenScopes []string - RouteMatched bool - RouteHost, TargetAudience string - RequestedScopes []string - CacheHit bool + Plugin string // plugin.Name(); framework-filled + Action InvocationAction // 5-value: allow | deny | skip | modify | observe + Phase InvocationPhase // "request" | "response"; framework-filled + Reason string // machine-stable code, e.g. "path_bypass" + Path string // request path; framework-filled + + // Plugin-specific diagnostic context. Opaque to the framework; + // rendered as key=value rows by abctl. Built-in plugins populate + // expected_issuer, token_subject, route_host, target_audience, + // cache_hit (snake_case). Third-party plugins define their own + // key space. Stringify booleans as "true"/"false" and []string + // as space-joined to match OAuth scope conventions. + Details map[string]string } type Invocations struct { @@ -534,17 +535,16 @@ type SessionEvent struct { At time.Time Direction Direction // inbound | outbound Phase SessionPhase // request | response | denied - A2A *A2AExtension // snapshot of pctx.Extensions.A2A - MCP *MCPExtension - Inference *InferenceExtension - Invocations *Invocations // per-plugin action records, filtered by phase - Plugins map[string]json.RawMessage // plugin-public events (escape-hatch /event suffix) - Identity *EventIdentity // Subject, ClientID, AgentID, Scopes - StatusCode int // response phase only - Error *EventError // populated on 4xx/5xx - Host string // :authority - TargetAudience string // outbound: resolved OAuth audience - Duration time.Duration // response: wall-clock since request entry + A2A *A2AExtension // snapshot of pctx.Extensions.A2A + MCP *MCPExtension + Inference *InferenceExtension + Invocations *Invocations // per-plugin action records, filtered by phase + Plugins map[string]json.RawMessage // plugin-public events (escape-hatch /event suffix) + Identity *EventIdentity // Subject, ClientID, AgentID, Scopes + StatusCode int // response phase only + Error *EventError // populated on 4xx/5xx + Host string // :authority + Duration time.Duration // response: wall-clock since request entry } ``` @@ -674,6 +674,8 @@ The plugin interface is **not** semver-stable yet (AuthBridge is pre-1.0). Chang - **Open plugin registry**: plugins self-register from `init()` via `plugins.RegisterPlugin`. Third-party plugins in external modules drop in via a side-effect import. Closed `registry` map literal removed. - **Config hot-reload**: new `pipeline.Holder` (atomic wrapper) + `authlib/reloader` package (fsnotify-driven). Listeners receive `*Holder` instead of `*Pipeline`; the reloader atomically swaps the holder's contents when the config file changes. `mode` and `listener.*` edits are refused (pod restart required); any other change is picked up within the kubelet sync window (~60s). See §9. - **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. Breaking changes will be announced in `authbridge/CHANGELOG.md` (TBD) before a 1.0 tag. diff --git a/authbridge/docs/plugin-reference.md b/authbridge/docs/plugin-reference.md index d46f1d231..70dc1e5c2 100644 --- a/authbridge/docs/plugin-reference.md +++ b/authbridge/docs/plugin-reference.md @@ -306,20 +306,15 @@ at least one. ```go type Invocation struct { - Plugin string // plugin.Name(); framework-filled - Action InvocationAction // 5-value: allow | deny | skip | modify | observe - Phase InvocationPhase // "request" | "response"; framework-filled - Reason string // machine-stable code - Path string // request path; framework-filled - - // Optional diagnostic fields; populated selectively: - ExpectedIssuer, ExpectedAudience string - TokenSubject string - TokenAudience, TokenScopes []string - RouteMatched bool - RouteHost, TargetAudience string - RequestedScopes []string - CacheHit bool + Plugin string // plugin.Name(); framework-filled + Action InvocationAction // 5-value: allow | deny | skip | modify | observe + Phase InvocationPhase // "request" | "response"; framework-filled + Reason string // machine-stable code + Path string // request path; framework-filled + + // Plugin-specific diagnostic context. Opaque to the framework; + // abctl renders as key=value rows in the detail pane. + Details map[string]string } ``` @@ -345,18 +340,27 @@ discriminates within an Action value. abctl filters can match either — `/skip` shows every skip action regardless of reason; `/path_bypass` narrows to that specific skip flavour. -**Which diagnostic fields to populate:** - -- Auth gates (jwt-validation and kin): `ExpectedIssuer`, - `ExpectedAudience`, `TokenSubject`, `TokenAudience`, `TokenScopes`. -- Outbound routers (token-exchange and kin): `RouteMatched`, - `RouteHost`, `TargetAudience`, `RequestedScopes`, `CacheHit`. -- Parsers: usually none — their semantic payload lives on the typed - extension slot (A2A / MCP / Inference). Emit with just Action + - Reason. - -**NEVER put raw tokens, signatures, or secrets in an Invocation.** -The session store has no auth. +**What to put in Details:** + +Suggested key conventions used by built-in plugins (operators +already know these; abctl filters match substring on both key and +value): + +- Auth gates (jwt-validation): `expected_issuer`, `expected_audience`, + `token_subject`, `token_audience`, `token_scopes`. +- Outbound routers (token-exchange): `route_matched` (`"true"`/`"false"`), + `route_host`, `target_audience`, `requested_scopes`, `cache_hit`. +- Parsers: usually no Details — their semantic payload lives on the + typed extension slot (A2A / MCP / Inference). Emit with just + Action + Reason. +- Third-party plugins: pick snake_case keys scoped to your semantic + domain (`tokens_remaining`, `quota_bucket`, `redaction_count`, etc.). + Stringify booleans as `"true"`/`"false"`; stringify `[]string` as + space-joined (OAuth scope convention). + +**NEVER put raw tokens, signatures, or client credentials in +`Details`.** The session store has no auth on it; only safe-to-log data +belongs in Invocations. ### 2. Named protocol extension (optional, for parsers) diff --git a/authbridge/docs/plugin-tutorial.md b/authbridge/docs/plugin-tutorial.md index bfb14f90a..f59c18372 100644 --- a/authbridge/docs/plugin-tutorial.md +++ b/authbridge/docs/plugin-tutorial.md @@ -86,14 +86,17 @@ pctx.Observe("matched_tools/call") // parser extracted data pctx.Modify("token_replaced") // plugin mutated the message ``` -For invocations that carry extra diagnostic context, use `Record`: +For invocations that carry extra diagnostic context, populate +`Invocation.Details`: ```go pctx.Record(pipeline.Invocation{ - Action: pipeline.ActionAllow, - Reason: "authorized", - TokenSubject: claims.Subject, - TokenScopes: claims.Scopes, + Action: pipeline.ActionAllow, + Reason: "authorized", + Details: map[string]string{ + "token_subject": claims.Subject, + "token_scopes": strings.Join(claims.Scopes, " "), + }, }) ``` From a11985f50cdb1b30eeabd7e77420789c23e21531 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sun, 10 May 2026 11:11:12 -0400 Subject: [PATCH 6/6] docs+test(authlib): Clarify Details multi-value encoding; lock aud comma-join PR #392 review surfaced two related gaps around Invocation.Details value encoding: 1. plugin-reference.md said "[]string space-joined (OAuth scope convention)" as a blanket rule, but jwt-validation writes aud with comma. The distinction is principled - RFC 6749 forbids spaces in scope tokens while RFC 7519 permits spaces in JWT audiences - but wasn't documented for third-party plugin authors. 2. plugin_test.go only covered single-element audiences, so a future drive-by change to strings.Join(..., " ") would pass CI and silently break consumers splitting on comma. Docs: expand the Details encoding block into a type->encoding table with the scope-vs-audience split and its RFC rationale. Add general guidance on delimiter choice for third-party []string fields. Test: new TestJWTValidation_OnRequest_MultiAudience_CommaJoined asserts Audience{"aud-a","aud-b"} renders as "aud-a,aud-b" and token_scopes keeps using space in the same record - locks both encodings in. No code change; jwt-validation already uses comma for aud and space for scopes. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../plugins/jwtvalidation/plugin_test.go | 40 +++++++++++++++++++ authbridge/docs/plugin-reference.md | 21 +++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/authbridge/authlib/plugins/jwtvalidation/plugin_test.go b/authbridge/authlib/plugins/jwtvalidation/plugin_test.go index ef75e5c8f..0db17cabf 100644 --- a/authbridge/authlib/plugins/jwtvalidation/plugin_test.go +++ b/authbridge/authlib/plugins/jwtvalidation/plugin_test.go @@ -318,3 +318,43 @@ func TestJWTValidation_OnRequest_PopulatesAuth_Allow(t *testing.T) { t.Errorf("token_audience = %q, want agent-aud", got.Details["token_audience"]) } } + +// TestJWTValidation_OnRequest_MultiAudience_CommaJoined pins the encoding +// of multi-value audiences to comma-join. JWT RFC 7519 allows spaces in +// aud values, so a space-joined form would be ambiguous for consumers +// splitting on the delimiter. Deliberately locked down here so a future +// change to strings.Join(..., " ") is caught before shipping. +func TestJWTValidation_OnRequest_MultiAudience_CommaJoined(t *testing.T) { + claims := &validation.Claims{ + Subject: "alice", + Issuer: "http://issuer.example", + Audience: []string{"aud-a", "aud-b"}, + ClientID: "caller", + Scopes: []string{"openid", "write"}, + } + inner := auth.New(auth.Config{ + Verifier: &mockJWTVerifier{claims: claims}, + Identity: auth.IdentityConfig{Audience: "aud-a"}, + }) + p := newTestJWTValidation(t, "http://issuer.example", inner) + + pctx := &pipeline.Context{Headers: http.Header{}, Path: "/api/call"} + pctx.Headers.Set("Authorization", "Bearer tok") + action := invokeOnRequest(p, pctx) + if action.Type != pipeline.Continue { + t.Fatalf("expected Continue, got %v (violation=%+v)", action.Type, action.Violation) + } + if pctx.Extensions.Invocations == nil || len(pctx.Extensions.Invocations.Inbound) != 1 { + t.Fatalf("expected one entry, got %+v", pctx.Extensions.Invocations) + } + got := pctx.Extensions.Invocations.Inbound[0] + if got.Details["token_audience"] != "aud-a,aud-b" { + t.Errorf("token_audience = %q, want %q (comma-joined per RFC 7519 spaces-allowed-in-aud)", + got.Details["token_audience"], "aud-a,aud-b") + } + // Also sanity-check scopes still use space — the two conventions + // must coexist in the same record. + if got.Details["token_scopes"] != "openid write" { + t.Errorf("token_scopes = %q, want %q", got.Details["token_scopes"], "openid write") + } +} diff --git a/authbridge/docs/plugin-reference.md b/authbridge/docs/plugin-reference.md index 70dc1e5c2..d48a11fac 100644 --- a/authbridge/docs/plugin-reference.md +++ b/authbridge/docs/plugin-reference.md @@ -355,8 +355,25 @@ value): Action + Reason. - Third-party plugins: pick snake_case keys scoped to your semantic domain (`tokens_remaining`, `quota_bucket`, `redaction_count`, etc.). - Stringify booleans as `"true"`/`"false"`; stringify `[]string` as - space-joined (OAuth scope convention). + +**Value encoding:** `Details` values are strings; plugins must +stringify non-string data themselves. Conventions across built-ins: + +| Go type | Encoding | Rationale | +|---|---|---| +| `string` | as-is | no transform | +| `bool` | `"true"` / `"false"` | stable, parse-friendly | +| `int` / `float` | `strconv.Itoa` / `strconv.FormatFloat` | decimal, no unit suffix | +| `[]string` — OAuth scopes | space-joined (`"openid email"`) | RFC 6749 forbids spaces in scope tokens, so space-delimited is unambiguous | +| `[]string` — JWT audiences | comma-joined (`"aud-a,aud-b"`) | RFC 7519 permits spaces in `aud`, so space-joining would be ambiguous | +| `[]string` — other | comma-joined by default; pick a delimiter that can't appear in your values | operator split-on-delimiter needs one predictable choice | +| `time.Time` | RFC 3339 | consistent with logs | +| `time.Duration` | `strconv.FormatInt(d.Milliseconds(), 10)` + `_ms` suffix on the key | milliseconds integer is abctl-friendly | + +If your field's elements might contain the delimiter, pick a different +delimiter and document it on the field rather than escape — consumers +doing `strings.Split` are simpler to write against an unambiguous +separator than against an escape convention. **NEVER put raw tokens, signatures, or client credentials in `Details`.** The session store has no auth on it; only safe-to-log data