From adf21f766a1270732262c6779904b272b4ae9d21 Mon Sep 17 00:00:00 2001 From: bitbiter-dev Date: Thu, 29 Oct 2026 22:15:56 +1300 Subject: [PATCH 1/2] feat(api): Auth errors carry a machine-readable code (#357) Every failure of login (bearer and web), register, refresh and change-password, and every rate-limiter rejection, is now application/problem+json with a stable `code` extension and the old English text in `detail`. Failures of one request field are ValidationProblems with the code listed under that field in `errors` (username, email, password, currentPassword, newPassword). Status codes are unchanged. The sliding-window limiters report no retry time, so a rate-limited response never carried Retry-After. Each policy now answers with its whole window (60 s sensitive, 900 s refresh), after which every permit is back. Contract tests run the real endpoints, mapper and limiter on the in-process test host and pin each code, status, content type and Retry-After with literals. Co-Authored-By: Claude Opus 5.5 --- .../Endpoints/AuthErrorContractTests.cs | 263 ++++++++++++++++++ .../Endpoints/AuthResponseMapperTests.cs | 134 --------- .../Infrastructure/AuthenticatedTestApp.cs | 6 +- .../Infrastructure/RateLimitingTests.cs | 59 ---- src/Anichron.API/Endpoints/AuthEndpoints.cs | 5 +- src/Anichron.API/Endpoints/AuthProblems.cs | 45 +++ .../Endpoints/AuthResponseMapper.cs | 85 +++--- .../Infrastructure/RateLimiting.cs | 41 ++- .../ServiceCollectionExtensions.cs | 33 +-- 9 files changed, 402 insertions(+), 269 deletions(-) create mode 100644 src/Anichron.API.Tests.Unit/Endpoints/AuthErrorContractTests.cs create mode 100644 src/Anichron.API/Endpoints/AuthProblems.cs diff --git a/src/Anichron.API.Tests.Unit/Endpoints/AuthErrorContractTests.cs b/src/Anichron.API.Tests.Unit/Endpoints/AuthErrorContractTests.cs new file mode 100644 index 00000000..e31dfd97 --- /dev/null +++ b/src/Anichron.API.Tests.Unit/Endpoints/AuthErrorContractTests.cs @@ -0,0 +1,263 @@ +using Anichron.API.Endpoints; +using Anichron.API.Services; +using Anichron.API.Settings; +using Anichron.API.Tests.Unit.Infrastructure; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; + +namespace Anichron.API.Tests.Unit.Endpoints; + +public sealed class AuthErrorContractTests +{ + private const string LoginPath = "/api/v1/auth/login"; + private const string LoginMobilePath = "/api/v1/auth/login/mobile"; + private const string RefreshPath = "/api/v1/auth/refresh"; + private const string RegisterPath = "/api/v1/auth/register"; + private const string ChangePasswordPath = "/api/v1/users/me/password"; + + private readonly IAuthService auth = Substitute.For(); + + private Task StartAsync(bool rateLimited = false) + => AuthenticatedTestApp.StartAsync( + api => + { + api.MapAuthEndpoints(); + api.MapUserEndpoints(); + }, + services => + { + services.AddSingleton(auth); + services.AddSingleton(new AuthCookieSettings { SameSite = SameSiteMode.Strict, RefreshTokenDays = 7 }); + services.AddSingleton(Substitute.For()); + services.AddSingleton(); + }, + rateLimited); + + private void LoginFails(AuthResult result) + => auth.LoginAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(result); + + private void RefreshFails(AuthResult result) + => auth.RefreshAsync(Arg.Any(), Arg.Any()).Returns(result); + + private static readonly object RegisterBody = + new { username = "alice", email = "alice@example.com", password = "SecurePass1!", inviteToken = "inv" }; + + private static async Task ChangePasswordAsync(AuthenticatedTestApp app) + { + using var request = new HttpRequestMessage(HttpMethod.Post, ChangePasswordPath) + { + Content = JsonContent.Create(new { currentPassword = "OldPass1!", newPassword = "NewPass1!" }), + }; + request.Headers.Add(AuthenticatedTestApp.UserHeader, Guid.NewGuid().ToString()); + return await app.Client.SendAsync(request, TestContext.Current.CancellationToken); + } + + private static Task PostAsync(AuthenticatedTestApp app, string path, object body) + => app.Client.PostAsJsonAsync(path, body, TestContext.Current.CancellationToken); + + private static async Task AssertFieldProblemAsync( + HttpResponseMessage response, int status, string code, string field, string detail) + { + ((int)response.StatusCode).Should().Be(status); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be(code); + problem.GetProperty("status").GetInt32().Should().Be(status); + problem.GetProperty("detail").GetString().Should().Be(detail); + var errors = problem.GetProperty("errors").EnumerateObject().ToList(); + errors.Should().ContainSingle().Which.Name.Should().Be(field); + errors[0].Value.EnumerateArray().Select(e => e.GetString()).Should().Equal(code); + } + + private static async Task ProblemAsync(HttpResponseMessage response) + { + response.Content.Headers.ContentType!.MediaType.Should().Be("application/problem+json"); + using var body = JsonDocument.Parse(await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken)); + return body.RootElement.Clone(); + } + + [Fact] + public async Task Login_WrongCredentials_Is401InvalidCredentialsAsync() + { + LoginFails(AuthResult.Fail(AuthError.InvalidCredentials)); + await using var app = await StartAsync(); + + using var response = await PostAsync(app, LoginPath, new { usernameOrEmail = "alice", password = "wrong" }); + + response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be("invalid_credentials"); + problem.GetProperty("status").GetInt32().Should().Be(401); + problem.GetProperty("detail").GetString().Should().Be("The username, email, or password is incorrect."); + } + + [Theory] + [InlineData(LoginPath)] + [InlineData(LoginMobilePath)] + public async Task Login_DisabledAccount_Is401AccountDisabledAsync(string path) + { + LoginFails(AuthResult.Fail(AuthError.AccountDisabled)); + await using var app = await StartAsync(); + + using var response = await PostAsync(app, path, new { usernameOrEmail = "alice", password = "SecurePass1!" }); + + response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be("account_disabled"); + problem.GetProperty("detail").GetString().Should().Be("This account has been disabled. Please contact an administrator."); + } + + [Fact] + public async Task Login_LockedAccount_Is429AccountLockedWithRetryAfterAsync() + { + LoginFails(AuthResult.Locked(60)); + await using var app = await StartAsync(); + + using var response = await PostAsync(app, LoginPath, new { usernameOrEmail = "alice", password = "wrong" }); + + response.StatusCode.Should().Be(HttpStatusCode.TooManyRequests); + response.Headers.RetryAfter!.Delta.Should().Be(TimeSpan.FromSeconds(60)); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be("account_locked"); + problem.GetProperty("status").GetInt32().Should().Be(429); + problem.GetProperty("detail").GetString().Should().Be("Too many failed login attempts. Please try again in 60 seconds."); + } + + [Fact] + public async Task Refresh_NoToken_Is401RefreshTokenRequiredAsync() + { + await using var app = await StartAsync(); + + using var response = await app.Client.PostAsync(RefreshPath, content: null, TestContext.Current.CancellationToken); + + response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be("refresh_token_required"); + problem.GetProperty("detail").GetString().Should().Be("Refresh token is required."); + } + + [Theory] + [InlineData(AuthError.TokenInvalid, "refresh_token_invalid", "Refresh token is invalid or has expired.")] + [InlineData(AuthError.AccountDisabled, "account_disabled", "This account has been disabled. Please contact an administrator.")] + public async Task Refresh_Rejected_Is401WithItsCodeAsync(AuthError error, string code, string detail) + { + RefreshFails(AuthResult.Fail(error)); + await using var app = await StartAsync(); + + using var response = await PostAsync(app, RefreshPath, new { refreshToken = "tok" }); + + response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be(code); + problem.GetProperty("status").GetInt32().Should().Be(401); + problem.GetProperty("detail").GetString().Should().Be(detail); + } + + [Fact] + public async Task Refresh_LockedAccount_Is429AccountLockedWithRetryAfterAsync() + { + RefreshFails(AuthResult.Locked(1)); + await using var app = await StartAsync(); + + using var response = await PostAsync(app, RefreshPath, new { refreshToken = "tok" }); + + response.StatusCode.Should().Be(HttpStatusCode.TooManyRequests); + response.Headers.RetryAfter!.Delta.Should().Be(TimeSpan.FromSeconds(1)); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be("account_locked"); + problem.GetProperty("detail").GetString().Should().Be("Too many failed login attempts. Please try again in 1 second."); + } + + [Fact] + public async Task Login_OverTheRateLimit_Is429RateLimitedWithRetryAfterAsync() + { + LoginFails(AuthResult.Fail(AuthError.InvalidCredentials)); + await using var app = await StartAsync(rateLimited: true); + for (var i = 0; i < 10; i++) + (await PostAsync(app, LoginPath, new { usernameOrEmail = "alice", password = "wrong" })).Dispose(); + + using var response = await PostAsync(app, LoginPath, new { usernameOrEmail = "alice", password = "wrong" }); + + response.StatusCode.Should().Be(HttpStatusCode.TooManyRequests); + response.Headers.RetryAfter!.Delta.Should().Be(TimeSpan.FromSeconds(60)); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be("rate_limited"); + problem.GetProperty("status").GetInt32().Should().Be(429); + problem.GetProperty("detail").GetString().Should().Be("Too many requests. Please try again later."); + } + + [Fact] + public async Task Refresh_OverTheRateLimit_Is429RateLimitedWithRetryAfterAsync() + { + RefreshFails(AuthResult.Fail(AuthError.TokenInvalid)); + await using var app = await StartAsync(rateLimited: true); + for (var i = 0; i < 5; i++) + (await PostAsync(app, RefreshPath, new { refreshToken = "tok" })).Dispose(); + + using var response = await PostAsync(app, RefreshPath, new { refreshToken = "tok" }); + + response.StatusCode.Should().Be(HttpStatusCode.TooManyRequests); + response.Headers.RetryAfter!.Delta.Should().Be(TimeSpan.FromMinutes(15)); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be("rate_limited"); + } + + [Theory] + [InlineData(AuthError.InvalidUsername, 422, "invalid_username", "username", + "Username must be 3–32 characters and may only contain letters, digits, underscores, and hyphens.")] + [InlineData(AuthError.InvalidEmail, 422, "invalid_email", "email", "The provided email address is not valid.")] + [InlineData(AuthError.UsernameTaken, 409, "username_taken", "username", "Username is already taken.")] + [InlineData(AuthError.EmailTaken, 409, "email_taken", "email", "An account with this email already exists.")] + [InlineData(AuthError.PasswordTooShort, 422, "password_too_short", "password", "Password must be at least 12 characters.")] + [InlineData(AuthError.PasswordTooLong, 422, "password_too_long", "password", "Password must be no longer than 128 characters.")] + [InlineData(AuthError.PasswordPwned, 422, "password_pwned", "password", + "This password has appeared in a known data breach. Please choose a different one.")] + public async Task Register_FieldRejected_IsAValidationProblemOnThatFieldAsync( + AuthError error, int status, string code, string field, string detail) + { + auth.RegisterAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(AuthResult.Fail(error)); + await using var app = await StartAsync(); + + using var response = await PostAsync(app, RegisterPath, RegisterBody); + + await AssertFieldProblemAsync(response, status, code, field, detail); + } + + [Fact] + public async Task Register_InvalidInvite_Is422InviteTokenInvalidAsync() + { + auth.RegisterAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(AuthResult.Fail(AuthError.InviteTokenInvalid)); + await using var app = await StartAsync(); + + using var response = await PostAsync(app, RegisterPath, RegisterBody); + + response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + var problem = await ProblemAsync(response); + problem.GetProperty("code").GetString().Should().Be("invite_token_invalid"); + problem.GetProperty("detail").GetString().Should().Be("The invite token is invalid, expired, or has already been used."); + problem.TryGetProperty("errors", out _).Should().BeFalse(); + } + + [Theory] + [InlineData(AuthError.InvalidCredentials, 400, "invalid_credentials", "currentPassword", + "The username, email, or password is incorrect.")] + [InlineData(AuthError.PasswordTooShort, 422, "password_too_short", "newPassword", "Password must be at least 12 characters.")] + [InlineData(AuthError.PasswordTooLong, 422, "password_too_long", "newPassword", "Password must be no longer than 128 characters.")] + [InlineData(AuthError.PasswordPwned, 422, "password_pwned", "newPassword", + "This password has appeared in a known data breach. Please choose a different one.")] + public async Task ChangePassword_FieldRejected_IsAValidationProblemOnThatFieldAsync( + AuthError error, int status, string code, string field, string detail) + { + auth.ChangePasswordAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(AuthResult.Fail(error)); + await using var app = await StartAsync(); + + using var response = await ChangePasswordAsync(app); + + await AssertFieldProblemAsync(response, status, code, field, detail); + } +} diff --git a/src/Anichron.API.Tests.Unit/Endpoints/AuthResponseMapperTests.cs b/src/Anichron.API.Tests.Unit/Endpoints/AuthResponseMapperTests.cs index 9a196903..080e3e76 100644 --- a/src/Anichron.API.Tests.Unit/Endpoints/AuthResponseMapperTests.cs +++ b/src/Anichron.API.Tests.Unit/Endpoints/AuthResponseMapperTests.cs @@ -71,28 +71,6 @@ public async Task GetRegistrationResult_Success_Returns200SetsRefreshCookieAndOm }); } - [Theory] - [InlineData(AuthError.UsernameTaken, 409)] - [InlineData(AuthError.EmailTaken, 409)] - [InlineData(AuthError.InvalidUsername, 422)] - [InlineData(AuthError.InvalidEmail, 422)] - [InlineData(AuthError.PasswordTooShort, 422)] - [InlineData(AuthError.PasswordTooLong, 422)] - [InlineData(AuthError.PasswordPwned, 422)] - [InlineData(AuthError.InviteTokenInvalid, 422)] - public async Task GetRegistrationResult_Failure_ReturnsExpectedStatusCode(AuthError error, int expectedStatus) - { - var http = NewHttp(); - var testee = new TestFixture().CreateTestee(); - - var result = testee.GetRegistrationResult( - AuthResult.Fail(error), - http, new PasswordPolicy(), new UsernamePolicy()); - await result.ExecuteAsync(http); - - http.Response.StatusCode.Should().Be(expectedStatus); - } - // ========================================================================== // GetLoginResult // ========================================================================== @@ -138,36 +116,6 @@ public async Task GetLoginResult_SuccessWithoutCookie_Returns200AndIncludesToken }); } - [Theory] - [InlineData(AuthError.InvalidCredentials)] - [InlineData(AuthError.AccountDisabled)] - public async Task GetLoginResult_AuthFailure_Returns401(AuthError error) - { - var http = NewHttp(); - var testee = new TestFixture().CreateTestee(); - - var result = testee.GetLoginResult(AuthResult.Fail(error), http, setCookie: false); - await result.ExecuteAsync(http); - - http.Response.StatusCode.Should().Be(401); - } - - [Fact] - public async Task GetLoginResult_AccountTemporarilyLocked_Returns429WithRetryAfterHeader() - { - var http = NewHttp(); - var testee = new TestFixture().CreateTestee(); - - var result = testee.GetLoginResult(AuthResult.Locked(60), http, setCookie: false); - await result.ExecuteAsync(http); - - Assert.Multiple(() => - { - http.Response.StatusCode.Should().Be(429); - http.Response.Headers.RetryAfter.ToString().Should().Be("60"); - }); - } - // ========================================================================== // GetRefreshResult // ========================================================================== @@ -207,54 +155,6 @@ public async Task GetRefreshResult_SuccessWithoutCookie_Returns200AndIncludesTok }); } - [Theory] - [InlineData(AuthError.TokenInvalid)] - [InlineData(AuthError.AccountDisabled)] - public async Task GetRefreshResult_AuthFailure_Returns401(AuthError error) - { - var http = NewHttp(); - var testee = new TestFixture().CreateTestee(); - - var result = testee.GetRefreshResult(AuthResult.Fail(error), http, setCookie: false); - await result.ExecuteAsync(http); - - http.Response.StatusCode.Should().Be(401); - } - - [Fact] - public async Task GetRefreshResult_AccountTemporarilyLocked_Returns429WithRetryAfterHeader() - { - var http = NewHttp(); - var testee = new TestFixture().CreateTestee(); - - var result = testee.GetRefreshResult(AuthResult.Locked(30), http, setCookie: false); - await result.ExecuteAsync(http); - - Assert.Multiple(() => - { - http.Response.StatusCode.Should().Be(429); - http.Response.Headers.RetryAfter.ToString().Should().Be("30"); - }); - } - - [Fact] - public async Task GetLoginResult_AccountTemporarilyLockedForOneSecond_ReturnsSingularSecondMessage() - { - var http = NewHttp(); - var testee = new TestFixture().CreateTestee(); - - var result = testee.GetLoginResult(AuthResult.Locked(1), http, setCookie: false); - await result.ExecuteAsync(http); - var body = await ReadBodyAsync(http.Response); - - Assert.Multiple(() => - { - http.Response.StatusCode.Should().Be(429); - body.Should().Contain("1 second"); - body.Should().NotContain("seconds"); - }); - } - [Fact] public void GetRegistrationResult_ErrorFromOtherContext_ThrowsUnreachableException() { @@ -358,23 +258,6 @@ public async Task GetChangePasswordResult_Success_Returns204NoContent() http.Response.StatusCode.Should().Be(204); } - [Fact] - public async Task GetChangePasswordResult_InvalidCredentials_Returns400WithMessage() - { - var http = NewHttp(); - var testee = new TestFixture().CreateTestee(); - - var result = testee.GetChangePasswordResult(AuthResult.Fail(AuthError.InvalidCredentials), new PasswordPolicy()); - await result.ExecuteAsync(http); - var body = await ReadBodyAsync(http.Response); - - Assert.Multiple(() => - { - http.Response.StatusCode.Should().Be(400); - body.Should().Contain("The username, email, or password is incorrect."); - }); - } - [Fact] public async Task GetChangePasswordResult_PasswordTooShort_Returns422WithPolicyTooShortMessage() { @@ -411,23 +294,6 @@ public async Task GetChangePasswordResult_PasswordTooLong_Returns422WithPolicyTo }); } - [Fact] - public async Task GetChangePasswordResult_PasswordPwned_Returns422WithPwnedMessage() - { - var http = NewHttp(); - var testee = new TestFixture().CreateTestee(); - - var result = testee.GetChangePasswordResult(AuthResult.Fail(AuthError.PasswordPwned), new PasswordPolicy()); - await result.ExecuteAsync(http); - var body = await ReadBodyAsync(http.Response); - - Assert.Multiple(() => - { - http.Response.StatusCode.Should().Be(422); - body.Should().Contain("This password has appeared in a known data breach. Please choose a different one."); - }); - } - [Fact] public void GetChangePasswordResult_ErrorFromOtherContext_ThrowsUnreachableException() { diff --git a/src/Anichron.API.Tests.Unit/Infrastructure/AuthenticatedTestApp.cs b/src/Anichron.API.Tests.Unit/Infrastructure/AuthenticatedTestApp.cs index 80cbbb30..3112baf0 100644 --- a/src/Anichron.API.Tests.Unit/Infrastructure/AuthenticatedTestApp.cs +++ b/src/Anichron.API.Tests.Unit/Infrastructure/AuthenticatedTestApp.cs @@ -30,14 +30,18 @@ internal sealed class AuthenticatedTestApp : IAsyncDisposable public HttpClient Client => app.GetTestClient(); public static async Task StartAsync( - Action mapEndpoints, Action configureServices) + Action mapEndpoints, Action configureServices, bool rateLimited = false) { var builder = ApiTestHost.CreateBuilder(); builder.WebHost.UseTestServer(); builder.Services.AddAuthentication("Test").AddScheme("Test", null); builder.Services.AddAuthorizationPolicies(); + if (rateLimited) + builder.Services.AddRateLimiting(); configureServices(builder.Services); var app = builder.Build(); + if (rateLimited) + app.UseRateLimiter(); app.UseAuthentication(); app.UseAuthorization(); mapEndpoints(app.MapGroup(ApiPaths.Base)); diff --git a/src/Anichron.API.Tests.Unit/Infrastructure/RateLimitingTests.cs b/src/Anichron.API.Tests.Unit/Infrastructure/RateLimitingTests.cs index 9475b2fb..30dda36e 100644 --- a/src/Anichron.API.Tests.Unit/Infrastructure/RateLimitingTests.cs +++ b/src/Anichron.API.Tests.Unit/Infrastructure/RateLimitingTests.cs @@ -1,35 +1,11 @@ using Anichron.API.Infrastructure; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.RateLimiting; using System.Net; -using System.Text.Json; -using System.Threading.RateLimiting; namespace Anichron.API.Tests.Unit.Infrastructure; public sealed class RateLimitingTests { - private sealed class TestLease(TimeSpan? retryAfter) : RateLimitLease - { - public override bool IsAcquired => false; - - public override IEnumerable MetadataNames - => retryAfter is null ? [] : [MetadataName.RetryAfter.Name]; - - public override bool TryGetMetadata(string metadataName, out object? metadata) - { - metadata = retryAfter; - return retryAfter is not null && metadataName == MetadataName.RetryAfter.Name; - } - } - - private static OnRejectedContext Rejected(TimeSpan? retryAfter) - { - var httpContext = new DefaultHttpContext(); - httpContext.Response.Body = new MemoryStream(); - return new OnRejectedContext { HttpContext = httpContext, Lease = new TestLease(retryAfter) }; - } - [Fact] public void PartitionKey_RemoteAddressKnown_IsTheAddress() { @@ -44,39 +20,4 @@ public void PartitionKey_RemoteAddressUnknown_SharesTheUnresolvedBucket() { RateLimiting.PartitionKey(new DefaultHttpContext()).Should().Be("unresolved"); } - - [Fact] - public async Task OnRejectedAsync_Always_Writes429WithErrorBodyAsync() - { - var context = Rejected(retryAfter: null); - - await RateLimiting.OnRejectedAsync(context, TestContext.Current.CancellationToken); - - var response = context.HttpContext.Response; - response.StatusCode.Should().Be(StatusCodes.Status429TooManyRequests); - response.Body.Position = 0; - using var body = await JsonDocument.ParseAsync(response.Body, cancellationToken: TestContext.Current.CancellationToken); - body.RootElement.GetProperty("error").GetString() - .Should().Be("Too many requests. Please try again later."); - } - - [Fact] - public async Task OnRejectedAsync_LeaseCarriesRetryAfter_SetsHeaderInWholeSecondsAsync() - { - var context = Rejected(TimeSpan.FromSeconds(12.7)); - - await RateLimiting.OnRejectedAsync(context, TestContext.Current.CancellationToken); - - context.HttpContext.Response.Headers.RetryAfter.ToString().Should().Be("12"); - } - - [Fact] - public async Task OnRejectedAsync_LeaseWithoutRetryAfter_OmitsHeaderAsync() - { - var context = Rejected(retryAfter: null); - - await RateLimiting.OnRejectedAsync(context, TestContext.Current.CancellationToken); - - context.HttpContext.Response.Headers.ContainsKey("Retry-After").Should().BeFalse(); - } } diff --git a/src/Anichron.API/Endpoints/AuthEndpoints.cs b/src/Anichron.API/Endpoints/AuthEndpoints.cs index 4f4a00f8..4c3db554 100644 --- a/src/Anichron.API/Endpoints/AuthEndpoints.cs +++ b/src/Anichron.API/Endpoints/AuthEndpoints.cs @@ -63,9 +63,8 @@ internal static async Task RefreshAsync( var rawToken = http.Request.Cookies[AuthMessages.RefreshTokenCookieName] ?? request?.RefreshToken; if (rawToken is null) { - return Results.Json( - data: new { error = AuthMessages.RefreshTokenRequired }, - statusCode: StatusCodes.Status401Unauthorized); + return AuthProblems.Problem( + StatusCodes.Status401Unauthorized, AuthErrorCodes.RefreshTokenRequired, AuthMessages.RefreshTokenRequired); } var result = await auth.RefreshAsync(rawToken, ct); diff --git a/src/Anichron.API/Endpoints/AuthProblems.cs b/src/Anichron.API/Endpoints/AuthProblems.cs new file mode 100644 index 00000000..f3e71725 --- /dev/null +++ b/src/Anichron.API/Endpoints/AuthProblems.cs @@ -0,0 +1,45 @@ +namespace Anichron.API.Endpoints; + +/// +/// Auth failures as Problem Details: a stable code extension a client branches on, and the +/// English text in detail for people reading the raw response. +/// +internal static class AuthProblems +{ + internal const string CodeExtension = "code"; + + internal static IResult Problem(int statusCode, string code, string detail) + => Results.Problem(detail: detail, statusCode: statusCode, extensions: Extensions(code)); + + /// A failure of one request field, with also listed under that field in errors. + /// The response status. + /// The failure's code. + /// The request field, as the client names it in JSON. + /// The English text. + internal static IResult FieldProblem(int statusCode, string code, string field, string detail) + => Results.ValidationProblem( + errors: new Dictionary { [field] = [code] }, + detail: detail, + statusCode: statusCode, + extensions: Extensions(code)); + + private static Dictionary Extensions(string code) => new() { [CodeExtension] = code }; +} + +internal static class AuthErrorCodes +{ + internal const string InvalidCredentials = "invalid_credentials"; + internal const string AccountDisabled = "account_disabled"; + internal const string AccountLocked = "account_locked"; + internal const string RateLimited = "rate_limited"; + internal const string RefreshTokenRequired = "refresh_token_required"; + internal const string RefreshTokenInvalid = "refresh_token_invalid"; + internal const string InvalidUsername = "invalid_username"; + internal const string InvalidEmail = "invalid_email"; + internal const string UsernameTaken = "username_taken"; + internal const string EmailTaken = "email_taken"; + internal const string PasswordTooShort = "password_too_short"; + internal const string PasswordTooLong = "password_too_long"; + internal const string PasswordPwned = "password_pwned"; + internal const string InviteTokenInvalid = "invite_token_invalid"; +} diff --git a/src/Anichron.API/Endpoints/AuthResponseMapper.cs b/src/Anichron.API/Endpoints/AuthResponseMapper.cs index f0d3ff95..3f191306 100644 --- a/src/Anichron.API/Endpoints/AuthResponseMapper.cs +++ b/src/Anichron.API/Endpoints/AuthResponseMapper.cs @@ -33,15 +33,18 @@ public IResult GetRegistrationResult(AuthResult result, HttpContext { return result.Error switch { - AuthError.UsernameTaken => Results.Conflict(new { error = AuthMessages.UsernameTaken }), - AuthError.EmailTaken => Results.Conflict(new { error = AuthMessages.EmailTaken }), - AuthError.InvalidUsername => Results.UnprocessableEntity(new { error = usernamePolicy.InvalidFormatMessage }), - AuthError.InvalidEmail => Results.UnprocessableEntity(new { error = AuthMessages.InvalidEmail }), - AuthError.PasswordTooShort => Results.UnprocessableEntity(new { error = passwordPolicy.TooShortMessage }), - AuthError.PasswordTooLong => Results.UnprocessableEntity(new { error = passwordPolicy.TooLongMessage }), - AuthError.PasswordPwned => Results.UnprocessableEntity(new { error = AuthMessages.PasswordPwned }), - AuthError.InviteTokenInvalid => Results.UnprocessableEntity(new { error = AuthMessages.InviteTokenInvalid }), - _ => throw new UnreachableException($"Unexpected AuthError in Register: {result.Error}"), + AuthError.UsernameTaken => AuthProblems.FieldProblem( + StatusCodes.Status409Conflict, AuthErrorCodes.UsernameTaken, Fields.Username, AuthMessages.UsernameTaken), + AuthError.EmailTaken => AuthProblems.FieldProblem( + StatusCodes.Status409Conflict, AuthErrorCodes.EmailTaken, Fields.Email, AuthMessages.EmailTaken), + AuthError.InvalidUsername => AuthProblems.FieldProblem( + StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.InvalidUsername, Fields.Username, usernamePolicy.InvalidFormatMessage), + AuthError.InvalidEmail => AuthProblems.FieldProblem( + StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.InvalidEmail, Fields.Email, AuthMessages.InvalidEmail), + AuthError.InviteTokenInvalid => AuthProblems.Problem( + StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.InviteTokenInvalid, AuthMessages.InviteTokenInvalid), + _ => PasswordRejectedResult(result.Error, Fields.Password, passwordPolicy) + ?? throw new UnreachableException($"Unexpected AuthError in Register: {result.Error}"), }; } @@ -56,12 +59,9 @@ public IResult GetLoginResult(AuthResult result, HttpContext http, b return result.Error switch { AuthError.AccountTemporarilyLocked => LockedResult(http, result.RetryAfterSeconds!.Value), - AuthError.AccountDisabled => Results.Json( - data: new { error = AuthMessages.AccountDisabled }, - statusCode: StatusCodes.Status401Unauthorized), - AuthError.InvalidCredentials => Results.Json( - data: new { error = AuthMessages.InvalidCredentials }, - statusCode: StatusCodes.Status401Unauthorized), + AuthError.AccountDisabled => AccountDisabledResult(), + AuthError.InvalidCredentials => AuthProblems.Problem( + StatusCodes.Status401Unauthorized, AuthErrorCodes.InvalidCredentials, AuthMessages.InvalidCredentials), _ => throw new UnreachableException($"Unexpected AuthError in Login: {result.Error}"), }; } @@ -75,12 +75,9 @@ public IResult GetRefreshResult(AuthResult result, HttpContext http, { return result.Error switch { - AuthError.TokenInvalid => Results.Json( - data: new { error = AuthMessages.RefreshTokenInvalid }, - statusCode: StatusCodes.Status401Unauthorized), - AuthError.AccountDisabled => Results.Json( - data: new { error = AuthMessages.AccountDisabled }, - statusCode: StatusCodes.Status401Unauthorized), + AuthError.TokenInvalid => AuthProblems.Problem( + StatusCodes.Status401Unauthorized, AuthErrorCodes.RefreshTokenInvalid, AuthMessages.RefreshTokenInvalid), + AuthError.AccountDisabled => AccountDisabledResult(), AuthError.AccountTemporarilyLocked => LockedResult(http, result.RetryAfterSeconds!.Value), _ => throw new UnreachableException($"Unexpected AuthError in Refresh: {result.Error}"), }; @@ -96,19 +93,10 @@ public IResult GetChangePasswordResult(AuthResult result, PasswordPolicy passwor return result.Error switch { - AuthError.InvalidCredentials => Results.Json( - data: new { error = AuthMessages.InvalidCredentials }, - statusCode: StatusCodes.Status400BadRequest), - AuthError.PasswordTooShort => Results.Json( - data: new { error = passwordPolicy.TooShortMessage }, - statusCode: StatusCodes.Status422UnprocessableEntity), - AuthError.PasswordTooLong => Results.Json( - data: new { error = passwordPolicy.TooLongMessage }, - statusCode: StatusCodes.Status422UnprocessableEntity), - AuthError.PasswordPwned => Results.Json( - data: new { error = AuthMessages.PasswordPwned }, - statusCode: StatusCodes.Status422UnprocessableEntity), - _ => throw new UnreachableException($"Unexpected AuthError in ChangePassword: {result.Error}"), + AuthError.InvalidCredentials => AuthProblems.FieldProblem( + StatusCodes.Status400BadRequest, AuthErrorCodes.InvalidCredentials, Fields.CurrentPassword, AuthMessages.InvalidCredentials), + _ => PasswordRejectedResult(result.Error, Fields.NewPassword, passwordPolicy) + ?? throw new UnreachableException($"Unexpected AuthError in ChangePassword: {result.Error}"), }; } @@ -199,6 +187,15 @@ public IResult GetAdminDeleteStorageConfigResult(AuthResult result) public void ClearRefreshCookie(HttpContext http) => http.Response.Cookies.Delete(AuthMessages.RefreshTokenCookieName); + private static class Fields + { + internal const string Username = "username"; + internal const string Email = "email"; + internal const string Password = "password"; + internal const string CurrentPassword = "currentPassword"; + internal const string NewPassword = "newPassword"; + } + private static AdminUserResponse ToAdminUserResponse(User user) => new(user.Id, user.Username, user.Email, user.IsAdmin, user.IsDisabled, user.StorageConfigs.Count); @@ -217,11 +214,25 @@ private IResult BuildTokenResponse(AuthTokens tokens, HttpContext http, bool set private static IResult LockedResult(HttpContext http, int secondsRemaining) { http.Response.Headers.RetryAfter = secondsRemaining.ToString(InvariantCulture); - return Results.Json( - new { error = AuthMessages.AccountTemporarilyLocked(secondsRemaining) }, - statusCode: StatusCodes.Status429TooManyRequests); + return AuthProblems.Problem( + StatusCodes.Status429TooManyRequests, AuthErrorCodes.AccountLocked, AuthMessages.AccountTemporarilyLocked(secondsRemaining)); } + private static IResult? PasswordRejectedResult(AuthError? error, string field, PasswordPolicy passwordPolicy) + => error switch + { + AuthError.PasswordTooShort => AuthProblems.FieldProblem( + StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.PasswordTooShort, field, passwordPolicy.TooShortMessage), + AuthError.PasswordTooLong => AuthProblems.FieldProblem( + StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.PasswordTooLong, field, passwordPolicy.TooLongMessage), + AuthError.PasswordPwned => AuthProblems.FieldProblem( + StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.PasswordPwned, field, AuthMessages.PasswordPwned), + _ => null, + }; + + private static IResult AccountDisabledResult() + => AuthProblems.Problem(StatusCodes.Status401Unauthorized, AuthErrorCodes.AccountDisabled, AuthMessages.AccountDisabled); + private void SetRefreshCookie(HttpContext http, string rawToken) { http.Response.Cookies.Append(AuthMessages.RefreshTokenCookieName, rawToken, new CookieOptions diff --git a/src/Anichron.API/Infrastructure/RateLimiting.cs b/src/Anichron.API/Infrastructure/RateLimiting.cs index de8c548b..1b5d4878 100644 --- a/src/Anichron.API/Infrastructure/RateLimiting.cs +++ b/src/Anichron.API/Infrastructure/RateLimiting.cs @@ -1,3 +1,4 @@ +using Anichron.API.Endpoints; using Microsoft.AspNetCore.RateLimiting; using System.Threading.RateLimiting; using static System.Globalization.CultureInfo; @@ -11,16 +12,36 @@ internal static class RateLimiting internal static string PartitionKey(HttpContext httpContext) => httpContext.Connection.RemoteIpAddress?.ToString() ?? "unresolved"; - internal static async ValueTask OnRejectedAsync(OnRejectedContext context, CancellationToken token) + internal static Task RejectAsync(HttpContext httpContext, TimeSpan retryAfter) { - context.HttpContext.Response.StatusCode = StatusCodes.Status429TooManyRequests; - if (context.Lease.TryGetMetadata(MetadataName.RetryAfter, out var retryAfter)) - { - context.HttpContext.Response.Headers.RetryAfter = - ((int)retryAfter.TotalSeconds).ToString(InvariantCulture); - } - - await context.HttpContext.Response.WriteAsJsonAsync( - new { error = AuthMessages.TooManyRequests }, token); + httpContext.Response.Headers.RetryAfter = ((int)retryAfter.TotalSeconds).ToString(InvariantCulture); + return AuthProblems.Problem( + StatusCodes.Status429TooManyRequests, AuthErrorCodes.RateLimited, AuthMessages.TooManyRequests) + .ExecuteAsync(httpContext); } } + +/// +/// A per-client sliding window. A rejection asks the client to wait one whole window, because a +/// sliding window's lease reports no retry time and every permit is back once a window has passed. +/// +/// The requests one client may make per window. +/// The window. +/// The segments the window slides by. +internal sealed class SlidingWindowPolicy(int permitLimit, TimeSpan window, int segmentsPerWindow) : IRateLimiterPolicy +{ + public Func OnRejected + => (context, _) => new ValueTask(RateLimiting.RejectAsync(context.HttpContext, window)); + + public RateLimitPartition GetPartition(HttpContext httpContext) + => RateLimitPartition.GetSlidingWindowLimiter( + partitionKey: RateLimiting.PartitionKey(httpContext), + factory: _ => new SlidingWindowRateLimiterOptions + { + PermitLimit = permitLimit, + Window = window, + SegmentsPerWindow = segmentsPerWindow, + QueueProcessingOrder = QueueProcessingOrder.OldestFirst, + QueueLimit = 0, + }); +} diff --git a/src/Anichron.API/Infrastructure/ServiceCollectionExtensions.cs b/src/Anichron.API/Infrastructure/ServiceCollectionExtensions.cs index dd31d4ad..3311c337 100644 --- a/src/Anichron.API/Infrastructure/ServiceCollectionExtensions.cs +++ b/src/Anichron.API/Infrastructure/ServiceCollectionExtensions.cs @@ -14,7 +14,6 @@ using System.IO.Abstractions; using System.Net; using System.Text; -using System.Threading.RateLimiting; namespace Anichron.API.Infrastructure; @@ -49,32 +48,16 @@ public IServiceCollection AddRateLimiting() { return services.AddRateLimiter(options => { - options.OnRejected = RateLimiting.OnRejectedAsync; - - options.AddPolicy(AuthRateLimitPolicies.Sensitive, httpContext => - RateLimitPartition.GetSlidingWindowLimiter( - partitionKey: RateLimiting.PartitionKey(httpContext), - factory: _ => new SlidingWindowRateLimiterOptions - { - PermitLimit = AppDefaults.RateLimit.Sensitive.PermitLimit, - Window = TimeSpan.FromSeconds(AppDefaults.RateLimit.Sensitive.WindowSeconds), - SegmentsPerWindow = AppDefaults.RateLimit.Sensitive.Segments, - QueueProcessingOrder = QueueProcessingOrder.OldestFirst, - QueueLimit = 0, - })); + options.AddPolicy(AuthRateLimitPolicies.Sensitive, new SlidingWindowPolicy( + AppDefaults.RateLimit.Sensitive.PermitLimit, + TimeSpan.FromSeconds(AppDefaults.RateLimit.Sensitive.WindowSeconds), + AppDefaults.RateLimit.Sensitive.Segments)); // Refresh tokens rotate on every use. Tighter policy required. - options.AddPolicy(AuthRateLimitPolicies.Refresh, httpContext => - RateLimitPartition.GetSlidingWindowLimiter( - partitionKey: RateLimiting.PartitionKey(httpContext), - factory: _ => new SlidingWindowRateLimiterOptions - { - PermitLimit = AppDefaults.RateLimit.Refresh.PermitLimit, - Window = TimeSpan.FromMinutes(AppDefaults.RateLimit.Refresh.WindowMinutes), - SegmentsPerWindow = AppDefaults.RateLimit.Refresh.Segments, - QueueProcessingOrder = QueueProcessingOrder.OldestFirst, - QueueLimit = 0, - })); + options.AddPolicy(AuthRateLimitPolicies.Refresh, new SlidingWindowPolicy( + AppDefaults.RateLimit.Refresh.PermitLimit, + TimeSpan.FromMinutes(AppDefaults.RateLimit.Refresh.WindowMinutes), + AppDefaults.RateLimit.Refresh.Segments)); }); } From 169627f610e4ced855d4f17c50818126d3afbd01 Mon Sep 17 00:00:00 2001 From: bitbiter-dev Date: Thu, 29 Oct 2026 22:26:40 +1300 Subject: [PATCH 2/2] refactor(api): Review follow-ups for auth error codes (#357) The password failures' mapping throws for an error it does not own instead of returning null for the caller to throw. The contract tests assert every problem through one helper, so each checks its status in the body too, and wrong credentials and lockout run on both login endpoints. The change-password endpoint test stubs a problem rather than the removed { error } shape. Doc comments say what, not why. Co-Authored-By: Claude Opus 5.5 --- .../Endpoints/AuthErrorContractTests.cs | 77 +++++++------------ .../Endpoints/AuthResponseMapperTests.cs | 4 +- .../Endpoints/UserEndpointsTests.cs | 2 +- src/Anichron.API/Endpoints/AuthProblems.cs | 5 +- .../Endpoints/AuthResponseMapper.cs | 10 +-- .../Infrastructure/RateLimiting.cs | 5 +- 6 files changed, 38 insertions(+), 65 deletions(-) diff --git a/src/Anichron.API.Tests.Unit/Endpoints/AuthErrorContractTests.cs b/src/Anichron.API.Tests.Unit/Endpoints/AuthErrorContractTests.cs index e31dfd97..9856d3fa 100644 --- a/src/Anichron.API.Tests.Unit/Endpoints/AuthErrorContractTests.cs +++ b/src/Anichron.API.Tests.Unit/Endpoints/AuthErrorContractTests.cs @@ -4,7 +4,6 @@ using Anichron.API.Tests.Unit.Infrastructure; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; -using System.Net; using System.Net.Http.Json; using System.Text.Json; @@ -58,14 +57,21 @@ private static async Task ChangePasswordAsync(Authenticated private static Task PostAsync(AuthenticatedTestApp app, string path, object body) => app.Client.PostAsJsonAsync(path, body, TestContext.Current.CancellationToken); - private static async Task AssertFieldProblemAsync( - HttpResponseMessage response, int status, string code, string field, string detail) + private static async Task AssertProblemAsync( + HttpResponseMessage response, int status, string code, string detail) { ((int)response.StatusCode).Should().Be(status); var problem = await ProblemAsync(response); problem.GetProperty("code").GetString().Should().Be(code); problem.GetProperty("status").GetInt32().Should().Be(status); problem.GetProperty("detail").GetString().Should().Be(detail); + return problem; + } + + private static async Task AssertFieldProblemAsync( + HttpResponseMessage response, int status, string code, string field, string detail) + { + var problem = await AssertProblemAsync(response, status, code, detail); var errors = problem.GetProperty("errors").EnumerateObject().ToList(); errors.Should().ContainSingle().Which.Name.Should().Be(field); errors[0].Value.EnumerateArray().Select(e => e.GetString()).Should().Equal(code); @@ -78,19 +84,17 @@ private static async Task ProblemAsync(HttpResponseMessage response return body.RootElement.Clone(); } - [Fact] - public async Task Login_WrongCredentials_Is401InvalidCredentialsAsync() + [Theory] + [InlineData(LoginPath)] + [InlineData(LoginMobilePath)] + public async Task Login_WrongCredentials_Is401InvalidCredentialsAsync(string path) { LoginFails(AuthResult.Fail(AuthError.InvalidCredentials)); await using var app = await StartAsync(); - using var response = await PostAsync(app, LoginPath, new { usernameOrEmail = "alice", password = "wrong" }); + using var response = await PostAsync(app, path, new { usernameOrEmail = "alice", password = "wrong" }); - response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be("invalid_credentials"); - problem.GetProperty("status").GetInt32().Should().Be(401); - problem.GetProperty("detail").GetString().Should().Be("The username, email, or password is incorrect."); + await AssertProblemAsync(response, 401, "invalid_credentials", "The username, email, or password is incorrect."); } [Theory] @@ -103,26 +107,21 @@ public async Task Login_DisabledAccount_Is401AccountDisabledAsync(string path) using var response = await PostAsync(app, path, new { usernameOrEmail = "alice", password = "SecurePass1!" }); - response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be("account_disabled"); - problem.GetProperty("detail").GetString().Should().Be("This account has been disabled. Please contact an administrator."); + await AssertProblemAsync(response, 401, "account_disabled", "This account has been disabled. Please contact an administrator."); } - [Fact] - public async Task Login_LockedAccount_Is429AccountLockedWithRetryAfterAsync() + [Theory] + [InlineData(LoginPath)] + [InlineData(LoginMobilePath)] + public async Task Login_LockedAccount_Is429AccountLockedWithRetryAfterAsync(string path) { LoginFails(AuthResult.Locked(60)); await using var app = await StartAsync(); - using var response = await PostAsync(app, LoginPath, new { usernameOrEmail = "alice", password = "wrong" }); + using var response = await PostAsync(app, path, new { usernameOrEmail = "alice", password = "wrong" }); - response.StatusCode.Should().Be(HttpStatusCode.TooManyRequests); response.Headers.RetryAfter!.Delta.Should().Be(TimeSpan.FromSeconds(60)); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be("account_locked"); - problem.GetProperty("status").GetInt32().Should().Be(429); - problem.GetProperty("detail").GetString().Should().Be("Too many failed login attempts. Please try again in 60 seconds."); + await AssertProblemAsync(response, 429, "account_locked", "Too many failed login attempts. Please try again in 60 seconds."); } [Fact] @@ -132,10 +131,7 @@ public async Task Refresh_NoToken_Is401RefreshTokenRequiredAsync() using var response = await app.Client.PostAsync(RefreshPath, content: null, TestContext.Current.CancellationToken); - response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be("refresh_token_required"); - problem.GetProperty("detail").GetString().Should().Be("Refresh token is required."); + await AssertProblemAsync(response, 401, "refresh_token_required", "Refresh token is required."); } [Theory] @@ -148,11 +144,7 @@ public async Task Refresh_Rejected_Is401WithItsCodeAsync(AuthError error, string using var response = await PostAsync(app, RefreshPath, new { refreshToken = "tok" }); - response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be(code); - problem.GetProperty("status").GetInt32().Should().Be(401); - problem.GetProperty("detail").GetString().Should().Be(detail); + await AssertProblemAsync(response, 401, code, detail); } [Fact] @@ -163,11 +155,8 @@ public async Task Refresh_LockedAccount_Is429AccountLockedWithRetryAfterAsync() using var response = await PostAsync(app, RefreshPath, new { refreshToken = "tok" }); - response.StatusCode.Should().Be(HttpStatusCode.TooManyRequests); response.Headers.RetryAfter!.Delta.Should().Be(TimeSpan.FromSeconds(1)); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be("account_locked"); - problem.GetProperty("detail").GetString().Should().Be("Too many failed login attempts. Please try again in 1 second."); + await AssertProblemAsync(response, 429, "account_locked", "Too many failed login attempts. Please try again in 1 second."); } [Fact] @@ -180,12 +169,8 @@ public async Task Login_OverTheRateLimit_Is429RateLimitedWithRetryAfterAsync() using var response = await PostAsync(app, LoginPath, new { usernameOrEmail = "alice", password = "wrong" }); - response.StatusCode.Should().Be(HttpStatusCode.TooManyRequests); response.Headers.RetryAfter!.Delta.Should().Be(TimeSpan.FromSeconds(60)); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be("rate_limited"); - problem.GetProperty("status").GetInt32().Should().Be(429); - problem.GetProperty("detail").GetString().Should().Be("Too many requests. Please try again later."); + await AssertProblemAsync(response, 429, "rate_limited", "Too many requests. Please try again later."); } [Fact] @@ -198,10 +183,8 @@ public async Task Refresh_OverTheRateLimit_Is429RateLimitedWithRetryAfterAsync() using var response = await PostAsync(app, RefreshPath, new { refreshToken = "tok" }); - response.StatusCode.Should().Be(HttpStatusCode.TooManyRequests); response.Headers.RetryAfter!.Delta.Should().Be(TimeSpan.FromMinutes(15)); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be("rate_limited"); + await AssertProblemAsync(response, 429, "rate_limited", "Too many requests. Please try again later."); } [Theory] @@ -235,10 +218,8 @@ public async Task Register_InvalidInvite_Is422InviteTokenInvalidAsync() using var response = await PostAsync(app, RegisterPath, RegisterBody); - response.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); - var problem = await ProblemAsync(response); - problem.GetProperty("code").GetString().Should().Be("invite_token_invalid"); - problem.GetProperty("detail").GetString().Should().Be("The invite token is invalid, expired, or has already been used."); + var problem = await AssertProblemAsync( + response, 422, "invite_token_invalid", "The invite token is invalid, expired, or has already been used."); problem.TryGetProperty("errors", out _).Should().BeFalse(); } diff --git a/src/Anichron.API.Tests.Unit/Endpoints/AuthResponseMapperTests.cs b/src/Anichron.API.Tests.Unit/Endpoints/AuthResponseMapperTests.cs index 080e3e76..99b229ed 100644 --- a/src/Anichron.API.Tests.Unit/Endpoints/AuthResponseMapperTests.cs +++ b/src/Anichron.API.Tests.Unit/Endpoints/AuthResponseMapperTests.cs @@ -176,7 +176,7 @@ public void GetRegistrationResult_UndefinedAuthError_ThrowsUnreachableException( AuthResult.Fail((AuthError)999), NewHttp(), new PasswordPolicy(), new UsernamePolicy()); - act.Should().Throw(); + act.Should().Throw().WithMessage("Unexpected AuthError in Register: 999"); } [Fact] @@ -311,7 +311,7 @@ public void GetChangePasswordResult_UndefinedAuthError_ThrowsUnreachableExceptio var act = () => testee.GetChangePasswordResult(AuthResult.Fail((AuthError)999), new PasswordPolicy()); - act.Should().Throw(); + act.Should().Throw().WithMessage("Unexpected AuthError in ChangePassword: 999"); } // ========================================================================== diff --git a/src/Anichron.API.Tests.Unit/Endpoints/UserEndpointsTests.cs b/src/Anichron.API.Tests.Unit/Endpoints/UserEndpointsTests.cs index 4e1deec4..8fcbd48d 100644 --- a/src/Anichron.API.Tests.Unit/Endpoints/UserEndpointsTests.cs +++ b/src/Anichron.API.Tests.Unit/Endpoints/UserEndpointsTests.cs @@ -187,7 +187,7 @@ public async Task ChangePasswordAsync_ValidClaim_AuthServiceReturnsError_Returns var policy = new PasswordPolicy(); var options = Options.Create(policy); var failureResult = AuthResult.Fail(AuthError.InvalidCredentials); - var expectedResult = Results.Json(new { error = "The username, email, or password is incorrect." }, statusCode: 400); + var expectedResult = Results.Problem(statusCode: 400); authService.ChangePasswordAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(failureResult); mapper.GetChangePasswordResult(failureResult, policy) diff --git a/src/Anichron.API/Endpoints/AuthProblems.cs b/src/Anichron.API/Endpoints/AuthProblems.cs index f3e71725..3f591a60 100644 --- a/src/Anichron.API/Endpoints/AuthProblems.cs +++ b/src/Anichron.API/Endpoints/AuthProblems.cs @@ -1,9 +1,6 @@ namespace Anichron.API.Endpoints; -/// -/// Auth failures as Problem Details: a stable code extension a client branches on, and the -/// English text in detail for people reading the raw response. -/// +/// Auth failures as Problem Details: a stable code extension, and the English text in detail. internal static class AuthProblems { internal const string CodeExtension = "code"; diff --git a/src/Anichron.API/Endpoints/AuthResponseMapper.cs b/src/Anichron.API/Endpoints/AuthResponseMapper.cs index 3f191306..2dcfc96d 100644 --- a/src/Anichron.API/Endpoints/AuthResponseMapper.cs +++ b/src/Anichron.API/Endpoints/AuthResponseMapper.cs @@ -43,8 +43,7 @@ public IResult GetRegistrationResult(AuthResult result, HttpContext StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.InvalidEmail, Fields.Email, AuthMessages.InvalidEmail), AuthError.InviteTokenInvalid => AuthProblems.Problem( StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.InviteTokenInvalid, AuthMessages.InviteTokenInvalid), - _ => PasswordRejectedResult(result.Error, Fields.Password, passwordPolicy) - ?? throw new UnreachableException($"Unexpected AuthError in Register: {result.Error}"), + _ => PasswordRejectedResult(result.Error, Fields.Password, passwordPolicy, "Register"), }; } @@ -95,8 +94,7 @@ public IResult GetChangePasswordResult(AuthResult result, PasswordPolicy passwor { AuthError.InvalidCredentials => AuthProblems.FieldProblem( StatusCodes.Status400BadRequest, AuthErrorCodes.InvalidCredentials, Fields.CurrentPassword, AuthMessages.InvalidCredentials), - _ => PasswordRejectedResult(result.Error, Fields.NewPassword, passwordPolicy) - ?? throw new UnreachableException($"Unexpected AuthError in ChangePassword: {result.Error}"), + _ => PasswordRejectedResult(result.Error, Fields.NewPassword, passwordPolicy, "ChangePassword"), }; } @@ -218,7 +216,7 @@ private static IResult LockedResult(HttpContext http, int secondsRemaining) StatusCodes.Status429TooManyRequests, AuthErrorCodes.AccountLocked, AuthMessages.AccountTemporarilyLocked(secondsRemaining)); } - private static IResult? PasswordRejectedResult(AuthError? error, string field, PasswordPolicy passwordPolicy) + private static IResult PasswordRejectedResult(AuthError? error, string field, PasswordPolicy passwordPolicy, string operation) => error switch { AuthError.PasswordTooShort => AuthProblems.FieldProblem( @@ -227,7 +225,7 @@ private static IResult LockedResult(HttpContext http, int secondsRemaining) StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.PasswordTooLong, field, passwordPolicy.TooLongMessage), AuthError.PasswordPwned => AuthProblems.FieldProblem( StatusCodes.Status422UnprocessableEntity, AuthErrorCodes.PasswordPwned, field, AuthMessages.PasswordPwned), - _ => null, + _ => throw new UnreachableException($"Unexpected AuthError in {operation}: {error}"), }; private static IResult AccountDisabledResult() diff --git a/src/Anichron.API/Infrastructure/RateLimiting.cs b/src/Anichron.API/Infrastructure/RateLimiting.cs index 1b5d4878..2deabea7 100644 --- a/src/Anichron.API/Infrastructure/RateLimiting.cs +++ b/src/Anichron.API/Infrastructure/RateLimiting.cs @@ -21,10 +21,7 @@ internal static Task RejectAsync(HttpContext httpContext, TimeSpan retryAfter) } } -/// -/// A per-client sliding window. A rejection asks the client to wait one whole window, because a -/// sliding window's lease reports no retry time and every permit is back once a window has passed. -/// +/// A per-client sliding window whose rejection asks the client to wait one whole window. /// The requests one client may make per window. /// The window. /// The segments the window slides by.