Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve user store domain not correctly set when trying organization switch grant #38

Closed
wants to merge 1 commit into from
Closed

Resolve user store domain not correctly set when trying organization switch grant #38

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions ...ava/org/wso2/carbon/identity/oauth2/grant/organizationswitch/OrganizationSwitchGrant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.CarbonConstants;
import org.wso2.carbon.identity.application.authentication.framework.exception.UserIdNotFoundException;
import org.wso2.carbon.identity.application.authentication.framework.model.AuthenticatedUser;
import org.wso2.carbon.identity.application.common.IdentityApplicationManagementException;
import org.wso2.carbon.identity.application.common.model.ApplicationBasicInfo;
import org.wso2.carbon.identity.application.mgt.ApplicationManagementService;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.oauth.common.OAuth2ErrorCodes;
import org.wso2.carbon.identity.oauth.common.OAuthConstants;
import org.wso2.carbon.identity.oauth.dao.OAuthAppDO;
Expand All @@ -54,7 +56,10 @@
import org.wso2.carbon.identity.organization.management.service.OrganizationManager;
import org.wso2.carbon.identity.organization.management.service.exception.OrganizationManagementException;
import org.wso2.carbon.identity.organization.management.service.exception.OrganizationManagementServerException;
import org.wso2.carbon.user.api.UserRealm;
import org.wso2.carbon.user.api.UserStoreException;
import org.wso2.carbon.user.core.common.AbstractUserStoreManager;
import org.wso2.carbon.user.core.service.RealmService;
import org.wso2.carbon.user.core.tenant.TenantManager;

import java.util.Arrays;
Expand All @@ -79,6 +84,7 @@ public class OrganizationSwitchGrant extends AbstractAuthorizationGrantHandler {
private static final Log LOG = LogFactory.getLog(OrganizationSwitchGrant.class);
private static final String TOKEN_BINDING_REFERENCE = "tokenBindingReference";
private static final String OAUTH_APP_PROPERTY = "OAuthAppDO";
public static final String ORG_USER_INVITATION_USER_DOMAIN = "OrganizationUserInvitation.PrimaryUserDomain";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to get this config from org mgt repos if there is no cyclic dependency issues. This will be resolved by doing https://github.com/wso2-extensions/identity-oauth2-grant-organization-switch/pull/38/files#r1903916703 anyway


@Override
public boolean validateGrant(OAuthTokenReqMessageContext tokReqMsgCtx) throws IdentityOAuth2Exception {
Expand Down Expand Up @@ -140,6 +146,11 @@ public boolean validateGrant(OAuthTokenReqMessageContext tokReqMsgCtx) throws Id
appResideOrgId.equals(authenticatedUser.getUserResidentOrganization())) {
authenticatedUser.setUserResidentOrganization(null);
}

/* The shared user's domain can be different from the original user's domain. Hence, resolve the correct user
store domain. */
resolveUserStoreDomain(authenticatedUser, accessingOrgId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for better readability, shall we return the relevant userstore from the private method and amend the authenticatedUser object in the main method

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the method name accordingly


tokReqMsgCtx.setAuthorizedUser(authenticatedUser);

String[] allowedScopes = tokReqMsgCtx.getOauth2AccessTokenReqDTO().getScope();
Expand Down Expand Up @@ -383,4 +394,30 @@ private String resolveImpersonator(JWTClaimsSet claimsSet) {
}
return null;
}

private void resolveUserStoreDomain(AuthenticatedUser authenticatedUser, String organizationId)
throws IdentityOAuth2Exception {

String userStoreDomain = IdentityUtil.getProperty(ORG_USER_INVITATION_USER_DOMAIN);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the userstore is configured as PRIMARY in this configuration but the userstore mgt configuration level if the admin has renamed the userstore name of PRIMARY, it is wrong to store PRIMARY as the userstore domain.
Let's gracefully handle that case. @DilshanSenarath has done similar fix in another place

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to fix it by introducing a util method to get ORG_USER_INVITATION_USER_DOMAIN resolving this mentioned considtion, instead reading the config directly from IdentityUtil.getProperty(ORG_USER_INVITATION_USER_DOMAIN)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HasiniSama Let's address the comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Better to have a util method which encapsulate the logic.

if (StringUtils.equals(authenticatedUser.getUserResidentOrganization(), organizationId)) {
try {
String tenantDomain = OrganizationSwitchGrantDataHolder.getInstance().getOrganizationManager()
.resolveTenantDomain(organizationId);
int tenantId = IdentityTenantUtil.getTenantId(tenantDomain);
userStoreDomain = getAbstractUserStoreManager(tenantId).getUser(authenticatedUser.getUserId(),
null).getUserStoreDomain();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

} catch (OrganizationManagementException | UserStoreException | UserIdNotFoundException e) {
throw new IdentityOAuth2Exception("Error while resolving user store domain of authenticated user.", e);
}
}
authenticatedUser.setUserStoreDomain(userStoreDomain);
}

private AbstractUserStoreManager getAbstractUserStoreManager(int tenantId) throws UserStoreException {

RealmService realmService = OrganizationSwitchGrantDataHolder.getInstance().getRealmService();
UserRealm tenantUserRealm = realmService.getTenantUserRealm(tenantId);
return (AbstractUserStoreManager) tenantUserRealm.getUserStoreManager();
}
}
Loading