diff --git a/readme.md b/readme.md index 48e865a..71222c0 100644 --- a/readme.md +++ b/readme.md @@ -41,8 +41,15 @@ To Be Played is split into three repositories: tiles in order, each with a size S, M or L), apart from the app appearance; the public answer sends both as `style`. The ticket's claims come from the layout: `sections` names the shown parts (`STATS` when any `STATS_` tile is shown), `profileStats` the stat groups of the `STATS_` tiles. The user's dashboard layout - (`dashboard_tiles`, NULL = never changed) is stored the same way. The ticket is not a login: - both services reject every token with a `typ` claim on normal endpoints. + (`dashboard_tiles`, NULL = never changed) is stored the same way. The ticket is not a login: every token names + its kind in `aud` (`tbp-login`, `tbp-public-profile`, `tbp-internal`, `iss=tbp-auth`), and both services check it. + A login's `sub` is the user id; the email is in no token. Until item 29 the tokens also carry the claims of before + item 26 (`userId`, `typ`), so an older Core still reads them. The claims are pinned in + `src/test/resources/contract/auth-core-contract.json`, the same file as in Core + (`ToBePlayed-Deployment/scripts/check-contract.sh` compares both). +- Errors: every error answer of a controller has the body `{code, message}` (a 401 or 403 of the security filters has none); a refused request is `400 VALIDATION_FAILED` with + `fields`, a duplicate email or display name `409 EMAIL_ALREADY_IN_USE` / `DISPLAY_NAME_ALREADY_IN_USE`, anything + unexpected `500 INTERNAL_ERROR` without details (`shared/api/GlobalExceptionController`). - **Roles**: one per user, a ladder `USER < AUDITOR < PLATFORM_ADMIN` (`users.role`, the first admin is set by SQL). The access token carries the active role in the `role` claim. **View as**: an admin switches the own login down to USER or AUDITOR and back (`POST /api/auth/view-as`); the choice is stored per login in @@ -68,7 +75,7 @@ To Be Played is split into three repositories: link, not in the landing stats. Email and display name stay taken; the Steam ID is freed, so an undo needs a new Steam link. A nightly job (04:15, Europe/Berlin) removes users flagged 30 days ago, in Core first and then in Auth. It calls Core's `/internal/accounts/*` with a - **service token** (EdDSA JWT, 60 s, `typ=service`). `app.account-cleanup.dry-run` (default `true`) only logs. + **service token** (EdDSA JWT, 60 s, `aud=tbp-internal`). `app.account-cleanup.dry-run` (default `true`) only logs. ## API diff --git a/src/main/java/auth_service/configuration/CacheConfig.java b/src/main/java/auth_service/configuration/CacheConfig.java index 9578be7..709bdd9 100644 --- a/src/main/java/auth_service/configuration/CacheConfig.java +++ b/src/main/java/auth_service/configuration/CacheConfig.java @@ -22,7 +22,7 @@ public class CacheConfig { @Bean public CacheManager cacheManager() { log.info("########### Productive cache active ###########"); - CaffeineCacheManager cacheManager = new CaffeineCacheManager("usersByEmail", "publicStats"); + CaffeineCacheManager cacheManager = new CaffeineCacheManager("users", "publicStats"); cacheManager.setCaffeine(caffeineCacheBuilder()); return cacheManager; } diff --git a/src/main/java/auth_service/configuration/DevCacheConfig.java b/src/main/java/auth_service/configuration/DevCacheConfig.java index 994577d..7645605 100644 --- a/src/main/java/auth_service/configuration/DevCacheConfig.java +++ b/src/main/java/auth_service/configuration/DevCacheConfig.java @@ -22,7 +22,7 @@ public class DevCacheConfig { @Bean public CacheManager cacheManager() { log.error("########### DEVELOPMENT CACHE ACTIVE ###########"); - CaffeineCacheManager cacheManager = new CaffeineCacheManager("usersByEmail", "publicStats"); + CaffeineCacheManager cacheManager = new CaffeineCacheManager("users", "publicStats"); cacheManager.setCaffeine(caffeineCacheBuilder()); return cacheManager; } diff --git a/src/main/java/auth_service/configuration/security/JwtService.java b/src/main/java/auth_service/configuration/security/JwtService.java index ebf2e98..7540c0a 100644 --- a/src/main/java/auth_service/configuration/security/JwtService.java +++ b/src/main/java/auth_service/configuration/security/JwtService.java @@ -23,6 +23,7 @@ import java.util.Base64; import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; /** @@ -32,8 +33,12 @@ import java.util.Optional; @Service public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceTokenIssuer, PublicKeyPublisher { + static final String ISSUER = "tbp-auth"; + static final String LOGIN_AUDIENCE = "tbp-login"; + static final String PROFILE_TICKET_AUDIENCE = "tbp-public-profile"; + static final String SERVICE_AUDIENCE = "tbp-internal"; private static final long PROFILE_TICKET_EXPIRATION_MS = 10 * 60 * 1000; - // Only non-login tokens carry this claim; isTokenValid rejects them. + // The kind of a token before item 26; item 29 removes it. private static final String TYPE_CLAIM = "typ"; private static final String PROFILE_TICKET_TYPE = "public-profile"; private static final long SERVICE_TOKEN_EXPIRATION_MS = 60 * 1000; @@ -64,10 +69,13 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT } @Override - public String generateToken(String userId, String email, Long steamId, Role role) { + public String generateToken(String userId, Long steamId, Role role) { var builder = Jwts.builder() .header().keyId(jwtConfig.getKid()).and() - .subject(email) + .issuer(ISSUER) + .audience().add(LOGIN_AUDIENCE).and() + .subject(userId) + // item 29 removes: a Core from before item 26 reads the user id only from this claim. .claim("userId", userId) .claim(ROLE_CLAIM, role.name()) .issuedAt(new Date(System.currentTimeMillis())) @@ -86,6 +94,8 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT public String generateProfileTicket(long userId, List profileStats, List sections) { return Jwts.builder() .header().keyId(jwtConfig.getKid()).and() + .issuer(ISSUER) + .audience().add(PROFILE_TICKET_AUDIENCE).and() .subject(PROFILE_TICKET_TYPE) .claim(TYPE_CLAIM, PROFILE_TICKET_TYPE) .claim("profileUserId", String.valueOf(userId)) @@ -101,6 +111,8 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT public String generateServiceToken() { return Jwts.builder() .header().keyId(jwtConfig.getKid()).and() + .issuer(ISSUER) + .audience().add(SERVICE_AUDIENCE).and() .subject("tbp-auth") .claim(TYPE_CLAIM, SERVICE_TOKEN_TYPE) .issuedAt(new Date(System.currentTimeMillis())) @@ -126,17 +138,36 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT public Optional parseLoginToken(String token) { try { Claims claims = parser.parseSignedClaims(token).getPayload(); - String userId = claims.get("userId", String.class); - if (claims.get(TYPE_CLAIM) != null || claims.getSubject() == null || userId == null) { + if (!isKind(claims, LOGIN_AUDIENCE, null)) { return Optional.empty(); } - return Optional.of(new UserPrincipal(userId, claims.getSubject(), claims.get("steamId", Long.class), + // item 29 removes: a token without aud comes from before item 26, with the user id in "userId". + String userId = hasAudience(claims) ? claims.getSubject() + : claims.getSubject() == null ? null : claims.get("userId", String.class); + if (userId == null) { + return Optional.empty(); + } + return Optional.of(new UserPrincipal(userId, claims.get("steamId", Long.class), Role.parse(claims.get(ROLE_CLAIM, String.class)))); } catch (Exception _) { return Optional.empty(); } } + private static boolean hasAudience(Claims claims) { + return claims.getAudience() != null && !claims.getAudience().isEmpty(); + } + + /** + * @param legacyType the {@code typ} of this kind before item 26, null for a login + */ + private static boolean isKind(Claims claims, String audience, String legacyType) { + if (hasAudience(claims)) { + return claims.getAudience().contains(audience) && ISSUER.equals(claims.getIssuer()); + } + return Objects.equals(legacyType, claims.get(TYPE_CLAIM, String.class)); + } + private static PrivateKey decodePrivateKey(String base64Pkcs8Der) { try { byte[] keyBytes = Base64.getDecoder().decode(base64Pkcs8Der); diff --git a/src/main/java/auth_service/modules/account/service/AccountService.java b/src/main/java/auth_service/modules/account/service/AccountService.java index 6e914bf..6d3a0e0 100644 --- a/src/main/java/auth_service/modules/account/service/AccountService.java +++ b/src/main/java/auth_service/modules/account/service/AccountService.java @@ -41,7 +41,7 @@ public class AccountService implements AccountDeleter { * Flags the user as deleted and ends all logins; the nightly cleanup removes the user 30 days later. Core * hides the user at once; when Core is down, the nightly cleanup tells it. */ - @CacheEvict(value = {"usersByEmail", "publicStats"}, allEntries = true) + @CacheEvict(value = {"users", "publicStats"}, allEntries = true) public void deleteAccount(long userId, String password) { String key = String.valueOf(userId); if (!attemptLimiter.tryAcquire(key, MAX_WRONG_PASSWORDS)) { @@ -62,7 +62,7 @@ public class AccountService implements AccountDeleter { @Override @RequiresRole(Role.PLATFORM_ADMIN) - @CacheEvict(value = {"usersByEmail", "publicStats"}, allEntries = true) + @CacheEvict(value = {"users", "publicStats"}, allEntries = true) public boolean deleteByAdmin(long userId) { return flagAndTellCore(userId); } 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 a70d599..153611e 100644 --- a/src/main/java/auth_service/modules/admin/service/AdminUserService.java +++ b/src/main/java/auth_service/modules/admin/service/AdminUserService.java @@ -39,7 +39,7 @@ public class AdminUserService { } @Transactional - @CacheEvict(value = {"usersByEmail", "publicStats"}, allEntries = true) + @CacheEvict(value = {"users", "publicStats"}, allEntries = true) public void setBlocked(long actorUserId, long targetUserId, boolean blocked) { TargetState target = checkedTarget(actorUserId, targetUserId); if (target.deleted()) { 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 e7bbd49..8e1cda0 100644 --- a/src/main/java/auth_service/modules/login/service/LoginService.java +++ b/src/main/java/auth_service/modules/login/service/LoginService.java @@ -9,7 +9,8 @@ import auth_service.modules.login.model.User; import auth_service.modules.login.persistence.MinimalUserRepository; import lombok.extern.slf4j.Slf4j; import org.springframework.security.crypto.password.PasswordEncoder; -import org.springframework.cache.annotation.CacheEvict; +import org.springframework.cache.Cache; +import org.springframework.cache.CacheManager; import org.springframework.stereotype.Service; import java.util.Optional; @@ -24,18 +25,20 @@ public class LoginService { private final TokenGenerator tokenGenerator; private final LoginAttemptService loginAttemptService; private final RefreshTokenService refreshTokenService; + private final CacheManager cacheManager; // Checked against for unknown emails, so they cost the same bcrypt time as a wrong password. private final String dummyHash; public LoginService(MinimalUserRepository minimalUserRepository, PasswordEncoder passwordEncoder, TokenGenerator tokenGenerator, LoginAttemptService loginAttemptService, - RefreshTokenService refreshTokenService) { + RefreshTokenService refreshTokenService, CacheManager cacheManager) { this.minimalUserRepository = minimalUserRepository; this.passwordEncoder = passwordEncoder; this.tokenGenerator = tokenGenerator; this.loginAttemptService = loginAttemptService; this.refreshTokenService = refreshTokenService; + this.cacheManager = cacheManager; this.dummyHash = passwordEncoder.encode(UUID.randomUUID().toString()); } @@ -46,8 +49,6 @@ public class LoginService { * @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. */ - // /api/users/me caches the user by email; a role set by SQL must show after the next login. - @CacheEvict(value = "usersByEmail", key = "#userToLogin.email") public Session login(User userToLogin, String clientIp, String previousRefreshToken) { String email = userToLogin.getEmail(); loginAttemptService.acquire(email, clientIp); @@ -69,9 +70,13 @@ public class LoginService { User user = foundUser.get(); loginAttemptService.recordSuccess(email, clientIp); minimalUserRepository.updateLoginDate(user.getId()); + // /api/users/me caches the user; a role set by SQL must show after the next login. + Cache users = cacheManager.getCache("users"); + if (users != null) { + users.evict(user.getId()); + } refreshTokenService.revoke(previousRefreshToken); - String accessToken = tokenGenerator.generateToken(String.valueOf(user.getId()), user.getEmail(), user.getSteamId(), - user.getRole()); + String accessToken = tokenGenerator.generateToken(String.valueOf(user.getId()), user.getSteamId(), user.getRole()); return new Session(accessToken, refreshTokenService.startFamily(user.getId())); } } diff --git a/src/main/java/auth_service/modules/login/service/RefreshTokenService.java b/src/main/java/auth_service/modules/login/service/RefreshTokenService.java index bccedd9..131674f 100644 --- a/src/main/java/auth_service/modules/login/service/RefreshTokenService.java +++ b/src/main/java/auth_service/modules/login/service/RefreshTokenService.java @@ -110,8 +110,7 @@ public class RefreshTokenService { if (switching && stored != Role.USER) { adminAuditLog.record(current.getId(), "VIEW_AS", null, requested == null ? "OWN_ROLE" : requested.name()); } - String accessToken = tokenGenerator.generateToken( - String.valueOf(current.getId()), current.getEmail(), current.getSteamId(), active); + String accessToken = tokenGenerator.generateToken(String.valueOf(current.getId()), current.getSteamId(), active); return Optional.of(new Session(accessToken, next)); } 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 0eba0ec..43b33d6 100644 --- a/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java +++ b/src/main/java/auth_service/modules/registration/api/RegistrationExceptionController.java @@ -37,7 +37,7 @@ public class RegistrationExceptionController { @ExceptionHandler(EmailAlreadyInUseException.class) public ResponseEntity clientError(EmailAlreadyInUseException ex) { var body = new ErrorResponse("EMAIL_ALREADY_IN_USE", ex.getMessage()); - return response(body, HttpStatus.BAD_REQUEST); + return response(body, HttpStatus.CONFLICT); } @ExceptionHandler(InvalidInvitationCodeException.class) @@ -49,7 +49,7 @@ public class RegistrationExceptionController { @ExceptionHandler(DisplayNameAlreadyRegisteredException.class) public ResponseEntity clientError(DisplayNameAlreadyRegisteredException ex) { var body = new ErrorResponse("DISPLAY_NAME_ALREADY_IN_USE", ex.getMessage()); - return response(body, HttpStatus.BAD_REQUEST); + return response(body, HttpStatus.CONFLICT); } diff --git a/src/main/java/auth_service/modules/user/api/SteamLinkController.java b/src/main/java/auth_service/modules/user/api/SteamLinkController.java index 76b08c1..b5e46df 100644 --- a/src/main/java/auth_service/modules/user/api/SteamLinkController.java +++ b/src/main/java/auth_service/modules/user/api/SteamLinkController.java @@ -44,7 +44,7 @@ public class SteamLinkController { */ @DeleteMapping(value = "/api/users/steam-link", version = "1.0") public ResponseEntity unlink(@AuthenticationPrincipal UserPrincipal principal) { - String token = userService.unlinkSteamId(principal.getIdAsLong(), principal.email(), principal.role()); + String token = userService.unlinkSteamId(principal.getIdAsLong(), principal.role()); return ResponseEntity.noContent() .header(HttpHeaders.SET_COOKIE, authCookies.access(token).toString()) .build(); diff --git a/src/main/java/auth_service/modules/user/api/UserController.java b/src/main/java/auth_service/modules/user/api/UserController.java index 0aeb1f7..1107c95 100644 --- a/src/main/java/auth_service/modules/user/api/UserController.java +++ b/src/main/java/auth_service/modules/user/api/UserController.java @@ -35,9 +35,7 @@ public class UserController { @Timed(value = "user.me.v1.requests.time", description = "Time taken to process v1 user profile requests") public ResponseEntity getCurrentUserV1(@AuthenticationPrincipal UserPrincipal principal) { - // this is the email from the login, if a user is not authenticated, it will not even come to this point. - var authenticatedEmail = principal.email(); - var profileResponse = this.userService.getUserByEmail(authenticatedEmail); + var profileResponse = this.userService.getUser(principal.getIdAsLong()); return ResponseEntity.ok(ApiMapper.toApi(profileResponse, principal.role())); } diff --git a/src/main/java/auth_service/modules/user/api/UserExceptionController.java b/src/main/java/auth_service/modules/user/api/UserExceptionController.java index 393dd5b..0ff34ec 100644 --- a/src/main/java/auth_service/modules/user/api/UserExceptionController.java +++ b/src/main/java/auth_service/modules/user/api/UserExceptionController.java @@ -4,9 +4,7 @@ import auth_service.shared.api.dto.response.ErrorResponse; import auth_service.modules.user.exception.DisplayNameAlreadyInUseException; import auth_service.modules.user.exception.UserNotFoundException; import lombok.NonNull; -import lombok.extern.slf4j.Slf4j; import org.springframework.core.annotation.Order; -import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -14,9 +12,7 @@ import org.springframework.web.bind.annotation.RestControllerAdvice; @RestControllerAdvice(assignableTypes = UserController.class) @Order(value = 1) -@Slf4j public class UserExceptionController { - private static final String DISPLAY_NAME_CONSTRAINT = "uq_users_display_name_lower"; private static @NonNull ResponseEntity response(ErrorResponse body, HttpStatus status) { return ResponseEntity.status(status.value()) @@ -37,23 +33,8 @@ public class UserExceptionController { return response(body, HttpStatus.NOT_FOUND); } - // A parallel request can save the same display name after the service check. Then the DB constraint fails. - @ExceptionHandler(DataIntegrityViolationException.class) - public ResponseEntity dataIntegrityError(DataIntegrityViolationException ex) { - var cause = ex.getMostSpecificCause().getMessage(); - if (cause != null && cause.contains(DISPLAY_NAME_CONSTRAINT)) { - return displayNameInUse(); - } - log.error("Data integrity violation in user request", ex); - return response(new ErrorResponse("INTERNAL_ERROR", "Unexpected database error."), HttpStatus.INTERNAL_SERVER_ERROR); - } - @ExceptionHandler(DisplayNameAlreadyInUseException.class) public ResponseEntity clientError(DisplayNameAlreadyInUseException ex) { - return displayNameInUse(); - } - - private static ResponseEntity displayNameInUse() { var body = new ErrorResponse("DISPLAY_NAME_ALREADY_IN_USE", "The display name is already in use."); return response(body, HttpStatus.CONFLICT); } diff --git a/src/main/java/auth_service/modules/user/persistence/UserRepository.java b/src/main/java/auth_service/modules/user/persistence/UserRepository.java index e5e8746..2bda8da 100644 --- a/src/main/java/auth_service/modules/user/persistence/UserRepository.java +++ b/src/main/java/auth_service/modules/user/persistence/UserRepository.java @@ -18,12 +18,9 @@ import java.util.Optional; */ public interface UserRepository { /** - * Finds a user by their email address. - * * @param email The email to search for - * - * @return An Optional containing the user if found + * The user with the settings. */ - Optional findByEmail(String email); + Optional findById(long userId); @NotNull UserSettings findSettingsByUserId(String userId); diff --git a/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java b/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java index dafe6a5..2d2d957 100644 --- a/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java +++ b/src/main/java/auth_service/modules/user/persistence/UserRepositoryImpl.java @@ -1,6 +1,5 @@ package auth_service.modules.user.persistence; -import auth_service.shared.account.EmailAddress; import auth_service.modules.user.exception.UserNotFoundException; import auth_service.modules.user.model.Appearance; import auth_service.modules.user.model.ProfileStyle; @@ -28,7 +27,7 @@ public class UserRepositoryImpl implements UserRepository { private EntityManager entityManager; @Override - public Optional findByEmail(String email) { + public Optional findById(long userId) { var criteriaBuilder = entityManager.getCriteriaBuilder(); var userEntityCriteriaQuery = criteriaBuilder.createQuery(UserEntity.class); var userEntity = userEntityCriteriaQuery.from(UserEntity.class); @@ -37,8 +36,7 @@ public class UserRepositoryImpl implements UserRepository { // A LEFT JOIN ensures the user is returned even if no settings exist yet. userEntity.fetch("settings", jakarta.persistence.criteria.JoinType.LEFT); - userEntityCriteriaQuery.select(userEntity).where(criteriaBuilder.equal( - userEntity.get("email"), EmailAddress.normalize(email))); + userEntityCriteriaQuery.select(userEntity).where(criteriaBuilder.equal(userEntity.get("id"), userId)); TypedQuery typedQuery = entityManager.createQuery(userEntityCriteriaQuery); diff --git a/src/main/java/auth_service/modules/user/service/UserService.java b/src/main/java/auth_service/modules/user/service/UserService.java index fbde765..ffd246e 100644 --- a/src/main/java/auth_service/modules/user/service/UserService.java +++ b/src/main/java/auth_service/modules/user/service/UserService.java @@ -31,16 +31,10 @@ public class UserService { private final UserRepository userRepository; private final TokenGenerator tokenGenerator; - /** - * Fetches the user by email. Results are cached to ensure blazing-fast UI rendering. - * - * @param email The authenticated user's email - * @return Optional containing the User model if found - */ @RequiresRole(Role.USER) - @Cacheable(value = "usersByEmail", key = "#email") - public User getUserByEmail(String email) { - return userRepository.findByEmail(email).orElseThrow(() -> new UserNotFoundException("Could not find user with email " + email)); + @Cacheable(value = "users", key = "#userId") + public User getUser(long userId) { + return userRepository.findById(userId).orElseThrow(() -> new UserNotFoundException("Could not find user " + userId)); } /** @@ -48,7 +42,7 @@ public class UserService { */ @RequiresRole(Role.USER) @Transactional - @CacheEvict(value = "usersByEmail", allEntries = true) + @CacheEvict(value = "users", key = "#userId") public void updateUserSettings(long userId, String displayName, UserSettings userSettings) { if (displayName != null && userRepository.isDisplayNameUsedByOtherUser(userId, displayName)) { throw new DisplayNameAlreadyInUseException("The display name is already in use."); @@ -63,7 +57,8 @@ public class UserService { * the ArchitectureTest. */ @Transactional - @CacheEvict(value = "usersByEmail", allEntries = true) + // All: the user who had typed the same Steam ID in loses it. + @CacheEvict(value = "users", allEntries = true) public SteamLinkOutcome linkVerifiedSteamId(long userId, long steamId) { Optional holder = userRepository.findSteamIdHolder(steamId); if (holder.isPresent() && holder.get().userId() != userId) { @@ -84,44 +79,44 @@ public class UserService { */ @RequiresRole(Role.USER) @Transactional - @CacheEvict(value = "usersByEmail", allEntries = true) - public String unlinkSteamId(long userId, String email, Role role) { + @CacheEvict(value = "users", key = "#userId") + public String unlinkSteamId(long userId, Role role) { userRepository.clearSteamId(userId); log.info("Steam ID unlinked for user {}", userId); - return tokenGenerator.generateToken(String.valueOf(userId), email, null, role); + return tokenGenerator.generateToken(String.valueOf(userId), null, role); } @RequiresRole(Role.USER) @Transactional - @CacheEvict(value = "usersByEmail", allEntries = true) + @CacheEvict(value = "users", key = "#userId") public void updateAppearance(long userId, Appearance appearance) { userRepository.updateAppearance(userId, appearance); } @RequiresRole(Role.USER) @Transactional - @CacheEvict(value = "usersByEmail", allEntries = true) + @CacheEvict(value = "users", key = "#userId") public void updateDashboardTiles(long userId, List tiles) { userRepository.updateDashboardTiles(userId, tiles == null ? null : Tile.distinct(tiles)); } @RequiresRole(Role.USER) @Transactional - @CacheEvict(value = "usersByEmail", allEntries = true) + @CacheEvict(value = "users", key = "#userId") public void updateProfileTiles(long userId, List tiles) { userRepository.updateProfileTiles(userId, Tile.distinct(tiles)); } @RequiresRole(Role.USER) @Transactional - @CacheEvict(value = "usersByEmail", allEntries = true) + @CacheEvict(value = "users", key = "#userId") public void updateFavoritePlatform(long userId, String platform) { userRepository.updateFavoritePlatform(userId, platform == null ? null : platform.strip()); } @RequiresRole(Role.USER) @Transactional - @CacheEvict(value = "usersByEmail", allEntries = true) + @CacheEvict(value = "users", key = "#userId") public void updateProfileStyle(long userId, ProfileStyle style) { userRepository.updateProfileStyle(userId, style); } diff --git a/src/main/java/auth_service/shared/api/GlobalExceptionController.java b/src/main/java/auth_service/shared/api/GlobalExceptionController.java new file mode 100644 index 0000000..2581db7 --- /dev/null +++ b/src/main/java/auth_service/shared/api/GlobalExceptionController.java @@ -0,0 +1,171 @@ +package auth_service.shared.api; + +import auth_service.shared.api.dto.response.ErrorResponse; + +import lombok.extern.slf4j.Slf4j; +import org.hibernate.exception.ConstraintViolationException; +import org.springframework.core.MethodParameter; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.ResponseEntity; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.core.AuthenticationException; +import org.springframework.validation.method.ParameterValidationResult; +import org.springframework.validation.FieldError; +import org.springframework.web.bind.MethodArgumentNotValidException; +import org.springframework.web.bind.MissingServletRequestParameterException; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.context.request.WebRequest; +import org.springframework.web.method.annotation.HandlerMethodValidationException; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; +import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + +import java.sql.SQLException; +import java.util.List; + +/** + * Gives every error the {@link ErrorResponse} body. Spring's own exceptions keep their status; a 400 is + * {@code VALIDATION_FAILED}. A module's own exceptions belong in its own handler. + */ +@RestControllerAdvice +@Order(Ordered.LOWEST_PRECEDENCE) +@Slf4j +public class GlobalExceptionController extends ResponseEntityExceptionHandler { + + static final String VALIDATION_FAILED = "VALIDATION_FAILED"; + private static final String INTERNAL_ERROR = "INTERNAL_ERROR"; + // Postgres unique_violation. The state decides: the message text follows the server's language. + private static final String UNIQUE_VIOLATION = "23505"; + private static final String EMAIL_CONSTRAINT = "users_email_key"; + private static final String EMAIL_LOWER_CONSTRAINT = "users_email_lower"; + private static final String DISPLAY_NAME_CONSTRAINT = "uq_users_display_name_lower"; + + @Override + protected ResponseEntity handleMethodArgumentNotValid(MethodArgumentNotValidException ex, + HttpHeaders headers, HttpStatusCode status, + WebRequest request) { + return validationFailed(ex.getBindingResult().getFieldErrors().stream() + .map(error -> new ErrorResponse.FieldError(error.getField(), error.getDefaultMessage())) + .toList()); + } + + @Override + protected ResponseEntity handleHandlerMethodValidationException(HandlerMethodValidationException ex, + HttpHeaders headers, + HttpStatusCode status, + WebRequest request) { + return validationFailed(ex.getParameterValidationResults().stream() + .flatMap(result -> result.getResolvableErrors().stream() + .map(error -> new ErrorResponse.FieldError( + error instanceof FieldError field ? field.getField() : parameterName(result), + error.getDefaultMessage()))) + .toList()); + } + + /** + * The base keeps its checks (a committed response, the error attribute of a 500); only the body is ours. + */ + @Override + protected ResponseEntity handleExceptionInternal(Exception ex, Object body, HttpHeaders headers, + HttpStatusCode statusCode, WebRequest request) { + ResponseEntity answer = super.handleExceptionInternal(ex, body, headers, statusCode, request); + if (answer == null) { + return null; + } + ErrorResponse error; + if (statusCode.is5xxServerError()) { + log.error("Unexpected error", ex); + error = new ErrorResponse(INTERNAL_ERROR, "Unexpected error"); + } else if (statusCode.value() == HttpStatus.BAD_REQUEST.value()) { + String field = ex instanceof MethodArgumentTypeMismatchException mismatch ? mismatch.getName() + : ex instanceof MissingServletRequestParameterException missing ? missing.getParameterName() : null; + error = new ErrorResponse(VALIDATION_FAILED, "The request is not valid", + field == null ? null : List.of(new ErrorResponse.FieldError(field, "Missing or not readable"))); + } else { + HttpStatus status = HttpStatus.resolve(statusCode.value()); + error = new ErrorResponse(status != null ? status.name() : "ERROR", + status != null ? status.getReasonPhrase() : "Error"); + } + return ResponseEntity.status(answer.getStatusCode()).headers(answer.getHeaders()).body(error); + } + + /** + * A parallel request can store the same email or display name after the service checked it. Then the unique + * index refuses the write. Any other refused write (a value too long, a missing row) is a bug and stays a 500. + */ + @ExceptionHandler(DataIntegrityViolationException.class) + public ResponseEntity dataIntegrity(DataIntegrityViolationException ex) throws Exception { + if (!isUniqueViolation(ex)) { + return unexpected(ex); + } + ErrorResponse body; + if (violates(ex, EMAIL_CONSTRAINT) || violates(ex, EMAIL_LOWER_CONSTRAINT)) { + body = new ErrorResponse("EMAIL_ALREADY_IN_USE", "The email is already in use."); + } else if (violates(ex, DISPLAY_NAME_CONSTRAINT)) { + body = new ErrorResponse("DISPLAY_NAME_ALREADY_IN_USE", "The display name is already in use."); + } else { + log.warn("Database refused a write: {}", ex.getMostSpecificCause().getMessage()); + body = new ErrorResponse("CONFLICT", "The data changed at the same time; try again"); + } + return ResponseEntity.status(HttpStatus.CONFLICT).body(body); + } + + /** + * Rethrows Spring Security's exceptions: its filters answer them with 401 and 403. + */ + @ExceptionHandler(Exception.class) + public ResponseEntity unexpected(Exception ex) throws Exception { + if (ex instanceof AccessDeniedException || ex instanceof AuthenticationException) { + throw ex; + } + log.error("Unexpected error", ex); + return ResponseEntity.internalServerError().body(new ErrorResponse(INTERNAL_ERROR, "Unexpected error")); + } + + /** + * Hibernate reads the name only from an English Postgres message. The name itself is in the message in every + * language, so the message is searched when Hibernate found none. + */ + private static boolean violates(DataIntegrityViolationException ex, String constraint) { + for (Throwable cause = ex; cause != null; cause = cause.getCause()) { + if (cause instanceof ConstraintViolationException violation && constraint.equals(violation.getConstraintName())) { + return true; + } + } + return String.valueOf(ex.getMostSpecificCause().getMessage()).contains(constraint); + } + + private static boolean isUniqueViolation(Throwable ex) { + for (Throwable cause = ex; cause != null; cause = cause.getCause()) { + if (cause instanceof SQLException sql && UNIQUE_VIOLATION.equals(sql.getSQLState())) { + return true; + } + } + return false; + } + + private static ResponseEntity validationFailed(List fields) { + return ResponseEntity.badRequest().body(new ErrorResponse(VALIDATION_FAILED, "The request is not valid", fields)); + } + + // The name in the request, e.g. "q" of @RequestParam("q") String query. + private static String parameterName(ParameterValidationResult result) { + MethodParameter parameter = result.getMethodParameter(); + RequestParam requestParam = parameter.getParameterAnnotation(RequestParam.class); + if (requestParam != null && !requestParam.name().isEmpty()) { + return requestParam.name(); + } + PathVariable pathVariable = parameter.getParameterAnnotation(PathVariable.class); + if (pathVariable != null && !pathVariable.name().isEmpty()) { + return pathVariable.name(); + } + return parameter.getParameterName(); + } +} diff --git a/src/main/java/auth_service/shared/api/dto/response/ErrorResponse.java b/src/main/java/auth_service/shared/api/dto/response/ErrorResponse.java index 1de58e6..9f1a867 100644 --- a/src/main/java/auth_service/shared/api/dto/response/ErrorResponse.java +++ b/src/main/java/auth_service/shared/api/dto/response/ErrorResponse.java @@ -1,13 +1,24 @@ package auth_service.shared.api.dto.response; +import com.fasterxml.jackson.annotation.JsonInclude; import org.jspecify.annotations.NonNull; +import java.util.List; + /** * Shared error response record for all modules. * * @param code A machine-readable error code. * @param message A human-readable error message. + * @param fields the refused request fields of a {@code VALIDATION_FAILED}, else left out */ -public record ErrorResponse(@NonNull String code, @NonNull String message) { -} +@JsonInclude(JsonInclude.Include.NON_NULL) +public record ErrorResponse(@NonNull String code, @NonNull String message, List fields) { + public ErrorResponse(@NonNull String code, @NonNull String message) { + this(code, message, null); + } + + public record FieldError(String field, String message) { + } +} diff --git a/src/main/java/auth_service/shared/security/TokenGenerator.java b/src/main/java/auth_service/shared/security/TokenGenerator.java index 911dcc7..d0f7aa0 100644 --- a/src/main/java/auth_service/shared/security/TokenGenerator.java +++ b/src/main/java/auth_service/shared/security/TokenGenerator.java @@ -9,11 +9,10 @@ public interface TokenGenerator { * Generates a token for the given user. * * @param userId The unique identifier of the user. - * @param email The email address of the user. * @param steamId The Steam ID of the user. * @param role The active role of the login. * @return The generated token string. */ - String generateToken(String userId, String email, Long steamId, Role role); + String generateToken(String userId, Long steamId, Role role); } diff --git a/src/main/java/auth_service/shared/security/UserPrincipal.java b/src/main/java/auth_service/shared/security/UserPrincipal.java index 5cf5327..1e13d4d 100644 --- a/src/main/java/auth_service/shared/security/UserPrincipal.java +++ b/src/main/java/auth_service/shared/security/UserPrincipal.java @@ -7,13 +7,12 @@ import java.security.Principal; */ public record UserPrincipal( String id, - String email, Long steamId, Role role ) implements Principal { - public UserPrincipal(String id, String email, Long steamId) { - this(id, email, steamId, Role.USER); + public UserPrincipal(String id, Long steamId) { + this(id, steamId, Role.USER); } @Override diff --git a/src/test/java/auth_service/configuration/security/JwtServiceTest.java b/src/test/java/auth_service/configuration/security/JwtServiceTest.java index a2acd1d..915839d 100644 --- a/src/test/java/auth_service/configuration/security/JwtServiceTest.java +++ b/src/test/java/auth_service/configuration/security/JwtServiceTest.java @@ -4,11 +4,14 @@ import auth_service.shared.security.Role; import auth_service.shared.security.SessionProperties; import auth_service.shared.security.UserPrincipal; import auth_service.shared.test.AbstractUnitTest; +import auth_service.shared.test.ContractFixture; import io.jsonwebtoken.Claims; import io.jsonwebtoken.Jwts; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.node.ObjectNode; import java.security.KeyFactory; import java.security.KeyPair; @@ -66,7 +69,7 @@ class JwtServiceTest extends AbstractUnitTest { @Test void generateToken_Success() { - var token = jwtService.generateToken("0815", "test@example.com", null, Role.USER); + var token = jwtService.generateToken("0815", null, Role.USER); assertNotNull(token); assertFalse(token.isEmpty()); @@ -74,7 +77,7 @@ class JwtServiceTest extends AbstractUnitTest { @Test void generateToken_setsKidHeader() { - var token = jwtService.generateToken("0815", "test@example.com", null, Role.USER); + var token = jwtService.generateToken("0815", null, Role.USER); var header = Jwts.parser() .verifyWith(publicKey) @@ -88,9 +91,9 @@ class JwtServiceTest extends AbstractUnitTest { @Test void parseLoginToken_ReturnsTheUser() { - var token = jwtService.generateToken("0815", "test@example.com", 123456789L, Role.USER); + var token = jwtService.generateToken("0815", 123456789L, Role.USER); - assertEquals(Optional.of(new UserPrincipal("0815", "test@example.com", 123456789L)), + assertEquals(Optional.of(new UserPrincipal("0815", 123456789L)), jwtService.parseLoginToken(token)); } @@ -101,7 +104,7 @@ class JwtServiceTest extends AbstractUnitTest { @Test void generateToken_ExpiresAfterTheAccessTokenTtl() { - var token = jwtService.generateToken("0815", "test@example.com", null, Role.USER); + var token = jwtService.generateToken("0815", null, Role.USER); var claims = Jwts.parser().verifyWith(publicKey).build().parseSignedClaims(token).getPayload(); long secondsLeft = (claims.getExpiration().getTime() - System.currentTimeMillis()) / 1000; @@ -162,7 +165,7 @@ class JwtServiceTest extends AbstractUnitTest { System.arraycopy(prefix, 0, der, 0, prefix.length); System.arraycopy(raw, 0, der, prefix.length, raw.length); PublicKey fromJwk = KeyFactory.getInstance("Ed25519").generatePublic(new X509EncodedKeySpec(der)); - String token = jwtService.generateToken("7", "a@b.de", null, Role.USER); + String token = jwtService.generateToken("7", null, Role.USER); assertEquals("7", Jwts.parser().verifyWith(fromJwk).build().parseSignedClaims(token).getPayload().get("userId")); } @@ -181,7 +184,7 @@ class JwtServiceTest extends AbstractUnitTest { @Test void parseLoginToken_ReadsTheRole() { - String token = jwtService.generateToken("7", "a@b.de", null, Role.AUDITOR); + String token = jwtService.generateToken("7", null, Role.AUDITOR); assertEquals(Role.AUDITOR, jwtService.parseLoginToken(token).orElseThrow().role()); } @@ -194,4 +197,52 @@ class JwtServiceTest extends AbstractUnitTest { assertEquals(Role.USER, jwtService.parseLoginToken(old).orElseThrow().role()); } + + // The claims as the other side reads them: iat and exp change per token. + private static JsonNode payload(String token) { + ObjectNode payload = (ObjectNode) ContractFixture.MAPPER.readTree(Base64.getUrlDecoder().decode(token.split("\\.")[1])); + payload.remove(List.of("iat", "exp")); + return payload; + } + + @SuppressWarnings("unchecked") + private String sign(JsonNode claims) { + return Jwts.builder().header().keyId(KID).and() + .claims(ContractFixture.MAPPER.treeToValue(claims, Map.class)) + .expiration(new Date(System.currentTimeMillis() + 60_000)) + .signWith(privateKey, Jwts.SIG.EdDSA).compact(); + } + + @Test + void tokens_MatchTheContractWithCore() { + assertEquals(ContractFixture.token("login"), + payload(jwtService.generateToken("42", 76561198000000001L, Role.USER))); + assertEquals(ContractFixture.token("publicProfile"), + payload(jwtService.generateProfileTicket(42L, List.of("OVERVIEW"), List.of("SUMMARY")))); + assertEquals(ContractFixture.token("service"), payload(jwtService.generateServiceToken())); + } + + @Test + void parseLoginToken_ALoginOfAnAuthFromBeforeItem26_IsAccepted() { + assertEquals("42", jwtService.parseLoginToken(sign(ContractFixture.legacyToken("login"))).orElseThrow().id()); + } + + @Test + void parseLoginToken_ALoginWithoutTheOldUserIdClaim_IsAccepted() { + ObjectNode login = (ObjectNode) ContractFixture.token("login").deepCopy(); + login.remove("userId"); + + assertEquals("42", jwtService.parseLoginToken(sign(login)).orElseThrow().id()); + } + + @Test + void parseLoginToken_ATokenForAnotherAudience_IsRejected() { + ObjectNode ticketAudience = (ObjectNode) ContractFixture.token("login").deepCopy(); + ticketAudience.putArray("aud").add(JwtService.PROFILE_TICKET_AUDIENCE); + ObjectNode otherIssuer = (ObjectNode) ContractFixture.token("login").deepCopy(); + otherIssuer.put("iss", "someone-else"); + + assertEquals(Optional.empty(), jwtService.parseLoginToken(sign(ticketAudience))); + assertEquals(Optional.empty(), jwtService.parseLoginToken(sign(otherIssuer))); + } } diff --git a/src/test/java/auth_service/configuration/security/RoleCheckFlowTest.java b/src/test/java/auth_service/configuration/security/RoleCheckFlowTest.java index e5cdfdd..128a923 100644 --- a/src/test/java/auth_service/configuration/security/RoleCheckFlowTest.java +++ b/src/test/java/auth_service/configuration/security/RoleCheckFlowTest.java @@ -93,7 +93,7 @@ class RoleCheckFlowTest extends AbstractIntegrationTest { private TokenGenerator tokenGenerator; private String token(Role role) { - return tokenGenerator.generateToken("4711", "probe@example.com", null, role); + return tokenGenerator.generateToken("4711", null, role); } private ResultActions call(String path, Role role) throws Exception { diff --git a/src/test/java/auth_service/modules/account/service/CoreAccountServiceTest.java b/src/test/java/auth_service/modules/account/service/CoreAccountServiceTest.java index 5ccab37..9a4724f 100644 --- a/src/test/java/auth_service/modules/account/service/CoreAccountServiceTest.java +++ b/src/test/java/auth_service/modules/account/service/CoreAccountServiceTest.java @@ -3,6 +3,7 @@ package auth_service.modules.account.service; import auth_service.modules.account.model.CoreCleanupResult; import auth_service.shared.security.ServiceTokenIssuer; import auth_service.shared.test.AbstractUnitTest; +import auth_service.shared.test.ContractFixture; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.http.HttpMethod; @@ -10,6 +11,7 @@ import org.springframework.http.MediaType; import org.springframework.test.web.client.MockRestServiceServer; import org.springframework.web.client.RestClient; import org.springframework.web.client.RestClientException; +import tools.jackson.databind.JsonNode; import java.util.List; import java.util.Map; @@ -39,15 +41,23 @@ class CoreAccountServiceTest extends AbstractUnitTest { service = new CoreAccountService(builder.build(), TOKENS); } - @Test - void markAndReplace_SendTheUserIdsWithTheServiceToken() { - core.expect(requestTo(CORE + "/internal/accounts/deleted")).andExpect(method(HttpMethod.POST)) + private void expect(String name) { + JsonNode call = ContractFixture.internalCall(name); + var expectation = core.expect(requestTo(CORE + call.path("path").asString())) + .andExpect(method(HttpMethod.valueOf(call.path("method").asString()))) .andExpect(header("Authorization", "Bearer service-token")) - .andExpect(content().json("{\"userIds\":[42]}")) - .andRespond(withNoContent()); - core.expect(requestTo(CORE + "/internal/accounts/deleted")).andExpect(method(HttpMethod.PUT)) - .andExpect(content().json("{\"userIds\":[]}")) - .andRespond(withNoContent()); + .andExpect(content().json(call.path("request").toString())); + if (call.has("response")) { + expectation.andRespond(withSuccess(call.path("response").toString(), MediaType.APPLICATION_JSON)); + } else { + expectation.andRespond(withNoContent()); + } + } + + @Test + void markAndReplace_SendTheContractBodiesWithTheServiceToken() { + expect("markDeleted"); + expect("replaceDeleted"); service.markDeleted(List.of(42L)); service.replaceDeleted(List.of()); @@ -56,12 +66,9 @@ class CoreAccountServiceTest extends AbstractUnitTest { } @Test - void purgeAndPrune_ReadTheResult() { - String result = "{\"dryRun\":true,\"users\":[42],\"rows\":{\"user_game\":3},\"refused\":false}"; - core.expect(requestTo(CORE + "/internal/accounts/purge")).andExpect(content().json("{\"userIds\":[42],\"dryRun\":false}")) - .andRespond(withSuccess(result, MediaType.APPLICATION_JSON)); - core.expect(requestTo(CORE + "/internal/accounts/prune")).andExpect(content().json("{\"knownUserIds\":[1,42],\"dryRun\":true}")) - .andRespond(withSuccess(result, MediaType.APPLICATION_JSON)); + void purgeAndPrune_ReadTheContractResult() { + expect("purge"); + expect("prune"); CoreCleanupResult expected = new CoreCleanupResult(true, List.of(42L), Map.of("user_game", 3), false); assertThat(service.purge(List.of(42L), false)).isEqualTo(expected); 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 a284084..c75756a 100644 --- a/src/test/java/auth_service/modules/login/service/LoginServiceTest.java +++ b/src/test/java/auth_service/modules/login/service/LoginServiceTest.java @@ -10,6 +10,8 @@ import auth_service.modules.login.model.User; import auth_service.modules.login.persistence.MinimalUserRepository; import auth_service.shared.test.AbstractUnitTest; import org.junit.jupiter.api.BeforeEach; +import org.springframework.cache.CacheManager; +import org.springframework.cache.concurrent.ConcurrentMapCacheManager; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.springframework.security.crypto.password.PasswordEncoder; @@ -17,6 +19,8 @@ import org.springframework.security.crypto.password.PasswordEncoder; import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -55,11 +59,13 @@ class LoginServiceTest extends AbstractUnitTest { private User user; + private final CacheManager cacheManager = new ConcurrentMapCacheManager("users"); + @BeforeEach void setUp() { lenient().when(passwordEncoder.encode(anyString())).thenReturn("dummy-hash"); loginService = new LoginService(minimalUserRepository, passwordEncoder, tokenGenerator, loginAttemptService, - refreshTokenService); + refreshTokenService, cacheManager); user = User.builder() .email("test@example.com") .password("password") @@ -77,7 +83,7 @@ class LoginServiceTest extends AbstractUnitTest { .build(); when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(true); - when(tokenGenerator.generateToken("815", "test@example.com", null, Role.USER)).thenReturn("mock-token"); + when(tokenGenerator.generateToken("815", null, Role.USER)).thenReturn("mock-token"); when(refreshTokenService.startFamily(815L)).thenReturn("mock-refresh"); // Act @@ -89,6 +95,20 @@ class LoginServiceTest extends AbstractUnitTest { verify(loginAttemptService).recordSuccess("test@example.com", IP); } + @Test + void login_Success_DropsOnlyThisUsersCachedProfile() { + var dbUser = User.builder().id(815L).email("test@example.com").password("encodedPassword").enabled(true).build(); + when(minimalUserRepository.findByEmail(user.getEmail())).thenReturn(Optional.of(dbUser)); + when(passwordEncoder.matches(user.getPassword(), dbUser.getPassword())).thenReturn(true); + cacheManager.getCache("users").put(815L, "old profile"); + cacheManager.getCache("users").put(999L, "other user"); + + loginService.login(user, IP, null); + + assertNull(cacheManager.getCache("users").get(815L)); + assertNotNull(cacheManager.getCache("users").get(999L)); + } + @Test void login_Success_RevokesTheRefreshTokenThisBrowserStillSends() { var dbUser = User.builder().id(815L).email("test@example.com").password("encodedPassword").enabled(true).build(); @@ -155,7 +175,7 @@ class LoginServiceTest extends AbstractUnitTest { // Act & Assert assertThrows(AccountBlockedException.class, () -> loginService.login(user, IP, null)); - verify(tokenGenerator, never()).generateToken(any(), any(), any(), any()); + verify(tokenGenerator, never()).generateToken(any(), any(), any()); verify(loginAttemptService).recordSuccess("test@example.com", IP); } @@ -177,7 +197,7 @@ class LoginServiceTest extends AbstractUnitTest { loginService.login(user, IP, null); - verify(tokenGenerator).generateToken("815", "test@example.com", null, Role.PLATFORM_ADMIN); + verify(tokenGenerator).generateToken("815", null, Role.PLATFORM_ADMIN); } @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 d90463e..5e08cc3 100644 --- a/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java +++ b/src/test/java/auth_service/modules/registration/api/RegistrationControllerTest.java @@ -1,6 +1,12 @@ package auth_service.modules.registration.api; import auth_service.configuration.security.JwtAuthenticationFilter; +import java.sql.SQLException; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; +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.EmailAlreadyInUseException; @@ -23,14 +29,18 @@ import org.springframework.http.MediaType; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.web.servlet.MockMvc; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; import static org.mockito.ArgumentMatchers.any; 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.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -59,6 +69,14 @@ class RegistrationControllerTest extends AbstractWebMvcTest { @Autowired private JacksonTester registrationRequestJson; + + // A German server: Hibernate reads no constraint name from this text, the handler must still find it. + private static DataIntegrityViolationException duplicate(String constraint) { + String text = "doppelter Schlüsselwert verletzt Unique-Constraint »" + constraint + "«"; + return new DataIntegrityViolationException(text, + new ConstraintViolationException(text, new SQLException(text, "23505"), null)); + } + @Test void register_Success() throws Exception { // Arrange @@ -93,13 +111,11 @@ class RegistrationControllerTest extends AbstractWebMvcTest { when(registrationService.register(any(Registration.class), any())) .thenThrow(new EmailAlreadyInUseException("The provided email address is already in use.")); - // Act & Assert - // Depending on your GlobalExceptionHandler, this could also be 400 or 409. - // Assuming standard behavior without advice or mapping to 400 for now. mockMvc.perform(post("/api/registration/new") .contentType(MediaType.APPLICATION_JSON) .content(registrationRequestJson.write(request).getJson())) - .andExpect(status().isBadRequest()); // Adjust status based on your @ExceptionHandler + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("EMAIL_ALREADY_IN_USE")); } @Test @@ -114,20 +130,53 @@ class RegistrationControllerTest extends AbstractWebMvcTest { } @Test - void register_DuplicateEmailRaceIsNotReportedAsConflict() throws Exception { - // Arrange + void register_DuplicateEmailRace_Is409EmailAlreadyInUse() throws Exception { var request = RegistrationRequest.builder() .email("test@example.com") .password("SuperSecret123!") .inviteCode("code") .build(); when(registrationService.register(any(Registration.class), any())) - .thenThrow(new DataIntegrityViolationException("users_email_key")); + .thenThrow(duplicate("users_email_lower")); - // Act & Assert: the user module's 409 mapping must not apply to registration - assertThrows(ServletException.class, () -> mockMvc.perform(post("/api/registration/new") - .contentType(MediaType.APPLICATION_JSON) - .content(registrationRequestJson.write(request).getJson()))); + mockMvc.perform(post("/api/registration/new") + .contentType(MediaType.APPLICATION_JSON) + .content(registrationRequestJson.write(request).getJson())) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("EMAIL_ALREADY_IN_USE")); + } + + @Test + void register_DisplayNameRace_Is409DisplayNameAlreadyInUse() throws Exception { + when(registrationService.register(any(Registration.class), any())) + .thenThrow(duplicate("uq_users_display_name_lower")); + + mockMvc.perform(post("/api/registration/new") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"test@example.com","password":"SuperSecret123!","inviteCode":"code","displayName":"Lobo"}""")) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("DISPLAY_NAME_ALREADY_IN_USE")); + } + + @Test + void register_UnreadableBody_Is400ValidationFailed() throws Exception { + mockMvc.perform(post("/api/registration/new").contentType(MediaType.APPLICATION_JSON).content("{not json")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")); + } + + @Test + void register_UnexpectedError_Is500WithoutDetails() throws Exception { + when(registrationService.register(any(Registration.class), any())).thenThrow(new IllegalStateException("secret detail")); + + mockMvc.perform(post("/api/registration/new") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"test@example.com","password":"SuperSecret123!","inviteCode":"code"}""")) + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.code").value("INTERNAL_ERROR")) + .andExpect(content().string(not(containsString("secret detail")))); } @Test @@ -136,7 +185,9 @@ class RegistrationControllerTest extends AbstractWebMvcTest { .contentType(MediaType.APPLICATION_JSON) .content(""" {"email":"test@example.com","password":"SuperSecret123!","inviteCode":"code","displayName":"x"}""")) - .andExpect(status().isBadRequest()); + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")) + .andExpect(jsonPath("$.fields[0].field").value("displayName")); verifyNoInteractions(registrationService); } @@ -238,4 +289,20 @@ class RegistrationControllerTest extends AbstractWebMvcTest { verify(registrationService).register(registration.capture(), any()); assertEquals("bob@example.com", registration.getValue().getEmail()); } + + @Test + @ExtendWith(OutputCaptureExtension.class) + void aSpringServerError_IsLoggedAsInternalError(CapturedOutput output) throws Exception { + when(registrationService.register(any(Registration.class), any())) + .thenThrow(new HttpMessageNotWritableException("body not writable")); + + mockMvc.perform(post("/api/registration/new") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"test@example.com","password":"SuperSecret123!","inviteCode":"code"}""")) + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.code").value("INTERNAL_ERROR")); + + assertTrue(output.getAll().contains("body not writable"), "the 500 left no trace in the log"); + } } 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 292abdd..a972c1a 100644 --- a/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java +++ b/src/test/java/auth_service/modules/registration/persistence/RegisteredUserRepositoryTest.java @@ -63,10 +63,13 @@ class RegisteredUserRepositoryTest extends AbstractPersistenceTest { entityManager.flush(); assertTrue(registeredUserRepository.existsByEmail("Dup@Example.com")); - assertThrows(org.hibernate.exception.ConstraintViolationException.class, () -> { + var refused = assertThrows(org.hibernate.exception.ConstraintViolationException.class, () -> { entityManager.persist(RegisteredUserEntity.builder().email("Dup@example.com").password("password") .enabled(true).registrationDate(java.time.LocalDateTime.now()).build()); entityManager.flush(); }); + // GlobalExceptionController decides on the state; on an English server Hibernate also gives the name. + assertEquals("23505", refused.getSQLState()); + assertEquals("users_email_lower", refused.getConstraintName()); } } diff --git a/src/test/java/auth_service/modules/user/api/UserControllerTest.java b/src/test/java/auth_service/modules/user/api/UserControllerTest.java index a7f2ff3..318815a 100644 --- a/src/test/java/auth_service/modules/user/api/UserControllerTest.java +++ b/src/test/java/auth_service/modules/user/api/UserControllerTest.java @@ -1,6 +1,8 @@ package auth_service.modules.user.api; import auth_service.modules.user.model.ProfileStyle; +import java.sql.SQLException; +import org.hibernate.exception.ConstraintViolationException; import auth_service.modules.user.model.Tile; import auth_service.configuration.security.JwtAuthenticationFilter; import auth_service.configuration.security.JwtService; @@ -69,17 +71,25 @@ class UserControllerTest extends AbstractWebMvcTest { SecurityContextHolder.clearContext(); } + + // A German server: Hibernate reads no constraint name from this text, the handler must still find it. + private static DataIntegrityViolationException duplicate(String constraint) { + String text = "doppelter Schlüsselwert verletzt Unique-Constraint »" + constraint + "«"; + return new DataIntegrityViolationException(text, + new ConstraintViolationException(text, new SQLException(text, "23505"), null)); + } + @Test void getCurrentUserV1_Success() throws Exception { // Arrange var email = "test@example.com"; var settings = UserSettings.builder().language("en").theme("dark").build(); var user = User.builder().email(email).settings(settings).build(); - var principal = new UserPrincipal("1", email, null); + var principal = new UserPrincipal("1", null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()) ); - when(userService.getUserByEmail(email)).thenReturn(user); + when(userService.getUser(1L)).thenReturn(user); // Act & Assert mockMvc.perform(get("/api/users/me") @@ -94,11 +104,11 @@ class UserControllerTest extends AbstractWebMvcTest { var email = "test@example.com"; var settings = UserSettings.builder().language("en").theme("dark").build(); var user = User.builder().email(email).steamId(76561198000000001L).steamIdVerified(true).settings(settings).build(); - var principal = new UserPrincipal("1", email, null); + var principal = new UserPrincipal("1", null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()) ); - when(userService.getUserByEmail(email)).thenReturn(user); + when(userService.getUser(1L)).thenReturn(user); // Act & Assert mockMvc.perform(get("/api/users/me") @@ -114,11 +124,11 @@ class UserControllerTest extends AbstractWebMvcTest { var email = "test@example.com"; var settings = UserSettings.builder().language("en").theme("dark").build(); var user = User.builder().email(email).settings(settings).build(); - var principal = new UserPrincipal("1", email, null); + var principal = new UserPrincipal("1", null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()) ); - when(userService.getUserByEmail(email)).thenReturn(user); + when(userService.getUser(1L)).thenReturn(user); // Act & Assert mockMvc.perform(get("/api/users/me") @@ -131,11 +141,11 @@ class UserControllerTest extends AbstractWebMvcTest { void getCurrentUserV1_NotFound() throws Exception { // Arrange var email = "notfound@example.com"; - var principal = new UserPrincipal("1", email, null); + var principal = new UserPrincipal("1", null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()) ); - when(userService.getUserByEmail(email)).thenThrow(new UserNotFoundException("User not found")); + when(userService.getUser(1L)).thenThrow(new UserNotFoundException("User not found")); // Act & Assert mockMvc.perform(get("/api/users/me") @@ -145,7 +155,7 @@ class UserControllerTest extends AbstractWebMvcTest { @Test void updateUserSettings_OldClientSendingASteamId_SavesTheRestAndIssuesNoCookie() throws Exception { - var principal = new UserPrincipal("1", "test@example.com", null); + var principal = new UserPrincipal("1", null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()) ); @@ -164,11 +174,11 @@ class UserControllerTest extends AbstractWebMvcTest { void updateUserSettings_OtherConstraintIsNotAConflict() throws Exception { // Arrange var email = "test@example.com"; - var principal = new UserPrincipal("1", email, null); + var principal = new UserPrincipal("1", null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()) ); - doThrow(new DataIntegrityViolationException("value too long for type character varying(10)")).when(userService).updateUserSettings(eq(1L), any(), any()); + doThrow(new DataIntegrityViolationException("Wert zu lang", new SQLException("Wert zu lang", "22001"))).when(userService).updateUserSettings(eq(1L), any(), any()); // Act & Assert mockMvc.perform(put("/api/users/settings") @@ -185,11 +195,11 @@ class UserControllerTest extends AbstractWebMvcTest { var email = "test@example.com"; var settings = UserSettings.builder().language("en").theme("dark").build(); var user = User.builder().email(email).displayName("Lobo").settings(settings).build(); - var principal = new UserPrincipal("1", email, null); + var principal = new UserPrincipal("1", null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()) ); - when(userService.getUserByEmail(email)).thenReturn(user); + when(userService.getUser(1L)).thenReturn(user); // Act & Assert mockMvc.perform(get("/api/users/me").accept(MediaType.APPLICATION_JSON)) @@ -201,7 +211,7 @@ class UserControllerTest extends AbstractWebMvcTest { void updateUserSettings_PassesDisplayName() throws Exception { // Arrange var email = "test@example.com"; - var principal = new UserPrincipal("1", email, null); + var principal = new UserPrincipal("1", null); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken(principal, null, Collections.emptyList()) ); @@ -218,7 +228,7 @@ class UserControllerTest extends AbstractWebMvcTest { @ValueSource(strings = {"ab", " Lobo", "Lobo ", "