Skip to content

Generalize Keycloak role extraction to a configurable claim path - #750

Open
CodeBuildder wants to merge 1 commit into
spiffe:mainfrom
CodeBuildder:generalize-keycloak-role-claim
Open

Generalize Keycloak role extraction to a configurable claim path#750
CodeBuildder wants to merge 1 commit into
spiffe:mainfrom
CodeBuildder:generalize-keycloak-role-claim

Conversation

@CodeBuildder

Copy link
Copy Markdown

Fixes #434

Root cause

pkg/agent/authentication/authenticator/keycloak.go hardcoded role extraction to realm_access.roles via a typed KeycloakClaim/RealmAccessSubclaim struct bound with JSON tags (AuthenticateRequest, using jwt.ParseWithClaims). There was no way to configure a different claim path, so any deployment where roles live somewhere else in the token - most commonly resource_access.<clientId>.roles, Keycloak's own default location for client-scoped roles - couldn't authenticate those roles at all.

Fix

  • Added an optional roleclaim HCL config field (dot-separated path), defaulting to realm_access.roles when unset, so no existing deployment's config needs to change.
  • AuthenticateRequest now parses into generic jwt.MapClaims instead of the fixed struct; a new extractRoles helper walks the configured path. Fails closed (no roles, not a crash) if the path doesn't resolve or resolves to something other than a string list.
  • Updated docs/plugins/plugin_server_authentication_keycloak.md and docs/conf/agent/full.conf to document the new option.

Related prior work: generalize_auth (a branch pushed but never opened as a PR, last touched 2024-05-02) attempted a similar dot-path approach. This PR takes the same direction but makes roleclaim optional - the branch made it required, which would have broken every existing deployment on upgrade - and adds test coverage the branch didn't have.

How this was verified

  • Reproduced the gap against a real, locally-run Keycloak + Tornjak stack: pulled a real access token, confirmed it carries roles under both realm_access.roles and resource_access.account.roles, and confirmed the pre-fix code only ever saw the first.
  • Added unit tests in keycloak_test.go: default path still extracts roles exactly as before (regression case), an arbitrary nested path extracts correctly, a flat single-segment path works, and fail-closed behavior for a missing/wrong-shaped claim.
  • Re-ran the same live-Keycloak repro against the fixed code: default config unchanged (control), roleclaim = "resource_access.account.roles" now correctly surfaces the previously-invisible roles.
  • go build ./..., go test ./..., golangci-lint run --timeout 7m (v2.1.6, matching CI) all pass.

The Keycloak authenticator hardcoded roles to realm_access.roles via a
typed KeycloakClaim struct, so any deployment putting roles elsewhere
in the token (e.g. resource_access.<clientId>.roles, Keycloak's own
location for client-scoped roles) had no way to authenticate those
roles at all.

Add an optional roleclaim config field (dot-separated path) that
defaults to realm_access.roles when unset, so existing deployments are
unaffected. AuthenticateRequest now parses into generic jwt.MapClaims
and walks the configured path via a new extractRoles helper instead of
relying on a fixed struct shape.

Fixes spiffe#434

Signed-off-by: Kaushik Kumaran <47471121+CodeBuildder@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generalize Keycloak authentication pkg to use arbitrary claim

1 participant