diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/MeResourceManager.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/MeResourceManager.java index 9f5714f8e..0e28e1144 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/MeResourceManager.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/MeResourceManager.java @@ -16,6 +16,7 @@ package org.wso2.charon3.core.protocol.endpoints; +import org.apache.commons.lang.StringUtils; import org.wso2.charon3.core.encoder.JSONDecoder; import org.wso2.charon3.core.encoder.JSONEncoder; import org.wso2.charon3.core.exceptions.BadRequestException; @@ -38,10 +39,14 @@ import org.wso2.charon3.core.utils.ResourceManagerUtil; import org.wso2.charon3.core.utils.codeutils.PatchOperation; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.wso2.charon3.core.schema.SCIMConstants.OperationalConstants.COLON; +import static org.wso2.charon3.core.utils.PatchOperationUtil.determineScimAttributes; + /** * A client MAY use a URL of the form "/Me" as a uri alias for * the User or other resource associated with the currently @@ -323,6 +328,8 @@ public SCIMResponse updateWithPATCH(String existingId, String scimObjectString, User newUser = null; + Map syncedAttributes = userManager.getSyncedUserAttributes(); + List deletedSyncedAttributes = new ArrayList<>(); for (PatchOperation operation : opList) { if (operation.getOperation().equals(SCIMConstants.OperationalConstants.ADD)) { @@ -338,13 +345,29 @@ public SCIMResponse updateWithPATCH(String existingId, String scimObjectString, } } else if (operation.getOperation().equals(SCIMConstants.OperationalConstants.REMOVE)) { - if (newUser == null) { - newUser = (User) PatchOperationUtil.doPatchRemove(operation, oldUser, copyOfOldUser, schema); - copyOfOldUser = (User) CopyUtil.deepCopy(newUser); - - } else { - newUser = (User) PatchOperationUtil.doPatchRemove(operation, newUser, copyOfOldUser, schema); - copyOfOldUser = (User) CopyUtil.deepCopy(newUser); + try { + if (newUser == null) { + newUser = (User) PatchOperationUtil.doPatchRemove(operation, oldUser, copyOfOldUser, + schema); + copyOfOldUser = (User) CopyUtil.deepCopy(newUser); + + } else { + newUser = (User) PatchOperationUtil.doPatchRemove(operation, newUser, copyOfOldUser, + schema); + copyOfOldUser = (User) CopyUtil.deepCopy(newUser); + } + } catch (BadRequestException e) { + /* + * This condition is for the migrated users who have enterprise user attributes and system + * schema attributes both mapped to a single local claim. In such cases, if both the scim + * attributes are specified to be removed in the patch request, for the second attribute, there + * will be an error thrown because it is already removed when processing the first attribute. + */ + if (!ResponseCodeConstants.INVALID_PATH.equals(e.getScimType()) || + determineScimAttributes(operation).stream() + .noneMatch(deletedSyncedAttributes::contains)) { + throw e; + } } } else if (operation.getOperation().equals(SCIMConstants.OperationalConstants.REPLACE)) { if (newUser == null) { @@ -360,6 +383,47 @@ public SCIMResponse updateWithPATCH(String existingId, String scimObjectString, } else { throw new BadRequestException("Unknown operation.", ResponseCodeConstants.INVALID_SYNTAX); } + + /* + * The following logic is for the migrated users who have enterprise user attributes and system schema + * attributes both mapped to a single local claim. In such cases, if any operation is only sent for + * one scim attributes, there will be conflicts for the final value because the other scim attribute's + * value is not changed. Therefore, we are removing the other scim attribute from the user object. + */ + if (syncedAttributes.isEmpty()) { + continue; + } + List scimAttributes = determineScimAttributes(operation); + for (String scimAttribute : scimAttributes) { + String syncedAttribute = syncedAttributes.get(scimAttribute); + + if (syncedAttribute == null) { + continue; + } + + int lastColonIndex = syncedAttribute.lastIndexOf(COLON); + String baseAttributeName = (lastColonIndex != -1) + ? syncedAttribute.substring(0, lastColonIndex) : StringUtils.EMPTY; + String subAttributeName = (lastColonIndex != -1) + ? syncedAttribute.substring(lastColonIndex + 1) : syncedAttribute; + String[] subAttributes = subAttributeName.split("\\."); + + switch (subAttributes.length) { + case 1: + newUser.deleteSubAttribute(baseAttributeName, subAttributes[0]); + deletedSyncedAttributes.add(syncedAttribute); + break; + case 2: + newUser.deleteSubSubAttribute(baseAttributeName, subAttributes[0], subAttributes[1]); + deletedSyncedAttributes.add(syncedAttribute); + break; + default: + break; + } + + copyOfOldUser = (User) CopyUtil.deepCopy(newUser); + syncedAttributes.remove(syncedAttribute); + } } //get the URIs of required attributes which must be given a value diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/UserResourceManager.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/UserResourceManager.java index b228d98f2..1b1c93c83 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/UserResourceManager.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/UserResourceManager.java @@ -18,8 +18,6 @@ import org.apache.commons.lang.StringUtils; -import org.json.JSONArray; -import org.json.JSONObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.wso2.charon3.core.attributes.Attribute; @@ -52,16 +50,13 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import static org.wso2.charon3.core.schema.SCIMConstants.OperationalConstants.COLON; -import static org.wso2.charon3.core.schema.SCIMConstants.OperationalConstants.DOT_SEPARATOR; -import static org.wso2.charon3.core.schema.SCIMConstants.OperationalConstants.OPEN_SQUARE_BRACKET; +import static org.wso2.charon3.core.utils.PatchOperationUtil.determineScimAttributes; /** * REST API exposed by Charon-Core to perform operations on UserResource. @@ -855,71 +850,4 @@ private SCIMResourceTypeSchema getSchema(UserManager userManager) throws BadRequ } return schema; } - - private static List determineScimAttributes(PatchOperation operation) { - - if (operation == null) { - return Collections.emptyList(); - } - List attributes = new ArrayList<>(); - String path = operation.getPath(); - if (path != null) { - int bracketIndex = path.indexOf(OPEN_SQUARE_BRACKET); - path = (bracketIndex != -1) ? path.substring(0, bracketIndex) : path; - } - - Object values = operation.getValues(); - if (values instanceof JSONObject) { - extractScimAttributes((JSONObject) values, path, attributes); - } else if (values instanceof JSONArray) { - extractScimAttributes((JSONArray) values, path, attributes); - } else if (values != null) { - attributes.add(path); - } - - return attributes.isEmpty() && path != null ? Collections.singletonList(path) : attributes; - } - - private static void extractScimAttributes(JSONObject jsonObject, String basePath, List attributes) { - - for (Iterator it = jsonObject.keys(); it.hasNext(); ) { - String key = it.next(); - Object value = jsonObject.get(key); - String separator = DOT_SEPARATOR; - if (defaultScimSchemas().contains(basePath)) { - separator = COLON; - } - String newPath = (basePath != null) ? basePath + separator + key : key; - - if (value instanceof JSONObject) { - extractScimAttributes((JSONObject) value, newPath, attributes); - } else if (value instanceof JSONArray) { - extractScimAttributes((JSONArray) value, newPath, attributes); - } else { - attributes.add(newPath); - } - } - } - - private static void extractScimAttributes(JSONArray jsonArray, String basePath, List attributes) { - - for (int i = 0; i < jsonArray.length(); i++) { - Object value = jsonArray.get(i); - if (value instanceof JSONObject) { - extractScimAttributes((JSONObject) value, basePath, attributes); - } else if (!(value instanceof JSONArray)) { - attributes.add(basePath); - } - } - } - - private static List defaultScimSchemas() { - - return Arrays.asList( - SCIMConstants.CORE_SCHEMA_URI, - SCIMConstants.USER_CORE_SCHEMA_URI, - SCIMConstants.ENTERPRISE_USER_SCHEMA_URI, - SCIMConstants.SYSTEM_USER_SCHEMA_URI - ); - } } diff --git a/modules/charon-core/src/main/java/org/wso2/charon3/core/utils/PatchOperationUtil.java b/modules/charon-core/src/main/java/org/wso2/charon3/core/utils/PatchOperationUtil.java index 5c0bb2f06..581b19f46 100644 --- a/modules/charon-core/src/main/java/org/wso2/charon3/core/utils/PatchOperationUtil.java +++ b/modules/charon-core/src/main/java/org/wso2/charon3/core/utils/PatchOperationUtil.java @@ -51,10 +51,15 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; +import static org.wso2.charon3.core.schema.SCIMConstants.OperationalConstants.COLON; +import static org.wso2.charon3.core.schema.SCIMConstants.OperationalConstants.DOT_SEPARATOR; +import static org.wso2.charon3.core.schema.SCIMConstants.OperationalConstants.OPEN_SQUARE_BRACKET; + /** * This provides the methods on the PATCH operation of any resource type. */ @@ -3613,4 +3618,71 @@ private static AbstractSCIMObject doPatchReplaceOnResource(AbstractSCIMObject ol throw new CharonException("Error in performing the replace operation", e); } } + + public static List determineScimAttributes(PatchOperation operation) { + + if (operation == null) { + return Collections.emptyList(); + } + List attributes = new ArrayList<>(); + String path = operation.getPath(); + if (path != null) { + int bracketIndex = path.indexOf(OPEN_SQUARE_BRACKET); + path = (bracketIndex != -1) ? path.substring(0, bracketIndex) : path; + } + + Object values = operation.getValues(); + if (values instanceof JSONObject) { + extractScimAttributes((JSONObject) values, path, attributes); + } else if (values instanceof JSONArray) { + extractScimAttributes((JSONArray) values, path, attributes); + } else if (values != null) { + attributes.add(path); + } + + return attributes.isEmpty() && path != null ? Collections.singletonList(path) : attributes; + } + + private static void extractScimAttributes(JSONObject jsonObject, String basePath, List attributes) { + + for (Iterator it = jsonObject.keys(); it.hasNext(); ) { + String key = it.next(); + Object value = jsonObject.get(key); + String separator = DOT_SEPARATOR; + if (defaultScimSchemas().contains(basePath)) { + separator = COLON; + } + String newPath = (basePath != null) ? basePath + separator + key : key; + + if (value instanceof JSONObject) { + extractScimAttributes((JSONObject) value, newPath, attributes); + } else if (value instanceof JSONArray) { + extractScimAttributes((JSONArray) value, newPath, attributes); + } else { + attributes.add(newPath); + } + } + } + + private static void extractScimAttributes(JSONArray jsonArray, String basePath, List attributes) { + + for (int i = 0; i < jsonArray.length(); i++) { + Object value = jsonArray.get(i); + if (value instanceof JSONObject) { + extractScimAttributes((JSONObject) value, basePath, attributes); + } else if (!(value instanceof JSONArray)) { + attributes.add(basePath); + } + } + } + + private static List defaultScimSchemas() { + + return Arrays.asList( + SCIMConstants.CORE_SCHEMA_URI, + SCIMConstants.USER_CORE_SCHEMA_URI, + SCIMConstants.ENTERPRISE_USER_SCHEMA_URI, + SCIMConstants.SYSTEM_USER_SCHEMA_URI + ); + } } diff --git a/modules/charon-core/src/test/java/org/wso2/charon3/core/protocol/endpoints/UserResourceManagerTest.java b/modules/charon-core/src/test/java/org/wso2/charon3/core/protocol/endpoints/UserResourceManagerTest.java index 993e206a9..5fa532e0b 100644 --- a/modules/charon-core/src/test/java/org/wso2/charon3/core/protocol/endpoints/UserResourceManagerTest.java +++ b/modules/charon-core/src/test/java/org/wso2/charon3/core/protocol/endpoints/UserResourceManagerTest.java @@ -18,7 +18,6 @@ package org.wso2.charon3.core.protocol.endpoints; -import org.json.JSONArray; import org.json.JSONObject; import org.mockito.MockedStatic; import org.mockito.Mockito; @@ -48,14 +47,9 @@ import org.wso2.charon3.core.schema.ServerSideValidator; import org.wso2.charon3.core.utils.CopyUtil; import org.wso2.charon3.core.utils.ResourceManagerUtil; -import org.wso2.charon3.core.utils.codeutils.PatchOperation; import org.wso2.charon3.core.utils.codeutils.SearchRequest; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -1361,92 +1355,4 @@ public void testUpdateWithPATCHRemove(String existingId, String scimObjectString userManager, attributes, excludeAttributes); Assert.assertEquals(scimResponse.getResponseStatus(), expectedScimResponseStatus); } - - @DataProvider(name = "scimAttributeData") - public Object[][] scimAttributeData() { - return new Object[][]{ - {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:emails", null), - Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User:emails") - }, - {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:userName", "john_doe"), - Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User:userName") - }, - {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:addresses", - new JSONObject() - .put("home", new JSONObject() - .put("details", new JSONObject() - .put("street", "123 Main St") - .put("zip", "12345"))) - .put("work", new JSONObject().put("street", "456 Work Rd"))), - Arrays.asList( - "urn:ietf:params:scim:schemas:core:2.0:User:addresses.home.details.street", - "urn:ietf:params:scim:schemas:core:2.0:User:addresses.home.details.zip", - "urn:ietf:params:scim:schemas:core:2.0:User:addresses.work.street") - }, - {createPatchOperation("urn:ietf:params:scim:schemas:extension:enterprise:2.0:User", - new JSONObject() - .put("department", "Engineering") - .put("organization", new JSONObject().put("name", "WSO2"))), - Arrays.asList( - "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department", - "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:organization.name") - }, - {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:emails", - new JSONArray() - .put(new JSONObject().put("primary", true).put("value", "test@example.com")) - .put(new JSONObject().put("value", "work@example.com"))), - Arrays.asList( - "urn:ietf:params:scim:schemas:core:2.0:User:emails.primary", - "urn:ietf:params:scim:schemas:core:2.0:User:emails.value", - "urn:ietf:params:scim:schemas:core:2.0:User:emails.value") - }, - {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:numbers", - new JSONArray() - .put(new JSONObject().put("type", "mobile").put("number", "123456")) - .put("invalid_entry")), - Arrays.asList( - "urn:ietf:params:scim:schemas:core:2.0:User:numbers.type", - "urn:ietf:params:scim:schemas:core:2.0:User:numbers.number", - "urn:ietf:params:scim:schemas:core:2.0:User:numbers") - }, - {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User", - new JSONArray() - .put(new JSONObject().put("email", "test@example.com")) - .put(new JSONObject().put("phone", "123-456-7890"))), - Arrays.asList( - "urn:ietf:params:scim:schemas:core:2.0:User:email", - "urn:ietf:params:scim:schemas:core:2.0:User:phone") - }, - {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:customAttributes", - new JSONObject()), - Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User:customAttributes") - }, - {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:emptyArray", - new JSONArray()), - Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User:emptyArray") - } - }; - } - - @Test(dataProvider = "scimAttributeData") - public void testDetermineScimAttributes(PatchOperation operation, List expectedAttributes) - throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { - - Method method = UserResourceManager.class.getDeclaredMethod("determineScimAttributes", PatchOperation.class); - method.setAccessible(true); - - @SuppressWarnings("unchecked") - List actualAttributes = (List) method.invoke(null, operation); - Assert.assertEqualsNoOrder(actualAttributes.toArray(), expectedAttributes.toArray(), - "The SCIM attributes do not match"); - } - - - private static PatchOperation createPatchOperation(String path, Object values) { - - PatchOperation operation = new PatchOperation(); - operation.setPath(path); - operation.setValues(values); - return operation; - } } diff --git a/modules/charon-core/src/test/java/org/wso2/charon3/core/utils/PatchOperationUtilTest.java b/modules/charon-core/src/test/java/org/wso2/charon3/core/utils/PatchOperationUtilTest.java index 9e1ee2d55..bb1c6e2df 100644 --- a/modules/charon-core/src/test/java/org/wso2/charon3/core/utils/PatchOperationUtilTest.java +++ b/modules/charon-core/src/test/java/org/wso2/charon3/core/utils/PatchOperationUtilTest.java @@ -18,6 +18,8 @@ package org.wso2.charon3.core.utils; +import org.json.JSONArray; +import org.json.JSONObject; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -39,6 +41,8 @@ import org.wso2.charon3.core.utils.codeutils.PatchOperation; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -395,4 +399,87 @@ private User getUser() throws InstantiationException, IllegalAccessException { return user; } + + @DataProvider(name = "scimAttributeData") + public Object[][] scimAttributeData() { + return new Object[][]{ + {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:emails", null), + Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User:emails") + }, + {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:userName", "john_doe"), + Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User:userName") + }, + {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:addresses", + new JSONObject() + .put("home", new JSONObject() + .put("details", new JSONObject() + .put("street", "123 Main St") + .put("zip", "12345"))) + .put("work", new JSONObject().put("street", "456 Work Rd"))), + Arrays.asList( + "urn:ietf:params:scim:schemas:core:2.0:User:addresses.home.details.street", + "urn:ietf:params:scim:schemas:core:2.0:User:addresses.home.details.zip", + "urn:ietf:params:scim:schemas:core:2.0:User:addresses.work.street") + }, + {createPatchOperation("urn:ietf:params:scim:schemas:extension:enterprise:2.0:User", + new JSONObject() + .put("department", "Engineering") + .put("organization", new JSONObject().put("name", "WSO2"))), + Arrays.asList( + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department", + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:organization.name") + }, + {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:emails", + new JSONArray() + .put(new JSONObject().put("primary", true).put("value", "test@example.com")) + .put(new JSONObject().put("value", "work@example.com"))), + Arrays.asList( + "urn:ietf:params:scim:schemas:core:2.0:User:emails.primary", + "urn:ietf:params:scim:schemas:core:2.0:User:emails.value", + "urn:ietf:params:scim:schemas:core:2.0:User:emails.value") + }, + {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:numbers", + new JSONArray() + .put(new JSONObject().put("type", "mobile").put("number", "123456")) + .put("invalid_entry")), + Arrays.asList( + "urn:ietf:params:scim:schemas:core:2.0:User:numbers.type", + "urn:ietf:params:scim:schemas:core:2.0:User:numbers.number", + "urn:ietf:params:scim:schemas:core:2.0:User:numbers") + }, + {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User", + new JSONArray() + .put(new JSONObject().put("email", "test@example.com")) + .put(new JSONObject().put("phone", "123-456-7890"))), + Arrays.asList( + "urn:ietf:params:scim:schemas:core:2.0:User:email", + "urn:ietf:params:scim:schemas:core:2.0:User:phone") + }, + {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:customAttributes", + new JSONObject()), + Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User:customAttributes") + }, + {createPatchOperation("urn:ietf:params:scim:schemas:core:2.0:User:emptyArray", + new JSONArray()), + Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User:emptyArray") + } + }; + } + + @Test(dataProvider = "scimAttributeData") + public void testDetermineScimAttributes(PatchOperation operation, List expectedAttributes) { + + List actualAttributes = PatchOperationUtil.determineScimAttributes(operation); + Assert.assertEqualsNoOrder(actualAttributes.toArray(), expectedAttributes.toArray(), + "The SCIM attributes do not match"); + } + + + private static PatchOperation createPatchOperation(String path, Object values) { + + PatchOperation operation = new PatchOperation(); + operation.setPath(path); + operation.setValues(values); + return operation; + } }