fix scim2 group & role issue #4824
Conversation
| @@ -1479,7 +1480,7 @@ public String getRoleNameByID(String roleID, String tenantDomain) throws Identit | |||
| + tenantDomain; | |||
| throw new IdentityRoleManagementServerException(UNEXPECTED_SERVER_ERROR.getCode(), errorMessage, e); | |||
| } | |||
| if (roleName == null) { | |||
| if (roleName == null || !SCIMCommonUtils.isHybridRole(roleName)) { | |||
There was a problem hiding this comment.
This logic is not very clear. You are trying to throw the error message "A role doesn't exist with id: " + roleID + " in the tenant domain: " + tenant domain, if the role name is null or the role name does not start with "Application" or "Internal". But from this method, we are trying to get the role name by passing the role id to the table IDN_SCIM_GROUP. So based on the logic if we get a role that does not contain the prefix "Application" or "Internal" it will throw an error. Is this expected?
There was a problem hiding this comment.
According to the current implementation, IDN_SCIM_GROUP table stores both roles and groups. This group endpoint code (https://github.com/wso2-support/identity-inbound-provisioning-scim2/blob/645b50a897929cd5cb3d32c5a1af3b6c9c6b933c/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java#L2620-L2624) uses this method (https://github.com/wso2-support/identity-inbound-provisioning-scim2/blob/645b50a897929cd5cb3d32c5a1af3b6c9c6b933c/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java#L2744) to differentiate between roles and groups in the same table. Since we should only get roles name (Exclude group names), we have to do a filtering. That's why I have included this logic.
| @@ -1479,7 +1480,8 @@ public String getRoleNameByID(String roleID, String tenantDomain) throws Identit | |||
| + tenantDomain; | |||
| throw new IdentityRoleManagementServerException(UNEXPECTED_SERVER_ERROR.getCode(), errorMessage, e); | |||
| } | |||
| if (roleName == null) { | |||
| // Verify whether the roleName is either null or a group name | |||
There was a problem hiding this comment.
Please complete the comment. Its not very clear
| @@ -1479,7 +1480,8 @@ public String getRoleNameByID(String roleID, String tenantDomain) throws Identit | |||
| + tenantDomain; | |||
| throw new IdentityRoleManagementServerException(UNEXPECTED_SERVER_ERROR.getCode(), errorMessage, e); | |||
| } | |||
| if (roleName == null) { | |||
| // Verify whether the roleName is either null or a group name | |||
There was a problem hiding this comment.
| // Verify whether the roleName is either null or a group name | |
| // Verify whether the roleName is either null or it's not contain any prefix Application/Internal |
Proposed changes in this pull request
This PR will fix the issue related to the scim2 Role endpoint. Followings are the issue details.