diff --git a/Contracts/Responses.cs b/Contracts/Responses.cs index 436b4f8..62b4ada 100644 --- a/Contracts/Responses.cs +++ b/Contracts/Responses.cs @@ -54,3 +54,28 @@ public record ResultItemDto( IReadOnlyList LinkedIds, IReadOnlyList LinkedTitles ); + +public record AuthSessionResponse(Guid Id, string Username, string? DisplayName, bool IsAdmin); + +public record StateSummaryResponse( + Phase CurrentPhase, + bool VotesFinal, + bool HasJoker, + bool ResultsOpen, + DateTimeOffset UpdatedAt, + int Players, + int Suggestions, + int Votes +); + +public record MeResponse( + Guid Id, + string Username, + string? DisplayName, + bool IsAdmin, + Phase CurrentPhase, + bool VotesFinal, + bool HasJoker +); + +public record PhaseTransitionResponse(Phase CurrentPhase, bool ResultsOpen); diff --git a/Endpoints/AdminEndpoints.cs b/Endpoints/AdminEndpoints.cs index de2a9bb..70ec4dd 100644 --- a/Endpoints/AdminEndpoints.cs +++ b/Endpoints/AdminEndpoints.cs @@ -36,7 +36,7 @@ public static class AdminEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.LinkSuggestionsAsync(player, request); }); @@ -45,7 +45,7 @@ public static class AdminEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.UnlinkSuggestionsAsync(player, request); }); diff --git a/Endpoints/AuthEndpoints.cs b/Endpoints/AuthEndpoints.cs index 766f4d3..506b77a 100644 --- a/Endpoints/AuthEndpoints.cs +++ b/Endpoints/AuthEndpoints.cs @@ -51,13 +51,12 @@ public static class AuthEndpoints await PlayerIdentityExtensions.SignInPlayerAsync(ctx, player); - return Results.Ok(new - { + return Results.Ok(new AuthSessionResponse( player.Id, player.Username, player.DisplayName, player.IsAdmin - }); + )); }); group.MapPost("/login", async ([FromBody] LoginRequest request, HttpContext ctx, AppDbContext db) => @@ -79,13 +78,12 @@ public static class AuthEndpoints await PlayerIdentityExtensions.SignInPlayerAsync(ctx, player); - return Results.Ok(new - { + return Results.Ok(new AuthSessionResponse( player.Id, player.Username, player.DisplayName, player.IsAdmin - }); + )); }); group.MapPost("/logout", async (HttpContext ctx) => diff --git a/Endpoints/ResultsEndpoints.cs b/Endpoints/ResultsEndpoints.cs index 57cae20..2bad7b0 100644 --- a/Endpoints/ResultsEndpoints.cs +++ b/Endpoints/ResultsEndpoints.cs @@ -16,7 +16,7 @@ public static class ResultsEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.GetResultsAsync(player); }); diff --git a/Endpoints/StateEndpoints.cs b/Endpoints/StateEndpoints.cs index dd5efef..41569b4 100644 --- a/Endpoints/StateEndpoints.cs +++ b/Endpoints/StateEndpoints.cs @@ -1,5 +1,6 @@ using GameList.Data; using GameList.Domain; +using GameList.Contracts; using Microsoft.EntityFrameworkCore; namespace GameList.Endpoints; @@ -14,21 +15,20 @@ public static class StateEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); var state = await db.AppState.AsNoTracking().FirstAsync(); var phase = EndpointHelpers.GetCurrentPhase(player.CurrentPhase, state.ResultsOpen); - var summary = new - { - CurrentPhase = phase, + var summary = new StateSummaryResponse( + phase, player.VotesFinal, player.HasJoker, state.ResultsOpen, state.UpdatedAt, - Players = await db.Players.CountAsync(), - Suggestions = await db.Suggestions.CountAsync(), - Votes = await db.Votes.CountAsync() - }; + await db.Players.CountAsync(), + await db.Suggestions.CountAsync(), + await db.Votes.CountAsync() + ); return Results.Ok(summary); }); @@ -36,27 +36,26 @@ public static class StateEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); var state = await db.AppState.AsNoTracking().FirstAsync(); var phase = EndpointHelpers.GetCurrentPhase(player.CurrentPhase, state.ResultsOpen); - return Results.Ok(new - { + return Results.Ok(new MeResponse( player.Id, - player.DisplayName, player.Username, + player.DisplayName, player.IsAdmin, - CurrentPhase = phase, + phase, player.VotesFinal, player.HasJoker - }); + )); }); group.MapPost("/me/phase/next", async (HttpContext ctx, AppDbContext db) => { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); var appState = await db.AppState.FirstAsync(); var reconciled = EndpointHelpers.ReconcilePlayerPhase(player, appState.ResultsOpen); @@ -72,18 +71,14 @@ public static class StateEndpoints player.CurrentPhase = next; player.VotesFinal = false; // moving forward clears any prior finalize await db.SaveChangesAsync(); - return Results.Ok(new - { - player.CurrentPhase, - appState.ResultsOpen - }); + return Results.Ok(new PhaseTransitionResponse(player.CurrentPhase, appState.ResultsOpen)); }); group.MapPost("/me/phase/prev", async (HttpContext ctx, AppDbContext db) => { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); var isAdmin = await EndpointHelpers.IsAdmin(ctx, db); if (!isAdmin) @@ -96,11 +91,7 @@ public static class StateEndpoints player.CurrentPhase = PrevPhase(player.CurrentPhase); player.VotesFinal = false; await db.SaveChangesAsync(); - return Results.Ok(new - { - player.CurrentPhase, - appState.ResultsOpen - }); + return Results.Ok(new PhaseTransitionResponse(player.CurrentPhase, appState.ResultsOpen)); }); } diff --git a/Endpoints/SuggestEndpoints.cs b/Endpoints/SuggestEndpoints.cs index ba92482..9a57648 100644 --- a/Endpoints/SuggestEndpoints.cs +++ b/Endpoints/SuggestEndpoints.cs @@ -15,7 +15,7 @@ public static class SuggestEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.GetMineAsync(player); }); @@ -24,7 +24,7 @@ public static class SuggestEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.CreateAsync(player, request); }).AddEndpointFilter(new PhaseOrJokerFilter()); @@ -33,7 +33,7 @@ public static class SuggestEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); var isAdmin = await EndpointHelpers.IsAdmin(ctx, db); return await service.DeleteAsync(player, isAdmin, id); @@ -43,7 +43,7 @@ public static class SuggestEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); var isAdmin = player.IsAdmin; return await service.UpdateAsync(player, isAdmin, id, request); @@ -53,7 +53,7 @@ public static class SuggestEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.GetAllAsync(player); }); diff --git a/Endpoints/VoteEndpoints.cs b/Endpoints/VoteEndpoints.cs index 29a390c..fbb600e 100644 --- a/Endpoints/VoteEndpoints.cs +++ b/Endpoints/VoteEndpoints.cs @@ -15,7 +15,7 @@ public static class VoteEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.GetMineAsync(player); }); @@ -24,7 +24,7 @@ public static class VoteEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.UpsertAsync(player, request); }); @@ -32,7 +32,7 @@ public static class VoteEndpoints { var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); return await service.SetFinalizeAsync(player, request); }); diff --git a/GameList.Tests/AuthTests.cs b/GameList.Tests/AuthTests.cs index 93e587d..cae84bf 100644 --- a/GameList.Tests/AuthTests.cs +++ b/GameList.Tests/AuthTests.cs @@ -161,6 +161,10 @@ public class AuthTests var resp = await player.GetAsync("/api/admin/vote-status"); Assert.Equal(HttpStatusCode.Unauthorized, resp.StatusCode); + var json = await resp.Content.ReadFromJsonAsync(); + Assert.Equal("Unauthorized", json.GetProperty("title").GetString()); + Assert.Equal("Unauthorized", json.GetProperty("detail").GetString()); + Assert.Equal("Unauthorized", json.GetProperty("error").GetString()); } [Fact] diff --git a/GameList.Tests/StateTests.cs b/GameList.Tests/StateTests.cs index 01c1aaf..cb3782e 100644 --- a/GameList.Tests/StateTests.cs +++ b/GameList.Tests/StateTests.cs @@ -212,7 +212,7 @@ public class StateTests await using var factory = new TestWebApplicationFactory(); var anon = factory.CreateClient(); var unauthorized = await anon.GetAsync("/api/state"); - Assert.NotEqual(HttpStatusCode.OK, unauthorized.StatusCode); + Assert.Equal(HttpStatusCode.Unauthorized, unauthorized.StatusCode); var client = factory.CreateClientWithCookies(); await client.RegisterAsync("counting"); @@ -224,6 +224,27 @@ public class StateTests Assert.True(suggestions.GetInt32() >= 1); } + [Fact] + public async Task State_endpoint_with_stale_cookie_returns_unauthorized_and_clears_cookie() + { + await using var factory = new TestWebApplicationFactory(); + var client = factory.CreateClientWithCookies(); + await client.RegisterAsync("stale"); + + await factory.WithDbContextAsync(async db => + { + var player = await db.Players.FirstAsync(); + db.Players.Remove(player); + await db.SaveChangesAsync(); + }); + + var resp = await client.GetAsync("/api/state"); + + Assert.Equal(HttpStatusCode.Unauthorized, resp.StatusCode); + Assert.True(resp.Headers.TryGetValues("Set-Cookie", out var cookies)); + Assert.Contains(cookies, c => c.Contains("player=", StringComparison.OrdinalIgnoreCase)); + } + [Fact] public async Task Health_endpoint_ok() { diff --git a/Infrastructure/AdminOnlyFilter.cs b/Infrastructure/AdminOnlyFilter.cs index 0d30132..63d4c1f 100644 --- a/Infrastructure/AdminOnlyFilter.cs +++ b/Infrastructure/AdminOnlyFilter.cs @@ -12,7 +12,7 @@ public class AdminOnlyFilter : IEndpointFilter var player = await EndpointHelpers.GetAuthenticatedPlayer(httpContext, db); if (player?.IsAdmin != true) { - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); } return await next(context); diff --git a/Infrastructure/PhaseOrJokerFilter.cs b/Infrastructure/PhaseOrJokerFilter.cs index 242a076..e6a27a2 100644 --- a/Infrastructure/PhaseOrJokerFilter.cs +++ b/Infrastructure/PhaseOrJokerFilter.cs @@ -16,7 +16,7 @@ public class PhaseOrJokerFilter : IEndpointFilter var db = httpContext.RequestServices.GetRequiredService(); var player = await EndpointHelpers.GetAuthenticatedPlayer(httpContext, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); var allow = phase == Phase.Suggest || (phase == Phase.Vote && player.HasJoker); diff --git a/Infrastructure/PhaseRequirementFilter.cs b/Infrastructure/PhaseRequirementFilter.cs index 2e805c0..1786b30 100644 --- a/Infrastructure/PhaseRequirementFilter.cs +++ b/Infrastructure/PhaseRequirementFilter.cs @@ -12,7 +12,7 @@ public class PhaseRequirementFilter(Phase required, bool allowAdminOverride = fa var db = httpContext.RequestServices.GetRequiredService(); var player = await EndpointHelpers.GetAuthenticatedPlayer(httpContext, db); if (player is null) - return Results.Unauthorized(); + return EndpointHelpers.UnauthorizedError(); var phase = await EndpointHelpers.GetCurrentPhaseAsync(db, player.Id); if (phase != required && !(allowAdminOverride && player.IsAdmin)) diff --git a/REVIEW.md b/REVIEW.md index 1fd1534..16f159d 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -10,7 +10,7 @@ Progress update (as of February 7, 2026): - Completed: build/test guardrails added (`.github/workflows/ci.yml`) and root ownership/setup docs added (`README.md:1`). - Completed: backend validators centralized for suggestions and auth (`Endpoints/SuggestionValidator.cs:7`, `Endpoints/AuthValidator.cs:11`). - Completed: request safety hardened for redirects and forwarded headers (`Program.cs:44`, `Program.cs:108`, `Endpoints/EndpointHelpers.cs:105`, `GameList.Tests/HelperTests.cs:121`, `GameList.Tests/HelperTests.cs:219`). -- In progress: API/response contract standardization now includes typed response DTOs, shared `ProblemDetails` helpers, and client parsing compatibility (`Contracts/Responses.cs:5`, `Endpoints/EndpointHelpers.cs:90`, `wwwroot/js/api.js:25`). +- Completed: API/response contract standardization now includes typed auth/state/workflow DTOs and shared `ProblemDetails` envelopes across endpoint/filter paths (`Contracts/Responses.cs:58`, `Endpoints/AuthEndpoints.cs:54`, `Endpoints/StateEndpoints.cs:22`, `Infrastructure/AdminOnlyFilter.cs:15`, `Endpoints/EndpointHelpers.cs:90`, `wwwroot/js/api.js:25`, `GameList.Tests/AuthTests.cs:164`). Top 5 maintainability risks (priority order): @@ -20,9 +20,9 @@ Top 5 maintainability risks (priority order): - Impact: hard-to-debug regressions and fragile refactors in UI workflows. 2. Rule duplication still present between backend/frontend validations (High) -- Suggestion validation is centralized on the backend (`Endpoints/SuggestionWorkflowService.cs:39`, `Endpoints/SuggestionWorkflowService.cs:115`, `Endpoints/SuggestionValidator.cs:7`) but frontend still duplicates parts (`wwwroot/js/ui.js:648`, `wwwroot/js/ui.js:1019`). -- Auth validation is centralized on the backend (`Endpoints/AuthEndpoints.cs:18`, `Endpoints/AuthEndpoints.cs:65`, `Endpoints/AuthValidator.cs:11`) while frontend length checks remain duplicated (`wwwroot/app.js:92`, `wwwroot/app.js:121`). -- Impact: inconsistent behavior and repeated fixes across server/client. +- Suggestion and auth validation are centralized on the backend (`Endpoints/SuggestionValidator.cs:7`, `Endpoints/AuthValidator.cs:11`, `Endpoints/AuthEndpoints.cs:18`, `Endpoints/AuthEndpoints.cs:65`). +- Frontend now keeps UX-only prechecks (required credentials and min/max player input affordances) instead of server-side policy duplication (`wwwroot/app.js:92`, `wwwroot/app.js:119`, `wwwroot/js/ui.js:645`). +- Impact: lower drift risk, though UI-side range hints still require occasional sync with server limits. 3. High-change, high-complexity frontend hotspots (High) - Git churn: `wwwroot/app.js` (76 changes), `wwwroot/js/ui.js` (55), `wwwroot/js/i18n.js` (50). @@ -31,8 +31,9 @@ Top 5 maintainability risks (priority order): - Impact: every UI change risks regressions outside its feature area. 4. Endpoint contract consistency and error shaping are still uneven (High) -- Service-layer extraction is now in place for suggestions, votes, admin, and results (`Endpoints/SuggestionWorkflowService.cs:8`, `Endpoints/VoteWorkflowService.cs:8`, `Endpoints/AdminWorkflowService.cs:8`, `Endpoints/ResultsWorkflowService.cs:8`), and typed response DTO adoption is underway (`Contracts/Responses.cs:5`, `Contracts/Responses.cs:35`), but not all auth/filter paths emit uniform error envelopes. -- Impact: API evolution and client compatibility changes are still high-friction. +- Service-layer extraction is in place for suggestions, votes, admin, and results (`Endpoints/SuggestionWorkflowService.cs:8`, `Endpoints/VoteWorkflowService.cs:8`, `Endpoints/AdminWorkflowService.cs:8`, `Endpoints/ResultsWorkflowService.cs:8`), and typed response DTOs now cover auth/state/workflow payloads (`Contracts/Responses.cs:5`, `Contracts/Responses.cs:58`, `Endpoints/AuthEndpoints.cs:54`, `Endpoints/StateEndpoints.cs:22`). +- Endpoint/filter unauthorized paths now use uniform `ProblemDetails` envelopes (`Infrastructure/AdminOnlyFilter.cs:15`, `Infrastructure/PhaseRequirementFilter.cs:15`, `Infrastructure/PhaseOrJokerFilter.cs:19`, `Endpoints/SuggestEndpoints.cs:18`, `Endpoints/VoteEndpoints.cs:18`, `Endpoints/ResultsEndpoints.cs:19`). +- Impact: API evolution risk is reduced; remaining inconsistency is framework-generated 401 challenge bodies for fully unauthenticated requests. 5. Static-analysis and frontend lint guardrails remain incomplete (Medium) - Build/test CI exists (`.github/workflows/ci.yml`) and project content rules are fixed (`GameList.csproj:17`-`GameList.csproj:21`), but analyzers/lint/format gates are still absent. @@ -100,11 +101,11 @@ Worst coupling points: - Effort / Risk: `M / Med`. - Dependencies (if any): none. -[P0][Partial] Centralize validation rules to stop backend/frontend drift -- Problem: Severity `High`, Category `Complexity/Duplication`. Validation rules are duplicated in multiple backend endpoints and frontend forms. Impact: inconsistent behavior and repeated fixes. -- Evidence: backend centralized in `Endpoints/SuggestionWorkflowService.cs:39`, `Endpoints/SuggestionWorkflowService.cs:115`, `Endpoints/SuggestionValidator.cs:7`, `Endpoints/AuthEndpoints.cs:18`, `Endpoints/AuthEndpoints.cs:65`, `Endpoints/AuthValidator.cs:11`; frontend duplicates remain in `wwwroot/js/ui.js:648`, `wwwroot/js/ui.js:1019`, `wwwroot/app.js:92`. -- Recommendation: Extract backend validators (e.g., `SuggestionValidator`, `AuthValidator`) and reuse in create/update paths; simplify frontend to UX-only prechecks and rely on server responses for source-of-truth. -- Acceptance criteria (testable): create/update share one backend validator path; tests cover validator once and both endpoints; frontend no longer re-implements server-only security rules. +[P0][Done] Centralize validation rules to stop backend/frontend drift +- Problem: Severity `High`, Category `Complexity/Duplication`. Validation rules were duplicated in multiple backend endpoints and frontend forms, increasing drift risk. +- Evidence: backend validators are centralized (`Endpoints/SuggestionValidator.cs:7`, `Endpoints/AuthValidator.cs:11`, `Endpoints/SuggestionWorkflowService.cs:39`, `Endpoints/SuggestionWorkflowService.cs:115`, `Endpoints/AuthEndpoints.cs:18`, `Endpoints/AuthEndpoints.cs:65`); frontend server-policy duplicates removed for auth length and image host/url security checks (`wwwroot/app.js:92`, `wwwroot/app.js:119`, `wwwroot/js/ui.js:645`). +- Recommendation: Keep backend validators as the source of truth; limit frontend checks to UX hints and required-field guidance. +- Acceptance criteria (testable): create/update share one backend validator path; tests cover validator-backed routes; frontend no longer re-implements server-only security rules. - Effort / Risk: `M / Med`. - Dependencies (if any): none. @@ -142,7 +143,7 @@ Worst coupling points: [P1] Remove legacy/dead paths to reduce cognitive load - Problem: Severity `Medium`, Category `Other`. Legacy `Reveal` phase and dead UI hooks remain in active code, increasing confusion. -- Evidence: `Domain/Phase.cs:6`, `Endpoints/StateEndpoints.cs:107`, `wwwroot/js/data.js:30`, `wwwroot/js/ui.js:156`, `wwwroot/js/ui.js:1191`. +- Evidence: `Domain/Phase.cs:6`, `Endpoints/StateEndpoints.cs:99`, `wwwroot/js/data.js:30`, `wwwroot/js/ui.js:156`, `wwwroot/js/ui.js:1149`. - Recommendation: Remove obsolete phase enum/value handling and dead UI references (`all-suggestions`, `nav-vote-next`). - Acceptance criteria (testable): no references to removed phase/UI ids remain; tests validate expected phase transitions only (`Suggest`, `Vote`, `Results`). - Effort / Risk: `S / Low`. @@ -164,11 +165,11 @@ Worst coupling points: - Effort / Risk: `M / Low`. - Dependencies (if any): P0 redirect-hardening task. -[P1][In Progress] Standardize API response contracts and error envelopes -- Problem: Severity `Medium`, Category `API/Contracts`. Success payloads are being typed and service-level errors now use a shared `ProblemDetails` helper, but some auth/filter paths still return bare status codes. -- Evidence: typed response DTOs added in `Contracts/Responses.cs:5` and used in service workflows (`Endpoints/SuggestionWorkflowService.cs:83`, `Endpoints/VoteWorkflowService.cs:83`, `Endpoints/AdminWorkflowService.cs:44`, `Endpoints/ResultsWorkflowService.cs:63`); shared error envelope helper added in `Endpoints/EndpointHelpers.cs:90` and used across workflows (`Endpoints/AdminWorkflowService.cs:98`, `Endpoints/SuggestionWorkflowService.cs:43`, `Endpoints/VoteWorkflowService.cs:32`, `Endpoints/ResultsWorkflowService.cs:14`); global exception handler now emits `ProblemDetails` with `error` compatibility field in `Infrastructure/PlayerIdentityExtensions.cs:48`; frontend parser supports `error`, `detail`, and `title` in `wwwroot/js/api.js:25`. -- Recommendation: finish migrating remaining auth/filter unauthorized paths to consistent problem payloads while keeping backward-compatible `error` fields during transition; continue replacing anonymous success payloads with DTOs. -- Acceptance criteria (testable): all API endpoints return typed success DTOs; all non-2xx responses include `ProblemDetails` fields and remain consumable by current frontend. +[P1][Done] Standardize API response contracts and error envelopes +- Problem: Severity `Medium`, Category `API/Contracts`. Success payloads and error envelopes were inconsistent across endpoint, service, and filter layers. +- Evidence: typed response DTOs now cover workflow + auth + state paths (`Contracts/Responses.cs:5`, `Contracts/Responses.cs:58`, `Endpoints/AuthEndpoints.cs:54`, `Endpoints/StateEndpoints.cs:22`, `Endpoints/SuggestionWorkflowService.cs:83`, `Endpoints/VoteWorkflowService.cs:83`, `Endpoints/AdminWorkflowService.cs:44`, `Endpoints/ResultsWorkflowService.cs:63`); shared `ProblemDetails` helper is used across services and endpoint filters (`Endpoints/EndpointHelpers.cs:90`, `Endpoints/SuggestEndpoints.cs:18`, `Endpoints/VoteEndpoints.cs:18`, `Infrastructure/AdminOnlyFilter.cs:15`, `Infrastructure/PhaseRequirementFilter.cs:15`, `Infrastructure/PhaseOrJokerFilter.cs:19`); global exception handling and client parsing remain compatible via `error`/`detail`/`title` (`Infrastructure/PlayerIdentityExtensions.cs:48`, `wwwroot/js/api.js:25`, `GameList.Tests/AuthTests.cs:164`). +- Recommendation: retain this contract as baseline and keep backward-compatible `error` extension in future error-shape changes. +- Acceptance criteria (testable): endpoint/service/filter responses use typed success payloads and standardized `ProblemDetails` error envelopes, with current frontend parsing unchanged. - Effort / Risk: `M / Med`. - Dependencies (if any): none. diff --git a/wwwroot/app.js b/wwwroot/app.js index 19d49e6..26e5f08 100644 --- a/wwwroot/app.js +++ b/wwwroot/app.js @@ -89,7 +89,6 @@ function setupHandlers() { e.preventDefault(); const username = $("login-username").value.trim(); const password = $("login-password").value; - if (username.length > 24) return toast("Username must be 24 characters or fewer.", true); if (!username || !password) return toast(t("auth.needCredentials"), true); if (!hasConsent() && !$("login-consent")?.checked) return toast(t("auth.cookieRequired"), true); try { @@ -117,8 +116,6 @@ function setupHandlers() { const displayName = $("register-displayName").value.trim(); const adminKey = $("register-adminkey").value.trim(); if (!displayName) return toast(t("toast.displayNameRequired") || "Display name is required.", true); - if (username.length > 24) return toast("Username must be 24 characters or fewer.", true); - if (displayName.length > 16) return toast("Display name must be 16 characters or fewer.", true); if (!username || !password) return toast(t("auth.needCredentials"), true); if (!hasConsent() && !$("register-consent")?.checked) return toast(t("auth.cookieRequired"), true); try { diff --git a/wwwroot/js/ui.js b/wwwroot/js/ui.js index e4c8a4e..6ca2c65 100644 --- a/wwwroot/js/ui.js +++ b/wwwroot/js/ui.js @@ -630,8 +630,6 @@ function openSuggestionModal({ title, submitLabel, initial = {}, onSubmit, lockT const errorBox = form.querySelector('[data-error="players"]'); const minInput = form.querySelector('input[name="minPlayers"]'); const maxInput = form.querySelector('input[name="maxPlayers"]'); - const screenshotError = form.querySelector('[data-error="screenshot"]'); - const screenshotInput = form.querySelector('input[name="screenshotUrl"]'); const markError = (msg) => { if (errorBox) { errorBox.textContent = msg; @@ -640,7 +638,6 @@ function openSuggestionModal({ title, submitLabel, initial = {}, onSubmit, lockT }; const clearError = () => { if (errorBox) errorBox.classList.add("hidden"); - if (screenshotError) screenshotError.classList.add("hidden"); }; clearError(); const min = data.minPlayers; @@ -657,15 +654,6 @@ function openSuggestionModal({ title, submitLabel, initial = {}, onSubmit, lockT markError(t("form.playersInvalid")); return; } - if (data.screenshotUrl && !isValidImageUrl(data.screenshotUrl)) { - if (screenshotError) { - screenshotError.textContent = t("form.screenshotInvalid"); - screenshotError.classList.remove("hidden"); - } - screenshotInput?.classList.add("input-error"); - return; - } - screenshotInput?.classList.remove("input-error"); if (!data.name?.trim()) return toast(t("toast.nameRequired"), true); try { await onSubmit(data, close, submitBtn); @@ -1016,36 +1004,6 @@ function openDeleteConfirmModal(s) { document.body.appendChild(overlay); } -function isValidImageUrl(url) { - if (!url) return true; - try { - const u = new URL(url); - const allowed = ["http:", "https:"]; - if (!allowed.includes(u.protocol)) return false; - const host = u.hostname.toLowerCase(); - const bannedHosts = [ - "bit.ly", - "tinyurl.com", - "t.co", - "goo.gl", - "ow.ly", - "is.gd", - "buff.ly", - "rebrand.ly", - "steamcommunity.com", - "store.steampowered.com", - ]; - if (bannedHosts.some((h) => host === h)) return false; - if (host === "imgur.com" && !u.pathname.startsWith("/a/") && !u.pathname.startsWith("/gallery/")) return false; - const path = u.pathname.toLowerCase(); - return [".png", ".jpg", ".jpeg", ".gif", ".webp", ".avif"].some((ext) => - path.endsWith(ext), - ); - } catch { - return false; - } -} - function linkRootId(s) { return s?.parentSuggestionId ?? s?.id; }