From 5fc1c81ece75a18648000dec5a23bef5459cb8f2 Mon Sep 17 00:00:00 2001 From: Pasindu Yeshan <61885844+PasinduYeshan@users.noreply.github.com> Date: Thu, 12 Oct 2023 11:00:59 +0530 Subject: [PATCH 1/6] Add support for bulkID in SCIM2 --- .../charon3/core/encoder/JSONDecoder.java | 14 +- .../core/protocol/BulkRequestProcessor.java | 262 ++++++++++++++---- 2 files changed, 222 insertions(+), 54 deletions(-) diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java index 118dbbcbd..8225e836c 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java @@ -15,6 +15,7 @@ */ package org.wso2.charon3.core.encoder; +import org.apache.commons.lang.StringUtils; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -53,8 +54,10 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static org.wso2.charon3.core.schema.SCIMDefinitions.DataType.BINARY; import static org.wso2.charon3.core.schema.SCIMDefinitions.DataType.BOOLEAN; @@ -881,6 +884,7 @@ public BulkRequestData decodeBulkData(String bulkResourceString) throws BadReque List rolesEndpointOperationList = new ArrayList<>(); int failOnErrorsAttribute; List schemas = new ArrayList<>(); + Set encounteredBulkIds = new HashSet<>(); JSONObject decodedObject; try { @@ -917,9 +921,15 @@ public BulkRequestData decodeBulkData(String bulkResourceString) throws BadReque if (requestMethod.equals(SCIMConstants.OperationalConstants.POST)) { - if (!member.optString(SCIMConstants.OperationalConstants.BULK_ID).equals("") && - member.optString(SCIMConstants.OperationalConstants.BULK_ID) != null) { + String bulkId = member.optString(SCIMConstants.OperationalConstants.BULK_ID); + if (StringUtils.isNotEmpty(bulkId)) { + if (encounteredBulkIds.contains(bulkId)) { + String error = "Duplicate bulkId found: " + bulkId; + logger.error(error); + throw new BadRequestException(error, ResponseCodeConstants.INVALID_VALUE); + } + encounteredBulkIds.add(bulkId); setRequestData(requestType, requestMethod, requestVersion, member, usersEndpointOperationList, groupsEndpointOperationList, rolesEndpointOperationList); } else { diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java index 7a0a1624f..2dd6440c2 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java @@ -17,6 +17,10 @@ */ package org.wso2.charon3.core.protocol; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; + import org.wso2.charon3.core.exceptions.BadRequestException; import org.wso2.charon3.core.extensions.RoleManager; import org.wso2.charon3.core.extensions.UserManager; @@ -30,6 +34,10 @@ import org.wso2.charon3.core.protocol.endpoints.UserResourceManager; import org.wso2.charon3.core.schema.SCIMConstants; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + /** * */ @@ -130,24 +138,30 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr } } + Map userIdMappings = getUserIdBulkIdMapping(bulkResponseData.getUserOperationResponse()); + for (BulkRequestContent bulkRequestContent : bulkRequestData.getGroupOperationRequests()) { if (failOnError == 0) { bulkResponseData.addGroupOperation - (getBulkResponseContent(bulkRequestContent, groupResourceManager)); + (getBulkResponseContent(bulkRequestContent, + userIdMappings, groupResourceManager)); } else { if (errors < failOnError) { bulkResponseData.addGroupOperation - (getBulkResponseContent(bulkRequestContent, groupResourceManager)); + (getBulkResponseContent(bulkRequestContent, + userIdMappings, groupResourceManager)); } } } for (BulkRequestContent bulkRequestContent : bulkRequestData.getRoleOperationRequests()) { if (failOnError == 0) { - bulkResponseData.addRoleOperation(getBulkResponseContent(bulkRequestContent, roleResourceManager)); + bulkResponseData.addRoleOperation(getBulkResponseContent(bulkRequestContent, userIdMappings, + roleResourceManager)); } else { if (errors < failOnError) { - bulkResponseData.addRoleOperation(getBulkResponseContent(bulkRequestContent, roleResourceManager)); + bulkResponseData.addRoleOperation(getBulkResponseContent(bulkRequestContent, + userIdMappings, roleResourceManager)); } } } @@ -155,66 +169,83 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr return bulkResponseData; } + private BulkResponseContent getBulkResponseContent + (BulkRequestContent bulkRequestContent, ResourceManager resourceManager) + throws BadRequestException { + return getBulkResponseContent(bulkRequestContent, null, resourceManager); + } private BulkResponseContent getBulkResponseContent - (BulkRequestContent bulkRequestContent, ResourceManager resourceManager) + (BulkRequestContent bulkRequestContent, Map userIdMappings, + ResourceManager resourceManager) throws BadRequestException { BulkResponseContent bulkResponseContent = null; SCIMResponse response; - if (bulkRequestContent.getMethod().equals(SCIMConstants.OperationalConstants.POST)) { - - if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.createRole(bulkRequestContent.getData(), roleManager); - } else { - response = resourceManager.create(bulkRequestContent.getData(), userManager, null, null); + processBulkRequestContent(bulkRequestContent, userIdMappings, bulkRequestContent.getMethod()); + + switch (bulkRequestContent.getMethod()) { + case SCIMConstants.OperationalConstants.POST: + if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { + response = resourceManager.createRole(bulkRequestContent.getData(), roleManager); + } else { + response = resourceManager.create(bulkRequestContent.getData(), userManager, + null, null); + } + + bulkResponseContent = createBulkResponseContent + (response, SCIMConstants.OperationalConstants.POST, bulkRequestContent); + errorsCheck(response); + break; + + case SCIMConstants.OperationalConstants.PUT: { + String resourceId = extractIDFromPath(bulkRequestContent.getPath()); + if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { + response = resourceManager.updateWithPUTRole(resourceId, + bulkRequestContent.getData(), roleManager); + } else { + response = resourceManager + .updateWithPUT(resourceId, bulkRequestContent.getData(), userManager, + null, null); + } + + bulkResponseContent = createBulkResponseContent + (response, SCIMConstants.OperationalConstants.PUT, bulkRequestContent); + errorsCheck(response); + break; } - bulkResponseContent = createBulkResponseContent - (response, SCIMConstants.OperationalConstants.POST, bulkRequestContent); - errorsCheck(response); - - } else if (bulkRequestContent.getMethod().equals(SCIMConstants.OperationalConstants.PUT)) { - - String resourceId = extractIDFromPath(bulkRequestContent.getPath()); - if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.updateWithPUTRole(resourceId, bulkRequestContent.getData(), roleManager); - } else { - response = resourceManager - .updateWithPUT(resourceId, bulkRequestContent.getData(), userManager, null, null); + case SCIMConstants.OperationalConstants.PATCH: { + String resourceId = extractIDFromPath(bulkRequestContent.getPath()); + if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { + response = resourceManager.updateWithPATCHRole(resourceId, + bulkRequestContent.getData(), roleManager); + } else { + response = resourceManager + .updateWithPATCH(resourceId, bulkRequestContent.getData(), userManager, + null, null); + } + + bulkResponseContent = createBulkResponseContent + (response, SCIMConstants.OperationalConstants.PATCH, bulkRequestContent); + errorsCheck(response); + break; } - bulkResponseContent = createBulkResponseContent - (response, SCIMConstants.OperationalConstants.PUT, bulkRequestContent); - errorsCheck(response); - - } else if (bulkRequestContent.getMethod().equals(SCIMConstants.OperationalConstants.PATCH)) { - - String resourceId = extractIDFromPath(bulkRequestContent.getPath()); - if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.updateWithPATCHRole(resourceId, bulkRequestContent.getData(), roleManager); - } else { - response = resourceManager - .updateWithPATCH(resourceId, bulkRequestContent.getData(), userManager, null, null); + case SCIMConstants.OperationalConstants.DELETE: { + String resourceId = extractIDFromPath(bulkRequestContent.getPath()); + if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { + response = resourceManager.deleteRole(resourceId, roleManager); + } else { + response = resourceManager.delete(resourceId, userManager); + } + + bulkResponseContent = createBulkResponseContent + (response, SCIMConstants.OperationalConstants.DELETE, bulkRequestContent); + errorsCheck(response); + break; } - - bulkResponseContent = createBulkResponseContent - (response, SCIMConstants.OperationalConstants.PATCH, bulkRequestContent); - errorsCheck(response); - - } else if (bulkRequestContent.getMethod().equals(SCIMConstants.OperationalConstants.DELETE)) { - - String resourceId = extractIDFromPath(bulkRequestContent.getPath()); - if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.deleteRole(resourceId, roleManager); - } else { - response = resourceManager.delete(resourceId, userManager); - } - - bulkResponseContent = createBulkResponseContent - (response, SCIMConstants.OperationalConstants.DELETE, bulkRequestContent); - errorsCheck(response); } return bulkResponseContent; } @@ -252,4 +283,131 @@ private void errorsCheck(SCIMResponse response) { } } + /** + * This method is used to process the bulk request content. + * This method will replace the bulk id with the created user id. + * + * @param bulkRequestContent Bulk request content + * @param userIDMappings User id bulk id mapping + * @param method HTTP method + * @throws BadRequestException Bad request exception + */ + private void processBulkRequestContent(BulkRequestContent bulkRequestContent, + Map userIDMappings, String method) + throws BadRequestException { + try { + if (userIDMappings == null + || userIDMappings.isEmpty() + || method.equals(SCIMConstants.OperationalConstants.DELETE)) { + return; + } + // Parse the data field to a JSON object. + JSONObject dataJson = new JSONObject(bulkRequestContent.getData()); + String usersOrMembersKey = getUsersOrMembersKey(bulkRequestContent.getPath()); + JSONArray usersArray = getUserArray(dataJson, method, usersOrMembersKey); + + if (usersArray != null) { + String bulkIdPrefix = SCIMConstants.OperationalConstants.BULK_ID + ":"; + + for (int i = 0; i < usersArray.length(); i++) { + JSONObject user = usersArray.getJSONObject(i); + String userValue = user.getString(SCIMConstants.OperationalConstants.VALUE); + if (userValue.startsWith(bulkIdPrefix)) { + String userBulkId = userValue.substring(bulkIdPrefix.length()); + String userId = userIDMappings.get(userBulkId); + if (userId != null) { + user.put(SCIMConstants.OperationalConstants.VALUE, userId); + } + } + } + } + bulkRequestContent.setData(dataJson.toString()); + + } catch (JSONException e) { + throw new BadRequestException("Error while parsing the data field of the bulk request content", + ResponseCodeConstants.INVALID_SYNTAX); + } + } + + private String getUsersOrMembersKey(String path) { + return path.contains(SCIMConstants.ROLE_ENDPOINT) ? SCIMConstants.RoleSchemaConstants.USERS : + SCIMConstants.GroupSchemaConstants.MEMBERS; + } + + /** + * This method is used to get the user array from the data JSON object. + * @param dataJson SCIM data JSON object + * @param method HTTP method + * @param usersOrMembersKey Users or members key + * + * @return User array + * @throws JSONException JSON exception + */ + private JSONArray getUserArray(JSONObject dataJson, String method, String usersOrMembersKey) + throws JSONException { + switch (method) { + case SCIMConstants.OperationalConstants.POST: + case SCIMConstants.OperationalConstants.PUT: + return dataJson.optJSONArray(usersOrMembersKey); + case SCIMConstants.OperationalConstants.PATCH: + return getUserArrayForPatch(dataJson, usersOrMembersKey); + default: + return null; + } + } + + private JSONArray getUserArrayForPatch(JSONObject dataJson, String usersOrMembersKey) throws JSONException { + JSONArray operations = dataJson.optJSONArray(SCIMConstants.OperationalConstants.OPERATIONS); + + if (operations != null) { + for (int i = 0; i < operations.length(); i++) { + JSONObject operation = operations.getJSONObject(i); + if (isAddOperation(operation, usersOrMembersKey)) { + if (operation.has(SCIMConstants.OperationalConstants.PATH)) { + return operation.optJSONArray(SCIMConstants.OperationalConstants.VALUE); + } else { + JSONObject valueObject = operation.optJSONObject(SCIMConstants.OperationalConstants.VALUE); + if (valueObject != null) { + return valueObject.optJSONArray(usersOrMembersKey); + } + } + } + } + } + return null; + } + + private boolean isAddOperation(JSONObject operation, String path) throws JSONException { + String operationType = operation.optString(SCIMConstants.OperationalConstants.OP); + String operationPath = operation.optString(SCIMConstants.OperationalConstants.PATH, ""); + + return SCIMConstants.OperationalConstants.ADD.equalsIgnoreCase(operationType) && + (operationPath.equals(path) || operationPath.isEmpty()); + } + + /** + * This method is used to get user id bulk id mapping from the bulk user operation response. + * @param bulkUserOperationResponse Bulk user operation response + * + * @return Bulk id user id mapping + */ + private static Map getUserIdBulkIdMapping(List bulkUserOperationResponse) { + Map userIdMappings = new HashMap<>(); + + for (BulkResponseContent bulkResponse : bulkUserOperationResponse) { + String bulkId = bulkResponse.getBulkID(); + + SCIMResponse response = bulkResponse.getScimResponse(); + if (response.getResponseStatus() == ResponseCodeConstants.CODE_CREATED) { + String locationHeader = response.getHeaderParamMap().get(SCIMConstants.LOCATION_HEADER); + + if (locationHeader != null) { + String[] locationHeaderParts = locationHeader.split("/"); + String userId = locationHeaderParts[locationHeaderParts.length - 1]; + userIdMappings.put(bulkId, userId); + } + } + } + return userIdMappings; + } } From 54d1fe5a8ff60a70d66c597500fcfd8c1096c601 Mon Sep 17 00:00:00 2001 From: Pasindu Yeshan <61885844+PasinduYeshan@users.noreply.github.com> Date: Thu, 12 Oct 2023 12:26:40 +0530 Subject: [PATCH 2/6] Reformat code --- .../core/protocol/BulkRequestProcessor.java | 82 +++++++++---------- 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java index 2dd6440c2..a2a3b7c2a 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java @@ -169,20 +169,19 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr return bulkResponseData; } - private BulkResponseContent getBulkResponseContent - (BulkRequestContent bulkRequestContent, ResourceManager resourceManager) - throws BadRequestException { + private BulkResponseContent getBulkResponseContent(BulkRequestContent bulkRequestContent, + ResourceManager resourceManager) throws BadRequestException { + return getBulkResponseContent(bulkRequestContent, null, resourceManager); } - private BulkResponseContent getBulkResponseContent - (BulkRequestContent bulkRequestContent, Map userIdMappings, - ResourceManager resourceManager) + private BulkResponseContent getBulkResponseContent(BulkRequestContent bulkRequestContent, + Map userIdMappings, + ResourceManager resourceManager) throws BadRequestException { BulkResponseContent bulkResponseContent = null; SCIMResponse response; - processBulkRequestContent(bulkRequestContent, userIdMappings, bulkRequestContent.getMethod()); switch (bulkRequestContent.getMethod()) { @@ -194,24 +193,23 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr null, null); } - bulkResponseContent = createBulkResponseContent - (response, SCIMConstants.OperationalConstants.POST, bulkRequestContent); + bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.POST, + bulkRequestContent); errorsCheck(response); break; case SCIMConstants.OperationalConstants.PUT: { String resourceId = extractIDFromPath(bulkRequestContent.getPath()); if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.updateWithPUTRole(resourceId, - bulkRequestContent.getData(), roleManager); + response = resourceManager.updateWithPUTRole(resourceId, bulkRequestContent.getData(), + roleManager); } else { - response = resourceManager - .updateWithPUT(resourceId, bulkRequestContent.getData(), userManager, + response = resourceManager.updateWithPUT(resourceId, bulkRequestContent.getData(), userManager, null, null); } - bulkResponseContent = createBulkResponseContent - (response, SCIMConstants.OperationalConstants.PUT, bulkRequestContent); + bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.PUT, + bulkRequestContent); errorsCheck(response); break; } @@ -219,16 +217,15 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr case SCIMConstants.OperationalConstants.PATCH: { String resourceId = extractIDFromPath(bulkRequestContent.getPath()); if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.updateWithPATCHRole(resourceId, - bulkRequestContent.getData(), roleManager); + response = resourceManager.updateWithPATCHRole(resourceId, bulkRequestContent.getData(), + roleManager); } else { - response = resourceManager - .updateWithPATCH(resourceId, bulkRequestContent.getData(), userManager, + response = resourceManager.updateWithPATCH(resourceId, bulkRequestContent.getData(), userManager, null, null); } - bulkResponseContent = createBulkResponseContent - (response, SCIMConstants.OperationalConstants.PATCH, bulkRequestContent); + bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.PATCH, + bulkRequestContent); errorsCheck(response); break; } @@ -241,8 +238,8 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr response = resourceManager.delete(resourceId, userManager); } - bulkResponseContent = createBulkResponseContent - (response, SCIMConstants.OperationalConstants.DELETE, bulkRequestContent); + bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.DELETE, + bulkRequestContent); errorsCheck(response); break; } @@ -251,6 +248,7 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr } private String extractIDFromPath(String path) throws BadRequestException { + String [] parts = path.split("[/]"); if (parts[2] != null) { return parts[2]; @@ -262,8 +260,8 @@ private String extractIDFromPath(String path) throws BadRequestException { private BulkResponseContent createBulkResponseContent(SCIMResponse response, String method, BulkRequestContent requestContent) { - BulkResponseContent bulkResponseContent = new BulkResponseContent(); + BulkResponseContent bulkResponseContent = new BulkResponseContent(); bulkResponseContent.setScimResponse(response); bulkResponseContent.setMethod(method); if (response.getHeaderParamMap() != null) { @@ -273,7 +271,6 @@ private BulkResponseContent createBulkResponseContent(SCIMResponse response, Str bulkResponseContent.setVersion(requestContent.getVersion()); return bulkResponseContent; - } private void errorsCheck(SCIMResponse response) { @@ -287,17 +284,16 @@ private void errorsCheck(SCIMResponse response) { * This method is used to process the bulk request content. * This method will replace the bulk id with the created user id. * - * @param bulkRequestContent Bulk request content - * @param userIDMappings User id bulk id mapping - * @param method HTTP method - * @throws BadRequestException Bad request exception + * @param bulkRequestContent Bulk request content. + * @param userIDMappings User id bulk id mapping. + * @param method HTTP method. + * @throws BadRequestException Bad request exception. */ - private void processBulkRequestContent(BulkRequestContent bulkRequestContent, - Map userIDMappings, String method) - throws BadRequestException { + private void processBulkRequestContent(BulkRequestContent bulkRequestContent, Map userIDMappings, + String method) throws BadRequestException { + try { - if (userIDMappings == null - || userIDMappings.isEmpty() + if (userIDMappings == null || userIDMappings.isEmpty() || method.equals(SCIMConstants.OperationalConstants.DELETE)) { return; } @@ -308,7 +304,6 @@ private void processBulkRequestContent(BulkRequestContent bulkRequestContent, if (usersArray != null) { String bulkIdPrefix = SCIMConstants.OperationalConstants.BULK_ID + ":"; - for (int i = 0; i < usersArray.length(); i++) { JSONObject user = usersArray.getJSONObject(i); String userValue = user.getString(SCIMConstants.OperationalConstants.VALUE); @@ -330,21 +325,23 @@ private void processBulkRequestContent(BulkRequestContent bulkRequestContent, } private String getUsersOrMembersKey(String path) { + return path.contains(SCIMConstants.ROLE_ENDPOINT) ? SCIMConstants.RoleSchemaConstants.USERS : SCIMConstants.GroupSchemaConstants.MEMBERS; } /** * This method is used to get the user array from the data JSON object. - * @param dataJson SCIM data JSON object - * @param method HTTP method + * @param dataJson SCIM data JSON object + * @param method HTTP method * @param usersOrMembersKey Users or members key * * @return User array - * @throws JSONException JSON exception + * @throws JSONException JSON exception */ private JSONArray getUserArray(JSONObject dataJson, String method, String usersOrMembersKey) throws JSONException { + switch (method) { case SCIMConstants.OperationalConstants.POST: case SCIMConstants.OperationalConstants.PUT: @@ -357,8 +354,8 @@ private JSONArray getUserArray(JSONObject dataJson, String method, String usersO } private JSONArray getUserArrayForPatch(JSONObject dataJson, String usersOrMembersKey) throws JSONException { - JSONArray operations = dataJson.optJSONArray(SCIMConstants.OperationalConstants.OPERATIONS); + JSONArray operations = dataJson.optJSONArray(SCIMConstants.OperationalConstants.OPERATIONS); if (operations != null) { for (int i = 0; i < operations.length(); i++) { JSONObject operation = operations.getJSONObject(i); @@ -378,6 +375,7 @@ private JSONArray getUserArrayForPatch(JSONObject dataJson, String usersOrMember } private boolean isAddOperation(JSONObject operation, String path) throws JSONException { + String operationType = operation.optString(SCIMConstants.OperationalConstants.OP); String operationPath = operation.optString(SCIMConstants.OperationalConstants.PATH, ""); @@ -387,13 +385,13 @@ private boolean isAddOperation(JSONObject operation, String path) throws JSONExc /** * This method is used to get user id bulk id mapping from the bulk user operation response. - * @param bulkUserOperationResponse Bulk user operation response + * @param bulkUserOperationResponse Bulk user operation response. * - * @return Bulk id user id mapping + * @return Bulk id user id mapping. */ private static Map getUserIdBulkIdMapping(List bulkUserOperationResponse) { - Map userIdMappings = new HashMap<>(); + Map userIdMappings = new HashMap<>(); for (BulkResponseContent bulkResponse : bulkUserOperationResponse) { String bulkId = bulkResponse.getBulkID(); From a6b8019b3ef7e6b528e9c67d114258a239271068 Mon Sep 17 00:00:00 2001 From: Pasindu Yeshan <61885844+PasinduYeshan@users.noreply.github.com> Date: Thu, 12 Oct 2023 13:31:26 +0530 Subject: [PATCH 3/6] Refactor code --- .../charon3/core/protocol/BulkRequestProcessor.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java index a2a3b7c2a..9abca4978 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java @@ -17,6 +17,7 @@ */ package org.wso2.charon3.core.protocol; +import org.apache.commons.lang.StringUtils; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -310,7 +311,7 @@ private void processBulkRequestContent(BulkRequestContent bulkRequestContent, Ma if (userValue.startsWith(bulkIdPrefix)) { String userBulkId = userValue.substring(bulkIdPrefix.length()); String userId = userIDMappings.get(userBulkId); - if (userId != null) { + if (StringUtils.isNotBlank(userId)) { user.put(SCIMConstants.OperationalConstants.VALUE, userId); } } @@ -332,12 +333,12 @@ private String getUsersOrMembersKey(String path) { /** * This method is used to get the user array from the data JSON object. - * @param dataJson SCIM data JSON object - * @param method HTTP method - * @param usersOrMembersKey Users or members key * + * @param dataJson SCIM data JSON object. + * @param method HTTP method. + * @param usersOrMembersKey Users or members key. * @return User array - * @throws JSONException JSON exception + * @throws JSONException JSON exception. */ private JSONArray getUserArray(JSONObject dataJson, String method, String usersOrMembersKey) throws JSONException { @@ -385,8 +386,8 @@ private boolean isAddOperation(JSONObject operation, String path) throws JSONExc /** * This method is used to get user id bulk id mapping from the bulk user operation response. - * @param bulkUserOperationResponse Bulk user operation response. * + * @param bulkUserOperationResponse Bulk user operation response. * @return Bulk id user id mapping. */ private static Map getUserIdBulkIdMapping(List bulkUserOperationResponse) { From 9fbebce161812255ed2ea0cfa0dfee1dce78cd1d Mon Sep 17 00:00:00 2001 From: Pasindu Yeshan <61885844+PasinduYeshan@users.noreply.github.com> Date: Sun, 15 Oct 2023 11:20:43 +0530 Subject: [PATCH 4/6] Remove log --- .../src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java | 1 - .../org/wso2/charon3/core/protocol/BulkRequestProcessor.java | 1 - 2 files changed, 2 deletions(-) diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java index 8225e836c..579e7837e 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/encoder/JSONDecoder.java @@ -926,7 +926,6 @@ public BulkRequestData decodeBulkData(String bulkResourceString) throws BadReque if (StringUtils.isNotEmpty(bulkId)) { if (encounteredBulkIds.contains(bulkId)) { String error = "Duplicate bulkId found: " + bulkId; - logger.error(error); throw new BadRequestException(error, ResponseCodeConstants.INVALID_VALUE); } encounteredBulkIds.add(bulkId); diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java index 9abca4978..40a864b5d 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java @@ -318,7 +318,6 @@ private void processBulkRequestContent(BulkRequestContent bulkRequestContent, Ma } } bulkRequestContent.setData(dataJson.toString()); - } catch (JSONException e) { throw new BadRequestException("Error while parsing the data field of the bulk request content", ResponseCodeConstants.INVALID_SYNTAX); From 561431f02d015ea8f198af329cb918498df83538 Mon Sep 17 00:00:00 2001 From: Pasindu Yeshan <61885844+PasinduYeshan@users.noreply.github.com> Date: Tue, 17 Oct 2023 14:45:49 +0530 Subject: [PATCH 5/6] Refactor code --- .../core/protocol/BulkRequestProcessor.java | 217 +++++++++--------- .../charon3/core/schema/SCIMConstants.java | 2 + 2 files changed, 107 insertions(+), 112 deletions(-) diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java index 40a864b5d..c88770894 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java @@ -51,7 +51,6 @@ public class BulkRequestProcessor { private UserManager userManager; private RoleManager roleManager; - public UserResourceManager getUserResourceManager() { return userResourceManager; } @@ -129,12 +128,10 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr for (BulkRequestContent bulkRequestContent : bulkRequestData.getUserOperationRequests()) { if (failOnError == 0) { - bulkResponseData.addUserOperation - (getBulkResponseContent(bulkRequestContent, userResourceManager)); + bulkResponseData.addUserOperation(getBulkResponseContent(bulkRequestContent, userResourceManager)); } else { if (errors < failOnError) { - bulkResponseData.addUserOperation - (getBulkResponseContent(bulkRequestContent, userResourceManager)); + bulkResponseData.addUserOperation(getBulkResponseContent(bulkRequestContent, userResourceManager)); } } @@ -143,14 +140,12 @@ public BulkResponseData processBulkRequests(BulkRequestData bulkRequestData) thr for (BulkRequestContent bulkRequestContent : bulkRequestData.getGroupOperationRequests()) { if (failOnError == 0) { - bulkResponseData.addGroupOperation - (getBulkResponseContent(bulkRequestContent, - userIdMappings, groupResourceManager)); + bulkResponseData.addGroupOperation( + getBulkResponseContent(bulkRequestContent, userIdMappings, groupResourceManager)); } else { if (errors < failOnError) { - bulkResponseData.addGroupOperation - (getBulkResponseContent(bulkRequestContent, - userIdMappings, groupResourceManager)); + bulkResponseData.addGroupOperation( + getBulkResponseContent(bulkRequestContent, userIdMappings, groupResourceManager)); } } @@ -176,77 +171,77 @@ private BulkResponseContent getBulkResponseContent(BulkRequestContent bulkReques return getBulkResponseContent(bulkRequestContent, null, resourceManager); } - private BulkResponseContent getBulkResponseContent(BulkRequestContent bulkRequestContent, - Map userIdMappings, - ResourceManager resourceManager) - throws BadRequestException { - - BulkResponseContent bulkResponseContent = null; - SCIMResponse response; - processBulkRequestContent(bulkRequestContent, userIdMappings, bulkRequestContent.getMethod()); - - switch (bulkRequestContent.getMethod()) { - case SCIMConstants.OperationalConstants.POST: - if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.createRole(bulkRequestContent.getData(), roleManager); - } else { - response = resourceManager.create(bulkRequestContent.getData(), userManager, - null, null); - } - - bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.POST, - bulkRequestContent); - errorsCheck(response); - break; - - case SCIMConstants.OperationalConstants.PUT: { - String resourceId = extractIDFromPath(bulkRequestContent.getPath()); - if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.updateWithPUTRole(resourceId, bulkRequestContent.getData(), - roleManager); - } else { - response = resourceManager.updateWithPUT(resourceId, bulkRequestContent.getData(), userManager, - null, null); - } - - bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.PUT, - bulkRequestContent); - errorsCheck(response); - break; - } - - case SCIMConstants.OperationalConstants.PATCH: { - String resourceId = extractIDFromPath(bulkRequestContent.getPath()); - if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.updateWithPATCHRole(resourceId, bulkRequestContent.getData(), - roleManager); - } else { - response = resourceManager.updateWithPATCH(resourceId, bulkRequestContent.getData(), userManager, - null, null); - } - - bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.PATCH, - bulkRequestContent); - errorsCheck(response); - break; - } - - case SCIMConstants.OperationalConstants.DELETE: { - String resourceId = extractIDFromPath(bulkRequestContent.getPath()); - if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { - response = resourceManager.deleteRole(resourceId, roleManager); - } else { - response = resourceManager.delete(resourceId, userManager); - } - - bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.DELETE, - bulkRequestContent); - errorsCheck(response); - break; - } - } - return bulkResponseContent; - } + private BulkResponseContent getBulkResponseContent(BulkRequestContent bulkRequestContent, + Map userIdMappings, + ResourceManager resourceManager) + throws BadRequestException { + + BulkResponseContent bulkResponseContent = null; + SCIMResponse response; + processBulkRequestContent(bulkRequestContent, userIdMappings, bulkRequestContent.getMethod()); + + switch (bulkRequestContent.getMethod()) { + case SCIMConstants.OperationalConstants.POST: + if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { + response = resourceManager.createRole(bulkRequestContent.getData(), roleManager); + } else { + response = resourceManager.create(bulkRequestContent.getData(), userManager, + null, null); + } + + bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.POST, + bulkRequestContent); + errorsCheck(response); + break; + + case SCIMConstants.OperationalConstants.PUT: { + String resourceId = extractIDFromPath(bulkRequestContent.getPath()); + if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { + response = resourceManager.updateWithPUTRole(resourceId, bulkRequestContent.getData(), + roleManager); + } else { + response = resourceManager.updateWithPUT(resourceId, bulkRequestContent.getData(), userManager, + null, null); + } + + bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.PUT, + bulkRequestContent); + errorsCheck(response); + break; + } + + case SCIMConstants.OperationalConstants.PATCH: { + String resourceId = extractIDFromPath(bulkRequestContent.getPath()); + if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { + response = resourceManager.updateWithPATCHRole(resourceId, bulkRequestContent.getData(), + roleManager); + } else { + response = resourceManager.updateWithPATCH(resourceId, bulkRequestContent.getData(), userManager, + null, null); + } + + bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.PATCH, + bulkRequestContent); + errorsCheck(response); + break; + } + + case SCIMConstants.OperationalConstants.DELETE: { + String resourceId = extractIDFromPath(bulkRequestContent.getPath()); + if (bulkRequestContent.getPath().contains(SCIMConstants.ROLE_ENDPOINT)) { + response = resourceManager.deleteRole(resourceId, roleManager); + } else { + response = resourceManager.delete(resourceId, userManager); + } + + bulkResponseContent = createBulkResponseContent(response, SCIMConstants.OperationalConstants.DELETE, + bulkRequestContent); + errorsCheck(response); + break; + } + } + return bulkResponseContent; + } private String extractIDFromPath(String path) throws BadRequestException { @@ -294,17 +289,18 @@ private void processBulkRequestContent(BulkRequestContent bulkRequestContent, Ma String method) throws BadRequestException { try { - if (userIDMappings == null || userIDMappings.isEmpty() - || method.equals(SCIMConstants.OperationalConstants.DELETE)) { + if (userIDMappings == null || userIDMappings.isEmpty() || + method.equals(SCIMConstants.OperationalConstants.DELETE)) { return; } - // Parse the data field to a JSON object. + JSONObject dataJson = new JSONObject(bulkRequestContent.getData()); String usersOrMembersKey = getUsersOrMembersKey(bulkRequestContent.getPath()); JSONArray usersArray = getUserArray(dataJson, method, usersOrMembersKey); if (usersArray != null) { - String bulkIdPrefix = SCIMConstants.OperationalConstants.BULK_ID + ":"; + String bulkIdPrefix = + SCIMConstants.OperationalConstants.BULK_ID + SCIMConstants.OperationalConstants.COLON; for (int i = 0; i < usersArray.length(); i++) { JSONObject user = usersArray.getJSONObject(i); String userValue = user.getString(SCIMConstants.OperationalConstants.VALUE); @@ -336,37 +332,32 @@ private String getUsersOrMembersKey(String path) { * @param dataJson SCIM data JSON object. * @param method HTTP method. * @param usersOrMembersKey Users or members key. - * @return User array + * @return User array. * @throws JSONException JSON exception. */ - private JSONArray getUserArray(JSONObject dataJson, String method, String usersOrMembersKey) - throws JSONException { + private JSONArray getUserArray(JSONObject dataJson, String method, String usersOrMembersKey) throws JSONException { - switch (method) { - case SCIMConstants.OperationalConstants.POST: - case SCIMConstants.OperationalConstants.PUT: - return dataJson.optJSONArray(usersOrMembersKey); - case SCIMConstants.OperationalConstants.PATCH: - return getUserArrayForPatch(dataJson, usersOrMembersKey); - default: - return null; + if (SCIMConstants.OperationalConstants.PATCH.equals(method)) { + return getUserArrayForPatch(dataJson, usersOrMembersKey); } + return dataJson.optJSONArray(usersOrMembersKey); } private JSONArray getUserArrayForPatch(JSONObject dataJson, String usersOrMembersKey) throws JSONException { JSONArray operations = dataJson.optJSONArray(SCIMConstants.OperationalConstants.OPERATIONS); - if (operations != null) { - for (int i = 0; i < operations.length(); i++) { - JSONObject operation = operations.getJSONObject(i); - if (isAddOperation(operation, usersOrMembersKey)) { - if (operation.has(SCIMConstants.OperationalConstants.PATH)) { - return operation.optJSONArray(SCIMConstants.OperationalConstants.VALUE); - } else { - JSONObject valueObject = operation.optJSONObject(SCIMConstants.OperationalConstants.VALUE); - if (valueObject != null) { - return valueObject.optJSONArray(usersOrMembersKey); - } + if (operations == null) { + return null; + } + for (int i = 0; i < operations.length(); i++) { + JSONObject operation = operations.getJSONObject(i); + if (isValidOperation(operation, usersOrMembersKey)) { + if (operation.has(SCIMConstants.OperationalConstants.PATH)) { + return operation.optJSONArray(SCIMConstants.OperationalConstants.VALUE); + } else { + JSONObject valueObject = operation.optJSONObject(SCIMConstants.OperationalConstants.VALUE); + if (valueObject != null) { + return valueObject.optJSONArray(usersOrMembersKey); } } } @@ -374,12 +365,13 @@ private JSONArray getUserArrayForPatch(JSONObject dataJson, String usersOrMember return null; } - private boolean isAddOperation(JSONObject operation, String path) throws JSONException { + private boolean isValidOperation(JSONObject operation, String path) throws JSONException { String operationType = operation.optString(SCIMConstants.OperationalConstants.OP); - String operationPath = operation.optString(SCIMConstants.OperationalConstants.PATH, ""); + String operationPath = operation.optString(SCIMConstants.OperationalConstants.PATH, StringUtils.EMPTY); - return SCIMConstants.OperationalConstants.ADD.equalsIgnoreCase(operationType) && + return (SCIMConstants.OperationalConstants.ADD.equalsIgnoreCase(operationType) || + SCIMConstants.OperationalConstants.REPLACE.equalsIgnoreCase(operationType)) && (operationPath.equals(path) || operationPath.isEmpty()); } @@ -400,7 +392,8 @@ private static Map getUserIdBulkIdMapping(List Date: Fri, 20 Oct 2023 15:12:19 +0530 Subject: [PATCH 6/6] Refactor code --- .../charon3/core/protocol/BulkRequestProcessor.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java index c88770894..2875527ef 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/BulkRequestProcessor.java @@ -290,7 +290,7 @@ private void processBulkRequestContent(BulkRequestContent bulkRequestContent, Ma try { if (userIDMappings == null || userIDMappings.isEmpty() || - method.equals(SCIMConstants.OperationalConstants.DELETE)) { + SCIMConstants.OperationalConstants.DELETE.equals(method)) { return; } @@ -354,11 +354,10 @@ private JSONArray getUserArrayForPatch(JSONObject dataJson, String usersOrMember if (isValidOperation(operation, usersOrMembersKey)) { if (operation.has(SCIMConstants.OperationalConstants.PATH)) { return operation.optJSONArray(SCIMConstants.OperationalConstants.VALUE); - } else { - JSONObject valueObject = operation.optJSONObject(SCIMConstants.OperationalConstants.VALUE); - if (valueObject != null) { - return valueObject.optJSONArray(usersOrMembersKey); - } + } + JSONObject valueObject = operation.optJSONObject(SCIMConstants.OperationalConstants.VALUE); + if (valueObject != null) { + return valueObject.optJSONArray(usersOrMembersKey); } } }