diff --git a/Infrastructure/EnsurePlayerExistsMiddleware.cs b/Infrastructure/EnsurePlayerExistsMiddleware.cs new file mode 100644 index 0000000..ad98321 --- /dev/null +++ b/Infrastructure/EnsurePlayerExistsMiddleware.cs @@ -0,0 +1,30 @@ +using GameList.Data; +using Microsoft.AspNetCore.Authentication; + +namespace GameList.Infrastructure; + +public class EnsurePlayerExistsMiddleware +{ + private readonly RequestDelegate _next; + + public EnsurePlayerExistsMiddleware(RequestDelegate next) + { + _next = next; + } + + public async Task InvokeAsync(HttpContext context, AppDbContext db) + { + if (context.User?.Identity?.IsAuthenticated == true) + { + var id = context.User.FindFirst(System.Security.Claims.ClaimTypes.NameIdentifier)?.Value; + if (string.IsNullOrWhiteSpace(id) || !Guid.TryParse(id, out var playerId) || await db.Players.FindAsync(playerId) is null) + { + await context.SignOutAsync(); + context.Response.StatusCode = StatusCodes.Status401Unauthorized; + return; + } + } + + await _next(context); + } +} diff --git a/Program.cs b/Program.cs index 339a5b2..ec0d732 100644 --- a/Program.cs +++ b/Program.cs @@ -80,6 +80,7 @@ if (!string.IsNullOrWhiteSpace(basePath)) app.UseGlobalExceptionLogging(); app.UseAuthentication(); +app.UseMiddleware(); app.UseAuthorization(); // Ensure database and migrations are applied on startup diff --git a/TASKS.md b/TASKS.md index 3cebef1..e5d8447 100644 --- a/TASKS.md +++ b/TASKS.md @@ -4,7 +4,7 @@ - [x] **Critical - Stored XSS via suggestion content/links** (`wwwroot/js/ui.js`, `wwwroot/index.html`, `Endpoints/SuggestEndpoints.cs`): Names, descriptions, and URLs are injected with `innerHTML` and unescaped links; `gameUrl`/`youtubeUrl` accept any scheme. A malicious suggestion can run script when viewed. Sanitize/escape on render (use `textContent`), validate URLs to http/https on the server, and consider HTML sanitization for descriptions. - [x] **High - SSRF surface in screenshot validation** (`Endpoints/EndpointHelpers.IsReachableImageAsync`, `SuggestEndpoints.cs`): The server performs HEAD/GET requests to arbitrary user-provided URLs with minimal limits, enabling internal port probing and latency spikes. Restrict to allowlisted hosts or disable remote fetch; at minimum block private IPs/localhost, disallow redirects, and enforce tight size/time limits. - [x] **High - Phase recalculation writes on every poll** (`Endpoints/EndpointHelpers.GetPhase`, `wwwroot/app.js`): `GetPhase` unconditionally calls `SaveChangesAsync`; the client polls `/api/state` every 4s, causing constant writes and SQLite WAL churn/locks even when nothing changes. Track a `changed` flag and persist only when state mutations occur; consider reducing poll frequency or using long polling/server push later. -- [ ] **Medium - Admin key accepted via query string** (`Endpoints/EndpointHelpers.IsAdmin`): Accepting `key` in the query leaks secrets via logs and referrers. Require the `X-Admin-Key` header (or authenticated admin account) only, and rate-limit/monitor attempts. +- [x] **Medium - Admin key accepted via query string** (`Endpoints/EndpointHelpers.IsAdmin`): Accepting `key` in the query leaks secrets via logs and referrers. Require the `X-Admin-Key` header (or authenticated admin account) only, and rate-limit/monitor attempts. - [x] **Medium - Brittle base path detection on the client** (`wwwroot/js/api.js`): The heuristic that picks the first path segment can double-prefix or break when the app is nested (e.g., `/picknplay/`). Prefer explicit `` or `` and drop the autodetect fallback. - [ ] **Maintenance - Centralize auth/phase concerns** (multiple endpoints): Each endpoint manually fetches the player, admin status, and phase gating, repeating logic and increasing drift risk. Introduce endpoint filters/middleware to attach the current player and enforce phase/admin requirements declaratively. -- [ ] **Maintenance - Clear invalid cookies** (`Infrastructure/PlayerIdentityExtensions.cs`): When a cookie references a deleted player, we keep reissuing it; endpoints respond 401 but the cookie persists. Clear the cookie if lookup fails to avoid client loops. +- [x] **Maintenance - Clear invalid cookies** (`Infrastructure/PlayerIdentityExtensions.cs`): When a cookie references a deleted player, we keep reissuing it; endpoints respond 401 but the cookie persists. Clear the cookie if lookup fails to avoid client loops.