From d2ab8a676fdd213da7cc3bb6f96acb164accdc18 Mon Sep 17 00:00:00 2001 From: Frank Tovar Date: Sun, 8 Feb 2026 21:48:07 +0100 Subject: [PATCH] Harden auth validation against null request fields --- Contracts/AuthRequests.cs | 4 ++-- Endpoints/AuthEndpoints.cs | 10 ++++++---- Endpoints/AuthValidator.cs | 8 ++++---- GameList.Tests/AuthTests.cs | 23 +++++++++++++++++++++++ TESTS.md | 1 + 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/Contracts/AuthRequests.cs b/Contracts/AuthRequests.cs index 24fb124..4ccdf9b 100644 --- a/Contracts/AuthRequests.cs +++ b/Contracts/AuthRequests.cs @@ -1,5 +1,5 @@ namespace GameList.Contracts; -public record RegisterRequest(string Username, string Password, string? DisplayName, string? AdminKey); +public record RegisterRequest(string? Username, string? Password, string? DisplayName, string? AdminKey); -public record LoginRequest(string Username, string Password); +public record LoginRequest(string? Username, string? Password); diff --git a/Endpoints/AuthEndpoints.cs b/Endpoints/AuthEndpoints.cs index d8c201f..b4da1a5 100644 --- a/Endpoints/AuthEndpoints.cs +++ b/Endpoints/AuthEndpoints.cs @@ -23,7 +23,7 @@ public static class AuthEndpoints { if (!AuthValidator.TryValidateRegistration(request, out var validated, out var registrationError)) { - authAttemptMonitor.RecordFailure(ctx, "auth-register", request.Username.Trim(), "validation-failed"); + authAttemptMonitor.RecordFailure(ctx, "auth-register", NormalizeActor(request.Username), "validation-failed"); return EndpointHelpers.BadRequestError(registrationError); } @@ -31,7 +31,7 @@ public static class AuthEndpoints if (exists) return EndpointHelpers.ConflictError("Username already taken."); - var (hash, salt) = PasswordHasher.HashPassword(request.Password); + var (hash, salt) = PasswordHasher.HashPassword(validated.Password); var expectedAdminKey = config["ADMIN_PASSWORD"]; var wantsAdmin = !string.IsNullOrWhiteSpace(validated.AdminKey); if (wantsAdmin) @@ -99,12 +99,12 @@ public static class AuthEndpoints { if (!AuthValidator.TryValidateLogin(request, out _, out var normalizedUsername, out var loginError)) { - authAttemptMonitor.RecordFailure(ctx, "auth-login", request.Username.Trim(), "validation-failed"); + authAttemptMonitor.RecordFailure(ctx, "auth-login", NormalizeActor(request.Username), "validation-failed"); return EndpointHelpers.BadRequestError(loginError); } var player = await db.Players.FirstOrDefaultAsync(p => p.NormalizedUsername == normalizedUsername); - if (player == null || !PasswordHasher.Verify(request.Password, player.PasswordHash, player.PasswordSalt)) + if (player == null || !PasswordHasher.Verify(request.Password ?? string.Empty, player.PasswordHash, player.PasswordSalt)) { authAttemptMonitor.RecordFailure(ctx, "auth-login", normalizedUsername, "invalid-credentials"); return EndpointHelpers.UnauthorizedError("Invalid username or password."); @@ -135,4 +135,6 @@ public static class AuthEndpoints return Results.NoContent(); }); } + + private static string NormalizeActor(string? username) => string.IsNullOrWhiteSpace(username) ? "(missing)" : username.Trim(); } diff --git a/Endpoints/AuthValidator.cs b/Endpoints/AuthValidator.cs index 6d3dd09..26c4d6e 100644 --- a/Endpoints/AuthValidator.cs +++ b/Endpoints/AuthValidator.cs @@ -12,7 +12,7 @@ internal static class AuthValidator public static bool TryValidateRegistration(RegisterRequest request, out ValidatedRegistration validated, out string error) { - var username = (request.Username).Trim(); + var username = (request.Username ?? string.Empty).Trim(); if (string.IsNullOrWhiteSpace(username) || username.Length > MaxUsernameLength) { validated = default; @@ -61,14 +61,14 @@ internal static class AuthValidator } var adminKey = EndpointHelpers.TrimTo(request.AdminKey, MaxAdminKeyLength); - validated = new ValidatedRegistration(username, username.ToLowerInvariant(), displayName, adminKey); + validated = new ValidatedRegistration(username, username.ToLowerInvariant(), password, displayName, adminKey); error = string.Empty; return true; } public static bool TryValidateLogin(LoginRequest request, out string username, out string normalizedUsername, out string error) { - username = (request.Username).Trim(); + username = (request.Username ?? string.Empty).Trim(); normalizedUsername = string.Empty; if (string.IsNullOrWhiteSpace(username) || string.IsNullOrWhiteSpace(request.Password)) @@ -94,5 +94,5 @@ internal static class AuthValidator return true; } - public readonly record struct ValidatedRegistration(string Username, string NormalizedUsername, string DisplayName, string? AdminKey); + public readonly record struct ValidatedRegistration(string Username, string NormalizedUsername, string Password, string DisplayName, string? AdminKey); } diff --git a/GameList.Tests/AuthTests.cs b/GameList.Tests/AuthTests.cs index 1c24f68..12595f4 100644 --- a/GameList.Tests/AuthTests.cs +++ b/GameList.Tests/AuthTests.cs @@ -212,6 +212,29 @@ public class AuthTests Assert.Equal(HttpStatusCode.BadRequest, badKey.StatusCode); } + [Fact] + public async Task Register_and_login_with_null_fields_return_bad_request() + { + await using var factory = new TestWebApplicationFactory(); + var client = factory.CreateClientWithCookies(); + + var register = await client.PostAsJsonAsync("/api/auth/register", new + { + Username = (string?)null, + Password = (string?)null, + DisplayName = (string?)null, + AdminKey = (string?)null + }); + Assert.Equal(HttpStatusCode.BadRequest, register.StatusCode); + + var login = await client.PostAsJsonAsync("/api/auth/login", new + { + Username = (string?)null, + Password = (string?)null + }); + Assert.Equal(HttpStatusCode.BadRequest, login.StatusCode); + } + [Fact] public async Task Non_admin_cannot_access_admin_routes() { diff --git a/TESTS.md b/TESTS.md index a7b0d48..2383387 100644 --- a/TESTS.md +++ b/TESTS.md @@ -33,6 +33,7 @@ stateDiagram-v2 ### 1) Authentication & Identity - Register success (player, admin key path) issues cookie, trims fields, stores normalized username, hashes password. - Register rejects missing/long username, weak password policy violations, missing display name, duplicate username, bad admin key, >24 chars username, >16 display name. +- Register/login null payload fields fail closed with `400` (no `500` on malformed JSON bodies). - Bootstrap-admin key path only works until the owner account exists; bootstrap admin is marked as owner. - Database uniqueness guard enforces single owner row (`IsOwner=true`) even if writes bypass endpoint-level checks. - `/api/auth/options` reports owner presence for registration UI behavior.