Skip to content

Commit

Permalink
Merge pull request #19 from OpenLMIS/OIS-55
Browse files Browse the repository at this point in the history
OIS-55: Introduce new secure password policy with zxcvbn4j
  • Loading branch information
pwargulak authored Sep 30, 2024
2 parents f6d5d6d + b74b78f commit 8777a88
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 9 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ dependencies {
compile 'org.apache.commons:commons-csv:1.4'
compile 'org.apache.commons:commons-collections4:4.1'
compile 'commons-io:commons-io:2.5'
compile 'com.nulab-inc:zxcvbn:1.9.0'

annotationProcessor 'org.projectlombok:lombok:1.18.22'

Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/openlmis/auth/i18n/MessageKeys.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ public abstract class MessageKeys {
+ ".passwordReset.notContainNumber";
public static final String USERS_PASSWORD_RESET_CONTAIN_SPACES = USERS
+ ".passwordReset.containSpaces";

public static final String USERS_PASSWORD_CONTAIN_USER_DETAILS = USERS
+ ".passwordReset.containUserDetails";
public static final String USERS_PASSWORD_TOO_WEAK = USERS
+ ".passwordReset.tooWeak";

public static final String USER_NOT_FOUND = USERS + ".notFound";

public static final String USER_NOT_FOUND_BY_EMAIL = USERS + ".notFoundByEmail";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@

package org.openlmis.auth.service.referencedata;

import java.util.Collections;
import java.util.Map;
import java.util.UUID;
import lombok.Getter;
import org.openlmis.auth.dto.ResultDto;
import org.openlmis.auth.dto.referencedata.UserMainDetailsDto;
import org.openlmis.auth.service.BaseCommunicationService;
import org.openlmis.auth.service.RequestParameters;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.data.domain.Page;
import org.springframework.stereotype.Service;

@Service
Expand Down Expand Up @@ -58,4 +61,18 @@ public ResultDto<Boolean> hasRight(UUID user, UUID right) {

return getResult(user + "/hasRight", parameters, Boolean.class);
}

/**
* This method retrieves a user with given name.
*
* @param name the name of user.
* @return UserDto containing user's data, or null if such user was not found.
*/
public UserMainDetailsDto findUser(String name) {
Map<String, Object> payload = Collections.singletonMap("username", name);

Page<UserMainDetailsDto> users = getPage("search", Collections.emptyMap(), payload);
return users.getContent().isEmpty() ? null : users.getContent().get(0);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
package org.openlmis.auth.web;

import org.openlmis.auth.dto.PasswordResetRequestDto;
import org.openlmis.auth.dto.referencedata.UserMainDetailsDto;
import org.openlmis.auth.i18n.MessageKeys;
import org.openlmis.auth.service.referencedata.UserReferenceDataService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import org.springframework.validation.Errors;

Expand All @@ -27,8 +30,14 @@ public class PasswordResetRequestDtoValidator extends BaseValidator {
private static final String PASS_FIELD = "newPassword";
private static final String REGEX_CONTAINS_NUMBER = "(?=.*[0-9]).+";
private static final String REGEX_CONTAINS_SPACES = "(?=\\S+$).+";
private static final String REGEX_SIZE_IS_BETWEEN_8_AND_16 = "^[a-zA-Z0-9]{8,16}$";

private static final String REGEX_SIZE_IS_BETWEEN_8_AND_72 = "^[a-zA-Z0-9]{8,72}$";

@Autowired
private UserReferenceDataService userReferenceDataService;

@Autowired
private PasswordStrengthValidator passwordStrengthValidator;

@Override
public boolean supports(Class<?> clazz) {
return PasswordResetRequestDto.class.equals(clazz);
Expand All @@ -38,22 +47,38 @@ public boolean supports(Class<?> clazz) {
public void validate(Object target, Errors errors) {
rejectIfEmptyOrWhitespace(errors, USERNAME, MessageKeys.ERROR_FIELD_REQUIRED);
rejectIfEmptyOrWhitespace(errors, PASS_FIELD, MessageKeys.ERROR_FIELD_REQUIRED);

if (!errors.hasErrors()) {
PasswordResetRequestDto passwordResetRequestDto = (PasswordResetRequestDto) target;
verifyPassword(passwordResetRequestDto.getNewPassword(), errors);
verifyPassword(passwordResetRequestDto, errors);
passwordStrengthValidator.verifyPasswordStrength(passwordResetRequestDto.getNewPassword());
}
}

private void verifyPassword(String password, Errors errors) {
private void verifyPassword(PasswordResetRequestDto passwordResetRequestDto, Errors errors) {
String password = passwordResetRequestDto.getNewPassword();
if (!password.matches(REGEX_CONTAINS_NUMBER)) {
rejectValue(errors, PASS_FIELD, MessageKeys.USERS_PASSWORD_RESET_NOT_CONTAIN_NUMBER);
}
if (!password.matches(REGEX_CONTAINS_SPACES)) {
rejectValue(errors, PASS_FIELD, MessageKeys.USERS_PASSWORD_RESET_CONTAIN_SPACES);
}
if (!password.matches(REGEX_SIZE_IS_BETWEEN_8_AND_16)) {
if (!password.matches(REGEX_SIZE_IS_BETWEEN_8_AND_72)) {
rejectValue(errors, PASS_FIELD, MessageKeys.USERS_PASSWORD_RESET_INVALID_PASSWORD_LENGTH);
}
UserMainDetailsDto userDetails =
userReferenceDataService.findUser(passwordResetRequestDto.getUsername());
if (containsUserDetails(password, userDetails.getUsername(),
userDetails.getFirstName(), userDetails.getLastName())) {
rejectValue(errors, PASS_FIELD, MessageKeys.USERS_PASSWORD_CONTAIN_USER_DETAILS);
}
}

private boolean containsUserDetails(String password, String username, String firstName,
String secondName) {
return password.toLowerCase().contains(username.toLowerCase())
|| password.toLowerCase().contains(firstName.toLowerCase())
|| password.toLowerCase().contains(secondName.toLowerCase());
}

}
76 changes: 76 additions & 0 deletions src/main/java/org/openlmis/auth/web/PasswordStrengthValidator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* This program is part of the OpenLMIS logistics management information system platform software.
* Copyright © 2017 VillageReach
*
* This program is free software: you can redistribute it and/or modify it under the terms
* of the GNU Affero General Public License as published by the Free Software Foundation, either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
* without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the GNU Affero General Public License for more details. You should have received a copy of
* the GNU Affero General Public License along with this program. If not, see
* http://www.gnu.org/licenses.  For additional information contact info@OpenLMIS.org.
*/

package org.openlmis.auth.web;

import com.nulabinc.zxcvbn.Feedback;
import com.nulabinc.zxcvbn.Strength;
import com.nulabinc.zxcvbn.Zxcvbn;
import java.util.List;
import lombok.AllArgsConstructor;
import lombok.Getter;
import org.apache.logging.log4j.util.Strings;
import org.openlmis.auth.exception.ValidationMessageException;
import org.openlmis.auth.i18n.MessageKeys;
import org.openlmis.auth.util.Message;
import org.springframework.stereotype.Component;

@Component
public class PasswordStrengthValidator {

private static final int MINIMAL_PASSWORD_STRENGTH_SCORE =
PasswordStrengthScore.GOOD.getScore();

/**
* Checks the strength of the given password.
*
* @param password the password to check.
* @throws ValidationMessageException if the password's strength is below the required threshold.
*/
public void verifyPasswordStrength(String password) {
Zxcvbn zxcvbn = new Zxcvbn();
Strength strength = zxcvbn.measure(password);

if (strength.getScore() < MINIMAL_PASSWORD_STRENGTH_SCORE) {
Feedback feedback = strength.getFeedback();
List<String> suggestions = feedback.getSuggestions();
String suggestionsMessage = buildSuggestionsMessage(suggestions);

// feedback.warning can sometimes be an empty string
String reason = (feedback.getWarning() == null || feedback.getWarning().isEmpty())
? Strings.EMPTY : feedback.getWarning() + " ";

throw new ValidationMessageException(new Message(MessageKeys.USERS_PASSWORD_TOO_WEAK,
reason,
suggestionsMessage));
}
}

private String buildSuggestionsMessage(List<String> suggestions) {
if (suggestions.isEmpty()) {
return Strings.EMPTY;
}
String suggestionPrefix = suggestions.size() > 1 ? "Suggestions: " : "Suggestion: ";
return suggestionPrefix + String.join(", ", suggestions);
}

@AllArgsConstructor
@Getter
enum PasswordStrengthScore {
WEAK(0), FAIR(1), GOOD(2), STRONG(3), VERY_STRONG(4);
private final int score;
}

}
4 changes: 3 additions & 1 deletion src/main/resources/messages_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ auth.error.username.invalid=Username contains invalid characters, only alphanume

users.logout.confirmation=You have successfully logged out!

users.passwordReset.invalidPasswordLength=Password size must be between 8 and 16.
users.passwordReset.invalidPasswordLength=Password size must be between 8 and 72.
users.passwordReset.notContainNumber=Password must contain at least 1 digit.
users.passwordReset.containSpaces=Password must not contain spaces.
users.passwordReset.containUserDetails=Password must not contain user data such as username, first name or last name.
users.passwordReset.tooWeak=Provided password is too weak. {0}{1}

users.notFound=User does not exist
users.notFoundByEmail=User with provided email does not exist.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.when;

import java.util.Locale;
Expand All @@ -26,10 +27,12 @@
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.junit.MockitoJUnitRunner;
import org.openlmis.auth.dto.PasswordResetRequestDto;
import org.openlmis.auth.dto.referencedata.UserMainDetailsDto;
import org.openlmis.auth.i18n.ExposedMessageSource;
import org.openlmis.auth.i18n.MessageKeys;
import org.openlmis.auth.service.referencedata.UserReferenceDataService;
import org.springframework.validation.BeanPropertyBindingResult;
import org.springframework.validation.Errors;
import org.springframework.validation.Validator;
Expand All @@ -43,22 +46,37 @@ public class PasswordResetRequestDtoValidatorTest {
@Mock
private ExposedMessageSource messageSource;

@Mock
private UserReferenceDataService userReferenceDataService;

@Mock
private PasswordStrengthValidator passwordStrengthValidator;

@InjectMocks
private Validator validator = new PasswordResetRequestDtoValidator();

private PasswordResetRequestDto request;
private Errors errors;
private UserMainDetailsDto userMainDetails;

@Before
public void setUp() throws Exception {
request = new PasswordResetRequestDto();
request.setNewPassword("testpassword1");
request.setUsername("testusername");

userMainDetails = new UserMainDetailsDto();
userMainDetails.setUsername(request.getUsername());
userMainDetails.setFirstName("testfirstname");
userMainDetails.setLastName("testlastname");

errors = new BeanPropertyBindingResult(request, "request");

when(messageSource.getMessage(anyString(), any(Object[].class), any(Locale.class)))
.thenAnswer(invocation -> invocation.getArgument(0, String.class));
when(userReferenceDataService.findUser(request.getUsername()))
.thenReturn(userMainDetails);
doNothing().when(passwordStrengthValidator).verifyPasswordStrength(anyString());
}

@Test
Expand Down Expand Up @@ -115,6 +133,32 @@ public void shouldRejectWhenPasswordIsNotProperSize() {
MessageKeys.USERS_PASSWORD_RESET_INVALID_PASSWORD_LENGTH);
}

@Test
public void shouldRejectWhenPasswordContainsUsername() {
request.setNewPassword("testusername1");

validator.validate(request, errors);

assertErrorMessage(errors, PASS_FIELD, MessageKeys.USERS_PASSWORD_CONTAIN_USER_DETAILS);
}

@Test
public void shouldRejectWhenPasswordContainsFirstName() {
request.setNewPassword("testfirstname1");

validator.validate(request, errors);

assertErrorMessage(errors, PASS_FIELD, MessageKeys.USERS_PASSWORD_CONTAIN_USER_DETAILS);
}

@Test
public void shouldRejectWhenPasswordContainsLastName() {
request.setNewPassword("testlastname1");

validator.validate(request, errors);

assertErrorMessage(errors, PASS_FIELD, MessageKeys.USERS_PASSWORD_CONTAIN_USER_DETAILS);
}

private void assertErrorMessage(Errors errors, String field, String expectedMessage) {
assertThat(errors.hasFieldErrors(field)).as(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* This program is part of the OpenLMIS logistics management information system platform software.
* Copyright © 2017 VillageReach
*
* This program is free software: you can redistribute it and/or modify it under the terms
* of the GNU Affero General Public License as published by the Free Software Foundation, either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
* without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the GNU Affero General Public License for more details. You should have received a copy of
* the GNU Affero General Public License along with this program. If not, see
* http://www.gnu.org/licenses.  For additional information contact info@OpenLMIS.org.
*/

package org.openlmis.auth.web;

import org.junit.Before;
import org.junit.Test;
import org.openlmis.auth.exception.ValidationMessageException;

public class PasswordStrengthValidatorTest {

private PasswordStrengthValidator passwordStrengthValidator;

@Before
public void setUp() {
passwordStrengthValidator = new PasswordStrengthValidator();
}

@Test(expected = ValidationMessageException.class)
public void shouldThrowExceptionWhenPasswordIsWeak() {
// given
final String weakPassword = "abc";

// when
passwordStrengthValidator.verifyPasswordStrength(weakPassword);
}
}

0 comments on commit 8777a88

Please sign in to comment.