Add client-level default token audience config - #4198
Conversation
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (31)
📝 WalkthroughWalkthroughAdds configurable default audiences for unbound access tokens, propagates the setting through OAuth configuration, applies it across grant handlers with client ID fallback, exposes it in the console, updates documentation, and adds unit and integration coverage. ChangesDefault Audience Configuration
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant OAuthClient
participant GrantHandler
participant AccessTokenBuilder
OAuthClient->>GrantHandler: Provide OAuth app configuration
GrantHandler->>OAuthClient: ResolveDefaultAudiences(clientID)
OAuthClient-->>GrantHandler: Default audience or clientID fallback
GrantHandler->>AccessTokenBuilder: Build token with resolved audiences
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
thiva-k
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integration/oauth/token/default_resource_server_test.go (1)
237-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDRY up request helpers.
requestClientCredentialsAsduplicates the HTTP request logic of the existingrequestClientCredentialsmethod (lines 131-153). You could refactorrequestClientCredentialsto delegate to this new helper to eliminate the duplication.♻️ Proposed refactor (outside this segment)
Update the existing
requestClientCredentialsmethod (around line 131) to reuse your new helper:-func (s *DefaultResourceServerTestSuite) requestClientCredentials(scope, resource string) (int, map[string]interface{}) { - form := url.Values{} - form.Set("grant_type", "client_credentials") - if scope != "" { - form.Set("scope", scope) - } - if resource != "" { - form.Set("resource", resource) - } - - req, err := http.NewRequest(http.MethodPost, testServerURL+"/oauth2/token", strings.NewReader(form.Encode())) - s.Require().NoError(err) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.SetBasicAuth(defaultRSTestClientID, defaultRSTestClientSecret) - - resp, err := s.client.Do(req) - s.Require().NoError(err) - defer resp.Body.Close() - - var respBody map[string]interface{} - s.Require().NoError(json.NewDecoder(resp.Body).Decode(&respBody)) - return resp.StatusCode, respBody -} +func (s *DefaultResourceServerTestSuite) requestClientCredentials(scope, resource string) (int, map[string]interface{}) { + return s.requestClientCredentialsAs(defaultRSTestClientID, defaultRSTestClientSecret, scope, resource) +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/oauth/token/default_resource_server_test.go` around lines 237 - 261, Refactor the existing requestClientCredentials method to delegate to requestClientCredentialsAs with its current client ID, client secret, scope, and no resource value. Remove the duplicated request construction and response decoding logic while preserving requestClientCredentials’ existing behavior and return values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/guides/key-concepts/tokens.mdx`:
- Line 19: Update the unbound-token audience rule in the relevant token
documentation bullet to use the configured default audience, falling back to
client_id, and change the condition from “without resource or permission scopes”
to “without resource and permission scopes.” Keep the existing audience behavior
for other token categories unchanged.
---
Nitpick comments:
In `@tests/integration/oauth/token/default_resource_server_test.go`:
- Around line 237-261: Refactor the existing requestClientCredentials method to
delegate to requestClientCredentialsAs with its current client ID, client
secret, scope, and no resource value. Remove the duplicated request construction
and response decoding logic while preserving requestClientCredentials’ existing
behavior and return values.
🪄 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: Pro Plus
Run ID: af5cd535-d365-4076-a4f1-10e72a5a7c40
📒 Files selected for processing (30)
api/agent.yamlapi/application.yamlbackend/internal/inboundclient/service.gobackend/internal/inboundclient/service_test.gobackend/internal/oauth/oauth2/granthandlers/authorization_code.gobackend/internal/oauth/oauth2/granthandlers/ciba.gobackend/internal/oauth/oauth2/granthandlers/ciba_test.gobackend/internal/oauth/oauth2/granthandlers/client_credentials.gobackend/internal/oauth/oauth2/granthandlers/client_credentials_test.gobackend/internal/oauth/oauth2/granthandlers/jwt_bearer.gobackend/internal/oauth/oauth2/granthandlers/token_exchange.gobackend/pkg/thunderidengine/providers/model.gobackend/pkg/thunderidengine/providers/oauth_client.gobackend/pkg/thunderidengine/providers/oauth_client_test.godocs/content/guides/getting-started/configuration.mdxdocs/content/guides/guides/agents/agent-authentication.mdxdocs/content/guides/guides/identity-providers/token-exchange-idp.mdxdocs/content/guides/guides/protocols/oauth-oidc/authorization-code.mdxdocs/content/guides/guides/protocols/oauth-oidc/backchannel-authentication.mdxdocs/content/guides/guides/protocols/oauth-oidc/client-credentials.mdxdocs/content/guides/guides/protocols/oauth-oidc/resource-indicators.mdxdocs/content/guides/guides/protocols/oauth-oidc/token-exchange.mdxdocs/content/guides/key-concepts/tokens.mdxfrontend/apps/console/src/features/agents/components/edit-agent/advanced-settings/EditAdvancedSettings.tsxfrontend/apps/console/src/features/applications/components/edit-application/advanced-settings/AudienceSection.tsxfrontend/apps/console/src/features/applications/components/edit-application/advanced-settings/EditAdvancedSettings.tsxfrontend/apps/console/src/features/applications/components/edit-application/advanced-settings/__tests__/AudienceSection.test.tsxfrontend/apps/console/src/features/applications/models/token.tsfrontend/packages/i18n/src/locales/en-US.tstests/integration/oauth/token/default_resource_server_test.go
305efec to
9556bb9
Compare
9556bb9 to
6ab6d6d
Compare
|
@thiva-k Should defaultAudience live under the Token tab rather than Advanced Settings? |
Keeping it under advanced settings for now since the token section is planned to be fully revamped |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
3 participants
Purpose
Fixes #3725. Access tokens that aren't bound to a resource server (OIDC-only or scopeless requests) defaulted their
audclaim to theclient_id, which is ambiguous and deviates from RFC 9068 (the client isn't the audience). This adds a configurable default audience per application/agent for exactly those unbound tokens.Approach
token.accessToken.defaultAudienceon OAuth applications and agents.resourceparameter and no permission scopes), the access token'sauduses this value, falling back toclient_idwhen unset. Resource-bound tokens are unaffected; they still get the resource server identifier. Precedence: explicitresource→defaultResourceServer→ per-appdefaultAudience→client_id.authorization_code,client_credentials,jwt_bearer,token_exchange,ciba); refresh tokens inherit the original binding.Related Issues
audvalue of an access token is set to the client_id, which is ambiguous #3725Summary by CodeRabbit
New Features
defaultAudienceconfiguration for access tokens when no resource server is targeted (aud falls back to client ID when unset).Documentation
audclaim behavior.Tests