Fix SCIM error message for invalid request values in V1 and V2 Roles …

…Remove-Add-Replace operations (fixes #20334)
wso2-extensions · Jul 1, 2024 · b974ba9 · b974ba9
1 parent d18b5ae
commit b974ba9
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 20 deletions.
diff --git a/...cim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java b/...cim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManager.java
@@ -771,21 +771,30 @@ private void updatePermissions(String roleId, List<PatchOperation> permissionOpe
 
     private void prepareAddedRemovedGroupLists(Set<String> addedGroupsIds, Set<String> removedGroupsIds,
                                                Set<String> replacedGroupsIds, PatchOperation groupOperation,
-                                               Map<String, String> groupObject, List<GroupBasicInfo> groupListOfRole) {
+                                               Map<String, String> groupObject, List<GroupBasicInfo> 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/...m2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java b/...m2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMRoleManagerV2.java
@@ -1125,13 +1125,7 @@ private void doUpdateGroups(String roleId, Set<String> newGroupIDList, Set<Strin
                         new ArrayList<>(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<String> getUserIDList(List<String> userList, String tenantDomain) t
 
     private void prepareInitialGroupLists(Set<String> givenAddedGroupsIds, Set<String> givenRemovedGroupsIds,
                                           Set<String> givenReplacedGroupsIds, PatchOperation groupOperation,
-                                          Map<String, String> groupObject) {
+                                          Map<String, String> 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;