fix(api-server): recognize OIDC service tokens as service callers for gRPC authz#1452
Merged
mergify[bot] merged 2 commits intomainfrom Apr 23, 2026
Merged
Conversation
… gRPC authz The control-plane gives runner pods OIDC tokens (SSO JWTs) for gRPC auth. These tokens pass JWT verification but carry the OIDC client username (e.g. ocm-ams-service), not the session creator's username. WatchSessionMessages then rejects the call with PERMISSION_DENIED because the JWT username doesn't match session.CreatedByUserId. Add GRPC_SERVICE_ACCOUNT env var support: when the JWT username matches this configured service account name, the pre-auth interceptor sets CallerTypeService on the context, allowing the call to bypass the per-user ownership check — same as static AMBIENT_API_TOKEN calls. The env var is sourced from the ambient-api-server secret's clientId key (same OIDC client ID the CP uses), with optional: true so deployments without OIDC continue to work. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
📝 WalkthroughWalkthroughBearer token middleware now reads an optional Changes
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/ambient-api-server/pkg/middleware/bearer_token.go (1)
25-38:⚠️ Potential issue | 🟠 MajorOIDC service-account support is unintentionally gated by
AMBIENT_API_TOKEN.On Line 27-30,
init()returns before readingGRPC_SERVICE_ACCOUNT, so whenAMBIENT_API_TOKENis unset the new gRPC service-account path never registers.Suggested fix
func init() { token := os.Getenv(ambientAPITokenEnv) - if token == "" { - glog.Infof("Service token auth disabled: %s not set", ambientAPITokenEnv) - return - } serviceAccount := os.Getenv(grpcServiceAccountEnv) - glog.Infof("Service token auth enabled via %s (gRPC only)", ambientAPITokenEnv) + if token == "" && serviceAccount == "" { + glog.Infof("Service token auth disabled: %s and %s not set", ambientAPITokenEnv, grpcServiceAccountEnv) + return + } + if token != "" { + glog.Infof("Service token auth enabled via %s (gRPC only)", ambientAPITokenEnv) + } else { + glog.Infof("Static service token disabled: %s not set", ambientAPITokenEnv) + } if serviceAccount != "" { glog.Infof("OIDC service account username: %s", serviceAccount) } pkgserver.RegisterPreAuthGRPCUnaryInterceptor(bearerTokenGRPCUnaryInterceptor(token, serviceAccount)) pkgserver.RegisterPreAuthGRPCStreamInterceptor(bearerTokenGRPCStreamInterceptor(token, serviceAccount)) }🤖 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.go` around lines 25 - 38, The init() currently returns when AMBIENT_API_TOKEN is empty, preventing GRPC_SERVICE_ACCOUNT handling; change init() to always read grpcServiceAccountEnv and register the appropriate gRPC pre-auth interceptors for the service-account path even if ambientAPITokenEnv is unset: read serviceAccount := os.Getenv(grpcServiceAccountEnv) before the token check (or remove the early return), log its presence with glog.Infof("OIDC service account username: %s", serviceAccount) when non-empty, and then conditionally call pkgserver.RegisterPreAuthGRPCUnaryInterceptor and pkgserver.RegisterPreAuthGRPCStreamInterceptor for the service-account flow (using bearerTokenGRPCUnaryInterceptor/bearerTokenGRPCStreamInterceptor or a service-account-specific interceptor) when serviceAccount != "", while still registering the token-based interceptors only when token != "".
🤖 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/manifests/base/core/ambient-api-server-service.yml`:
- Around line 107-112: The base manifest sets the GRPC_SERVICE_ACCOUNT env var
from secretKeyRef (name: ambient-api-server, key: clientId) with optional: true,
which leaves GRPC_SERVICE_ACCOUNT empty by default and prevents the OIDC
service-caller mapping from activating; remove the optional: true from the
GRPC_SERVICE_ACCOUNT secretKeyRef in the base manifest so the env var must be
present by default, and if you need optional behavior for specific environments,
add optional: true back only in the overlay manifests that require it.
---
Outside diff comments:
In `@components/ambient-api-server/pkg/middleware/bearer_token.go`:
- Around line 25-38: The init() currently returns when AMBIENT_API_TOKEN is
empty, preventing GRPC_SERVICE_ACCOUNT handling; change init() to always read
grpcServiceAccountEnv and register the appropriate gRPC pre-auth interceptors
for the service-account path even if ambientAPITokenEnv is unset: read
serviceAccount := os.Getenv(grpcServiceAccountEnv) before the token check (or
remove the early return), log its presence with glog.Infof("OIDC service account
username: %s", serviceAccount) when non-empty, and then conditionally call
pkgserver.RegisterPreAuthGRPCUnaryInterceptor and
pkgserver.RegisterPreAuthGRPCStreamInterceptor for the service-account flow
(using bearerTokenGRPCUnaryInterceptor/bearerTokenGRPCStreamInterceptor or a
service-account-specific interceptor) when serviceAccount != "", while still
registering the token-based interceptors only when token != "".
🪄 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: 9c2d3057-fec3-4102-ac56-c9fdea1dd379
📒 Files selected for processing (3)
components/ambient-api-server/pkg/middleware/bearer_token.gocomponents/ambient-api-server/pkg/middleware/bearer_token_grpc.gocomponents/manifests/base/core/ambient-api-server-service.yml
Comment on lines
+107
to
+112
| - name: GRPC_SERVICE_ACCOUNT | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: ambient-api-server | ||
| key: clientId | ||
| optional: true |
Contributor
There was a problem hiding this comment.
GRPC_SERVICE_ACCOUNT is silently disabled in base manifests.
optional: true on Line 112 combined with the base ambient-api-server Secret lacking clientId means this env var is empty by default, so the new OIDC service-caller mapping won’t activate.
Suggested fix
- optional: true
+ optional: falseIf you need optional behavior in some environments, keep this optional only in those overlays, not in base.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/base/core/ambient-api-server-service.yml` around lines
107 - 112, The base manifest sets the GRPC_SERVICE_ACCOUNT env var from
secretKeyRef (name: ambient-api-server, key: clientId) with optional: true,
which leaves GRPC_SERVICE_ACCOUNT empty by default and prevents the OIDC
service-caller mapping from activating; remove the optional: true from the
GRPC_SERVICE_ACCOUNT secretKeyRef in the base manifest so the env var must be
present by default, and if you need optional behavior for specific environments,
add optional: true back only in the overlay manifests that require it.
Contributor
Merge Queue Status
This pull request spent 10 seconds in the queue, including 1 second running CI. Required conditions to merge |
This was referenced Apr 24, 2026
mergify Bot
pushed a commit
that referenced
this pull request
Apr 24, 2026
…E_ACCOUNT is set (#1455) ## Summary Bug in #1452: the `init()` guard requires `AMBIENT_API_TOKEN` to be non-empty before registering gRPC pre-auth interceptors. On Stage, only `GRPC_SERVICE_ACCOUNT` is set — `AMBIENT_API_TOKEN` is not in the api-server env vars — so interceptors are never registered and the OIDC service caller logic is dead code. ## Evidence Stage api-server env vars (no `AMBIENT_API_TOKEN`): ``` AMBIENT_ENV: production GRPC_SERVICE_ACCOUNT: {secretKeyRef: {key: clientId, name: ambient-api-server}} ``` Stage api-server startup logs — no bearer token init messages, confirming early return: ``` I0423 23:54:22.927314 Enabling JWT authentication middleware # no "Service token auth enabled" or "OIDC service account" line ``` Runner pods still getting `PERMISSION_DENIED` on `WatchSessionMessages` because the interceptor that would set `CallerTypeService` was never registered. ## Fix Change `init()` guard from `if token == ""` to `if token == "" && serviceAccount == ""` — register interceptors when either env var is set. ## Test plan - [ ] After merge + deploy, verify api-server logs show `OIDC service account username: ocm-ams-service` - [ ] Create session, tail runner logs — no `PERMISSION_DENIED` on `WatchSessionMessages` 🤖 Generated with [Claude Code](https://claude.ai/code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated gRPC authentication middleware initialization to support multiple authentication configuration options simultaneously. Bearer token interceptors are now enabled based on the presence of either authentication method, providing greater flexibility in authentication setup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: user <u@example.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ocm-ams-service), not the session creator's usernameWatchSessionMessagesrejects the call withPERMISSION_DENIEDbecause the JWT username doesn't matchsession.CreatedByUserIdGRPC_SERVICE_ACCOUNTenv var: when the JWT'spreferred_usernamematches this value, the pre-auth interceptor setsCallerTypeService, bypassing per-user ownership checks — same as staticAMBIENT_API_TOKENcallsambient-api-serversecret'sclientIdkey (same OIDC client ID the CP uses),optional: trueTest plan
acpctl startPERMISSION_DENIEDerrors onWatchSessionMessages🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores