diff --git a/readme.md b/readme.md index 08a3958..9710adb 100644 --- a/readme.md +++ b/readme.md @@ -16,6 +16,10 @@ To Be Played is split into three repositories: - Registration by email confirmation (a single-use link, valid 24 hours, on whose page the owner chooses the password; unconfirmed requests are deleted after 7 days) and then admin approval, login with email and password (BCrypt). Emails are stored trimmed and in lower case (V19, unique on `lower(email)`); a password has 8 characters and at most 72 bytes (BCrypt's limit). +- **Password reset**: "forgot password" mails a link (`/reset-password#`, single use, valid 1 hour, only the + SHA-256 stored, a new mail replaces the old link). Only an ACTIVE user gets a working link; every other address gets + the same `204` and no mail. A reset ends all logins; "change password" (current password + new one) ends all logins + but the current one. A PLATFORM_ADMIN can send the same mail to an active user. Both send a "password changed" mail. - Brute-force protection in memory: 5 failed logins per email or 20 per IP, and 10 failed or 3 taken registrations per IP, block for 15 minutes with `429 TOO_MANY_ATTEMPTS` and `Retry-After`. Unknown emails cost the same BCrypt time as a wrong password, and every failed login gets the same error. @@ -85,7 +89,7 @@ To Be Played is split into three repositories: the deploy get the page. - **Mails** from the platform mailbox: "a new registration waits" to the admin (`APP_MAIL_ADMIN_TO`, with the link to the admin page), and to the user "confirm your address" (`CONFIRM_EMAIL`, sent first; the admin mail follows the click), then "your account is active" or "your - registration was denied", later "your account is blocked" and "your account will be deleted from ". The registration sends the browser language (`de` for a German browser, else `en`), which also becomes + registration was denied", later "your account is blocked" and "your account will be deleted from ", and "reset your password" (`PASSWORD_RESET`, the link) and "your password was changed" (`PASSWORD_CHANGED`, after a reset or a change). The registration sends the browser language (`de` for a German browser, else `en`), which also becomes the user's app language. Plain text only. See "Mail flow". - **Broadcast mail**: a PLATFORM_ADMIN writes one mail with subject and text in English and German (plain text), and sends it to all active users or to picked ones, each in their app language. Every active user gets it. @@ -108,6 +112,9 @@ To Be Played is split into three repositories: | `POST` | `/api/registration/confirm` (`{token, password}`; `204`; `410 CONFIRMATION_INVALID`) | public | | `POST` | `/api/registration/resend` (`{email}`; always `204`) | public | | `POST` | `/api/auth/login` | public, sets both cookies | +| `POST` | `/api/auth/password-reset` (`{email}`; always `204`; `429 TOO_MANY_ATTEMPTS`: 5 per IP and 15 min) | public | +| `POST` | `/api/auth/password-reset/confirm` (`{token, password}`; `204`; `410 RESET_INVALID`; `429`) | public | +| `POST` | `/api/auth/password` (`{currentPassword, newPassword}`; `204`; `403 INVALID_PASSWORD`; `429`; under `/api/auth` for the refresh cookie, which keeps the current login) | access token | | `POST` | `/api/auth/refresh` | refresh cookie, replaces both cookies | | `POST` | `/api/auth/logout` | refresh cookie, clears both cookies | | `GET` | `/api/users/me` | access token | @@ -121,6 +128,7 @@ To Be Played is split into three repositories: | `PUT` | `/api/admin/users/{id}/blocked` (`{blocked}`; `409 SELF_ACTION`, `TARGET_IS_ADMIN`, `ALREADY_DELETED`, `TARGET_IS_PENDING`) | PLATFORM_ADMIN | | `DELETE` | `/api/admin/users/{id}` (`409 SELF_ACTION`, `TARGET_IS_ADMIN`, `TARGET_IS_PENDING`, `TARGET_IS_UNCONFIRMED`) | PLATFORM_ADMIN | | `DELETE` | `/api/admin/users/{id}/unconfirmed` (hard delete of an unconfirmed request; `204`; `404`; `409 NOT_UNCONFIRMED`, `SELF_ACTION`, `TARGET_IS_ADMIN`) | PLATFORM_ADMIN | +| `POST` | `/api/admin/users/{id}/password-reset` (`204`; `409 NOT_ACTIVE`, `SELF_ACTION`, `TARGET_IS_ADMIN`) | PLATFORM_ADMIN | | `GET` | `/api/hints` (`{results}`: running hints not closed for good, newest first) | access token | | `PUT` | `/api/hints/{id}/done` (`204`; `404 HINT_NOT_FOUND`) | access token | | `GET` | `/api/admin/hints` (`{results}` with `status` PLANNED/ACTIVE/ENDED by the server clock and `doneCount`, newest first) | PLATFORM_ADMIN | @@ -339,6 +347,28 @@ sequenceDiagram end ``` +### Password reset flow + +The link is valid for 1 hour and works once: the reset is one `UPDATE ... WHERE password_reset_hash = ? AND expires > now +AND ` that also clears the hash. At most 3 reset mails per address and hour (over all IPs) and 5 requests per IP +per 15 minutes; failed confirms are limited to 10 per IP. Other logins keep their access token until it expires (up to +15 minutes); their refresh tokens end at once. + +```mermaid +sequenceDiagram + participant U as User + participant A as Auth + participant D as Admin + U->>A: POST /api/auth/password-reset {email} + A->>A: active user: store token hash + expiry, one mail row + A-->>U: 204 (same for any address) + A-->>U: mail "reset your password" (link with token) + U->>A: POST /api/auth/password-reset/confirm {token, password} + A->>A: new password, hash cleared, all refresh tokens revoked + A-->>U: 204 and mail "your password was changed" + Note over D,A: POST /api/admin/users/{id}/password-reset sends the same mail (active users only) and writes the audit log +``` + ### Steam ID and block sync to Core Core keeps its own copy of the Steam ID and of the block. After a verified Steam link (also for the account that diff --git a/src/main/java/auth_service/configuration/security/SecurityConfig.java b/src/main/java/auth_service/configuration/security/SecurityConfig.java index 437a0d3..a5ab7a3 100644 --- a/src/main/java/auth_service/configuration/security/SecurityConfig.java +++ b/src/main/java/auth_service/configuration/security/SecurityConfig.java @@ -97,6 +97,8 @@ public class SecurityConfig { // 401, not Spring's default 403: the frontend renews the access cookie only on a 401. .exceptionHandling(e -> e.authenticationEntryPoint(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED))) .authorizeHttpRequests(auth -> auth + // Under /api/auth only for the refresh cookie; it changes the password of the logged-in user. + .requestMatchers(HttpMethod.POST, "/api/auth/password").authenticated() .requestMatchers(WHITE_LIST_URLS).permitAll() // The active role: an admin who views as USER gets 403 here. .requestMatchers("/api/admin/**").hasRole("PLATFORM_ADMIN") diff --git a/src/main/java/auth_service/modules/account/api/AccountController.java b/src/main/java/auth_service/modules/account/api/AccountController.java index 0da0cad..34237e4 100644 --- a/src/main/java/auth_service/modules/account/api/AccountController.java +++ b/src/main/java/auth_service/modules/account/api/AccountController.java @@ -1,15 +1,23 @@ package auth_service.modules.account.api; +import auth_service.modules.account.api.dto.request.ChangePasswordRequest; import auth_service.modules.account.api.dto.request.DeleteAccountRequest; +import auth_service.modules.account.api.dto.request.ForgotPasswordRequest; +import auth_service.modules.account.api.dto.request.ResetPasswordRequest; import auth_service.modules.account.service.AccountService; +import auth_service.modules.account.service.PasswordResetService; import auth_service.shared.api.AuthCookies; +import auth_service.shared.api.ClientIp; import auth_service.shared.security.UserPrincipal; +import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseEntity; import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.web.bind.annotation.CookieValue; import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RestController; @@ -18,6 +26,7 @@ import org.springframework.web.bind.annotation.RestController; public class AccountController { private final AccountService accountService; + private final PasswordResetService passwordResetService; private final AuthCookies authCookies; @DeleteMapping(value = "/api/users/me") @@ -28,4 +37,29 @@ public class AccountController { authCookies.cleared().forEach(cookie -> headers.add(HttpHeaders.SET_COOKIE, cookie.toString())); return ResponseEntity.noContent().headers(headers).build(); } + + @PostMapping(value = "/api/auth/password-reset", consumes = "application/json") + public ResponseEntity requestReset(@Valid @RequestBody ForgotPasswordRequest request, + HttpServletRequest httpRequest) { + passwordResetService.requestReset(request.email(), ClientIp.of(httpRequest)); + return ResponseEntity.noContent().build(); + } + + @PostMapping(value = "/api/auth/password-reset/confirm", consumes = "application/json") + public ResponseEntity resetPassword(@Valid @RequestBody ResetPasswordRequest request, + HttpServletRequest httpRequest) { + passwordResetService.resetPassword(request.token(), request.password(), ClientIp.of(httpRequest)); + return ResponseEntity.noContent().build(); + } + + // Under /api/auth only because the refresh cookie has that path; it tells which login stays. + @PostMapping(value = "/api/auth/password", consumes = "application/json") + public ResponseEntity changePassword(@AuthenticationPrincipal UserPrincipal principal, + @Valid @RequestBody ChangePasswordRequest request, + @CookieValue(name = AuthCookies.REFRESH_COOKIE, required = false) + String refreshToken) { + accountService.changePassword(principal.getIdAsLong(), request.currentPassword(), request.newPassword(), + refreshToken); + return ResponseEntity.noContent().build(); + } } diff --git a/src/main/java/auth_service/modules/account/api/AccountExceptionController.java b/src/main/java/auth_service/modules/account/api/AccountExceptionController.java index d2cf347..1cdc3d1 100644 --- a/src/main/java/auth_service/modules/account/api/AccountExceptionController.java +++ b/src/main/java/auth_service/modules/account/api/AccountExceptionController.java @@ -1,7 +1,8 @@ package auth_service.modules.account.api; import auth_service.modules.account.exception.InvalidPasswordException; -import auth_service.modules.account.exception.TooManyDeleteAttemptsException; +import auth_service.modules.account.exception.PasswordResetInvalidException; +import auth_service.modules.account.exception.TooManyAttemptsException; import auth_service.shared.api.dto.response.ErrorResponse; import org.springframework.core.annotation.Order; import org.springframework.http.HttpHeaders; @@ -22,8 +23,14 @@ public class AccountExceptionController { .body(new ErrorResponse("INVALID_PASSWORD", ex.getMessage())); } - @ExceptionHandler(TooManyDeleteAttemptsException.class) - public ResponseEntity clientError(TooManyDeleteAttemptsException ex) { + @ExceptionHandler(PasswordResetInvalidException.class) + public ResponseEntity clientError(PasswordResetInvalidException ex) { + return ResponseEntity.status(HttpStatus.GONE).contentType(MediaType.APPLICATION_JSON) + .body(new ErrorResponse("RESET_INVALID", ex.getMessage())); + } + + @ExceptionHandler(TooManyAttemptsException.class) + public ResponseEntity clientError(TooManyAttemptsException ex) { return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS) .header(HttpHeaders.RETRY_AFTER, String.valueOf(ex.getRetryAfterSeconds())) .contentType(MediaType.APPLICATION_JSON) diff --git a/src/main/java/auth_service/modules/account/api/dto/request/ChangePasswordRequest.java b/src/main/java/auth_service/modules/account/api/dto/request/ChangePasswordRequest.java new file mode 100644 index 0000000..ea0e656 --- /dev/null +++ b/src/main/java/auth_service/modules/account/api/dto/request/ChangePasswordRequest.java @@ -0,0 +1,25 @@ +package auth_service.modules.account.api.dto.request; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import jakarta.validation.constraints.AssertTrue; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; + +import java.nio.charset.StandardCharsets; + +public record ChangePasswordRequest(@NotBlank String currentPassword, @NotBlank @Size(min = 8) String newPassword) { + + /** + * BCrypt uses only the first 72 bytes and refuses a longer password. + */ + @AssertTrue(message = "The password may have at most 72 bytes") + @JsonIgnore + public boolean isPasswordShortEnough() { + return newPassword == null || newPassword.getBytes(StandardCharsets.UTF_8).length <= 72; + } + + @Override + public String toString() { + return "ChangePasswordRequest[]"; + } +} diff --git a/src/main/java/auth_service/modules/account/api/dto/request/ForgotPasswordRequest.java b/src/main/java/auth_service/modules/account/api/dto/request/ForgotPasswordRequest.java new file mode 100644 index 0000000..2bbf529 --- /dev/null +++ b/src/main/java/auth_service/modules/account/api/dto/request/ForgotPasswordRequest.java @@ -0,0 +1,13 @@ +package auth_service.modules.account.api.dto.request; + +import auth_service.shared.account.EmailAddress; +import jakarta.validation.constraints.Email; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; + +public record ForgotPasswordRequest(@NotBlank @Email @Size(max = EmailAddress.MAX_LENGTH) String email) { + + public ForgotPasswordRequest { + email = EmailAddress.normalize(email); + } +} diff --git a/src/main/java/auth_service/modules/account/api/dto/request/ResetPasswordRequest.java b/src/main/java/auth_service/modules/account/api/dto/request/ResetPasswordRequest.java new file mode 100644 index 0000000..d1647a5 --- /dev/null +++ b/src/main/java/auth_service/modules/account/api/dto/request/ResetPasswordRequest.java @@ -0,0 +1,25 @@ +package auth_service.modules.account.api.dto.request; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import jakarta.validation.constraints.AssertTrue; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; + +import java.nio.charset.StandardCharsets; + +public record ResetPasswordRequest(@NotBlank @Size(max = 64) String token, @NotBlank @Size(min = 8) String password) { + + /** + * BCrypt uses only the first 72 bytes and refuses a longer password. + */ + @AssertTrue(message = "The password may have at most 72 bytes") + @JsonIgnore + public boolean isPasswordShortEnough() { + return password == null || password.getBytes(StandardCharsets.UTF_8).length <= 72; + } + + @Override + public String toString() { + return "ResetPasswordRequest[]"; + } +} diff --git a/src/main/java/auth_service/modules/account/exception/PasswordResetInvalidException.java b/src/main/java/auth_service/modules/account/exception/PasswordResetInvalidException.java new file mode 100644 index 0000000..7f8114a --- /dev/null +++ b/src/main/java/auth_service/modules/account/exception/PasswordResetInvalidException.java @@ -0,0 +1,7 @@ +package auth_service.modules.account.exception; + +public class PasswordResetInvalidException extends RuntimeException { + public PasswordResetInvalidException() { + super("The reset link is unknown, used or expired."); + } +} diff --git a/src/main/java/auth_service/modules/account/exception/TooManyAttemptsException.java b/src/main/java/auth_service/modules/account/exception/TooManyAttemptsException.java new file mode 100644 index 0000000..e1f308e --- /dev/null +++ b/src/main/java/auth_service/modules/account/exception/TooManyAttemptsException.java @@ -0,0 +1,13 @@ +package auth_service.modules.account.exception; + +import lombok.Getter; + +@Getter +public class TooManyAttemptsException extends RuntimeException { + private final long retryAfterSeconds; + + public TooManyAttemptsException(long retryAfterSeconds) { + super("Too many attempts. Try again later."); + this.retryAfterSeconds = retryAfterSeconds; + } +} diff --git a/src/main/java/auth_service/modules/account/exception/TooManyDeleteAttemptsException.java b/src/main/java/auth_service/modules/account/exception/TooManyDeleteAttemptsException.java deleted file mode 100644 index 0f3a300..0000000 --- a/src/main/java/auth_service/modules/account/exception/TooManyDeleteAttemptsException.java +++ /dev/null @@ -1,13 +0,0 @@ -package auth_service.modules.account.exception; - -import lombok.Getter; - -@Getter -public class TooManyDeleteAttemptsException extends RuntimeException { - private final long retryAfterSeconds; - - public TooManyDeleteAttemptsException(long retryAfterSeconds) { - super("Too many wrong passwords. Try again later."); - this.retryAfterSeconds = retryAfterSeconds; - } -} diff --git a/src/main/java/auth_service/modules/account/repo/AccountRepository.java b/src/main/java/auth_service/modules/account/repo/AccountRepository.java index 697923f..3505ae5 100644 --- a/src/main/java/auth_service/modules/account/repo/AccountRepository.java +++ b/src/main/java/auth_service/modules/account/repo/AccountRepository.java @@ -1,6 +1,8 @@ package auth_service.modules.account.repo; import auth_service.modules.account.model.CoreAccountState; +import auth_service.shared.account.ActiveUser; +import auth_service.shared.account.EmailAddress; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; @@ -50,6 +52,54 @@ public class AccountRepository { return flagged; } + public Optional findActiveIdByEmail(String email) { + return jdbc.queryForList("SELECT u.id FROM users u WHERE u.email = ? AND " + ActiveUser.of("u"), Long.class, + EmailAddress.normalize(email)).stream().findFirst(); + } + + /** + * Replaces an older reset link of the user. + * + * @return false when the user is not active + */ + public boolean storePasswordReset(long userId, String tokenHash, Instant expiresAt) { + return jdbc.update(""" + UPDATE users u SET password_reset_hash = ?, password_reset_expires_at = ? + WHERE u.id = ? AND\s""" + ActiveUser.of("u"), tokenHash, Timestamp.from(expiresAt), userId) == 1; + } + + /** + * Sets the password and uses up the link in one statement, so a link works once. + * + * @return the id of the user, empty when the link is unknown, used, expired or the user is not active + */ + public Optional resetPassword(String tokenHash, String passwordHash, Instant now) { + return jdbc.queryForList(""" + UPDATE users u SET password = ?, password_reset_hash = NULL, password_reset_expires_at = NULL + WHERE u.password_reset_hash = ? AND u.password_reset_expires_at > ? AND\s""" + ActiveUser.of("u") + + " RETURNING u.id", Long.class, passwordHash, tokenHash, Timestamp.from(now)).stream().findFirst(); + } + + /** + * Also ends an open reset link. + */ + public void updatePassword(long userId, String passwordHash) { + jdbc.update(""" + UPDATE users SET password = ?, password_reset_hash = NULL, password_reset_expires_at = NULL + WHERE id = ?""", passwordHash, userId); + } + + /** + * Ends every login of the user but the one of keepTokenHash; null ends all. IS DISTINCT FROM, because for an + * unknown hash the subquery is NULL and a plain comparison would revoke nothing. + */ + public int revokeSessions(long userId, String keepTokenHash, Instant now) { + return jdbc.update(""" + UPDATE refresh_tokens SET revoked_at = ? WHERE user_id = ? AND revoked_at IS NULL + AND family_id IS DISTINCT FROM (SELECT family_id FROM refresh_tokens WHERE token_hash = ? AND user_id = ?)""", + Timestamp.from(now), userId, keepTokenHash, userId); + } + public List findFlaggedIds() { return jdbc.queryForList("SELECT id FROM users WHERE deleted_at IS NOT NULL ORDER BY id", Long.class); } diff --git a/src/main/java/auth_service/modules/account/service/AccountService.java b/src/main/java/auth_service/modules/account/service/AccountService.java index a383437..d08db21 100644 --- a/src/main/java/auth_service/modules/account/service/AccountService.java +++ b/src/main/java/auth_service/modules/account/service/AccountService.java @@ -1,11 +1,12 @@ package auth_service.modules.account.service; import auth_service.modules.account.exception.InvalidPasswordException; -import auth_service.modules.account.exception.TooManyDeleteAttemptsException; +import auth_service.modules.account.exception.TooManyAttemptsException; import auth_service.modules.account.repo.AccountRepository; import auth_service.shared.account.AccountDeleter; import auth_service.shared.mail.AccountMailer; import auth_service.shared.security.AttemptLimiter; +import auth_service.shared.security.OpaqueToken; import auth_service.shared.security.RequiresRole; import auth_service.shared.security.Role; import lombok.extern.slf4j.Slf4j; @@ -59,23 +60,34 @@ public class AccountService implements AccountDeleter { @Caching(evict = {@CacheEvict(value = {"users", "activeUsers"}, key = "#userId"), @CacheEvict(value = "publicStats", allEntries = true)}) public void deleteAccount(long userId, String password) { - String key = String.valueOf(userId); - if (!attemptLimiter.tryAcquire(key, MAX_WRONG_PASSWORDS)) { - throw new TooManyDeleteAttemptsException(LOCK.toSeconds()); - } + String key = acquireAttempt(userId); Optional hash = accountRepository.findPasswordHashOfActive(userId); if (hash.isEmpty()) { // Already flagged, e.g. a second click: the answer is the same. attemptLimiter.release(key); return; } - if (!passwordEncoder.matches(password, hash.get())) { - throw new InvalidPasswordException(); - } - attemptLimiter.reset(key); + checkPassword(key, password, hash.get()); flagAndTellCore(userId); } + /** + * Keeps the login of refreshToken (may be null: then all logins end) and ends every other one. + */ + public void changePassword(long userId, String currentPassword, String newPassword, String refreshToken) { + String key = acquireAttempt(userId); + checkPassword(key, currentPassword, + accountRepository.findPasswordHashOfActive(userId).orElseThrow(InvalidPasswordException::new)); + String hash = passwordEncoder.encode(newPassword); + transactions.executeWithoutResult(status -> { + accountRepository.updatePassword(userId, hash); + accountRepository.revokeSessions(userId, refreshToken == null ? null : OpaqueToken.hash(refreshToken), + Instant.now()); + accountMailer.passwordChanged(userId); + }); + log.info("Password of user {} changed", userId); + } + @Override @RequiresRole(Role.PLATFORM_ADMIN) @Caching(evict = {@CacheEvict(value = {"users", "activeUsers"}, key = "#userId"), @@ -84,6 +96,22 @@ public class AccountService implements AccountDeleter { return flagAndTellCore(userId); } + // Delete account and change password share the limit: both guess the same password. + private String acquireAttempt(long userId) { + String key = String.valueOf(userId); + if (!attemptLimiter.tryAcquire(key, MAX_WRONG_PASSWORDS)) { + throw new TooManyAttemptsException(LOCK.toSeconds()); + } + return key; + } + + private void checkPassword(String key, String password, String hash) { + if (!passwordEncoder.matches(password, hash)) { + throw new InvalidPasswordException(); + } + attemptLimiter.reset(key); + } + // The flag and the mail commit together, and before Core is called. private boolean flagAndTellCore(long userId) { Instant now = Instant.now(); diff --git a/src/main/java/auth_service/modules/account/service/PasswordResetService.java b/src/main/java/auth_service/modules/account/service/PasswordResetService.java new file mode 100644 index 0000000..ab0f4b1 --- /dev/null +++ b/src/main/java/auth_service/modules/account/service/PasswordResetService.java @@ -0,0 +1,92 @@ +package auth_service.modules.account.service; + +import auth_service.modules.account.exception.PasswordResetInvalidException; +import auth_service.modules.account.exception.TooManyAttemptsException; +import auth_service.modules.account.repo.AccountRepository; +import auth_service.shared.account.PasswordResetSender; +import auth_service.shared.mail.AccountMailer; +import auth_service.shared.security.AttemptLimiter; +import auth_service.shared.security.OpaqueToken; +import auth_service.shared.security.Public; +import auth_service.shared.security.RequiresRole; +import auth_service.shared.security.Role; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.time.Duration; +import java.time.Instant; + +/** + * Forgot password: a mail with a link that sets a new password once. Only active users get a working link. The raw + * token and the address are never logged. + */ +@Service +@Public +@Slf4j +@RequiredArgsConstructor +public class PasswordResetService implements PasswordResetSender { + + static final Duration RESET_TTL = Duration.ofHours(1); + static final Duration WINDOW = Duration.ofMinutes(15); + static final int MAX_REQUESTS_PER_IP = 5; + static final int MAX_FAILURES_PER_IP = 10; + static final int MAX_MAILS_PER_ADDRESS = 3; + static final Duration MAIL_WINDOW = Duration.ofHours(1); + + private final AccountRepository accountRepository; + private final PasswordEncoder passwordEncoder; + private final AccountMailer accountMailer; + private final AttemptLimiter requests = new AttemptLimiter(WINDOW); + private final AttemptLimiter failures = new AttemptLimiter(WINDOW); + private final AttemptLimiter mails = new AttemptLimiter(MAIL_WINDOW); + + /** + * The same answer for a known and an unknown address. Over the mail limit of an address nothing is sent, also + * silently. + */ + @Transactional + public void requestReset(String email, String clientIp) { + if (!requests.tryAcquire(clientIp, MAX_REQUESTS_PER_IP)) { + throw new TooManyAttemptsException(WINDOW.toSeconds()); + } + if (!mails.tryAcquire(email, MAX_MAILS_PER_ADDRESS)) { + return; + } + accountRepository.findActiveIdByEmail(email).ifPresent(this::sendResetMail); + } + + /** + * Sets the password and ends every login of the user. + */ + @Transactional + public void resetPassword(String rawToken, String newPassword, String clientIp) { + if (!failures.tryAcquire(clientIp, MAX_FAILURES_PER_IP)) { + throw new TooManyAttemptsException(WINDOW.toSeconds()); + } + long userId = accountRepository.resetPassword(OpaqueToken.hash(rawToken), passwordEncoder.encode(newPassword), + Instant.now()).orElseThrow(PasswordResetInvalidException::new); + failures.release(clientIp); + accountRepository.revokeSessions(userId, null, Instant.now()); + accountMailer.passwordChanged(userId); + log.info("Password of user {} reset by mail link", userId); + } + + @Override + @RequiresRole(Role.PLATFORM_ADMIN) + @Transactional + public boolean sendByAdmin(long userId) { + return sendResetMail(userId); + } + + private boolean sendResetMail(long userId) { + String token = OpaqueToken.generate(); + if (!accountRepository.storePasswordReset(userId, OpaqueToken.hash(token), Instant.now().plus(RESET_TTL))) { + return false; + } + accountMailer.passwordReset(userId, token); + return true; + } +} diff --git a/src/main/java/auth_service/modules/admin/api/AdminUserController.java b/src/main/java/auth_service/modules/admin/api/AdminUserController.java index 602fdd1..d972857 100644 --- a/src/main/java/auth_service/modules/admin/api/AdminUserController.java +++ b/src/main/java/auth_service/modules/admin/api/AdminUserController.java @@ -16,6 +16,7 @@ import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; 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; @@ -71,6 +72,13 @@ public class AdminUserController { return ResponseEntity.noContent().build(); } + @PostMapping("/{userId}/password-reset") + public ResponseEntity sendPasswordReset(@AuthenticationPrincipal UserPrincipal principal, + @PathVariable long userId) { + adminUserService.sendPasswordReset(principal.getIdAsLong(), userId); + return ResponseEntity.noContent().build(); + } + private static AdminUserResponse toApi(AdminUser user) { return new AdminUserResponse(user.id(), user.email(), user.displayName(), user.role().name(), user.registrationDate(), user.lastLoginDate(), user.steamLinked(), user.blocked(), user.deletedAt(), diff --git a/src/main/java/auth_service/modules/admin/exception/AdminActionRefusedException.java b/src/main/java/auth_service/modules/admin/exception/AdminActionRefusedException.java index b8ce1c1..9bd01d1 100644 --- a/src/main/java/auth_service/modules/admin/exception/AdminActionRefusedException.java +++ b/src/main/java/auth_service/modules/admin/exception/AdminActionRefusedException.java @@ -5,7 +5,7 @@ import lombok.Getter; @Getter public class AdminActionRefusedException extends RuntimeException { /** - * SELF_ACTION, TARGET_IS_ADMIN, ALREADY_DELETED, NOT_PENDING, TARGET_IS_PENDING, TARGET_IS_UNCONFIRMED or NOT_UNCONFIRMED + * SELF_ACTION, TARGET_IS_ADMIN, ALREADY_DELETED, NOT_PENDING, TARGET_IS_PENDING, TARGET_IS_UNCONFIRMED, NOT_UNCONFIRMED or NOT_ACTIVE */ private final String code; diff --git a/src/main/java/auth_service/modules/admin/service/AdminUserService.java b/src/main/java/auth_service/modules/admin/service/AdminUserService.java index 8ed8ef8..1b6001a 100644 --- a/src/main/java/auth_service/modules/admin/service/AdminUserService.java +++ b/src/main/java/auth_service/modules/admin/service/AdminUserService.java @@ -7,6 +7,7 @@ import auth_service.modules.admin.model.TargetState; import auth_service.modules.admin.repo.AdminUserRepository; import auth_service.shared.account.AccountDeleter; import auth_service.shared.account.AccountStatePublisher; +import auth_service.shared.account.PasswordResetSender; import auth_service.shared.audit.AdminAuditLog; import auth_service.shared.mail.AccountMailer; import auth_service.shared.security.RequiresRole; @@ -33,6 +34,7 @@ public class AdminUserService { private final AccountDeleter accountDeleter; private final AccountStatePublisher accountStatePublisher; private final AccountMailer accountMailer; + private final PasswordResetSender passwordResetSender; public AdminUserPage findUsers(String query, String status, int page, int size) { String search = query == null ? "" : query.trim(); @@ -111,6 +113,19 @@ public class AdminUserService { log.info("Admin {} deleted the unconfirmed registration of user {}", actorUserId, targetUserId); } + /** + * The same mail as "Forgot password". The admin never sees or sets a password. + */ + @Transactional + public void sendPasswordReset(long actorUserId, long targetUserId) { + checkedTarget(actorUserId, targetUserId); + if (!passwordResetSender.sendByAdmin(targetUserId)) { + throw new AdminActionRefusedException("NOT_ACTIVE", "Only an active account can get a reset mail"); + } + adminAuditLog.record(actorUserId, "SEND_PASSWORD_RESET", targetUserId, null); + log.info("Admin {} sent a password reset to user {}", actorUserId, targetUserId); + } + private TargetState checkedTarget(long actorUserId, long targetUserId) { if (actorUserId == targetUserId) { throw new AdminActionRefusedException("SELF_ACTION", "Admins cannot block or delete their own account"); diff --git a/src/main/java/auth_service/modules/login/persistence/MinimalUserRepository.java b/src/main/java/auth_service/modules/login/persistence/MinimalUserRepository.java index dddb6d6..e21f990 100644 --- a/src/main/java/auth_service/modules/login/persistence/MinimalUserRepository.java +++ b/src/main/java/auth_service/modules/login/persistence/MinimalUserRepository.java @@ -18,5 +18,11 @@ public interface MinimalUserRepository { Optional findById(long userId); + /** + * Reads the password hash and holds a lock on the row until the transaction ends, so a password change commits + * before or after the caller's writes, never between. + */ + Optional lockPasswordHash(long userId); + void updateLoginDate(long userId); } diff --git a/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java b/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java index 53130ce..98dd711 100644 --- a/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java +++ b/src/main/java/auth_service/modules/login/persistence/MinimalUserRepositoryImpl.java @@ -43,6 +43,12 @@ public class MinimalUserRepositoryImpl implements MinimalUserRepository { return jdbc.query(SELECT_USER + "WHERE id = ?", USER_ROW, userId).stream().findFirst(); } + @Override + public Optional lockPasswordHash(long userId) { + return jdbc.query("SELECT password FROM users WHERE id = ? FOR SHARE", (rs, row) -> rs.getString("password"), + userId).stream().findFirst(); + } + @Override public void updateLoginDate(long userId) { jdbc.update("UPDATE users SET last_login_date = ? WHERE id = ?", LocalDateTime.now(Clock.systemUTC()), userId); diff --git a/src/main/java/auth_service/modules/login/persistence/RefreshTokenRepository.java b/src/main/java/auth_service/modules/login/persistence/RefreshTokenRepository.java index 0b751e2..0e99d16 100644 --- a/src/main/java/auth_service/modules/login/persistence/RefreshTokenRepository.java +++ b/src/main/java/auth_service/modules/login/persistence/RefreshTokenRepository.java @@ -14,6 +14,11 @@ public interface RefreshTokenRepository { */ void save(long userId, UUID familyId, String tokenHash, Instant createdAt, Instant expiresAt, Role activeRole); + /** + * Without a lock: the caller locks the user first, as a password change does, so the two cannot deadlock. + */ + Optional findUserIdByHash(String tokenHash); + /** * Locks the row until the transaction ends, so two refreshes with the same token run one after the other. */ diff --git a/src/main/java/auth_service/modules/login/persistence/RefreshTokenRepositoryImpl.java b/src/main/java/auth_service/modules/login/persistence/RefreshTokenRepositoryImpl.java index 6e6c1b4..2e36a09 100644 --- a/src/main/java/auth_service/modules/login/persistence/RefreshTokenRepositoryImpl.java +++ b/src/main/java/auth_service/modules/login/persistence/RefreshTokenRepositoryImpl.java @@ -31,6 +31,15 @@ public class RefreshTokenRepositoryImpl implements RefreshTokenRepository { entityManager.persist(entity); } + @Override + public Optional findUserIdByHash(String tokenHash) { + return entityManager.createQuery("SELECT t.userId FROM RefreshTokenEntity t WHERE t.tokenHash = :hash", Long.class) + .setParameter("hash", tokenHash) + .getResultList() + .stream() + .findFirst(); + } + @Override public Optional findByHashForUpdate(String tokenHash) { return entityManager.createQuery( diff --git a/src/main/java/auth_service/modules/login/service/LoginService.java b/src/main/java/auth_service/modules/login/service/LoginService.java index cfe0fa2..ce3f4bc 100644 --- a/src/main/java/auth_service/modules/login/service/LoginService.java +++ b/src/main/java/auth_service/modules/login/service/LoginService.java @@ -83,6 +83,6 @@ public class LoginService { } refreshTokenService.revoke(previousRefreshToken); String accessToken = tokenGenerator.generateToken(String.valueOf(user.getId()), user.getSteamId(), user.getRole()); - return new Session(accessToken, refreshTokenService.startFamily(user.getId())); + return new Session(accessToken, refreshTokenService.startFamily(user.getId(), hash)); } } diff --git a/src/main/java/auth_service/modules/login/service/RefreshTokenService.java b/src/main/java/auth_service/modules/login/service/RefreshTokenService.java index 131674f..de3e0bf 100644 --- a/src/main/java/auth_service/modules/login/service/RefreshTokenService.java +++ b/src/main/java/auth_service/modules/login/service/RefreshTokenService.java @@ -1,5 +1,6 @@ package auth_service.modules.login.service; +import auth_service.modules.login.exception.InvalidCredentialsException; import auth_service.modules.login.model.RefreshToken; import auth_service.modules.login.model.Session; import auth_service.modules.login.model.User; @@ -46,8 +47,16 @@ public class RefreshTokenService { private final SessionProperties sessionProperties; private final AdminAuditLog adminAuditLog; + /** + * @param passwordHash the hash the login checked. A password change in between ended every login, so this one + * must not start. + * @throws InvalidCredentialsException when the password is not the one checked + */ @Transactional - public String startFamily(long userId) { + public String startFamily(long userId, String passwordHash) { + if (!userRepository.lockPasswordHash(userId).map(current -> current.equals(passwordHash)).orElse(false)) { + throw new InvalidCredentialsException("Invalid email or password"); + } return issue(userId, UUID.randomUUID(), Instant.now(), null); } @@ -79,6 +88,12 @@ public class RefreshTokenService { return Optional.empty(); } Instant now = Instant.now(); + // The user lock comes before the token lock, as in a password change. A change that commits after this + // point sees the new token when it revokes; one that committed before has revoked this token already. + Optional owner = refreshTokenRepository.findUserIdByHash(hash(rawToken)); + if (owner.isEmpty() || userRepository.lockPasswordHash(owner.get()).isEmpty()) { + return Optional.empty(); + } Optional found = refreshTokenRepository.findByHashForUpdate(hash(rawToken)); if (found.isEmpty()) { return Optional.empty(); diff --git a/src/main/java/auth_service/modules/mail/model/MailTemplate.java b/src/main/java/auth_service/modules/mail/model/MailTemplate.java index f8f4293..69a126e 100644 --- a/src/main/java/auth_service/modules/mail/model/MailTemplate.java +++ b/src/main/java/auth_service/modules/mail/model/MailTemplate.java @@ -126,6 +126,52 @@ public enum MailTemplate { ToBePlayed """), + // %1$s greeting, %2$s link. + PASSWORD_RESET( + "Reset your ToBePlayed password", """ + Hello %s, + + someone asked to reset the password of your account. Choose a new password with this link (valid for 1 hour, once): + %s + + If it was not you, ignore this mail; your password stays as it is. + + ToBePlayed + """, + "Setze dein ToBePlayed-Passwort zurück", """ + Hallo %s, + + jemand möchte das Passwort deines Kontos zurücksetzen. Wähle über diesen Link ein neues Passwort (1 Stunde gültig, einmal nutzbar): + %s + + Wenn du das nicht warst, ignoriere diese Mail; dein Passwort bleibt, wie es ist. + + ToBePlayed + """), + + // %1$s greeting, %2$s link to the reset page. + PASSWORD_CHANGED( + "Your ToBePlayed password was changed", """ + Hello %s, + + the password of your account was changed, and your other logins were ended. + If it was not you, reset your password at once: + %s + and answer this mail. + + ToBePlayed + """, + "Dein ToBePlayed-Passwort wurde geändert", """ + Hallo %s, + + das Passwort deines Kontos wurde geändert, und deine anderen Anmeldungen wurden beendet. + Wenn du das nicht warst, setze dein Passwort sofort zurück: + %s + und antworte auf diese Mail. + + ToBePlayed + """), + // Admin text only: %1$s subject, %2$s body. BROADCAST( "%1$s", """ diff --git a/src/main/java/auth_service/modules/mail/service/MailService.java b/src/main/java/auth_service/modules/mail/service/MailService.java index 4d9188d..753a25b 100644 --- a/src/main/java/auth_service/modules/mail/service/MailService.java +++ b/src/main/java/auth_service/modules/mail/service/MailService.java @@ -72,6 +72,18 @@ public class MailService implements AccountMailer { user.greeting(), MailTemplate.date(user.language(), deletedFrom))); } + @Override + public void passwordReset(long userId, String rawToken) { + repository.findRecipient(userId).ifPresent(user -> insertForUser(MailTemplate.PASSWORD_RESET, userId, user, + user.greeting(), frontendUrl + "/reset-password#" + rawToken)); + } + + @Override + public void passwordChanged(long userId) { + repository.findRecipient(userId).ifPresent(user -> insertForUser(MailTemplate.PASSWORD_CHANGED, userId, user, + user.greeting(), frontendUrl + "/reset-password")); + } + private void insertForUser(MailTemplate template, long userId, Recipient user, Object... args) { repository.insert(template, userId, user.email(), template.render(user.language(), args)); } diff --git a/src/main/java/auth_service/shared/account/PasswordResetSender.java b/src/main/java/auth_service/shared/account/PasswordResetSender.java new file mode 100644 index 0000000..6d46d30 --- /dev/null +++ b/src/main/java/auth_service/shared/account/PasswordResetSender.java @@ -0,0 +1,9 @@ +package auth_service.shared.account; + +/** + * Port for a reset mail the user did not ask for, e.g. by an admin. False when the user is not active (cannot log + * in); then nothing is sent. + */ +public interface PasswordResetSender { + boolean sendByAdmin(long userId); +} diff --git a/src/main/java/auth_service/shared/mail/AccountMailer.java b/src/main/java/auth_service/shared/mail/AccountMailer.java index 04325c1..d839854 100644 --- a/src/main/java/auth_service/shared/mail/AccountMailer.java +++ b/src/main/java/auth_service/shared/mail/AccountMailer.java @@ -28,4 +28,11 @@ public interface AccountMailer { * @param deletedFrom the first day the nightly cleanup may remove the account */ void accountDeleted(long userId, LocalDate deletedFrom); + + /** + * The link with the raw token; the caller stores only its hash. + */ + void passwordReset(long userId, String rawToken); + + void passwordChanged(long userId); } diff --git a/src/main/resources/db/migration/V29__Password_Reset.sql b/src/main/resources/db/migration/V29__Password_Reset.sql new file mode 100644 index 0000000..f1c7377 --- /dev/null +++ b/src/main/resources/db/migration/V29__Password_Reset.sql @@ -0,0 +1,5 @@ +-- SHA-256 hex of the reset link token, NULL once used; the expiry is 1 h from the last send. A new send replaces it. +ALTER TABLE users ADD COLUMN password_reset_hash VARCHAR(64), + ADD COLUMN password_reset_expires_at TIMESTAMPTZ; +CREATE UNIQUE INDEX uq_users_password_reset_hash ON users (password_reset_hash) + WHERE password_reset_hash IS NOT NULL; diff --git a/src/test/java/auth_service/configuration/security/PublicEndpointsTest.java b/src/test/java/auth_service/configuration/security/PublicEndpointsTest.java index 6f99b0a..43ed177 100644 --- a/src/test/java/auth_service/configuration/security/PublicEndpointsTest.java +++ b/src/test/java/auth_service/configuration/security/PublicEndpointsTest.java @@ -2,6 +2,7 @@ package auth_service.configuration.security; import auth_service.shared.test.AbstractIntegrationTest; import org.junit.jupiter.api.Test; +import org.springframework.http.MediaType; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.web.servlet.MockMvc; @@ -9,6 +10,7 @@ import java.util.Set; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -47,4 +49,20 @@ class PublicEndpointsTest extends AbstractIntegrationTest { .andExpect(status().isOk()) .andExpect(jsonPath("$.commit").isString()); } + + @Test + void changePasswordNeedsALogin() throws Exception { + mockMvc.perform(post("/api/auth/password").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"currentPassword\":\"x\",\"newPassword\":\"12345678\"}")) + .andExpect(status().isUnauthorized()); + } + + @Test + void forgotPasswordIsPublic() throws Exception { + mockMvc.perform(post("/api/auth/password-reset").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .header("X-Forwarded-For", freshClientIp()).contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"public-endpoints-nobody@example.com\"}")) + .andExpect(status().isNoContent()); + } } diff --git a/src/test/java/auth_service/modules/account/api/PasswordResetFlowTest.java b/src/test/java/auth_service/modules/account/api/PasswordResetFlowTest.java new file mode 100644 index 0000000..49be4a4 --- /dev/null +++ b/src/test/java/auth_service/modules/account/api/PasswordResetFlowTest.java @@ -0,0 +1,230 @@ +package auth_service.modules.account.api; + +import auth_service.configuration.security.ClientHeaderFilter; +import auth_service.shared.api.AuthCookies; +import auth_service.shared.test.AbstractIntegrationTest; +import jakarta.servlet.http.Cookie; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; +import org.springframework.http.MediaType; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.ResultActions; + +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * Forgot password, change password and the admin reset against a real database. Each test uses its own address, + * because the mail limit per address lives in memory. + */ +class PasswordResetFlowTest extends AbstractIntegrationTest { + + private static final String PASSWORD = "correct horse battery"; + private static final String NEW_PASSWORD = "brand new password"; + private static final Pattern LINK = Pattern.compile("/reset-password#(\\S+)"); + + @Autowired + private MockMvc mockMvc; + + @Autowired + private JdbcTemplate jdbc; + + @Autowired + private PasswordEncoder passwordEncoder; + + @BeforeEach + void clean() { + jdbc.update("DELETE FROM mail_outbox"); + jdbc.update("DELETE FROM users WHERE email LIKE 'reset-flow-%'"); + } + + private long insertUser(String email, String role, String status) { + Long id = jdbc.queryForObject("INSERT INTO users (email, password, enabled, role, status, display_name) " + + "VALUES (?, ?, true, ?, ?, ?) RETURNING id", Long.class, email, passwordEncoder.encode(PASSWORD), + role, status, "Flow" + System.nanoTime() % 100000); + addSettingsRows(); + return id; + } + + private MockHttpServletResponse login(String email, String password) throws Exception { + return mockMvc.perform(post("/api/auth/login").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .header("X-Forwarded-For", freshClientIp()).contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + email + "\",\"password\":\"" + password + "\"}")) + .andReturn().getResponse(); + } + + private ResultActions refresh(MockHttpServletResponse session) throws Exception { + return mockMvc.perform(post("/api/auth/refresh").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .cookie(session.getCookie(AuthCookies.REFRESH_COOKIE))); + } + + private ResultActions requestReset(String email) throws Exception { + return mockMvc.perform(post("/api/auth/password-reset").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .header("X-Forwarded-For", freshClientIp()).contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + email + "\"}")); + } + + private ResultActions confirm(String token, String password) throws Exception { + return mockMvc.perform(post("/api/auth/password-reset/confirm").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .header("X-Forwarded-For", freshClientIp()).contentType(MediaType.APPLICATION_JSON) + .content("{\"token\":\"" + token + "\",\"password\":\"" + password + "\"}")); + } + + private ResultActions change(MockHttpServletResponse session, String current, String next) throws Exception { + return mockMvc.perform(post("/api/auth/password").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .cookie(session.getCookie(AuthCookies.ACCESS_COOKIE), session.getCookie(AuthCookies.REFRESH_COOKIE)) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"currentPassword\":\"" + current + "\",\"newPassword\":\"" + next + "\"}")); + } + + private List resetBodies() { + return jdbc.queryForList("SELECT body FROM mail_outbox WHERE kind = 'PASSWORD_RESET' ORDER BY id", String.class); + } + + private String lastToken() { + List bodies = resetBodies(); + Matcher link = LINK.matcher(bodies.get(bodies.size() - 1)); + assertThat(link.find()).isTrue(); + return link.group(1); + } + + private long mails(String kind) { + return jdbc.queryForObject("SELECT COUNT(*) FROM mail_outbox WHERE kind = ?", Long.class, kind); + } + + @Test + @ExtendWith(OutputCaptureExtension.class) + void forgotPassword_MailsALinkThatSetsANewPasswordOnceAndEndsAllLogins(CapturedOutput output) throws Exception { + String email = "reset-flow-main@example.com"; + insertUser(email, "USER", "ACTIVE"); + MockHttpServletResponse sessionA = login(email, PASSWORD); + MockHttpServletResponse sessionB = login(email, PASSWORD); + + requestReset(email).andExpect(status().isNoContent()); + assertThat(resetBodies()).hasSize(1); + String token = lastToken(); + confirm(token, NEW_PASSWORD).andExpect(status().isNoContent()); + + refresh(sessionA).andExpect(status().isUnauthorized()); + refresh(sessionB).andExpect(status().isUnauthorized()); + assertThat(login(email, PASSWORD).getStatus()).isEqualTo(401); + assertThat(login(email, NEW_PASSWORD).getStatus()).isEqualTo(200); + assertThat(mails("PASSWORD_CHANGED")).isEqualTo(1); + confirm(token, NEW_PASSWORD).andExpect(status().isGone()).andExpect(jsonPath("$.code").value("RESET_INVALID")); + assertThat(output.getAll()).doesNotContain(token); + } + + @Test + void forgotPassword_SameAnswerAndNoMailForAnUnknownOrNotActiveAddress() throws Exception { + insertUser("reset-flow-pending@example.com", "USER", "PENDING"); + + requestReset("reset-flow-unknown@example.com").andExpect(status().isNoContent()); + requestReset("reset-flow-pending@example.com").andExpect(status().isNoContent()); + + assertThat(mails("PASSWORD_RESET")).isZero(); + } + + @Test + void twoRequests_OnlyTheNewestLinkWorks() throws Exception { + String email = "reset-flow-two@example.com"; + insertUser(email, "USER", "ACTIVE"); + requestReset(email).andExpect(status().isNoContent()); + String first = lastToken(); + requestReset(email).andExpect(status().isNoContent()); + String second = lastToken(); + + confirm(first, NEW_PASSWORD).andExpect(status().isGone()); + confirm(second, NEW_PASSWORD).andExpect(status().isNoContent()); + } + + @Test + void confirm_RefusesATooShortOrTooLongPassword() throws Exception { + String email = "reset-flow-rules@example.com"; + insertUser(email, "USER", "ACTIVE"); + requestReset(email); + String token = lastToken(); + + confirm(token, "1234567").andExpect(status().isBadRequest()); + confirm(token, "a".repeat(73)).andExpect(status().isBadRequest()); + confirm(token, NEW_PASSWORD).andExpect(status().isNoContent()); + } + + @Test + void changePassword_KeepsTheCurrentLoginAndEndsTheOthers() throws Exception { + String email = "reset-flow-change@example.com"; + insertUser(email, "USER", "ACTIVE"); + MockHttpServletResponse sessionA = login(email, PASSWORD); + MockHttpServletResponse sessionB = login(email, PASSWORD); + + change(sessionA, PASSWORD, NEW_PASSWORD).andExpect(status().isNoContent()); + + refresh(sessionA).andExpect(status().isNoContent()); + refresh(sessionB).andExpect(status().isUnauthorized()); + assertThat(login(email, NEW_PASSWORD).getStatus()).isEqualTo(200); + assertThat(mails("PASSWORD_CHANGED")).isEqualTo(1); + } + + @Test + void changePassword_WrongCurrentPasswordIs403() throws Exception { + String email = "reset-flow-wrong@example.com"; + insertUser(email, "USER", "ACTIVE"); + + change(login(email, PASSWORD), "wrong password", NEW_PASSWORD).andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value("INVALID_PASSWORD")); + + assertThat(login(email, PASSWORD).getStatus()).isEqualTo(200); + } + + @Test + void changePassword_WithoutLoginIs401() throws Exception { + mockMvc.perform(post("/api/auth/password").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"currentPassword\":\"" + PASSWORD + "\",\"newPassword\":\"" + NEW_PASSWORD + "\"}")) + .andExpect(status().isUnauthorized()); + } + + private ResultActions adminReset(MockHttpServletResponse session, long userId) throws Exception { + return mockMvc.perform(post("/api/admin/users/" + userId + "/password-reset") + .header(ClientHeaderFilter.CLIENT_HEADER, "web") + .cookie(new Cookie(AuthCookies.ACCESS_COOKIE, session.getCookie(AuthCookies.ACCESS_COOKIE).getValue()))); + } + + @Test + void adminReset_MailsTheLinkAndWritesTheAudit() throws Exception { + insertUser("reset-flow-admin@example.com", "PLATFORM_ADMIN", "ACTIVE"); + long player = insertUser("reset-flow-player@example.com", "USER", "ACTIVE"); + long pending = insertUser("reset-flow-pend@example.com", "USER", "PENDING"); + MockHttpServletResponse admin = login("reset-flow-admin@example.com", PASSWORD); + + adminReset(admin, player).andExpect(status().isNoContent()); + + assertThat(resetBodies()).hasSize(1); + confirm(lastToken(), NEW_PASSWORD).andExpect(status().isNoContent()); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM admin_audit WHERE action = 'SEND_PASSWORD_RESET' " + + "AND target_user_id = ?", Long.class, player)).isEqualTo(1); + adminReset(admin, pending).andExpect(status().isConflict()).andExpect(jsonPath("$.code").value("NOT_ACTIVE")); + } + + @Test + void adminReset_AsAPlainUserIs403() throws Exception { + long other = insertUser("reset-flow-other@example.com", "USER", "ACTIVE"); + insertUser("reset-flow-plain@example.com", "USER", "ACTIVE"); + + adminReset(login("reset-flow-plain@example.com", PASSWORD), other).andExpect(status().isForbidden()); + + assertThat(mails("PASSWORD_RESET")).isZero(); + } +} diff --git a/src/test/java/auth_service/modules/account/api/PasswordResetRaceTest.java b/src/test/java/auth_service/modules/account/api/PasswordResetRaceTest.java new file mode 100644 index 0000000..6d4d907 --- /dev/null +++ b/src/test/java/auth_service/modules/account/api/PasswordResetRaceTest.java @@ -0,0 +1,152 @@ +package auth_service.modules.account.api; + +import auth_service.configuration.security.ClientHeaderFilter; +import auth_service.modules.account.service.PasswordResetService; +import auth_service.shared.api.AuthCookies; +import auth_service.shared.test.AbstractIntegrationTest; +import jakarta.servlet.http.Cookie; +import javax.sql.DataSource; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.MediaType; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.test.context.bean.override.mockito.MockitoSpyBean; +import org.springframework.test.web.servlet.MockMvc; + +import java.sql.Connection; +import java.sql.Statement; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * A reset that commits while a login with the old password is between its password check and its new refresh token. + * The spy runs the reset exactly in that gap, so the order is fixed, not timed. + */ +class PasswordResetRaceTest extends AbstractIntegrationTest { + + private static final String PASSWORD = "correct horse battery"; + private static final String NEW_PASSWORD = "brand new password"; + private static final Pattern LINK = Pattern.compile("/reset-password#(\\S+)"); + + @Autowired + private MockMvc mockMvc; + + @Autowired + private JdbcTemplate jdbc; + + @Autowired + private DataSource dataSource; + + @MockitoSpyBean + private PasswordEncoder passwordEncoder; + + @Autowired + private PasswordResetService passwordResetService; + + @BeforeEach + void clean() { + jdbc.update("DELETE FROM mail_outbox"); + jdbc.update("DELETE FROM users WHERE email LIKE 'reset-race-%'"); + } + + @Test + void loginRacingAResetGetsNoSessionThatOutlivesTheReset() throws Exception { + String email = "reset-race-login@example.com"; + jdbc.update("INSERT INTO users (email, password, enabled, role, status, display_name) " + + "VALUES (?, ?, true, 'USER', 'ACTIVE', ?)", email, passwordEncoder.encode(PASSWORD), + "Race" + System.nanoTime() % 100000); + addSettingsRows(); + mockMvc.perform(post("/api/auth/password-reset").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .header("X-Forwarded-For", freshClientIp()).contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + email + "\"}")) + .andExpect(status().isNoContent()); + Matcher link = LINK.matcher(jdbc.queryForObject( + "SELECT body FROM mail_outbox WHERE kind = 'PASSWORD_RESET' ORDER BY id DESC LIMIT 1", String.class)); + assertThat(link.find()).isTrue(); + String token = link.group(1); + + // The attacker knows the old password and logs in; the owner's reset commits during that login's bcrypt check. + AtomicReference pendingToken = new AtomicReference<>(token); + doAnswer(invocation -> { + boolean matches = (boolean) invocation.callRealMethod(); + String resetToken = pendingToken.getAndSet(null); + if (resetToken != null) { + passwordResetService.resetPassword(resetToken, NEW_PASSWORD, "198.51.100.7"); + } + return matches; + }).when(passwordEncoder).matches(any(), any()); + + MockHttpServletResponse attacker = mockMvc.perform(post("/api/auth/login") + .header(ClientHeaderFilter.CLIENT_HEADER, "web").header("X-Forwarded-For", freshClientIp()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + email + "\",\"password\":\"" + PASSWORD + "\"}")) + .andReturn().getResponse(); + assertThat(pendingToken.get()).as("the reset ran inside the login").isNull(); + + int afterReset = attacker.getStatus() != 200 ? 401 : mockMvc.perform(post("/api/auth/refresh") + .header(ClientHeaderFilter.CLIENT_HEADER, "web") + .cookie(attacker.getCookie(AuthCookies.REFRESH_COOKIE))) + .andReturn().getResponse().getStatus(); + assertThat(afterReset).as("refresh with the session of the old password, after the reset").isEqualTo(401); + } + + /** + * A password change holds the user row, uncommitted. A refresh must wait for it, then find its token revoked. + * Without the user lock the refresh would rotate at once and the revoke could miss the new token. + */ + @Test + void refreshRacingAPasswordChangeWaitsForItAndGetsNoLiveToken() throws Exception { + String email = "reset-race-refresh@example.com"; + jdbc.update("INSERT INTO users (email, password, enabled, role, status, display_name) " + + "VALUES (?, ?, true, 'USER', 'ACTIVE', ?)", email, passwordEncoder.encode(PASSWORD), + "Race" + System.nanoTime() % 100000); + addSettingsRows(); + long userId = jdbc.queryForObject("SELECT id FROM users WHERE email = ?", Long.class, email); + MockHttpServletResponse login = mockMvc.perform(post("/api/auth/login") + .header(ClientHeaderFilter.CLIENT_HEADER, "web").header("X-Forwarded-For", freshClientIp()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + email + "\",\"password\":\"" + PASSWORD + "\"}")) + .andExpect(status().isOk()).andReturn().getResponse(); + Cookie cookie = login.getCookie(AuthCookies.REFRESH_COOKIE); + + ExecutorService executor = Executors.newSingleThreadExecutor(); + try (Connection owner = dataSource.getConnection()) { + owner.setAutoCommit(false); + try (Statement statement = owner.createStatement()) { + statement.executeUpdate("UPDATE users SET password = 'changed' WHERE id = " + userId); + Future refresh = executor.submit(() -> mockMvc.perform(post("/api/auth/refresh") + .header(ClientHeaderFilter.CLIENT_HEADER, "web").cookie(cookie)) + .andReturn().getResponse().getStatus()); + long deadline = System.currentTimeMillis() + 10_000; + while (jdbc.queryForObject("SELECT count(*) FROM pg_stat_activity WHERE wait_event_type = 'Lock'", + Integer.class) == 0) { + assertThat(System.currentTimeMillis()).as("the refresh waits for the password change") + .isLessThan(deadline); + Thread.sleep(20); + } + statement.executeUpdate("UPDATE refresh_tokens SET revoked_at = now() WHERE user_id = " + userId + + " AND revoked_at IS NULL"); + owner.commit(); + assertThat(refresh.get(10, TimeUnit.SECONDS)).isEqualTo(401); + } + } finally { + executor.shutdownNow(); + } + assertThat(jdbc.queryForObject("SELECT count(*) FROM refresh_tokens WHERE user_id = ? AND revoked_at IS NULL", + Integer.class, userId)).isZero(); + } +} diff --git a/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java b/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java index eaa24f5..4ad7be2 100644 --- a/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java +++ b/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java @@ -12,6 +12,7 @@ import java.time.Instant; import java.time.LocalDateTime; import java.time.temporal.ChronoUnit; import java.util.List; +import java.util.Optional; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -132,4 +133,95 @@ class AccountRepositoryTest extends AbstractPersistenceTest { assertThat(count("select count(*) from users where id = ?", oldWithValidLink)).isEqualTo(1); } + + private void setStatus(long userId, String set) { + jdbc.update("update users set " + set + " where id = ?", userId); + } + + @Test + void passwordReset_OnlyAnActiveUserGetsALink() { + assertThat(repository.findActiveIdByEmail("Alice@Example.com ")).contains(alice); + assertThat(repository.storePasswordReset(alice, "h1", NOW.plusSeconds(3600))).isTrue(); + + for (String notActive : new String[]{"status = 'UNCONFIRMED'", "status = 'PENDING'", "status = 'DENIED'", + "enabled = false", "deleted_at = now()"}) { + setStatus(bob, notActive); + assertThat(repository.findActiveIdByEmail("bob@example.com")).as(notActive).isEmpty(); + assertThat(repository.storePasswordReset(bob, "h2", NOW.plusSeconds(3600))).as(notActive).isFalse(); + setStatus(bob, "status = 'ACTIVE', enabled = true, deleted_at = NULL"); + } + } + + @Test + void resetPassword_ReturnsTheIdOnceAndSetsThePassword() { + repository.storePasswordReset(alice, "h1", NOW.plusSeconds(3600)); + + assertThat(repository.resetPassword("h1", "new-hash", NOW)).isEqualTo(Optional.of(alice)); + assertThat(repository.resetPassword("h1", "newer-hash", NOW)).isEmpty(); + + assertThat(repository.findPasswordHashOfActive(alice)).contains("new-hash"); + assertThat(jdbc.queryForObject("select password_reset_hash from users where id = ?", String.class, alice)).isNull(); + } + + @Test + void resetPassword_ExpiredOrReplacedOrBlockedLinkIsEmpty() { + repository.storePasswordReset(alice, "expired", NOW.minusSeconds(1)); + assertThat(repository.resetPassword("expired", "x", NOW)).isEmpty(); + + repository.storePasswordReset(alice, "first", NOW.plusSeconds(3600)); + repository.storePasswordReset(alice, "second", NOW.plusSeconds(3600)); + assertThat(repository.resetPassword("first", "x", NOW)).as("newest link only").isEmpty(); + + setStatus(alice, "enabled = false"); + assertThat(repository.resetPassword("second", "x", NOW)).as("blocked after the mail").isEmpty(); + assertThat(repository.findPasswordHashOfActive(alice)).contains("hash-alice@example.com"); + } + + @Test + void updatePassword_EndsAnOpenResetLink() { + repository.storePasswordReset(alice, "h1", NOW.plusSeconds(3600)); + + repository.updatePassword(alice, "changed"); + + assertThat(repository.findPasswordHashOfActive(alice)).contains("changed"); + assertThat(repository.resetPassword("h1", "x", NOW)).isEmpty(); + } + + private void insertToken(long userId, String hash, UUID family) { + jdbc.update(""" + insert into refresh_tokens (user_id, token_hash, family_id, expires_at, created_at) + values (?, ?, ?, now() + interval '30 days', now())""", userId, hash, family); + } + + private long activeTokens(long userId) { + return count("select count(*) from refresh_tokens where user_id = ? and revoked_at is null", userId); + } + + @Test + void revokeSessions_KeepsTheFamilyOfTheGivenHashAlsoWhenItWasRotated() { + UUID kept = UUID.randomUUID(); + insertToken(alice, "kept-old", kept); + insertToken(alice, "kept-new", kept); + jdbc.update("update refresh_tokens set revoked_at = now() where token_hash = 'kept-old'"); + insertToken(bob, "bobs", UUID.randomUUID()); + + repository.revokeSessions(alice, "kept-old", NOW); + + assertThat(count("select count(*) from refresh_tokens where token_hash = 'kept-new' and revoked_at is null")).isEqualTo(1); + assertThat(activeTokens(alice)).as("the seed login is gone").isEqualTo(1); + assertThat(activeTokens(bob)).as("another user").isEqualTo(2); + } + + @Test + void revokeSessions_WithoutAHashRevokesAllAndAnotherUsersHashKeepsNothing() { + insertToken(bob, "bobs-hash", UUID.randomUUID()); + + repository.revokeSessions(alice, "bobs-hash", NOW); + assertThat(activeTokens(alice)).isZero(); + + insertToken(alice, "again", UUID.randomUUID()); + repository.revokeSessions(alice, null, NOW); + assertThat(activeTokens(alice)).isZero(); + assertThat(activeTokens(bob)).isEqualTo(2); + } } diff --git a/src/test/java/auth_service/modules/account/service/AccountServiceTest.java b/src/test/java/auth_service/modules/account/service/AccountServiceTest.java index 6e1ffcc..f8c96fd 100644 --- a/src/test/java/auth_service/modules/account/service/AccountServiceTest.java +++ b/src/test/java/auth_service/modules/account/service/AccountServiceTest.java @@ -1,9 +1,10 @@ package auth_service.modules.account.service; import auth_service.modules.account.exception.InvalidPasswordException; -import auth_service.modules.account.exception.TooManyDeleteAttemptsException; +import auth_service.modules.account.exception.TooManyAttemptsException; import auth_service.modules.account.repo.AccountRepository; import auth_service.shared.mail.AccountMailer; +import auth_service.shared.security.OpaqueToken; import auth_service.shared.test.AbstractUnitTest; import org.junit.jupiter.api.BeforeEach; @@ -97,7 +98,7 @@ class AccountServiceTest extends AbstractUnitTest { assertThatThrownBy(() -> service.deleteAccount(USER_ID, "wrong")).isInstanceOf(InvalidPasswordException.class); } - assertThatThrownBy(() -> service.deleteAccount(USER_ID, "right")).isInstanceOf(TooManyDeleteAttemptsException.class); + assertThatThrownBy(() -> service.deleteAccount(USER_ID, "right")).isInstanceOf(TooManyAttemptsException.class); verify(accountRepository, never()).flagDeleted(anyLong(), any()); } @@ -131,4 +132,46 @@ class AccountServiceTest extends AbstractUnitTest { verifyNoInteractions(coreAccountService); verifyNoInteractions(accountMailer); } + + @Test + void changePassword_StoresTheNewHashKeepsTheLoginOfTheCookieAndMails() { + when(passwordEncoder.encode("new password")).thenReturn("new-hash"); + + service.changePassword(USER_ID, "right", "new password", "refresh-cookie"); + + verify(accountRepository).updatePassword(USER_ID, "new-hash"); + verify(accountRepository).revokeSessions(eq(USER_ID), eq(OpaqueToken.hash("refresh-cookie")), any()); + verify(accountMailer).passwordChanged(USER_ID); + } + + @Test + void changePassword_WithoutACookieEndsAllLogins() { + service.changePassword(USER_ID, "right", "new password", null); + + verify(accountRepository).revokeSessions(eq(USER_ID), eq(null), any()); + } + + @Test + void changePassword_WrongCurrentPasswordChangesNothing() { + assertThatThrownBy(() -> service.changePassword(USER_ID, "wrong", "new password", "cookie")) + .isInstanceOf(InvalidPasswordException.class); + + verify(accountRepository, never()).updatePassword(anyLong(), any()); + verifyNoInteractions(accountMailer); + } + + @Test + void changePassword_SixthWrongPasswordIsRateLimitedAlsoAfterWrongDeletes() { + for (int i = 0; i < 3; i++) { + assertThatThrownBy(() -> service.deleteAccount(USER_ID, "wrong")).isInstanceOf(InvalidPasswordException.class); + } + for (int i = 0; i < 2; i++) { + assertThatThrownBy(() -> service.changePassword(USER_ID, "wrong", "new password", null)) + .isInstanceOf(InvalidPasswordException.class); + } + + assertThatThrownBy(() -> service.changePassword(USER_ID, "right", "new password", null)) + .isInstanceOf(TooManyAttemptsException.class); + verify(accountRepository, never()).updatePassword(anyLong(), any()); + } } diff --git a/src/test/java/auth_service/modules/account/service/PasswordResetServiceTest.java b/src/test/java/auth_service/modules/account/service/PasswordResetServiceTest.java new file mode 100644 index 0000000..f241d6c --- /dev/null +++ b/src/test/java/auth_service/modules/account/service/PasswordResetServiceTest.java @@ -0,0 +1,168 @@ +package auth_service.modules.account.service; + +import auth_service.modules.account.exception.PasswordResetInvalidException; +import auth_service.modules.account.exception.TooManyAttemptsException; +import auth_service.modules.account.repo.AccountRepository; +import auth_service.shared.mail.AccountMailer; +import auth_service.shared.security.OpaqueToken; +import auth_service.shared.test.AbstractUnitTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.springframework.security.crypto.password.PasswordEncoder; + +import java.time.Duration; +import java.time.Instant; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +class PasswordResetServiceTest extends AbstractUnitTest { + + private static final long USER_ID = 42L; + private static final String EMAIL = "alice@example.com"; + private static final String IP = "198.51.100.7"; + + @Mock + private AccountRepository accountRepository; + + @Mock + private PasswordEncoder passwordEncoder; + + @Mock + private AccountMailer accountMailer; + + private PasswordResetService service; + + @BeforeEach + void setUp() { + service = new PasswordResetService(accountRepository, passwordEncoder, accountMailer); + } + + @Test + void requestReset_StoresTheHashOfTheMailedTokenForOneHour() { + when(accountRepository.findActiveIdByEmail(EMAIL)).thenReturn(Optional.of(USER_ID)); + when(accountRepository.storePasswordReset(eq(USER_ID), anyString(), any())).thenReturn(true); + Instant before = Instant.now(); + + service.requestReset(EMAIL, IP); + + ArgumentCaptor raw = ArgumentCaptor.forClass(String.class); + verify(accountMailer).passwordReset(eq(USER_ID), raw.capture()); + ArgumentCaptor expires = ArgumentCaptor.forClass(Instant.class); + verify(accountRepository).storePasswordReset(eq(USER_ID), eq(OpaqueToken.hash(raw.getValue())), expires.capture()); + assertThat(expires.getValue()).isBetween(before.plus(Duration.ofHours(1)), Instant.now().plus(Duration.ofHours(1))); + } + + @Test + void requestReset_ForAnUnknownOrInactiveAddressMailsNothingAndDoesNotThrow() { + when(accountRepository.findActiveIdByEmail(EMAIL)).thenReturn(Optional.empty()); + + assertThatNoException().isThrownBy(() -> service.requestReset(EMAIL, IP)); + + verifyNoInteractions(accountMailer); + verify(accountRepository, never()).storePasswordReset(anyLong(), any(), any()); + } + + @Test + void requestReset_FourthRequestForOneAddressInTheHourStoresAndMailsNothing() { + when(accountRepository.findActiveIdByEmail(EMAIL)).thenReturn(Optional.of(USER_ID)); + when(accountRepository.storePasswordReset(eq(USER_ID), anyString(), any())).thenReturn(true); + + for (int i = 0; i < 4; i++) { + service.requestReset(EMAIL, "198.51.100." + i); + } + + verify(accountRepository, org.mockito.Mockito.times(3)).storePasswordReset(eq(USER_ID), anyString(), any()); + verify(accountMailer, org.mockito.Mockito.times(3)).passwordReset(eq(USER_ID), anyString()); + } + + @Test + void requestReset_SixthRequestFromOneIpIs429() { + for (int i = 0; i < 5; i++) { + service.requestReset("user" + i + "@example.com", IP); + } + + assertThatThrownBy(() -> service.requestReset("other@example.com", IP)) + .isInstanceOf(TooManyAttemptsException.class); + } + + @Test + void resetPassword_SetsThePasswordEndsAllLoginsAndMails() { + when(passwordEncoder.encode("new password")).thenReturn("new-hash"); + when(accountRepository.resetPassword(eq(OpaqueToken.hash("raw")), eq("new-hash"), any())) + .thenReturn(Optional.of(USER_ID)); + + service.resetPassword("raw", "new password", IP); + + verify(accountRepository).revokeSessions(eq(USER_ID), eq(null), any()); + verify(accountMailer).passwordChanged(USER_ID); + } + + @Test + void resetPassword_SuccessGivesTheFailureBack() { + when(accountRepository.resetPassword(eq(OpaqueToken.hash("good")), any(), any())).thenReturn(Optional.of(USER_ID)); + when(accountRepository.resetPassword(eq(OpaqueToken.hash("bad")), any(), any())).thenReturn(Optional.empty()); + + for (int i = 0; i < 9; i++) { + assertThatThrownBy(() -> service.resetPassword("bad", "new password", IP)) + .isInstanceOf(PasswordResetInvalidException.class); + } + service.resetPassword("good", "new password", IP); + + assertThatThrownBy(() -> service.resetPassword("bad", "new password", IP)) + .isInstanceOf(PasswordResetInvalidException.class); + } + + @Test + void resetPassword_InvalidTokenEndsNoLoginAndSendsNoMail() { + when(accountRepository.resetPassword(any(), any(), any())).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.resetPassword("raw", "new password", IP)) + .isInstanceOf(PasswordResetInvalidException.class); + + verify(accountRepository, never()).revokeSessions(anyLong(), any(), any()); + verifyNoInteractions(accountMailer); + } + + @Test + void resetPassword_EleventhFailureFromOneIpIs429() { + when(accountRepository.resetPassword(any(), any(), any())).thenReturn(Optional.empty()); + for (int i = 0; i < 10; i++) { + assertThatThrownBy(() -> service.resetPassword("raw", "new password", IP)) + .isInstanceOf(PasswordResetInvalidException.class); + } + + assertThatThrownBy(() -> service.resetPassword("raw", "new password", IP)) + .isInstanceOf(TooManyAttemptsException.class); + } + + @Test + void sendByAdmin_IsFalseAndMailsNothingWhenTheStoreIsRefused() { + when(accountRepository.storePasswordReset(eq(USER_ID), anyString(), any())).thenReturn(false); + + assertThat(service.sendByAdmin(USER_ID)).isFalse(); + + verifyNoInteractions(accountMailer); + } + + @Test + void sendByAdmin_MailsTheLinkForAnActiveUser() { + when(accountRepository.storePasswordReset(eq(USER_ID), anyString(), any())).thenReturn(true); + + assertThat(service.sendByAdmin(USER_ID)).isTrue(); + + verify(accountMailer).passwordReset(eq(USER_ID), anyString()); + } +} diff --git a/src/test/java/auth_service/modules/admin/service/AdminUserServiceTest.java b/src/test/java/auth_service/modules/admin/service/AdminUserServiceTest.java index 28a3e2a..30d1698 100644 --- a/src/test/java/auth_service/modules/admin/service/AdminUserServiceTest.java +++ b/src/test/java/auth_service/modules/admin/service/AdminUserServiceTest.java @@ -6,6 +6,7 @@ import auth_service.modules.admin.model.TargetState; import auth_service.modules.admin.repo.AdminUserRepository; import auth_service.shared.account.AccountDeleter; import auth_service.shared.account.AccountStatePublisher; +import auth_service.shared.account.PasswordResetSender; import auth_service.shared.audit.AdminAuditLog; import auth_service.shared.mail.AccountMailer; import auth_service.shared.security.Role; @@ -47,12 +48,15 @@ class AdminUserServiceTest extends AbstractUnitTest { @Mock private AccountStatePublisher accountStatePublisher; + @Mock + private PasswordResetSender passwordResetSender; + private AdminUserService service; @BeforeEach void setUp() { service = new AdminUserService(adminUserRepository, adminAuditLog, accountDeleter, accountStatePublisher, - accountMailer); + accountMailer, passwordResetSender); } private void target(Role role, boolean deleted) { @@ -86,6 +90,34 @@ class AdminUserServiceTest extends AbstractUnitTest { verify(accountMailer, never()).accountBlocked(anyLong()); } + @Test + void sendPasswordReset_SendsAndWritesTheAudit() { + target(Role.USER, false); + when(passwordResetSender.sendByAdmin(TARGET)).thenReturn(true); + + service.sendPasswordReset(ADMIN, TARGET); + + verify(adminAuditLog).record(ADMIN, "SEND_PASSWORD_RESET", TARGET, null); + } + + @Test + void sendPasswordReset_OfANonActiveUserIsRefusedWithoutAudit() { + target(Role.USER, false); + when(passwordResetSender.sendByAdmin(TARGET)).thenReturn(false); + + assertThat(refusal(() -> service.sendPasswordReset(ADMIN, TARGET))).isEqualTo("NOT_ACTIVE"); + verifyNoInteractions(adminAuditLog); + } + + @Test + void sendPasswordReset_ToYourselfOrAnotherAdminIsRefusedAndSendsNothing() { + assertThat(refusal(() -> service.sendPasswordReset(ADMIN, ADMIN))).isEqualTo("SELF_ACTION"); + target(Role.PLATFORM_ADMIN, false); + + assertThat(refusal(() -> service.sendPasswordReset(ADMIN, TARGET))).isEqualTo("TARGET_IS_ADMIN"); + verifyNoInteractions(passwordResetSender); + } + @Test void block_BlocksAndWritesTheAudit() { target(Role.USER, false); diff --git a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java index 3676b9b..2f71196 100644 --- a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java +++ b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java @@ -85,7 +85,7 @@ class LoginServiceTest extends AbstractUnitTest { when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(true); when(tokenGenerator.generateToken("815", null, Role.USER)).thenReturn("mock-token"); - when(refreshTokenService.startFamily(815L)).thenReturn("mock-refresh"); + when(refreshTokenService.startFamily(815L, dbUser.getPassword())).thenReturn("mock-refresh"); // Act var result = loginService.login(user, IP, null); @@ -252,6 +252,6 @@ class LoginServiceTest extends AbstractUnitTest { assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP, null)); - verify(refreshTokenService, never()).startFamily(anyLong()); + verify(refreshTokenService, never()).startFamily(anyLong(), any()); } } diff --git a/src/test/java/auth_service/modules/mail/model/MailTemplateTest.java b/src/test/java/auth_service/modules/mail/model/MailTemplateTest.java index 95aeaa7..5b79528 100644 --- a/src/test/java/auth_service/modules/mail/model/MailTemplateTest.java +++ b/src/test/java/auth_service/modules/mail/model/MailTemplateTest.java @@ -63,6 +63,16 @@ class MailTemplateTest extends AbstractUnitTest { assertThat(MailTemplate.date(null, date)).isEqualTo("5 November 2026"); } + @ParameterizedTest + @EnumSource(value = MailTemplate.class, names = {"PASSWORD_RESET", "PASSWORD_CHANGED"}) + void render_PasswordMails_CarryTheLinkInBothLanguages(MailTemplate template) { + for (String language : new String[]{"de", "en"}) { + Mail mail = template.render(language, "Alice", "https://tbp.example/reset-password#tok"); + + assertThat(mail.body()).contains("https://tbp.example/reset-password#tok"); + } + } + @Test void render_ConfirmEmail_CarriesTheLinkInBothLanguages() { for (String language : new String[]{"de", "en"}) { diff --git a/src/test/java/auth_service/modules/mail/service/MailServiceTest.java b/src/test/java/auth_service/modules/mail/service/MailServiceTest.java index 8803e6a..04e8299 100644 --- a/src/test/java/auth_service/modules/mail/service/MailServiceTest.java +++ b/src/test/java/auth_service/modules/mail/service/MailServiceTest.java @@ -65,6 +65,27 @@ class MailServiceTest extends AbstractUnitTest { assertThat(mail.body()).startsWith("Hallo Alice,").contains("https://tbp.example/confirm-email#raw-token"); } + @Test + void passwordReset_MailsTheLinkWithTheTokenInTheFragment() { + user("Alice", "en"); + + service(ADMIN).passwordReset(USER, "raw-token"); + + Mail mail = inserted(MailTemplate.PASSWORD_RESET, "alice@example.com"); + assertThat(mail.body()).startsWith("Hello Alice,").contains("https://tbp.example/reset-password#raw-token"); + } + + @Test + void passwordChanged_MailsTheUserInTheirLanguageWithTheResetPageLink() { + user("Alice", "de"); + + service(ADMIN).passwordChanged(USER); + + Mail mail = inserted(MailTemplate.PASSWORD_CHANGED, "alice@example.com"); + assertThat(mail.subject()).contains("geändert"); + assertThat(mail.body()).contains("https://tbp.example/reset-password").doesNotContain("#"); + } + @Test void registrationRequested_NamesTheUserWithoutADisplayNameAsDash() { user(null, "de");