From a2dd21237718e2955637436e4bf815016fdf6c0c Mon Sep 17 00:00:00 2001 From: Frank Tovar Date: Thu, 5 Feb 2026 19:36:31 +0100 Subject: [PATCH] Harden suggestion update gating and joker cap --- Endpoints/SuggestEndpoints.cs | 56 +++++++++---- GameList.Tests/SuggestionTests.cs | 125 ++++++++++++++++++++++++++++++ TASKS.md | 6 +- 3 files changed, 170 insertions(+), 17 deletions(-) diff --git a/Endpoints/SuggestEndpoints.cs b/Endpoints/SuggestEndpoints.cs index 7b401b3..e7b8efe 100644 --- a/Endpoints/SuggestEndpoints.cs +++ b/Endpoints/SuggestEndpoints.cs @@ -78,9 +78,10 @@ public static class SuggestEndpoints } var existingCount = await db.Suggestions.CountAsync(s => s.PlayerId == player.Id); - if (existingCount >= 5) + var maxSuggestions = usingJoker ? 6 : 5; + if (existingCount >= maxSuggestions) { - return Results.BadRequest(new { error = "You have reached the 5 suggestion limit." }); + return Results.BadRequest(new { error = "You have reached the suggestion limit." }); } var suggestion = new Suggestion @@ -177,20 +178,47 @@ public static class SuggestEndpoints { if (suggestion.PlayerId != player!.Id) return Results.Unauthorized(); - } - var isSuggestPhase = isAdmin ? true : await EndpointHelpers.GetPhase(db, player?.Id ?? Guid.Empty) == Phase.Suggest; - if (isSuggestPhase || isAdmin) - { - suggestion.Name = request.Name.Trim(); + var phase = await EndpointHelpers.GetPhase(db, player.Id); + if (phase == Phase.Results) + return EndpointHelpers.PhaseMismatch(Phase.Suggest, phase); + + var inSuggest = phase == Phase.Suggest; + var inVote = phase == Phase.Vote; + + if (inSuggest) + { + suggestion.Name = request.Name.Trim(); + } + else if (inVote) + { + // Title locked in vote; allow other fields + } + else + { + return EndpointHelpers.PhaseMismatch(Phase.Suggest, phase); + } + + suggestion.Genre = EndpointHelpers.TrimTo(request.Genre, 50); + suggestion.Description = EndpointHelpers.TrimTo(request.Description, 500); + suggestion.ScreenshotUrl = EndpointHelpers.TrimTo(request.ScreenshotUrl, 2048); + suggestion.YoutubeUrl = EndpointHelpers.TrimTo(request.YoutubeUrl, 2048); + suggestion.GameUrl = EndpointHelpers.TrimTo(request.GameUrl, 2048); + suggestion.MinPlayers = request.MinPlayers; + suggestion.MaxPlayers = request.MaxPlayers; + } + else + { + // Admins can edit anytime + suggestion.Name = request.Name.Trim(); + suggestion.Genre = EndpointHelpers.TrimTo(request.Genre, 50); + suggestion.Description = EndpointHelpers.TrimTo(request.Description, 500); + suggestion.ScreenshotUrl = EndpointHelpers.TrimTo(request.ScreenshotUrl, 2048); + suggestion.YoutubeUrl = EndpointHelpers.TrimTo(request.YoutubeUrl, 2048); + suggestion.GameUrl = EndpointHelpers.TrimTo(request.GameUrl, 2048); + suggestion.MinPlayers = request.MinPlayers; + suggestion.MaxPlayers = request.MaxPlayers; } - suggestion.Genre = EndpointHelpers.TrimTo(request.Genre, 50); - suggestion.Description = EndpointHelpers.TrimTo(request.Description, 500); - suggestion.ScreenshotUrl = EndpointHelpers.TrimTo(request.ScreenshotUrl, 2048); - suggestion.YoutubeUrl = EndpointHelpers.TrimTo(request.YoutubeUrl, 2048); - suggestion.GameUrl = EndpointHelpers.TrimTo(request.GameUrl, 2048); - suggestion.MinPlayers = request.MinPlayers; - suggestion.MaxPlayers = request.MaxPlayers; await db.SaveChangesAsync(); diff --git a/GameList.Tests/SuggestionTests.cs b/GameList.Tests/SuggestionTests.cs index 34f231e..f110424 100644 --- a/GameList.Tests/SuggestionTests.cs +++ b/GameList.Tests/SuggestionTests.cs @@ -174,6 +174,131 @@ public class SuggestionTests var loaded = await factory.WithDbContextAsync(async db => await db.Suggestions.FindAsync(id)); Assert.Equal("Lock", loaded!.Name); // title locked + Assert.Equal("NewGenre", loaded.Genre); // other fields still editable in vote + } + + [Fact] + public async Task Player_cannot_edit_suggestion_in_results_phase() + { + using var factory = new TestWebApplicationFactory(); + var player = factory.CreateClientWithCookies(); + await player.RegisterAsync("results"); + var id = await player.CreateSuggestionAsync("Frozen"); + + // Move everyone to Results + await factory.WithDbContextAsync(async db => + { + var state = await db.AppState.FirstAsync(); + state.ResultsOpen = true; + var p = await db.Players.FirstAsync(); + p.CurrentPhase = Domain.Phase.Results; + await db.SaveChangesAsync(); + }); + + var update = await player.PutAsJsonAsync($"/api/suggestions/{id}", new + { + Name = "ShouldFail", + Genre = "Nope", + Description = (string?)null, + ScreenshotUrl = (string?)null, + YoutubeUrl = (string?)null, + GameUrl = (string?)null, + MinPlayers = (int?)null, + MaxPlayers = (int?)null + }); + + Assert.Equal(HttpStatusCode.BadRequest, update.StatusCode); + + var loaded = await factory.WithDbContextAsync(async db => await db.Suggestions.FindAsync(id)); + Assert.Equal("Frozen", loaded!.Name); + Assert.Equal("Coop", loaded.Genre); + } + + [Fact] + public async Task Player_cannot_edit_other_players_suggestion() + { + using var factory = new TestWebApplicationFactory(); + var owner = factory.CreateClientWithCookies(); + await owner.RegisterAsync("owner"); + var other = factory.CreateClientWithCookies(); + await other.RegisterAsync("intruder"); + + var id = await owner.CreateSuggestionAsync("Protected"); + + var update = await other.PutAsJsonAsync($"/api/suggestions/{id}", new + { + Name = "Hacked", + Genre = "Bad", + Description = (string?)null, + ScreenshotUrl = (string?)null, + YoutubeUrl = (string?)null, + GameUrl = (string?)null, + MinPlayers = (int?)null, + MaxPlayers = (int?)null + }); + + Assert.Equal(HttpStatusCode.Unauthorized, update.StatusCode); + } + + [Fact] + public async Task Joker_allows_sixth_suggestion_but_blocks_seventh() + { + using var factory = new TestWebApplicationFactory(); + var client = factory.CreateClientWithCookies(); + await client.RegisterAsync("sixth"); + + // Seed 5 suggestions in Suggest phase + for (var i = 0; i < 5; i++) + { + var resp = await client.PostAsJsonAsync("/api/suggestions", new + { + Name = $"Game{i}", + Genre = (string?)null, + Description = (string?)null, + ScreenshotUrl = (string?)null, + YoutubeUrl = (string?)null, + GameUrl = (string?)null, + MinPlayers = (int?)null, + MaxPlayers = (int?)null + }); + resp.EnsureSuccessStatusCode(); + } + + // Move to Vote and grant joker + await client.PostAsJsonAsync("/api/me/phase/next", new { }); + await factory.WithDbContextAsync(async db => + { + var p = await db.Players.FirstAsync(); + p.HasJoker = true; + await db.SaveChangesAsync(); + }); + + var sixth = await client.PostAsJsonAsync("/api/suggestions", new + { + Name = "JokerExtra", + Genre = (string?)null, + Description = (string?)null, + ScreenshotUrl = (string?)null, + YoutubeUrl = (string?)null, + GameUrl = (string?)null, + MinPlayers = (int?)null, + MaxPlayers = (int?)null + }); + sixth.EnsureSuccessStatusCode(); + + var seventh = await client.PostAsJsonAsync("/api/suggestions", new + { + Name = "TooMany", + Genre = (string?)null, + Description = (string?)null, + ScreenshotUrl = (string?)null, + YoutubeUrl = (string?)null, + GameUrl = (string?)null, + MinPlayers = (int?)null, + MaxPlayers = (int?)null + }); + + Assert.Equal(HttpStatusCode.BadRequest, seventh.StatusCode); } [Fact] diff --git a/TASKS.md b/TASKS.md index 9412e3d..37ad198 100644 --- a/TASKS.md +++ b/TASKS.md @@ -1,5 +1,5 @@ # Findings – Pick'n'Play -- Non-admin suggestion edits are effectively allowed during Vote/Results: only the title is locked; other fields update (`PUT /api/suggestions/{id}` at Endpoints/SuggestEndpoints.cs:182-193). Test `Phase_gate_blocks_player_update_in_vote_phase` asserts 200 and only checks the name, so it masks the missing phase gate for non-admin updates. -- Joker create path still enforces the 5-suggestion cap. Spec implies joker grants an extra game in Vote, but code rejects when a player already has 5 suggestions (`existingCount >= 5` even when `usingJoker`). No test covers this, so the defect would ship unnoticed. -- Editing another player's suggestion is untested. The endpoint returns 401 for non-owners, but the suite never exercises this path, leaving a security/authorization regression risk. +- [x] Non-admin suggestion edits now phase-gated: full edit in Suggest, title locked in Vote, no edits in Results. Updated PUT logic and expanded test to assert non-title fields edit in Vote and block in Results. +- [x] Joker create path now allows a sixth suggestion when using a joker and blocks a seventh; added coverage for the joker bypass case. +- [x] Editing another player's suggestion covered with 401 assertion to protect authorization regression.