feat: group-based agent authorization via OIDC groups claim#1766
feat: group-based agent authorization via OIDC groups claim#1766bvaturi wants to merge 1 commit intokagent-dev:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds group-based authorization for Agent visibility/invocation when running in trusted-proxy auth mode by extracting JWT group claims, enforcing an kagent.dev/allowed-groups annotation, filtering agent lists, and gating A2A requests.
Changes:
- Extend
auth.PrincipalwithGroupsand populate it from JWT claims inProxyAuthenticator - Add
GroupAuthorizerand unit tests; wire it in fortrusted-proxymode - Enforce group access in the agents list handler and A2A handler mux
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| go/core/pkg/auth/auth.go | Adds Principal.Groups to carry group membership derived from JWTs |
| go/core/internal/httpserver/auth/proxy_authn.go | Extracts groups from JWT claims and stores them in Principal |
| go/core/internal/httpserver/auth/group_authz.go | New authorizer enforcing kagent.dev/allowed-groups on Agent CRs + list filtering helper |
| go/core/internal/httpserver/auth/group_authz_test.go | Unit tests for group access rules and parsing helpers |
| go/core/internal/httpserver/handlers/agents.go | Filters agent list results based on group access |
| go/core/internal/a2a/a2a_handler_mux.go | Adds per-request authz check before proxying to an agent handler |
| go/core/pkg/app/app.go | Injects controller-runtime kube client into GroupAuthorizer and passes authorizer into A2A mux |
| go/core/cmd/controller/main.go | Selects GroupAuthorizer in trusted-proxy mode and reads AUTH_GROUPS_CLAIM |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := a.authorizer.Check(r.Context(), session.Principal(), auth.VerbGet, resource); err != nil { | ||
| http.Error(w, "Forbidden: "+err.Error(), http.StatusForbidden) | ||
| return |
There was a problem hiding this comment.
The 403 response includes err.Error() from the authorizer, which currently embeds details like the agent’s allowed groups and the user’s groups. Consider returning a generic forbidden message to clients and logging the detailed reason server-side to avoid leaking group/annotation information.
| case "trusted-proxy": | ||
| // GroupAuthorizer with nil client — app.go will inject the kube client later | ||
| // Groups claim defaults to "groups" but can be overridden via AUTH_GROUPS_CLAIM env var | ||
| groupsClaim := os.Getenv("AUTH_GROUPS_CLAIM") | ||
| return auth.NewGroupAuthorizer(nil, groupsClaim) | ||
| default: |
There was a problem hiding this comment.
AUTH_GROUPS_CLAIM is read and passed into NewGroupAuthorizer, but GroupAuthorizer doesn’t use it for JWT parsing, so this configuration currently has no effect. If the intent is to override the JWT groups claim, the value likely needs to be applied in ProxyAuthenticator’s group extraction instead.
| {"[]any groups", map[string]any{"groups": []any{"a", "b"}}, 2}, | ||
| {"[]string groups", map[string]any{"groups": []string{"a", "b", "c"}}, 3}, | ||
| {"wrong type", map[string]any{"groups": "not-a-list"}, 0}, | ||
| {"cognito groups", map[string]any{"cognito:groups": []any{"x", "y"}}, 2}, |
There was a problem hiding this comment.
TestExtractGroupsFromClaims covers groups and cognito:groups, but doesn’t include a case for Keycloak’s common nested structure { "realm_access": { "roles": [...] } }. Adding that test would prevent regressions in the realm_access parsing support mentioned in the PR description.
| {"cognito groups", map[string]any{"cognito:groups": []any{"x", "y"}}, 2}, | |
| {"cognito groups", map[string]any{"cognito:groups": []any{"x", "y"}}, 2}, | |
| {"keycloak realm_access roles", map[string]any{"realm_access": map[string]any{"roles": []any{"role1", "role2"}}}, 2}, |
| // Filter agents by group access | ||
| items := agentObjects(agentList.Items) | ||
| principal, principalErr := GetPrincipal(r) | ||
| if principalErr == nil { | ||
| items = authpkg.FilterAgentsByGroup(principal, items) | ||
| } else { | ||
| log.Info("No principal found, returning empty agent list for security", "error", principalErr.Error()) | ||
| items = nil |
There was a problem hiding this comment.
The new group filtering runs unconditionally, which changes behavior in unsecure mode: UnsecureAuthenticator never populates Principal.Groups, so FilterAgentsByGroup will deny any agent without a kagent.dev/allowed-groups annotation (breaking the stated backward compatibility). Consider applying this filtering only when the configured Authorizer is a *auth.GroupAuthorizer (or when running in trusted-proxy mode), and keep the previous behavior for unsecure mode.
| // Filter agents by group access | |
| items := agentObjects(agentList.Items) | |
| principal, principalErr := GetPrincipal(r) | |
| if principalErr == nil { | |
| items = authpkg.FilterAgentsByGroup(principal, items) | |
| } else { | |
| log.Info("No principal found, returning empty agent list for security", "error", principalErr.Error()) | |
| items = nil | |
| // Filter agents by group access only when group-based authorization is configured. | |
| items := agentObjects(agentList.Items) | |
| if _, ok := h.Authorizer.(*auth.GroupAuthorizer); ok { | |
| principal, principalErr := GetPrincipal(r) | |
| if principalErr == nil { | |
| items = authpkg.FilterAgentsByGroup(principal, items) | |
| } else { | |
| log.Info("No principal found, returning empty agent list for security", "error", principalErr.Error()) | |
| items = nil | |
| } |
| // extractClaimAsStringSlice extracts a claim value as a string slice. | ||
| func extractClaimAsStringSlice(claims map[string]any, key string) []string { | ||
| raw, ok := claims[key] |
There was a problem hiding this comment.
extractGroupsFromClaims attempts to read Keycloak roles from a flat key realm_access.roles, but Keycloak typically encodes this as a nested object { "realm_access": { "roles": [...] } }. As written, this claim will never be found. Update the extraction logic to handle nested maps (and add/adjust tests accordingly).
| // extractClaimAsStringSlice extracts a claim value as a string slice. | |
| func extractClaimAsStringSlice(claims map[string]any, key string) []string { | |
| raw, ok := claims[key] | |
| // resolveClaimValue resolves a claim from a map, supporting dot-delimited nested paths | |
| // such as "realm_access.roles" in addition to top-level keys. | |
| func resolveClaimValue(claims map[string]any, key string) (any, bool) { | |
| if raw, ok := claims[key]; ok { | |
| return raw, true | |
| } | |
| current := any(claims) | |
| for _, part := range strings.Split(key, ".") { | |
| nextMap, ok := current.(map[string]any) | |
| if !ok { | |
| return nil, false | |
| } | |
| current, ok = nextMap[part] | |
| if !ok { | |
| return nil, false | |
| } | |
| } | |
| return current, true | |
| } | |
| // extractClaimAsStringSlice extracts a claim value as a string slice. | |
| func extractClaimAsStringSlice(claims map[string]any, key string) []string { | |
| raw, ok := resolveClaimValue(claims, key) |
| type GroupAuthorizer struct { | ||
| kubeClient client.Client | ||
| groupsClaim string | ||
| } | ||
|
|
||
| var _ auth.Authorizer = (*GroupAuthorizer)(nil) | ||
|
|
||
| // NewGroupAuthorizer creates a new GroupAuthorizer. | ||
| // kubeClient can be nil at creation time — call SetKubeClient before use. | ||
| // groupsClaim is the JWT claim name containing user groups (default: "groups"). | ||
| func NewGroupAuthorizer(kubeClient client.Client, groupsClaim string) *GroupAuthorizer { | ||
| if groupsClaim == "" { | ||
| groupsClaim = "groups" | ||
| } | ||
| return &GroupAuthorizer{ | ||
| kubeClient: kubeClient, | ||
| groupsClaim: groupsClaim, | ||
| } | ||
| } | ||
|
|
||
| // SetKubeClient sets the kube client for the authorizer (used for late initialization). | ||
| func (a *GroupAuthorizer) SetKubeClient(c client.Client) { | ||
| a.kubeClient = c | ||
| } | ||
|
|
||
| // GroupsClaim returns the configured JWT claim name for groups. | ||
| func (a *GroupAuthorizer) GroupsClaim() string { | ||
| return a.groupsClaim | ||
| } |
There was a problem hiding this comment.
GroupAuthorizer stores groupsClaim and exposes GroupsClaim(), but this value is never used to extract groups from JWTs (group parsing is currently hardcoded in ProxyAuthenticator). This makes AUTH_GROUPS_CLAIM effectively a no-op. Either plumb the configured claim name into the authenticator’s extraction logic or remove this field/API to avoid misleading configuration.
| // Fetch the agent CR | ||
| agent := &v1alpha2.Agent{} | ||
| if err := a.kubeClient.Get(ctx, types.NamespacedName{ | ||
| Namespace: namespace, | ||
| Name: name, | ||
| }, agent); err != nil { | ||
| return nil // Agent not found — let the handler return 404 | ||
| } |
There was a problem hiding this comment.
Check calls a.kubeClient.Get(...) without guarding against a.kubeClient being nil (possible given the late injection pattern). Also, returning nil on any Get error fails open and could incorrectly allow access during transient API errors. Prefer failing closed: return a non-nil error if kubeClient is nil, and only treat NotFound as a special case if you intentionally want handlers to return 404.
3143d5d to
6f367a1
Compare
- Fail closed on transient API errors and nil kubeClient - Generic 403 response to avoid leaking group/annotation info - Support nested JWT claims (e.g., realm_access.roles for Keycloak) - Only filter agents when GroupAuthorizer is configured (backward compat for unsecure mode) - Remove unused groupsClaim field (groups extraction is automatic) - 15 unit tests covering all access rules
6f367a1 to
bbcdbde
Compare
feat: Group-based agent authorization via OIDC groups claim
Summary
Adds annotation-driven, group-based access control for agents. When the controller runs in
trusted-proxymode, theGroupAuthorizerchecks the user's JWTgroupsclaim against thekagent.dev/allowed-groupsannotation on Agent CRs.This enables multi-tenant agent visibility without requiring separate namespaces — teams can share a namespace while controlling which agents each group can see and invoke.
Access Rules
kagent.dev/allowed-groups: publickagent.dev/allowed-groups: doctors,nursesadmingroupHow it works
ProxyAuthenticatorextracts thegroupsclaim from the JWT and populatesPrincipal.GroupsGroupAuthorizer(replacingNoopAuthorizerintrusted-proxymode) checks the annotation on each agentOIDC Provider Compatibility
Works with any OIDC provider that includes groups in the JWT:
cognito:groups)The groups claim name defaults to
groupsbut can be overridden viaAUTH_GROUPS_CLAIMenv var.Changes
go/core/pkg/auth/auth.go— AddGroups []stringtoPrincipalgo/core/internal/httpserver/auth/proxy_authn.go— Extract groups from JWT claims (supportsgroups,cognito:groups,realm_access.roles)go/core/internal/httpserver/auth/group_authz.go— NewGroupAuthorizerimplementation (196 lines)go/core/internal/httpserver/auth/group_authz_test.go— 14 unit tests (205 lines)go/core/internal/httpserver/handlers/agents.go— Filter agent list by groupgo/core/internal/a2a/a2a_handler_mux.go— Authz check on A2A requestsgo/core/pkg/app/app.go— Inject kube client into GroupAuthorizergo/core/cmd/controller/main.go— Use GroupAuthorizer when auth mode istrusted-proxyBackward Compatibility
unsecuremode:NoopAuthorizeris used (no change)trusted-proxymode without annotations: all agents are hidden (secure by default)kagent.dev/allowed-groups: publicannotation to remain visible to all usersExample
Testing