Skip to content

Commit

Permalink
change passwordChange REST & services
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukaskabc
lukaskabc committed Jul 18, 2024
1 parent f08af2b commit 474e0af
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 130 deletions.
15 changes: 0 additions & 15 deletions src/main/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDao.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,6 @@ public PasswordChangeRequestDao(EntityManager em, Configuration configuration) {
this.persistenceConfig = configuration.getPersistence();
}

public Optional<PasswordChangeRequest> findByUsername(String username) {
Objects.requireNonNull(username);
try {
return Optional.of(
em.createQuery("SELECT t FROM " + type.getSimpleName() + " t WHERE t.userAccount.username = :username ORDER BY t.createdAt DESC", type)
.setParameter("username", username, persistenceConfig.getLanguage())
.setMaxResults(1).getSingleResult()
);
} catch (NoResultException e) {
return Optional.empty();
} catch (RuntimeException e) {
throw new PersistenceException(e);
}
}

public List<PasswordChangeRequest> findAllByUsername(String username) {
Objects.requireNonNull(username);
try {
Expand Down
18 changes: 9 additions & 9 deletions src/main/java/cz/cvut/kbss/termit/rest/PasswordChangeController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
Expand All @@ -40,30 +40,30 @@ public PasswordChangeController(PasswordChangeService passwordChangeService) {

@Operation(description = "Requests a password reset for the specified username.")
@ApiResponses({
@ApiResponse(responseCode = "200", description = "Password reset request accepted, email sent"),
@ApiResponse(responseCode = "204", description = "Password reset request accepted, email sent"),
@ApiResponse(responseCode = "404", description = "User with the specified username not found.")
})
@PreAuthorize("permitAll()")
@PostMapping(path = "/reset/{username}")
@PostMapping(consumes = {MediaType.TEXT_PLAIN_VALUE})
public ResponseEntity<Void> requestPasswordReset(
@Parameter(description = "Username of the user") @PathVariable String username) {
@Parameter(description = "Username of the user") @RequestBody String username) {
LOG.info("Password reset requested for user {}.", username);
passwordChangeService.requestPasswordReset(username);
return new ResponseEntity<>(HttpStatus.OK);
return new ResponseEntity<>(HttpStatus.NO_CONTENT);
}

@Operation(description = "Changes the password for the specified user.")
@ApiResponses({
@ApiResponse(responseCode = "200", description = "Password changed"),
@ApiResponse(responseCode = "403", description = "Invalid or expired token")
@ApiResponse(responseCode = "204", description = "Password changed"),
@ApiResponse(responseCode = "409", description = "Invalid or expired token")
})
@PreAuthorize("permitAll()")
@PostMapping(path = "/change", consumes = {MediaType.APPLICATION_JSON_VALUE, JsonLd.MEDIA_TYPE})
@PutMapping(consumes = {MediaType.APPLICATION_JSON_VALUE, JsonLd.MEDIA_TYPE})
public ResponseEntity<Void> changePassword(
@Parameter(
description = "Token with URI for password reset") @RequestBody PasswordChangeDto passwordChangeDto) {
LOG.info("Password change requested with token {}", passwordChangeDto.getToken());
passwordChangeService.changePassword(passwordChangeDto);
return new ResponseEntity<>(HttpStatus.OK);
return new ResponseEntity<>(HttpStatus.NO_CONTENT);
}
}
2 changes: 1 addition & 1 deletion src/main/java/cz/cvut/kbss/termit/rest/handler/RestExceptionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,6 @@ public ResponseEntity<ErrorInfo> vocabularyImportException(HttpServletRequest re
@ExceptionHandler
public ResponseEntity<ErrorInfo> invalidPasswordChangeRequestException(HttpServletRequest request, InvalidPasswordChangeRequestException e) {
logException(e, request);
return new ResponseEntity<>(ErrorInfo.createWithMessageAndMessageId(e.getMessage(), e.getMessageId(), request.getRequestURI()), HttpStatus.FORBIDDEN);
return new ResponseEntity<>(ErrorInfo.createWithMessageAndMessageId(e.getMessage(), e.getMessageId(), request.getRequestURI()), HttpStatus.CONFLICT);
}
}
3 changes: 1 addition & 2 deletions src/main/java/cz/cvut/kbss/termit/service/business/PasswordChangeService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ public class PasswordChangeService {
private final UserRepositoryService userRepositoryService;
private final PasswordChangeNotifier passwordChangeNotifier;

@Autowired
public PasswordChangeService(PasswordChangeRequestRepositoryService passwordChangeRequestRepositoryService,
Configuration configuration, UserRepositoryService userRepositoryService,
PasswordChangeNotifier passwordChangeNotifier) {
Expand All @@ -52,7 +51,7 @@ public void requestPasswordReset(String username) {
passwordChangeNotifier.sendEmail(request);
}

public boolean isValid(PasswordChangeRequest request) {
private boolean isValid(PasswordChangeRequest request) {
return request.getCreatedAt().plus(securityConfig.getPasswordChangeRequestValidity()).isAfter(Instant.now());
}

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/cz/cvut/kbss/termit/service/mail/Postman.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ public Postman(Environment env, @Autowired(required = false) JavaMailSender mail

@PostConstruct
public void postConstruct() {
// exclude mailSender check for test environment
if(mailSender == null && Arrays.stream(env.getActiveProfiles()).noneMatch("test"::equals)) {
if(mailSender == null) {
throw new ValidationException("Mail server not configured.");
}
}
Expand Down
8 changes: 0 additions & 8 deletions ...n/java/cz/cvut/kbss/termit/service/repository/PasswordChangeRequestRepositoryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,6 @@ public PasswordChangeRequest create(UserAccount userAccount) {
return request;
}

public PasswordChangeRequest findByUsernameRequired(String username) {
return findByUsername(username).orElseThrow(() -> NotFoundException.create(PasswordChangeRequest.class.getSimpleName(), username));
}

public Optional<PasswordChangeRequest> findByUsername(String username) {
return passwordChangeRequestDao.findByUsername(username);
}

public List<PasswordChangeRequest> findAllByUsername(String username) {
return passwordChangeRequestDao.findAllByUsername(username);
}
Expand Down
59 changes: 0 additions & 59 deletions src/test/java/cz/cvut/kbss/termit/persistence/dao/PasswordChangeRequestDaoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,65 +28,6 @@ private String randomToken() {
return UUID.randomUUID().toString();
}

@Test
void findByUsernameReturnsResultMatchingTheToken() {
final UserAccount user = Generator.generateUserAccountWithPassword();
transactional(() -> em.persist(user));

final String TOKEN = randomToken();
final PasswordChangeRequest passwordChangeRequest = new PasswordChangeRequest();
passwordChangeRequest.setToken(TOKEN);
passwordChangeRequest.setUserAccount(user);
passwordChangeRequest.setCreatedAt(Instant.now());
transactional(() -> em.persist(passwordChangeRequest));

final Optional<PasswordChangeRequest> result = sut.findByUsername(user.getUsername());
assertTrue(result.isPresent());
assertEquals(TOKEN, result.get().getToken());
assertEquals(user.getUri(), result.get().getUserAccount().getUri());
}

@Test
void findByUsernameReturnsEmptyOptionalWhenNoMatchingRequestIsFound() {
final UserAccount user = Generator.generateUserAccountWithPassword();
transactional(() -> em.persist(user));

final String TOKEN = randomToken();
final PasswordChangeRequest passwordChangeRequest = new PasswordChangeRequest();
passwordChangeRequest.setToken(TOKEN);
passwordChangeRequest.setUserAccount(user);
passwordChangeRequest.setCreatedAt(Instant.now());
transactional(() -> em.persist(passwordChangeRequest));

final Optional<PasswordChangeRequest> result = sut.findByUsername("[email protected]");
assertFalse(result.isPresent());
}

@Test
void findByUsernameReturnsSingleResultWhenMultipleArePresent() {
final UserAccount user = Generator.generateUserAccountWithPassword();
transactional(() -> em.persist(user));

final String TOKEN = randomToken();
final String ANOTHER_TOKEN = randomToken();
final PasswordChangeRequest passwordChangeRequest = new PasswordChangeRequest();
final PasswordChangeRequest secondPasswordChangeRequest = new PasswordChangeRequest();
passwordChangeRequest.setToken(TOKEN);
secondPasswordChangeRequest.setToken(ANOTHER_TOKEN);
passwordChangeRequest.setUserAccount(user);
secondPasswordChangeRequest.setUserAccount(user);
// make the password request the latest
passwordChangeRequest.setCreatedAt(Instant.now().plusSeconds(1));
secondPasswordChangeRequest.setCreatedAt(Instant.now());
transactional(() -> em.persist(passwordChangeRequest));
transactional(() -> em.persist(secondPasswordChangeRequest));

final Optional<PasswordChangeRequest> result = sut.findByUsername(user.getUsername());
assertTrue(result.isPresent());
assertEquals(TOKEN, result.get().getToken());
assertEquals(user.getUri(), result.get().getUserAccount().getUri());
}

@Test
void findAllByUsernameReturnsAllResults() {
final UserAccount user = Generator.generateUserAccountWithPassword();
Expand Down
24 changes: 15 additions & 9 deletions src/test/java/cz/cvut/kbss/termit/rest/PasswordChangeControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import java.util.UUID;

import static cz.cvut.kbss.termit.service.business.PasswordChangeService.INVALID_TOKEN_ERROR_MESSAGE_ID;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.refEq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

Expand All @@ -43,8 +45,10 @@ void setUp() {
void passwordResetRequestsPasswordReset() throws Exception {
final UserAccount userAccount = Generator.generateUserAccount();

mockMvc.perform(post("/password/reset/" + userAccount.getUsername()))
.andExpect(status().isOk());
mockMvc.perform(post("/password")
.content(userAccount.getUsername())
.contentType(MediaType.TEXT_PLAIN))
.andExpect(status().isNoContent());
verify(passwordChangeService).requestPasswordReset(userAccount.getUsername());
}

Expand All @@ -56,10 +60,12 @@ void passwordChangeRequestedInvalidUserNotFoundReturned() throws Exception {
doThrow(NotFoundException.create(UserAccount.class, username))
.when(passwordChangeService).requestPasswordReset(username);

mockMvc.perform(post("/password/reset/" + username))
mockMvc.perform(post("/password")
.content(username)
.contentType(MediaType.TEXT_PLAIN))
.andExpect(status().isNotFound());

verify(passwordChangeService).requestPasswordReset(username);
verify(passwordChangeService).requestPasswordReset(eq(username));
}

@Test
Expand All @@ -69,13 +75,13 @@ void passwordChangeChangesPassword() throws Exception {
dto.setNewPassword(UUID.randomUUID().toString());
dto.setToken(UUID.randomUUID().toString());

mockMvc.perform(post("/password/change").content(toJson(dto)).contentType(MediaType.APPLICATION_JSON_VALUE))
.andExpect(status().isOk());
mockMvc.perform(put("/password").content(toJson(dto)).contentType(MediaType.APPLICATION_JSON_VALUE))
.andExpect(status().isNoContent());
verify(passwordChangeService).changePassword(refEq(dto));
}

@Test
void passwordChangeRequestedWithInvalidTokenForbiddenReturned() throws Exception {
void passwordChangeRequestedWithInvalidTokenConflictReturned() throws Exception {
final PasswordChangeDto dto = new PasswordChangeDto();
dto.setUri(Generator.generateUri());
dto.setNewPassword(UUID.randomUUID().toString());
Expand All @@ -84,9 +90,9 @@ void passwordChangeRequestedWithInvalidTokenForbiddenReturned() throws Exception
doThrow(new InvalidPasswordChangeRequestException("Invalid or expired password change link", INVALID_TOKEN_ERROR_MESSAGE_ID))
.when(passwordChangeService).changePassword(refEq(dto));

mockMvc.perform(post("/password/change").content(toJson(dto)).contentType(MediaType.APPLICATION_JSON_VALUE))
mockMvc.perform(put("/password").content(toJson(dto)).contentType(MediaType.APPLICATION_JSON_VALUE))
.andExpect(jsonPath("messageId").value(INVALID_TOKEN_ERROR_MESSAGE_ID))
.andExpect(status().isForbidden());
.andExpect(status().isConflict());

verify(passwordChangeService).changePassword(refEq(dto));
}
Expand Down
26 changes: 1 addition & 25 deletions src/test/java/cz/cvut/kbss/termit/service/business/PasswordChangeServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,30 +117,6 @@ void requestPasswordResetInvalidUsernameExceptionThrown() {
verify(userRepositoryService).findByUsername(username);
}


@Test
void expiredRequestIsNotValid() {
final PasswordChangeRequest request = new PasswordChangeRequest();
request.setCreatedAt(Instant.now().minus(configuration.getSecurity()
.getPasswordChangeRequestValidity())
.minusNanos(1));

boolean isValid = sut.isValid(request);
assertFalse(isValid);
}

@Test
void requestIsValid() {
final PasswordChangeRequest request = new PasswordChangeRequest();
Instant created = Instant.now().minus(configuration.getSecurity()
.getPasswordChangeRequestValidity()
.dividedBy(2));
request.setCreatedAt(created);

boolean isValid = sut.isValid(request);
assertTrue(isValid);
}

@Test
void changePasswordValidRequestPasswordChanged() {
final UserAccount account = Generator.generateUserAccountWithPassword();
Expand Down Expand Up @@ -184,7 +160,7 @@ void changePasswordRequestNotFoundExceptionThrown() {
}

@Test
void changePasswordInvalidRequestExceptionThrown() {
void changePasswordExpiredRequestExceptionThrown() {
final PasswordChangeRequest request = new PasswordChangeRequest();
request.setCreatedAt(Instant.now().minus(configuration.getSecurity()
.getPasswordChangeRequestValidity())
Expand Down

0 comments on commit 474e0af

Please sign in to comment.