xTr1m/GameList
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove admin key support; admin must be authenticated

Browse Source
This commit is contained in:
Frank Tovar
2026-02-05 17:07:37 +01:00
parent 52960a78bc
commit 6b5f8a66c9
4 changed files with 23 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
Endpoints/AdminEndpoints.cs
View File

@@ -13,9 +13,9 @@ public static class AdminEndpoints
{ {
var admin = app.MapGroup("/api/admin"); var admin = app.MapGroup("/api/admin");
admin.MapPost("/results", async ([FromBody] Contracts.ResultsOpenRequest request, HttpContext ctx, AppDbContext db, IConfiguration config) => admin.MapPost("/results", async ([FromBody] Contracts.ResultsOpenRequest request, HttpContext ctx, AppDbContext db) =>
{ {
if (!await EndpointHelpers.IsAdmin(ctx, db, config)) return Results.Unauthorized(); if (!await EndpointHelpers.IsAdmin(ctx, db)) return Results.Unauthorized();
var state = await db.AppState.FirstAsync(); var state = await db.AppState.FirstAsync();
state.ResultsOpen = request.ResultsOpen; state.ResultsOpen = request.ResultsOpen;
@@ -36,9 +36,9 @@ public static class AdminEndpoints
return Results.Ok(new { currentState.ResultsOpen, currentState.UpdatedAt }); return Results.Ok(new { currentState.ResultsOpen, currentState.UpdatedAt });
}); });
admin.MapGet("/vote-status", async (HttpContext ctx, AppDbContext db, IConfiguration config) => admin.MapGet("/vote-status", async (HttpContext ctx, AppDbContext db) =>
{ {
if (!await EndpointHelpers.IsAdmin(ctx, db, config)) return Results.Unauthorized(); if (!await EndpointHelpers.IsAdmin(ctx, db)) return Results.Unauthorized();
var voters = await db.Players var voters = await db.Players
.AsNoTracking() .AsNoTracking()
@@ -59,9 +59,9 @@ public static class AdminEndpoints
return Results.Ok(new { voters, ready, waiting }); return Results.Ok(new { voters, ready, waiting });
}); });
admin.MapPost("/joker", async ([FromBody] GrantJokerRequest request, HttpContext ctx, AppDbContext db, IConfiguration config) => admin.MapPost("/joker", async ([FromBody] GrantJokerRequest request, HttpContext ctx, AppDbContext db) =>
{ {
if (!await EndpointHelpers.IsAdmin(ctx, db, config)) return Results.Unauthorized(); if (!await EndpointHelpers.IsAdmin(ctx, db)) return Results.Unauthorized();
var player = await db.Players.FirstOrDefaultAsync(p => p.Id == request.PlayerId); var player = await db.Players.FirstOrDefaultAsync(p => p.Id == request.PlayerId);
if (player is null) return Results.NotFound(new { error = "Player not found." }); if (player is null) return Results.NotFound(new { error = "Player not found." });
@@ -76,9 +76,9 @@ public static class AdminEndpoints
return Results.Ok(new { player.Id, player.HasJoker }); return Results.Ok(new { player.Id, player.HasJoker });
}); });
admin.MapDelete("/players/{playerId:guid}", async (Guid playerId, HttpContext ctx, AppDbContext db, IConfiguration config) => admin.MapDelete("/players/{playerId:guid}", async (Guid playerId, HttpContext ctx, AppDbContext db) =>
{ {
if (!await EndpointHelpers.IsAdmin(ctx, db, config)) return Results.Unauthorized(); if (!await EndpointHelpers.IsAdmin(ctx, db)) return Results.Unauthorized();
var player = await db.Players var player = await db.Players
.Include(p => p.Suggestions) .Include(p => p.Suggestions)
@@ -111,10 +111,10 @@ public static class AdminEndpoints
return Results.Ok(new { DeletedPlayerId = playerId }); return Results.Ok(new { DeletedPlayerId = playerId });
}); });
admin.MapPost("/link-suggestions", async ([FromBody] LinkSuggestionsRequest request, HttpContext ctx, AppDbContext db, IConfiguration config) => admin.MapPost("/link-suggestions", async ([FromBody] LinkSuggestionsRequest request, HttpContext ctx, AppDbContext db) =>
{ {
var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db);
if (player is null || !await EndpointHelpers.IsAdmin(ctx, db, config)) return Results.Unauthorized(); if (player is null || !await EndpointHelpers.IsAdmin(ctx, db)) return Results.Unauthorized();
var phase = await EndpointHelpers.GetPhase(db, player.Id); var phase = await EndpointHelpers.GetPhase(db, player.Id);
if (phase != Phase.Vote) if (phase != Phase.Vote)
@@ -183,10 +183,10 @@ public static class AdminEndpoints
}); });
}); });
admin.MapPost("/unlink-suggestions", async ([FromBody] UnlinkSuggestionsRequest request, HttpContext ctx, AppDbContext db, IConfiguration config) => admin.MapPost("/unlink-suggestions", async ([FromBody] UnlinkSuggestionsRequest request, HttpContext ctx, AppDbContext db) =>
{ {
var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db);
if (player is null || !await EndpointHelpers.IsAdmin(ctx, db, config)) return Results.Unauthorized(); if (player is null || !await EndpointHelpers.IsAdmin(ctx, db)) return Results.Unauthorized();
var phase = await EndpointHelpers.GetPhase(db, player.Id); var phase = await EndpointHelpers.GetPhase(db, player.Id);
if (phase != Phase.Vote) if (phase != Phase.Vote)
@@ -238,9 +238,9 @@ public static class AdminEndpoints
}); });
}); });
admin.MapPost("/reset", async (HttpContext ctx, AppDbContext db, IConfiguration config) => admin.MapPost("/reset", async (HttpContext ctx, AppDbContext db) =>
{ {
if (!await EndpointHelpers.IsAdmin(ctx, db, config)) return Results.Unauthorized(); if (!await EndpointHelpers.IsAdmin(ctx, db)) return Results.Unauthorized();
await db.Votes.ExecuteDeleteAsync(); await db.Votes.ExecuteDeleteAsync();
await db.Suggestions.ExecuteDeleteAsync(); await db.Suggestions.ExecuteDeleteAsync();
@@ -256,9 +256,9 @@ public static class AdminEndpoints
return Results.Ok(new { Phase = Phase.Suggest, state.ResultsOpen, state.UpdatedAt }); return Results.Ok(new { Phase = Phase.Suggest, state.ResultsOpen, state.UpdatedAt });
}); });
admin.MapPost("/factory-reset", async (HttpContext ctx, AppDbContext db, IConfiguration config) => admin.MapPost("/factory-reset", async (HttpContext ctx, AppDbContext db) =>
{ {
if (!await EndpointHelpers.IsAdmin(ctx, db, config)) return Results.Unauthorized(); if (!await EndpointHelpers.IsAdmin(ctx, db)) return Results.Unauthorized();
await using var tx = await db.Database.BeginTransactionAsync(); await using var tx = await db.Database.BeginTransactionAsync();

8
Endpoints/EndpointHelpers.cs
View File

@@ -235,14 +235,10 @@ internal static class EndpointHelpers
return uri.Scheme is "http" or "https"; return uri.Scheme is "http" or "https";
} }
public static async Task<bool> IsAdmin(HttpContext ctx, AppDbContext db, IConfiguration config) public static async Task<bool> IsAdmin(HttpContext ctx, AppDbContext db)
{ {
var player = await GetAuthenticatedPlayer(ctx, db); var player = await GetAuthenticatedPlayer(ctx, db);
if (player?.IsAdmin == true) return true; return player?.IsAdmin == true;
var provided = ctx.Request.Headers["X-Admin-Key"].FirstOrDefault();
var expected = config["ADMIN_PASSWORD"];
return !string.IsNullOrWhiteSpace(expected) && provided == expected;
} }
public static AppState NewAppState() => new() public static AppState NewAppState() => new()

8
Endpoints/SuggestEndpoints.cs
View File

@@ -109,11 +109,11 @@ public static class SuggestEndpoints
return Results.Created($"/api/suggestions/{suggestion.Id}", new { suggestion.Id }); return Results.Created($"/api/suggestions/{suggestion.Id}", new { suggestion.Id });
}); });
group.MapDelete("/{id:int}", async (int id, HttpContext ctx, AppDbContext db, IConfiguration config) => group.MapDelete("/{id:int}", async (int id, HttpContext ctx, AppDbContext db) =>
{ {
var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db);
if (player is null) return Results.Unauthorized(); if (player is null) return Results.Unauthorized();
var isAdmin = await EndpointHelpers.IsAdmin(ctx, db, config); var isAdmin = await EndpointHelpers.IsAdmin(ctx, db);
var phase = await EndpointHelpers.GetPhase(db, player.Id); var phase = await EndpointHelpers.GetPhase(db, player.Id);
if (!isAdmin && phase != Phase.Suggest) if (!isAdmin && phase != Phase.Suggest)
@@ -138,10 +138,10 @@ public static class SuggestEndpoints
return Results.NoContent(); return Results.NoContent();
}); });
group.MapPut("/{id:int}", async (int id, [FromBody] SuggestionRequest request, HttpContext ctx, AppDbContext db, IConfiguration config, IHttpClientFactory http) => group.MapPut("/{id:int}", async (int id, [FromBody] SuggestionRequest request, HttpContext ctx, AppDbContext db, IHttpClientFactory http) =>
{ {
var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db); var player = await EndpointHelpers.GetAuthenticatedPlayer(ctx, db);
var isAdmin = await EndpointHelpers.IsAdmin(ctx, db, config); var isAdmin = await EndpointHelpers.IsAdmin(ctx, db);
if (!isAdmin) if (!isAdmin)
{ {

2
TASKS.md
View File

@@ -2,7 +2,7 @@
- [x] **Critical - Unsigned auth cookie allows impersonation** (`Infrastructure/PlayerIdentityExtensions.cs`, `Endpoints/EndpointHelpers.cs`, `wwwroot/js/api.js`): The `player` cookie stores a raw GUID and is trusted on every request; `/api/suggestions/all` exposes other players' IDs, so a user can copy an ID, set the cookie, and become that account (including admins). Move to ASP.NET Core cookie auth with signed/encrypted tickets or a server-side session table, and avoid exposing player IDs where not necessary. - [x] **Critical - Unsigned auth cookie allows impersonation** (`Infrastructure/PlayerIdentityExtensions.cs`, `Endpoints/EndpointHelpers.cs`, `wwwroot/js/api.js`): The `player` cookie stores a raw GUID and is trusted on every request; `/api/suggestions/all` exposes other players' IDs, so a user can copy an ID, set the cookie, and become that account (including admins). Move to ASP.NET Core cookie auth with signed/encrypted tickets or a server-side session table, and avoid exposing player IDs where not necessary.
- [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] **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.
- [ ] **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 - 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. - [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. - [ ] **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 `<meta name="app-base">` or `<base>` and drop the autodetect fallback. - [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 `<meta name="app-base">` or `<base>` and drop the autodetect fallback.