Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed password reset for disabled accounts #1373

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* [UI]: Fix issue where year is displayed incorrectly when the last day of the week for the date is in another year. See [PR 1364](https://github.com/phac-nml/irida/pull/1364)
* [Developer]: Fix issue where large downloads silent failed due to async request timeout. See [PR 1368](https://github.com/phac-nml/irida/pull/1368)
* [Developer]: Update to Spring Boot 2.7.3 and update various other dependencies. See [PR 1369](https://github.com/phac-nml/irida/pull/1369)
* [Developer]: Fixed issue with disabled user accounts requesting a password reset. See [PR 1373](https://github.com/phac-nml/irida/pull/1373)

## [22.05.5] - 2022/06/28
* [UI]: Fixed bug preventing export of project samples table due to invalid url. [PR 1331](https://github.com/phac-nml/irida/pull/1331)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package ca.corefacility.bioinformatics.irida.exceptions;

/**
* Thrown when a password reset is requested by a user whose account is disabled
*
*/

public class IridaAccountDisabledException extends Exception {
public IridaAccountDisabledException(final String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.springframework.ui.Model;

import ca.corefacility.bioinformatics.irida.exceptions.EntityNotFoundException;
import ca.corefacility.bioinformatics.irida.exceptions.IridaAccountDisabledException;
import ca.corefacility.bioinformatics.irida.exceptions.PasswordReusedException;
import ca.corefacility.bioinformatics.irida.model.user.PasswordReset;
import ca.corefacility.bioinformatics.irida.model.user.Role;
Expand Down Expand Up @@ -86,16 +87,15 @@ public String createAndSendNewPasswordResetEmail(String usernameOrEmail, Locale
SecurityContextHolder.clearContext();
throw new UIEmailSendException(
messageSource.getMessage("server.ForgotPassword.errorSendingInstructions", null, locale));
}
catch (EntityNotFoundException ex) {
} catch (EntityNotFoundException ex) {
SecurityContextHolder.clearContext();
throw new EntityNotFoundException(
messageSource.getMessage("server.ForgotPassword.emailOrUsernameNotExist", null, locale));
messageSource.getMessage("server.ForgotPassword.accountNotFoundOrDisabled", null, locale));
}
} catch (EntityNotFoundException | UsernameNotFoundException ex) {
} catch (EntityNotFoundException | UsernameNotFoundException | IridaAccountDisabledException ex) {
SecurityContextHolder.clearContext();
throw new EntityNotFoundException(
messageSource.getMessage("server.ForgotPassword.emailOrUsernameNotExist", null, locale));
messageSource.getMessage("server.ForgotPassword.accountNotFoundOrDisabled", null, locale));
}
}

Expand Down Expand Up @@ -172,13 +172,19 @@ public String setNewPassword(String resetId, String password, Model model, Local
* Create a new password reset for a given {@link User} and send a reset password link via email
*
* @param user The user to create the reset for
* @throws IridaAccountDisabledException if the account is disabled
*/
private void createNewPasswordReset(User user) {
PasswordReset passwordReset = new PasswordReset(user);
passwordResetService.create(passwordReset);

// send a reset password link to user via email
emailController.sendPasswordResetLinkEmail(user, passwordReset);
private void createNewPasswordReset(User user) throws IridaAccountDisabledException {
// Only create a password reset for a user that is enabled
if (user.isEnabled()) {
PasswordReset passwordReset = new PasswordReset(user);
passwordResetService.create(passwordReset);

// send a reset password link to user via email
emailController.sendPasswordResetLinkEmail(user, passwordReset);
} else {
throw new IridaAccountDisabledException("User account is disabled");
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/i18n/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2872,7 +2872,7 @@ ForgotPassword.link.returnToLogin=Return to Login Page

server.ForgotPassword.checkEmail=Check your email for password reset instructions
server.ForgotPassword.errorSendingInstructions=There was an error sending password reset instructions. Please contact the system administrator
server.ForgotPassword.emailOrUsernameNotExist=If a user with the provided email address or username exists, you will receive an email with password reset instructions.
server.ForgotPassword.accountNotFoundOrDisabled=If a user with the provided email address or username exists, and is enabled, you will receive an email with password reset instructions.

# ========================================================================================== #
# ActivateAccount Component #
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,19 @@ public boolean checkSuccess() {
return false;
}
}

public boolean checkAccountDisabledMessage() {
try {
WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(10L));
wait.until(ExpectedConditions.presenceOfElementLocated(By.className("t-forgot-password-alert")));
WebElement element = driver.findElement(By.className("t-forgot-password-alert"));
wait.until(ExpectedConditions.textToBePresentInElement(element,
"If a user with the provided email address or username exists, and is enabled, you will receive an email with password reset instructions."));
return element.getText()
.contains(
"If a user with the provided email address or username exists, and is enabled, you will receive an email with password reset instructions.");
} catch (Exception e) {
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,14 @@ public void testCreateResetBadEmail() {
passwordResetPage.enterEmail(usernameOrEmail);
assertFalse(passwordResetPage.checkSuccess());
}

@Test
public void testCreateResetDisabledAccount() {
passwordResetPage.goTo();
passwordResetPage.clickForgotPasswordLink();
String usernameOrEmail = "[email protected]";
passwordResetPage.enterEmail(usernameOrEmail);
assertFalse(passwordResetPage.checkSuccess(), "Password reset for a disabled account should not be successful");
assertTrue(passwordResetPage.checkAccountDisabledMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.ui.Model;

import ca.corefacility.bioinformatics.irida.exceptions.EntityNotFoundException;
import ca.corefacility.bioinformatics.irida.model.user.PasswordReset;
import ca.corefacility.bioinformatics.irida.model.user.User;
import ca.corefacility.bioinformatics.irida.ria.web.exceptions.UIConstraintViolationException;
Expand All @@ -21,6 +22,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.*;

public class UIPasswordResetServiceTest {
Expand Down Expand Up @@ -82,4 +84,14 @@ void testSetNewPassword() throws UIConstraintViolationException {

assertEquals("success", result, "Result should be success");
}

@Test
void testCreateAndSendNewPasswordResetEmailAccountDisabled() {
when(userService.loadUserByEmail(user.getEmail())).thenReturn(user);
user.setEnabled(false);

assertThrows(EntityNotFoundException.class, () -> {
service.createAndSendNewPasswordResetEmail(user.getEmail(), Locale.ENGLISH);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
password="$2a$10$jFFix3ZyyoNy7HwavYjXauV0vByoPVbS1WnRpxPBCTKFXwEJeyXiK"
phoneNumber="867-5309" username="differentUser" enabled="true"
system_role="ROLE_USER" credentialsNonExpired="true" />
<user id="3" createdDate="2013-07-18 14:20:19.0" modifiedDate="2013-07-18 14:20:19.0"
email="[email protected]" firstName="Mr." lastName="DisabledAccountUser"
password="$2a$10$jFFix3ZyyoNy7HwavYjXauV0vByoPVbS1WnRpxPBCTKFXwEJeyXiK"
phoneNumber="867-5309" username="differentUserAccountDisabled" enabled="false"
system_role="ROLE_USER" credentialsNonExpired="true" />

<password_reset id="XYZ" createdDate="2013-07-18 14:20:19.0"
user_id="2" />
Expand Down