Add integration tests for performing authorizations for sub organization applications #26359

ShanChathusanda93 · 2025-12-02T15:01:42Z

Changes in this PR

$subject

Summary by CodeRabbit

Tests
- Added a new end-to-end integration test suite for OAuth2 authorization of sub-organization applications and expanded test setup/cleanup across org scopes.
New Features
- Introduced organization-scoped REST flows for application management, OIDC inbound retrieval, organization lifecycle, and SCIM role operations.
- Added an organization-switch OAuth2 grant constant.
Documentation / Resources
- Added JSON mappings of API endpoints to required permissions and updated test configuration.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-02T15:01:59Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds organization-scoped REST methods and helpers (OAuth2, Org management, SCIM), a new OAuth2 grant constant, two permissions JSON resources, a TestNG entry, and a comprehensive integration test exercising sub-organization OAuth2 application and token flows.

Changes

Cohort / File(s)	Summary
OAuth2 REST client (org-scoped) `modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java`	Added org-scoped methods: `createOrganizationApplication(ApplicationModel, String)`, `getOrganizationApplication(String, String)`, `getOIDCInboundDetailsOfOrganizationApp(String, String)` and private helper `getConfigOfOrganizationApp(...)`. Mirrors non-org endpoints, uses Bearer token.
Test abstract utilities `modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java`	Added `addOrganizationApplication(String, ApplicationConfig, String)` and `getOIDCInboundDetailsOfOrganizationApplication(String, String)` delegating to org-scoped REST client methods.
Organization management client `modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java`	Added `addOrganizationWithToken(String, String, String)` and `deleteOrganizationWithToken(String, String)` to create/delete organizations using an explicit access token.
SCIM2 client (org role ops) `modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/SCIM2RestClient.java`	Added `updateOrganizationUserRole(PatchOperationRequestObject, String, String)` and `addOrganizationV2Roles(RoleV2, String)` for org-scoped role update/creation using Bearer token.
Integration test `modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2AuthorizationForSubOrganizationAppsTestCase.java`	New end-to-end integration test class exercising sub-organization app flows: org creation, app creation/sharing, M2M tokens, user/role provisioning, organization-switch grant flows, opaque vs JWT tokens, introspection, API calls, and cleanup.
Constants `modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/OAuth2Constant.java`	Added `public static final String OAUTH2_GRANT_TYPE_ORGANIZATION_SWITCH = "organization_switch";`.
Test resources & config `modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/management-app-authorized-apis.json`, `modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/organization-app-authorized-apis.json`, `modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml`	Added JSON mappings for management and organization API permissions; added `OAuth2AuthorizationForSubOrganizationAppsTestCase` entry to `testng.xml`.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Test as Integration Test
participant AuthServer as Authorization Server
participant OAuthClient as OAuth2RestClient
participant OrgClient as OrgMgtRestClient
participant SCIMClient as SCIM2RestClient
participant API as Management/API Resource

rect rgb(235,245,255)
Note over Test,AuthServer: Setup & configuration
end

Test->>AuthServer: request M2M token
AuthServer-->>Test: M2M token

Test->>OrgClient: addOrganizationWithToken(orgName, orgHandle, m2mToken)
OrgClient->>AuthServer: POST /o/admin/organizations (Bearer m2mToken)
AuthServer-->>OrgClient: 201 Created (orgId)
OrgClient-->>Test: orgId

Test->>OAuthClient: createOrganizationApplication(appModel, switchedAccessToken)
OAuthClient->>AuthServer: POST /o/api/server/v1/applications (Bearer switchedAccessToken)
AuthServer-->>OAuthClient: 201 Created (Location → appId)
OAuthClient-->>Test: appId

Test->>SCIMClient: addOrganizationV2Roles(role, accessToken)
SCIMClient->>AuthServer: POST /o/scim2/Roles (Bearer accessToken)
AuthServer-->>SCIMClient: 201 Created (roleId)
SCIMClient-->>Test: roleId

Test->>AuthServer: exchange for organization-switched token (grant_type=organization_switch)
AuthServer-->>Test: access token (opaque or JWT)

Test->>API: call management API (Authorization: Bearer token)
API->>AuthServer: introspect/validate token
AuthServer-->>API: introspection / claims
API-->>Test: 200 OK / resource response

Note over Test: Cleanup (delete app, roles, org)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through tokens, roles, and grants,
Sub-org apps sprouted in tidy pants,
REST calls scurried, introspection sang,
JWT dots and opaque tails rang,
A tiny rabbit cheers these integration plants.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 32.61% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add integration tests for performing authorizations for sub organization applications' directly and clearly describes the main change—a new comprehensive integration test class (OAuth2AuthorizationForSubOrganizationAppsTestCase) with supporting helper methods and resources for OAuth2 authorization workflows in sub-organization contexts.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 4

🧹 Nitpick comments (10)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/OAuth2Constant.java (1)
31-31: Consider using this constant in OrgMgtRestClient.java.

The new constant is correctly defined. However, OrgMgtRestClient.java at line 274 uses a hardcoded string "organization_switch" instead of referencing this constant:
urlParameters.add(new BasicNameValuePair(OAuth2Constant.GRANT_TYPE_NAME, "organization_switch"));
Consider updating to use the constant for consistency:
-urlParameters.add(new BasicNameValuePair(OAuth2Constant.GRANT_TYPE_NAME, "organization_switch"));
+urlParameters.add(new BasicNameValuePair(OAuth2Constant.GRANT_TYPE_NAME, OAuth2Constant.OAUTH2_GRANT_TYPE_ORGANIZATION_SWITCH));
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java (1)
272-332: Consider extracting common application configuration logic.

This method largely duplicates the logic in addApplication(ApplicationConfig) (lines 192-241). The only differences are the application name source and the REST client methods used.

Consider extracting the common application model building logic into a private helper method to reduce duplication:
private ApplicationModel buildApplicationModel(String applicationName, ApplicationConfig applicationConfig) {
    // Common configuration building logic
}
Then both methods can use this helper, with only the REST client call differing.
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java (7)
56-59: Avoid wildcard imports for clarity and to prevent namespace conflicts.

Using wildcard imports (import java.util.*; and import static org.testng.Assert.*;) can lead to ambiguity and makes it harder to track which classes are actually used.

Consider explicit imports:
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
100-113: Static mutable fields may cause issues in parallel test execution.

CLIENT_ID, CLIENT_SECRET, ORG_APP_CLIENT_ID, and ORG_APP_CLIENT_SECRET are static but mutated during test setup. If tests run in parallel, this could lead to race conditions.

Consider making these instance fields instead:
-    private static String CLIENT_ID;
-    private static String CLIENT_SECRET;
-    private static String ORG_APP_CLIENT_ID;
-    private static String ORG_APP_CLIENT_SECRET;
+    private String clientId;
+    private String clientSecret;
+    private String orgAppClientId;
+    private String orgAppClientSecret;
264-286: Created resources are not cleaned up after test.

The test creates an organization application (ORGANIZATION_TEST_APP_NAME) but doesn't delete it in cleanup. This could leave test artifacts and affect subsequent test runs.

Consider storing the created orgAppId as a class field and deleting it in atEnd():
// In atEnd():
if (testOrgAppId != null) {
    oAuth2RestClient.deleteApplication(testOrgAppId);
}
288-317: Created test user is not cleaned up.

Similar to the application, the user created with TEST_ORG_END_USER_USERNAME is not deleted after the test completes.

Store the user ID and delete it in the cleanup phase to maintain test isolation.

384-455: Significant code duplication between updateAuthorizedAPIs and updateOrganizationAppAuthorizedAPIs.

These two methods have nearly identical logic, differing only in the path construction and headers used. Consider extracting common logic.
private void updateAuthorizedAPIsInternal(String appId, JSONObject authorizedAPIs, 
        String apiBasePath, String appBasePath, Header[] headers) throws JSONException, IOException {
    
    RestBaseClient restBaseClient = new RestBaseClient();
    for (Iterator<String> apiNameIterator = authorizedAPIs.keys(); apiNameIterator.hasNext(); ) {
        String apiName = apiNameIterator.next();
        Object requiredScopes = authorizedAPIs.get(apiName);
        // ... common logic
    }
}
551-553: Redundant EntityUtils.consume after EntityUtils.toString.

EntityUtils.toString() already consumes the entity, making the subsequent EntityUtils.consume() call unnecessary.
         HttpResponse response = client.execute(httpPost);
         String responseString = EntityUtils.toString(response.getEntity(), "UTF-8");
-        EntityUtils.consume(response.getEntity());
         JSONParser parser = new JSONParser();
596-604: getAuthorizationCodeFromURL returns null silently on failure.

Returning null when the authorization code is not found could lead to cryptic NullPointerExceptions downstream. Consider throwing an exception with a descriptive message.
 private String getAuthorizationCodeFromURL(String location) {
 
     URI uri = URI.create(location);
     return URLEncodedUtils.parse(uri, StandardCharsets.UTF_8).stream()
             .filter(param -> "code".equals(param.getName()))
             .map(NameValuePair::getValue)
             .findFirst()
-            .orElse(null);
+            .orElseThrow(() -> new IllegalStateException(
+                "Authorization code not found in URL: " + location));
 }
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java (1)
531-541: Consider adding error handling for failed requests.

Similar to getConfig, this method doesn't validate the response status code before returning the entity. A failed request would return an error JSON that would fail deserialization upstream. While this is consistent with existing code, consider improving both.
 private String getConfigOrOrganizationApp(String appId, String inboundType,
                                           String accessToken) throws Exception {

     String endPointUrl = subOrgApplicationManagementApiBasePath + PATH_SEPARATOR + appId + INBOUND_PROTOCOLS_BASE_PATH +
             PATH_SEPARATOR + inboundType;

     try (CloseableHttpResponse response = getResponseOfHttpGet(endPointUrl,
             getHeadersWithBearerToken(accessToken))) {
+        if (response.getStatusLine().getStatusCode() >= 400) {
+            throw new RuntimeException("Failed to get inbound config. Status: " + 
+                response.getStatusLine().getStatusCode());
+        }
         return EntityUtils.toString(response.getEntity());
     }
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb17c20 and 666cb80.

📒 Files selected for processing (8)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java (3 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java (8 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java (2 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/SCIM2RestClient.java (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/OAuth2Constant.java (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/management-app-authorized-apis.json (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/organization-app-authorized-apis.json (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java (1)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/OAuth2Constant.java (1)

OAuth2Constant (24-150)

🔇 Additional comments (13)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/SCIM2RestClient.java (1)

522-541: LGTM!

The new updateOrganizationUserRole method correctly follows the existing patterns in this class for organization-scoped operations. It properly uses the getSubOrgRolesV2Path() endpoint builder and bearer token authentication consistent with similar methods like updateUsersOfRoleV2InSubOrg.

modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/management-app-authorized-apis.json (1)

1-35: LGTM!

The JSON configuration correctly maps management API endpoints to their required internal permissions. The structure is well-organized and follows a consistent permission naming pattern aligned with WSO2 Identity Server conventions.

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java (2)

137-154: LGTM!

The new addOrganizationWithToken method correctly follows the existing pattern of the addOrganization overloads, allowing tests to provide an externally managed token for organization creation.

210-224: LGTM!

The new deleteOrganizationWithToken method correctly mirrors the existing deleteOrganization method while accepting an externally provided access token. The HTTP 204 NO_CONTENT assertion is properly included.

modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/organization-app-authorized-apis.json (1)

10-15: Verify the role permissions scope difference is intentional.

The /o/scim2/Roles endpoint here includes 4 permissions (view, create, update, delete), while the same endpoint in management-app-authorized-apis.json includes 7 permissions (additionally: internal_org_role_mgt_permissions_update, internal_org_role_mgt_users_update, internal_org_role_mgt_groups_update).

Please confirm this difference is intentional based on the distinct use cases of these two configuration files (organization-app vs management-app contexts).

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java (3)

285-287: Null config fallback returns a non-organization application.

When applicationConfig is null, this method returns getBasicOAuthApplication(CALLBACK_URL) which creates a regular application, not an organization-scoped one. This may be unexpected since the method name suggests organization-specific behavior.

Consider either:

Creating an organization-scoped basic application when config is null

Throwing an exception if config is null for organization applications

Documenting this behavior in the Javadoc

432-443: LGTM!

The new addOrganizationRole method follows the established pattern of the existing addRole method, cleanly delegating to the organization-scoped REST client method.

486-499: LGTM!

The new getOIDCInboundDetailsOfOrganizationApplication method cleanly mirrors the existing getOIDCInboundDetailsOfApplication method for organization-scoped operations.

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java (1)

115-163: LGTM - Well-structured test setup.

The setup method properly initializes HTTP clients with cookie handling, creates necessary REST clients, and establishes the organization hierarchy required for testing. The disabled redirect strategy is appropriate for OAuth flow testing where manual redirect handling is needed.

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java (4)

170-192: LGTM - Consistent with existing pattern.

The createOrganizationApplication method follows the same pattern as createApplication, appropriately using Bearer token authentication for organization-scoped operations.

279-299: LGTM - Follows existing pattern correctly.

The getOrganizationApplication method correctly implements organization-scoped application retrieval using Bearer token authentication, consistent with the existing getApplication method pattern.

490-504: LGTM - Proper delegation to private helper.

The getOIDCInboundDetailsOfOrganizationApp method correctly delegates to getConfigOrOrganizationApp and deserializes the response, following the same pattern as getOIDCInboundDetails.

891-898: LGTM - Consistent path construction.

The getOrganizationSCIM2RoleV2Path method follows the established pattern for constructing organization-scoped paths, correctly handling both super tenant and regular tenant domains.

...wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java

coderabbitai · 2025-12-02T15:05:12Z

...wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java

+                assertEquals(appAuthorizedAPIsResponse.getStatusLine().getStatusCode(), HttpStatus.SC_OK,
+                        String.format("Authorized APIs update failed for application with ID: %s for API: %s.",
+                                appAuthorizedAPIsResponse, apiName));


⚠️ Potential issue | 🟡 Minor

Error message logs response object instead of response body.

The error message includes appAuthorizedAPIsResponse (the response object) instead of the actual response body, which won't provide useful debugging information.

- assertEquals(appAuthorizedAPIsResponse.getStatusLine().getStatusCode(), HttpStatus.SC_OK, - String.format("Authorized APIs update failed for application with ID: %s for API: %s.", - appAuthorizedAPIsResponse, apiName)); + assertEquals(appAuthorizedAPIsResponse.getStatusLine().getStatusCode(), HttpStatus.SC_OK, + String.format("Authorized APIs update failed for application with ID: %s for API: %s.", + appId, apiName));

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

assertEquals(appAuthorizedAPIsResponse.getStatusLine().getStatusCode(), HttpStatus.SC_OK,

String.format("Authorized APIs update failed for application with ID: %s for API: %s.",

appAuthorizedAPIsResponse, apiName));

assertEquals(appAuthorizedAPIsResponse.getStatusLine().getStatusCode(), HttpStatus.SC_OK,

String.format("Authorized APIs update failed for application with ID: %s for API: %s.",

appId, apiName));

🤖 Prompt for AI Agents

In modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java around lines 413-415, the assertion error message logs the response object (appAuthorizedAPIsResponse) instead of the response body; update the assertion to extract the response body string (the HTTP entity/content) from appAuthorizedAPIsResponse and include that body in the formatted error message so failures show the actual server response content rather than the response object reference.

...s-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java

Copilot

Pull request overview

This PR adds comprehensive integration tests for OAuth2 authorization code grant flows in sub-organization contexts. The tests cover the complete workflow including organization management, application provisioning, user management, role assignment, and token exchange operations.

Key Changes

Added new integration test case OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase to verify OAuth2 authorization code grant for sub-organization applications
Introduced organization-scoped operations in REST clients (OAuth2RestClient, SCIM2RestClient, OrgMgtRestClient) to support token-based authentication for organization-level operations
Added OAUTH2_GRANT_TYPE_ORGANIZATION_SWITCH constant to support organization switching grant type
Added JSON configuration files defining authorized APIs for management and organization applications

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java	New comprehensive integration test covering authorization code flow for sub-organization apps, including organization creation, app provisioning, user management, and API authorization
OAuth2Constant.java	Added constant for organization_switch grant type
SCIM2RestClient.java	Added `updateOrganizationUserRole` method to support role updates with token authentication
OrgMgtRestClient.java	Added token-based methods `addOrganizationWithToken` and `deleteOrganizationWithToken` for organization management
OAuth2RestClient.java	Added organization-specific methods for application, role, and OIDC configuration management with token authentication
OAuth2ServiceAbstractIntegrationTest.java	Added helper methods `addOrganizationApplication`, `addOrganizationRole`, and `getOIDCInboundDetailsOfOrganizationApplication` for organization-level operations
organization-app-authorized-apis.json	Configuration file defining authorized API scopes for organization applications
management-app-authorized-apis.json	Configuration file defining authorized API scopes for management applications

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-03T13:38:22Z

...s-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java

        }
    }

+    private String getConfigOrOrganizationApp(String appId, String inboundType,


The method name getConfigOrOrganizationApp contains a typo. It should be getConfigOfOrganizationApp (using "Of" instead of "Or"). The current name suggests "or" as in a boolean operator rather than "of" which correctly expresses possession/relationship.

Copilot · 2025-12-03T13:38:22Z

...est/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java

+     * Get Application oidc inbound configuration details with a given application id of an organization.
+     *
+     * @param appId Application ID.
+     * @param switchedAccessToken Authorized token to get the OIDC inbound configurations or an application.


The JavaDoc contains a typo. "configurations or an application" should be "configurations of an application". The word "or" should be "of".

Suggested change

* @param switchedAccessToken Authorized token to get the OIDC inbound configurations or an application.

* @param switchedAccessToken Authorized token to get the OIDC inbound configurations of an application.

Copilot · 2025-12-03T13:38:23Z

...est/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java

+            oidcConfig.refreshToken(refreshTokenConfiguration);
+        }
+
+        if (applicationConfig.getAudienceList() != null && !applicationConfig.getRequestedClaimList().isEmpty()) {


The condition on line 313 checks !applicationConfig.getRequestedClaimList().isEmpty() but this is inconsistent with the logic. The audience list should be added to the ID token configuration when the audience list is not null, regardless of whether the requested claim list is empty. The condition should check !applicationConfig.getAudienceList().isEmpty() instead, or simply set the audience if the list is not null.

Suggested change

if (applicationConfig.getAudienceList() != null && !applicationConfig.getRequestedClaimList().isEmpty()) {

if (applicationConfig.getAudienceList() != null && !applicationConfig.getAudienceList().isEmpty()) {

Copilot · 2025-12-03T13:38:23Z

...s-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java

+     * Create a v2 role in the organization.
+     *
+     * @param role an instance of RoleV2.
+     * @param accessToken Authorized token to create v2 roles in an organization.
+     * @return the roleID.


[nitpick] JavaDoc inconsistency: The documentation style is inconsistent. This method uses lowercase "v2" while other similar methods use "V2" (uppercase). For consistency with the existing createV2Roles method (line 195), this should say "Create a V2 role in the organization." The return value documentation should also say "the role ID" (with space) to match the pattern in line 198.

Suggested change

* Create a v2 role in the organization.

*

* @param role an instance of RoleV2.

* @param accessToken Authorized token to create v2 roles in an organization.

* @return the roleID.

* Create a V2 role in the organization.

*

* @param role an instance of RoleV2.

* @param accessToken Authorized token to create V2 roles in an organization.

* @return the role ID.

...wso2/identity/integration/test/oauth2/OAuth2AuthorizationForSubOrganizationAppsTestCase.java

Copilot · 2025-12-03T13:38:24Z

...est/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java

+                                                                                     String switchedAccessToken)
+            throws Exception {
+
+        return restClient.getOIDCInboundDetailsOfOrganizationApp(appId,  switchedAccessToken);


Double space detected: There are two spaces between appId, and switchedAccessToken in the method call. This should be a single space for consistency with coding standards.

Suggested change

return restClient.getOIDCInboundDetailsOfOrganizationApp(appId, switchedAccessToken);

return restClient.getOIDCInboundDetailsOfOrganizationApp(appId, switchedAccessToken);

No two spaces

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-04T14:26:02Z

...wso2/identity/integration/test/oauth2/OAuth2AuthorizationForSubOrganizationAppsTestCase.java

+        headerList[1] = new BasicHeader(AUTHORIZATION_ATTRIBUTE, BEARER_TOKEN_AUTHORIZATION_ATTRIBUTE
+                + switchedM2MToken);


[nitpick] Extra space after the token in the Authorization header construction. The code currently has two spaces between "BEARER_TOKEN_AUTHORIZATION_ATTRIBUTE" and "switchedM2MToken", but only one is needed.

Suggested change

headerList[1] = new BasicHeader(AUTHORIZATION_ATTRIBUTE, BEARER_TOKEN_AUTHORIZATION_ATTRIBUTE

+ switchedM2MToken);

headerList[1] = new BasicHeader(AUTHORIZATION_ATTRIBUTE, BEARER_TOKEN_AUTHORIZATION_ATTRIBUTE.trim()

+ " " + switchedM2MToken.trim());

Here [1] is having only one space

[1]

product-is/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/RestBaseClient.java

Line 51 in a820658

public static final String BEARER_TOKEN_AUTHORIZATION_ATTRIBUTE = "Bearer ";

Copilot · 2025-12-04T14:26:03Z

...est/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java

+     * Get Application oidc inbound configuration details with a given application id of an organization.
+     *
+     * @param appId Application ID.
+     * @param switchedAccessToken Authorized token to get the OIDC inbound configurations or an application.


Typo in the comment: 'or' should be 'of'. The comment reads "Authorized token to get the OIDC inbound configurations or an application" but should be "Authorized token to get the OIDC inbound configurations of an application".

Suggested change

* @param switchedAccessToken Authorized token to get the OIDC inbound configurations or an application.

* @param switchedAccessToken Authorized token to get the OIDC inbound configurations of an application.

...wso2/identity/integration/test/oauth2/OAuth2AuthorizationForSubOrganizationAppsTestCase.java

...wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (8)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java (4)
288-317: Avoid shadowing scim2RestClient field and ensure the org-scoped client is closed

Inside testInvokeUserMgtAPIsFromOrganizationAppToken you declare:
SCIM2RestClient scim2RestClient = new SCIM2RestClient(serverURL, tenantInfo);
This shadows the class field scim2RestClient and is never closed, which:

Makes the code harder to read (two different scim2RestClients in the same class).

Leaks the underlying HTTP client for the org-scoped instance.

Consider renaming and closing it:
-        // Creating the URLs with the tenant details provided.
-        SCIM2RestClient scim2RestClient = new SCIM2RestClient(serverURL, tenantInfo);
+        // Creating the URLs with the tenant details provided.
+        SCIM2RestClient orgScim2RestClient = new SCIM2RestClient(serverURL, tenantInfo);
@@
-        String subOrgUserId = scim2RestClient.createSubOrgUser(endUser, orgAppAccessToken);
+        String subOrgUserId = orgScim2RestClient.createSubOrgUser(endUser, orgAppAccessToken);
@@
-        org.json.simple.JSONObject subOrgUserObject = scim2RestClient.getSubOrgUser(subOrgUserId, null,
-                orgAppAccessToken);
+        org.json.simple.JSONObject subOrgUserObject = orgScim2RestClient.getSubOrgUser(subOrgUserId, null,
+                orgAppAccessToken);
@@
-        assertTrue(subOrgUserObject.get("emails").toString().contains(TEST_ORG_END_USER_EMAIL), "Expected " +
-                "user emails are not present in the response.");
+        assertTrue(subOrgUserObject.get("emails").toString().contains(TEST_ORG_END_USER_EMAIL), "Expected " +
+                "user emails are not present in the response.");
+        orgScim2RestClient.closeHttpClient();
(Or use a try/finally if you want to be defensive against assertion failures.)

1-1: Add standard license header to the new test class

This new test class is missing the usual WSO2 Apache 2.0 license header that appears in other test classes in this package. Please add the standard header for consistency and compliance.

165-172: Harden teardown: close all clients and guard against partial initialization

atEnd() currently:

Deletes the organization but does not close orgMgtRestClient.

Assumes all fields (organizationId, accessToken, clients) are non-null and will succeed; an exception in one operation will skip the rest.

Suggested refactor:

Close orgMgtRestClient as well.

Wrap each cleanup step in its own try-catch with null checks so one failure doesn’t prevent others.
    @AfterClass(alwaysRun = true)
    public void atEnd() throws Exception {

-        orgMgtRestClient.deleteOrganizationWithToken(organizationId, accessToken);
-        restClient.closeHttpClient();
-        scim2RestClient.closeHttpClient();
-        client.close();
+        try {
+            if (organizationId != null && accessToken != null && orgMgtRestClient != null) {
+                orgMgtRestClient.deleteOrganizationWithToken(organizationId, accessToken);
+            }
+        } catch (Exception e) {
+            // Log if you have a logger; keep going with other cleanups.
+        }
+        try {
+            if (orgMgtRestClient != null) {
+                orgMgtRestClient.closeHttpClient();
+            }
+        } catch (Exception ignored) { }
+        try {
+            if (restClient != null) {
+                restClient.closeHttpClient();
+            }
+        } catch (Exception ignored) { }
+        try {
+            if (scim2RestClient != null) {
+                scim2RestClient.closeHttpClient();
+            }
+        } catch (Exception ignored) { }
+        try {
+            if (client != null) {
+                client.close();
+            }
+        } catch (Exception ignored) { }
    }
384-418: Make authorized-API updates more robust and fix assertion message

In updateAuthorizedAPIs:

Assumes at least one API resource exists

If the filter doesn’t match any API resource, this line will throw:
apiUUID = apiResourceResponseBody.getJSONArray(API_RESOURCES).getJSONObject(0).getString(ID);
Consider asserting that the array is non-empty so failures are explicit:
-                JSONObject apiResourceResponseBody =
-                        new JSONObject(EntityUtils.toString(apiResourceResponse.getEntity()));
-                apiUUID = apiResourceResponseBody.getJSONArray(API_RESOURCES).getJSONObject(0).getString(ID);
+                JSONObject apiResourceResponseBody =
+                        new JSONObject(EntityUtils.toString(apiResourceResponse.getEntity()));
+                if (apiResourceResponseBody.getJSONArray(API_RESOURCES).isEmpty()) {
+                    throw new AssertionError("No API resource found for identifier: " + apiName);
+                }
+                apiUUID = apiResourceResponseBody.getJSONArray(API_RESOURCES).getJSONObject(0).getString(ID);
(Adjust for the exact JSON API; some versions expose length() instead of isEmpty().)

Assertion message logs the response object, not app id

The failure message currently interpolates appAuthorizedAPIsResponse instead of the application id:
String.format("Authorized APIs update failed for application with ID: %s for API: %s.",
        appAuthorizedAPIsResponse, apiName)
This produces an unhelpful string like CloseableHttpResponse@.... Use appId (and, optionally, the response body) instead:
-                assertEquals(appAuthorizedAPIsResponse.getStatusLine().getStatusCode(), HttpStatus.SC_OK,
-                        String.format("Authorized APIs update failed for application with ID: %s for API: %s.",
-                                appAuthorizedAPIsResponse, apiName));
+                String responseBody = EntityUtils.toString(appAuthorizedAPIsResponse.getEntity());
+                assertEquals(appAuthorizedAPIsResponse.getStatusLine().getStatusCode(), HttpStatus.SC_OK,
+                        String.format("Authorized APIs update failed for application with ID: %s for API: %s. " +
+                                      "Response body: %s", appId, apiName, responseBody));
The same two concerns apply to updateOrganizationAppAuthorizedAPIs; it would be good to align both helpers.
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java (2)
272-332: Fix applicationConfig null handling and audience condition in addOrganizationApplication

Two issues here:

Null applicationConfig path creates a root app, not an org app

When applicationConfig is null, the method delegates to getBasicOAuthApplication(CALLBACK_URL), which internally creates a root application via addApplication(application), ignoring both applicationName and switchedAccessToken. That contradicts the method contract (“Add an application to an organization”) and will never produce an org-scoped app in that path.

If you intend to support a “default config” branch, it should still go through restClient.createOrganizationApplication(...), for example:
-        if (applicationConfig == null) {
-            return getBasicOAuthApplication(CALLBACK_URL);
-        }
+        if (applicationConfig == null) {
+            ApplicationModel defaultApp = new ApplicationModel();
+            defaultApp.setName(applicationName);
+
+            OpenIDConnectConfiguration oidcConfig = new OpenIDConnectConfiguration();
+            oidcConfig.setGrantTypes(Collections.singletonList("authorization_code"));
+            oidcConfig.setCallbackURLs(Collections.singletonList(CALLBACK_URL));
+
+            InboundProtocols inboundProtocolsConfig = new InboundProtocols();
+            inboundProtocolsConfig.setOidc(oidcConfig);
+            defaultApp.setInboundProtocolConfiguration(inboundProtocolsConfig);
+
+            String appId = restClient.createOrganizationApplication(defaultApp, switchedAccessToken);
+            return restClient.getOrganizationApplication(appId, switchedAccessToken);
+        }
(Adjust defaults to whatever is appropriate for your org apps.)

Audience configuration is gated on requested-claims emptiness

The condition
if (applicationConfig.getAudienceList() != null && !applicationConfig.getRequestedClaimList().isEmpty()) {
    oidcConfig.idToken(new IdTokenConfiguration().audience(applicationConfig.getAudienceList()));
}
ties audience configuration to requested claims rather than the actual audience list. If requestedClaimList is empty but audienceList is populated, audiences will not be applied.

You likely want the same fix that was suggested earlier for the non-org addApplication path—check the audience list itself:
-        if (applicationConfig.getAudienceList() != null && !applicationConfig.getRequestedClaimList().isEmpty()) {
+        if (applicationConfig.getAudienceList() != null && !applicationConfig.getAudienceList().isEmpty()) {
             oidcConfig.idToken(new IdTokenConfiguration().audience(applicationConfig.getAudienceList()));
         }
Consider applying the same audience-condition fix to the existing addApplication(ApplicationConfig) method for consistency.

486-499: Tidy OIDC org-inbound helper Javadoc and call formatting

The logic of getOIDCInboundDetailsOfOrganizationApplication is fine, but there are two small nits:

Javadoc parameter text: “Authorized token to get the OIDC inbound configurations or an application” should be “of an application”.

Method call has a double space between appId, and switchedAccessToken.

Suggested cleanup:
-     * @param switchedAccessToken Authorized token to get the OIDC inbound configurations or an application.
+     * @param switchedAccessToken Authorized token to get the OIDC inbound configurations of an application.
@@
-        return restClient.getOIDCInboundDetailsOfOrganizationApp(appId,  switchedAccessToken);
+        return restClient.getOIDCInboundDetailsOfOrganizationApp(appId, switchedAccessToken);
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java (2)
211-227: Add error handling and Location-header checks in createOrganizationV2Roles

createOrganizationV2Roles still assumes success and directly indexes the Location header:
String[] locationElements = response.getHeaders(LOCATION_HEADER)[0].toString().split(PATH_SEPARATOR);
return locationElements[locationElements.length - 1];
If the role creation fails (4xx/5xx) or the header is missing, this will throw an ArrayIndexOutOfBoundsException or NullPointerException and obscure the real server error.

Align this method with the robustness of createOrganizationApplication by validating the response and the header:
    public String createOrganizationV2Roles(RoleV2 role, String accessToken) throws IOException {

        String jsonRequest = toJSONString(role);
        try (CloseableHttpResponse response = getResponseOfHttpPost(organizationRoleV2ApiBasePath, jsonRequest,
                getHeadersWithBearerToken(accessToken))) {
-            String[] locationElements = response.getHeaders(LOCATION_HEADER)[0].toString().split(PATH_SEPARATOR);
-            return locationElements[locationElements.length - 1];
+            int status = response.getStatusLine().getStatusCode();
+            if (status < 200 || status >= 300) {
+                String responseBody = EntityUtils.toString(response.getEntity());
+                throw new IOException("Error occurred while creating the role. Status: " + status +
+                        ", Response: " + responseBody);
+            }
+            Header[] locationHeaders = response.getHeaders(LOCATION_HEADER);
+            if (locationHeaders == null || locationHeaders.length == 0) {
+                throw new IOException("Location header missing in role creation response");
+            }
+            String[] locationElements = locationHeaders[0].toString().split(PATH_SEPARATOR);
+            return locationElements[locationElements.length - 1];
        }
    }
490-505: Clarify naming of getConfigOrOrganizationApp and reuse pattern

The org-scoped OIDC helper:
public OpenIDConnectConfiguration getOIDCInboundDetailsOfOrganizationApp(String appId, String accessToken)
delegates to getConfigOrOrganizationApp, whose name is confusing (“Or” instead of “Of”) and doesn’t convey that it’s org-specific inbound-config retrieval:
private String getConfigOrOrganizationApp(String appId, String inboundType, String accessToken)
The implementation itself is fine, but for readability and consistency with getConfig(...) you may want to:

Rename the helper to something like getOrgInboundConfig and fix the “Or/Of” typo.

Narrow its responsibility to “org inbound config” (which is how you currently use it) rather than a vague “config or organization app”.

Example:
-    private String getConfigOrOrganizationApp(String appId, String inboundType,
-                                              String accessToken) throws Exception {
+    private String getOrgInboundConfig(String appId, String inboundType, String accessToken) throws Exception {
@@
-        String responseBody = getConfigOrOrganizationApp(appId, OIDC, accessToken);
+        String responseBody = getOrgInboundConfig(appId, OIDC, accessToken);
This keeps the org/non-org variants parallel (getConfig vs getOrgInboundConfig) and makes future maintenance easier.

🧹 Nitpick comments (1)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java (1)

264-286: Close the temporary OAuth2RestClient to avoid leaking HTTP connections

testInvokeApplicationMgtAPIsFromOrganizationAppToken creates a new OAuth2RestClient instance but never closes its underlying HTTP client:

OAuth2RestClient oAuth2RestClient = new OAuth2RestClient(serverURL, tenantInfo);

Over many tests this can accumulate open connections. Since the test method already throws Exception, you can safely close it in a finally block:

-        OAuth2RestClient oAuth2RestClient = new OAuth2RestClient(serverURL, tenantInfo);
-
-        // Validate application creation
-        ApplicationModel applicationModel = getOrganizationAppModel(ORGANIZATION_TEST_APP_NAME);
-        String orgAppId = oAuth2RestClient.createOrganizationApplication(applicationModel, orgAppAccessToken);
-        ...
+        OAuth2RestClient oAuth2RestClient = new OAuth2RestClient(serverURL, tenantInfo);
+        try {
+            // Validate application creation
+            ApplicationModel applicationModel = getOrganizationAppModel(ORGANIZATION_TEST_APP_NAME);
+            String orgAppId = oAuth2RestClient.createOrganizationApplication(applicationModel, orgAppAccessToken);
+            ...
+        } finally {
+            oAuth2RestClient.closeHttpClient();
+        }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 666cb80 and c176ccd.

📒 Files selected for processing (9)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase.java (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java (3 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OAuth2RestClient.java (8 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java (2 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/SCIM2RestClient.java (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/OAuth2Constant.java (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/management-app-authorized-apis.json (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/organization-app-authorized-apis.json (1 hunks)
modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/oauth2/organization-app-authorized-apis.json
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/OrgMgtRestClient.java
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/OAuth2Constant.java

🧰 Additional context used

🧬 Code graph analysis (1)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/OAuth2ServiceAbstractIntegrationTest.java (1)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/OAuth2Constant.java (1)

OAuth2Constant (24-150)

🔇 Additional comments (2)

modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/SCIM2RestClient.java (1)

522-541: Org-scoped role update helper is consistent with existing patterns

The updateOrganizationUserRole implementation correctly mirrors existing SCIM role update helpers (path, headers, and 200 assertion) and cleanly encapsulates the org-scoped PATCH call. No changes needed.

modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml (1)

157-157: New OAuth2 sub-org test is correctly wired into TestNG suite

Including OAuth2AuthorizationCodeGrantForSubOrgAppsTestCase in the default configuration test group is consistent with other OAuth2 tests and preserves existing suite settings.