From 6ac6ff7270e94378a06f75df915517cdb9db68ca Mon Sep 17 00:00:00 2001 From: Mwesigye John Bosco Date: Tue, 21 Nov 2023 20:07:58 +0300 Subject: [PATCH] FINERACT-2005: Prohibit password reuse --- .../domain/GlobalConfigurationProperty.java | 9 +- .../RestrictReuseOfPasswordException.java | 30 ++++ .../api/AppUserApiConstant.java | 2 +- ...wordReuseGlobalConfigurationException.java | 30 ++++ ...WritePlatformServiceJpaRepositoryImpl.java | 29 +++- .../UserAdministrationConfiguration.java | 7 +- .../db/changelog/tenant/changelog-tenant.xml | 1 + .../tenant/parts/0135_enhance_security.xml | 45 ++++++ ...ePlatformServiceJpaRepositoryImplTest.java | 146 ++++++++++++++++++ 9 files changed, 293 insertions(+), 6 deletions(-) create mode 100644 fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/exception/RestrictReuseOfPasswordException.java create mode 100644 fineract-provider/src/main/java/org/apache/fineract/useradministration/exception/ProhibitPasswordReuseGlobalConfigurationException.java create mode 100644 fineract-provider/src/main/resources/db/changelog/tenant/parts/0135_enhance_security.xml create mode 100644 fineract-provider/src/test/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImplTest.java diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/GlobalConfigurationProperty.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/GlobalConfigurationProperty.java index a38401d249d..6d835e88f1f 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/GlobalConfigurationProperty.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/domain/GlobalConfigurationProperty.java @@ -33,6 +33,7 @@ import org.apache.fineract.infrastructure.core.api.JsonCommand; import org.apache.fineract.infrastructure.core.domain.AbstractPersistableCustom; import org.apache.fineract.infrastructure.security.exception.ForcePasswordResetException; +import org.apache.fineract.infrastructure.security.exception.RestrictReuseOfPasswordException; @Entity @Table(name = "c_configuration") @@ -106,7 +107,13 @@ public Map update(final JsonCommand command) { throw new ForcePasswordResetException(); } } - + final String restrictReUseOfPasswordPropertyName = "Restrict-re-use-of-password"; + if (this.name.equalsIgnoreCase(restrictReUseOfPasswordPropertyName)) { + if ((this.enabled == true && command.hasParameter(valueParamName) && (this.value == 0)) + || (this.enabled == true && !command.hasParameter(valueParamName) && (previousValue == 0))) { + throw new RestrictReuseOfPasswordException(); + } + } return actualChanges; } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/exception/RestrictReuseOfPasswordException.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/exception/RestrictReuseOfPasswordException.java new file mode 100644 index 00000000000..65854c41654 --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/exception/RestrictReuseOfPasswordException.java @@ -0,0 +1,30 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.infrastructure.security.exception; + +import org.apache.fineract.infrastructure.core.exception.AbstractPlatformDomainRuleException; + +public class RestrictReuseOfPasswordException extends AbstractPlatformDomainRuleException { + + public RestrictReuseOfPasswordException() { + super("error.msg.restrict.reuse.password.value.must.be.greater.than.zero", + "For enabling 'Restrict-re-use-of-password' configuration , the value (number of previous password/s to reuse) must be set to a number greater than 0."); + } + +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/useradministration/api/AppUserApiConstant.java b/fineract-provider/src/main/java/org/apache/fineract/useradministration/api/AppUserApiConstant.java index 7c9862c4710..b0c0a7a9089 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/useradministration/api/AppUserApiConstant.java +++ b/fineract-provider/src/main/java/org/apache/fineract/useradministration/api/AppUserApiConstant.java @@ -24,6 +24,6 @@ private AppUserApiConstant() { } - public static final int numberOfPreviousPasswords = 3; + public static final String RESTRICT_RE_USE_OF_PASSWORD = "Restrict-re-use-of-password"; } diff --git a/fineract-provider/src/main/java/org/apache/fineract/useradministration/exception/ProhibitPasswordReuseGlobalConfigurationException.java b/fineract-provider/src/main/java/org/apache/fineract/useradministration/exception/ProhibitPasswordReuseGlobalConfigurationException.java new file mode 100644 index 00000000000..c551e3debab --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/useradministration/exception/ProhibitPasswordReuseGlobalConfigurationException.java @@ -0,0 +1,30 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.useradministration.exception; + +import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; + +public class ProhibitPasswordReuseGlobalConfigurationException extends PlatformApiDataValidationException { + + public ProhibitPasswordReuseGlobalConfigurationException() { + super("error.msg.Restrict-re-use-of-password.is.disabled.contact.system.admin.to.enable.it", + "Reset Password is terminated. Please reach-out to your admin to enable [Restrict-re-use-of-password] in global configuration", + null); + } +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java index 6805f90ea46..2756fadb17a 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java @@ -32,6 +32,8 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.fineract.commands.service.CommandWrapperBuilder; +import org.apache.fineract.infrastructure.configuration.data.GlobalConfigurationPropertyData; +import org.apache.fineract.infrastructure.configuration.service.ConfigurationReadPlatformService; import org.apache.fineract.infrastructure.core.api.JsonCommand; import org.apache.fineract.infrastructure.core.data.ApiParameterError; import org.apache.fineract.infrastructure.core.data.CommandProcessingResult; @@ -66,6 +68,7 @@ import org.springframework.data.domain.Sort; import org.springframework.orm.jpa.JpaSystemException; import org.springframework.security.authentication.AuthenticationServiceException; +import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.ObjectUtils; @@ -83,6 +86,8 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit private final AppUserPreviousPasswordRepository appUserPreviewPasswordRepository; private final StaffRepositoryWrapper staffRepositoryWrapper; private final ClientRepositoryWrapper clientRepositoryWrapper; + private final PasswordEncoder passwordEncoder; + private final ConfigurationReadPlatformService configurationReadPlatformService; @Override @Transactional @@ -233,19 +238,33 @@ public CommandProcessingResult updateUser(final Long userId, final JsonCommand c * Encode the new submitted password and retrieve the last N used passwords to check if the current submitted * password matches with one of them. */ - private AppUserPreviousPassword getCurrentPasswordToSaveAsPreview(final AppUser user, final JsonCommand command) { + public AppUserPreviousPassword getCurrentPasswordToSaveAsPreview(final AppUser user, final JsonCommand command) { final String passWordEncodedValue = user.getEncodedPassword(command, this.platformPasswordEncoder); + String originalPassword = command.stringValueOfParameterNamed("password"); AppUserPreviousPassword currentPasswordToSaveAsPreview = null; + Integer numberOfPreviousPasswords; + + GlobalConfigurationPropertyData restrictReuseOfPasswordConfig = configurationReadPlatformService + .retrieveGlobalConfiguration(AppUserApiConstant.RESTRICT_RE_USE_OF_PASSWORD); + + if (restrictReuseOfPasswordConfig.isEnabled() && restrictReuseOfPasswordConfig.getValue() > 0) { + numberOfPreviousPasswords = restrictReuseOfPasswordConfig.getValue().intValue(); + } else { + return new AppUserPreviousPassword(user); + } if (passWordEncodedValue != null) { - PageRequest pageRequest = PageRequest.of(0, AppUserApiConstant.numberOfPreviousPasswords, Sort.Direction.DESC, "removalDate"); + PageRequest pageRequest = PageRequest.of(0, numberOfPreviousPasswords, Sort.Direction.DESC, "id"); final List nLastUsedPasswords = this.appUserPreviewPasswordRepository.findByUserId(user.getId(), pageRequest); + // validate current password before saving it as previous + validatePasswordShouldNotBeReused(originalPassword, user.getPassword()); for (AppUserPreviousPassword aPreviewPassword : nLastUsedPasswords) { if (aPreviewPassword.getPassword().equals(passWordEncodedValue)) { throw new PasswordPreviouslyUsedException(); } + validatePasswordShouldNotBeReused(originalPassword, aPreviewPassword.getPassword()); } currentPasswordToSaveAsPreview = new AppUserPreviousPassword(user); @@ -304,4 +323,10 @@ private RuntimeException handleDataIntegrityIssues(final JsonCommand command, fi log.error("handleDataIntegrityIssues: Neither duplicate username nor existing user; unknown error occured", dve); return ErrorHandler.getMappable(dve, "error.msg.unknown.data.integrity.issue", "Unknown data integrity issue with resource."); } + + private void validatePasswordShouldNotBeReused(String originalPassword, String aPreviewPassword) { + if (this.passwordEncoder.matches(originalPassword, aPreviewPassword)) { + throw new PasswordPreviouslyUsedException(); + } + } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/useradministration/starter/UserAdministrationConfiguration.java b/fineract-provider/src/main/java/org/apache/fineract/useradministration/starter/UserAdministrationConfiguration.java index f092ee0e385..1c596aba1b0 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/useradministration/starter/UserAdministrationConfiguration.java +++ b/fineract-provider/src/main/java/org/apache/fineract/useradministration/starter/UserAdministrationConfiguration.java @@ -18,6 +18,7 @@ */ package org.apache.fineract.useradministration.starter; +import org.apache.fineract.infrastructure.configuration.service.ConfigurationReadPlatformService; import org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator; import org.apache.fineract.infrastructure.security.service.PlatformPasswordEncoder; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; @@ -56,6 +57,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.security.crypto.password.PasswordEncoder; @Configuration public class UserAdministrationConfiguration { @@ -75,10 +77,11 @@ public AppUserWritePlatformService appUserWritePlatformService(PlatformSecurityC PlatformPasswordEncoder platformPasswordEncoder, AppUserRepository appUserRepository, OfficeRepositoryWrapper officeRepositoryWrapper, RoleRepository roleRepository, UserDataValidator fromApiJsonDeserializer, AppUserPreviousPasswordRepository appUserPreviewPasswordRepository, StaffRepositoryWrapper staffRepositoryWrapper, - ClientRepositoryWrapper clientRepositoryWrapper) { + ClientRepositoryWrapper clientRepositoryWrapper, PasswordEncoder passwordEncoder, + ConfigurationReadPlatformService configurationReadPlatformService) { return new AppUserWritePlatformServiceJpaRepositoryImpl(context, userDomainService, platformPasswordEncoder, appUserRepository, officeRepositoryWrapper, roleRepository, fromApiJsonDeserializer, appUserPreviewPasswordRepository, staffRepositoryWrapper, - clientRepositoryWrapper); + clientRepositoryWrapper, passwordEncoder, configurationReadPlatformService); } @Bean diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml b/fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml index 2085cf5fbf4..105856e64d1 100644 --- a/fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml +++ b/fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml @@ -154,4 +154,5 @@ + diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/parts/0135_enhance_security.xml b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0135_enhance_security.xml new file mode 100644 index 00000000000..c39061ed339 --- /dev/null +++ b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0135_enhance_security.xml @@ -0,0 +1,45 @@ + + + + + + + + + + select count(1) from c_configuration where name = 'Restrict-re-use-of-password'; + + + + + + + + + + + + + + diff --git a/fineract-provider/src/test/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImplTest.java b/fineract-provider/src/test/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImplTest.java new file mode 100644 index 00000000000..dfc87d951a5 --- /dev/null +++ b/fineract-provider/src/test/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImplTest.java @@ -0,0 +1,146 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.useradministration.service; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.List; +import org.apache.fineract.infrastructure.configuration.data.GlobalConfigurationPropertyData; +import org.apache.fineract.infrastructure.configuration.service.ConfigurationReadPlatformService; +import org.apache.fineract.infrastructure.core.api.JsonCommand; +import org.apache.fineract.infrastructure.core.service.DateUtils; +import org.apache.fineract.infrastructure.security.service.PlatformPasswordEncoder; +import org.apache.fineract.useradministration.domain.AppUser; +import org.apache.fineract.useradministration.domain.AppUserPreviousPassword; +import org.apache.fineract.useradministration.domain.AppUserPreviousPasswordRepository; +import org.hamcrest.CoreMatchers; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.springframework.security.crypto.password.PasswordEncoder; + +public class AppUserWritePlatformServiceJpaRepositoryImplTest { + + @Mock + private PlatformPasswordEncoder platformPasswordEncoder; + @Mock + private ConfigurationReadPlatformService configurationReadPlatformService; + @Mock + private AppUserPreviousPasswordRepository appUserPreviewPasswordRepository; + @Mock + private PasswordEncoder passwordEncoder; + + @InjectMocks + private AppUserWritePlatformServiceJpaRepositoryImpl appUserService; + + private AppUser user; + + @BeforeEach + public void setup() { + MockitoAnnotations.initMocks(this); + user = mock(AppUser.class); + } + + @AfterEach + public void tearDown() { + user = null; + } + + @Test + public void testThatResetPasswordRequiresGlobalConfigurationEnabledToWork() { + try { + + user = mock(AppUser.class); + JsonCommand command = mock(JsonCommand.class); + String encodedPassword = "{SHA-256}{1}5787039480429368bf94732aacc771cd0a3ea02bcf504ffe1185ab94213bc63a"; // decoded-password: + + when(user.getId()).thenReturn(1L); + when(user.getPassword()).thenReturn(encodedPassword); + Mockito.mockStatic(DateUtils.class); + when(DateUtils.getLocalDateOfTenant()).thenReturn(LocalDate.of(2023, 9, 10)); + when(user.getEncodedPassword(command, platformPasswordEncoder)).thenReturn(encodedPassword); + + GlobalConfigurationPropertyData config = new GlobalConfigurationPropertyData(); + config.setEnabled(false); + config.setValue(1L); // Number of previous passwords + when(configurationReadPlatformService.retrieveGlobalConfiguration("Restrict-re-use-of-password")).thenReturn(config); + appUserService.getCurrentPasswordToSaveAsPreview(user, command); + + Mockito.verify(appUserPreviewPasswordRepository, Mockito.times(0)).findByUserId(eq(1L), any()); + Mockito.verify(configurationReadPlatformService, Mockito.times(1)).retrieveGlobalConfiguration(Mockito.anyString()); + + } catch (Exception e) { + fail("testThatResetPasswordRequiresGlobalConfigurationEnabledToWork has failed"); + assertThat(e.getMessage(), CoreMatchers.containsString( + "Reset Password is terminated. Please reach-out to your admin to enable [Restrict-re-use-of-password] in global configuration")); + + } + + } + + @Test + public void testThatICanNotResetPasswordToTheAlreadyUsedPassword() { + try { + user = mock(AppUser.class); + JsonCommand command = mock(JsonCommand.class); + String encodedPassword = "{SHA-256}{1}5787039480429368bf94732aacc771cd0a3ea02bcf504ffe1185ab94213bc63a"; // decoded-password: + + String originalPassword = "password"; + + when(user.getId()).thenReturn(1L); + when(user.getPassword()).thenReturn(encodedPassword); + when(DateUtils.getLocalDateOfTenant()).thenReturn(LocalDate.of(2023, 9, 10)); + when(user.getEncodedPassword(command, platformPasswordEncoder)).thenReturn(encodedPassword); + when(command.stringValueOfParameterNamed("password")).thenReturn(originalPassword); + + GlobalConfigurationPropertyData config = new GlobalConfigurationPropertyData(); + config.setEnabled(true); + config.setValue(1L); // Number of previous passwords + when(configurationReadPlatformService.retrieveGlobalConfiguration("Restrict-re-use-of-password")).thenReturn(config); + + List passwordHistory = new ArrayList<>(); + AppUserPreviousPassword previousPassword = new AppUserPreviousPassword(user); + passwordHistory.add(previousPassword); + when(appUserPreviewPasswordRepository.findByUserId(eq(1L), any())).thenReturn(passwordHistory); + when(passwordEncoder.matches(originalPassword, encodedPassword)).thenReturn(Boolean.TRUE); + + appUserService.getCurrentPasswordToSaveAsPreview(user, command); + + fail("testThatICanNotResetPasswordToTheAlreadyUsedPassword has failed"); + + } catch (Exception e) { + Mockito.verify(passwordEncoder, Mockito.times(1)).matches(Mockito.anyString(), Mockito.anyString()); + Mockito.verify(configurationReadPlatformService, Mockito.times(1)).retrieveGlobalConfiguration(Mockito.anyString()); + Mockito.verify(appUserPreviewPasswordRepository, Mockito.times(1)).findByUserId(eq(1L), any()); + assertThat(e.getMessage(), CoreMatchers.containsString("The submitted password has already been used in the past")); + } + } + +}