Extend Agent Authorization Protocol to organizations#3228
Open
HasiniSama wants to merge 1 commit intowso2-extensions:masterfrom
Open
Extend Agent Authorization Protocol to organizations#3228HasiniSama wants to merge 1 commit intowso2-extensions:masterfrom
HasiniSama wants to merge 1 commit intowso2-extensions:masterfrom
Conversation
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 | ||
| #### Log Improvement Suggestion No: 2 |
📝 WalkthroughWalkthroughThis PR extends the OAuth module to support agent identity resolution in the authorization utility. It declares a new dependency on the organization agent sharing library, updates the organization management version, and implements agent-specific associated-user-id lookup logic in the AuthzUtil class. ChangesAgent Identity Support in OAuth
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/AuthzUtil.java (1)
210-225: ⚡ Quick winAdd focused tests for the new agent branch.
Please add coverage for: agent-store success, agent-store not-associated (client exception), and non-agent-store fallback path. This is an auth-critical branch and worth locking down with tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/AuthzUtil.java` around lines 210 - 225, Add unit tests covering the new agent-specific branch in AuthzUtil: create tests that (1) simulate IdentityUtil.isAgentIdentityEnabled() true and IdentityUtil.getAgentIdentityUserstoreName() matching authenticatedUser.getUserStoreDomain() and assert OrganizationSharedAgentUtil.getAgentIdOfAssociatedAgentByOrgId(associatedUserId, authenticatedUser.getAccessingOrganization()) returns an agent id (success path); (2) simulate the same agent-store match but have OrganizationSharedAgentUtil.getAgentIdOfAssociatedAgentByOrgId(...) return empty and assert AuthzUtil throws IdentityOAuth2ClientException (not-associated path); and (3) simulate either isAgentIdentityEnabled false or agent-store name not matching authenticatedUser.getUserStoreDomain() and assert the method falls back to the existing non-agent sharing logic (fallback path). Use mocking for IdentityUtil and OrganizationSharedAgentUtil and reference methods: IdentityUtil.isAgentIdentityEnabled(), IdentityUtil.getAgentIdentityUserstoreName(), authenticatedUser.getUserStoreDomain(), OrganizationSharedAgentUtil.getAgentIdOfAssociatedAgentByOrgId(...), and verify thrown types and returned values accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/AuthzUtil.java`:
- Around line 210-225: Add unit tests covering the new agent-specific branch in
AuthzUtil: create tests that (1) simulate IdentityUtil.isAgentIdentityEnabled()
true and IdentityUtil.getAgentIdentityUserstoreName() matching
authenticatedUser.getUserStoreDomain() and assert
OrganizationSharedAgentUtil.getAgentIdOfAssociatedAgentByOrgId(associatedUserId,
authenticatedUser.getAccessingOrganization()) returns an agent id (success
path); (2) simulate the same agent-store match but have
OrganizationSharedAgentUtil.getAgentIdOfAssociatedAgentByOrgId(...) return empty
and assert AuthzUtil throws IdentityOAuth2ClientException (not-associated path);
and (3) simulate either isAgentIdentityEnabled false or agent-store name not
matching authenticatedUser.getUserStoreDomain() and assert the method falls back
to the existing non-agent sharing logic (fallback path). Use mocking for
IdentityUtil and OrganizationSharedAgentUtil and reference methods:
IdentityUtil.isAgentIdentityEnabled(),
IdentityUtil.getAgentIdentityUserstoreName(),
authenticatedUser.getUserStoreDomain(),
OrganizationSharedAgentUtil.getAgentIdOfAssociatedAgentByOrgId(...), and verify
thrown types and returned values accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5c8311d7-bb38-4247-8cf4-cf81dd287eeb
📒 Files selected for processing (3)
components/org.wso2.carbon.identity.oauth/pom.xmlcomponents/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/AuthzUtil.javapom.xml
HasiniSama
commented
May 6, 2026
| throw new IdentityOAuth2Exception("Error while resolving shared agent ID", e); | ||
| } | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This should be combined with the below user sharing logic
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.
Proposed changes in this pull request
$subject