refactor(user): check only the shape of a tile id

Core and the Frontend skip ids they do not know; Core never shares the overview or the activity.
This commit is contained in:
lobo committed 2026-09-30 15:00:47 +02:00
1 parent 06f8089f5a
commit 89e2fa3df2
6 files changed
+42 -39

No files matched your search

+2 -2
View File
@@ -102,9 +102,9 @@ To Be Played is split into three repositories:
| `GET` | `/api/steam-link/callback` | public, Steam's redirect back; binding cookie; redirects to the frontend settings | | `GET` | `/api/steam-link/callback` | public, Steam's redirect back; binding cookie; redirects to the frontend settings |
| `PUT` | `/api/users/settings/appearance` | access token | | `PUT` | `/api/users/settings/appearance` | access token |
| `PUT` | `/api/users/settings/profile-style` (`{accent, corners, bannerAppid, backgroundAppid}`; `204`) | access token | | `PUT` | `/api/users/settings/profile-style` (`{accent, corners, bannerAppid, backgroundAppid}`; `204`) | access token |
| `PUT` | `/api/users/settings/dashboard-tiles` (`{tiles: [{id, size}]}` in order, at most 20; `{tiles: null}` resets; `204`) | access token | | `PUT` | `/api/users/settings/dashboard-tiles` (`{tiles: [{id, size}]}` in order, at most 20; `{tiles: null}` resets; `204`). Auth checks only the id shape `[A-Z_]{1,30}`: Core and the Frontend skip ids they do not know, and Core never shares the overview or the activity | access token |
| `PUT` | `/api/users/settings/favorite-platform` (`{platform}`, free text up to 32, e.g. `PS5`; `null` clears it; `204`) | access token | | `PUT` | `/api/users/settings/favorite-platform` (`{platform}`, free text up to 32, e.g. `PS5`; `null` clears it; `204`) | access token |
| `PUT` | `/api/users/settings/profile-tiles` (`{tiles: [{id, size}]}`: `SUMMARY`, `STATUS`, `PLAYING`, `FINISHED`, `STATS_<GROUP>`; `204`) | access token | | `PUT` | `/api/users/settings/profile-tiles` (`{tiles: [{id, size}]}`, e.g. `SUMMARY`, `STATUS`, `PLAYING`, `FINISHED`, `STATS_<GROUP>`; `204`) | access token |
| `GET` | `/api/public/profiles/{displayName}` | public | | `GET` | `/api/public/profiles/{displayName}` | public |
| `GET` | `/api/stats/public` | public | | `GET` | `/api/stats/public` | public |
| `GET` | `/api/version` (`{commit, builtAt}` of the running image, from the CI build; `dev` locally) | public | | `GET` | `/api/version` (`{commit, builtAt}` of the running image, from the CI build; `dev` locally) | public |
@@ -1,7 +1,6 @@
package auth_service.modules.user.api.dto.request; package auth_service.modules.user.api.dto.request;
import jakarta.validation.Valid; import jakarta.validation.Valid;
import jakarta.validation.constraints.AssertTrue;
import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size; import jakarta.validation.constraints.Size;
@@ -11,12 +10,4 @@ import java.util.List;
* @param tiles the shown tiles in their order; null resets to the default dashboard * @param tiles the shown tiles in their order; null resets to the default dashboard
*/ */
public record DashboardTilesRequest(@Size(max = 20) List<@NotNull @Valid TileRequest> tiles) { public record DashboardTilesRequest(@Size(max = 20) List<@NotNull @Valid TileRequest> tiles) {
public static final String IDS = "GAME_COUNT|MOST_PLAYED|PLAYING|PILE_OF_SHAME|ACTIVITY|ACHIEVEMENTS|STATS_OVERVIEW|"
+ ProfileTilesRequest.STAT_IDS;
@AssertTrue(message = "unknown tile id")
boolean isKnownIds() {
return TileRequest.knownIds(tiles, IDS);
}
} }
@@ -1,24 +1,13 @@
package auth_service.modules.user.api.dto.request; package auth_service.modules.user.api.dto.request;
import jakarta.validation.Valid; import jakarta.validation.Valid;
import jakarta.validation.constraints.AssertTrue;
import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size; import jakarta.validation.constraints.Size;
import java.util.List; import java.util.List;
/** /**
* @param tiles the shown parts of the public profile in their order; the STATS_ tiles are the shared stat groups * @param tiles the shown parts of the public profile in their order; the STATS_ tiles are Core's stat groups
*/ */
public record ProfileTilesRequest(@NotNull @Size(max = 20) List<@NotNull @Valid TileRequest> tiles) { public record ProfileTilesRequest(@NotNull @Size(max = 20) List<@NotNull @Valid TileRequest> tiles) {
/** The stat groups of Core's stats page, as tiles. */
public static final String STAT_IDS = "STATS_PILE|STATS_TREND|STATS_TOP_GAMES|STATS_TASTE|STATS_HABITS|STATS_FINISHED";
public static final String IDS = "SUMMARY|STATUS|PLAYING|FINISHED|" + STAT_IDS;
@AssertTrue(message = "unknown tile id")
boolean isKnownIds() {
return TileRequest.knownIds(tiles, IDS);
}
} }
@@ -3,17 +3,10 @@ package auth_service.modules.user.api.dto.request;
import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Pattern; import jakarta.validation.constraints.Pattern;
import java.util.List;
/** /**
* The id is checked by the layout request that holds the tile, with {@link #knownIds}. * @param id any tile id of the Frontend; Core and the Frontend skip ids they do not know. At most 30 characters:
* the column stores "ID:SIZE" in 32.
*/ */
public record TileRequest(@NotNull String id, @NotNull @Pattern(regexp = "S|M|L") String size) { public record TileRequest(@NotNull @Pattern(regexp = "[A-Z_]{1,30}") String id,
@NotNull @Pattern(regexp = "S|M|L") String size) {
/**
* @return true when every id matches; a missing list, tile or id is left to its own constraint
*/
static boolean knownIds(List<TileRequest> tiles, String ids) {
return tiles == null || tiles.stream().allMatch(tile -> tile == null || tile.id() == null || tile.id().matches(ids));
}
} }
@@ -467,19 +467,32 @@ class UserControllerTest extends AbstractWebMvcTest {
verify(userService).updateProfileTiles(1L, List.of(new Tile("STATS_PILE", "M"))); verify(userService).updateProfileTiles(1L, List.of(new Tile("STATS_PILE", "M")));
} }
// A dashboard tile is no profile tile and the other way round; activity and the game count stay private. // Core and the Frontend know the tiles; Auth checks only the shape (Core keeps activity and the overview private).
@Test
void updateTiles_ANewTileIdIsPassedThrough() throws Exception {
SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken(
new UserPrincipal("1", null), null, Collections.emptyList()));
mockMvc.perform(put("/api/users/settings/profile-tiles")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"tiles\":[{\"id\":\"STATS_NEW_GROUP\",\"size\":\"M\"}]}"))
.andExpect(status().isNoContent());
verify(userService).updateProfileTiles(1L, List.of(new Tile("STATS_NEW_GROUP", "M")));
}
@ParameterizedTest @ParameterizedTest
@ValueSource(strings = { @ValueSource(strings = {
"dashboard-tiles|{\"tiles\":[{\"id\":\"SUMMARY\",\"size\":\"M\"}]}", "dashboard-tiles|{\"tiles\":[{\"id\":\"stats_pile\",\"size\":\"M\"}]}",
"dashboard-tiles|{\"tiles\":[{\"id\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\",\"size\":\"M\"}]}",
"dashboard-tiles|{\"tiles\":[{\"id\":\"PLAYING\",\"size\":\"XL\"}]}", "dashboard-tiles|{\"tiles\":[{\"id\":\"PLAYING\",\"size\":\"XL\"}]}",
"dashboard-tiles|{\"tiles\":[{\"id\":\"PLAYING\"}]}", "dashboard-tiles|{\"tiles\":[{\"id\":\"PLAYING\"}]}",
"dashboard-tiles|{\"tiles\":[null]}", "dashboard-tiles|{\"tiles\":[null]}",
"profile-tiles|{\"tiles\":[{\"id\":\"ACTIVITY\",\"size\":\"M\"}]}", "profile-tiles|{\"tiles\":[{\"id\":\"\",\"size\":\"M\"}]}",
"profile-tiles|{\"tiles\":[{\"id\":\"STATS_OVERVIEW\",\"size\":\"M\"}]}",
"profile-tiles|{\"tiles\":null}", "profile-tiles|{\"tiles\":null}",
"profile-tiles|{}" "profile-tiles|{}"
}) })
void updateTiles_UnknownIdBadSizeOrMissing_IsRejected(String pathAndBody) throws Exception { void updateTiles_BadIdBadSizeOrMissing_IsRejected(String pathAndBody) throws Exception {
SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken( SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken(
new UserPrincipal("1", null), null, Collections.emptyList())); new UserPrincipal("1", null), null, Collections.emptyList()));
String[] parts = pathAndBody.split("\\|", 2); String[] parts = pathAndBody.split("\\|", 2);
@@ -357,6 +357,23 @@ class UserRepositoryTest extends AbstractPersistenceTest {
assertNull(findByEmail("reset@example.com").orElseThrow().getSettings().getDashboardTiles()); assertNull(findByEmail("reset@example.com").orElseThrow().getSettings().getDashboardTiles());
} }
@Test
void updateTiles_TheLongestIdAuthAcceptsIsStored() {
UserEntity userEntity = persisted(UserEntity.builder()
.email("longtile@example.com")
.password("password")
.enabled(true)
.registrationDate(LocalDateTime.now())
.build());
List<Tile> profile = List.of(new Tile("STATS_" + "A".repeat(24), "M"));
userRepository.update(userEntity.getId(), user -> user.getSettings().setProfileTiles(profile));
entityManager.flush();
entityManager.clear();
assertEquals(profile, findByEmail("longtile@example.com").orElseThrow().getSettings().getProfileTiles());
}
private void updateSettings(long userId, String displayName, UserSettings settings) { private void updateSettings(long userId, String displayName, UserSettings settings) {
userRepository.update(userId, user -> { userRepository.update(userId, user -> {
user.setDisplayName(displayName); user.setDisplayName(displayName);