refactor(auth): route password reset through service layer + e2e helper
- PasswordResetService injects UserService instead of AppUserRepository.
- New UserService.findByEmailOptional preserves the silent-fail behaviour of
the old findByEmail-returning-Optional path; the existing throwing
findByEmail is unchanged.
- New PasswordResetService.findLatestActiveTokenForEmail exposes the latest
active reset token without leaking the repository upward.
- New @Profile("e2e") PasswordResetTestHelper wraps that read so the
AuthE2EController no longer touches PasswordResetTokenRepository directly.
Profile guard moves from the controller-only annotation to also cover the
helper bean, so the production graph never instantiates either.
Refs #417 (C6.1 violation #2 + C6.2 violation #12).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,8 @@
|
|||||||
package org.raddatz.familienarchiv.controller;
|
package org.raddatz.familienarchiv.controller;
|
||||||
|
|
||||||
import java.time.LocalDateTime;
|
import io.swagger.v3.oas.annotations.Operation;
|
||||||
|
import lombok.RequiredArgsConstructor;
|
||||||
import org.raddatz.familienarchiv.repository.PasswordResetTokenRepository;
|
import org.raddatz.familienarchiv.service.PasswordResetTestHelper;
|
||||||
import org.springframework.context.annotation.Profile;
|
import org.springframework.context.annotation.Profile;
|
||||||
import org.springframework.http.ResponseEntity;
|
import org.springframework.http.ResponseEntity;
|
||||||
import org.springframework.web.bind.annotation.GetMapping;
|
import org.springframework.web.bind.annotation.GetMapping;
|
||||||
@@ -10,10 +10,6 @@ import org.springframework.web.bind.annotation.RequestMapping;
|
|||||||
import org.springframework.web.bind.annotation.RequestParam;
|
import org.springframework.web.bind.annotation.RequestParam;
|
||||||
import org.springframework.web.bind.annotation.RestController;
|
import org.springframework.web.bind.annotation.RestController;
|
||||||
|
|
||||||
import io.swagger.v3.oas.annotations.Operation;
|
|
||||||
|
|
||||||
import lombok.RequiredArgsConstructor;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Test-only endpoint to retrieve a password reset token by email.
|
* Test-only endpoint to retrieve a password reset token by email.
|
||||||
* Only active under the "e2e" Spring profile.
|
* Only active under the "e2e" Spring profile.
|
||||||
@@ -24,14 +20,14 @@ import lombok.RequiredArgsConstructor;
|
|||||||
@RequiredArgsConstructor
|
@RequiredArgsConstructor
|
||||||
public class AuthE2EController {
|
public class AuthE2EController {
|
||||||
|
|
||||||
private final PasswordResetTokenRepository tokenRepository;
|
private final PasswordResetTestHelper passwordResetTestHelper;
|
||||||
|
|
||||||
// Hidden from the OpenAPI spec — this endpoint must never appear in the generated api.ts
|
// Hidden from the OpenAPI spec — this endpoint must never appear in the generated api.ts
|
||||||
// even when the e2e profile is active alongside the dev profile during spec generation.
|
// even when the e2e profile is active alongside the dev profile during spec generation.
|
||||||
@Operation(hidden = true)
|
@Operation(hidden = true)
|
||||||
@GetMapping("/reset-token-for-test")
|
@GetMapping("/reset-token-for-test")
|
||||||
public ResponseEntity<String> getResetTokenForTest(@RequestParam String email) {
|
public ResponseEntity<String> getResetTokenForTest(@RequestParam String email) {
|
||||||
return tokenRepository.findLatestActiveTokenByEmail(email, LocalDateTime.now())
|
return passwordResetTestHelper.getResetTokenForTest(email)
|
||||||
.map(ResponseEntity::ok)
|
.map(ResponseEntity::ok)
|
||||||
.orElse(ResponseEntity.notFound().build());
|
.orElse(ResponseEntity.notFound().build());
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ import org.raddatz.familienarchiv.exception.DomainException;
|
|||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
import org.raddatz.familienarchiv.model.AppUser;
|
import org.raddatz.familienarchiv.model.AppUser;
|
||||||
import org.raddatz.familienarchiv.model.PasswordResetToken;
|
import org.raddatz.familienarchiv.model.PasswordResetToken;
|
||||||
import org.raddatz.familienarchiv.repository.AppUserRepository;
|
|
||||||
import org.raddatz.familienarchiv.repository.PasswordResetTokenRepository;
|
import org.raddatz.familienarchiv.repository.PasswordResetTokenRepository;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
import org.springframework.beans.factory.annotation.Value;
|
import org.springframework.beans.factory.annotation.Value;
|
||||||
@@ -30,7 +29,7 @@ import lombok.extern.slf4j.Slf4j;
|
|||||||
@Slf4j
|
@Slf4j
|
||||||
public class PasswordResetService {
|
public class PasswordResetService {
|
||||||
|
|
||||||
private final AppUserRepository userRepository;
|
private final UserService userService;
|
||||||
private final PasswordResetTokenRepository tokenRepository;
|
private final PasswordResetTokenRepository tokenRepository;
|
||||||
private final PasswordEncoder passwordEncoder;
|
private final PasswordEncoder passwordEncoder;
|
||||||
|
|
||||||
@@ -49,7 +48,7 @@ public class PasswordResetService {
|
|||||||
* If no mail sender is configured, logs a warning.
|
* If no mail sender is configured, logs a warning.
|
||||||
*/
|
*/
|
||||||
public void requestReset(String email, String appBaseUrl) {
|
public void requestReset(String email, String appBaseUrl) {
|
||||||
Optional<AppUser> userOpt = userRepository.findByEmail(email);
|
Optional<AppUser> userOpt = userService.findByEmailOptional(email);
|
||||||
if (userOpt.isEmpty()) {
|
if (userOpt.isEmpty()) {
|
||||||
log.debug("Password reset requested for unknown email: {}", email);
|
log.debug("Password reset requested for unknown email: {}", email);
|
||||||
return;
|
return;
|
||||||
@@ -82,12 +81,21 @@ public class PasswordResetService {
|
|||||||
|
|
||||||
AppUser user = resetToken.getUser();
|
AppUser user = resetToken.getUser();
|
||||||
user.setPassword(passwordEncoder.encode(request.getNewPassword()));
|
user.setPassword(passwordEncoder.encode(request.getNewPassword()));
|
||||||
userRepository.save(user);
|
userService.save(user);
|
||||||
|
|
||||||
resetToken.setUsed(true);
|
resetToken.setUsed(true);
|
||||||
tokenRepository.save(resetToken);
|
tokenRepository.save(resetToken);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns the raw token string of the most recent active (unused, unexpired)
|
||||||
|
* reset token for the given email, if any. Used by the e2e helper to drive
|
||||||
|
* automated password-reset flows; production code paths never call this.
|
||||||
|
*/
|
||||||
|
public Optional<String> findLatestActiveTokenForEmail(String email) {
|
||||||
|
return tokenRepository.findLatestActiveTokenByEmail(email, LocalDateTime.now());
|
||||||
|
}
|
||||||
|
|
||||||
/** Nightly cleanup of expired and used tokens. */
|
/** Nightly cleanup of expired and used tokens. */
|
||||||
@Scheduled(cron = "0 0 3 * * *")
|
@Scheduled(cron = "0 0 3 * * *")
|
||||||
@Transactional
|
@Transactional
|
||||||
|
|||||||
@@ -0,0 +1,24 @@
|
|||||||
|
package org.raddatz.familienarchiv.service;
|
||||||
|
|
||||||
|
import lombok.RequiredArgsConstructor;
|
||||||
|
import org.springframework.context.annotation.Profile;
|
||||||
|
import org.springframework.stereotype.Service;
|
||||||
|
|
||||||
|
import java.util.Optional;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* E2E-only thin wrapper around {@link PasswordResetService} that exposes
|
||||||
|
* the latest active reset token for a given email. Loaded only when the
|
||||||
|
* {@code e2e} Spring profile is active so production code paths never see it.
|
||||||
|
*/
|
||||||
|
@Service
|
||||||
|
@Profile("e2e")
|
||||||
|
@RequiredArgsConstructor
|
||||||
|
public class PasswordResetTestHelper {
|
||||||
|
|
||||||
|
private final PasswordResetService passwordResetService;
|
||||||
|
|
||||||
|
public Optional<String> getResetTokenForTest(String email) {
|
||||||
|
return passwordResetService.findLatestActiveTokenForEmail(email);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -248,6 +248,14 @@ public class UserService {
|
|||||||
.orElseThrow(() -> DomainException.notFound(ErrorCode.USER_NOT_FOUND, "No user found for email: " + email));
|
.orElseThrow(() -> DomainException.notFound(ErrorCode.USER_NOT_FOUND, "No user found for email: " + email));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public Optional<AppUser> findByEmailOptional(String email) {
|
||||||
|
return userRepository.findByEmail(email);
|
||||||
|
}
|
||||||
|
|
||||||
|
public AppUser save(AppUser user) {
|
||||||
|
return userRepository.save(user);
|
||||||
|
}
|
||||||
|
|
||||||
public List<AppUser> getAllUsers() {
|
public List<AppUser> getAllUsers() {
|
||||||
return userRepository.findAll();
|
return userRepository.findAll();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -22,7 +22,6 @@ import org.raddatz.familienarchiv.dto.ResetPasswordRequest;
|
|||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.model.AppUser;
|
import org.raddatz.familienarchiv.model.AppUser;
|
||||||
import org.raddatz.familienarchiv.model.PasswordResetToken;
|
import org.raddatz.familienarchiv.model.PasswordResetToken;
|
||||||
import org.raddatz.familienarchiv.repository.AppUserRepository;
|
|
||||||
import org.raddatz.familienarchiv.repository.PasswordResetTokenRepository;
|
import org.raddatz.familienarchiv.repository.PasswordResetTokenRepository;
|
||||||
import org.springframework.mail.MailSendException;
|
import org.springframework.mail.MailSendException;
|
||||||
import org.springframework.mail.SimpleMailMessage;
|
import org.springframework.mail.SimpleMailMessage;
|
||||||
@@ -33,7 +32,7 @@ import org.springframework.test.util.ReflectionTestUtils;
|
|||||||
@ExtendWith(MockitoExtension.class)
|
@ExtendWith(MockitoExtension.class)
|
||||||
class PasswordResetServiceTest {
|
class PasswordResetServiceTest {
|
||||||
|
|
||||||
@Mock AppUserRepository userRepository;
|
@Mock UserService userService;
|
||||||
@Mock PasswordResetTokenRepository tokenRepository;
|
@Mock PasswordResetTokenRepository tokenRepository;
|
||||||
@Mock PasswordEncoder passwordEncoder;
|
@Mock PasswordEncoder passwordEncoder;
|
||||||
@Mock JavaMailSender mailSender;
|
@Mock JavaMailSender mailSender;
|
||||||
@@ -53,7 +52,7 @@ class PasswordResetServiceTest {
|
|||||||
@Test
|
@Test
|
||||||
void requestReset_savesTokenForKnownEmail() {
|
void requestReset_savesTokenForKnownEmail() {
|
||||||
AppUser user = makeUser("user@example.com");
|
AppUser user = makeUser("user@example.com");
|
||||||
when(userRepository.findByEmail("user@example.com")).thenReturn(Optional.of(user));
|
when(userService.findByEmailOptional("user@example.com")).thenReturn(Optional.of(user));
|
||||||
|
|
||||||
service.requestReset("user@example.com", "http://localhost:3000");
|
service.requestReset("user@example.com", "http://localhost:3000");
|
||||||
|
|
||||||
@@ -65,7 +64,7 @@ class PasswordResetServiceTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
void requestReset_doesNothingForUnknownEmail() {
|
void requestReset_doesNothingForUnknownEmail() {
|
||||||
when(userRepository.findByEmail("ghost@example.com")).thenReturn(Optional.empty());
|
when(userService.findByEmailOptional("ghost@example.com")).thenReturn(Optional.empty());
|
||||||
|
|
||||||
service.requestReset("ghost@example.com", "http://localhost:3000");
|
service.requestReset("ghost@example.com", "http://localhost:3000");
|
||||||
|
|
||||||
@@ -93,7 +92,7 @@ class PasswordResetServiceTest {
|
|||||||
service.resetPassword(req);
|
service.resetPassword(req);
|
||||||
|
|
||||||
verify(passwordEncoder).encode("newpass");
|
verify(passwordEncoder).encode("newpass");
|
||||||
verify(userRepository).save(argThat(u -> u.getPassword().equals("hashed-newpass")));
|
verify(userService).save(argThat(u -> u.getPassword().equals("hashed-newpass")));
|
||||||
assertThat(token.isUsed()).isTrue();
|
assertThat(token.isUsed()).isTrue();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -153,7 +152,7 @@ class PasswordResetServiceTest {
|
|||||||
void requestReset_skipsEmail_whenMailSenderIsNull() {
|
void requestReset_skipsEmail_whenMailSenderIsNull() {
|
||||||
ReflectionTestUtils.setField(service, "mailSender", null);
|
ReflectionTestUtils.setField(service, "mailSender", null);
|
||||||
AppUser user = makeUser("user@example.com");
|
AppUser user = makeUser("user@example.com");
|
||||||
when(userRepository.findByEmail("user@example.com")).thenReturn(Optional.of(user));
|
when(userService.findByEmailOptional("user@example.com")).thenReturn(Optional.of(user));
|
||||||
|
|
||||||
// Must not throw even without mail sender
|
// Must not throw even without mail sender
|
||||||
service.requestReset("user@example.com", "http://localhost:3000");
|
service.requestReset("user@example.com", "http://localhost:3000");
|
||||||
@@ -167,7 +166,7 @@ class PasswordResetServiceTest {
|
|||||||
// mailSender is @Autowired(required=false) — not in constructor, so needs explicit injection
|
// mailSender is @Autowired(required=false) — not in constructor, so needs explicit injection
|
||||||
ReflectionTestUtils.setField(service, "mailSender", mailSender);
|
ReflectionTestUtils.setField(service, "mailSender", mailSender);
|
||||||
AppUser user = makeUser("user@example.com");
|
AppUser user = makeUser("user@example.com");
|
||||||
when(userRepository.findByEmail("user@example.com")).thenReturn(Optional.of(user));
|
when(userService.findByEmailOptional("user@example.com")).thenReturn(Optional.of(user));
|
||||||
doThrow(new MailSendException("SMTP error")).when(mailSender).send(any(SimpleMailMessage.class));
|
doThrow(new MailSendException("SMTP error")).when(mailSender).send(any(SimpleMailMessage.class));
|
||||||
|
|
||||||
// Must not propagate the MailException
|
// Must not propagate the MailException
|
||||||
|
|||||||
@@ -0,0 +1,35 @@
|
|||||||
|
package org.raddatz.familienarchiv.service;
|
||||||
|
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.junit.jupiter.api.extension.ExtendWith;
|
||||||
|
import org.mockito.InjectMocks;
|
||||||
|
import org.mockito.Mock;
|
||||||
|
import org.mockito.junit.jupiter.MockitoExtension;
|
||||||
|
|
||||||
|
import java.util.Optional;
|
||||||
|
|
||||||
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
import static org.mockito.Mockito.when;
|
||||||
|
|
||||||
|
@ExtendWith(MockitoExtension.class)
|
||||||
|
class PasswordResetTestHelperTest {
|
||||||
|
|
||||||
|
@Mock PasswordResetService passwordResetService;
|
||||||
|
@InjectMocks PasswordResetTestHelper helper;
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void getResetTokenForTest_returnsToken_whenPresent() {
|
||||||
|
when(passwordResetService.findLatestActiveTokenForEmail("user@example.com"))
|
||||||
|
.thenReturn(Optional.of("abc123"));
|
||||||
|
|
||||||
|
assertThat(helper.getResetTokenForTest("user@example.com")).contains("abc123");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void getResetTokenForTest_returnsEmpty_whenAbsent() {
|
||||||
|
when(passwordResetService.findLatestActiveTokenForEmail("ghost@example.com"))
|
||||||
|
.thenReturn(Optional.empty());
|
||||||
|
|
||||||
|
assertThat(helper.getResetTokenForTest("ghost@example.com")).isEmpty();
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -58,6 +58,34 @@ class UserServiceTest {
|
|||||||
assertThat(userService.findByEmail("admin@example.com")).isEqualTo(user);
|
assertThat(userService.findByEmail("admin@example.com")).isEqualTo(user);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── findByEmailOptional ──────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void findByEmailOptional_returnsEmpty_whenMissing() {
|
||||||
|
when(userRepository.findByEmail("ghost@example.com")).thenReturn(Optional.empty());
|
||||||
|
|
||||||
|
assertThat(userService.findByEmailOptional("ghost@example.com")).isEmpty();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void findByEmailOptional_returnsUser_whenFound() {
|
||||||
|
AppUser user = AppUser.builder().id(UUID.randomUUID()).email("admin@example.com").build();
|
||||||
|
when(userRepository.findByEmail("admin@example.com")).thenReturn(Optional.of(user));
|
||||||
|
|
||||||
|
assertThat(userService.findByEmailOptional("admin@example.com")).contains(user);
|
||||||
|
}
|
||||||
|
|
||||||
|
// ─── save ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void save_delegatesToRepository() {
|
||||||
|
AppUser user = AppUser.builder().id(UUID.randomUUID()).email("u@example.com").build();
|
||||||
|
when(userRepository.save(user)).thenReturn(user);
|
||||||
|
|
||||||
|
assertThat(userService.save(user)).isEqualTo(user);
|
||||||
|
verify(userRepository).save(user);
|
||||||
|
}
|
||||||
|
|
||||||
// ─── deleteUser ───────────────────────────────────────────────────────────
|
// ─── deleteUser ───────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user