From 0fb3fe85d638d67e6c11c53b0c53659c70d27030 Mon Sep 17 00:00:00 2001 From: ThaminduDilshan Date: Fri, 10 May 2024 16:26:29 +0530 Subject: [PATCH] Remove password status claim and add event callbacks handlers --- .../user/UpdateUserPasswordFunction.java | 2 +- .../user/UpdateUserPasswordFunctionImpl.java | 150 +++++++----------- .../UpdateUserPasswordFunctionImplTest.java | 110 ++++--------- 3 files changed, 91 insertions(+), 171 deletions(-) diff --git a/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunction.java b/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunction.java index cc06217f..d289bd85 100644 --- a/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunction.java +++ b/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunction.java @@ -31,7 +31,7 @@ public interface UpdateUserPasswordFunction { * * @param user Authenticated user. * @param parameters Parameters. It is mandatory to provide the new password as the first parameter. - * Then an optional local claim can be provided to hold the password update status. + * Then an optional map of event handlers can be provided. */ void updateUserPassword(JsAuthenticatedUser user, Object... parameters); } diff --git a/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunctionImpl.java b/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunctionImpl.java index 2877ad41..532b6c1d 100644 --- a/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunctionImpl.java +++ b/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunctionImpl.java @@ -21,6 +21,8 @@ import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.wso2.carbon.identity.application.authentication.framework.AsyncProcess; +import org.wso2.carbon.identity.application.authentication.framework.config.model.graph.JsGraphBuilder; import org.wso2.carbon.identity.application.authentication.framework.config.model.graph.js.JsAuthenticatedUser; import org.wso2.carbon.identity.application.authentication.framework.exception.FrameworkException; import org.wso2.carbon.identity.central.log.mgt.utils.LoggerUtils; @@ -30,6 +32,9 @@ import org.wso2.carbon.user.core.UserStoreManager; import org.wso2.carbon.utils.DiagnosticLog; +import java.util.Collections; +import java.util.Map; + /** * Function to update user password. */ @@ -41,48 +46,52 @@ public class UpdateUserPasswordFunctionImpl implements UpdateUserPasswordFunctio public void updateUserPassword(JsAuthenticatedUser user, Object... parameters) { if (user == null) { - LOG.debug("User is not defined."); - if (LoggerUtils.isDiagnosticLogsEnabled()) { - DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder( - Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, - Constants.LogConstants.ActionIDs.VALIDATE_INPUT_PARAMS - ); - diagnosticLogBuilder.resultMessage("User is not defined.") - .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) - .resultStatus(DiagnosticLog.ResultStatus.FAILED); - LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); - } - - return; + throw new IllegalArgumentException("User is not defined."); } if (parameters == null || parameters.length == 0) { - LOG.debug("Password parameters are not defined."); - if (LoggerUtils.isDiagnosticLogsEnabled()) { - DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder( - Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, - Constants.LogConstants.ActionIDs.VALIDATE_INPUT_PARAMS - ); - diagnosticLogBuilder.resultMessage("Password parameters are not defined.") - .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) - .resultStatus(DiagnosticLog.ResultStatus.FAILED); - LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); - } - - return; + throw new IllegalArgumentException("Password is not defined."); } String newPassword = null; - String passwordMigrationStatusClaim = null; + Map eventHandlers = null; if (parameters.length == 2) { - LOG.debug("Both password and password migration status claim parameters are provided."); + LOG.debug("Both password and event handlers are provided."); newPassword = (String) parameters[0]; - passwordMigrationStatusClaim = (String) parameters[1]; + + if (parameters[1] instanceof Map) { + eventHandlers = (Map) parameters[1]; + } else { + throw new IllegalArgumentException("Invalid argument type. Expected eventHandlers " + + "(Map)."); + } } else { - LOG.debug("Only the new password is provided."); + LOG.debug("Only the password is provided."); newPassword = (String) parameters[0]; } + if (eventHandlers != null) { + String finalNewPassword = newPassword; + AsyncProcess asyncProcess = new AsyncProcess((context, asyncReturn) -> { + try { + doUpdatePassword(user, finalNewPassword); + asyncReturn.accept(context, Collections.emptyMap(), Constants.OUTCOME_SUCCESS); + } catch (FrameworkException e) { + asyncReturn.accept(context, Collections.emptyMap(), Constants.OUTCOME_FAIL); + } + }); + JsGraphBuilder.addLongWaitProcess(asyncProcess, eventHandlers); + } else { + try { + doUpdatePassword(user, newPassword); + } catch (FrameworkException e) { + // Ignore FrameworkException as the function is not expected to throw any. + } + } + } + + private void doUpdatePassword(JsAuthenticatedUser user, String newPassword) throws FrameworkException { + if (StringUtils.isBlank(newPassword)) { LOG.debug("The provided password is empty."); if (LoggerUtils.isDiagnosticLogsEnabled()) { @@ -95,8 +104,7 @@ public void updateUserPassword(JsAuthenticatedUser user, Object... parameters) { .resultStatus(DiagnosticLog.ResultStatus.FAILED); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } - - return; + throw new IllegalArgumentException("The provided password is empty."); } try { @@ -111,38 +119,6 @@ public void updateUserPassword(JsAuthenticatedUser user, Object... parameters) { UserStoreManager userStoreManager = Utils.getUserStoreManager( tenantDomain, userRealm, userStoreDomain); - // Check for password migration status only if the claim is present. - if (StringUtils.isNotBlank(passwordMigrationStatusClaim)) { - String passwordMigrationStatus = userStoreManager.getUserClaimValue( - username, passwordMigrationStatusClaim, null); - - if (LOG.isDebugEnabled()) { - LOG.debug(String.format("Password migration status for the user: %s in tenant: %s is: %s", - username, tenantDomain, passwordMigrationStatus)); - } - - if (Boolean.parseBoolean(passwordMigrationStatus)) { - if (LOG.isDebugEnabled()) { - LOG.debug(String.format("Password migration has already been completed for the " + - "user: %s in tenant: %s", username, tenantDomain)); - } - if (LoggerUtils.isDiagnosticLogsEnabled()) { - DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = - new DiagnosticLog.DiagnosticLogBuilder( - Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, - Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD - ); - diagnosticLogBuilder.resultMessage("Password migration has already been completed " + - "for the user.").inputParam("user", loggableUserId) - .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) - .resultStatus(DiagnosticLog.ResultStatus.FAILED); - LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); - } - - return; - } - } - // Update the user password. userStoreManager.updateCredentialByAdmin(username, newPassword); @@ -162,43 +138,19 @@ public void updateUserPassword(JsAuthenticatedUser user, Object... parameters) { .resultStatus(DiagnosticLog.ResultStatus.SUCCESS); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } - - // Update the password migration status claim. - if (StringUtils.isNotBlank(passwordMigrationStatusClaim)) { - if (LOG.isDebugEnabled()) { - LOG.debug(String.format("Updating the password migration status claim: %s " + - "for the user: %s in tenant: %s to true.", - passwordMigrationStatusClaim, username, tenantDomain)); - } - userStoreManager.setUserClaimValue(username, passwordMigrationStatusClaim, "true", null); - - LOG.debug("Password migration status claim updated successfully for the user: " + username - + " in tenant: " + tenantDomain + "."); - if (LoggerUtils.isDiagnosticLogsEnabled()) { - DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = - new DiagnosticLog.DiagnosticLogBuilder( - Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, - Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD - ); - diagnosticLogBuilder.resultMessage("Password migration status claim updated successfully.") - .inputParam("user", loggableUserId) - .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) - .resultStatus(DiagnosticLog.ResultStatus.SUCCESS); - LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); - } - } } else { if (LOG.isDebugEnabled()) { LOG.debug(String.format("Unable to find user realm for the user: %s " + "in tenant: %s", username, tenantDomain)); } + String message = "Unable to find user realm for the user."; if (LoggerUtils.isDiagnosticLogsEnabled()) { DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder( Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD ); - diagnosticLogBuilder.resultMessage("Unable to find user realm for the user.") + diagnosticLogBuilder.resultMessage(message) .inputParam("user", loggableUserId) .inputParam("tenantDomain", tenantDomain) .inputParam("userStoreDomain", userStoreDomain) @@ -206,32 +158,42 @@ public void updateUserPassword(JsAuthenticatedUser user, Object... parameters) { .resultStatus(DiagnosticLog.ResultStatus.FAILED); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } + + throw new FrameworkException(message); } } else { - LOG.debug("Unable to get wrapped content for the user."); + String message = "Unable to get wrapped content for the user."; + + LOG.debug(message); if (LoggerUtils.isDiagnosticLogsEnabled()) { DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder( Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD ); - diagnosticLogBuilder.resultMessage("Unable to get wrapped content for the user.") + diagnosticLogBuilder.resultMessage(message) .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) .resultStatus(DiagnosticLog.ResultStatus.FAILED); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } + + throw new FrameworkException(message); } } catch (UserStoreException | FrameworkException e) { - LOG.error("Error occurred while updating the user password.", e); + String message = "Error occurred while updating the user password."; + + LOG.error(message, e); if (LoggerUtils.isDiagnosticLogsEnabled()) { DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder( Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD ); - diagnosticLogBuilder.resultMessage("Error occurred while updating the user password.") + diagnosticLogBuilder.resultMessage(message) .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) .resultStatus(DiagnosticLog.ResultStatus.FAILED); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } + + throw new FrameworkException(message, e); } } } diff --git a/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/test/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunctionImplTest.java b/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/test/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunctionImplTest.java index 640761ca..b2a810a5 100644 --- a/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/test/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunctionImplTest.java +++ b/components/org.wso2.carbon.identity.conditional.auth.functions.user/src/test/java/org/wso2/carbon/identity/conditional/auth/functions/user/UpdateUserPasswordFunctionImplTest.java @@ -46,7 +46,9 @@ import org.wso2.carbon.utils.DiagnosticLog; import java.lang.reflect.Field; +import java.util.Collections; import java.util.List; +import java.util.Map; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; @@ -134,98 +136,51 @@ public Object[][] updateUserPasswordWithEmptyInputDataProvider() { return new Object[][]{ {null, "newPassword", null, "User is not defined."}, - {jsAuthenticatedUser, null, null, "Password parameters are not defined."}, + {jsAuthenticatedUser, null, null, "Password is not defined."}, {jsAuthenticatedUser, "", null, "The provided password is empty."}, - {jsAuthenticatedUser, "", "testClaim", "The provided password is empty."} + {jsAuthenticatedUser, "newPassword", Collections.EMPTY_LIST, "Invalid argument type. " + + "Expected eventHandlers (Map)."} }; } @Test(dataProvider = "updateUserPasswordWithEmptyInputDataProvider") public void testUpdateUserPasswordWithEmptyInput(JsAuthenticatedUser user, String password, - String claimURI, String logMessage) - throws UserStoreException, IdentityEventException { - - if (password == null && claimURI == null) { - testFunction.updateUserPassword(user); - } else { - testFunction.updateUserPassword(user, password, claimURI); + List eventHandlers, String errorMessage) + throws UserStoreException { + + try { + if (password == null) { + testFunction.updateUserPassword(user); + } else if (eventHandlers != null) { + testFunction.updateUserPassword(user, password, eventHandlers); + } else { + testFunction.updateUserPassword(user, password); + } + } catch (IllegalArgumentException e) { + // Assert for the correct exception. + Assert.assertEquals(e.getMessage(), errorMessage); } // Assert that user store manager methods are never invoked. verify(userStoreManagerMock, times(0)).updateCredentialByAdmin(anyString(), anyString()); + } + + @Test + public void testUpdateUserPassword() throws UserStoreException, IdentityEventException { + + testFunction.updateUserPassword(jsAuthenticatedUser, PASSWORD); + + // Assert that updateCredentialByAdmin method is invoked. + verify(userStoreManagerMock, times(1)).updateCredentialByAdmin(USERNAME, PASSWORD); // Assert for the correct diagnostic logs. ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(Event.class); verify(identityEventServiceMock).handleEvent(logEventCaptor.capture()); - assertDiagnosticLog(logEventCaptor.getValue(), Constants.LogConstants.ActionIDs.VALIDATE_INPUT_PARAMS, - DiagnosticLog.ResultStatus.FAILED, logMessage); + assertDiagnosticLog(logEventCaptor.getValue(), Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD, + DiagnosticLog.ResultStatus.SUCCESS, "User password updated successfully."); } - @DataProvider(name = "updateUserPasswordDataProvider") - public Object[][] updateUserPasswordDataProvider() { - - return new Object[][]{ - {null, null}, - {PASSWORD_UPDATE_STATUS_CLAIM, null}, - {PASSWORD_UPDATE_STATUS_CLAIM, "false"}, - {PASSWORD_UPDATE_STATUS_CLAIM, "true"} - }; - } - - @Test(dataProvider = "updateUserPasswordDataProvider") - public void testUpdateUserPassword(String claim, String claimValue) - throws UserStoreException, IdentityEventException { - - if (claim != null) { - when(userStoreManagerMock.getUserClaimValue(USERNAME, PASSWORD_UPDATE_STATUS_CLAIM, null)) - .thenReturn(claimValue); - testFunction.updateUserPassword(jsAuthenticatedUser, PASSWORD, PASSWORD_UPDATE_STATUS_CLAIM); - - if ("true".equals(claimValue)) { - // Assert that user store manager methods are never invoked when the password is already updated. - verify(userStoreManagerMock, times(0)).updateCredentialByAdmin(USERNAME, PASSWORD); - - // Assert for the correct diagnostic logs. - ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(Event.class); - verify(identityEventServiceMock).handleEvent(logEventCaptor.capture()); - assertDiagnosticLog(logEventCaptor.getValue(), - Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD, DiagnosticLog.ResultStatus.FAILED, - "Password migration has already been completed for the user."); - } else { - // Assert that user store manager methods are invoked when the password is not updated. - verify(userStoreManagerMock, times(1)).updateCredentialByAdmin(USERNAME, PASSWORD); - verify(userStoreManagerMock, times(1)) - .getUserClaimValue(USERNAME, PASSWORD_UPDATE_STATUS_CLAIM, null); - verify(userStoreManagerMock, times(1)) - .setUserClaimValue(USERNAME, PASSWORD_UPDATE_STATUS_CLAIM, "true", null); - - // Assert for the correct diagnostic logs. - ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(Event.class); - verify(identityEventServiceMock, times(2)).handleEvent(logEventCaptor.capture()); - List eventList = logEventCaptor.getAllValues(); - - assertDiagnosticLog(eventList.get(0), Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD, - DiagnosticLog.ResultStatus.SUCCESS, "User password updated successfully."); - - assertDiagnosticLog(eventList.get(1), - Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD, DiagnosticLog.ResultStatus.SUCCESS, - "Password migration status claim updated successfully."); - } - } else { - testFunction.updateUserPassword(jsAuthenticatedUser, PASSWORD); - - // Assert that only the updateCredentialByAdmin method is invoked. - verify(userStoreManagerMock, times(1)).updateCredentialByAdmin(USERNAME, PASSWORD); - verify(userStoreManagerMock, times(0)) - .setUserClaimValue(USERNAME, PASSWORD_UPDATE_STATUS_CLAIM, "true", null); - - // Assert for the correct diagnostic logs. - ArgumentCaptor logEventCaptor = ArgumentCaptor.forClass(Event.class); - verify(identityEventServiceMock).handleEvent(logEventCaptor.capture()); - assertDiagnosticLog(logEventCaptor.getValue(), Constants.LogConstants.ActionIDs.UPDATE_USER_PASSWORD, - DiagnosticLog.ResultStatus.SUCCESS, "User password updated successfully."); - } - } + // TODO: Add tests for async handling with callbacks. private void assertDiagnosticLog(Event capturedEvent, String expectedActionID, DiagnosticLog.ResultStatus expectedResultStatus, String expectedMessage) { @@ -237,3 +192,6 @@ private void assertDiagnosticLog(Event capturedEvent, String expectedActionID, Assert.assertEquals(diagnosticLog.getResultMessage(), expectedMessage); } } + +// TODO: Fix jacoco coverage issue. +// May be have to fix for http module as well.