Add oauth support for shared user direct login to sub-organizations.#3159
Add oauth support for shared user direct login to sub-organizations.#3159Yasasr1 wants to merge 3 commits intowso2-extensions:nextfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
| // If the logged-in user is shared, resident and accessing orgs are already set. | ||
| if (StringUtils.isNotBlank(appResidentOrganizationId) && !authzCodeDO.getAuthorizedUser().isSharedUser()) { | ||
| if (log.isDebugEnabled()) { | ||
| log.debug("Setting accessing organization id: " + appResidentOrganizationId + | ||
| " and user resident organization in the authorization code data object."); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 1
| // If the logged-in user is shared, resident and accessing orgs are already set. | |
| if (StringUtils.isNotBlank(appResidentOrganizationId) && !authzCodeDO.getAuthorizedUser().isSharedUser()) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("Setting accessing organization id: " + appResidentOrganizationId + | |
| " and user resident organization in the authorization code data object."); | |
| // If the logged-in user is shared, resident and accessing orgs are already set. | |
| if (StringUtils.isNotBlank(appResidentOrganizationId) && !authzCodeDO.getAuthorizedUser().isSharedUser()) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("User is not a shared user. Proceeding to set organization details."); | |
| } |
|
|
||
| insertTokenPrepStmt.executeUpdate(); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 2
| insertTokenPrepStmt.executeUpdate(); | |
| insertTokenPrepStmt.executeUpdate(); | |
| log.debug("Access token stored successfully for user: " + userStoreDomain + "/" + authenticatedUser); |
| } | ||
| } catch (OrganizationManagementException e) { | ||
| throw new IdentityOAuth2Exception("Error while resolving tenant domain for accessing organization: " + | ||
| authzUser.getAccessingOrganization(), e); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 3
| } | |
| } catch (OrganizationManagementException e) { | |
| throw new IdentityOAuth2Exception("Error while resolving tenant domain for accessing organization: " + | |
| authzUser.getAccessingOrganization(), e); | |
| } catch (OrganizationManagementException e) { | |
| log.error("Error while resolving tenant domain for accessing organization: " + authzUser.getAccessingOrganization()); | |
| throw new IdentityOAuth2Exception("Error while resolving tenant domain for accessing organization: " + | |
| authzUser.getAccessingOrganization(), e); |
| prepStmt.setInt(18, appTenantId); | ||
|
|
There was a problem hiding this comment.
Log Improvement Suggestion No: 4
| prepStmt.setInt(18, appTenantId); | |
| prepStmt.setInt(18, appTenantId); | |
| prepStmt.executeUpdate(); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Authorization code stored successfully for client: " + consumerKey + ", user: " + authzCodeDO.getAuthorizedUser().getLoggableUserId() + ", isSharedUser: " + authzCodeDO.getAuthorizedUser().isSharedUser()); | |
| } |
| user.setUserResidentOrganization(userOrganizationId); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 5
| user.setUserResidentOrganization(userOrganizationId); | |
| } | |
| user.setUserResidentOrganization(userOrganizationId); | |
| } | |
| if (log.isDebugEnabled()) { | |
| log.debug("Authorization code validated successfully. User: " + user.getLoggableUserId() + ", isSharedUser: " + isSharedUser + ", userOrg: " + user.getUserResidentOrganization() + ", accessingOrg: " + user.getAccessingOrganization()); | |
| } |
|
|
||
| public static final String STORE_AUTHORIZATION_CODE_WITH_PKCE_IDP_NAME_WITH_IS_SHARED_USER = "INSERT INTO " + |
There was a problem hiding this comment.
Log Improvement Suggestion No: 6
| public static final String STORE_AUTHORIZATION_CODE_WITH_PKCE_IDP_NAME_WITH_IS_SHARED_USER = "INSERT INTO " + | |
| public static final String STORE_AUTHORIZATION_CODE_WITH_PKCE_IDP_NAME = "INSERT INTO " + | |
| "IDN_OAUTH2_AUTHORIZATION_CODE " + | |
| "(CODE_ID, AUTHORIZATION_CODE, CONSUMER_KEY_ID, CALLBACK_URL, SCOPE, AUTHZ_USER, USER_DOMAIN, TENANT_ID, " + | |
| "TIME_CREATED, VALIDITY_PERIOD, SUBJECT_IDENTIFIER, PKCE_CODE_CHALLENGE, PKCE_CODE_CHALLENGE_METHOD, " + | |
| "AUTHORIZATION_CODE_HASH, IDP_ID) " + | |
| "SELECT ?,?,IDN_OAUTH_CONSUMER_APPS.ID,?,?,?,?,?,?,?,?,?,?,?,IDP.ID " + | |
| "FROM IDN_OAUTH_CONSUMER_APPS, IDP WHERE CONSUMER_KEY=? AND IDP.NAME=? AND IDP.TENANT_ID=? " + | |
| "AND IDN_OAUTH_CONSUMER_APPS.TENANT_ID=?"; | |
| /** | |
| * SQL query to store authorization code with PKCE, IDP name and shared user information. | |
| * This query adds IS_SHARED_USER field to track shared user scenarios. | |
| */ |
|
|
||
| public static final String VALIDATE_AUTHZ_CODE_WITH_PKCE_IDP_NAME_WITH_SHARED_USER = |
There was a problem hiding this comment.
Log Improvement Suggestion No: 7
| public static final String VALIDATE_AUTHZ_CODE_WITH_PKCE_IDP_NAME_WITH_SHARED_USER = | |
| public static final String VALIDATE_AUTHZ_CODE_WITH_PKCE_IDP_NAME_WITH_SHARED_USER = | |
| "SELECT AUTHZ_USER, USER_DOMAIN, " + | |
| "IDN_OAUTH2_AUTHORIZATION_CODE.TENANT_ID, SCOPE, CALLBACK_URL, TIME_CREATED,VALIDITY_PERIOD, STATE, " + | |
| "TOKEN_ID, AUTHORIZATION_CODE, CODE_ID, SUBJECT_IDENTIFIER, PKCE_CODE_CHALLENGE, " + | |
| "PKCE_CODE_CHALLENGE_METHOD, IDP.NAME, IS_SHARED_USER FROM IDN_OAUTH2_AUTHORIZATION_CODE " + | |
| "INNER JOIN IDP ON IDP_ID=IDP.ID WHERE CONSUMER_KEY_ID = (SELECT ID FROM IDN_OAUTH_CONSUMER_APPS " + | |
| "WHERE CONSUMER_KEY = ? AND TENANT_ID = ?) AND AUTHORIZATION_CODE_HASH = ?"; | |
| /** | |
| * SQL query to validate authorization code with PKCE, IDP name and shared user information. | |
| * Returns IS_SHARED_USER field to identify if the user is shared across organizations. | |
| */ |
| private void resolveAccessingAndResidentOrgsForOrganizationSSOUsers( | ||
| AuthenticatedUser authenticatedUser, String authzCode) { |
There was a problem hiding this comment.
Log Improvement Suggestion No: 8
| private void resolveAccessingAndResidentOrgsForOrganizationSSOUsers( | |
| AuthenticatedUser authenticatedUser, String authzCode) { | |
| private void resolveAccessingAndResidentOrgsForOrganizationSSOUsers( | |
| AuthenticatedUser authenticatedUser, String authzCode) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("Resolving accessing and resident organizations for user: " + authenticatedUser.getUserName()); | |
| } |
| authenticatedUser.setAccessingOrganization(accessingOrganization); | ||
| authenticatedUser.setUserResidentOrganization(userResideOrganization); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 9
| authenticatedUser.setAccessingOrganization(accessingOrganization); | |
| authenticatedUser.setUserResidentOrganization(userResideOrganization); | |
| authenticatedUser.setAccessingOrganization(accessingOrganization); | |
| authenticatedUser.setUserResidentOrganization(userResideOrganization); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Set accessing organization: " + accessingOrganization + " and resident organization: " + | |
| userResideOrganization + " for user: " + authenticatedUser.getUserName()); | |
| } |
| public static AuthenticatedUser createAuthenticatedUser(String username, String userStoreDomain, String | ||
| tenantDomain, String idpName, String accessingOrganization, int appTenantID, boolean isSharedUser) | ||
| throws IdentityOAuth2Exception { |
There was a problem hiding this comment.
Log Improvement Suggestion No: 10
| public static AuthenticatedUser createAuthenticatedUser(String username, String userStoreDomain, String | |
| tenantDomain, String idpName, String accessingOrganization, int appTenantID, boolean isSharedUser) | |
| throws IdentityOAuth2Exception { | |
| public static AuthenticatedUser createAuthenticatedUser(String username, String userStoreDomain, String | |
| tenantDomain, String idpName, String accessingOrganization, int appTenantID, boolean isSharedUser) | |
| throws IdentityOAuth2Exception { | |
| log.debug("Creating authenticated user for username: {}, userStoreDomain: {}, tenantDomain: {}, " + | |
| "accessingOrganization: {}, isSharedUser: {}", username, userStoreDomain, tenantDomain, | |
| accessingOrganization, isSharedUser); |
| // For shared users, tenant domain of the user resident organization is already set. | ||
| if (!authenticatedUser.isSharedUser()) { | ||
| authenticatedUser.setTenantDomain(appTenantDomain); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 11
| // For shared users, tenant domain of the user resident organization is already set. | |
| if (!authenticatedUser.isSharedUser()) { | |
| authenticatedUser.setTenantDomain(appTenantDomain); | |
| } | |
| // For shared users, tenant domain of the user resident organization is already set. | |
| if (!authenticatedUser.isSharedUser()) { | |
| log.debug("Setting tenant domain to application tenant domain: {} for non-shared user", appTenantDomain); | |
| authenticatedUser.setTenantDomain(appTenantDomain); | |
| } else { | |
| log.debug("Skipping tenant domain update for shared user. Keeping user resident organization tenant domain"); | |
| } |
| String clientId = authzReqMessageContext.getAuthorizationReqDTO().getConsumerKey(); | ||
| String appId = getApplicationId(clientId, tenantDomain); | ||
| // When user is not accessing the resident organization, resolve the application id from the shared app table. | ||
| if (!AuthzUtil.isUserAccessingResidentOrganization(authzReqMessageContext.getAuthorizationReqDTO().getUser())) { | ||
| // However, for shared user direct login flow, no need to resolve the shared app id. |
There was a problem hiding this comment.
Log Improvement Suggestion No: 12
| String clientId = authzReqMessageContext.getAuthorizationReqDTO().getConsumerKey(); | |
| String appId = getApplicationId(clientId, tenantDomain); | |
| // When user is not accessing the resident organization, resolve the application id from the shared app table. | |
| if (!AuthzUtil.isUserAccessingResidentOrganization(authzReqMessageContext.getAuthorizationReqDTO().getUser())) { | |
| // However, for shared user direct login flow, no need to resolve the shared app id. | |
| String clientId = authzReqMessageContext.getAuthorizationReqDTO().getConsumerKey(); | |
| String appId = getApplicationId(clientId, tenantDomain); | |
| log.debug("Validating scope for client: {} in tenant: {}", clientId, tenantDomain); | |
| // When user is not accessing the resident organization, resolve the application id from the shared app table. |
| String orgId = authzReqMessageContext.getAuthorizationReqDTO().getUser().getAccessingOrganization(); | ||
| String appResideOrgId = resolveOrgIdByTenantDomain(tenantDomain); | ||
| appId = SharedAppResolveDAO.resolveSharedApplication(appResideOrgId, appId, orgId); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 13
| String orgId = authzReqMessageContext.getAuthorizationReqDTO().getUser().getAccessingOrganization(); | |
| String appResideOrgId = resolveOrgIdByTenantDomain(tenantDomain); | |
| appId = SharedAppResolveDAO.resolveSharedApplication(appResideOrgId, appId, orgId); | |
| String orgId = authzReqMessageContext.getAuthorizationReqDTO().getUser().getAccessingOrganization(); | |
| String appResideOrgId = resolveOrgIdByTenantDomain(tenantDomain); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Resolving shared application for org: {} with app residing org: {}", orgId, appResideOrgId); | |
| } | |
| appId = SharedAppResolveDAO.resolveSharedApplication(appResideOrgId, appId, orgId); |
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.
There was a problem hiding this comment.
Pull request overview
This PR enhances OAuth2 authorization-code and access-token handling to better support “shared users” logging in directly to sub-organizations, including persisting and re-hydrating shared-user state and improving organization/tenant resolution behavior.
Changes:
- Persist and retrieve
IS_SHARED_USERfor authorization codes and access tokens (SQL + DAO updates). - Adjust organization/tenant resolution logic for shared users during scope validation, code generation, and token retrieval.
- Bump Carbon Identity Framework dependency version.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
pom.xml |
Updates Carbon Identity Framework version to pick up required framework-level changes. |
.../DefaultOAuth2ScopeValidator.java |
Skips shared-app resolution for shared-user direct login flows. |
.../OAuth2Util.java |
Propagates shared-user flag into AuthenticatedUser creation and adjusts org-bound tenant-domain assignment rules. |
.../AuthorizationCodeGrantHandler.java |
Resolves both accessing + resident org for organization SSO users when exchanging auth codes. |
.../SQLQueries.java |
Adds new SQL variants that include IS_SHARED_USER and deprecates old ones. |
.../AuthorizationCodeDAOImpl.java |
Persists/reads IS_SHARED_USER for auth codes and updates org resolution for shared users. |
.../AccessTokenDAOImpl.java |
Persists/reads IS_SHARED_USER for tokens and adjusts tenant resolution when fetching latest token for shared users. |
.../ResponseTypeHandlerUtil.java |
Prevents overwriting org context during auth-code generation for shared users. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static final String STORE_AUTHORIZATION_CODE_WITH_PKCE_IDP_NAME_WITH_IS_SHARED_USER = "INSERT INTO " + | ||
| "IDN_OAUTH2_AUTHORIZATION_CODE (CODE_ID, AUTHORIZATION_CODE, CONSUMER_KEY_ID, CALLBACK_URL, SCOPE, " + | ||
| "AUTHZ_USER, USER_DOMAIN, TENANT_ID, TIME_CREATED, VALIDITY_PERIOD, SUBJECT_IDENTIFIER, " + | ||
| "PKCE_CODE_CHALLENGE, PKCE_CODE_CHALLENGE_METHOD, AUTHORIZATION_CODE_HASH, IDP_ID, IS_SHARED_USER) " + | ||
| "SELECT ?,?,IDN_OAUTH_CONSUMER_APPS.ID,?,?,?,?,?,?,?,?,?,?,?,IDP.ID,? FROM IDN_OAUTH_CONSUMER_APPS, " + | ||
| "IDP WHERE CONSUMER_KEY=? AND IDP.NAME=? AND IDP.TENANT_ID=? AND IDN_OAUTH_CONSUMER_APPS.TENANT_ID=?"; |
There was a problem hiding this comment.
These queries assume an IS_SHARED_USER column exists in IDN_OAUTH2_AUTHORIZATION_CODE (and related DAO code now inserts/selects it). I couldn’t find any DB schema / upgrade scripts in the repo (including the H2 test dbScripts/identity.sql) that add this column, so fresh deployments and unit tests will fail with “column not found”. Please add the required DDL + migration steps (with a sensible default) and update the test DB scripts accordingly.
| public static final String VALIDATE_AUTHZ_CODE_WITH_PKCE_IDP_NAME_WITH_SHARED_USER = | ||
| "SELECT AUTHZ_USER, USER_DOMAIN, " + | ||
| "IDN_OAUTH2_AUTHORIZATION_CODE.TENANT_ID, SCOPE, CALLBACK_URL, TIME_CREATED,VALIDITY_PERIOD, STATE, " + | ||
| "TOKEN_ID, AUTHORIZATION_CODE, CODE_ID, SUBJECT_IDENTIFIER, PKCE_CODE_CHALLENGE, " + | ||
| "PKCE_CODE_CHALLENGE_METHOD, IDP.NAME, IS_SHARED_USER FROM IDN_OAUTH2_AUTHORIZATION_CODE " + | ||
| "INNER JOIN IDP ON IDP_ID=IDP.ID WHERE CONSUMER_KEY_ID = (SELECT ID FROM IDN_OAUTH_CONSUMER_APPS " + | ||
| "WHERE CONSUMER_KEY = ? AND TENANT_ID = ?) AND AUTHORIZATION_CODE_HASH = ?"; |
There was a problem hiding this comment.
Naming inconsistency: most new constants use the ...WITH_IS_SHARED_USER suffix, but this one uses ...WITH_SHARED_USER. Aligning the constant name with the existing pattern will make it easier to discover/grep and reduces the chance of selecting the wrong query constant later.
| private void resolveAccessingAndResidentOrgsForOrganizationSSOUsers( | ||
| AuthenticatedUser authenticatedUser, String authzCode) { | ||
|
|
||
| if (authenticatedUser.isFederatedUser() && FrameworkConstants.ORGANIZATION_LOGIN_IDP_NAME | ||
| .equals(authenticatedUser.getFederatedIdPName())) { | ||
| String userResideOrganization = resolveUserResidentOrganization(AuthorizationGrantCache.getInstance() | ||
| .getValueFromCacheByCode(new AuthorizationGrantCacheKey(authzCode)).getUserAttributes()); | ||
| authenticatedUser.setAccessingOrganization(userResideOrganization); | ||
| String accessingOrganization = resolveUserAccessingOrganization(AuthorizationGrantCache.getInstance() | ||
| .getValueFromCacheByCode(new AuthorizationGrantCacheKey(authzCode)).getUserAttributes()); | ||
| authenticatedUser.setAccessingOrganization(accessingOrganization); |
There was a problem hiding this comment.
AuthorizationGrantCache.getValueFromCacheByCode(...) is called twice with the same key, which is unnecessary work and makes the logic harder to follow. Consider fetching the cache entry once into a local variable and reusing its getUserAttributes() for both resident/accessing org resolution.
| public static final String INSERT_OAUTH2_ACCESS_TOKEN_WITH_IDP_NAME_WITH_IS_SHARED_USER = | ||
| "INSERT INTO IDN_OAUTH2_ACCESS_TOKEN " + | ||
| "(ACCESS_TOKEN, REFRESH_TOKEN, CONSUMER_KEY_ID, AUTHZ_USER, TENANT_ID, USER_DOMAIN, TIME_CREATED, " + | ||
| "REFRESH_TOKEN_TIME_CREATED, VALIDITY_PERIOD, REFRESH_TOKEN_VALIDITY_PERIOD, TOKEN_SCOPE_HASH, " + | ||
| "TOKEN_STATE, USER_TYPE, TOKEN_ID, GRANT_TYPE, SUBJECT_IDENTIFIER, ACCESS_TOKEN_HASH, REFRESH_TOKEN_HASH," + | ||
| "IDP_ID, TOKEN_BINDING_REF, AUTHORIZED_ORGANIZATION, IS_SHARED_USER) SELECT ?,?," + | ||
| "IDN_OAUTH_CONSUMER_APPS.ID,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,IDP.ID,?,?,? " | ||
| + "FROM IDN_OAUTH_CONSUMER_APPS, IDP WHERE IDP.NAME=? AND IDP.TENANT_ID=? AND CONSUMER_KEY=? AND " + | ||
| "IDN_OAUTH_CONSUMER_APPS.TENANT_ID=?"; |
There was a problem hiding this comment.
This insert adds IS_SHARED_USER to IDN_OAUTH2_ACCESS_TOKEN, but the repository’s DB schema/test scripts (e.g., components/.../src/test/resources/dbScripts/identity.sql) don’t currently define this column. Without the corresponding DDL/migration, token persistence and unit tests will fail at runtime. Please add the column (defaulting to non-shared) in the relevant DB scripts and upgrade paths.
…ng shared app logins.
This pull request enhances support for "shared users" in the OAuth2 authorization and token handling logic. The changes ensure that the
IS_SHARED_USERattribute is correctly persisted and retrieved in both access tokens and authorization codes, and that related SQL queries and data access logic are updated accordingly. Additionally, the logic for resolving organization and tenant information for shared users is improved.Key changes include:
Database and SQL Layer Updates:
Added the
IS_SHARED_USERcolumn to relevant SQL queries and table insertions for both access tokens and authorization codes. New SQL queries were introduced to handle the additional column, and deprecated queries without this support.Updated prepared statement logic to set the
IS_SHARED_USERvalue when inserting access tokens and authorization codes.Access Token and Authorization Code Logic:
Modified retrieval and validation logic to read the
IS_SHARED_USERattribute from the database, set it in theAuthenticatedUserobject, and handle organization resolution for shared users.Updated logic to correctly resolve the tenant ID based on the accessing organization for shared users when fetching the latest access token.
Authorization Code Generation and Context Handling:
ResponseTypeHandlerUtilto avoid overwriting organization context for shared users, ensuring correct organization information is set only when appropriate.Dependency and Import Updates:
Dependent on
Related issue