Enable Authorization Code grant support for sub organization OAuth2 applications #2971

ShanChathusanda93 · 2025-11-07T09:25:45Z

Proposed changes in this pull request

$subject
Depends on Resolve application tenant domain when sub organization applications login flow wso2/carbon-identity-framework#7619

...ity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/AuthzUtil.java

wso2-engineering · 2025-11-07T09:26:54Z

....oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtil.java

+     */
+    public static String getSPTenantDomainFromClientId(String clientId, String tenantDomain) {
+
+        try {


Log Improvement Suggestion No: 3

Suggested change

*/

public static String getSPTenantDomainFromClientId(String clientId, String tenantDomain) {

try {

public static String getSPTenantDomainFromClientId(String clientId, String tenantDomain) {

if (log.isDebugEnabled()) {

log.debug("Retrieving SP tenant domain for client ID: " + clientId + " from tenant: " + tenantDomain);

}

try {

....oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtil.java

...c/main/java/org/wso2/carbon/identity/oauth2/authz/handlers/util/ResponseTypeHandlerUtil.java

...a/org/wso2/carbon/identity/oauth2/authz/validators/AbstractResponseTypeRequestValidator.java

...entity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImpl.java

...java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AuthorizationCodeGrantHandler.java

...so2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java

wso2-engineering

AI Agent Log Improvement Checklist

⚠️ Warning: AI-Generated Review Comments

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
#### Log Improvement Suggestion No: 3
#### Log Improvement Suggestion No: 4
#### Log Improvement Suggestion No: 5
#### Log Improvement Suggestion No: 6
#### Log Improvement Suggestion No: 7
#### Log Improvement Suggestion No: 8
#### Log Improvement Suggestion No: 9
#### Log Improvement Suggestion No: 10
#### Log Improvement Suggestion No: 11
#### Log Improvement Suggestion No: 12
#### Log Improvement Suggestion No: 13
#### Log Improvement Suggestion No: 14

Copilot

Pull request overview

This pull request enables Authorization Code grant support for sub-organization OAuth2 applications by adding organization-aware context handling throughout the OAuth2/OIDC flows. The changes allow applications registered in sub-organizations to properly authenticate users and issue tokens while maintaining proper tenant and organization isolation.

Key Changes

Updated OAuth2 utility methods to accept explicit tenant domain parameters instead of relying solely on thread-local context
Added organization context resolution logic throughout authorization code generation, validation, and OIDC logout flows
Modified OIDC session cookie path handling to support sub-organization applications with tenant-qualified paths

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pom.xml	Updated carbon.identity.framework.version to 7.8.598 to support the new sub-organization OAuth2 features
OIDCLogoutServlet.java	Added organization context resolution for logout flows, including tenant domain lookup from organization ID and proper error handling for OrganizationManagementException
DefaultOIDCSessionStateManager.java	Enhanced cookie path handling to support sub-organization applications by checking applicationResidentOrganizationId and setting appropriate tenant-qualified cookie paths
OAuth2ServiceTest.java	Updated test mocks to use the new two-parameter OAuth2Util methods that require explicit tenant domain
OAuth2Util.java	Modified getIdTokenIssuer to use root tenant domain when applicationResidentOrganizationId is present, ensuring correct issuer for sub-organization apps
AuthorizationCodeGrantHandler.java	Updated validatePKCECode to accept tenantDomain parameter and pass it to getOAuthAppDO for proper tenant-aware app retrieval
AuthorizationCodeDAOImpl.java	Added organization context handling in authorization code validation, resolving tenant ID from organization and setting user organization context on AuthenticatedUser
AbstractResponseTypeRequestValidator.java	Modified client validation to resolve login tenant ID and pass it to OAuth2Util methods for proper tenant-aware app lookups
ResponseTypeHandlerUtil.java	Added logic to set accessing organization and user resident organization IDs on authorization code generation for sub-org apps
AuthzUtilTest.java	Updated test to mock OAuth2Util methods with explicit tenant domain parameters for proper test isolation
OAuth2ParEndpointTest.java	Added tenant domain mocking and updated OAuth2Util method calls to use two-parameter versions
OAuth2AuthzEndpointTest.java	Added AuthzUtil.isLegacyAuthzRuntime mocking to support new authorization runtime behavior
EndpointUtil.java	Added new overloaded getSPTenantDomainFromClientId method that accepts explicit tenant domain parameter
AuthzUtil.java	Enhanced multiple methods to resolve tenant domain from organization context, including populateValidationResponseWithAppDetail, getSpTenantDomain, getLoginTenantDomain, and associateAuthenticationHistory

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

Copilot · 2025-11-21T19:08:30Z

...c/main/java/org/wso2/carbon/identity/oauth2/authz/handlers/util/ResponseTypeHandlerUtil.java

+                        " and user resident organization in the authorization code data object.");
+            }
+            authzCodeDO.getAuthorizedUser().setAccessingOrganization(appResidentOrganizationId);
+            authzCodeDO.getAuthorizedUser().setUserResidentOrganization(appResidentOrganizationId);


The logic in this code block appears to set both setAccessingOrganization and setUserResidentOrganization to the same value (appResidentOrganizationId). This seems incorrect - the user resident organization should be different from the accessing organization (the app's organization).

Looking at similar logic in AuthorizationCodeDAOImpl.java (lines 278-283), the correct pattern is:

setAccessingOrganization(appResidentOrganizationId) - the organization where the app resides

setUserResidentOrganization(userOrganizationId) - the organization where the user resides

The user resident organization should be resolved from the user's tenant domain, not set to the app's resident organization ID.

Suggested change

authzCodeDO.getAuthorizedUser().setUserResidentOrganization(appResidentOrganizationId);

String userTenantDomain = authzCodeDO.getAuthorizedUser().getTenantDomain();

String userResidentOrganizationId = OAuth2Util.getOrganizationIdByTenantDomain(userTenantDomain);

authzCodeDO.getAuthorizedUser().setUserResidentOrganization(userResidentOrganizationId);

Copilot · 2025-11-21T19:08:31Z

...c.session/src/main/java/org/wso2/carbon/identity/oidc/session/servlet/OIDCLogoutServlet.java

            log.error(msg + " Client id from id token: " + clientId, e);
            redirectURL = getErrorPageURL(OAuth2ErrorCodes.ACCESS_DENIED, msg);
+        } catch (OrganizationManagementException e) {
+            String msg = "Error occurred while resolving tenant domain from application resident org id.";


The error message "Error occurred while resolving tenant domain from application resident org id." is missing a specific detail. It should include the organization ID in the log message (similar to line 418 which has "for organization") to aid in debugging when issues occur.

Suggested change

String msg = "Error occurred while resolving tenant domain from application resident org id.";

String msg = "Error occurred while resolving tenant domain from application resident org id: " +

appResidentOrgId + ".";

Copilot · 2025-11-21T19:08:31Z

...c/main/java/org/wso2/carbon/identity/oauth2/authz/handlers/util/ResponseTypeHandlerUtil.java

+                log.debug("Setting accessing organization id: " + appResidentOrganizationId +
+                        " and user resident organization in the authorization code data object.");


The debug log message claims to be "Setting accessing organization id: ... and user resident organization" but only logs the appResidentOrganizationId. This is misleading. Since both values are being set to the same ID (which is incorrect per comment #1), this message should be updated when the bug is fixed to log both values distinctly, or the message should be corrected to reflect what's actually being done.

Suggested change

log.debug("Setting accessing organization id: " + appResidentOrganizationId +

" and user resident organization in the authorization code data object.");

log.debug("Setting both accessing organization id and user resident organization to: " +

appResidentOrganizationId + " in the authorization code data object.");

Copilot · 2025-11-21T19:08:32Z

...c.session/src/main/java/org/wso2/carbon/identity/oidc/session/servlet/OIDCLogoutServlet.java

            redirectURL = getErrorPageURL(OAuth2ErrorCodes.ACCESS_DENIED, msg);
            return getRedirectURL(redirectURL, request);
+        } catch (OrganizationManagementException e) {
+            String msg = "Error occurred while resolving tenant domain for organization.";


The exception is caught but not logged. Unlike the other catch blocks (lines 400-401, 412-413) which log the error using log.debug(), this catch block for OrganizationManagementException doesn't log the exception. This makes debugging difficult when organization resolution fails. Consider adding:

if (log.isDebugEnabled()) { log.debug(msg, e); }

Suggested change

String msg = "Error occurred while resolving tenant domain for organization.";

String msg = "Error occurred while resolving tenant domain for organization.";

if (log.isDebugEnabled()) {

log.debug(msg, e);

}

Copilot · 2025-11-21T19:08:32Z

...c.session/src/main/java/org/wso2/carbon/identity/oidc/session/servlet/OIDCLogoutServlet.java

+        } catch (OrganizationManagementException e) {
+            String msg = "Error occurred while resolving tenant domain from application resident org id.";
+            log.error(msg + " Client id from id token: " + clientId, e);
+            redirectURL = getErrorPageURL(OAuth2ErrorCodes.ACCESS_DENIED, msg);


The exception is caught but not logged. Unlike the other catch blocks in the same method (lines 1069-1071 which log using log.error()), this catch block for OrganizationManagementException doesn't log the exception. This makes debugging difficult when organization resolution fails. Consider adding:

log.error(msg + " Client id from id token: " + clientId, e);

...ity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/AuthzUtil.java

SujanSanjula96 · 2025-11-22T05:10:26Z

....oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtil.java

+            log.error("Error while getting oauth app for client Id: " + clientId, e);
+            return MultitenantConstants.SUPER_TENANT_DOMAIN_NAME;
+        } catch (InvalidOAuthClientException e) {
+            if (log.isDebugEnabled()) {
+                log.debug("Error while getting oauth app for client Id: " + clientId, e);
+            }
+            return MultitenantConstants.SUPER_TENANT_DOMAIN_NAME;
+        }


Are these fallbacks intentional?

...c/main/java/org/wso2/carbon/identity/oauth2/authz/handlers/util/ResponseTypeHandlerUtil.java

codecov · 2025-11-23T11:08:30Z

Codecov Report

❌ Patch coverage is 25.00000% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.15%. Comparing base (1a416de) to head (41e01ac).
⚠️ Report is 42 commits behind head on master.

Files with missing lines	Patch %	Lines
...entity/oidc/session/servlet/OIDCLogoutServlet.java	13.33%	13 Missing ⚠️
...carbon/identity/oauth/endpoint/util/AuthzUtil.java	45.00%	10 Missing and 1 partial ⚠️
.../identity/oauth2/dao/AuthorizationCodeDAOImpl.java	28.57%	8 Missing and 2 partials ⚠️
...y/oidc/session/DefaultOIDCSessionStateManager.java	0.00%	9 Missing ⚠️
...bon/identity/oauth/endpoint/util/EndpointUtil.java	0.00%	8 Missing ⚠️
...g/wso2/carbon/identity/oauth2/util/OAuth2Util.java	22.22%	5 Missing and 2 partials ⚠️
...2/authz/handlers/util/ResponseTypeHandlerUtil.java	28.57%	4 Missing and 1 partial ⚠️
.../handlers/grant/AuthorizationCodeGrantHandler.java	0.00%	3 Missing ⚠️

❌ Your patch status has failed because the patch coverage (25.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2971      +/-   ##
============================================
- Coverage     58.07%   57.15%   -0.92%     
+ Complexity     9628     9483     -145     
============================================
  Files           669      669              
  Lines         53605    55101    +1496     
  Branches      12235    12640     +405     
============================================
+ Hits          31131    31494     +363     
- Misses        18118    19185    +1067     
- Partials       4356     4422      +66

Flag	Coverage Δ
unit	`41.14% <25.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

AnuradhaSK · 2025-11-24T04:39:50Z

...entity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImpl.java

                }
                user.setAuthenticatedSubjectIdentifier(subjectIdentifier, serviceProvider);
+                if (StringUtils.isNotEmpty(appResidentOrganizationId)) {
+                    String userOrganizationId = OAuth2ServiceComponentHolder.getInstance().getOrganizationManager()


here the user's org id is always the sub org app residing org righ?
Better to resolve the user resident org correctly as this logic will fail when the app is shared

The user resident organization is resolved from the tenant domain. Tenant domain is extracted from the database result and it is used to create the authenticated user object as well (line 267).

AnuradhaSK · 2025-11-24T04:42:07Z

....oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtil.java

+            return MultitenantConstants.SUPER_TENANT_DOMAIN_NAME;
+        } catch (InvalidOAuthClientException e) {
+            if (log.isDebugEnabled()) {
+                log.debug("Error while getting oauth app for client Id: " + clientId, e);
+            }
+            return MultitenantConstants.SUPER_TENANT_DOMAIN_NAME;


Why it's fallback to carbon super tenant? Ideally it should be an error case or invalid tenant id (-1)

...ity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/AuthzUtil.java

ShanChathusanda93 · 2025-11-24T08:26:54Z

...ity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/AuthzUtil.java

        try {
-            OAuthAppDO appDO = OAuth2Util.getAppInformationByClientId(clientId);
+            OAuthAppDO appDO;
+            String appResidentOrgId = PrivilegedCarbonContext.getThreadLocalCarbonContext()


Introduce a new method to get the application tenant domain and reuse it

ShanChathusanda93 · 2025-11-24T08:43:24Z

...ity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/AuthzUtil.java

+        String appResidentOrgId = PrivilegedCarbonContext.getThreadLocalCarbonContext()
+                .getApplicationResidentOrganizationId();
+        if (StringUtils.isNotEmpty(appResidentOrgId)) {
+            String appResidentTenantDomain;
+            try {
+                appResidentTenantDomain = FrameworkUtils.resolveTenantDomainFromOrganizationId(appResidentOrgId);
+            } catch (FrameworkException e) {
+                throw new InvalidRequestException("Error resolving tenant domain from organization id: "
+                        + appResidentOrgId, OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ErrorCodes.OAuth2SubErrorCodes
+                        .INVALID_REQUEST);
+            }
+            return EndpointUtil.getSPTenantDomainFromClientId(clientId, appResidentTenantDomain);
+        }


Move this to line 2882

ShanChathusanda93 · 2025-11-24T08:50:56Z

....oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtil.java

+     * @param tenantDomain Tenant domain to which the client ID belongs.
+     * @return tenantDomain domain of the service provider.
+     */
+    public static String getSPTenantDomainFromClientId(String clientId, String tenantDomain) {


Verifying method

piraveena · 2025-11-25T04:31:15Z

...ity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/AuthzUtil.java

+         session is stored against the primary organization.
+        */
+        try {
+            String appTenantDomain = OAuth2Util.getTenantDomainOfOauthApp(


resultFromLogin.getoAuth2Parameters().getClientId() can this be null here?

This is populated from the session. Based on the current path that the operation is hapenning, this is not getting null.

piraveena · 2025-11-25T04:32:24Z

...ity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/AuthzUtil.java

+            if (OrganizationManagementUtil.isOrganization(appTenantDomain)) {
+                String appOrgId = OAuth2ServiceComponentHolder.getInstance().getOrganizationManager()
+                        .resolveOrganizationId(appTenantDomain);
+                String primaryOrgId = OAuth2ServiceComponentHolder.getInstance().getOrganizationManager()


Is primary orgId and login org Id are same?

No, primary org id is the org id of the root organization. Login org id is the application resident organization

piraveena · 2025-11-25T04:34:38Z

...so2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java

                Boolean.parseBoolean(IdentityUtil.getProperty(ALLOW_SESSION_BOUND_TOKENS_AFTER_IDLE_SESSION_EXPIRY));
    }
+
+    public static String extractTenantDomain() throws IdentityOAuth2Exception {


Add javadoc comments. Can we rename this method? getLoginTenant?

piraveena · 2025-11-25T04:35:43Z

...sion/src/main/java/org/wso2/carbon/identity/oidc/session/DefaultOIDCSessionStateManager.java

+            if (StringUtils.isNotBlank(appResidentOrgId)) {
+                // Handling the cookie path for request coming with the path `/t/<tenant-domain>/o/<org-id>`.
+                // The cookie will be set to the root tenant path.
+                loginTenantDomain = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain();


Shall we get the loginTenantDomain from the OAuth2Util.extractTenantDomain method and simply these logics?

No we cannot. Here we need the root organization tenant domain since the cookies needs to be stored in the root tenant level.

…pplications

piraveena · 2025-11-25T11:17:03Z

....oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtil.java

+            if (StringUtils.equals(extractedTenantDomain, appTenantDomain)) {
+                return appTenantDomain;
+            }
+            throw new OAuthSystemException("Provided tenant domain: " + extractedTenantDomain + " does not " +


Is this a new type of exception? Are we using this in other places? isn't this a client error.

We have used this exception in the Endpoint utils

piraveena · 2025-11-25T11:38:44Z

Please send the PRs for rest of the unit tests

jenkins-is-staging · 2025-11-25T11:41:01Z

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/19668294722

jenkins-is-staging · 2025-11-25T12:33:12Z

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/19668294722
Status: success

jenkins-is-staging

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/19668294722