From 8388c51640e2cf7e67754cf3a865b4afd720cf6a Mon Sep 17 00:00:00 2001 From: Bimsara Bodaragama Date: Fri, 28 Jun 2024 09:55:55 +0530 Subject: [PATCH 1/6] Fix: SCIM error handling for group display name (fixes #20344) --- .../identity/scim2/common/impl/SCIMRoleManagerV2.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java index 63cfab4b..0b755eb0 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java @@ -1125,7 +1125,12 @@ private void doUpdateGroups(String roleId, Set newGroupIDList, Set(deleteGroupIDList), tenantDomain); } catch (IdentityRoleManagementException e) { if (RoleConstants.Error.INVALID_REQUEST.getCode().equals(e.getErrorCode())) { - throw new BadRequestException(e.getMessage()); + // Custom error message and SCIM type + String customMessage = "Invalid request: Group display name is not supported. Please use the group ID instead."; + String scimType = "invalidSyntax"; // From RFC 7644 Table 9 + + // Throw BadRequestException with custom message and scimType + throw new BadRequestException(customMessage, scimType); } throw new CharonException( String.format("Error occurred while updating groups in the role with ID: %s", roleId), e); From 9f11dc42750c1345c9ebb2cd21ced2c7bf3c5c6e Mon Sep 17 00:00:00 2001 From: Bimsara Bodaragama Date: Fri, 28 Jun 2024 11:32:57 +0530 Subject: [PATCH 2/6] Fix SCIM error handling with generalized message for invalid request values (fixes #20334) --- .../carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java index 0b755eb0..0f52bfb8 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java @@ -1126,11 +1126,11 @@ private void doUpdateGroups(String roleId, Set newGroupIDList, Set Date: Fri, 28 Jun 2024 16:27:45 +0530 Subject: [PATCH 3/6] Generalize SCIM error message for invalid request values (fixes #20334) --- .../carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java index 0f52bfb8..28d9847b 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java @@ -1125,8 +1125,9 @@ private void doUpdateGroups(String roleId, Set newGroupIDList, Set(deleteGroupIDList), tenantDomain); } catch (IdentityRoleManagementException e) { if (RoleConstants.Error.INVALID_REQUEST.getCode().equals(e.getErrorCode())) { - // Custom error message and SCIM type - String errorMessage = "Invalid request: The provided value is not supported. Please use the group ID instead."; + // Error message and SCIM type + String errorMessage = "Updating groups of the role by display name is not supported. " + + "Update using group id instead."; String scimType = "invalidSyntax"; // From RFC 7644 Table 9 // Throw BadRequestException with custom message and scimType From b974ba9606102966ce3ceb702105778fa9623348 Mon Sep 17 00:00:00 2001 From: Bimsara Bodaragama Date: Mon, 1 Jul 2024 14:35:49 +0530 Subject: [PATCH 4/6] Fix SCIM error message for invalid request values in V1 and V2 Roles Remove-Add-Replace operations (fixes #20334) --- .../scim2/common/impl/SCIMRoleManager.java | 23 ++++++++++----- .../scim2/common/impl/SCIMRoleManagerV2.java | 28 ++++++++++--------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java index 5b792e3f..24add48d 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java @@ -771,21 +771,30 @@ private void updatePermissions(String roleId, List permissionOpe private void prepareAddedRemovedGroupLists(Set addedGroupsIds, Set removedGroupsIds, Set replacedGroupsIds, PatchOperation groupOperation, - Map groupObject, List groupListOfRole) { + Map groupObject, List groupListOfRole) + throws BadRequestException { + + String value = groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE); + String errorMessage = + "Updating groups of the role by display name is not supported. Update using group id instead."; + + if (value == null) { + throw new BadRequestException(errorMessage, ResponseCodeConstants.INVALID_SYNTAX); + } switch (groupOperation.getOperation()) { case (SCIMConstants.OperationalConstants.ADD): - removedGroupsIds.remove(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); - if (!isGroupExist(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE), groupListOfRole)) { - addedGroupsIds.add(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); + removedGroupsIds.remove(value); + if (!isGroupExist(value, groupListOfRole)) { + addedGroupsIds.add(value); } break; case (SCIMConstants.OperationalConstants.REMOVE): - addedGroupsIds.remove(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); - removedGroupsIds.add(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); + addedGroupsIds.remove(value); + removedGroupsIds.add(value); break; case (SCIMConstants.OperationalConstants.REPLACE): - replacedGroupsIds.add(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); + replacedGroupsIds.add(value); break; } } diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java index 28d9847b..c30a4235 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java @@ -1125,13 +1125,7 @@ private void doUpdateGroups(String roleId, Set newGroupIDList, Set(deleteGroupIDList), tenantDomain); } catch (IdentityRoleManagementException e) { if (RoleConstants.Error.INVALID_REQUEST.getCode().equals(e.getErrorCode())) { - // Error message and SCIM type - String errorMessage = "Updating groups of the role by display name is not supported. " - + "Update using group id instead."; - String scimType = "invalidSyntax"; // From RFC 7644 Table 9 - - // Throw BadRequestException with custom message and scimType - throw new BadRequestException(errorMessage, scimType); + throw new BadRequestException(); } throw new CharonException( String.format("Error occurred while updating groups in the role with ID: %s", roleId), e); @@ -1220,19 +1214,27 @@ private List getUserIDList(List userList, String tenantDomain) t private void prepareInitialGroupLists(Set givenAddedGroupsIds, Set givenRemovedGroupsIds, Set givenReplacedGroupsIds, PatchOperation groupOperation, - Map groupObject) { + Map groupObject) throws BadRequestException { + + String value = groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE); + String errorMessage = + "Updating groups of the role by display name is not supported. Update using group id instead."; + + if (value == null) { + throw new BadRequestException(errorMessage, ResponseCodeConstants.INVALID_SYNTAX); + } switch (groupOperation.getOperation()) { case (SCIMConstants.OperationalConstants.ADD): - givenRemovedGroupsIds.remove(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); - givenAddedGroupsIds.add(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); + givenRemovedGroupsIds.remove(value); + givenAddedGroupsIds.add(value); break; case (SCIMConstants.OperationalConstants.REMOVE): - givenAddedGroupsIds.remove(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); - givenRemovedGroupsIds.add(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); + givenAddedGroupsIds.remove(value); + givenRemovedGroupsIds.add(value); break; case (SCIMConstants.OperationalConstants.REPLACE): - givenReplacedGroupsIds.add(groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE)); + givenReplacedGroupsIds.add(value); break; default: break; From 33c09b9a171af45c68a735d134e20e2d3c477245 Mon Sep 17 00:00:00 2001 From: Bimsara Bodaragama Date: Mon, 1 Jul 2024 14:53:40 +0530 Subject: [PATCH 5/6] Refactor SCIM error handling to directly use error message string (fixes #20334) --- .../carbon/identity/scim2/common/impl/SCIMRoleManager.java | 6 +++--- .../identity/scim2/common/impl/SCIMRoleManagerV2.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java index 24add48d..120853f5 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java @@ -775,11 +775,11 @@ private void prepareAddedRemovedGroupLists(Set addedGroupsIds, Set givenAddedGroupsIds, Set groupObject) throws BadRequestException { String value = groupObject.get(SCIMConstants.CommonSchemaConstants.VALUE); - String errorMessage = - "Updating groups of the role by display name is not supported. Update using group id instead."; if (value == null) { - throw new BadRequestException(errorMessage, ResponseCodeConstants.INVALID_SYNTAX); + throw new BadRequestException( + "Updating groups of the role by display name is not supported. Update using group id instead.", + ResponseCodeConstants.INVALID_SYNTAX); } switch (groupOperation.getOperation()) { From 096187839928323f15d114316c0d70d23199e786 Mon Sep 17 00:00:00 2001 From: Bimsara Bodaragama Date: Mon, 1 Jul 2024 22:22:50 +0530 Subject: [PATCH 6/6] Refactor prepareInitialGroupLists to use StringUtils for null and empty check (fixes #20334) --- .../wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java | 2 +- .../carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java index 120853f5..889fbb6a 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java @@ -776,7 +776,7 @@ private void prepareAddedRemovedGroupLists(Set addedGroupsIds, Set givenAddedGroupsIds, Set