From 73cfcb58e2e7b757f4987508d9d4cbe6744841b9 Mon Sep 17 00:00:00 2001 From: lobo Date: Tue, 29 Sep 2026 14:48:28 +0200 Subject: [PATCH] refactor(api): one error body, token audiences, contract fixture --- readme.md | 16 +- .../security/DevJwtAuthenticationFilter.java | 2 +- .../configuration/security/JwtService.java | 39 ++++- .../library/api/GameImportController.java | 13 +- .../library/api/LibraryController.java | 24 +-- .../library/api/ManualGameController.java | 15 +- .../library/api/PublicProfileController.java | 4 +- .../shared/api/ErrorResponse.java | 21 +++ .../shared/api/GlobalExceptionController.java | 145 ++++++++++++++++++ .../shared/security/UserPrincipal.java | 5 +- .../security/AccessTokenFiltersTest.java | 2 +- .../security/ExpiredAccessTokenTest.java | 13 +- .../security/JwtServiceTest.java | 66 +++++++- .../security/PublicEndpointsTest.java | 16 +- .../security/RoleCheckFlowTest.java | 12 +- .../api/InternalAccountControllerTest.java | 55 +++++-- .../library/api/GameCorrectionFlowTest.java | 11 +- .../library/api/GameMergeFlowTest.java | 13 +- .../api/LibraryControllerStatsTest.java | 57 ++++++- .../api/LibraryControllerSteamIdGoneTest.java | 10 +- .../library/api/ManualGameControllerTest.java | 18 ++- .../api/PublicProfileControllerTest.java | 14 +- .../shared/test/ContractTokens.java | 120 +++++++++++++++ .../contract/auth-core-contract.json | 71 +++++++++ 24 files changed, 633 insertions(+), 129 deletions(-) create mode 100644 src/main/java/core_service/shared/api/ErrorResponse.java create mode 100644 src/main/java/core_service/shared/api/GlobalExceptionController.java create mode 100644 src/test/java/core_service/shared/test/ContractTokens.java create mode 100644 src/test/resources/contract/auth-core-contract.json diff --git a/readme.md b/readme.md index 997bb20..a5b57b4 100644 --- a/readme.md +++ b/readme.md @@ -77,14 +77,20 @@ To Be Played is split into three repositories: Core never issues tokens. It verifies Auth's EdDSA (Ed25519) JWTs with public keys only. It loads them from Auth's JWKS by the token's `kid` and loads again when a new `kid` shows up (at most every 10 seconds). Loaded keys stay until Core restarts. `JWT_PUBLIC_KEY` is used only until the first JWKS load works. The token comes from Auth's -`tbp_access` cookie or a `Bearer` header, whichever verifies first. Core reads `userId`, `steamId` and the active -`role` (missing or unknown = USER) from the claims and grants that role and every lower one -(`USER < AUDITOR < PLATFORM_ADMIN`). +`tbp_access` cookie or a `Bearer` header, whichever verifies first. Every token names its kind in `aud` +(`tbp-login`, `tbp-public-profile`, `tbp-internal`, with `iss=tbp-auth`); a login's `sub` is the user id. Core reads +`steamId` and the active `role` (missing or unknown = USER) from the claims and grants that role and every lower one +(`USER < AUDITOR < PLATFORM_ADMIN`). A token without `aud` comes from an Auth from before item 26 (kind in `typ`, +user id in `userId`) and is still accepted; item 29 removes that path. The claims are pinned in +`src/test/resources/contract/auth-core-contract.json`, the same file as in Auth; the tests build tokens only from it. `POST`, `PUT` and `DELETE` need an `X-TBP-Client` or `Authorization` header (CSRF guard), and CORS allows only the frontend origin. -A token with a `typ` claim (for example a profile ticket) is rejected on normal endpoints; the profile ticket is +A token for another audience (for example a profile ticket) is rejected on normal endpoints; the profile ticket is accepted only by `GET /api/public/profiles/library` in the `X-Profile-Ticket` header. -`/internal/**` has its own filter chain: it accepts only a service token of Auth (`typ=service`, with `exp`, sent +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: [{field, message}]`, a duplicate key `409 CONFLICT`, anything unexpected `500 INTERNAL_ERROR` without details +(`shared/api/GlobalExceptionController`). +`/internal/**` has its own filter chain: it accepts only a service token of Auth (`aud=tbp-internal`, with `exp`, sent as `Bearer`), never a login, also not in the dev profile. Traefik exposes the path, so the signed token is its only guard. diff --git a/src/main/java/core_service/configuration/security/DevJwtAuthenticationFilter.java b/src/main/java/core_service/configuration/security/DevJwtAuthenticationFilter.java index 94d5a5d..1b39648 100644 --- a/src/main/java/core_service/configuration/security/DevJwtAuthenticationFilter.java +++ b/src/main/java/core_service/configuration/security/DevJwtAuthenticationFilter.java @@ -65,6 +65,6 @@ public class DevJwtAuthenticationFilter extends OncePerRequestFilter { private UserPrincipal dummyPrincipal() { log.trace("Dev mode: no JWT present, injecting dummy principal"); - return new UserPrincipal("1", "dev@example.com", 76561198000000001L); + return new UserPrincipal("1", 76561198000000001L); } } diff --git a/src/main/java/core_service/configuration/security/JwtService.java b/src/main/java/core_service/configuration/security/JwtService.java index 5929dc0..414bd42 100644 --- a/src/main/java/core_service/configuration/security/JwtService.java +++ b/src/main/java/core_service/configuration/security/JwtService.java @@ -10,17 +10,26 @@ import io.jsonwebtoken.Jwts; import org.springframework.stereotype.Service; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; /** * Verifies JWTs issued by tbp-auth-service. Verify-only: {@link JwksKeyLocator} finds the Ed25519 public key. + *

+ * A token names its kind in {@code aud}, and {@code sub} is the user id of a login. A token without {@code aud} comes + * from an Auth from before item 26: its kind is in {@code typ} and the user id in {@code userId}. Item 29 removes + * that path. */ @Service public class JwtService implements ProfileTicketVerifier { - // Only non-login tokens carry this claim; parseLoginToken rejects them. + 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"; + // 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 String SERVICE_TOKEN_TYPE = "service"; @@ -36,6 +45,20 @@ public class JwtService implements ProfileTicketVerifier { return parser.parseSignedClaims(token).getPayload(); } + 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)); + } + /** * The user of a login token with a valid signature. Empty for anything else, also for a profile ticket: it is * signed with the same key, but is no login. @@ -43,11 +66,15 @@ public class JwtService implements ProfileTicketVerifier { public Optional parseLoginToken(String token) { try { Claims claims = claims(token); - 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), + 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) instanceof String role ? role : null))); } catch (Exception e) { return Optional.empty(); @@ -60,7 +87,7 @@ public class JwtService implements ProfileTicketVerifier { public boolean isServiceToken(String token) { try { Claims claims = claims(token); - return SERVICE_TOKEN_TYPE.equals(claims.get(TYPE_CLAIM, String.class)) && claims.getExpiration() != null; + return isKind(claims, SERVICE_AUDIENCE, SERVICE_TOKEN_TYPE) && claims.getExpiration() != null; } catch (Exception e) { return false; } @@ -74,7 +101,7 @@ public class JwtService implements ProfileTicketVerifier { public Optional verifyProfileTicket(String ticket) { try { Claims claims = claims(ticket); - if (!PROFILE_TICKET_TYPE.equals(claims.get(TYPE_CLAIM, String.class))) { + if (!isKind(claims, PROFILE_TICKET_AUDIENCE, PROFILE_TICKET_TYPE)) { return Optional.empty(); } long userId = Long.parseLong(claims.get("profileUserId", String.class)); diff --git a/src/main/java/core_service/modules/library/api/GameImportController.java b/src/main/java/core_service/modules/library/api/GameImportController.java index 85f0fca..f04858b 100644 --- a/src/main/java/core_service/modules/library/api/GameImportController.java +++ b/src/main/java/core_service/modules/library/api/GameImportController.java @@ -1,5 +1,6 @@ package core_service.modules.library.api; +import core_service.shared.api.ErrorResponse; import core_service.modules.library.api.dto.ImportHistoryDto; import core_service.modules.library.api.dto.ImportRequest; import core_service.modules.library.api.dto.ImportSaveRequest; @@ -28,9 +29,9 @@ import java.util.Map; @RequiredArgsConstructor public class GameImportController { - private static final Map NOT_FOUND = Map.of("code", "IMPORT_NOT_FOUND", "message", "No such import"); + private static final ErrorResponse NOT_FOUND = new ErrorResponse("IMPORT_NOT_FOUND", "No such import"); private static final ResponseEntity IMPORT_RUNNING = ResponseEntity.status(HttpStatus.CONFLICT) - .body(Map.of("code", "IMPORT_RUNNING", "message", "Another import is still being matched")); + .body(new ErrorResponse("IMPORT_RUNNING", "Another import is still being matched")); private final GameImportService importService; @@ -42,7 +43,7 @@ public class GameImportController { .body(Map.of("id", created.id())); case GameImportService.CreateResult.AlreadyMatching ignored -> IMPORT_RUNNING; case GameImportService.CreateResult.SearchUnavailable ignored -> ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE) - .body(Map.of("code", "IGDB_OFF", "message", "Importing games needs the IGDB app")); + .body(new ErrorResponse("IGDB_OFF", "Importing games needs the IGDB app")); }; } @@ -65,7 +66,7 @@ public class GameImportController { case DONE -> ResponseEntity.accepted().build(); case NOT_FOUND -> ResponseEntity.status(HttpStatus.NOT_FOUND).body(NOT_FOUND); case WRONG_STATUS, ALREADY_MATCHING -> ResponseEntity.status(HttpStatus.CONFLICT) - .body(Map.of("code", "IMPORT_NOT_READY", "message", "The import is still matching or saved already")); + .body(new ErrorResponse("IMPORT_NOT_READY", "The import is still matching or saved already")); }; } @@ -75,7 +76,7 @@ public class GameImportController { case DONE -> ResponseEntity.accepted().build(); case NOT_FOUND -> ResponseEntity.status(HttpStatus.NOT_FOUND).body(NOT_FOUND); case WRONG_STATUS -> ResponseEntity.status(HttpStatus.CONFLICT) - .body(Map.of("code", "IMPORT_NOT_FAILED", "message", "Only a failed import can be tried again")); + .body(new ErrorResponse("IMPORT_NOT_FAILED", "Only a failed import can be tried again")); case ALREADY_MATCHING -> IMPORT_RUNNING; }; } @@ -93,7 +94,7 @@ public class GameImportController { case DONE -> ResponseEntity.noContent().build(); case NOT_FOUND -> ResponseEntity.status(HttpStatus.NOT_FOUND).body(NOT_FOUND); case WRONG_STATUS, ALREADY_MATCHING -> ResponseEntity.status(HttpStatus.CONFLICT) - .body(Map.of("code", "IMPORT_NOT_SAVED", "message", "Only a saved import can be undone")); + .body(new ErrorResponse("IMPORT_NOT_SAVED", "Only a saved import can be undone")); }; } } diff --git a/src/main/java/core_service/modules/library/api/LibraryController.java b/src/main/java/core_service/modules/library/api/LibraryController.java index 3fb14a6..510c21f 100644 --- a/src/main/java/core_service/modules/library/api/LibraryController.java +++ b/src/main/java/core_service/modules/library/api/LibraryController.java @@ -1,5 +1,6 @@ package core_service.modules.library.api; +import core_service.shared.api.ErrorResponse; import core_service.modules.library.api.dto.AchievementSummaryDto; import core_service.modules.library.api.dto.ActivityDto; import core_service.modules.library.api.dto.BacklogEntryDto; @@ -62,6 +63,7 @@ import java.util.Set; @RequiredArgsConstructor public class LibraryController { + private static final ErrorResponse GAME_NOT_FOUND = new ErrorResponse("GAME_NOT_FOUND", "Game not in user library"); private static final Set ALLOWED_SIZES = Set.of(20, 40, 80); private static final int DEFAULT_SIZE = 20; private static final int MIN_ACTIVITY_DAYS = 7; @@ -129,7 +131,7 @@ public class LibraryController { @PostMapping("/sync") public ResponseEntity forceSync(@AuthenticationPrincipal UserPrincipal principal) { if (principal.steamId() == null) { - return ResponseEntity.badRequest().body(Map.of("code", "NO_STEAM_ID", "message", "User has no Steam ID")); + return ResponseEntity.badRequest().body(new ErrorResponse("NO_STEAM_ID", "User has no Steam ID")); } try { syncService.forceSync(principal.getIdAsLong(), String.valueOf(principal.steamId())); @@ -152,7 +154,7 @@ public class LibraryController { return gameDetailService.getGameDetail(userId, steamId, appid) .>map(ResponseEntity::ok) .orElseGet(() -> ResponseEntity.status(HttpStatus.NOT_FOUND) - .body(Map.of("code", "GAME_NOT_FOUND", "message", "Game not in user library"))); + .body(GAME_NOT_FOUND)); } /** @@ -201,9 +203,9 @@ public class LibraryController { case GameMergeService.Result.Merged merged -> ResponseEntity.ok(new MergedGameDto(merged.appid())); case GameMergeService.Result.NotFound _ -> unknownGame(); case GameMergeService.Result.SameGame _ -> ResponseEntity.badRequest() - .body(Map.of("code", "SAME_GAME", "message", "A game cannot be merged with itself")); + .body(new ErrorResponse("SAME_GAME", "A game cannot be merged with itself")); case GameMergeService.Result.SteamGames _ -> ResponseEntity.badRequest() - .body(Map.of("code", "STEAM_GAMES", "message", "Two Steam games cannot be merged")); + .body(new ErrorResponse("STEAM_GAMES", "Two Steam games cannot be merged")); }; } @@ -212,11 +214,11 @@ public class LibraryController { } private static ResponseEntity unknownGame() { - return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of("code", "GAME_NOT_FOUND", "message", "Unknown game")); + return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ErrorResponse("GAME_NOT_FOUND", "Unknown game")); } private static ResponseEntity invalidCorrection(String message) { - return ResponseEntity.badRequest().body(Map.of("code", "INVALID_CORRECTION", "message", message)); + return ResponseEntity.badRequest().body(new ErrorResponse("INVALID_CORRECTION", message)); } @PutMapping("/games/{appid}/backlog") @@ -226,7 +228,7 @@ public class LibraryController { return backlogService.update(principal.getIdAsLong(), appid, entry) .>map(ResponseEntity::ok) .orElseGet(() -> ResponseEntity.status(HttpStatus.NOT_FOUND) - .body(Map.of("code", "GAME_NOT_FOUND", "message", "Game not in user library"))); + .body(GAME_NOT_FOUND)); } @PutMapping("/games/{appid}/hidden") @@ -236,9 +238,9 @@ public class LibraryController { return switch (backlogService.setHidden(principal.getIdAsLong(), appid, update.hidden())) { case DONE -> ResponseEntity.ok(update); case NOT_FOUND -> ResponseEntity.status(HttpStatus.NOT_FOUND) - .body(Map.of("code", "GAME_NOT_FOUND", "message", "Game not in user library")); + .body(GAME_NOT_FOUND); case MANUAL_GAME -> ResponseEntity.status(HttpStatus.CONFLICT) - .body(Map.of("code", "MANUAL_GAME", "message", "A game added by hand can be deleted, not hidden")); + .body(new ErrorResponse("MANUAL_GAME", "A game added by hand can be deleted, not hidden")); }; } @@ -248,7 +250,7 @@ public class LibraryController { @Valid @RequestBody FinishedHintDismissal dismissal) { if (!backlogService.dismissFinishedHint(principal.getIdAsLong(), appid, dismissal.dismissed())) { return ResponseEntity.status(HttpStatus.NOT_FOUND) - .body(Map.of("code", "GAME_NOT_FOUND", "message", "Game not in user library")); + .body(GAME_NOT_FOUND); } return ResponseEntity.noContent().build(); } @@ -260,7 +262,7 @@ public class LibraryController { return backlogService.updateStatus(principal.getIdAsLong(), appid, update.status()) .>map(ResponseEntity::ok) .orElseGet(() -> ResponseEntity.status(HttpStatus.NOT_FOUND) - .body(Map.of("code", "GAME_NOT_FOUND", "message", "Game not in user library"))); + .body(GAME_NOT_FOUND)); } @GetMapping("/activity") diff --git a/src/main/java/core_service/modules/library/api/ManualGameController.java b/src/main/java/core_service/modules/library/api/ManualGameController.java index 25eb42c..1bf90fe 100644 --- a/src/main/java/core_service/modules/library/api/ManualGameController.java +++ b/src/main/java/core_service/modules/library/api/ManualGameController.java @@ -1,5 +1,6 @@ package core_service.modules.library.api; +import core_service.shared.api.ErrorResponse; import core_service.modules.library.api.dto.AddGameRequest; import core_service.modules.library.api.dto.AddedGameDto; import core_service.modules.library.api.dto.GameSearchDto; @@ -23,7 +24,6 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; import java.net.URI; -import java.util.Map; /** * Games added by hand from an IGDB search, next to the Steam library. @@ -33,9 +33,8 @@ import java.util.Map; @RequiredArgsConstructor public class ManualGameController { - private static final Map NOT_FOUND = Map.of("code", "GAME_NOT_FOUND", "message", "Game not in user library"); - private static final Map STEAM_GAME = Map.of("code", "STEAM_GAME", - "message", "Steam games come from the sync and cannot be changed here"); + private static final ErrorResponse NOT_FOUND = new ErrorResponse("GAME_NOT_FOUND", "Game not in user library"); + private static final ErrorResponse STEAM_GAME = new ErrorResponse("STEAM_GAME", "Steam games come from the sync and cannot be changed here"); private final ManualGameService manualGameService; @@ -45,7 +44,7 @@ public class ManualGameController { return manualGameService.search(principal.getIdAsLong(), query) .>map(ResponseEntity::ok) .orElseGet(() -> ResponseEntity.status(HttpStatus.BAD_GATEWAY) - .body(Map.of("code", "IGDB_FAILED", "message", "IGDB did not answer"))); + .body(new ErrorResponse("IGDB_FAILED", "IGDB did not answer"))); } @PostMapping("/games") @@ -58,11 +57,11 @@ public class ManualGameController { .created(URI.create("/api/library/games/" + added.appid())) .body(new AddedGameDto(added.appid())); case ManualGameService.AddResult.AlreadyInLibrary ignored -> ResponseEntity.status(HttpStatus.CONFLICT) - .body(Map.of("code", "ALREADY_IN_LIBRARY", "message", "The game is already in the library")); + .body(new ErrorResponse("ALREADY_IN_LIBRARY", "The game is already in the library")); case ManualGameService.AddResult.UnknownGame ignored -> ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body(Map.of("code", "UNKNOWN_GAME", "message", "IGDB has no such game or did not answer")); + .body(new ErrorResponse("UNKNOWN_GAME", "IGDB has no such game or did not answer")); case ManualGameService.AddResult.SearchUnavailable ignored -> ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE) - .body(Map.of("code", "IGDB_OFF", "message", "Adding games needs the IGDB app")); + .body(new ErrorResponse("IGDB_OFF", "Adding games needs the IGDB app")); }; } diff --git a/src/main/java/core_service/modules/library/api/PublicProfileController.java b/src/main/java/core_service/modules/library/api/PublicProfileController.java index 87ed34e..65bc496 100644 --- a/src/main/java/core_service/modules/library/api/PublicProfileController.java +++ b/src/main/java/core_service/modules/library/api/PublicProfileController.java @@ -1,5 +1,6 @@ package core_service.modules.library.api; +import core_service.shared.api.ErrorResponse; import core_service.modules.library.service.LibraryQueryService; import core_service.shared.security.ProfileTicket; import core_service.shared.security.ProfileTicketVerifier; @@ -11,7 +12,6 @@ import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; -import java.util.Map; import java.util.Optional; /** @@ -31,6 +31,6 @@ public class PublicProfileController { return (ticket == null ? Optional.empty() : profileTicketVerifier.verifyProfileTicket(ticket)) .>map(t -> ResponseEntity.ok(queryService.getPublicLibrary(t.userId(), t.profileStats(), t.sections()))) .orElseGet(() -> ResponseEntity.status(HttpStatus.UNAUTHORIZED) - .body(Map.of("code", "INVALID_TICKET", "message", "Missing, invalid or expired profile ticket"))); + .body(new ErrorResponse("INVALID_TICKET", "Missing, invalid or expired profile ticket"))); } } diff --git a/src/main/java/core_service/shared/api/ErrorResponse.java b/src/main/java/core_service/shared/api/ErrorResponse.java new file mode 100644 index 0000000..a4f441a --- /dev/null +++ b/src/main/java/core_service/shared/api/ErrorResponse.java @@ -0,0 +1,21 @@ +package core_service.shared.api; + +import com.fasterxml.jackson.annotation.JsonInclude; + +import java.util.List; + +/** + * The body of every error answer. + * + * @param fields the refused request fields of a {@code VALIDATION_FAILED}, else left out + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public record ErrorResponse(String code, String message, List fields) { + + public ErrorResponse(String code, String message) { + this(code, message, null); + } + + public record FieldError(String field, String message) { + } +} diff --git a/src/main/java/core_service/shared/api/GlobalExceptionController.java b/src/main/java/core_service/shared/api/GlobalExceptionController.java new file mode 100644 index 0000000..23b3735 --- /dev/null +++ b/src/main/java/core_service/shared/api/GlobalExceptionController.java @@ -0,0 +1,145 @@ +package core_service.shared.api; + +import lombok.extern.slf4j.Slf4j; +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"; + + @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 duplicate key is a parallel request that won. 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); + } + log.warn("Database refused a write: {}", ex.getMostSpecificCause().getMessage()); + return ResponseEntity.status(HttpStatus.CONFLICT) + .body(new ErrorResponse("CONFLICT", "The data changed at the same time; try again")); + } + + /** + * 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")); + } + + 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/core_service/shared/security/UserPrincipal.java b/src/main/java/core_service/shared/security/UserPrincipal.java index cd62358..91502ed 100644 --- a/src/main/java/core_service/shared/security/UserPrincipal.java +++ b/src/main/java/core_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/core_service/configuration/security/AccessTokenFiltersTest.java b/src/test/java/core_service/configuration/security/AccessTokenFiltersTest.java index 556dde8..7a30415 100644 --- a/src/test/java/core_service/configuration/security/AccessTokenFiltersTest.java +++ b/src/test/java/core_service/configuration/security/AccessTokenFiltersTest.java @@ -23,7 +23,7 @@ import static org.mockito.Mockito.lenient; */ class AccessTokenFiltersTest extends AbstractUnitTest { - private static final UserPrincipal USER = new UserPrincipal("42", "a@b.de", 76561198000000042L); + private static final UserPrincipal USER = new UserPrincipal("42", 76561198000000042L); @Mock private JwtService jwtService; diff --git a/src/test/java/core_service/configuration/security/ExpiredAccessTokenTest.java b/src/test/java/core_service/configuration/security/ExpiredAccessTokenTest.java index 694a2b4..59bbcc9 100644 --- a/src/test/java/core_service/configuration/security/ExpiredAccessTokenTest.java +++ b/src/test/java/core_service/configuration/security/ExpiredAccessTokenTest.java @@ -1,16 +1,15 @@ package core_service.configuration.security; +import core_service.shared.security.Role; import core_service.shared.test.AbstractIntegrationTest; +import core_service.shared.test.ContractTokens; import jakarta.servlet.http.Cookie; -import io.jsonwebtoken.Jwts; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; -import java.util.Date; - import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -25,13 +24,7 @@ class ExpiredAccessTokenTest extends AbstractIntegrationTest { private MockMvc mockMvc; private static String expiredAccessToken() { - return Jwts.builder() - .subject("a@b.de") - .claim("userId", "42") - .issuedAt(new Date(System.currentTimeMillis() - 120_000)) - .expiration(new Date(System.currentTimeMillis() - 60_000)) - .signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA) - .compact(); + return ContractTokens.sign(ContractTokens.login(42, Role.USER, null), TEST_KEYS.getPrivate(), -60_000); } @Test diff --git a/src/test/java/core_service/configuration/security/JwtServiceTest.java b/src/test/java/core_service/configuration/security/JwtServiceTest.java index c411c88..6a1b2e4 100644 --- a/src/test/java/core_service/configuration/security/JwtServiceTest.java +++ b/src/test/java/core_service/configuration/security/JwtServiceTest.java @@ -4,6 +4,7 @@ import core_service.shared.security.ProfileTicket; import core_service.shared.security.Role; import core_service.shared.security.UserPrincipal; import core_service.shared.test.AbstractUnitTest; +import core_service.shared.test.ContractTokens; import io.jsonwebtoken.Jwts; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -16,6 +17,7 @@ import java.security.PrivateKey; import java.util.Base64; import java.util.Date; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -62,7 +64,7 @@ class JwtServiceTest extends AbstractUnitTest { void parseLoginToken_ReturnsTheUser() { String token = signToken("0815", "test@example.com", 123456789L); - assertEquals(Optional.of(new UserPrincipal("0815", "test@example.com", 123456789L)), + assertEquals(Optional.of(new UserPrincipal("0815", 123456789L)), jwtService.parseLoginToken(token)); } @@ -205,4 +207,66 @@ class JwtServiceTest extends AbstractUnitTest { assertEquals(Role.USER, jwtService.parseLoginToken(token).orElseThrow().role()); } + + private String sign(Map claims) { + return ContractTokens.sign(claims, signingKey, 60_000); + } + + // The shapes of item 29: without the claims an Auth from before item 26 needs. + private static Map without(Map claims, String... names) { + for (String name : names) { + claims.remove(name); + } + return claims; + } + + @Test + void contractTokens_OfEveryKind_AreAccepted() { + assertEquals("42", jwtService.parseLoginToken(sign(ContractTokens.claims(ContractTokens.LOGIN))).orElseThrow().id()); + assertEquals(42L, jwtService.verifyProfileTicket(sign(ContractTokens.claims(ContractTokens.PUBLIC_PROFILE))) + .orElseThrow().userId()); + assertTrue(jwtService.isServiceToken(sign(ContractTokens.claims(ContractTokens.SERVICE)))); + } + + @Test + void legacyTokens_OfAnAuthFromBeforeItem26_AreAccepted() { + assertEquals("42", jwtService.parseLoginToken(sign(ContractTokens.legacyClaims(ContractTokens.LOGIN))).orElseThrow().id()); + assertEquals(42L, jwtService.verifyProfileTicket(sign(ContractTokens.legacyClaims(ContractTokens.PUBLIC_PROFILE))) + .orElseThrow().userId()); + assertTrue(jwtService.isServiceToken(sign(ContractTokens.legacyClaims(ContractTokens.SERVICE)))); + } + + @Test + void tokensWithoutTheOldClaims_AreAccepted() { + assertEquals("42", jwtService.parseLoginToken(sign(without(ContractTokens.claims(ContractTokens.LOGIN), "userId"))) + .orElseThrow().id()); + assertEquals(42L, jwtService.verifyProfileTicket(sign(without(ContractTokens.claims(ContractTokens.PUBLIC_PROFILE), "typ"))) + .orElseThrow().userId()); + assertTrue(jwtService.isServiceToken(sign(without(ContractTokens.claims(ContractTokens.SERVICE), "typ")))); + } + + @Test + void aTokenForAnotherAudience_IsRejected() { + Map ticketAsLogin = ContractTokens.claims(ContractTokens.LOGIN); + ticketAsLogin.put("aud", List.of(JwtService.PROFILE_TICKET_AUDIENCE)); + Map loginAsService = ContractTokens.claims(ContractTokens.SERVICE); + loginAsService.put("aud", List.of(JwtService.LOGIN_AUDIENCE)); + Map loginAsTicket = ContractTokens.claims(ContractTokens.PUBLIC_PROFILE); + loginAsTicket.put("aud", List.of(JwtService.LOGIN_AUDIENCE)); + + assertEquals(Optional.empty(), jwtService.parseLoginToken(sign(ticketAsLogin))); + assertFalse(jwtService.isServiceToken(sign(loginAsService))); + assertEquals(Optional.empty(), jwtService.verifyProfileTicket(sign(loginAsTicket))); + } + + @Test + void aTokenWithAnAudienceButAnotherIssuer_IsRejected() { + Map login = ContractTokens.claims(ContractTokens.LOGIN); + login.put("iss", "someone-else"); + Map service = ContractTokens.claims(ContractTokens.SERVICE); + service.remove("iss"); + + assertEquals(Optional.empty(), jwtService.parseLoginToken(sign(login))); + assertFalse(jwtService.isServiceToken(sign(service))); + } } diff --git a/src/test/java/core_service/configuration/security/PublicEndpointsTest.java b/src/test/java/core_service/configuration/security/PublicEndpointsTest.java index b474a94..a61fd2a 100644 --- a/src/test/java/core_service/configuration/security/PublicEndpointsTest.java +++ b/src/test/java/core_service/configuration/security/PublicEndpointsTest.java @@ -1,6 +1,6 @@ package core_service.configuration.security; -import io.jsonwebtoken.Jwts; +import core_service.shared.test.ContractTokens; import org.junit.jupiter.api.Tag; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -19,8 +19,8 @@ import org.testcontainers.junit.jupiter.Testcontainers; import java.security.KeyPair; import java.security.KeyPairGenerator; import java.util.Base64; -import java.util.Date; import java.util.List; +import java.util.Map; import java.util.Set; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -77,9 +77,10 @@ class PublicEndpointsTest { @Test void profileTicketIsNoLogin() throws Exception { - String ticket = Jwts.builder().subject("public-profile").claim("typ", "public-profile").claim("profileUserId", "1") - .claim("userId", "1").expiration(new Date(System.currentTimeMillis() + 60_000)) - .signWith(KEYS.getPrivate(), Jwts.SIG.EdDSA).compact(); + Map claims = ContractTokens.ticket(1, null, null); + // A ticket that also carries a user id is still no login. + claims.put("userId", "1"); + String ticket = ContractTokens.sign(claims, KEYS.getPrivate(), 60_000); int status = mockMvc.perform(get("/api/library/games-count").header("Authorization", "Bearer " + ticket)) .andReturn().getResponse().getStatus(); @@ -95,9 +96,8 @@ class PublicEndpointsTest { // Without a login: a class role marker on the way would answer 401. @Test void publicProfileWithSharedStats_WorksWithoutALogin() throws Exception { - String ticket = Jwts.builder().subject("public-profile").claim("typ", "public-profile").claim("profileUserId", "1") - .claim("profileStats", List.of("TOP_GAMES", "OVERVIEW")).expiration(new Date(System.currentTimeMillis() + 60_000)) - .signWith(KEYS.getPrivate(), Jwts.SIG.EdDSA).compact(); + String ticket = ContractTokens.sign(ContractTokens.ticket(1, List.of("TOP_GAMES", "OVERVIEW"), null), + KEYS.getPrivate(), 60_000); mockMvc.perform(get("/api/public/profiles/library").header("X-Profile-Ticket", ticket)) .andExpect(status().isOk()) diff --git a/src/test/java/core_service/configuration/security/RoleCheckFlowTest.java b/src/test/java/core_service/configuration/security/RoleCheckFlowTest.java index 94ad95e..976935b 100644 --- a/src/test/java/core_service/configuration/security/RoleCheckFlowTest.java +++ b/src/test/java/core_service/configuration/security/RoleCheckFlowTest.java @@ -4,7 +4,7 @@ import core_service.shared.security.Public; import core_service.shared.security.RequiresRole; import core_service.shared.security.Role; import core_service.shared.test.AbstractIntegrationTest; -import io.jsonwebtoken.Jwts; +import core_service.shared.test.ContractTokens; import jakarta.servlet.http.Cookie; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -18,8 +18,6 @@ import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilde import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; -import java.util.Date; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -93,13 +91,7 @@ class RoleCheckFlowTest extends AbstractIntegrationTest { private ProbeService probeService; private static String token(Role role) { - return Jwts.builder() - .subject("probe@example.com") - .claim("userId", "4711") - .claim("role", role.name()) - .expiration(new Date(System.currentTimeMillis() + 60_000)) - .signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA) - .compact(); + return ContractTokens.loginToken(4711, role); } private ResultActions call(String path, Role role) throws Exception { diff --git a/src/test/java/core_service/modules/account/api/InternalAccountControllerTest.java b/src/test/java/core_service/modules/account/api/InternalAccountControllerTest.java index 0fac1b0..bfe916a 100644 --- a/src/test/java/core_service/modules/account/api/InternalAccountControllerTest.java +++ b/src/test/java/core_service/modules/account/api/InternalAccountControllerTest.java @@ -1,25 +1,29 @@ package core_service.modules.account.api; import core_service.shared.test.AbstractIntegrationTest; -import io.jsonwebtoken.JwtBuilder; -import io.jsonwebtoken.Jwts; +import core_service.shared.test.ContractTokens; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.ResultActions; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import tools.jackson.databind.JsonNode; -import java.util.Date; +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.request; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -40,20 +44,15 @@ class InternalAccountControllerTest extends AbstractIntegrationTest { @AfterEach void cleanUp() { - jdbc.update("delete from deleted_user where user_id = ?", USER_ID); - } - - private static String token(JwtBuilder builder) { - return builder.expiration(new Date(System.currentTimeMillis() + 60_000)) - .signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA).compact(); + jdbc.update("delete from deleted_user where user_id in (?, 42)", USER_ID); } private static String serviceToken() { - return token(Jwts.builder().subject("tbp-auth").claim("typ", "service")); + return ContractTokens.serviceToken(); } private static String loginToken() { - return token(Jwts.builder().subject("a@b.de").claim("userId", String.valueOf(USER_ID))); + return ContractTokens.loginToken(USER_ID); } private ResultActions send(MockHttpServletRequestBuilder request, String body, String token) throws Exception { @@ -107,6 +106,38 @@ class InternalAccountControllerTest extends AbstractIntegrationTest { @Test void prune_WithAnEmptyListIs400() throws Exception { - send(post("/internal/accounts/prune"), "{\"knownUserIds\":[],\"dryRun\":false}", serviceToken()).andExpect(status().isBadRequest()); + send(post("/internal/accounts/prune"), "{\"knownUserIds\":[],\"dryRun\":false}", serviceToken()) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")) + .andExpect(jsonPath("$.fields[0].field").value("knownUserIds")); + } + + private static MockHttpServletRequestBuilder contractRequest(JsonNode call) { + return request(HttpMethod.valueOf(call.path("method").asString()), call.path("path").asString()); + } + + // The calls as Auth's CoreAccountService sends them, with the service tokens of an old and a new Auth. + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void contractCalls_AreAnswered(boolean legacyToken) throws Exception { + String token = legacyToken ? ContractTokens.sign(ContractTokens.legacyClaims(ContractTokens.SERVICE)) + : ContractTokens.serviceToken(); + // replaceDeleted with [] removes every tombstone, also those of other tests in this database. + List tombstones = jdbc.queryForList("select user_id from deleted_user", Long.class); + try { + for (String name : List.of("markDeleted", "replaceDeleted")) { + JsonNode call = ContractTokens.internalCall(name); + send(contractRequest(call), call.path("request").toString(), token).andExpect(status().isNoContent()); + } + } finally { + tombstones.forEach(userId -> jdbc.update("insert into deleted_user (user_id) values (?) on conflict do nothing", userId)); + } + for (String name : List.of("purge", "prune")) { + JsonNode call = ContractTokens.internalCall(name); + ResultActions result = send(contractRequest(call), call.path("request").toString(), token).andExpect(status().isOk()); + for (String field : call.path("response").propertyNames()) { + result.andExpect(jsonPath("$." + field).exists()); + } + } } } diff --git a/src/test/java/core_service/modules/library/api/GameCorrectionFlowTest.java b/src/test/java/core_service/modules/library/api/GameCorrectionFlowTest.java index 3aba9e6..ef11fdd 100644 --- a/src/test/java/core_service/modules/library/api/GameCorrectionFlowTest.java +++ b/src/test/java/core_service/modules/library/api/GameCorrectionFlowTest.java @@ -11,7 +11,7 @@ import core_service.modules.library.repo.SteamGameRepository; import core_service.modules.library.repo.UserGameRepository; import core_service.shared.security.Role; import core_service.shared.test.AbstractIntegrationTest; -import io.jsonwebtoken.Jwts; +import core_service.shared.test.ContractTokens; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -22,7 +22,6 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.ResultActions; import java.time.Instant; -import java.util.Date; import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.contains; @@ -77,13 +76,7 @@ class GameCorrectionFlowTest extends AbstractIntegrationTest { } private static String token(Role role) { - return Jwts.builder() - .subject("auditor@example.com") - .claim("userId", String.valueOf(AUDITOR_ID)) - .claim("role", role.name()) - .expiration(new Date(System.currentTimeMillis() + 60_000)) - .signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA) - .compact(); + return ContractTokens.loginToken(AUDITOR_ID, role); } private ResultActions correct(Role role, long appid, String field, String valueJson) throws Exception { diff --git a/src/test/java/core_service/modules/library/api/GameMergeFlowTest.java b/src/test/java/core_service/modules/library/api/GameMergeFlowTest.java index a8ae9f1..d278ee2 100644 --- a/src/test/java/core_service/modules/library/api/GameMergeFlowTest.java +++ b/src/test/java/core_service/modules/library/api/GameMergeFlowTest.java @@ -7,16 +7,16 @@ import core_service.modules.library.domain.GameStatus; import core_service.modules.library.domain.SteamGame; import core_service.modules.library.domain.UserGame; import core_service.modules.library.domain.UserGameId; +import core_service.modules.library.domain.UserStats; import core_service.modules.library.repo.GameCorrectionRepository; import core_service.modules.library.repo.GameMoodsRepository; import core_service.modules.library.repo.GameTimeToBeatRepository; import core_service.modules.library.repo.SteamGameRepository; import core_service.modules.library.repo.UserGameRepository; import core_service.modules.library.repo.UserStatsRepository; -import core_service.modules.library.domain.UserStats; import core_service.shared.security.Role; import core_service.shared.test.AbstractIntegrationTest; -import io.jsonwebtoken.Jwts; +import core_service.shared.test.ContractTokens; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -30,7 +30,6 @@ import org.springframework.test.web.servlet.ResultActions; import java.time.Instant; import java.time.ZoneId; import java.time.temporal.ChronoUnit; -import java.util.Date; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -94,13 +93,7 @@ class GameMergeFlowTest extends AbstractIntegrationTest { } private static String token(Role role) { - return Jwts.builder() - .subject("auditor@example.com") - .claim("userId", String.valueOf(AUDITOR_ID)) - .claim("role", role.name()) - .expiration(new Date(System.currentTimeMillis() + 60_000)) - .signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA) - .compact(); + return ContractTokens.loginToken(AUDITOR_ID, role); } private ResultActions merge(Role role, long appid, long otherAppid) throws Exception { diff --git a/src/test/java/core_service/modules/library/api/LibraryControllerStatsTest.java b/src/test/java/core_service/modules/library/api/LibraryControllerStatsTest.java index a5a7b36..b4e4a38 100644 --- a/src/test/java/core_service/modules/library/api/LibraryControllerStatsTest.java +++ b/src/test/java/core_service/modules/library/api/LibraryControllerStatsTest.java @@ -14,17 +14,28 @@ import core_service.shared.test.AbstractIntegrationTest; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.DataIntegrityViolationException; +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.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.web.servlet.MockMvc; +import java.sql.SQLException; import java.time.Instant; import java.util.List; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -102,6 +113,50 @@ class LibraryControllerStatsTest extends AbstractIntegrationTest { @Test void stats_UnknownGroup_Is400() throws Exception { mockMvc.perform(get("/api/library/stats").param("groups", "MONEY")) - .andExpect(status().isBadRequest()); + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")) + .andExpect(jsonPath("$.fields[0].field").value("groups")); + } + + @Test + void anUnexpectedError_Is500WithoutDetails() throws Exception { + doThrow(new IllegalStateException("secret detail")).when(moodService).refreshListedInBackground(USER_ID); + + mockMvc.perform(get("/api/library/stats")) + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.code").value("INTERNAL_ERROR")) + .andExpect(content().string(not(containsString("secret detail")))); + } + + @Test + void aDatabaseConflict_Is409() throws Exception { + doThrow(new DataIntegrityViolationException("doppelter Schlüsselwert", new SQLException("doppelter Schlüsselwert", "23505"))).when(moodService).refreshListedInBackground(USER_ID); + + mockMvc.perform(get("/api/library/stats")) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("CONFLICT")); + } + + @Test + void anotherRefusedWrite_Is500() throws Exception { + doThrow(new DataIntegrityViolationException("Wert zu lang", new SQLException("Wert zu lang", "22001"))) + .when(moodService).refreshListedInBackground(USER_ID); + + mockMvc.perform(get("/api/library/stats")) + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.code").value("INTERNAL_ERROR")); + } + + @Test + @ExtendWith(OutputCaptureExtension.class) + void aSpringServerError_IsLoggedAsInternalError(CapturedOutput output) throws Exception { + doThrow(new HttpMessageNotWritableException("body not writable")) + .when(moodService).refreshListedInBackground(USER_ID); + + mockMvc.perform(get("/api/library/stats")) + .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/core_service/modules/library/api/LibraryControllerSteamIdGoneTest.java b/src/test/java/core_service/modules/library/api/LibraryControllerSteamIdGoneTest.java index 60f7e3e..bf70656 100644 --- a/src/test/java/core_service/modules/library/api/LibraryControllerSteamIdGoneTest.java +++ b/src/test/java/core_service/modules/library/api/LibraryControllerSteamIdGoneTest.java @@ -2,7 +2,7 @@ package core_service.modules.library.api; import core_service.modules.library.repo.UserSteamSyncRepository; import core_service.shared.test.AbstractIntegrationTest; -import io.jsonwebtoken.Jwts; +import core_service.shared.test.ContractTokens; import jakarta.servlet.http.Cookie; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -13,7 +13,6 @@ import org.springframework.test.web.servlet.MockMvc; import java.time.Duration; import java.time.Instant; -import java.util.Date; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -42,12 +41,7 @@ class LibraryControllerSteamIdGoneTest extends AbstractIntegrationTest { } private static Cookie loginWithoutSteamId() { - return new Cookie("tbp_access", Jwts.builder() - .subject("unlinked@example.com") - .claim("userId", String.valueOf(USER_ID)) - .expiration(new Date(System.currentTimeMillis() + 60_000)) - .signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA) - .compact()); + return new Cookie("tbp_access", ContractTokens.loginToken(USER_ID)); } @Test diff --git a/src/test/java/core_service/modules/library/api/ManualGameControllerTest.java b/src/test/java/core_service/modules/library/api/ManualGameControllerTest.java index 4daf59a..1731919 100644 --- a/src/test/java/core_service/modules/library/api/ManualGameControllerTest.java +++ b/src/test/java/core_service/modules/library/api/ManualGameControllerTest.java @@ -3,8 +3,8 @@ package core_service.modules.library.api; import core_service.configuration.security.ClientHeaderFilter; import core_service.modules.igdb_adapter.service.IgdbApiService; import core_service.modules.library.domain.GameSource; -import core_service.modules.library.domain.GameTimeToBeat; import core_service.modules.library.domain.GameStatus; +import core_service.modules.library.domain.GameTimeToBeat; import core_service.modules.library.domain.SteamGame; import core_service.modules.library.domain.UserGame; import core_service.modules.library.domain.UserGameId; @@ -12,6 +12,7 @@ import core_service.modules.library.repo.GameTimeToBeatRepository; import core_service.modules.library.repo.SteamGameRepository; import core_service.modules.library.repo.UserGameRepository; import core_service.shared.test.AbstractIntegrationTest; +import core_service.shared.test.ContractTokens; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -215,6 +216,14 @@ class ManualGameControllerTest extends AbstractIntegrationTest { add(119388, " ").andExpect(status().isBadRequest()); } + @Test + void search_OneLetter_Is400WithTheField() throws Exception { + mockMvc.perform(get("/api/library/game-search").param("q", "a")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")) + .andExpect(jsonPath("$.fields[0].field").value("q")); + } + @Test void search_MarksGamesAlreadyInTheLibrary() throws Exception { steamGameRepository.save(new SteamGame(1245620L, "ELDEN RING", null, Instant.now())); @@ -342,12 +351,7 @@ class ManualGameControllerTest extends AbstractIntegrationTest { // Signed like an Auth login token, for a user who saved no Steam ID. private static String tokenWithoutSteamId() { - return io.jsonwebtoken.Jwts.builder() - .subject("no-steam@example.com") - .claim("userId", String.valueOf(USER_ID)) - .expiration(new java.util.Date(System.currentTimeMillis() + 60_000)) - .signWith(TEST_KEYS.getPrivate(), io.jsonwebtoken.Jwts.SIG.EdDSA) - .compact(); + return ContractTokens.loginToken(USER_ID); } @Test diff --git a/src/test/java/core_service/modules/library/api/PublicProfileControllerTest.java b/src/test/java/core_service/modules/library/api/PublicProfileControllerTest.java index 4121a03..e0b91e2 100644 --- a/src/test/java/core_service/modules/library/api/PublicProfileControllerTest.java +++ b/src/test/java/core_service/modules/library/api/PublicProfileControllerTest.java @@ -11,7 +11,7 @@ import core_service.modules.library.repo.SteamGameRepository; import core_service.modules.library.repo.UserGameAchievementMetaRepository; import core_service.modules.library.repo.UserGameRepository; import core_service.shared.test.AbstractIntegrationTest; -import io.jsonwebtoken.Jwts; +import core_service.shared.test.ContractTokens; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -19,7 +19,6 @@ import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.test.web.servlet.MockMvc; import java.time.Instant; -import java.util.Date; import java.util.List; import static org.hamcrest.Matchers.hasSize; @@ -49,20 +48,15 @@ class PublicProfileControllerTest extends AbstractIntegrationTest { private UserGameAchievementMetaRepository userAchievementMetaRepository; private static String ticket(long userId) { - return Jwts.builder().subject("public-profile").claim("typ", "public-profile").claim("profileUserId", String.valueOf(userId)) - .expiration(new Date(System.currentTimeMillis() + 60_000)).signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA).compact(); + return ContractTokens.ticketToken(userId, null, null); } private static String ticket(long userId, List profileStats) { - return Jwts.builder().subject("public-profile").claim("typ", "public-profile").claim("profileUserId", String.valueOf(userId)) - .claim("profileStats", profileStats) - .expiration(new Date(System.currentTimeMillis() + 60_000)).signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA).compact(); + return ContractTokens.ticketToken(userId, profileStats, null); } private static String ticket(long userId, List profileStats, List sections) { - return Jwts.builder().subject("public-profile").claim("typ", "public-profile").claim("profileUserId", String.valueOf(userId)) - .claim("profileStats", profileStats).claim("sections", sections) - .expiration(new Date(System.currentTimeMillis() + 60_000)).signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA).compact(); + return ContractTokens.ticketToken(userId, profileStats, sections); } private UserGame game(long appid, int playtime, GameStatus status, Integer rating) { diff --git a/src/test/java/core_service/shared/test/ContractTokens.java b/src/test/java/core_service/shared/test/ContractTokens.java new file mode 100644 index 0000000..2bab3f0 --- /dev/null +++ b/src/test/java/core_service/shared/test/ContractTokens.java @@ -0,0 +1,120 @@ +package core_service.shared.test; + +import core_service.shared.security.Role; +import io.jsonwebtoken.Jwts; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.json.JsonMapper; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.security.PrivateKey; +import java.util.Date; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * Tokens only in the shapes of {@code contract/auth-core-contract.json}, the file Auth checks its tokens against. A + * test changes the values, never the claim names. + */ +public final class ContractTokens { + + public static final String LOGIN = "login"; + public static final String PUBLIC_PROFILE = "publicProfile"; + public static final String SERVICE = "service"; + + private static final JsonMapper MAPPER = JsonMapper.builder().build(); + private static final JsonNode CONTRACT = load(); + + private ContractTokens() { + } + + private static JsonNode load() { + try (InputStream in = ContractTokens.class.getResourceAsStream("/contract/auth-core-contract.json")) { + return MAPPER.readTree(in); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + /** + * The claims Auth signs for this kind today. + */ + @SuppressWarnings("unchecked") + public static Map claims(String kind) { + return new LinkedHashMap<>(MAPPER.treeToValue(CONTRACT.path("tokens").required(kind), Map.class)); + } + + /** + * The claims of an Auth from before item 26. + */ + @SuppressWarnings("unchecked") + public static Map legacyClaims(String kind) { + return new LinkedHashMap<>(MAPPER.treeToValue(CONTRACT.path("legacyTokens").required(kind), Map.class)); + } + + public static JsonNode internalCall(String name) { + return CONTRACT.path("internal").required(name); + } + + public static String sign(Map claims, PrivateKey key, long expiresInMs) { + return Jwts.builder() + .claims(claims) + .expiration(new Date(System.currentTimeMillis() + expiresInMs)) + .signWith(key, Jwts.SIG.EdDSA) + .compact(); + } + + public static String sign(Map claims) { + return sign(claims, AbstractIntegrationTest.TEST_KEYS.getPrivate(), 60_000); + } + + /** + * @param steamId null for a user without a linked Steam account + */ + public static Map login(long userId, Role role, Long steamId) { + Map claims = claims(LOGIN); + claims.put("sub", String.valueOf(userId)); + claims.put("userId", String.valueOf(userId)); + claims.put("role", role.name()); + putOrRemove(claims, "steamId", steamId); + return claims; + } + + public static String loginToken(long userId, Role role) { + return sign(login(userId, role, null)); + } + + public static String loginToken(long userId) { + return loginToken(userId, Role.USER); + } + + /** + * @param profileStats null for a ticket without the claim + * @param sections null for a ticket without the claim + */ + public static Map ticket(long userId, List profileStats, List sections) { + Map claims = claims(PUBLIC_PROFILE); + claims.put("profileUserId", String.valueOf(userId)); + putOrRemove(claims, "profileStats", profileStats); + putOrRemove(claims, "sections", sections); + return claims; + } + + public static String ticketToken(long userId, List profileStats, List sections) { + return sign(ticket(userId, profileStats, sections)); + } + + public static String serviceToken() { + return sign(claims(SERVICE)); + } + + private static void putOrRemove(Map claims, String name, Object value) { + if (value == null) { + claims.remove(name); + } else { + claims.put(name, value); + } + } +} diff --git a/src/test/resources/contract/auth-core-contract.json b/src/test/resources/contract/auth-core-contract.json new file mode 100644 index 0000000..d83d832 --- /dev/null +++ b/src/test/resources/contract/auth-core-contract.json @@ -0,0 +1,71 @@ +{ + "_about": "The token and internal-call contract between Auth and Core. The same file is in both repos; ToBePlayed-Deployment/scripts/check-contract.sh compares them. iat and exp are left out. legacyTokens are the shapes of an Auth from before item 26: Core and Auth accept them until item 29.", + "tokens": { + "login": { + "iss": "tbp-auth", + "aud": ["tbp-login"], + "sub": "42", + "userId": "42", + "role": "USER", + "steamId": 76561198000000001 + }, + "publicProfile": { + "iss": "tbp-auth", + "aud": ["tbp-public-profile"], + "sub": "public-profile", + "typ": "public-profile", + "profileUserId": "42", + "profileStats": ["OVERVIEW"], + "sections": ["SUMMARY"] + }, + "service": { + "iss": "tbp-auth", + "aud": ["tbp-internal"], + "sub": "tbp-auth", + "typ": "service" + } + }, + "legacyTokens": { + "login": { + "sub": "user@example.com", + "userId": "42", + "role": "USER", + "steamId": 76561198000000001 + }, + "publicProfile": { + "sub": "public-profile", + "typ": "public-profile", + "profileUserId": "42", + "profileStats": ["OVERVIEW"], + "sections": ["SUMMARY"] + }, + "service": { + "sub": "tbp-auth", + "typ": "service" + } + }, + "internal": { + "markDeleted": { + "method": "POST", + "path": "/internal/accounts/deleted", + "request": {"userIds": [42]} + }, + "replaceDeleted": { + "method": "PUT", + "path": "/internal/accounts/deleted", + "request": {"userIds": []} + }, + "purge": { + "method": "POST", + "path": "/internal/accounts/purge", + "request": {"userIds": [42], "dryRun": false}, + "response": {"dryRun": true, "users": [42], "rows": {"user_game": 3}, "refused": false} + }, + "prune": { + "method": "POST", + "path": "/internal/accounts/prune", + "request": {"knownUserIds": [1, 42], "dryRun": true}, + "response": {"dryRun": true, "users": [42], "rows": {"user_game": 3}, "refused": false} + } + } +}