Merge pull request wso2-extensions#176 from vinumaddumage/fix-roles-p…

…atch-filter Fix org role patch remove operation with no filter expression
dewniMW · Nov 2, 2022 · d721761 · d721761
2 parents f47713d + 0741458
commit d721761
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 18 deletions.
diff --git a/...wso2/carbon/identity/organization/management/role/management/service/RoleManagerImpl.java b/...wso2/carbon/identity/organization/management/role/management/service/RoleManagerImpl.java
@@ -59,12 +59,12 @@
 import static org.wso2.carbon.identity.organization.management.role.management.service.constant.RoleManagementConstants.USERS;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_GETTING_GROUP_VALIDITY;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_INVALID_ATTRIBUTE_PATCHING;
-import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_INVALID_FILTER_FORMAT;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_INVALID_GROUP_ID;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_INVALID_ORGANIZATION;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_INVALID_ROLE;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_INVALID_USER_ID;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_REMOVING_REQUIRED_ATTRIBUTE;
+import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_RETRIEVING_ORG_ROLES_INVALID_FILTER_FORMAT;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_ROLE_DISPLAY_NAME_ALREADY_EXISTS;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_ROLE_DISPLAY_NAME_MULTIPLE_VALUES;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_ROLE_DISPLAY_NAME_NULL;
@@ -289,7 +289,7 @@ private void getExpressionNodes(String filter, List<ExpressionNode> expressionNo
                 Utils.setExpressionNodeAndOperatorLists(rootNode, expressionNodes, operators, true);
             }
         } catch (IOException | IdentityException e) {
-            throw handleClientException(ERROR_CODE_INVALID_FILTER_FORMAT);
+            throw handleClientException(ERROR_CODE_RETRIEVING_ORG_ROLES_INVALID_FILTER_FORMAT, filter);
         }
     }
 

diff --git a/...n/identity/organization/management/role/management/service/dao/RoleManagementDAOImpl.java b/...n/identity/organization/management/role/management/service/dao/RoleManagementDAOImpl.java
@@ -164,7 +164,7 @@
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_GETTING_ROLE_FROM_ORGANIZATION_ID_ROLE_NAME;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_GETTING_USERS_USING_ROLE_ID;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_INVALID_ATTRIBUTE;
-import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_INVALID_FILTER_FORMAT;
+import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_ORG_ROLE_PATCH_REMOVE_OPERATION_INVALID_FILTER_FORMAT;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_PATCHING_ROLE;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_REMOVING_GROUPS_FROM_ROLE;
 import static org.wso2.carbon.identity.organization.management.service.constant.OrganizationManagementConstants.ErrorMessages.ERROR_CODE_REMOVING_PERMISSIONS_FROM_ROLE;
@@ -713,22 +713,25 @@ private void patchOperationReplace(String roleId, String path, List<String> valu
     private void patchOperationRemove(String roleId, String path) throws OrganizationManagementException {
 
         String patchPath = path;
+        String pathFilterExpression = StringUtils.EMPTY;
         if (patchPath.contains("[")) {
-            patchPath = patchPath.split("\\[")[0];
+            if (StringUtils.strip(patchPath).endsWith("]")) {
+                patchPath = StringUtils.strip(patchPath.split("\\[")[0]);
+                //get the filter expression associated with the path.
+                pathFilterExpression = StringUtils.strip(path.split("\\[")[1].replace("]", "")).toLowerCase();
+            } else {
+                throw handleClientException(ERROR_CODE_ORG_ROLE_PATCH_REMOVE_OPERATION_INVALID_FILTER_FORMAT,
+                        StringUtils.strip(path.substring(path.indexOf("[") + 1)), roleId);
+            }
         }
-        patchPath = StringUtils.strip(patchPath);
-
-        //get the values associated with the path.
-        String pathValues = StringUtils.strip(path.split("\\[")[1].replace("]", ""))
-                .toLowerCase();
 
-        if (StringUtils.isNotBlank(pathValues)) {
+        if (StringUtils.isNotBlank(pathFilterExpression)) {
             if (StringUtils.equalsIgnoreCase(patchPath, GROUPS)) {
-                patchRemoveOpWithFilters(roleId, pathValues, GROUPS);
+                patchRemoveOpWithFilters(roleId, pathFilterExpression, GROUPS);
             } else if (StringUtils.equalsIgnoreCase(patchPath, USERS)) {
-                patchRemoveOpWithFilters(roleId, pathValues, USERS);
+                patchRemoveOpWithFilters(roleId, pathFilterExpression, USERS);
             } else if (StringUtils.equalsIgnoreCase(patchPath, PERMISSIONS)) {
-                patchRemoveOpWithFilters(roleId, pathValues, PERMISSIONS);
+                patchRemoveOpWithFilters(roleId, pathFilterExpression, PERMISSIONS);
             }
         } else {
             if (StringUtils.equalsIgnoreCase(patchPath, USERS)) {
@@ -886,12 +889,12 @@ private void appendFilterQuery(List<ExpressionNode> expressionNodes, List<String
      * The filtering for patch operation, remove operation.
      *
      * @param roleId The ID of the role.
-     * @param values The values passed along with path.
+     * @param filter The filter expression passed along with path.
      * @param path   The patch operation path.
      * @throws OrganizationManagementException This exception is thrown when an error occurs while retrieving
      *                                         the values.
      */
-    private void patchRemoveOpWithFilters(String roleId, String values, String path)
+    private void patchRemoveOpWithFilters(String roleId, String filter, String path)
             throws OrganizationManagementException {
 
         String filterDbColumn = StringUtils.EMPTY;
@@ -928,7 +931,7 @@ private void patchRemoveOpWithFilters(String roleId, String values, String path)
         List<ExpressionNode> expressionNodes = new ArrayList<>();
         List<String> operators = new ArrayList<>();
         try {
-            FilterTreeBuilder filterTreeBuilder = new FilterTreeBuilder(values);
+            FilterTreeBuilder filterTreeBuilder = new FilterTreeBuilder(filter);
             Node rootNode = filterTreeBuilder.buildTree();
             Utils.setExpressionNodeAndOperatorLists(rootNode, expressionNodes, operators, false);
             FilterQueryBuilder filterQueryBuilder = new FilterQueryBuilder();
@@ -949,7 +952,8 @@ private void patchRemoveOpWithFilters(String roleId, String values, String path)
                 }
             }
         } catch (IOException | IdentityException e) {
-            throw handleClientException(ERROR_CODE_INVALID_FILTER_FORMAT);
+            throw handleClientException(ERROR_CODE_ORG_ROLE_PATCH_REMOVE_OPERATION_INVALID_FILTER_FORMAT, filter,
+                    roleId);
         }
     }
 

diff --git a/pom.xml b/pom.xml
@@ -398,7 +398,7 @@
         <org.wso2.identity.organization.mgt.imp.pkg.version.range>[1.0.0,2.0.0)
         </org.wso2.identity.organization.mgt.imp.pkg.version.range>
 
-        <identity.organization.management.core.version>1.0.17</identity.organization.management.core.version>
+        <identity.organization.management.core.version>1.0.28</identity.organization.management.core.version>
         <org.wso2.identity.organization.mgt.core.imp.pkg.version.range>[1.0.0,2.0.0)
         </org.wso2.identity.organization.mgt.core.imp.pkg.version.range>