diff --git a/readme.md b/readme.md index 5b1da23..3142c0f 100644 --- a/readme.md +++ b/readme.md @@ -13,7 +13,8 @@ To Be Played is split into three repositories: ## Features -- Registration by admin approval, login with email and password (BCrypt). Emails are stored trimmed and in +- 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). - 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 @@ -83,7 +84,7 @@ To Be Played is split into three repositories: set it (`PUT /api/users/settings/welcome-seen`). V24 sets it for every existing user, so only users registered after 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 "we got your registration", then "your account is active" or "your + 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 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), @@ -105,6 +106,8 @@ To Be Played is split into three repositories: | Method | Path | Auth | |---|---|---| | `POST` | `/api/registration/new` | public | +| `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/refresh` | refresh cookie, replaces both cookies | | `POST` | `/api/auth/logout` | refresh cookie, clears both cookies | @@ -113,11 +116,12 @@ To Be Played is split into three repositories: | `POST` | `/api/users/steam-link` | access token, returns Steam's login URL and sets the binding cookie | | `DELETE` | `/api/users/steam-link` | access token, removes the Steam ID and sets a new access cookie | | `POST` | `/api/auth/view-as` (`{role}`, null = own role; new cookies; `403 ROLE_NOT_ALLOWED` above the own role) | refresh cookie | -| `GET` | `/api/admin/users?query=&status=&page=&size=` (`{content, page, size, totalElements, totalPages}`; `status` PENDING, ACTIVE or DENIED, empty for all; each user has `status`) | PLATFORM_ADMIN | +| `GET` | `/api/admin/users?query=&status=&page=&size=` (`{content, page, size, totalElements, totalPages}`; `status` UNCONFIRMED, PENDING, ACTIVE or DENIED, empty for all; each user has `status`) | PLATFORM_ADMIN | | `GET` | `/api/admin/users/pending-count` (`{count}`: registrations that wait) | PLATFORM_ADMIN | -| `PUT` | `/api/admin/users/{id}/decision` (`{accepted}`; `204`; `409 NOT_PENDING`) | PLATFORM_ADMIN | +| `PUT` | `/api/admin/users/{id}/decision` (`{accepted}`; `204`; `409 NOT_PENDING`, also for an unconfirmed request) | PLATFORM_ADMIN | | `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`) | 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 | | `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 | @@ -304,7 +308,14 @@ sequenceDiagram ### Registration flow -A registration is a request: the account is PENDING until a PLATFORM_ADMIN decides. While 20 wait, the +A registration is a request: the account is UNCONFIRMED until the address owner opens the link in the mail (24 hours, +single use, only its SHA-256 is stored; a resend replaces it) and chooses the password there, then PENDING until a +PLATFORM_ADMIN decides. The registration form has no password: an unconfirmed user has none and cannot log in (login +answers the same `401` as for an unknown address), so nobody can set the password of an address that is not theirs. +The admin list shows an unconfirmed request as such; it cannot be accepted, the admin can delete it for good. The badge counts PENDING only and the admin mail follows the confirmation. The nightly cleanup deletes it after 7 days (not while a resent link is still valid), also in dry run. +Registering again with an unconfirmed address is a resend: a new link, no second row. At most 3 confirmation mails per +address and hour (over all IPs, registration and resend together); over that the answer stays the same and no mail is +sent. Unconfirmed requests count toward the 20 that wait. While 20 wait, the registration answers `409 PENDING_LIMIT_REACHED` and stores nothing. Two requests at the same moment can make it 21. ```mermaid @@ -313,10 +324,13 @@ sequenceDiagram participant A as Auth participant D as Admin U->>A: POST /api/registration/new - A->>A: users.status = PENDING, two mail rows + A->>A: users.status = UNCONFIRMED, one mail row + A-->>U: mail "confirm your address" (link) + U->>A: POST /api/registration/confirm {token, password} + A->>A: users.status = PENDING, password hash stored A-->>D: mail "a new registration waits" - A-->>U: mail "we got your registration" U->>A: login: 403 ACCOUNT_PENDING + Note over A: nightly: UNCONFIRMED older than 7 days is deleted D->>A: PUT /api/admin/users/{id}/decision {accepted} alt accepted A->>A: status = ACTIVE 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 dba245d..9b2f4ba 100644 --- a/src/main/java/auth_service/modules/account/repo/AccountRepository.java +++ b/src/main/java/auth_service/modules/account/repo/AccountRepository.java @@ -10,6 +10,7 @@ import org.springframework.transaction.annotation.Transactional; import java.sql.Timestamp; import java.time.Instant; +import java.time.LocalDateTime; import java.util.Collection; import java.util.List; import java.util.Optional; @@ -88,4 +89,14 @@ public class AccountRepository { } return namedJdbc.update("DELETE FROM users WHERE id IN (:ids)", new MapSqlParameterSource("ids", userIds)); } + + /** + * Settings go by their ON DELETE CASCADE. registration_date is a naive UTC timestamp. A row whose resent link + * is still valid stays. + */ + public int deleteUnconfirmedBefore(LocalDateTime cutoff) { + return jdbc.update(""" + DELETE FROM users WHERE status = 'UNCONFIRMED' AND registration_date < ? + AND (email_confirmation_expires_at IS NULL OR email_confirmation_expires_at < now())""", cutoff); + } } diff --git a/src/main/java/auth_service/modules/account/service/AccountCleanupService.java b/src/main/java/auth_service/modules/account/service/AccountCleanupService.java index 9de307d..2ee93db 100644 --- a/src/main/java/auth_service/modules/account/service/AccountCleanupService.java +++ b/src/main/java/auth_service/modules/account/service/AccountCleanupService.java @@ -7,8 +7,10 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; +import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.time.LocalDateTime; import java.util.List; /** @@ -20,6 +22,7 @@ import java.util.List; public class AccountCleanupService { static final String ZONE = "Europe/Berlin"; + static final Duration UNCONFIRMED_RETENTION = Duration.ofDays(7); private final AccountRepository accountRepository; private final CoreAccountService coreAccountService; @@ -37,6 +40,7 @@ public class AccountCleanupService { @Scheduled(cron = "${app.account-cleanup.cron}", zone = ZONE) public void runNightly() { + purgeUnconfirmed(); try { coreAccountService.replaceDeleted(accountRepository.findFlaggedIds()); } catch (RuntimeException e) { @@ -48,6 +52,19 @@ public class AccountCleanupService { pruneOrphans(); } + // Not gated by dryRun: an unconfirmed row never was an account and has nothing in Core. + private void purgeUnconfirmed() { + try { + int deleted = accountRepository.deleteUnconfirmedBefore( + LocalDateTime.now(Clock.systemUTC()).minus(UNCONFIRMED_RETENTION)); + if (deleted > 0) { + log.info("Account cleanup: deleted {} unconfirmed registrations", deleted); + } + } catch (RuntimeException e) { + log.warn("Account cleanup: purge of unconfirmed registrations failed: {}", e.getMessage()); + } + } + private void sendStates() { try { coreAccountService.replaceStates(accountRepository.findAllStates()); 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 ddab170..602fdd1 100644 --- a/src/main/java/auth_service/modules/admin/api/AdminUserController.java +++ b/src/main/java/auth_service/modules/admin/api/AdminUserController.java @@ -64,6 +64,13 @@ public class AdminUserController { return ResponseEntity.noContent().build(); } + @DeleteMapping(value = "/{userId}/unconfirmed") + public ResponseEntity deleteUnconfirmed(@AuthenticationPrincipal UserPrincipal principal, + @PathVariable long userId) { + adminUserService.deleteUnconfirmed(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 9275139..b8ce1c1 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 or TARGET_IS_PENDING + * SELF_ACTION, TARGET_IS_ADMIN, ALREADY_DELETED, NOT_PENDING, TARGET_IS_PENDING, TARGET_IS_UNCONFIRMED or NOT_UNCONFIRMED */ private final String code; diff --git a/src/main/java/auth_service/modules/admin/model/AdminUser.java b/src/main/java/auth_service/modules/admin/model/AdminUser.java index 7d44f74..9f7eabb 100644 --- a/src/main/java/auth_service/modules/admin/model/AdminUser.java +++ b/src/main/java/auth_service/modules/admin/model/AdminUser.java @@ -6,7 +6,7 @@ import java.time.Instant; import java.time.LocalDateTime; /** - * @param status PENDING (registration waits for an admin), ACTIVE or DENIED + * @param status UNCONFIRMED (address not confirmed yet), PENDING (registration waits for an admin), ACTIVE or DENIED * @param deletedAt set while the account waits for the nightly cleanup */ public record AdminUser(long id, String email, String displayName, Role role, LocalDateTime registrationDate, diff --git a/src/main/java/auth_service/modules/admin/model/TargetState.java b/src/main/java/auth_service/modules/admin/model/TargetState.java index bc8860a..a110384 100644 --- a/src/main/java/auth_service/modules/admin/model/TargetState.java +++ b/src/main/java/auth_service/modules/admin/model/TargetState.java @@ -2,5 +2,5 @@ package auth_service.modules.admin.model; import auth_service.shared.security.Role; -public record TargetState(Role role, boolean deleted, boolean pending) { +public record TargetState(Role role, boolean deleted, boolean pending, boolean unconfirmed) { } diff --git a/src/main/java/auth_service/modules/admin/repo/AdminUserRepository.java b/src/main/java/auth_service/modules/admin/repo/AdminUserRepository.java index 25902d2..9f79d72 100644 --- a/src/main/java/auth_service/modules/admin/repo/AdminUserRepository.java +++ b/src/main/java/auth_service/modules/admin/repo/AdminUserRepository.java @@ -53,7 +53,7 @@ public class AdminUserRepository { /** * @param query part of the email or display name, any case; empty for all users - * @param status PENDING, ACTIVE or DENIED; empty for all + * @param status UNCONFIRMED, PENDING, ACTIVE or DENIED; empty for all */ public List findPage(String query, String status, int page, int size) { Object[] match = matchArgs(query); @@ -74,7 +74,7 @@ public class AdminUserRepository { public Optional findState(long userId) { return jdbc.query("SELECT role, deleted_at, status FROM users WHERE id = ?", (rs, row) -> new TargetState(Role.parse(rs.getString("role")), rs.getTimestamp("deleted_at") != null, - "PENDING".equals(rs.getString("status"))), + "PENDING".equals(rs.getString("status")), "UNCONFIRMED".equals(rs.getString("status"))), userId).stream().findFirst(); } @@ -95,6 +95,15 @@ public class AdminUserRepository { Timestamp.from(now), userId) > 0; } + /** + * Hard delete, like the nightly purge: the address is free at once. Settings go by their ON DELETE CASCADE. + * + * @return false when the user is not unconfirmed (any more) + */ + public boolean deleteUnconfirmed(long userId) { + return jdbc.update("DELETE FROM users WHERE id = ? AND status = 'UNCONFIRMED'", userId) > 0; + } + /** * Blocking also ends every login of the user. * 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 11cf51f..8ed8ef8 100644 --- a/src/main/java/auth_service/modules/admin/service/AdminUserService.java +++ b/src/main/java/auth_service/modules/admin/service/AdminUserService.java @@ -71,7 +71,7 @@ public class AdminUserService { @Caching(evict = {@CacheEvict(value = {"users", "activeUsers"}, key = "#targetUserId"), @CacheEvict(value = "publicStats", allEntries = true)}) public void setBlocked(long actorUserId, long targetUserId, boolean blocked) { - TargetState target = notPending(checkedTarget(actorUserId, targetUserId)); + TargetState target = notARequest(checkedTarget(actorUserId, targetUserId)); if (target.deleted()) { throw new AdminActionRefusedException("ALREADY_DELETED", "The account is already deleted"); } @@ -90,13 +90,27 @@ public class AdminUserService { * transaction: the flag commits before the port calls Core, so Core never hides a user Auth did not flag. */ public void delete(long actorUserId, long targetUserId) { - notPending(checkedTarget(actorUserId, targetUserId)); + notARequest(checkedTarget(actorUserId, targetUserId)); if (accountDeleter.deleteByAdmin(targetUserId)) { adminAuditLog.record(actorUserId, "DELETE", targetUserId, null); log.info("Admin {} deleted user {}", actorUserId, targetUserId); } } + /** + * Only for a request whose address was never confirmed: a hard delete, no mail. The nightly purge does the same + * after 7 days. + */ + @Transactional + public void deleteUnconfirmed(long actorUserId, long targetUserId) { + if (!checkedTarget(actorUserId, targetUserId).unconfirmed() + || !adminUserRepository.deleteUnconfirmed(targetUserId)) { + throw new AdminActionRefusedException("NOT_UNCONFIRMED", "The registration is confirmed already"); + } + adminAuditLog.record(actorUserId, "DELETE_UNCONFIRMED", targetUserId, null); + log.info("Admin {} deleted the unconfirmed registration of 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"); @@ -109,10 +123,13 @@ public class AdminUserService { return target; } - private static TargetState notPending(TargetState target) { + private static TargetState notARequest(TargetState target) { if (target.pending()) { throw new AdminActionRefusedException("TARGET_IS_PENDING", "Accept or deny the registration first"); } + if (target.unconfirmed()) { + throw new AdminActionRefusedException("TARGET_IS_UNCONFIRMED", "Delete the unconfirmed registration instead"); + } return target; } } 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 7ef443e..cfe0fa2 100644 --- a/src/main/java/auth_service/modules/login/service/LoginService.java +++ b/src/main/java/auth_service/modules/login/service/LoginService.java @@ -45,7 +45,8 @@ public class LoginService { /** * Authenticates a user and starts a session. Unknown email, wrong password and deleted account all fail with the - * same InvalidCredentialsException; a pending or blocked account says so, but only after the right password. + * same InvalidCredentialsException; a pending or blocked account says so, but only after the right + * password. An unconfirmed user has no password and fails like a wrong one. * * @param previousRefreshToken the refresh cookie this browser still sends, or null. The new cookie replaces it, * so its login is revoked; else it would stay valid unseen for 30 days. 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 2c5369f..bd79719 100644 --- a/src/main/java/auth_service/modules/mail/model/MailTemplate.java +++ b/src/main/java/auth_service/modules/mail/model/MailTemplate.java @@ -29,20 +29,27 @@ public enum MailTemplate { %s """), - REGISTRATION_RECEIVED( - "Your ToBePlayed registration has arrived", """ + // %1$s greeting, %2$s link. + CONFIRM_EMAIL( + "Confirm your email address for ToBePlayed", """ Hello %s, - thank you for your registration. An administrator checks each new account. - You get a mail as soon as it is decided. + thank you for your registration. Please confirm your email address with this link (valid for 24 hours): + %s + + After that an administrator checks your request, and you get a mail as soon as it is decided. + If you did not register, ignore this mail. ToBePlayed """, - "Deine ToBePlayed-Registrierung ist angekommen", """ + "Bestätige deine E-Mail-Adresse für ToBePlayed", """ Hallo %s, - danke für deine Registrierung. Ein Administrator prüft jedes neue Konto. - Du bekommst eine Mail, sobald entschieden ist. + danke für deine Registrierung. Bitte bestätige deine E-Mail-Adresse über diesen Link (24 Stunden gültig): + %s + + Danach prüft ein Administrator deine Anfrage, und du bekommst eine Mail, sobald entschieden ist. + Wenn du dich nicht registriert hast, ignoriere diese Mail. ToBePlayed """), 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 3950af8..4d9188d 100644 --- a/src/main/java/auth_service/modules/mail/service/MailService.java +++ b/src/main/java/auth_service/modules/mail/service/MailService.java @@ -30,6 +30,12 @@ public class MailService implements AccountMailer { this.frontendUrl = frontendUrl; } + @Override + public void confirmEmail(long userId, String rawToken) { + repository.findRecipient(userId).ifPresent(user -> insertForUser(MailTemplate.CONFIRM_EMAIL, userId, user, + user.greeting(), frontendUrl + "/confirm-email#" + rawToken)); + } + @Override public void registrationRequested(long userId) { repository.findRecipient(userId).ifPresent(user -> { @@ -39,7 +45,6 @@ public class MailService implements AccountMailer { .render(ADMIN_LANGUAGE, user.displayName() == null ? "-" : user.displayName(), user.email(), frontendUrl + "/admin/users")); } - insertForUser(MailTemplate.REGISTRATION_RECEIVED, userId, user, user.greeting()); }); } diff --git a/src/main/java/auth_service/modules/registration/api/RegistrationController.java b/src/main/java/auth_service/modules/registration/api/RegistrationController.java index a905bbc..9a72c65 100644 --- a/src/main/java/auth_service/modules/registration/api/RegistrationController.java +++ b/src/main/java/auth_service/modules/registration/api/RegistrationController.java @@ -1,7 +1,9 @@ package auth_service.modules.registration.api; import auth_service.modules.registration.api.dto.mapper.ApiMapper; +import auth_service.modules.registration.api.dto.request.ConfirmationRequest; import auth_service.modules.registration.api.dto.request.RegistrationRequest; +import auth_service.modules.registration.api.dto.request.ResendConfirmationRequest; import auth_service.modules.registration.api.dto.response.RegistrationResponse; import auth_service.modules.registration.service.RegistrationService; import auth_service.shared.api.ClientIp; @@ -28,4 +30,16 @@ public class RegistrationController { var userId = registrationService.register(ApiMapper.mapToModel(request), ClientIp.of(httpRequest)); return ResponseEntity.ok(RegistrationResponse.builder().userId(userId).build()); } + + @PostMapping(value = "/confirm", consumes = "application/json") + public ResponseEntity confirm(@Valid @RequestBody ConfirmationRequest request, HttpServletRequest httpRequest) { + registrationService.confirm(request.token(), request.password(), ClientIp.of(httpRequest)); + return ResponseEntity.noContent().build(); + } + + @PostMapping(value = "/resend", consumes = "application/json") + public ResponseEntity resend(@Valid @RequestBody ResendConfirmationRequest request, HttpServletRequest httpRequest) { + registrationService.resend(request.email(), ClientIp.of(httpRequest)); + return ResponseEntity.noContent().build(); + } } diff --git a/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java b/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java index e158fcc..6d825b2 100644 --- a/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java +++ b/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java @@ -1,6 +1,7 @@ package auth_service.modules.registration.api; import auth_service.shared.api.dto.response.ErrorResponse; +import auth_service.modules.registration.exception.ConfirmationInvalidException; import auth_service.modules.registration.exception.EmailAlreadyInUseException; import auth_service.modules.registration.exception.PendingLimitReachedException; import auth_service.modules.registration.exception.DisplayNameAlreadyRegisteredException; @@ -51,6 +52,8 @@ public class RegistrationExceptionController { return response(body, HttpStatus.CONFLICT); } - - + @ExceptionHandler(ConfirmationInvalidException.class) + public ResponseEntity clientError(ConfirmationInvalidException ex) { + return response(new ErrorResponse("CONFIRMATION_INVALID", ex.getMessage()), HttpStatus.GONE); + } } diff --git a/src/main/java/auth_service/modules/registration/api/dto/mapper/ApiMapper.java b/src/main/java/auth_service/modules/registration/api/dto/mapper/ApiMapper.java index bdc5704..bf777da 100644 --- a/src/main/java/auth_service/modules/registration/api/dto/mapper/ApiMapper.java +++ b/src/main/java/auth_service/modules/registration/api/dto/mapper/ApiMapper.java @@ -10,7 +10,6 @@ public abstract class ApiMapper { public static Registration mapToModel(RegistrationRequest registrationRequest) { return Registration.builder(). email(registrationRequest.email()). - rawPassword(registrationRequest.password()). displayName(registrationRequest.displayName()). language(registrationRequest.language()). build(); diff --git a/src/main/java/auth_service/modules/registration/api/dto/request/ConfirmationRequest.java b/src/main/java/auth_service/modules/registration/api/dto/request/ConfirmationRequest.java new file mode 100644 index 0000000..5bf10bc --- /dev/null +++ b/src/main/java/auth_service/modules/registration/api/dto/request/ConfirmationRequest.java @@ -0,0 +1,28 @@ +package auth_service.modules.registration.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; + +/** + * The password is set here, by the owner of the address, and never at registration. + */ +public record ConfirmationRequest(@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 "ConfirmationRequest[]"; + } +} diff --git a/src/main/java/auth_service/modules/registration/api/dto/request/RegistrationRequest.java b/src/main/java/auth_service/modules/registration/api/dto/request/RegistrationRequest.java index 5b8a3c2..bea4961 100644 --- a/src/main/java/auth_service/modules/registration/api/dto/request/RegistrationRequest.java +++ b/src/main/java/auth_service/modules/registration/api/dto/request/RegistrationRequest.java @@ -2,22 +2,18 @@ package auth_service.modules.registration.api.dto.request; import auth_service.shared.api.validation.DisplayNameFormat; import auth_service.shared.account.EmailAddress; -import com.fasterxml.jackson.annotation.JsonIgnore; -import jakarta.validation.constraints.AssertTrue; import jakarta.validation.constraints.Email; import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.Pattern; import jakarta.validation.constraints.Size; import lombok.Builder; -import java.nio.charset.StandardCharsets; - /** - * No Steam ID: it comes only from "Sign in through Steam", later in the settings. + * No Steam ID: it comes only from "Sign in through Steam", later in the settings. No password: the address owner + * sets it when confirming the address. */ @Builder public record RegistrationRequest(@NotBlank @Email @Size(max = EmailAddress.MAX_LENGTH) String email, - @NotBlank @Size(min = 8) String password, @Pattern(regexp = DisplayNameFormat.PATTERN) String displayName, // For the mails; null = never chosen. @Pattern(regexp = "de|en") String language) { @@ -25,13 +21,4 @@ public record RegistrationRequest(@NotBlank @Email @Size(max = EmailAddress.MAX_ public RegistrationRequest { email = EmailAddress.normalize(email); } - - /** - * 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; - } } diff --git a/src/main/java/auth_service/modules/registration/api/dto/request/ResendConfirmationRequest.java b/src/main/java/auth_service/modules/registration/api/dto/request/ResendConfirmationRequest.java new file mode 100644 index 0000000..cdc1b03 --- /dev/null +++ b/src/main/java/auth_service/modules/registration/api/dto/request/ResendConfirmationRequest.java @@ -0,0 +1,13 @@ +package auth_service.modules.registration.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 ResendConfirmationRequest(@NotBlank @Email @Size(max = EmailAddress.MAX_LENGTH) String email) { + + public ResendConfirmationRequest { + email = EmailAddress.normalize(email); + } +} diff --git a/src/main/java/auth_service/modules/registration/exception/ConfirmationInvalidException.java b/src/main/java/auth_service/modules/registration/exception/ConfirmationInvalidException.java new file mode 100644 index 0000000..098b4b9 --- /dev/null +++ b/src/main/java/auth_service/modules/registration/exception/ConfirmationInvalidException.java @@ -0,0 +1,7 @@ +package auth_service.modules.registration.exception; + +public class ConfirmationInvalidException extends RuntimeException { + public ConfirmationInvalidException() { + super("The confirmation link is unknown, used or expired."); + } +} diff --git a/src/main/java/auth_service/modules/registration/model/Registration.java b/src/main/java/auth_service/modules/registration/model/Registration.java index 8fc26c5..692f581 100644 --- a/src/main/java/auth_service/modules/registration/model/Registration.java +++ b/src/main/java/auth_service/modules/registration/model/Registration.java @@ -2,16 +2,11 @@ package auth_service.modules.registration.model; import lombok.Builder; import lombok.Data; -import lombok.ToString; @Builder @Data public class Registration { private String email; - @ToString.Exclude - private String rawPassword; - @ToString.Exclude - private String encryptedPassword; private String displayName; private String language; } diff --git a/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepository.java b/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepository.java index 29faa7c..c0a7ca8 100644 --- a/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepository.java +++ b/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepository.java @@ -2,15 +2,32 @@ package auth_service.modules.registration.persistence; import auth_service.modules.registration.model.Registration; +import java.time.Instant; +import java.util.Optional; + public interface RegisteredUserRepository { boolean existsByEmail(String email); boolean existsByDisplayName(String displayName); /** - * Stores the user as PENDING: an admin accepts or denies the registration. + * Stores the user as UNCONFIRMED: the address owner confirms by link, then an admin accepts or denies it. */ long save(Registration registration); - long countPending(); + /** + * UNCONFIRMED and PENDING: the requests that wait for the owner or the admin. + */ + long countWaiting(); + + Optional findUnconfirmedIdByEmail(String email); + + void storeConfirmation(long userId, String tokenHash, Instant expiresAt); + + /** + * Single use: sets PENDING and the password hash and clears the token hash. + * + * @return the user id, empty when the hash is unknown, used or expired + */ + Optional confirm(String tokenHash, String passwordHash, Instant now); } diff --git a/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryImpl.java b/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryImpl.java index 4fe6149..ddf700a 100644 --- a/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryImpl.java +++ b/src/main/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryImpl.java @@ -7,8 +7,11 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; +import java.sql.Timestamp; import java.time.Clock; +import java.time.Instant; import java.time.LocalDateTime; +import java.util.Optional; @Repository @RequiredArgsConstructor @@ -32,9 +35,9 @@ public class RegisteredUserRepositoryImpl implements RegisteredUserRepository { @Transactional public long save(Registration registration) { Long id = jdbc.queryForObject(""" - INSERT INTO users (email, password, enabled, display_name, registration_date, status) - VALUES (?, ?, TRUE, ?, ?, 'PENDING') RETURNING id""", Long.class, - registration.getEmail(), registration.getEncryptedPassword(), registration.getDisplayName(), + INSERT INTO users (email, enabled, display_name, registration_date, status) + VALUES (?, TRUE, ?, ?, 'UNCONFIRMED') RETURNING id""", Long.class, + registration.getEmail(), registration.getDisplayName(), LocalDateTime.now(Clock.systemUTC())); if (id == null) { throw new IllegalStateException("No id for the new user"); @@ -46,8 +49,28 @@ public class RegisteredUserRepositoryImpl implements RegisteredUserRepository { } @Override - public long countPending() { - Long count = jdbc.queryForObject("SELECT COUNT(*) FROM users WHERE status = 'PENDING'", Long.class); + public long countWaiting() { + Long count = jdbc.queryForObject("SELECT COUNT(*) FROM users WHERE status IN ('UNCONFIRMED', 'PENDING')", Long.class); return count != null ? count : 0L; } + + @Override + public Optional findUnconfirmedIdByEmail(String email) { + return jdbc.queryForList("SELECT id FROM users WHERE email = ? AND status = 'UNCONFIRMED'", Long.class, + EmailAddress.normalize(email)).stream().findFirst(); + } + + @Override + public void storeConfirmation(long userId, String tokenHash, Instant expiresAt) { + jdbc.update("UPDATE users SET email_confirmation_hash = ?, email_confirmation_expires_at = ? WHERE id = ?", + tokenHash, Timestamp.from(expiresAt), userId); + } + + @Override + public Optional confirm(String tokenHash, String passwordHash, Instant now) { + return jdbc.queryForList(""" + UPDATE users SET status = 'PENDING', password = ?, email_confirmation_hash = NULL, email_confirmation_expires_at = NULL + WHERE email_confirmation_hash = ? AND email_confirmation_expires_at > ? AND status = 'UNCONFIRMED' + RETURNING id""", Long.class, passwordHash, tokenHash, Timestamp.from(now)).stream().findFirst(); + } } diff --git a/src/main/java/auth_service/modules/registration/service/RegistrationAttemptService.java b/src/main/java/auth_service/modules/registration/service/RegistrationAttemptService.java index 806fe1c..492a844 100644 --- a/src/main/java/auth_service/modules/registration/service/RegistrationAttemptService.java +++ b/src/main/java/auth_service/modules/registration/service/RegistrationAttemptService.java @@ -18,6 +18,10 @@ public class RegistrationAttemptService { static final Duration LOCK_DURATION = Duration.ofMinutes(15); private final AttemptLimiter limiter = new AttemptLimiter(LOCK_DURATION); + static final int MAX_CONFIRMATION_MAILS_PER_ADDRESS = 3; + static final Duration CONFIRMATION_MAIL_WINDOW = Duration.ofHours(1); + + private final AttemptLimiter confirmationMails = new AttemptLimiter(CONFIRMATION_MAIL_WINDOW); private final AttemptLimiter requests = new AttemptLimiter(LOCK_DURATION); public void acquire(String clientIp) { @@ -38,4 +42,12 @@ public class RegistrationAttemptService { throw new TooManyRegistrationAttemptsException(LOCK_DURATION.toSeconds()); } } + + /** + * At most 3 confirmation mails per address and hour over all IPs, so nobody can fill a stranger's inbox or the + * daily mail budget. The caller answers the same when it is false. + */ + public boolean tryConfirmationMail(String email) { + return confirmationMails.tryAcquire(email, MAX_CONFIRMATION_MAILS_PER_ADDRESS); + } } diff --git a/src/main/java/auth_service/modules/registration/service/RegistrationService.java b/src/main/java/auth_service/modules/registration/service/RegistrationService.java index ede4fc8..fe690d3 100644 --- a/src/main/java/auth_service/modules/registration/service/RegistrationService.java +++ b/src/main/java/auth_service/modules/registration/service/RegistrationService.java @@ -1,17 +1,23 @@ package auth_service.modules.registration.service; +import auth_service.modules.registration.exception.ConfirmationInvalidException; import auth_service.modules.registration.exception.EmailAlreadyInUseException; import auth_service.modules.registration.exception.DisplayNameAlreadyRegisteredException; import auth_service.modules.registration.exception.PendingLimitReachedException; import auth_service.modules.registration.model.Registration; import auth_service.modules.registration.persistence.RegisteredUserRepository; import auth_service.shared.mail.AccountMailer; +import auth_service.shared.security.OpaqueToken; import auth_service.shared.security.Public; import lombok.RequiredArgsConstructor; 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; +import java.util.Optional; + @Service @Public @RequiredArgsConstructor @@ -23,32 +29,71 @@ public class RegistrationService { private final AccountMailer accountMailer; static final int MAX_PENDING = 20; + static final Duration CONFIRMATION_TTL = Duration.ofHours(24); /** - * Stores a request that waits for the admin. When MAX_PENDING wait already, it is refused and counts as a failed - * attempt of the IP. + * Stores an unconfirmed request without a password and mails the confirmation link. The address owner sets the + * password when confirming; the admin sees the request after that. When MAX_PENDING requests wait already, it is + * refused and counts as a failed attempt of the IP. An address that is already unconfirmed gets a new link and no + * second row. */ // The user and the mail rows commit together. @Transactional public long register(Registration registration, String clientIp) { registrationAttemptService.acquire(clientIp); + Optional unconfirmed = registeredUserRepository.findUnconfirmedIdByEmail(registration.getEmail()); + if (unconfirmed.isPresent()) { + registrationAttemptService.release(clientIp); + registrationAttemptService.countRequest(clientIp); + sendConfirmation(unconfirmed.get(), registration.getEmail()); + return unconfirmed.get(); + } // ponytail: two requests at the same time can pass at 19 and make 21; a pg_advisory_xact_lock would make it // exact, but the limit is a brake, not a quota. - if (registeredUserRepository.countPending() >= MAX_PENDING) { + if (registeredUserRepository.countWaiting() >= MAX_PENDING) { throw new PendingLimitReachedException(); } validate(registration); registrationAttemptService.release(clientIp); registrationAttemptService.countRequest(clientIp); - var rawPassword = registration.getRawPassword(); - registration.setEncryptedPassword(passwordEncoder.encode(rawPassword)); - long userId = registeredUserRepository.save(registration); - accountMailer.registrationRequested(userId); + sendConfirmation(userId, registration.getEmail()); return userId; } + /** + * Sets the password of the account. A failed confirmation counts as a failed attempt of the IP. + */ + @Transactional + public void confirm(String rawToken, String rawPassword, String clientIp) { + registrationAttemptService.acquire(clientIp); + long userId = registeredUserRepository + .confirm(OpaqueToken.hash(rawToken), passwordEncoder.encode(rawPassword), Instant.now()) + .orElseThrow(ConfirmationInvalidException::new); + registrationAttemptService.release(clientIp); + accountMailer.registrationRequested(userId); + } + + /** + * Never fails for an unknown address: the answer must not tell whether it exists. + */ + @Transactional + public void resend(String email, String clientIp) { + registrationAttemptService.countRequest(clientIp); + registeredUserRepository.findUnconfirmedIdByEmail(email).ifPresent(userId -> sendConfirmation(userId, email)); + } + + // A new link replaces the old one. Over the per-address limit nothing is sent and nothing tells the caller. + private void sendConfirmation(long userId, String email) { + if (!registrationAttemptService.tryConfirmationMail(email)) { + return; + } + String token = OpaqueToken.generate(); + registeredUserRepository.storeConfirmation(userId, OpaqueToken.hash(token), Instant.now().plus(CONFIRMATION_TTL)); + accountMailer.confirmEmail(userId, token); + } + private void validate(Registration registration) { if (registeredUserRepository.existsByEmail(registration.getEmail())) { throw new EmailAlreadyInUseException("The provided email address is already in use."); diff --git a/src/main/java/auth_service/shared/mail/AccountMailer.java b/src/main/java/auth_service/shared/mail/AccountMailer.java index ec99143..04325c1 100644 --- a/src/main/java/auth_service/shared/mail/AccountMailer.java +++ b/src/main/java/auth_service/shared/mail/AccountMailer.java @@ -9,7 +9,12 @@ import java.time.LocalDate; public interface AccountMailer { /** - * "A new registration waits" to the admin and "we got your registration" to the user. + * The link with the raw token; the caller stores only its hash. + */ + void confirmEmail(long userId, String rawToken); + + /** + * "A new registration waits" to the admin, after the user confirmed the address. */ void registrationRequested(long userId); diff --git a/src/main/java/auth_service/shared/security/OpaqueToken.java b/src/main/java/auth_service/shared/security/OpaqueToken.java new file mode 100644 index 0000000..6bdda3d --- /dev/null +++ b/src/main/java/auth_service/shared/security/OpaqueToken.java @@ -0,0 +1,34 @@ +package auth_service.shared.security; + +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.util.Base64; +import java.util.HexFormat; + +/** + * A random token for a mail link. The raw token goes to the user, only the hash is stored, never log the raw one. + */ +public final class OpaqueToken { + + private static final SecureRandom RANDOM = new SecureRandom(); + + private OpaqueToken() { + } + + public static String generate() { + byte[] bytes = new byte[32]; + RANDOM.nextBytes(bytes); + return Base64.getUrlEncoder().withoutPadding().encodeToString(bytes); + } + + public static String hash(String rawToken) { + try { + byte[] digest = MessageDigest.getInstance("SHA-256").digest(rawToken.getBytes(StandardCharsets.UTF_8)); + return HexFormat.of().formatHex(digest); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 is missing", e); + } + } +} diff --git a/src/main/resources/db/migration/V27__Email_Confirmation.sql b/src/main/resources/db/migration/V27__Email_Confirmation.sql new file mode 100644 index 0000000..e2e8774 --- /dev/null +++ b/src/main/resources/db/migration/V27__Email_Confirmation.sql @@ -0,0 +1,12 @@ +-- UNCONFIRMED until the address owner clicks the link in the mail; then PENDING for the admin. +ALTER TABLE users ALTER COLUMN status TYPE VARCHAR(11); +ALTER TABLE users DROP CONSTRAINT users_status_check; +ALTER TABLE users ADD CONSTRAINT users_status_check + CHECK (status IN ('UNCONFIRMED', 'PENDING', 'ACTIVE', 'DENIED')); +-- An unconfirmed user has no password: the owner of the address sets it on the confirm page. +ALTER TABLE users ALTER COLUMN password DROP NOT NULL; +-- SHA-256 hex of the link token, NULL once used; the expiry is 24 h from the last send. +ALTER TABLE users ADD COLUMN email_confirmation_hash VARCHAR(64), + ADD COLUMN email_confirmation_expires_at TIMESTAMPTZ; +CREATE UNIQUE INDEX uq_users_email_confirmation_hash ON users (email_confirmation_hash) + WHERE email_confirmation_hash IS NOT NULL; diff --git a/src/test/java/auth_service/modules/account/api/DeletedAccountFlowTest.java b/src/test/java/auth_service/modules/account/api/DeletedAccountFlowTest.java index 7300e98..e5f77cd 100644 --- a/src/test/java/auth_service/modules/account/api/DeletedAccountFlowTest.java +++ b/src/test/java/auth_service/modules/account/api/DeletedAccountFlowTest.java @@ -192,7 +192,7 @@ class DeletedAccountFlowTest extends AbstractIntegrationTest { } @Test - void registration_WaitsAndDoesNotCountUntilTheAccept() throws Exception { + void registration_IsUnconfirmedAndDoesNotCount() throws Exception { String newEmail = "delete-flow-registered@example.com"; jdbc.update("DELETE FROM users WHERE email = ?", newEmail); long before = totalUsers(); @@ -201,12 +201,11 @@ class DeletedAccountFlowTest extends AbstractIntegrationTest { mockMvc.perform(post("/api/registration/new") .header(ClientHeaderFilter.CLIENT_HEADER, "web").header("X-Forwarded-For", freshClientIp()) .contentType(MediaType.APPLICATION_JSON) - .content("{\"email\":\"" + newEmail + "\",\"password\":\"" + PASSWORD - + "\"}")) + .content("{\"email\":\"" + newEmail + "\"}")) .andExpect(status().is2xxSuccessful()); assertThat(jdbc.queryForObject("SELECT status FROM users WHERE email = ?", String.class, newEmail)) - .isEqualTo("PENDING"); + .isEqualTo("UNCONFIRMED"); assertThat(totalUsers()).isEqualTo(before); } finally { jdbc.update("DELETE FROM users WHERE email = ?", newEmail); @@ -220,8 +219,7 @@ class DeletedAccountFlowTest extends AbstractIntegrationTest { mockMvc.perform(post("/api/registration/new") .header(ClientHeaderFilter.CLIENT_HEADER, "web").header("X-Forwarded-For", freshClientIp()) .contentType(MediaType.APPLICATION_JSON) - .content("{\"email\":\"" + EMAIL + "\",\"password\":\"" + PASSWORD - + "\"}")) + .content("{\"email\":\"" + EMAIL + "\"}")) .andExpect(jsonPath("$.code").value("EMAIL_ALREADY_IN_USE")); } } 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 db25412..84b0c33 100644 --- a/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java +++ b/src/test/java/auth_service/modules/account/repo/AccountRepositoryTest.java @@ -9,6 +9,7 @@ import org.springframework.jdbc.core.JdbcTemplate; import java.sql.Timestamp; import java.time.Instant; +import java.time.LocalDateTime; import java.time.temporal.ChronoUnit; import java.util.List; import java.util.UUID; @@ -100,4 +101,35 @@ class AccountRepositoryTest extends AbstractPersistenceTest { assertThat(count("select count(*) from users where id = ?", bob)).isEqualTo(1); assertThat(repository.deleteUsers(List.of())).isZero(); } + + @Test + void deleteUnconfirmedBefore_RemovesOldUnconfirmedUsersWithTheirSettingsOnly() { + long old = insertUser("old-unconfirmed@example.com"); + long recent = insertUser("recent-unconfirmed@example.com"); + long oldPending = insertUser("old-pending@example.com"); + LocalDateTime now = LocalDateTime.now(); + jdbc.update("update users set status = 'UNCONFIRMED', registration_date = ? where id = ?", now.minusDays(8), old); + jdbc.update("update users set status = 'UNCONFIRMED', registration_date = ? where id = ?", now.minusDays(6), recent); + jdbc.update("update users set status = 'PENDING', registration_date = ? where id = ?", now.minusDays(8), oldPending); + + assertThat(repository.deleteUnconfirmedBefore(now.minusDays(7))).isEqualTo(1); + + assertThat(count("select count(*) from users where id = ?", old)).isZero(); + assertThat(count("select count(*) from user_settings where user_id = ?", old)).isZero(); + assertThat(count("select count(*) from users where id in (?, ?)", recent, oldPending)).isEqualTo(2); + } + + @Test + void deleteUnconfirmedBefore_KeepsAnOldUnconfirmedUserWhoseResentLinkIsStillValid() { + long oldWithValidLink = insertUser("old-valid-link@example.com"); + LocalDateTime now = LocalDateTime.now(); + jdbc.update(""" + update users set status = 'UNCONFIRMED', registration_date = ?, email_confirmation_hash = 'h', + email_confirmation_expires_at = now() + interval '20 hours' where id = ?""", + now.minusDays(8), oldWithValidLink); + + assertThat(repository.deleteUnconfirmedBefore(now.minusDays(7))).isZero(); + + assertThat(count("select count(*) from users where id = ?", oldWithValidLink)).isEqualTo(1); + } } diff --git a/src/test/java/auth_service/modules/account/service/AccountCleanupServiceTest.java b/src/test/java/auth_service/modules/account/service/AccountCleanupServiceTest.java index fa9e109..81239cd 100644 --- a/src/test/java/auth_service/modules/account/service/AccountCleanupServiceTest.java +++ b/src/test/java/auth_service/modules/account/service/AccountCleanupServiceTest.java @@ -6,17 +6,23 @@ import auth_service.modules.account.repo.AccountRepository; 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.InOrder; import org.mockito.Mock; import org.springframework.web.client.RestClientException; +import java.time.Clock; import java.time.Duration; +import java.time.LocalDateTime; import java.util.List; import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -140,4 +146,19 @@ class AccountCleanupServiceTest extends AbstractUnitTest { verify(accountRepository).deleteUsers(List.of(7L)); } + + @Test + void runNightly_PurgesUnconfirmedRegistrationsEvenInDryRunAndBeforeTheCoreCalls() { + when(coreAccountService.purge(any(), anyBoolean())).thenReturn(core(true)); + when(coreAccountService.prune(any(), anyBoolean())).thenReturn(core(true)); + + service(true).runNightly(); + + InOrder order = inOrder(accountRepository, coreAccountService); + ArgumentCaptor cutoff = ArgumentCaptor.forClass(LocalDateTime.class); + order.verify(accountRepository).deleteUnconfirmedBefore(cutoff.capture()); + order.verify(coreAccountService).replaceDeleted(any()); + assertThat(cutoff.getValue()).isBetween(LocalDateTime.now(Clock.systemUTC()).minusDays(7).minusMinutes(1), + LocalDateTime.now(Clock.systemUTC()).minusDays(7).plusMinutes(1)); + } } diff --git a/src/test/java/auth_service/modules/admin/api/AdminFlowTest.java b/src/test/java/auth_service/modules/admin/api/AdminFlowTest.java index 5e3a3a1..220c8fe 100644 --- a/src/test/java/auth_service/modules/admin/api/AdminFlowTest.java +++ b/src/test/java/auth_service/modules/admin/api/AdminFlowTest.java @@ -134,6 +134,46 @@ class AdminFlowTest extends AbstractIntegrationTest { decide(admin, playerId, true).andExpect(status().isConflict()).andExpect(jsonPath("$.code").value("NOT_PENDING")); } + private ResultActions deleteUnconfirmed(Session session, long userId) throws Exception { + return mockMvc.perform(delete("/api/admin/users/" + userId + "/unconfirmed") + .header(ClientHeaderFilter.CLIENT_HEADER, "web") + .cookie(new Cookie(AuthCookies.ACCESS_COOKIE, session.access()))); + } + + @Test + void unconfirmedRequest_IsListedCannotBeAcceptedAndTheAdminDeletesIt() throws Exception { + jdbc.update("UPDATE users SET status = 'UNCONFIRMED' WHERE id = ?", playerId); + Session admin = Session.of(login(ADMIN)); + + mockMvc.perform(get("/api/admin/users").param("status", "UNCONFIRMED") + .cookie(new Cookie(AuthCookies.ACCESS_COOKIE, admin.access()))) + .andExpect(jsonPath("$.content[?(@.email == '" + PLAYER + "')].status").value("UNCONFIRMED")); + decide(admin, playerId, true).andExpect(status().isConflict()).andExpect(jsonPath("$.code").value("NOT_PENDING")); + block(admin, playerId, true).andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("TARGET_IS_UNCONFIRMED")); + + deleteUnconfirmed(admin, playerId).andExpect(status().isNoContent()); + + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM users WHERE id = ?", Long.class, playerId)).isZero(); + assertThat(audits("DELETE_UNCONFIRMED", playerId)).isEqualTo(1); + deleteUnconfirmed(admin, playerId).andExpect(status().isNotFound()); + } + + @Test + void deleteUnconfirmed_RefusesOtherStatusesAndAUser() throws Exception { + Session admin = Session.of(login(ADMIN)); + Session player = Session.of(login(PLAYER)); + + deleteUnconfirmed(player, playerId).andExpect(status().isForbidden()); + deleteUnconfirmed(admin, playerId).andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("NOT_UNCONFIRMED")); + jdbc.update("UPDATE users SET status = 'PENDING' WHERE id = ?", playerId); + deleteUnconfirmed(admin, playerId).andExpect(status().isConflict()); + deleteUnconfirmed(admin, otherAdminId).andExpect(status().isConflict()); + + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM users WHERE id = ?", Long.class, playerId)).isEqualTo(1); + } + @Test void deniedRequest_CannotLogInAndWaitsForTheCleanup() throws Exception { jdbc.update("UPDATE users SET status = 'PENDING' WHERE id = ?", playerId); diff --git a/src/test/java/auth_service/modules/admin/repo/AdminUserRepositoryTest.java b/src/test/java/auth_service/modules/admin/repo/AdminUserRepositoryTest.java index 3b402db..6055b7b 100644 --- a/src/test/java/auth_service/modules/admin/repo/AdminUserRepositoryTest.java +++ b/src/test/java/auth_service/modules/admin/repo/AdminUserRepositoryTest.java @@ -119,4 +119,27 @@ class AdminUserRepositoryTest extends AbstractPersistenceTest { assertThat(jdbc.queryForObject("SELECT status FROM users WHERE id = ?", String.class, alice)).isEqualTo("DENIED"); assertThat(repository.findState(alice).orElseThrow().deleted()).isTrue(); } + + @Test + void findPageAndCount_ListAnUnconfirmedUserAndFilterByIt() { + jdbc.update("UPDATE users SET status = 'UNCONFIRMED' WHERE id = ?", alice); + + assertThat(repository.count("", "")).isEqualTo(3); + assertThat(repository.findPage("", "UNCONFIRMED", 0, 10)).extracting(AdminUser::status) + .containsExactly("UNCONFIRMED"); + assertThat(repository.count("", "PENDING")).isZero(); + assertThat(repository.findState(alice).orElseThrow().unconfirmed()).isTrue(); + } + + @Test + void deleteUnconfirmed_RemovesOnlyAnUnconfirmedUserWithItsSettings() { + assertThat(repository.deleteUnconfirmed(alice)).isFalse(); + jdbc.update("UPDATE users SET status = 'UNCONFIRMED' WHERE id = ?", alice); + + assertThat(repository.deleteUnconfirmed(alice)).isTrue(); + assertThat(repository.deleteUnconfirmed(alice)).isFalse(); + assertThat(repository.count("", "")).isEqualTo(2); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM user_settings WHERE user_id = ?", Long.class, alice)) + .isZero(); + } } 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 ccdfbc7..28a3e2a 100644 --- a/src/test/java/auth_service/modules/admin/service/AdminUserServiceTest.java +++ b/src/test/java/auth_service/modules/admin/service/AdminUserServiceTest.java @@ -56,11 +56,15 @@ class AdminUserServiceTest extends AbstractUnitTest { } private void target(Role role, boolean deleted) { - when(adminUserRepository.findState(TARGET)).thenReturn(Optional.of(new TargetState(role, deleted, false))); + when(adminUserRepository.findState(TARGET)).thenReturn(Optional.of(new TargetState(role, deleted, false, false))); } private void pendingTarget() { - when(adminUserRepository.findState(TARGET)).thenReturn(Optional.of(new TargetState(Role.USER, false, true))); + when(adminUserRepository.findState(TARGET)).thenReturn(Optional.of(new TargetState(Role.USER, false, true, false))); + } + + private void unconfirmedTarget() { + when(adminUserRepository.findState(TARGET)).thenReturn(Optional.of(new TargetState(Role.USER, false, false, true))); } private String refusal(Runnable action) { @@ -194,4 +198,38 @@ class AdminUserServiceTest extends AbstractUnitTest { assertThat(refusal(() -> service.delete(ADMIN, TARGET))).isEqualTo("TARGET_IS_PENDING"); verifyNoInteractions(accountDeleter); } + + @Test + void deleteUnconfirmed_DeletesAndAudits() { + unconfirmedTarget(); + when(adminUserRepository.deleteUnconfirmed(TARGET)).thenReturn(true); + + service.deleteUnconfirmed(ADMIN, TARGET); + + verify(adminAuditLog).record(ADMIN, "DELETE_UNCONFIRMED", TARGET, null); + verifyNoInteractions(accountMailer, accountDeleter); + } + + @Test + void deleteUnconfirmed_RefusesAnyOtherStatusAndALostRace() { + pendingTarget(); + assertThat(refusal(() -> service.deleteUnconfirmed(ADMIN, TARGET))).isEqualTo("NOT_UNCONFIRMED"); + target(Role.USER, false); + assertThat(refusal(() -> service.deleteUnconfirmed(ADMIN, TARGET))).isEqualTo("NOT_UNCONFIRMED"); + unconfirmedTarget(); + when(adminUserRepository.deleteUnconfirmed(TARGET)).thenReturn(false); + assertThat(refusal(() -> service.deleteUnconfirmed(ADMIN, TARGET))).isEqualTo("NOT_UNCONFIRMED"); + + verifyNoInteractions(adminAuditLog); + } + + @Test + void decideBlockAndDelete_RefuseAnUnconfirmedRequest() { + unconfirmedTarget(); + + assertThat(refusal(() -> service.decide(ADMIN, TARGET, true))).isEqualTo("NOT_PENDING"); + assertThat(refusal(() -> service.setBlocked(ADMIN, TARGET, true))).isEqualTo("TARGET_IS_UNCONFIRMED"); + assertThat(refusal(() -> service.delete(ADMIN, TARGET))).isEqualTo("TARGET_IS_UNCONFIRMED"); + verifyNoInteractions(accountDeleter, accountMailer); + } } diff --git a/src/test/java/auth_service/modules/login/model/UserTest.java b/src/test/java/auth_service/modules/login/model/UserTest.java index 02bf883..3544f52 100644 --- a/src/test/java/auth_service/modules/login/model/UserTest.java +++ b/src/test/java/auth_service/modules/login/model/UserTest.java @@ -1,6 +1,5 @@ package auth_service.modules.login.model; -import auth_service.modules.registration.model.Registration; import auth_service.shared.test.AbstractUnitTest; import org.junit.jupiter.api.Test; @@ -20,7 +19,5 @@ class UserTest extends AbstractUnitTest { void toString_ShowsNoPassword() { assertThat(User.builder().email("a@example.com").password("hash-secret").build().toString()) .contains("a@example.com").doesNotContain("hash-secret"); - assertThat(Registration.builder().email("a@example.com").rawPassword("raw-secret").encryptedPassword("hash-secret") - .build().toString()).doesNotContain("raw-secret").doesNotContain("hash-secret"); } } 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 2270324..3676b9b 100644 --- a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java +++ b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java @@ -192,6 +192,15 @@ class LoginServiceTest extends AbstractUnitTest { verify(loginAttemptService).recordSuccess("test@example.com", IP); } + @Test + void login_UnconfirmedUserWithoutAPassword_LooksLikeAnyWrongPassword() { + User dbUser = User.builder().id(815L).email("test@example.com").enabled(true).build(); + when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); + + assertThrows(InvalidCredentialsException.class, () -> loginService.login(user, IP, null)); + verify(tokenGenerator, never()).generateToken(any(), any(), any()); + } + @Test void login_PendingUserWithAWrongPassword_LooksLikeAnyWrongPassword() { User dbUser = User.builder().id(815L).email("test@example.com").password("encodedPassword").enabled(true) 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 632aad5..b5403ba 100644 --- a/src/test/java/auth_service/modules/mail/model/MailTemplateTest.java +++ b/src/test/java/auth_service/modules/mail/model/MailTemplateTest.java @@ -62,4 +62,13 @@ class MailTemplateTest extends AbstractUnitTest { assertThat(MailTemplate.date("en", date)).isEqualTo("5 November 2026"); assertThat(MailTemplate.date(null, date)).isEqualTo("5 November 2026"); } + + @Test + void render_ConfirmEmail_CarriesTheLinkInBothLanguages() { + for (String language : new String[]{"de", "en"}) { + Mail mail = MailTemplate.CONFIRM_EMAIL.render(language, "Alice", "https://tbp.example/confirm-email#tok"); + + assertThat(mail.body()).contains("https://tbp.example/confirm-email#tok"); + } + } } diff --git a/src/test/java/auth_service/modules/mail/service/MailFlowTest.java b/src/test/java/auth_service/modules/mail/service/MailFlowTest.java index 5aad1bc..c9c3f13 100644 --- a/src/test/java/auth_service/modules/mail/service/MailFlowTest.java +++ b/src/test/java/auth_service/modules/mail/service/MailFlowTest.java @@ -6,7 +6,6 @@ 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.mockito.ArgumentCaptor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.MediaType; import org.springframework.jdbc.core.JdbcTemplate; @@ -14,9 +13,12 @@ import org.springframework.mail.MailSendException; import org.springframework.mail.SimpleMailMessage; 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.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -26,6 +28,7 @@ import static org.mockito.Mockito.verify; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; /** @@ -36,6 +39,9 @@ class MailFlowTest extends AbstractIntegrationTest { private static final String PASSWORD = "correct horse battery"; private static final String NEW_USER = "mail-flow-new@example.com"; private static final String ADMIN = "mail-flow-admin@example.com"; + // The per-address mail limit lives in memory for an hour, so the limit tests use an address no other test mails. + private static final String OWNER = "mail-flow-owner@example.com"; + private static final String CAPPED = "mail-flow-capped@example.com"; private static final String PLAYER = "mail-flow-player@example.com"; @Autowired @@ -64,17 +70,19 @@ class MailFlowTest extends AbstractIntegrationTest { } private String accessToken(String email) throws Exception { + return login(email).andReturn().getResponse().getCookie(AuthCookies.ACCESS_COOKIE).getValue(); + } + + private ResultActions login(String email) throws Exception { return mockMvc.perform(post("/api/auth/login").header(ClientHeaderFilter.CLIENT_HEADER, "web") - .contentType(MediaType.APPLICATION_JSON) - .content("{\"email\":\"" + email + "\",\"password\":\"" + PASSWORD + "\"}")) - .andReturn().getResponse().getCookie(AuthCookies.ACCESS_COOKIE).getValue(); + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + email + "\",\"password\":\"" + PASSWORD + "\"}")); } private void register() throws Exception { mockMvc.perform(post("/api/registration/new").header(ClientHeaderFilter.CLIENT_HEADER, "web").header("X-Forwarded-For", freshClientIp()) .contentType(MediaType.APPLICATION_JSON) - .content("{\"email\":\"" + NEW_USER + "\",\"password\":\"" + PASSWORD - + "\",\"displayName\":\"MailFlow\",\"language\":\"de\"}")) + .content("{\"email\":\"" + NEW_USER + "\",\"displayName\":\"MailFlow\",\"language\":\"de\"}")) .andExpect(status().isOk()); } @@ -83,22 +91,48 @@ class MailFlowTest extends AbstractIntegrationTest { } @Test - void registration_WritesBothMailsAndTheJobSendsThem() throws Exception { + void registration_MailsTheLinkFirstAndTheAdminOnlyAfterTheClick() throws Exception { register(); assertThat(rows()).extracting(row -> row.get("kind") + " " + row.get("to_address") + " " + row.get("status")) - .containsExactly("REGISTRATION_REQUESTED admin@test.local PENDING", - "REGISTRATION_RECEIVED " + NEW_USER + " PENDING"); - assertThat(rows().get(1).get("subject")).isEqualTo("Deine ToBePlayed-Registrierung ist angekommen"); + .containsExactly("CONFIRM_EMAIL " + NEW_USER + " PENDING"); + assertThat(rows().get(0).get("subject")).isEqualTo("Bestätige deine E-Mail-Adresse für ToBePlayed"); + String body = jdbc.queryForObject("SELECT body FROM mail_outbox", String.class); + Matcher link = Pattern.compile("/confirm-email#(\\S+)").matcher(body); + assertThat(link.find()).isTrue(); + String token = link.group(1); + assertThat(body).doesNotContain(jdbc.queryForObject("SELECT email_confirmation_hash FROM users WHERE email = ?", + String.class, NEW_USER)); + + confirm(token, PASSWORD).andExpect(status().isNoContent()); + + assertThat(rows()).extracting(row -> row.get("kind") + " " + row.get("to_address")) + .containsExactly("CONFIRM_EMAIL " + NEW_USER, "REGISTRATION_REQUESTED admin@test.local"); + assertThat(jdbc.queryForObject("SELECT status FROM users WHERE email = ?", String.class, NEW_USER)).isEqualTo("PENDING"); + login(NEW_USER).andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value("ACCOUNT_PENDING")); + confirm(token, PASSWORD).andExpect(status().isGone()); mailOutboxService.sendDue(); - ArgumentCaptor sent = ArgumentCaptor.forClass(SimpleMailMessage.class); - verify(mailSender, times(2)).send(sent.capture()); - assertThat(sent.getAllValues()).extracting(SimpleMailMessage::getFrom).containsOnly("noreply@test.local"); + verify(mailSender, times(2)).send(any(SimpleMailMessage.class)); assertThat(rows()).extracting(row -> row.get("status")).containsOnly("SENT"); } + @Test + void login_BeforeTheClick_IsRefusedAndTheUserHasNoPassword() throws Exception { + register(); + + login(NEW_USER).andExpect(status().isUnauthorized()); + assertThat(jdbc.queryForObject("SELECT password FROM users WHERE email = ?", String.class, NEW_USER)).isNull(); + } + + private ResultActions confirm(String token, String password) throws Exception { + return mockMvc.perform(post("/api/registration/confirm").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .header("X-Forwarded-For", freshClientIp()).contentType(MediaType.APPLICATION_JSON) + .content("{\"token\":\"" + token + "\",\"password\":\"" + password + "\"}")); + } + @Test void acceptAndDeny_MailTheUser() throws Exception { insertUser(ADMIN, "PLATFORM_ADMIN"); @@ -128,7 +162,7 @@ class MailFlowTest extends AbstractIntegrationTest { assertThat(rows()).extracting(row -> row.get("status") + " " + row.get("attempts") + " " + row.get("last_error")) .containsOnly("PENDING 1 connection refused"); assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM mail_outbox WHERE next_attempt_at > NOW()", Integer.class)) - .isEqualTo(2); + .isEqualTo(1); jdbc.update("UPDATE mail_outbox SET attempts = ?, next_attempt_at = NOW()", MailOutboxService.MAX_ATTEMPTS - 1); mailOutboxService.sendDue(); @@ -142,8 +176,7 @@ class MailFlowTest extends AbstractIntegrationTest { mockMvc.perform(post("/api/registration/new").header(ClientHeaderFilter.CLIENT_HEADER, "web").header("X-Forwarded-For", freshClientIp()) .contentType(MediaType.APPLICATION_JSON) - .content("{\"email\":\"" + NEW_USER + "\",\"password\":\"" + PASSWORD - + "\"}")) + .content("{\"email\":\"" + NEW_USER + "\"}")) .andExpect(status().isConflict()); assertThat(rows()).isEmpty(); @@ -192,4 +225,83 @@ class MailFlowTest extends AbstractIntegrationTest { assertThat(rows()).extracting(row -> row.get("kind") + " " + row.get("to_address")) .containsExactly("ACCOUNT_DELETED " + PLAYER); } + + private ResultActions registerAs(String email) throws Exception { + return mockMvc.perform(post("/api/registration/new").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .header("X-Forwarded-For", freshClientIp()).contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + email + "\",\"language\":\"de\"}")); + } + + private int confirmMailsTo(String email) { + return jdbc.queryForObject("SELECT COUNT(*) FROM mail_outbox WHERE kind = 'CONFIRM_EMAIL' AND to_address = ?", + Integer.class, email); + } + + private String newestToken() { + String body = jdbc.queryForObject("SELECT body FROM mail_outbox WHERE kind = 'CONFIRM_EMAIL' ORDER BY id DESC LIMIT 1", + String.class); + Matcher link = Pattern.compile("/confirm-email#(\\S+)").matcher(body); + assertThat(link.find()).isTrue(); + return link.group(1); + } + + // A stranger who registers an address first sets no password: the owner's click sets it, and only a link from + // the owner's mail can do that. Both mails carry a token of their own; the newest one wins. + @Test + void strangerRegisteringFirst_cannotLogInToTheOwnersAccount() throws Exception { + registerAs(OWNER).andExpect(status().isOk()); + String strangerToken = newestToken(); + registerAs(OWNER).andExpect(status().isOk()); // the address owner, later: same row, new link + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM users WHERE email = ?", Integer.class, OWNER)).isEqualTo(1); + + confirm(strangerToken, "stranger password 1").andExpect(status().isGone()); + confirm(newestToken(), PASSWORD).andExpect(status().isNoContent()); + insertAdminAndAccept(); + + login(OWNER).andExpect(status().isOk()); + int strangerLogin = mockMvc.perform(post("/api/auth/login").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + OWNER + "\",\"password\":\"stranger password 1\"}")) + .andReturn().getResponse().getStatus(); + assertThat(strangerLogin).isEqualTo(401); + } + + private void insertAdminAndAccept() throws Exception { + insertUser(ADMIN, "PLATFORM_ADMIN"); + long userId = jdbc.queryForObject("SELECT id FROM users WHERE email = ?", Long.class, OWNER); + mockMvc.perform(put("/api/admin/users/" + userId + "/decision").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .cookie(new Cookie(AuthCookies.ACCESS_COOKIE, accessToken(ADMIN))) + .contentType(MediaType.APPLICATION_JSON).content("{\"accepted\":true}")) + .andExpect(status().isNoContent()); + } + + // An address has at most 3 mails per hour over all IPs, and every answer stays the same. + @Test + void resendFromManyIps_isCappedPerAddress() throws Exception { + registerAs(CAPPED).andExpect(status().isOk()); + + for (int i = 0; i < 10; i++) { + mockMvc.perform(post("/api/registration/resend").header(ClientHeaderFilter.CLIENT_HEADER, "web") + .header("X-Forwarded-For", freshClientIp()).contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"" + CAPPED + "\"}")) + .andExpect(status().isNoContent()); + } + for (int i = 0; i < 5; i++) { + registerAs(CAPPED).andExpect(status().isOk()); + } + + assertThat(confirmMailsTo(CAPPED)).isEqualTo(3); + } + + @Test + void unconfirmedRegistrations_countTowardsThePendingLimit() throws Exception { + int maxPending = 20; // RegistrationService.MAX_PENDING + + for (int i = 0; i < maxPending + 1; i++) { + registerAs("mail-flow-flood-" + i + "@example.com"); + } + + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM mail_outbox WHERE kind = 'CONFIRM_EMAIL'", Integer.class)) + .isEqualTo(maxPending); + } } 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 0fdfd02..8803e6a 100644 --- a/src/test/java/auth_service/modules/mail/service/MailServiceTest.java +++ b/src/test/java/auth_service/modules/mail/service/MailServiceTest.java @@ -51,17 +51,26 @@ class MailServiceTest extends AbstractUnitTest { assertThat(inserted(MailTemplate.REGISTRATION_REQUESTED, ADMIN).body()) .startsWith("Eine neue Registrierung").contains("Alice", "alice@example.com", "https://tbp.example/admin/users"); - assertThat(inserted(MailTemplate.REGISTRATION_RECEIVED, "alice@example.com").body()).startsWith("Hello Alice,"); + verify(repository, never()).insert(any(), anyLong(), eq("alice@example.com"), any()); } @Test - void registrationRequested_GreetsByEmailWithoutADisplayName() { + void confirmEmail_MailsTheLinkWithTheTokenInTheFragmentInTheUserLanguage() { + user("Alice", "de"); + + service(ADMIN).confirmEmail(USER, "raw-token"); + + Mail mail = inserted(MailTemplate.CONFIRM_EMAIL, "alice@example.com"); + assertThat(mail.subject()).contains("Bestätige"); + assertThat(mail.body()).startsWith("Hallo Alice,").contains("https://tbp.example/confirm-email#raw-token"); + } + + @Test + void registrationRequested_NamesTheUserWithoutADisplayNameAsDash() { user(null, "de"); service(ADMIN).registrationRequested(USER); - assertThat(inserted(MailTemplate.REGISTRATION_RECEIVED, "alice@example.com").body()) - .startsWith("Hallo alice@example.com,"); assertThat(inserted(MailTemplate.REGISTRATION_REQUESTED, ADMIN).body()).contains("Name: -"); } @@ -71,8 +80,7 @@ class MailServiceTest extends AbstractUnitTest { service("").registrationRequested(USER); - verify(repository, never()).insert(eq(MailTemplate.REGISTRATION_REQUESTED), anyLong(), anyString(), any()); - inserted(MailTemplate.REGISTRATION_RECEIVED, "alice@example.com"); + verify(repository, never()).insert(any(), anyLong(), anyString(), any()); } @Test diff --git a/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java b/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java index f31d782..f35542e 100644 --- a/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java +++ b/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java @@ -9,6 +9,7 @@ import org.springframework.http.converter.HttpMessageNotWritableException; import org.hibernate.exception.ConstraintViolationException; import auth_service.configuration.security.JwtService; import auth_service.modules.registration.api.dto.request.RegistrationRequest; +import auth_service.modules.registration.exception.ConfirmationInvalidException; import auth_service.modules.registration.exception.EmailAlreadyInUseException; import auth_service.modules.registration.exception.PendingLimitReachedException; import auth_service.modules.registration.exception.TooManyRegistrationAttemptsException; @@ -18,6 +19,7 @@ import auth_service.shared.test.AbstractWebMvcTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.json.AutoConfigureJsonTesters; @@ -37,6 +39,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @@ -83,7 +86,6 @@ class RegistrationControllerTest extends AbstractWebMvcTest { // Arrange var request = RegistrationRequest.builder() .email("test@example.com") - .password("SuperSecret123!") .build(); long expectedUserId = 42L; @@ -104,7 +106,6 @@ class RegistrationControllerTest extends AbstractWebMvcTest { // Arrange var request = RegistrationRequest.builder() .email("test@example.com") - .password("SuperSecret123!") .build(); when(registrationService.register(any(Registration.class), any())) @@ -124,7 +125,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!","inviteCode":"code","steamId":"76561198000000001"}""")) + {"email":"test@example.com","inviteCode":"code","steamId":"76561198000000001"}""")) .andExpect(status().isOk()); } @@ -132,7 +133,6 @@ class RegistrationControllerTest extends AbstractWebMvcTest { void register_DuplicateEmailRace_Is409EmailAlreadyInUse() throws Exception { var request = RegistrationRequest.builder() .email("test@example.com") - .password("SuperSecret123!") .build(); when(registrationService.register(any(Registration.class), any())) .thenThrow(duplicate("users_email_lower")); @@ -152,7 +152,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!","displayName":"Lobo"}""")) + {"email":"test@example.com","displayName":"Lobo"}""")) .andExpect(status().isConflict()) .andExpect(jsonPath("$.code").value("DISPLAY_NAME_ALREADY_IN_USE")); } @@ -164,7 +164,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!","language":"de"}""")) + {"email":"test@example.com","language":"de"}""")) .andExpect(status().isOk()); ArgumentCaptor registration = ArgumentCaptor.forClass(Registration.class); @@ -177,7 +177,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!","language":"fr"}""")) + {"email":"test@example.com","language":"fr"}""")) .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")); } @@ -196,7 +196,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!"}""")) + {"email":"test@example.com"}""")) .andExpect(status().isInternalServerError()) .andExpect(jsonPath("$.code").value("INTERNAL_ERROR")) .andExpect(content().string(not(containsString("secret detail")))); @@ -207,7 +207,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!","displayName":"x"}""")) + {"email":"test@example.com","displayName":"x"}""")) .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")) .andExpect(jsonPath("$.fields[0].field").value("displayName")); @@ -224,7 +224,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!","displayName":"Lobo"}""")) + {"email":"test@example.com","displayName":"Lobo"}""")) .andExpect(status().isOk()); // Assert @@ -243,7 +243,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { .header("X-Forwarded-For", "10.0.0.1, 203.0.113.7") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!"}""")) + {"email":"test@example.com"}""")) .andExpect(status().isOk()) .andExpect(jsonPath("$.userId").value(42L)); } @@ -258,7 +258,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!"}""")) + {"email":"test@example.com"}""")) .andExpect(status().isTooManyRequests()) .andExpect(header().string("Retry-After", "900")) .andExpect(jsonPath("$.code").value("TOO_MANY_ATTEMPTS")); @@ -271,33 +271,15 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!"}""")) + {"email":"test@example.com"}""")) .andExpect(status().isConflict()) .andExpect(jsonPath("$.code").value("PENDING_LIMIT_REACHED")); } @ParameterizedTest - @CsvSource(delimiter = '|', value = { - " |SuperSecret123!", - "no-at-sign|SuperSecret123!", - "test@example.com| ", - "test@example.com|short", - }) - void register_InvalidInput_Is400WithoutRegistering(String email, String password) throws Exception { - var request = RegistrationRequest.builder().email(email).password(password).build(); - - mockMvc.perform(post("/api/registration/new") - .contentType(MediaType.APPLICATION_JSON) - .content(registrationRequestJson.write(request).getJson())) - .andExpect(status().isBadRequest()); - - verifyNoInteractions(registrationService); - } - - // BCrypt uses only 72 bytes: a longer password would mostly be ignored, or refused with a 500. - @Test - void register_PasswordOver72Bytes_Is400() throws Exception { - var request = RegistrationRequest.builder().email("test@example.com").password("é".repeat(37)).build(); + @ValueSource(strings = {" ", "no-at-sign"}) + void register_InvalidEmail_Is400WithoutRegistering(String email) throws Exception { + var request = RegistrationRequest.builder().email(email).build(); mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) @@ -309,7 +291,7 @@ class RegistrationControllerTest extends AbstractWebMvcTest { @Test void register_Email_IsTrimmedAndLowerCase() throws Exception { - var request = RegistrationRequest.builder().email(" Bob@Example.COM ").password("SuperSecret123!").build(); + var request = RegistrationRequest.builder().email(" Bob@Example.COM ").build(); when(registrationService.register(any(Registration.class), any())).thenReturn(1L); mockMvc.perform(post("/api/registration/new") @@ -331,10 +313,89 @@ class RegistrationControllerTest extends AbstractWebMvcTest { mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(""" - {"email":"test@example.com","password":"SuperSecret123!"}""")) + {"email":"test@example.com"}""")) .andExpect(status().isInternalServerError()) .andExpect(jsonPath("$.code").value("INTERNAL_ERROR")); assertTrue(output.getAll().contains("body not writable"), "the 500 left no trace in the log"); } + + @Test + void confirm_Is204AndPassesTokenPasswordAndIp() throws Exception { + mockMvc.perform(post("/api/registration/confirm").header("X-Forwarded-For", "203.0.113.7") + .contentType(MediaType.APPLICATION_JSON).content("{\"token\":\"abc\",\"password\":\"SuperSecret123!\"}")) + .andExpect(status().isNoContent()); + + verify(registrationService).confirm("abc", "SuperSecret123!", "203.0.113.7"); + } + + @Test + void confirm_AnInvalidLink_Is410ConfirmationInvalid() throws Exception { + doThrow(new ConfirmationInvalidException()).when(registrationService).confirm(any(), any(), any()); + + mockMvc.perform(post("/api/registration/confirm").contentType(MediaType.APPLICATION_JSON) + .content("{\"token\":\"abc\",\"password\":\"SuperSecret123!\"}")) + .andExpect(status().isGone()) + .andExpect(jsonPath("$.code").value("CONFIRMATION_INVALID")); + } + + @ParameterizedTest + @CsvSource(delimiter = '|', value = { + "abc| ", + "abc|short", + "abc|ééééééééééééééééééééééééééééééééééééé", + }) + void confirm_InvalidInput_Is400WithoutConfirming(String token, String password) throws Exception { + mockMvc.perform(post("/api/registration/confirm").contentType(MediaType.APPLICATION_JSON) + .content("{\"token\":\"" + token + "\",\"password\":\"" + password + "\"}")) + .andExpect(status().isBadRequest()); + + verifyNoInteractions(registrationService); + } + + @Test + void confirm_BlankToken_Is400() throws Exception { + mockMvc.perform(post("/api/registration/confirm").contentType(MediaType.APPLICATION_JSON) + .content("{\"token\":\" \",\"password\":\"SuperSecret123!\"}")) + .andExpect(status().isBadRequest()); + + verifyNoInteractions(registrationService); + } + + @Test + void confirm_NoPassword_Is400() throws Exception { + mockMvc.perform(post("/api/registration/confirm").contentType(MediaType.APPLICATION_JSON) + .content("{\"token\":\"abc\"}")) + .andExpect(status().isBadRequest()); + + verifyNoInteractions(registrationService); + } + + @Test + void resend_Is204AndNormalizesTheEmail() throws Exception { + mockMvc.perform(post("/api/registration/resend").contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\" Bob@Example.COM \"}")) + .andExpect(status().isNoContent()); + + verify(registrationService).resend(eq("bob@example.com"), any()); + } + + @Test + void resend_BadEmail_Is400() throws Exception { + mockMvc.perform(post("/api/registration/resend").contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"no-at-sign\"}")) + .andExpect(status().isBadRequest()); + + verifyNoInteractions(registrationService); + } + + @Test + void resend_TooManyRequests_Is429WithRetryAfter() throws Exception { + doThrow(new TooManyRegistrationAttemptsException(900)).when(registrationService).resend(any(), any()); + + mockMvc.perform(post("/api/registration/resend").contentType(MediaType.APPLICATION_JSON) + .content("{\"email\":\"bob@example.com\"}")) + .andExpect(status().isTooManyRequests()) + .andExpect(header().string("Retry-After", "900")); + } } diff --git a/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java b/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java index 4047e61..b3ff4d1 100644 --- a/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java +++ b/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java @@ -7,10 +7,13 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.DuplicateKeyException; import org.springframework.jdbc.core.JdbcTemplate; +import java.time.Instant; import java.util.Map; +import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -23,7 +26,7 @@ class RegisteredUserRepositoryTest extends AbstractPersistenceTest { private JdbcTemplate jdbc; private static Registration registration(String email, String displayName) { - return Registration.builder().email(email).encryptedPassword("hashed").displayName(displayName).build(); + return Registration.builder().email(email).displayName(displayName).build(); } @Test @@ -48,7 +51,7 @@ class RegisteredUserRepositoryTest extends AbstractPersistenceTest { Map saved = jdbc.queryForMap("SELECT * FROM users WHERE id = ?", id); assertEquals("save@example.com", saved.get("email")); - assertEquals("hashed", saved.get("password")); + assertNull(saved.get("password")); assertEquals(true, saved.get("enabled")); assertEquals("Saver", saved.get("display_name")); assertEquals("USER", saved.get("role")); @@ -65,13 +68,13 @@ class RegisteredUserRepositoryTest extends AbstractPersistenceTest { } @Test - void save_StoresAPendingRequestAndCountsIt() { - long before = registeredUserRepository.countPending(); + void save_StoresAnUnconfirmedRequestThatCountsAsWaiting() { + long before = registeredUserRepository.countWaiting(); long id = registeredUserRepository.save(registration("pending@example.com", null)); - assertEquals("PENDING", jdbc.queryForObject("SELECT status FROM users WHERE id = ?", String.class, id)); - assertEquals(before + 1, registeredUserRepository.countPending()); + assertEquals("UNCONFIRMED", jdbc.queryForObject("SELECT status FROM users WHERE id = ?", String.class, id)); + assertEquals(before + 1, registeredUserRepository.countWaiting()); } @Test @@ -95,4 +98,43 @@ class RegisteredUserRepositoryTest extends AbstractPersistenceTest { // GlobalExceptionController finds the name in the message, in every server language. assertTrue(refused.getMostSpecificCause().getMessage().contains("users_email_lower")); } + + @Test + void confirm_ReturnsTheIdOnceAndMakesTheUserPending() { + long id = registeredUserRepository.save(registration("confirm-once@example.com", null)); + registeredUserRepository.storeConfirmation(id, "hash-once", Instant.now().plusSeconds(60)); + + assertEquals(Optional.of(id), registeredUserRepository.confirm("hash-once", "pw-hash", Instant.now())); + assertEquals(Optional.empty(), registeredUserRepository.confirm("hash-once", "pw-hash", Instant.now())); + assertEquals("PENDING", jdbc.queryForObject("SELECT status FROM users WHERE id = ?", String.class, id)); + assertEquals("pw-hash", jdbc.queryForObject("SELECT password FROM users WHERE id = ?", String.class, id)); + assertEquals(Optional.empty(), registeredUserRepository.findUnconfirmedIdByEmail("confirm-once@example.com")); + } + + @Test + void confirm_AnExpiredHashIsEmptyAndKeepsTheUserUnconfirmed() { + long id = registeredUserRepository.save(registration("confirm-late@example.com", null)); + registeredUserRepository.storeConfirmation(id, "hash-late", Instant.now().minusSeconds(1)); + + assertEquals(Optional.empty(), registeredUserRepository.confirm("hash-late", "pw-hash", Instant.now())); + assertEquals(Optional.of(id), registeredUserRepository.findUnconfirmedIdByEmail("Confirm-Late@example.com")); + assertNull(jdbc.queryForObject("SELECT password FROM users WHERE id = ?", String.class, id)); + } + + @Test + void storeConfirmation_ANewHashInvalidatesTheOldOne() { + long id = registeredUserRepository.save(registration("confirm-resend@example.com", null)); + registeredUserRepository.storeConfirmation(id, "hash-old", Instant.now().plusSeconds(60)); + registeredUserRepository.storeConfirmation(id, "hash-new", Instant.now().plusSeconds(60)); + + assertEquals(Optional.empty(), registeredUserRepository.confirm("hash-old", "pw-hash", Instant.now())); + assertEquals(Optional.of(id), registeredUserRepository.confirm("hash-new", "pw-hash", Instant.now())); + } + + @Test + void findUnconfirmedIdByEmail_IgnoresAPendingUser() { + jdbc.update("INSERT INTO users (email, password, enabled, status) VALUES ('pend@example.com', 'x', true, 'PENDING')"); + + assertEquals(Optional.empty(), registeredUserRepository.findUnconfirmedIdByEmail("pend@example.com")); + } } diff --git a/src/test/java/auth_service/modules/registration/service/RegistrationAttemptServiceTest.java b/src/test/java/auth_service/modules/registration/service/RegistrationAttemptServiceTest.java index 72e81d6..70239b3 100644 --- a/src/test/java/auth_service/modules/registration/service/RegistrationAttemptServiceTest.java +++ b/src/test/java/auth_service/modules/registration/service/RegistrationAttemptServiceTest.java @@ -16,6 +16,7 @@ import java.util.concurrent.atomic.AtomicInteger; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -106,4 +107,14 @@ class RegistrationAttemptServiceTest extends AbstractUnitTest { assertTrue(passed.get() <= RegistrationAttemptService.MAX_FAILURES_PER_IP, "attempts that passed the limit: " + passed.get()); } + + @Test + void allowsThreeConfirmationMailsPerAddressWhateverTheIp() { + for (int i = 0; i < RegistrationAttemptService.MAX_CONFIRMATION_MAILS_PER_ADDRESS; i++) { + assertTrue(attempts.tryConfirmationMail("a@example.com")); + } + + assertFalse(attempts.tryConfirmationMail("a@example.com")); + assertTrue(attempts.tryConfirmationMail("b@example.com")); + } } diff --git a/src/test/java/auth_service/modules/registration/service/RegistrationServiceTest.java b/src/test/java/auth_service/modules/registration/service/RegistrationServiceTest.java index bdcb07f..3b53cd7 100644 --- a/src/test/java/auth_service/modules/registration/service/RegistrationServiceTest.java +++ b/src/test/java/auth_service/modules/registration/service/RegistrationServiceTest.java @@ -1,5 +1,6 @@ package auth_service.modules.registration.service; +import auth_service.modules.registration.exception.ConfirmationInvalidException; import auth_service.modules.registration.exception.EmailAlreadyInUseException; import auth_service.modules.registration.exception.DisplayNameAlreadyRegisteredException; import auth_service.modules.registration.exception.PendingLimitReachedException; @@ -7,16 +8,24 @@ import auth_service.modules.registration.exception.TooManyRegistrationAttemptsEx import auth_service.modules.registration.model.Registration; import auth_service.modules.registration.persistence.RegisteredUserRepository; 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.InjectMocks; 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.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; /** @@ -47,7 +56,6 @@ class RegistrationServiceTest extends AbstractUnitTest { void setUp() { validRegistration = Registration.builder() .email("test@example.com") - .rawPassword("SuperSecret123!") .build(); } @@ -55,8 +63,8 @@ class RegistrationServiceTest extends AbstractUnitTest { void register_Success() { // Arrange long expectedUserId = 42L; + when(registrationAttemptService.tryConfirmationMail(any())).thenReturn(true); when(registeredUserRepository.existsByEmail(validRegistration.getEmail())).thenReturn(false); - when(passwordEncoder.encode(validRegistration.getRawPassword())).thenReturn("hashedPasswordXYZ"); when(registeredUserRepository.save(any(Registration.class))).thenReturn(expectedUserId); // Act @@ -64,18 +72,85 @@ class RegistrationServiceTest extends AbstractUnitTest { // Assert assertEquals(expectedUserId, result); - verify(passwordEncoder).encode("SuperSecret123!"); + verifyNoInteractions(passwordEncoder); verify(registeredUserRepository).save(validRegistration); - assertEquals("hashedPasswordXYZ", validRegistration.getEncryptedPassword()); verify(registrationAttemptService).acquire(IP); verify(registrationAttemptService).release(IP); verify(registrationAttemptService).countRequest(IP); - verify(accountMailer).registrationRequested(expectedUserId); + verify(accountMailer, never()).registrationRequested(anyLong()); + ArgumentCaptor token = ArgumentCaptor.forClass(String.class); + verify(accountMailer).confirmEmail(eq(expectedUserId), token.capture()); + ArgumentCaptor expires = ArgumentCaptor.forClass(Instant.class); + verify(registeredUserRepository).storeConfirmation(eq(expectedUserId), eq(OpaqueToken.hash(token.getValue())), + expires.capture()); + assertThat(expires.getValue()).isBetween(Instant.now().plus(Duration.ofHours(23)), + Instant.now().plus(Duration.ofHours(25))); + } + + @Test + void register_AnUnconfirmedEmail_IsNoConflictNoSecondRowButANewLink() { + when(registrationAttemptService.tryConfirmationMail(any())).thenReturn(true); + when(registeredUserRepository.findUnconfirmedIdByEmail(validRegistration.getEmail())).thenReturn(Optional.of(5L)); + + assertEquals(5L, registrationService.register(validRegistration, IP)); + + verify(registeredUserRepository, never()).save(any()); + verify(registeredUserRepository, never()).existsByEmail(anyString()); + verify(passwordEncoder, never()).encode(any()); + verify(registrationAttemptService).release(IP); + verify(registrationAttemptService).countRequest(IP); + verify(accountMailer).confirmEmail(eq(5L), anyString()); + } + + @Test + void confirm_AValidToken_ReleasesTheAttemptAndTellsTheAdmin() { + when(passwordEncoder.encode("SuperSecret123!")).thenReturn("hashedPasswordXYZ"); + when(registeredUserRepository.confirm(eq(OpaqueToken.hash("raw")), eq("hashedPasswordXYZ"), any())) + .thenReturn(Optional.of(9L)); + + registrationService.confirm("raw", "SuperSecret123!", IP); + + verify(registrationAttemptService).acquire(IP); + verify(registrationAttemptService).release(IP); + verify(accountMailer).registrationRequested(9L); + } + + @Test + void confirm_AnInvalidToken_CountsAsAFailedAttemptAndMailsNothing() { + when(registeredUserRepository.confirm(anyString(), any(), any())).thenReturn(Optional.empty()); + + assertThrows(ConfirmationInvalidException.class, () -> registrationService.confirm("raw", "SuperSecret123!", IP)); + + verify(registrationAttemptService, never()).release(anyString()); + verifyNoInteractions(accountMailer); + } + + @Test + void resend_AnUnknownAddress_MailsNothingAndDoesNotThrow() { + when(registeredUserRepository.findUnconfirmedIdByEmail("nobody@example.com")).thenReturn(Optional.empty()); + + registrationService.resend("nobody@example.com", IP); + + verify(registrationAttemptService).countRequest(IP); + verifyNoInteractions(accountMailer); + verify(registeredUserRepository, never()).storeConfirmation(anyLong(), anyString(), any()); + } + + @Test + void resend_AnUnconfirmedAddress_StoresANewHashAndMailsTheLink() { + when(registeredUserRepository.findUnconfirmedIdByEmail("a@example.com")).thenReturn(Optional.of(5L)); + when(registrationAttemptService.tryConfirmationMail("a@example.com")).thenReturn(true); + + registrationService.resend("a@example.com", IP); + + verify(registrationAttemptService).countRequest(IP); + verify(registeredUserRepository).storeConfirmation(eq(5L), anyString(), any()); + verify(accountMailer).confirmEmail(eq(5L), anyString()); } @Test void register_AFullQueue_RefusesAndCountsAsAFailedAttempt() { - when(registeredUserRepository.countPending()).thenReturn((long) RegistrationService.MAX_PENDING); + when(registeredUserRepository.countWaiting()).thenReturn((long) RegistrationService.MAX_PENDING); assertThrows(PendingLimitReachedException.class, () -> registrationService.register(validRegistration, IP)); @@ -85,10 +160,43 @@ class RegistrationServiceTest extends AbstractUnitTest { verifyNoInteractions(accountMailer); } + @Test + void register_AFullQueue_StillAnswersAnUnconfirmedAddressWithANewLink() { + when(registeredUserRepository.findUnconfirmedIdByEmail(validRegistration.getEmail())).thenReturn(Optional.of(5L)); + when(registrationAttemptService.tryConfirmationMail(any())).thenReturn(true); + + assertEquals(5L, registrationService.register(validRegistration, IP)); + + verify(registeredUserRepository, never()).countWaiting(); + } + + @Test + void register_AnUnconfirmedAddressOverTheMailLimit_AnswersTheSameAndMailsNothing() { + when(registeredUserRepository.findUnconfirmedIdByEmail(validRegistration.getEmail())).thenReturn(Optional.of(5L)); + when(registrationAttemptService.tryConfirmationMail(validRegistration.getEmail())).thenReturn(false); + + assertEquals(5L, registrationService.register(validRegistration, IP)); + + verify(registeredUserRepository, never()).storeConfirmation(anyLong(), anyString(), any()); + verifyNoInteractions(accountMailer); + } + + @Test + void resend_OverTheMailLimit_MailsNothingAndKeepsTheOldLink() { + when(registeredUserRepository.findUnconfirmedIdByEmail("a@example.com")).thenReturn(Optional.of(5L)); + when(registrationAttemptService.tryConfirmationMail("a@example.com")).thenReturn(false); + + registrationService.resend("a@example.com", IP); + + verify(registeredUserRepository, never()).storeConfirmation(anyLong(), anyString(), any()); + verifyNoInteractions(accountMailer); + } + @Test void register_OneBelowTheLimit_IsTaken() { - when(registeredUserRepository.countPending()).thenReturn((long) RegistrationService.MAX_PENDING - 1); + when(registeredUserRepository.countWaiting()).thenReturn((long) RegistrationService.MAX_PENDING - 1); when(registeredUserRepository.save(any(Registration.class))).thenReturn(43L); + when(registrationAttemptService.tryConfirmationMail(any())).thenReturn(true); assertEquals(43L, registrationService.register(validRegistration, IP)); } diff --git a/src/test/java/auth_service/shared/security/OpaqueTokenTest.java b/src/test/java/auth_service/shared/security/OpaqueTokenTest.java new file mode 100644 index 0000000..b595e0d --- /dev/null +++ b/src/test/java/auth_service/shared/security/OpaqueTokenTest.java @@ -0,0 +1,23 @@ +package auth_service.shared.security; + +import auth_service.shared.test.AbstractUnitTest; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class OpaqueTokenTest extends AbstractUnitTest { + + @Test + void generate_Is43UrlSafeCharsAndDiffersPerCall() { + String first = OpaqueToken.generate(); + + assertThat(first).matches("[A-Za-z0-9_-]{43}"); + assertThat(OpaqueToken.generate()).isNotEqualTo(first); + } + + @Test + void hash_Is64HexCharsAndDeterministic() { + assertThat(OpaqueToken.hash("abc")).matches("[0-9a-f]{64}").isEqualTo(OpaqueToken.hash("abc")) + .isNotEqualTo(OpaqueToken.hash("abd")); + } +}