fix(api-server): handle Keycloak service-account- prefix in OIDC username matching#1465
Conversation
…name matching Keycloak client credentials tokens have preferred_username set to "service-account-<clientId>" rather than the raw client ID. The GRPC_SERVICE_ACCOUNT env var is populated from the clientId secret key (e.g. "ocm-ams-service"), but the JWT username is "service-account-ocm-ams-service". The direct comparison always failed, so CallerTypeService was never set and WatchSessionMessages returned PERMISSION_DENIED. Add isServiceAccount() helper that matches both the raw client ID and the Keycloak-prefixed form. Evidence: decoded the OIDC token from Red Hat SSO client credentials grant — preferred_username="service-account-ocm-ams-service". 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
📝 WalkthroughWalkthroughUpdated gRPC bearer-token interceptors to delegate service-account caller attribution to a new Changes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ambient-api-server/pkg/middleware/bearer_token_grpc.go`:
- Around line 98-104: isServiceAccount currently treats a JWT as a service
account solely by preferred_username pattern; change its signature to accept and
validate the client identity (azp/client_id) as well and require that azp
matches the configuredAccount (or a configured service client id) in addition to
the username check. Update the two call sites that invoke isServiceAccount (the
handlers that extract preferred_username) to also extract the azp/client_id
claim from the parsed JWT and pass it into the revised isServiceAccount
signature (adjust any variable names such as jwtUsername and configuredAccount
to include jwtAzp/jwtClientID), and ensure the logic enforces both username
pattern OR explicit client id match before returning true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bcee3422-603d-4ad0-bf87-6dba4e996baa
📒 Files selected for processing (2)
components/ambient-api-server/pkg/middleware/bearer_token_grpc.gocomponents/ambient-api-server/pkg/middleware/bearer_token_test.go
| func isServiceAccount(jwtUsername, configuredAccount string) bool { | ||
| if configuredAccount == "" { | ||
| return false | ||
| } | ||
| return jwtUsername == configuredAccount || | ||
| jwtUsername == keycloakServiceAccountPrefix+configuredAccount | ||
| } |
There was a problem hiding this comment.
Service caller attribution is tied only to username text, which is too weak for authz.
At Line 102-Line 103, isServiceAccount grants service caller status from preferred_username pattern alone. Since CallerTypeService gates privileged paths (e.g., inbox stream access), this should also bind to a client identity claim (e.g., azp/client_id) rather than username shape only.
Proposed hardening
-func isServiceAccount(jwtUsername, configuredAccount string) bool {
+func isServiceAccount(jwtUsername, jwtAZP, configuredAccount string) bool {
if configuredAccount == "" {
return false
}
+ if jwtAZP != configuredAccount {
+ return false
+ }
return jwtUsername == configuredAccount ||
jwtUsername == keycloakServiceAccountPrefix+configuredAccount
}Then pass azp extracted from JWT at the two call sites (Line 32 and Line 59).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/ambient-api-server/pkg/middleware/bearer_token_grpc.go` around
lines 98 - 104, isServiceAccount currently treats a JWT as a service account
solely by preferred_username pattern; change its signature to accept and
validate the client identity (azp/client_id) as well and require that azp
matches the configuredAccount (or a configured service client id) in addition to
the username check. Update the two call sites that invoke isServiceAccount (the
handlers that extract preferred_username) to also extract the azp/client_id
claim from the parsed JWT and pass it into the revised isServiceAccount
signature (adjust any variable names such as jwtUsername and configuredAccount
to include jwtAzp/jwtClientID), and ensure the logic enforces both username
pattern OR explicit client id match before returning true.
Merge Queue Status
This pull request spent 9 seconds in the queue, including 1 second running CI. Required conditions to merge |
Summary
preferred_usernametoservice-account-<clientId>(e.g.service-account-ocm-ams-service), not the raw client ID (ocm-ams-service)GRPC_SERVICE_ACCOUNTis populated from theclientIdsecret key, so the comparisonusername == serviceAccountUsernamealways failedisServiceAccount()helper that matches both the raw client ID and the Keycloak-prefixed formEvidence
Decoded the actual OIDC token issued by Red Hat SSO via client credentials grant:
{ "sub": "90d50f80-2bb6-4b7a-8781-9e322691dc33", "preferred_username": "service-account-ocm-ams-service", "azp": "ocm-ams-service" }This confirms the
service-account-prefix is added by Keycloak for all client credentials tokens.Root cause trace
preferred_username="service-account-ocm-ams-service"GRPC_SERVICE_ACCOUNT="ocm-ams-service"→ no match → CallerTypeService not set"service-account-ocm-ams-service"IsServiceCaller(ctx)→ false"service-account-ocm-ams-service" \!= "mturansk"→ PERMISSION_DENIEDTest plan
TestIsServiceAccount— 6 cases)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests