diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java index 83e231ea2..14b1c8c65 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java @@ -32,6 +32,7 @@ import org.json.JSONObject; import org.wso2.carbon.CarbonConstants; import org.wso2.carbon.context.PrivilegedCarbonContext; +import org.wso2.carbon.identity.action.execution.api.model.ActionExecutionStatus; import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkUtils; import org.wso2.carbon.identity.application.common.IdentityApplicationManagementException; import org.wso2.carbon.identity.application.common.model.ServiceProvider; @@ -5931,13 +5932,44 @@ private void updateUserClaims(User user, Map oldClaimList, if (isExecutableUserProfileUpdate) { - Map multiValuedClaimsToModify = - SCIMCommonUtils.getSimpleMultiValuedClaimsToModify(oldClaimList, simpleMultiValuedClaimsToBeAdded, - simpleMultiValuedClaimsToBeRemoved); - userClaimsToBeModifiedIncludingMultiValueClaims.putAll(multiValuedClaimsToModify); + populateMultiValuedClaimsToModify(oldClaimList, simpleMultiValuedClaimsToBeAdded, + simpleMultiValuedClaimsToBeRemoved, userClaimsToBeModifiedIncludingMultiValueClaims); - preUpdateProfileActionExecutor.execute(user, userClaimsToBeModifiedIncludingMultiValueClaims, - userClaimsToBeDeleted); + ActionExecutionStatus actionExecutionStatus = preUpdateProfileActionExecutor.executeAndGetStatus(user, + userClaimsToBeModifiedIncludingMultiValueClaims, userClaimsToBeDeleted); + if (actionExecutionStatus != null) { + + Map userClaimsToBeAddedFromAction = (HashMap) + actionExecutionStatus.getResponseContext().get("userClaimsToBeAdded"); + Map userClaimsToBeModifiedFromAction = (HashMap) + actionExecutionStatus.getResponseContext().get("userClaimsToBeModified"); + Map userClaimsToBeDeletedFromAction = (HashMap) + actionExecutionStatus.getResponseContext().get("userClaimsToBeRemoved"); + Map> simpleMultiValuedClaimsToBeAddedFromAction = (HashMap>) + actionExecutionStatus.getResponseContext().get("multiValuedClaimsToBeAdded"); + Map> simpleMultiValuedClaimsToBeRemovedFromAction = (HashMap>) + actionExecutionStatus.getResponseContext().get("multiValuedClaimsToBeRemoved"); + + if (userClaimsToBeAddedFromAction != null) { + userClaimsToBeAdded.putAll(userClaimsToBeAddedFromAction); + userClaimsToBeModified.putAll(userClaimsToBeAddedFromAction); + } + if (userClaimsToBeModifiedFromAction != null) { + userClaimsToBeModified.putAll(userClaimsToBeModifiedFromAction); + } + if (userClaimsToBeDeletedFromAction != null) { + userClaimsToBeDeleted.putAll(userClaimsToBeDeletedFromAction); + userClaimsToBeModified.keySet().removeAll(userClaimsToBeDeletedFromAction.keySet()); + } + if (simpleMultiValuedClaimsToBeAddedFromAction != null) { + simpleMultiValuedClaimsToBeAdded.putAll(simpleMultiValuedClaimsToBeAddedFromAction); + } + if (simpleMultiValuedClaimsToBeRemovedFromAction != null) { + simpleMultiValuedClaimsToBeRemoved.putAll(simpleMultiValuedClaimsToBeRemovedFromAction); + } + populateMultiValuedClaimsToModify(oldClaimList, simpleMultiValuedClaimsToBeAdded, + simpleMultiValuedClaimsToBeRemoved, userClaimsToBeModifiedIncludingMultiValueClaims); + } } if (log.isDebugEnabled()) { @@ -5983,6 +6015,27 @@ private void updateUserClaims(User user, Map oldClaimList, } } + /** + * Populate both modifiable single and multi-valued claims. + * + * @param oldClaimList User claim list for the user's existing state. + * @param simpleMultiValuedClaimsToBeAdded Map of simple multi-valued claims to be added. + * @param simpleMultiValuedClaimsToBeRemoved Map of simple multi-valued claims to be removed. + * @param userClaimsToBeModifiedIncludingMultiValueClaims Map of both single and multi valued claims to be modified. + * @return Map with the same keys and values as lists of strings, splitting multi-valued claims if necessary. + */ + private void populateMultiValuedClaimsToModify(Map oldClaimList, Map> + simpleMultiValuedClaimsToBeAdded, Map> simpleMultiValuedClaimsToBeRemoved, + Map userClaimsToBeModifiedIncludingMultiValueClaims) + { + + Map multiValuedClaimsToModify = + SCIMCommonUtils.getSimpleMultiValuedClaimsToModify(oldClaimList, simpleMultiValuedClaimsToBeAdded, + simpleMultiValuedClaimsToBeRemoved); + userClaimsToBeModifiedIncludingMultiValueClaims.putAll(multiValuedClaimsToModify); + } + /** * Convert the claim values to list of strings. * diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/internal/action/PreUpdateProfileActionExecutor.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/internal/action/PreUpdateProfileActionExecutor.java index 2822c266b..963bd6231 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/internal/action/PreUpdateProfileActionExecutor.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/internal/action/PreUpdateProfileActionExecutor.java @@ -76,10 +76,18 @@ public class PreUpdateProfileActionExecutor { public void execute(User user, Map userClaimsToBeModified, Map userClaimsToBeDeleted) throws UserStoreException { + executeAndGetStatus(user, userClaimsToBeModified, userClaimsToBeDeleted); + } + + // Triggers the execution of pre update profile extension at profile update with PUT and returns execution status. + public ActionExecutionStatus executeAndGetStatus(User user, Map userClaimsToBeModified, + Map userClaimsToBeDeleted) + throws UserStoreException { + ActionExecutorService actionExecutorService = SCIMCommonComponentHolder.getActionExecutorService(); if (!actionExecutorService.isExecutionEnabled(ActionType.PRE_UPDATE_PROFILE)) { - return; + return null; } LOG.debug("Executing pre update profile action for user: " + user.getId()); @@ -96,7 +104,7 @@ public void execute(User user, Map userClaimsToBeModified, actionExecutorService.execute(ActionType.PRE_UPDATE_PROFILE, flowContext, IdentityContext.getThreadLocalIdentityContext().getTenantDomain()); - handleActionExecutionStatus(actionExecutionStatus); + return handleActionExecutionStatus(actionExecutionStatus); } catch (ActionExecutionException e) { throw new UserActionExecutionServerException(UserActionError.PRE_UPDATE_PROFILE_ACTION_SERVER_ERROR, "Error while executing pre update profile action.", e); @@ -119,12 +127,12 @@ private UserActionRequestDTO buildUserActionRequestDTO(User user, Map actionExecutionStatus) + private ActionExecutionStatus handleActionExecutionStatus(ActionExecutionStatus actionExecutionStatus) throws UserStoreException { switch (actionExecutionStatus.getStatus()) { case SUCCESS: - return; + return actionExecutionStatus; case FAILED: Failure failure = (Failure) actionExecutionStatus.getResponse(); throw new UserActionExecutionClientException(UserActionError.PRE_UPDATE_PROFILE_ACTION_EXECUTION_FAILED, diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java index d17087e9c..252a00aec 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java @@ -42,6 +42,7 @@ import org.wso2.carbon.identity.compatibility.settings.core.exception.CompatibilitySettingException; import org.wso2.carbon.identity.compatibility.settings.core.model.CompatibilitySetting; import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkUtils; +import org.wso2.carbon.identity.action.execution.api.model.ActionExecutionStatus; import org.wso2.carbon.identity.application.common.IdentityApplicationManagementException; import org.wso2.carbon.identity.application.common.model.InboundProvisioningConfig; import org.wso2.carbon.identity.application.common.model.ServiceProvider; @@ -2753,6 +2754,139 @@ public void testUpdateUserClaims_MultiValuedClaims() throws Exception { } } + @Test + public void testUpdateUserClaims_WhenActionResponseContainsAddedModifiedRemovedClaims() throws Exception { + + SCIMUserManager scimUserManager = spy(new SCIMUserManager( + mockedUserStoreManager, + mockClaimMetadataManagementService, + MultitenantConstants.SUPER_TENANT_DOMAIN_NAME + )); + User user = mock(User.class); + when(user.getId()).thenReturn("foo"); + + String accountLocked = "http://wso2.org/claims/identity/accountLocked"; + String accountState = "http://wso2.org/claims/identity/accountState"; + String accountDisabled = "http://wso2.org/claims/identity/accountDisabled"; + + Map oldClaims = new HashMap<>(); + oldClaims.put(accountLocked, "old"); + oldClaims.put(accountState, "deleteMe"); + + Map newClaims = new HashMap<>(); + newClaims.put(accountLocked, "new"); + + Map multiValuedClaims = Collections.emptyMap(); + + Field preUpdateField = SCIMUserManager.class.getDeclaredField("preUpdateProfileActionExecutor"); + preUpdateField.setAccessible(true); + PreUpdateProfileActionExecutor executorMock = mock(PreUpdateProfileActionExecutor.class); + preUpdateField.set(scimUserManager, executorMock); + + ActionExecutionStatus actionExecutionStatus = mock(ActionExecutionStatus.class); + Map responseContext = new HashMap<>(); + Map addedFromAction = new HashMap<>(); + addedFromAction.put(accountDisabled, "addedValue"); + Map modifiedFromAction = new HashMap<>(); + modifiedFromAction.put(accountState, "modifiedValue"); + Map removedFromAction = new HashMap<>(); + removedFromAction.put(accountLocked, ""); + + responseContext.put("userClaimsToBeAdded", addedFromAction); + responseContext.put("userClaimsToBeModified", modifiedFromAction); + responseContext.put("userClaimsToBeRemoved", removedFromAction); + when(actionExecutionStatus.getResponseContext()).thenReturn(responseContext); + scimCommonUtils.when(() -> SCIMCommonUtils.isExecutableUserProfileUpdate(anyMap(), anyMap(), anyMap(), anyMap())) + .thenReturn(true); + doReturn(actionExecutionStatus).when(executorMock) + .executeAndGetStatus(any(User.class), anyMap(), anyMap()); + + Field carbonUMField = SCIMUserManager.class.getDeclaredField("carbonUM"); + carbonUMField.setAccessible(true); + carbonUMField.set(scimUserManager, mockedUserStoreManager); + + doNothing().when(mockedUserStoreManager).setUserClaimValuesWithID(anyString(), anyMap(), isNull()); + doNothing().when(mockedUserStoreManager).deleteUserClaimValueWithID(anyString(), anyString(), isNull()); + + Method m = SCIMUserManager.class.getDeclaredMethod( + "updateUserClaims", User.class, Map.class, Map.class, Map.class + ); + m.setAccessible(true); + m.invoke(scimUserManager, user, oldClaims, newClaims, multiValuedClaims); + + verify(mockedUserStoreManager).setUserClaimValuesWithID(eq("foo"), + argThat(map -> map.containsKey(accountDisabled) + && map.containsKey(accountState) + && !map.containsKey(accountLocked)), + isNull()); + } + + @Test + public void testUpdateUserClaims_WhenActionResponseContainsMultiValuedClaims() throws Exception { + + SCIMUserManager scimUserManager = spy(new SCIMUserManager( + mockedUserStoreManager, + mockClaimMetadataManagementService, + MultitenantConstants.SUPER_TENANT_DOMAIN_NAME + )); + User user = mock(User.class); + when(user.getId()).thenReturn("foo"); + + Map oldClaims = new HashMap<>(); + oldClaims.put("emails", "a@wso2.com,b@wso2.com"); + + Map newClaims = new HashMap<>(); + newClaims.put("emails", "a@wso2.com,b@wso2.com"); + + Map multiValuedClaims = new HashMap<>(); + multiValuedClaims.put("emails", ""); + + Field preUpdateField = SCIMUserManager.class.getDeclaredField("preUpdateProfileActionExecutor"); + preUpdateField.setAccessible(true); + PreUpdateProfileActionExecutor executorMock = mock(PreUpdateProfileActionExecutor.class); + preUpdateField.set(scimUserManager, executorMock); + + ActionExecutionStatus actionExecutionStatus = mock(ActionExecutionStatus.class); + Map responseContext = new HashMap<>(); + Map> multiAddedFromAction = new HashMap<>(); + multiAddedFromAction.put("emails", Collections.singletonList("c@wso2.com")); + Map> multiRemovedFromAction = new HashMap<>(); + multiRemovedFromAction.put("emails", Collections.singletonList("a@wso2.com")); + + responseContext.put("multiValuedClaimsToBeAdded", multiAddedFromAction); + responseContext.put("multiValuedClaimsToBeRemoved", multiRemovedFromAction); + when(actionExecutionStatus.getResponseContext()).thenReturn(responseContext); + scimCommonUtils.when(() -> SCIMCommonUtils.isExecutableUserProfileUpdate(anyMap(), anyMap(), anyMap(), anyMap())) + .thenReturn(true); + doReturn(actionExecutionStatus).when(executorMock) + .executeAndGetStatus(any(User.class), anyMap(), anyMap()); + + Field carbonUMField = SCIMUserManager.class.getDeclaredField("carbonUM"); + carbonUMField.setAccessible(true); + carbonUMField.set(scimUserManager, mockedUserStoreManager); + + try (MockedStatic fw = mockStatic(FrameworkUtils.class)) { + fw.when(FrameworkUtils::getMultiAttributeSeparator).thenReturn(","); + + doNothing().when(mockedUserStoreManager).setUserClaimValuesWithID(anyString(), anyMap(), anyMap(), + anyMap(), anyMap(), isNull()); + + Method m = SCIMUserManager.class.getDeclaredMethod( + "updateUserClaims", User.class, Map.class, Map.class, Map.class + ); + m.setAccessible(true); + m.invoke(scimUserManager, user, oldClaims, newClaims, multiValuedClaims); + + verify(mockedUserStoreManager, times(1)).setUserClaimValuesWithID( + eq("foo"), + anyMap(), + argThat(map -> map.containsKey("emails") && map.get("emails").contains("c@wso2.com")), + argThat(map -> map.containsKey("emails") && map.get("emails").contains("a@wso2.com")), + anyMap(), + isNull()); + } + } + @DataProvider(name = "syncedUserAttributesData") public Object[][] syncedUserAttributesData() { diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/internal/action/PreUpdateProfileActionExecutorTest.java b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/internal/action/PreUpdateProfileActionExecutorTest.java index c003f1add..e41d58b24 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/internal/action/PreUpdateProfileActionExecutorTest.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/internal/action/PreUpdateProfileActionExecutorTest.java @@ -163,6 +163,51 @@ public void testExecutionWhenActionExecutionIsDisabled() throws Exception { Collections.emptyMap(), Collections.emptyMap())); } + @Test + public void testExecuteAndGetStatusReturnsNullWhenActionExecutionIsDisabled() throws Exception { + + when(actionExecutorService.isExecutionEnabled(ActionType.PRE_UPDATE_PROFILE)).thenReturn(false); + + User user = getSCIMUser(); + ActionExecutionStatus status = + preUpdateProfileActionExecutor.executeAndGetStatus(user, Collections.emptyMap(), + Collections.emptyMap()); + + assertEquals(status, null); + verify(actionExecutorService, Mockito.never()).execute(any(), any(), any()); + } + + @Test + public void testExecuteAndGetStatusReturnsSuccessStatusWhenActionSucceeds() throws Exception { + + setOrganizationToIdentityContext(); + when(actionExecutorService.isExecutionEnabled(ActionType.PRE_UPDATE_PROFILE)).thenReturn(true); + + ActionExecutionStatus successStatus = mock(ActionExecutionStatus.class); + when(successStatus.getStatus()).thenReturn(ActionExecutionStatus.Status.SUCCESS); + when(actionExecutorService.execute(eq(ActionType.PRE_UPDATE_PROFILE), any(), any())).thenReturn(successStatus); + + LocalClaim newClaim = getMockedLocalClaim(NEW_SINGLEVALUE_CLAIM1); + LocalClaim deletingClaim = getMockedLocalClaim(DELETING_SINGLEVALUE_CLAIM4); + when(claimMetadataManagementService.getLocalClaim(eq(NEW_SINGLEVALUE_CLAIM1.getClaimURI()), any(String.class))) + .thenReturn(Optional.of(newClaim)); + when(claimMetadataManagementService.getLocalClaim(eq(DELETING_SINGLEVALUE_CLAIM4.getClaimURI()), + any(String.class))).thenReturn(Optional.of(deletingClaim)); + + User user = getSCIMUser(); + Map claimsToModify = new HashMap<>(); + claimsToModify.put(NEW_SINGLEVALUE_CLAIM1.getClaimURI(), NEW_SINGLEVALUE_CLAIM1.getInputValueAsString()); + Map claimsToDelete = new HashMap<>(); + claimsToDelete.put(DELETING_SINGLEVALUE_CLAIM4.getClaimURI(), + DELETING_SINGLEVALUE_CLAIM4.getInputValueAsString()); + + ActionExecutionStatus status = + preUpdateProfileActionExecutor.executeAndGetStatus(user, claimsToModify, claimsToDelete); + + assertNotNull(status); + assertEquals(status, successStatus); + } + @Test public void testSuccessExecutionForClaimUpdateExecutionAtPutOperation() throws Exception {