From d26fb96133b2df70fa3d31a9613f17642f0c5e6d Mon Sep 17 00:00:00 2001 From: 0xFF Date: Tue, 20 Dec 2016 12:21:58 +0300 Subject: [PATCH] JC-2379 Clear password and passwordConfirm fields when they're different during new user registration --- .../TransactionalAuthenticator.java | 44 ++++++++++- .../TransactionalAuthenticatorTest.java | 74 +++++++++++++++++-- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/jcommune-service/src/main/java/org/jtalks/jcommune/service/transactional/TransactionalAuthenticator.java b/jcommune-service/src/main/java/org/jtalks/jcommune/service/transactional/TransactionalAuthenticator.java index 8424ee0fb9..76eaa60787 100644 --- a/jcommune-service/src/main/java/org/jtalks/jcommune/service/transactional/TransactionalAuthenticator.java +++ b/jcommune-service/src/main/java/org/jtalks/jcommune/service/transactional/TransactionalAuthenticator.java @@ -146,7 +146,7 @@ public AuthenticationStatus authenticate(LoginUserDto loginUserDto, HttpServletR user = getByUsername(loginUserDto.getUserName()); result = authenticateDefault(user, loginUserDto.getPassword(), loginUserDto.isRememberMe(), request, response); } catch (NotFoundException e) { - LOGGER.info("User was not found during login process, username = {}, IP={}", + LOGGER.info("User was not found during login process, username = {}, IP={}", loginUserDto.getUserName(), loginUserDto.getClientIp()); result = authenticateByPluginAndUpdateUserInfo(loginUserDto, true, request, response); } catch(DisabledException e) { @@ -177,7 +177,7 @@ private AuthenticationStatus authenticateByPluginAndUpdateUserInfo(LoginUserDto String passwordHash = encryptionService.encryptPassword(loginUserDto.getPassword()); String encodedUsername; try { - encodedUsername = loginUserDto.getUserName() == null ? null : + encodedUsername = loginUserDto.getUserName() == null ? null : URLEncoder.encode(loginUserDto.getUserName(), "UTF-8").replace("+", "%20"); } catch (UnsupportedEncodingException e) { LOGGER.error("Could not encode username '{}'", loginUserDto.getUserName()); @@ -341,7 +341,12 @@ public BindingResult register(RegisterUserDto registerUserDto) } if(result.hasErrors()) { - userDto.setPassword(notEncodedPassword); + if (wrongPasswordConfirmError(result)) { + removePasswords(registerUserDto); + result = removeWrongPasswordConfirmError(result); + }else { + userDto.setPassword(notEncodedPassword); + } } return result; @@ -446,5 +451,36 @@ public JCUser storeRegisteredUser(UserDto userDto) { LOGGER.info("JCUser registered: {}", user.getUsername()); return user; } - + + private void removePasswords(final RegisterUserDto registerUserDto) { + registerUserDto.setPasswordConfirm(null); + registerUserDto.getUserDto().setPassword(null); + } + + private BindingResult removeWrongPasswordConfirmError(BindingResult srcErrors) { + BindingResult result = new BeanPropertyBindingResult(srcErrors.getTarget(), srcErrors.getObjectName()); + for (FieldError error : srcErrors.getFieldErrors()) { + if (wrongPasswordConfirmError(error)) { + result.addError(new FieldError(error.getObjectName(), error.getField(), error.getDefaultMessage())); + } else { + result.addError(error); + } + } + + return result; + } + + private boolean wrongPasswordConfirmError(final FieldError error) { + return "passwordConfirm".equals(error.getField()); + } + + private boolean wrongPasswordConfirmError(final BindingResult errors) { + for (FieldError error : errors.getFieldErrors()) { + if (wrongPasswordConfirmError(error)) { + return true; + } + } + + return false; + } } diff --git a/jcommune-service/src/test/java/org/jtalks/jcommune/service/transactional/TransactionalAuthenticatorTest.java b/jcommune-service/src/test/java/org/jtalks/jcommune/service/transactional/TransactionalAuthenticatorTest.java index 7957017337..5b46216146 100644 --- a/jcommune-service/src/test/java/org/jtalks/jcommune/service/transactional/TransactionalAuthenticatorTest.java +++ b/jcommune-service/src/test/java/org/jtalks/jcommune/service/transactional/TransactionalAuthenticatorTest.java @@ -21,23 +21,24 @@ import org.jtalks.common.model.entity.User; import org.jtalks.common.service.security.SecurityContextHolderFacade; import org.jtalks.jcommune.model.dao.UserDao; +import org.jtalks.jcommune.model.dto.LoginUserDto; import org.jtalks.jcommune.model.dto.RegisterUserDto; import org.jtalks.jcommune.model.dto.UserDto; import org.jtalks.jcommune.model.entity.JCUser; +import org.jtalks.jcommune.plugin.api.PluginLoader; import org.jtalks.jcommune.plugin.api.core.AuthenticationPlugin; import org.jtalks.jcommune.plugin.api.core.Plugin; import org.jtalks.jcommune.plugin.api.core.RegistrationPlugin; import org.jtalks.jcommune.plugin.api.exceptions.NoConnectionException; import org.jtalks.jcommune.plugin.api.exceptions.UnexpectedErrorException; +import org.jtalks.jcommune.plugin.api.filters.TypeFilter; import org.jtalks.jcommune.service.Authenticator; import org.jtalks.jcommune.service.PluginService; -import org.jtalks.jcommune.service.util.AuthenticationStatus; import org.jtalks.jcommune.service.nontransactional.EncryptionService; import org.jtalks.jcommune.service.nontransactional.ImageService; import org.jtalks.jcommune.service.nontransactional.MailService; -import org.jtalks.jcommune.plugin.api.PluginLoader; -import org.jtalks.jcommune.plugin.api.filters.TypeFilter; import org.jtalks.jcommune.service.security.AdministrationGroup; +import org.jtalks.jcommune.service.util.AuthenticationStatus; import org.mockito.Mock; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.BadCredentialsException; @@ -60,11 +61,18 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.jtalks.jcommune.model.dto.LoginUserDto; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.refEq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; -import static org.testng.Assert.*; +import static org.testng.Assert.assertEquals; import static org.unitils.reflectionassert.ReflectionAssert.assertReflectionEquals; /** @@ -486,6 +494,60 @@ public void userMustHaveInitialFieldValuesWhenPluginRegistrationFailWithRegistra assertReflectionEquals(expectedRegisterUserDto, registerUserDto); } + @Test + public void userMustSetToNullPasswordAndPasswordConfirmFieldsWhenTheyAreDifferent() throws Exception { + + Validator customValidator = new Validator() { + @Override + public boolean supports(Class clazz) { + return true; + } + + @Override + public void validate(Object target, Errors errors) { + errors.rejectValue("passwordConfirm", "", "Password and confirmation password do not match"); + } + }; + RegisterUserDto registerUserDto = createRegisterUserDto("username", "password", "email@mail.ru", null); + registerUserDto.setPasswordConfirm("wrongPassword"); + EncryptionService realEncryptionService = new EncryptionService(new Md5PasswordEncoder()); + ReflectionTestUtils.setField(authenticator, "encryptionService", realEncryptionService); + ReflectionTestUtils.setField(authenticator, "validator", customValidator); + when(pluginService.getRegistrationPlugins()).thenReturn(Collections.EMPTY_MAP); + + authenticator.register(registerUserDto); + RegisterUserDto expectedRegisterUserDto = createRegisterUserDto("username", null, "email@mail.ru", null); + + assertReflectionEquals(expectedRegisterUserDto, registerUserDto); + } + + @Test + public void userMustRestorePasswordAndPasswordConfirmFieldsWhenTheyAreEquals() throws Exception { + Validator customValidator = new Validator() { + @Override + public boolean supports(Class clazz) { + return true; + } + + @Override + public void validate(Object target, Errors errors) { + errors.rejectValue("userDto.email", "", "An email format should be like mail@mail.ru"); + } + }; + RegisterUserDto registerUserDto = createRegisterUserDto("username", "password", "email", null); + registerUserDto.setPasswordConfirm("password"); + EncryptionService realEncryptionService = new EncryptionService(new Md5PasswordEncoder()); + ReflectionTestUtils.setField(authenticator, "encryptionService", realEncryptionService); + ReflectionTestUtils.setField(authenticator, "validator", customValidator); + when(pluginService.getRegistrationPlugins()).thenReturn(Collections.EMPTY_MAP); + + authenticator.register(registerUserDto); + RegisterUserDto expectedRegisterUserDto = createRegisterUserDto("username", "password", "email", null); + expectedRegisterUserDto.setPasswordConfirm("password"); + + assertReflectionEquals(expectedRegisterUserDto, registerUserDto); + } + private RegisterUserDto createRegisterUserDto(String username, String password, String email, String honeypotCaptcha) { RegisterUserDto registerUserDto = new RegisterUserDto(); UserDto userDto = new UserDto();