From 77a10e38caa6b292fe5e0e9fccb3816eb4012745 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 13:00:03 -0300 Subject: [PATCH 1/8] fix(auth): cap reset abuse per account, key code hashes, send reset email off the request path - Forgot Password is limited to 3 codes an hour and 10 a day per email, and an account gets 10 failed code checks a day across every code it is sent, so new client addresses and new codes no longer buy more guesses. Refused requests answer exactly like accepted ones. - The reset email is queued to a background sender, and unregistered addresses store a row no code can match, so both paths do the same work and return without waiting on the mail provider. Each request also clears expired codes. - Code hashes are HMAC-SHA256 under a key derived with HKDF from the JWT signing secret; rows in the previous unkeyed format stop matching. - Email and code are read only from the JSON body. --- .../AuthenticationControllerTests.cs | 16 +- .../AuthenticationServiceTests.cs | 72 +++--- .../Controllers/AuthenticationController.cs | 11 +- .../PasswordResetEmailDelivery.cs | 99 +++++++ Api.SeaHavenIndustries/Program.cs | 1 + .../ForgetPasswordDataService.cs | 21 +- .../Interfaces/IForgetPasswordDataService.cs | 9 +- .../DependencyInjection/ServicesModule.cs | 3 + .../Helpers/InMemoryPasswordResetThrottle.cs | 119 +++++++++ .../Helpers/PasswordResetCodeSecrets.cs | 35 ++- .../Implementation/AuthenticationService.cs | 70 +++-- .../Interfaces/IPasswordResetEmailQueue.cs | 13 + .../Interfaces/IPasswordResetThrottle.cs | 24 ++ .../PasswordResetAbuseLimitsTests.cs | 241 ++++++++++++++++++ .../PasswordResetFlowTests.cs | 30 ++- .../PasswordResetTestHost.cs | 33 ++- 16 files changed, 702 insertions(+), 95 deletions(-) create mode 100644 Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs create mode 100644 SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs create mode 100644 SeaHaven.Services/Interfaces/IPasswordResetEmailQueue.cs create mode 100644 SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs create mode 100644 SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationControllerTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationControllerTests.cs index 5ec4371..14a61da 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationControllerTests.cs @@ -173,8 +173,8 @@ public class AuthenticationControllerTests var service = new Mock(); var controller = NewController(service); - var registered = await controller.ForgetPassword(new ForgetPasswordRequest_Dto { Email = "a@b.com" }, null, CancellationToken.None); - var unregistered = await controller.ForgetPassword(null, "x@y.com", CancellationToken.None); + var registered = await controller.ForgetPassword(new ForgetPasswordRequest_Dto { Email = "a@b.com" }, CancellationToken.None); + var unregistered = await controller.ForgetPassword(new ForgetPasswordRequest_Dto { Email = "x@y.com" }, CancellationToken.None); var ok = registered.Should().BeOfType().Subject; var response = ok.Value.Should().BeOfType().Subject; @@ -195,7 +195,7 @@ public class AuthenticationControllerTests logger.Setup(x => x.IsEnabled(It.IsAny())).Returns(true); var controller = new AuthenticationController(service.Object, logger.Object); - var result = await controller.ForgetPassword(new ForgetPasswordRequest_Dto { Email = "a@b.com" }, null, CancellationToken.None); + var result = await controller.ForgetPassword(new ForgetPasswordRequest_Dto { Email = "a@b.com" }, CancellationToken.None); var response = result.Should().BeOfType().Subject.Value.Should().BeOfType().Subject; response.Message.Should().Be(AuthenticationController.ForgetPasswordMessage); @@ -219,7 +219,7 @@ public class AuthenticationControllerTests var controller = NewController(service); - var result = await controller.VerificationCode(new VerificationCode_Dto { Email = "a@b.com", Code = "123456" }, null, null, CancellationToken.None); + var result = await controller.VerificationCode(new VerificationCode_Dto { Email = "a@b.com", Code = "123456" }, CancellationToken.None); if (matched) { @@ -238,16 +238,16 @@ public class AuthenticationControllerTests } [Fact] - public async Task VerificationCode_QueryOnlyCode_PassesNoEmailToTheService() + public async Task VerificationCode_WithoutABody_PassesNoEmailOrCodeToTheService() { var service = new Mock(); var controller = NewController(service); - var result = await controller.VerificationCode(null, null, "123456", CancellationToken.None); + var result = await controller.VerificationCode(null, CancellationToken.None); result.Should().BeOfType().Subject.Value.Should().BeOfType() .Which.Message.Should().Be("Code Not Matched"); - service.Verify(s => s.VerifyCodeAsync(null, "123456", It.IsAny()), Times.Once); + service.Verify(s => s.VerifyCodeAsync(null, null, It.IsAny()), Times.Once); } [Fact] @@ -258,7 +258,7 @@ public class AuthenticationControllerTests .ThrowsAsync(new InvalidOperationException("SECRET-internal-stack-detail")); var controller = NewController(service); - var result = await controller.VerificationCode(new VerificationCode_Dto { Email = "a@b.com", Code = "1" }, null, null, CancellationToken.None); + var result = await controller.VerificationCode(new VerificationCode_Dto { Email = "a@b.com", Code = "1" }, CancellationToken.None); Json(result.Should().BeOfType().Subject.Value) .Should().Be(Json(new Response { Status = "Error", Message = "Code Not Matched" })); diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs index ef8a297..6b539d2 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs @@ -19,14 +19,14 @@ public class AuthenticationServiceTests private static AuthenticationService NewService( Mock userData, Mock forget, - Mock email, + Mock email, out Mock> store, out Mock> hasher) { var (manager, s, h) = IdentityTestHelpers.CreateUserManager(); store = s; hasher = h; - return new AuthenticationService(manager, Microsoft.Extensions.Options.Options.Create(JwtOptions), userData.Object, forget.Object, email.Object, TimeProvider.System); + return new AuthenticationService(manager, Microsoft.Extensions.Options.Options.Create(JwtOptions), userData.Object, forget.Object, email.Object, new InMemoryPasswordResetThrottle(TimeProvider.System), TimeProvider.System); } private static JwtOptions JwtOptions => new() @@ -39,7 +39,7 @@ public class AuthenticationServiceTests [Fact] public async Task Login_UnknownUser_ReturnsNull() { - var service = NewService(new Mock(), new Mock(), new Mock(), out var store, out _); + var service = NewService(new Mock(), new Mock(), new Mock(), out var store, out _); store.Setup(s => s.FindByNameAsync(It.IsAny(), It.IsAny())).ReturnsAsync((ApplicationUser?)null); var result = await service.LoginAsync("nobody", "pw", CancellationToken.None); @@ -51,7 +51,7 @@ public class AuthenticationServiceTests public async Task Login_DeletedUser_RejectedBeforePasswordCheck() { var deletedUser = IdentityTestHelpers.User(isDeleted: true); - var service = NewService(new Mock(), new Mock(), new Mock(), out var store, out var hasher); + var service = NewService(new Mock(), new Mock(), new Mock(), out var store, out var hasher); store.Setup(s => s.FindByNameAsync(It.IsAny(), It.IsAny())).ReturnsAsync(deletedUser); store.Setup(s => s.GetRolesAsync(deletedUser, It.IsAny())).ReturnsAsync(new List()); @@ -65,7 +65,7 @@ public class AuthenticationServiceTests public async Task Login_ValidUser_ReturnsTokenFirstRoleAndIdentity() { var user = IdentityTestHelpers.User(); - var service = NewService(new Mock(), new Mock(), new Mock(), out var store, out var hasher); + var service = NewService(new Mock(), new Mock(), new Mock(), out var store, out var hasher); store.Setup(s => s.FindByNameAsync(It.IsAny(), It.IsAny())).ReturnsAsync(user); store.Setup(s => s.GetRolesAsync(user, It.IsAny())).ReturnsAsync(new List { "Admin", "Manager" }); store.As>() @@ -88,7 +88,7 @@ public class AuthenticationServiceTests public async Task Login_BadPassword_ReturnsNull() { var user = IdentityTestHelpers.User(); - var service = NewService(new Mock(), new Mock(), new Mock(), out var store, out var hasher); + var service = NewService(new Mock(), new Mock(), new Mock(), out var store, out var hasher); store.Setup(s => s.FindByNameAsync(It.IsAny(), It.IsAny())).ReturnsAsync(user); store.As>() .Setup(s => s.GetPasswordHashAsync(user, It.IsAny())).ReturnsAsync("hash"); @@ -99,22 +99,24 @@ public class AuthenticationServiceTests result.Should().BeNull(); } + private static byte[] ResetKey => PasswordResetCodeSecrets.DeriveKey(JwtOptions.Secret); + [Fact] - public async Task ForgetPassword_RegisteredEmail_StoresOnlyASaltedHashAndEmailsTheCode() + public async Task ForgetPassword_RegisteredEmail_StoresOnlyAKeyedHashAndQueuesTheCode() { var user = IdentityTestHelpers.User(); var userData = new Mock(); userData.Setup(u => u.GetByEmailNormalizedAsync("alice@example.com", It.IsAny())).ReturnsAsync(user); var forget = new Mock(); - var email = new Mock(); + var email = new Mock(); string? stored = null, salt = null, body = null; DateTime expires = default; - forget.Setup(f => f.ReplaceCodeAsync(user.Email!, user.Id, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) - .Callback((_, _, h, s, e, _) => { stored = h; salt = s; expires = e; }) + forget.Setup(f => f.ReplaceCodeAsync(user.Email!, user.Id, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((_, _, h, s, e, _, _) => { stored = h; salt = s; expires = e; }) .Returns(Task.CompletedTask); - email.Setup(e => e.SendEmailAsync(user.Email!, "Forget Password Request.", It.IsAny())) + email.Setup(e => e.TryEnqueue(user.Email!, "Forget Password Request.", It.IsAny())) .Callback((_, _, b) => body = b) - .ReturnsAsync(true); + .Returns(true); var service = NewService(userData, forget, email, out _, out _); var before = DateTime.UtcNow; @@ -124,25 +126,27 @@ public class AuthenticationServiceTests var code = System.Text.RegularExpressions.Regex.Match(body!, @"Your Password Reset Code is: (\d{6})").Groups[1].Value; code.Should().HaveLength(6); stored.Should().NotBe(code).And.MatchRegex("^[0-9a-f]{64}$"); - PasswordResetCodeSecrets.Matches(salt!, code, stored!).Should().BeTrue(); + PasswordResetCodeSecrets.Matches(ResetKey, salt!, code, stored!).Should().BeTrue(); expires.Should().BeCloseTo(before.AddMinutes(15), TimeSpan.FromSeconds(5)); } [Fact] - public async Task ForgetPassword_UnknownEmail_SendsNothingButStillDoesTheDatabaseRoundTrip() + public async Task ForgetPassword_UnknownEmail_MakesTheSameDataCallsAndQueuesNothing() { var userData = new Mock(); - userData.Setup(u => u.GetByEmailNormalizedAsync(It.IsAny(), It.IsAny())).ReturnsAsync((ApplicationUser?)null); - var forget = new Mock(); - var email = new Mock(); + userData.Setup(u => u.GetByEmailNormalizedAsync("alice@example.com", It.IsAny())).ReturnsAsync(IdentityTestHelpers.User()); + var registered = new Mock(); + var unregistered = new Mock(); + var email = new Mock(); - var service = NewService(userData, forget, email, out _, out _); + await NewService(userData, registered, email, out _, out _).ForgetPasswordAsync("alice@example.com", CancellationToken.None); + await NewService(userData, unregistered, email, out _, out _).ForgetPasswordAsync("nope@example.com", CancellationToken.None); - await service.ForgetPasswordAsync("nope@example.com", CancellationToken.None); - - forget.Verify(f => f.RemoveByEmailAsync("nope@example.com", It.IsAny()), Times.Once); - forget.Verify(f => f.ReplaceCodeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - email.Verify(e => e.SendEmailAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + registered.Invocations.Select(call => call.Method.Name) + .Should().Equal(unregistered.Invocations.Select(call => call.Method.Name)) + .And.Equal(nameof(IForgetPasswordDataService.ReplaceCodeAsync)); + unregistered.Verify(f => f.ReplaceCodeAsync("nope@example.com", string.Empty, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + email.Verify(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } [Fact] @@ -152,13 +156,13 @@ public class AuthenticationServiceTests userData.Setup(u => u.GetByEmailNormalizedAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(IdentityTestHelpers.User(isDeleted: true)); var forget = new Mock(); - var email = new Mock(); + var email = new Mock(); var service = NewService(userData, forget, email, out _, out _); await service.ForgetPasswordAsync("alice@example.com", CancellationToken.None); - email.Verify(e => e.SendEmailAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + email.Verify(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -171,7 +175,7 @@ public class AuthenticationServiceTests { var forget = new Mock(MockBehavior.Strict); - var service = NewService(new Mock(), forget, new Mock(), out _, out _); + var service = NewService(new Mock(), forget, new Mock(), out _, out _); var result = await service.VerifyCodeAsync(emailAddress, code, CancellationToken.None); @@ -184,7 +188,7 @@ public class AuthenticationServiceTests var forget = new Mock(); forget.Setup(f => f.GetByEmailAsync(It.IsAny(), It.IsAny())).ReturnsAsync((ForgetPasswordCode?)null); - var service = NewService(new Mock(), forget, new Mock(), out var store, out _); + var service = NewService(new Mock(), forget, new Mock(), out var store, out _); var result = await service.ResetPasswordAsync("a@b.com", "999999", "new", CancellationToken.None); @@ -196,12 +200,12 @@ public class AuthenticationServiceTests [Fact] public async Task ResetPassword_AttemptBudgetSpent_DeletesTheCodeAndFails() { - var pending = new ForgetPasswordCode { Id = 7, Email = "a@b.com", UserId = "u1", CodeSalt = "s", CodeHash = PasswordResetCodeSecrets.Hash("s", "123456"), FailedAttempts = 5 }; + var pending = new ForgetPasswordCode { Id = 7, Email = "a@b.com", UserId = "u1", CodeSalt = "s", CodeHash = PasswordResetCodeSecrets.Hash(ResetKey, "s", "123456"), FailedAttempts = 5 }; var forget = new Mock(); forget.Setup(f => f.GetByEmailAsync("a@b.com", It.IsAny())).ReturnsAsync(pending); forget.Setup(f => f.TryConsumeAttemptAsync(7, AuthenticationService.MaxCodeAttempts, It.IsAny(), It.IsAny())).ReturnsAsync(false); - var service = NewService(new Mock(), forget, new Mock(), out var store, out _); + var service = NewService(new Mock(), forget, new Mock(), out var store, out _); var result = await service.ResetPasswordAsync("a@b.com", "123456", "New@67890", CancellationToken.None); @@ -217,11 +221,11 @@ public class AuthenticationServiceTests public void ResetCodeHash_IsSaltedAndComparedByValue(string issued, string candidate, bool expected) { var salt = PasswordResetCodeSecrets.NewSalt(); - var hash = PasswordResetCodeSecrets.Hash(salt, issued); + var hash = PasswordResetCodeSecrets.Hash(ResetKey, salt, issued); - PasswordResetCodeSecrets.Matches(salt, candidate, hash).Should().Be(expected); - PasswordResetCodeSecrets.Hash(PasswordResetCodeSecrets.NewSalt(), issued).Should().NotBe(hash); - PasswordResetCodeSecrets.Matches("", issued, hash).Should().BeFalse(); - PasswordResetCodeSecrets.Matches(salt, issued, "").Should().BeFalse(); + PasswordResetCodeSecrets.Matches(ResetKey, salt, candidate, hash).Should().Be(expected); + PasswordResetCodeSecrets.Hash(ResetKey, PasswordResetCodeSecrets.NewSalt(), issued).Should().NotBe(hash); + PasswordResetCodeSecrets.Matches(ResetKey, "", issued, hash).Should().BeFalse(); + PasswordResetCodeSecrets.Matches(ResetKey, salt, issued, "").Should().BeFalse(); } } diff --git a/Api.SeaHavenIndustries/Controllers/AuthenticationController.cs b/Api.SeaHavenIndustries/Controllers/AuthenticationController.cs index 469c1e9..1c4ddf4 100644 --- a/Api.SeaHavenIndustries/Controllers/AuthenticationController.cs +++ b/Api.SeaHavenIndustries/Controllers/AuthenticationController.cs @@ -111,20 +111,19 @@ namespace Api.SeaHavenIndustries.Controllers public const string ForgetPasswordMessage = "If that email belongs to an account, a reset code has been sent to it."; - // Email and code are read from the JSON body so they stay out of URLs and proxy - // access logs; the query-string form is still accepted for older clients. + // Email and code are read only from the JSON body, so they never appear in URLs + // or in proxy and load balancer access logs. [AllowAnonymous] [HttpPost()] [Route("ForgetPassword")] [EnableRateLimiting(PasswordResetRateLimiting.ForgetPasswordPolicy)] public async Task ForgetPassword( [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] ForgetPasswordRequest_Dto? body, - [FromQuery(Name = "Email")] string? email, CancellationToken cancellationToken) { try { - await _authenticationService.ForgetPasswordAsync(body?.Email ?? email, cancellationToken); + await _authenticationService.ForgetPasswordAsync(body?.Email, cancellationToken); } catch (Exception ex) { @@ -142,13 +141,11 @@ namespace Api.SeaHavenIndustries.Controllers [EnableRateLimiting(PasswordResetRateLimiting.VerificationCodePolicy)] public async Task VerificationCode( [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] VerificationCode_Dto? body, - [FromQuery] string? email, - [FromQuery] string? code, CancellationToken cancellationToken) { try { - if (await _authenticationService.VerifyCodeAsync(body?.Email ?? email, body?.Code ?? code, cancellationToken)) + if (await _authenticationService.VerifyCodeAsync(body?.Email, body?.Code, cancellationToken)) { return Ok(new Response { Status = "Success ", Message = "Code Matched" }); diff --git a/Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs b/Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs new file mode 100644 index 0000000..348073f --- /dev/null +++ b/Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs @@ -0,0 +1,99 @@ +using System.Threading.Channels; +using SeaHaven.Services.Interfaces; + +namespace Api.SeaHavenIndustries.HostedServices +{ + public static class PasswordResetEmailDelivery + { + /// + /// Registers the process-wide reset email queue and the background service that + /// drains it. Both must be singletons: the request and the sender share one channel. + /// + public static IServiceCollection AddPasswordResetEmailDelivery(this IServiceCollection services) + { + services.AddSingleton(); + services.AddSingleton(provider => provider.GetRequiredService()); + services.AddHostedService(); + return services; + } + } + + public sealed record PasswordResetEmail(string EmailTo, string Subject, string Body); + + public sealed class PasswordResetEmailChannel : IPasswordResetEmailQueue + { + public const int Capacity = 1000; + + private readonly Channel _channel = Channel.CreateBounded( + new BoundedChannelOptions(Capacity) + { + FullMode = BoundedChannelFullMode.DropWrite, + SingleReader = true + }); + private readonly ILogger _logger; + private int _pending; + + public PasswordResetEmailChannel(ILogger logger) + { + _logger = logger; + } + + public ChannelReader Reader => _channel.Reader; + + /// Emails accepted and not yet handed to the mail provider. + public int Pending => Volatile.Read(ref _pending); + + public bool TryEnqueue(string emailTo, string subject, string body) + { + Interlocked.Increment(ref _pending); + if (_channel.Writer.TryWrite(new PasswordResetEmail(emailTo, subject, body))) + return true; + + Interlocked.Decrement(ref _pending); + _logger.LogWarning("Password reset email queue is full; an email was dropped."); + return false; + } + + public void MarkHandled() => Interlocked.Decrement(ref _pending); + } + + public sealed class PasswordResetEmailSenderHostedService : BackgroundService + { + private readonly PasswordResetEmailChannel _channel; + private readonly IServiceScopeFactory _scopeFactory; + private readonly ILogger _logger; + + public PasswordResetEmailSenderHostedService( + PasswordResetEmailChannel channel, + IServiceScopeFactory scopeFactory, + ILogger logger) + { + _channel = channel; + _scopeFactory = scopeFactory; + _logger = logger; + } + + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + await foreach (var email in _channel.Reader.ReadAllAsync(stoppingToken)) + { + try + { + await using var scope = _scopeFactory.CreateAsyncScope(); + var sender = scope.ServiceProvider.GetRequiredService(); + if (!await sender.SendEmailAsync(email.EmailTo, email.Subject, email.Body)) + _logger.LogWarning("Password reset email was not accepted by the mail provider."); + } + catch (Exception ex) + { + // The message can echo the recipient or the body, so only the type is logged. + _logger.LogError("Password reset email failed with {ExceptionType}.", ex.GetType().FullName); + } + finally + { + _channel.MarkHandled(); + } + } + } + } +} diff --git a/Api.SeaHavenIndustries/Program.cs b/Api.SeaHavenIndustries/Program.cs index 5dd1d2f..073a64a 100644 --- a/Api.SeaHavenIndustries/Program.cs +++ b/Api.SeaHavenIndustries/Program.cs @@ -65,6 +65,7 @@ builder.Services.AddResponseCompression(opts => new[] { "application/octet-stream" }); }); builder.Services.AddPasswordResetRateLimiting(); +builder.Services.AddPasswordResetEmailDelivery(); builder.Services.AddCors(option => option.AddDefaultPolicy(builder => builder.AllowAnyOrigin().AllowAnyHeader().AllowAnyMethod())); diff --git a/SeaHaven.DataServices/Implementation/ForgetPasswordDataService.cs b/SeaHaven.DataServices/Implementation/ForgetPasswordDataService.cs index 7e3b3c4..c824fc9 100644 --- a/SeaHaven.DataServices/Implementation/ForgetPasswordDataService.cs +++ b/SeaHaven.DataServices/Implementation/ForgetPasswordDataService.cs @@ -13,14 +13,17 @@ namespace SeaHaven.DataServices.Implementation _context = context; } - public async Task ReplaceCodeAsync(string email, string userId, string codeHash, string codeSalt, DateTime expiresAtUtc, CancellationToken cancellationToken) + public async Task ReplaceCodeAsync(string email, string userId, string codeHash, string codeSalt, DateTime expiresAtUtc, DateTime nowUtc, CancellationToken cancellationToken) { var normalizedEmail = Normalize(email); - var existing = await _context.ForgetPasswordCodes - .Where(u => u.Email.ToLower().Trim() == normalizedEmail) - .ToListAsync(cancellationToken); - _context.ForgetPasswordCodes.RemoveRange(existing); + await using var transaction = await _context.Database.BeginTransactionAsync(cancellationToken); + + // Every request also clears expired codes of any email, which keeps the + // table bounded and gives each request the same database work. + await _context.ForgetPasswordCodes + .Where(u => u.Email.ToLower().Trim() == normalizedEmail || u.ExpiresAtUtc <= nowUtc) + .ExecuteDeleteAsync(cancellationToken); _context.ForgetPasswordCodes.Add(new ForgetPasswordCode { @@ -33,6 +36,14 @@ namespace SeaHaven.DataServices.Implementation FailedAttempts = 0 }); await _context.SaveChangesAsync(cancellationToken); + await transaction.CommitAsync(cancellationToken); + } + + public async Task PurgeExpiredAsync(DateTime nowUtc, CancellationToken cancellationToken) + { + await _context.ForgetPasswordCodes + .Where(u => u.ExpiresAtUtc <= nowUtc) + .ExecuteDeleteAsync(cancellationToken); } public async Task GetByEmailAsync(string email, CancellationToken cancellationToken) diff --git a/SeaHaven.DataServices/Interfaces/IForgetPasswordDataService.cs b/SeaHaven.DataServices/Interfaces/IForgetPasswordDataService.cs index a7de61e..c34e9ba 100644 --- a/SeaHaven.DataServices/Interfaces/IForgetPasswordDataService.cs +++ b/SeaHaven.DataServices/Interfaces/IForgetPasswordDataService.cs @@ -4,8 +4,13 @@ namespace SeaHaven.DataServices.Interfaces { public interface IForgetPasswordDataService { - /// Deletes every pending code for the email, then stores the new one. - Task ReplaceCodeAsync(string email, string userId, string codeHash, string codeSalt, DateTime expiresAtUtc, CancellationToken cancellationToken); + /// + /// In one transaction, deletes every pending code for the email and every expired + /// code, then stores the new one. + /// + Task ReplaceCodeAsync(string email, string userId, string codeHash, string codeSalt, DateTime expiresAtUtc, DateTime nowUtc, CancellationToken cancellationToken); + + Task PurgeExpiredAsync(DateTime nowUtc, CancellationToken cancellationToken); /// Returns the pending code for exactly this email, or null. Task GetByEmailAsync(string email, CancellationToken cancellationToken); diff --git a/SeaHaven.Services/DependencyInjection/ServicesModule.cs b/SeaHaven.Services/DependencyInjection/ServicesModule.cs index e24dec3..a6c2f53 100644 --- a/SeaHaven.Services/DependencyInjection/ServicesModule.cs +++ b/SeaHaven.Services/DependencyInjection/ServicesModule.cs @@ -73,6 +73,9 @@ namespace SeaHaven.Services.DependencyInjection services.AddValidatorsFromAssembly(assembly); + // Process-wide counters: a scoped instance would start empty on every request. + services.AddSingleton(); + services.AddScoped( sp => (IWorkOrderReconciliationRunner)sp.GetRequiredService()); diff --git a/SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs b/SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs new file mode 100644 index 0000000..00ad7aa --- /dev/null +++ b/SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs @@ -0,0 +1,119 @@ +using System.Security.Cryptography; +using System.Text; +using SeaHaven.Services.Interfaces; + +namespace SeaHaven.Services.Helpers +{ + /// + /// Process-wide sliding-window counters for . + /// Registered as a singleton; the API runs as a single instance, and a restart + /// clears the windows. Emails are held only as SHA-256 digests. + /// + public sealed class InMemoryPasswordResetThrottle : IPasswordResetThrottle + { + public const int CodeRequestsPerHour = 3; + public const int CodeRequestsPerDay = 10; + public const int FailedChecksPerDay = 10; + + private static readonly TimeSpan Hour = TimeSpan.FromHours(1); + private static readonly TimeSpan Day = TimeSpan.FromDays(1); + private const int SweepEvery = 1024; + + private readonly TimeProvider _timeProvider; + private readonly object _gate = new(); + private readonly Dictionary _accounts = new(StringComparer.Ordinal); + private int _operations; + + public InMemoryPasswordResetThrottle(TimeProvider timeProvider) + { + _timeProvider = timeProvider; + } + + public bool TryAcceptCodeRequest(string email) + { + var now = _timeProvider.GetUtcNow(); + lock (_gate) + { + var account = AccountFor(email, now); + if (account.Requests.Count >= CodeRequestsPerDay + || account.Requests.Count(at => at > now - Hour) >= CodeRequestsPerHour) + { + return false; + } + + account.Requests.Add(now); + return true; + } + } + + public bool TryReserveCheck(string email) + { + var now = _timeProvider.GetUtcNow(); + lock (_gate) + { + var account = AccountFor(email, now); + if (account.FailedChecks.Count >= FailedChecksPerDay) + return false; + + account.FailedChecks.Add(now); + return true; + } + } + + public void ReleaseCheck(string email) + { + var now = _timeProvider.GetUtcNow(); + lock (_gate) + { + var account = AccountFor(email, now); + if (account.FailedChecks.Count > 0) + account.FailedChecks.RemoveAt(account.FailedChecks.Count - 1); + } + } + + /// The same normalization the user lookup applies: trimmed, invariant upper case. + public static string KeyFor(string email) + { + var normalized = (email ?? string.Empty).Trim().ToUpperInvariant(); + return Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(normalized))); + } + + private Account AccountFor(string email, DateTimeOffset now) + { + if (++_operations % SweepEvery == 0) + Sweep(now); + + var key = KeyFor(email); + if (!_accounts.TryGetValue(key, out var account)) + { + account = new Account(); + _accounts[key] = account; + } + + account.Prune(now - Day); + return account; + } + + private void Sweep(DateTimeOffset now) + { + foreach (var (key, account) in _accounts.ToList()) + { + account.Prune(now - Day); + if (account.Requests.Count == 0 && account.FailedChecks.Count == 0) + _accounts.Remove(key); + } + } + + private sealed class Account + { + public List Requests { get; } = new(); + public List FailedChecks { get; } = new(); + + public void Prune(DateTimeOffset cutoff) + { + Requests.RemoveAll(at => at <= cutoff); + FailedChecks.RemoveAll(at => at <= cutoff); + } + } + } +} diff --git a/SeaHaven.Services/Helpers/PasswordResetCodeSecrets.cs b/SeaHaven.Services/Helpers/PasswordResetCodeSecrets.cs index 39762dd..edee811 100644 --- a/SeaHaven.Services/Helpers/PasswordResetCodeSecrets.cs +++ b/SeaHaven.Services/Helpers/PasswordResetCodeSecrets.cs @@ -6,10 +6,24 @@ namespace SeaHaven.Services.Helpers { /// /// Generation and hashing for emailed password reset codes. The raw code exists - /// only in memory and in the email sent to the account holder. + /// only in memory and in the email sent to the account holder. Hashes are keyed + /// with a server-side key, so a copy of the database alone cannot be used to + /// brute-force the six-digit codes offline. /// public static class PasswordResetCodeSecrets { + private static readonly byte[] KeyInfo = Encoding.UTF8.GetBytes("password-reset-code-v1"); + + /// + /// Derives the code-hashing key from an existing server secret with HKDF, so no + /// new secret is needed and the derived key is useless for anything else. + /// + public static byte[] DeriveKey(string serverSecret) + { + ArgumentException.ThrowIfNullOrEmpty(serverSecret); + return HKDF.DeriveKey(HashAlgorithmName.SHA256, Encoding.UTF8.GetBytes(serverSecret), 32, Array.Empty(), KeyInfo); + } + public static string NewCode() { return RandomNumberGenerator.GetInt32(0, 1_000_000).ToString("D6", CultureInfo.InvariantCulture); @@ -20,19 +34,26 @@ namespace SeaHaven.Services.Helpers return Convert.ToHexString(RandomNumberGenerator.GetBytes(16)).ToLowerInvariant(); } - public static string Hash(string salt, string code) + /// A value shaped like a hash that no code can match. + public static string NewUnmatchableHash() { - ArgumentNullException.ThrowIfNull(salt); - ArgumentNullException.ThrowIfNull(code); - return Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(salt + ":" + code))).ToLowerInvariant(); + return Convert.ToHexString(RandomNumberGenerator.GetBytes(32)).ToLowerInvariant(); } - public static bool Matches(string salt, string candidate, string expectedHash) + public static string Hash(byte[] key, string salt, string code) + { + ArgumentNullException.ThrowIfNull(key); + ArgumentNullException.ThrowIfNull(salt); + ArgumentNullException.ThrowIfNull(code); + return Convert.ToHexString(HMACSHA256.HashData(key, Encoding.UTF8.GetBytes(salt + ":" + code))).ToLowerInvariant(); + } + + public static bool Matches(byte[] key, string salt, string candidate, string expectedHash) { if (string.IsNullOrEmpty(salt) || string.IsNullOrEmpty(expectedHash)) return false; - var actual = Encoding.ASCII.GetBytes(Hash(salt, candidate.Trim())); + var actual = Encoding.ASCII.GetBytes(Hash(key, salt, candidate.Trim())); var expected = Encoding.ASCII.GetBytes(expectedHash); return CryptographicOperations.FixedTimeEquals(actual, expected); } diff --git a/SeaHaven.Services/Implementation/AuthenticationService.cs b/SeaHaven.Services/Implementation/AuthenticationService.cs index 1a0d263..94e0833 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -19,28 +19,37 @@ namespace SeaHaven.Services.Implementation private readonly JwtOptions _jwtOptions; private readonly IUserDataService _userDataService; private readonly IForgetPasswordDataService _forgetPasswordDataService; - private readonly IEmailSender _emailSender; + private readonly IPasswordResetEmailQueue _resetEmails; + private readonly IPasswordResetThrottle _resetThrottle; private readonly TimeProvider _timeProvider; + private byte[]? _resetCodeKey; public static readonly TimeSpan ResetCodeLifetime = TimeSpan.FromMinutes(15); public const int MaxCodeAttempts = 5; + /// Longest address stored for a reset request; Identity caps emails at 256. + public const int MaxResetEmailLength = 256; + public AuthenticationService( UserManager userManager, IOptions jwtOptions, IUserDataService userDataService, IForgetPasswordDataService forgetPasswordDataService, - IEmailSender emailSender, + IPasswordResetEmailQueue resetEmails, + IPasswordResetThrottle resetThrottle, TimeProvider timeProvider) { _userManager = userManager; _jwtOptions = jwtOptions.Value; _userDataService = userDataService; _forgetPasswordDataService = forgetPasswordDataService; - _emailSender = emailSender; + _resetEmails = resetEmails; + _resetThrottle = resetThrottle; _timeProvider = timeProvider; } + private byte[] ResetCodeKey => _resetCodeKey ??= PasswordResetCodeSecrets.DeriveKey(_jwtOptions.Secret); + public async Task LoginAsync(string? username, string? password, CancellationToken cancellationToken) { var user = await _userManager.FindByNameAsync(username ?? ""); @@ -119,26 +128,42 @@ namespace SeaHaven.Services.Implementation public async Task ForgetPasswordAsync(string? email, CancellationToken cancellationToken) { - // Registered and unregistered addresses take the same path up to the - // email send: one user lookup, one code generated and hashed, one write. + // Registered and unregistered addresses do the same work: one user lookup, + // one code generated and hashed, and the same replace in the database. An + // unregistered address gets a row no code can match. The email itself is + // queued, so the response never waits on the mail provider. var requested = email?.Trim() ?? string.Empty; - var user = requested.Length == 0 - ? null - : await _userDataService.GetByEmailNormalizedAsync(requested, cancellationToken); - var code = PasswordResetCodeSecrets.NewCode(); - var salt = PasswordResetCodeSecrets.NewSalt(); - var hash = PasswordResetCodeSecrets.Hash(salt, code); + if (requested.Length == 0 || requested.Length > MaxResetEmailLength) + return; - if (user == null || user.IsDeleted == true || string.IsNullOrWhiteSpace(user.Email)) + var user = await _userDataService.GetByEmailNormalizedAsync(requested, cancellationToken); + var nowUtc = _timeProvider.GetUtcNow().UtcDateTime; + if (!_resetThrottle.TryAcceptCodeRequest(requested)) { - await _forgetPasswordDataService.RemoveByEmailAsync(requested, cancellationToken); + // Over the per-email limit: keep the current code and send nothing. + await _forgetPasswordDataService.PurgeExpiredAsync(nowUtc, cancellationToken); return; } - var expiresAtUtc = _timeProvider.GetUtcNow().UtcDateTime.Add(ResetCodeLifetime); - await _forgetPasswordDataService.ReplaceCodeAsync(user.Email, user.Id, hash, salt, expiresAtUtc, cancellationToken); - var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; - await _emailSender.SendEmailAsync(user.Email, "Forget Password Request.", body); + var code = PasswordResetCodeSecrets.NewCode(); + var salt = PasswordResetCodeSecrets.NewSalt(); + var hash = PasswordResetCodeSecrets.Hash(ResetCodeKey, salt, code); + var active = user != null && user.IsDeleted != true && !string.IsNullOrWhiteSpace(user.Email); + + await _forgetPasswordDataService.ReplaceCodeAsync( + active ? user!.Email! : requested, + active ? user!.Id : string.Empty, + active ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), + salt, + nowUtc.Add(ResetCodeLifetime), + nowUtc, + cancellationToken); + + if (active) + { + var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; + _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); + } } public async Task VerifyCodeAsync(string? email, string? code, CancellationToken cancellationToken) @@ -192,6 +217,12 @@ namespace SeaHaven.Services.Implementation if (string.IsNullOrWhiteSpace(email) || string.IsNullOrWhiteSpace(code)) return null; + // The per-account budget spans every code the account is sent, so asking for + // new codes does not buy more guesses. A slot is reserved before comparing and + // given back only when the code matches. + if (!_resetThrottle.TryReserveCheck(email)) + return null; + var pending = await _forgetPasswordDataService.GetByEmailAsync(email, cancellationToken); if (pending == null) return null; @@ -203,8 +234,11 @@ namespace SeaHaven.Services.Implementation return null; } - if (PasswordResetCodeSecrets.Matches(pending.CodeSalt, code, pending.CodeHash)) + if (PasswordResetCodeSecrets.Matches(ResetCodeKey, pending.CodeSalt, code, pending.CodeHash)) + { + _resetThrottle.ReleaseCheck(email); return pending; + } if (pending.FailedAttempts + 1 >= MaxCodeAttempts) await _forgetPasswordDataService.RemoveByEmailAsync(pending.Email, cancellationToken); diff --git a/SeaHaven.Services/Interfaces/IPasswordResetEmailQueue.cs b/SeaHaven.Services/Interfaces/IPasswordResetEmailQueue.cs new file mode 100644 index 0000000..948f2d0 --- /dev/null +++ b/SeaHaven.Services/Interfaces/IPasswordResetEmailQueue.cs @@ -0,0 +1,13 @@ +namespace SeaHaven.Services.Interfaces +{ + /// + /// Hands a password reset email to a background sender, so the request that asked + /// for it does not wait on the mail provider and cannot be timed against one that + /// sent nothing. + /// + public interface IPasswordResetEmailQueue + { + /// Returns false when the queue is full and the email was dropped. + bool TryEnqueue(string emailTo, string subject, string body); + } +} diff --git a/SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs b/SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs new file mode 100644 index 0000000..8cdcc2e --- /dev/null +++ b/SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs @@ -0,0 +1,24 @@ +namespace SeaHaven.Services.Interfaces +{ + /// + /// Per-account limits on the anonymous password reset flow, keyed on the + /// normalized email so they hold however many client addresses an attacker uses. + /// + public interface IPasswordResetThrottle + { + /// + /// Counts a code request for the email and returns true while it is within the + /// hourly and daily limits. A refused request is not counted. + /// + bool TryAcceptCodeRequest(string email); + + /// + /// Reserves one failed check for the email before a code is compared. Returns + /// false once the account has used its failed checks for the window. + /// + bool TryReserveCheck(string email); + + /// Gives back the reservation of a check whose code matched. + void ReleaseCheck(string email); + } +} diff --git a/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs b/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs new file mode 100644 index 0000000..2431cc4 --- /dev/null +++ b/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs @@ -0,0 +1,241 @@ +using System.Net; +using Api.SeaHavenIndustries.Controllers; +using Api.SeaHavenIndustries.HostedServices; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using SeaHaven.Services.Helpers; +using SeaHaven.Services.Interfaces; + +namespace SeaHavenIndustries.Tests; + +/// +/// Limits that hold per account rather than per client address, the email send +/// being off the request path, and the keyed code hash. +/// +public sealed class PasswordResetAbuseLimitsTests +{ + private const string Alice = "alice@example.com"; + private const string OldPassword = "Old@12345"; + private const string NewPassword = "New@67890"; + private const string CodeNotMatched = "{\"status\":\"Error\",\"message\":\"Code Not Matched\"}"; + private const string ResetFailed = "{\"status\":\"Error\",\"message\":\"Your email or code not found please check\"}"; + + private static int _nextClient; + + // A fresh client address per request, so the per-IP limit never masks the per-account one. + private static string NextClient() + { + var n = Interlocked.Increment(ref _nextClient); + return $"198.51.{n / 250 % 250}.{n % 250 + 1}"; + } + + [Fact] + public async Task Code_requests_are_capped_per_email_at_three_an_hour_and_ten_a_day() + { + await using var host = await PasswordResetTestHost.StartAsync(); + await host.AddUserAsync(Alice, OldPassword); + + var responses = new List(); + for (var request = 0; request < 3; request++) + responses.Add(await (await host.ForgetPasswordAsync(Alice, NextClient())).Content.ReadAsStringAsync()); + var liveCode = host.Sent.LatestCodeFor(Alice); + + // A differently cased and padded address is the same account. + responses.Add(await (await host.ForgetPasswordAsync(" ALICE@Example.com ", NextClient())).Content.ReadAsStringAsync()); + + Assert.Equal(3, host.Sent.Messages.Count); + Assert.Single(responses.Distinct()); + // The refused request neither sent a code nor replaced the one already sent. + Assert.Equal(HttpStatusCode.OK, (await host.VerifyAsync(Alice, liveCode, NextClient())).StatusCode); + + for (var hour = 1; hour <= 3; hour++) + { + host.Time.Advance(TimeSpan.FromHours(1)); + for (var request = 0; request < 3; request++) + await host.ForgetPasswordAsync(Alice, NextClient()); + } + + Assert.Equal(10, host.Sent.Messages.Count); + + host.Time.Advance(TimeSpan.FromHours(20)); + await host.ForgetPasswordAsync(Alice, NextClient()); + Assert.Equal(10, host.Sent.Messages.Count); + + host.Time.Advance(TimeSpan.FromHours(1)); + await host.ForgetPasswordAsync(Alice, NextClient()); + Assert.Equal(11, host.Sent.Messages.Count); + } + + [Fact] + public async Task Failed_checks_are_capped_per_account_across_every_code_it_is_sent() + { + await using var host = await PasswordResetTestHost.StartAsync(); + await host.AddUserAsync(Alice, OldPassword); + + for (var round = 0; round < 2; round++) + { + await host.ForgetPasswordAsync(Alice, NextClient()); + var code = host.Sent.LatestCodeFor(Alice); + for (var attempt = 0; attempt < 5; attempt++) + await host.VerifyAsync(Alice, WrongCode(code), NextClient()); + } + + await host.ForgetPasswordAsync(Alice, NextClient()); + var third = host.Sent.LatestCodeFor(Alice); + + Assert.Equal(CodeNotMatched, await (await host.VerifyAsync(Alice, third, NextClient())).Content.ReadAsStringAsync()); + Assert.Equal(ResetFailed, await (await host.ResetAsync(Alice, third, NewPassword, NextClient())).Content.ReadAsStringAsync()); + Assert.Equal(HttpStatusCode.OK, (await host.LoginAsync(Alice, OldPassword)).StatusCode); + + host.Time.Advance(TimeSpan.FromDays(1)); + await host.ForgetPasswordAsync(Alice, NextClient()); + Assert.Equal(HttpStatusCode.OK, (await host.ResetAsync(Alice, host.Sent.LatestCodeFor(Alice), NewPassword, NextClient())).StatusCode); + } + + [Fact] + public void Concurrent_checks_can_never_exceed_the_account_budget() + { + var throttle = new InMemoryPasswordResetThrottle(new ManualTimeProvider()); + + var reserved = 0; + Parallel.For(0, 64, _ => + { + if (throttle.TryReserveCheck(Alice)) + Interlocked.Increment(ref reserved); + }); + + Assert.Equal(InMemoryPasswordResetThrottle.FailedChecksPerDay, reserved); + } + + [Fact] + public void A_matching_check_gives_its_reservation_back() + { + var throttle = new InMemoryPasswordResetThrottle(new ManualTimeProvider()); + + for (var check = 0; check < 50; check++) + { + Assert.True(throttle.TryReserveCheck(Alice)); + throttle.ReleaseCheck(Alice); + } + + Assert.True(throttle.TryReserveCheck(Alice.ToUpperInvariant())); + } + + [Fact] + public async Task ForgetPassword_answers_before_the_email_is_sent() + { + var sender = new BlockingEmailSender(); + await using var host = await PasswordResetTestHost.StartAsync(sender); + await host.AddUserAsync(Alice, OldPassword); + try + { + var response = await host.Client + .SendAsync(PasswordResetTestHost.Post("api/Authentication/ForgetPassword", new { email = Alice })) + .WaitAsync(TimeSpan.FromSeconds(5)); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Contains(AuthenticationController.ForgetPasswordMessage, await response.Content.ReadAsStringAsync()); + Assert.False(sender.Completed); + } + finally + { + sender.Release(); + } + + await host.WaitForEmailDrainAsync(); + Assert.True(sender.Completed); + } + + [Fact] + public async Task An_unregistered_email_gets_the_same_database_write_but_a_code_nothing_can_match() + { + await using var host = await PasswordResetTestHost.StartAsync(); + var alice = await host.AddUserAsync(Alice, OldPassword); + + await host.ForgetPasswordAsync(Alice); + await host.ForgetPasswordAsync("nobody@example.com"); + + var rows = await host.PendingCodesAsync(); + Assert.Equal(2, rows.Count); + var registered = Assert.Single(rows, row => row.Email == Alice); + var unregistered = Assert.Single(rows, row => row.Email == "nobody@example.com"); + Assert.Equal(alice.Id, registered.UserId); + Assert.Equal(string.Empty, unregistered.UserId); + Assert.Matches("^[0-9a-f]{64}$", unregistered.CodeHash); + Assert.Equal(registered.ExpiresAtUtc, unregistered.ExpiresAtUtc); + Assert.Single(host.Sent.Messages); + + Assert.Equal(CodeNotMatched, await (await host.VerifyAsync("nobody@example.com", "000000")).Content.ReadAsStringAsync()); + } + + [Fact] + public async Task Each_request_clears_expired_codes_so_decoy_rows_do_not_accumulate() + { + await using var host = await PasswordResetTestHost.StartAsync(); + + await host.ForgetPasswordAsync("first@example.com"); + await host.ForgetPasswordAsync("second@example.com"); + host.Time.Advance(TimeSpan.FromMinutes(16)); + await host.ForgetPasswordAsync("third@example.com"); + + var row = Assert.Single(await host.PendingCodesAsync()); + Assert.Equal("third@example.com", row.Email); + } + + [Fact] + public async Task A_code_stored_in_the_previous_unkeyed_format_no_longer_matches() + { + await using var host = await PasswordResetTestHost.StartAsync(); + var alice = await host.AddUserAsync(Alice, OldPassword); + var code = await host.AddSiblingCodeAsync(Alice, alice.Id, keyedHash: false); + + Assert.Equal(CodeNotMatched, await (await host.VerifyAsync(Alice, code)).Content.ReadAsStringAsync()); + Assert.Equal(ResetFailed, await (await host.ResetAsync(Alice, code, NewPassword)).Content.ReadAsStringAsync()); + } + + [Fact] + public void A_code_hashed_under_another_server_secret_does_not_match() + { + var salt = PasswordResetCodeSecrets.NewSalt(); + var hash = PasswordResetCodeSecrets.Hash(PasswordResetCodeSecrets.DeriveKey(new string('a', 64)), salt, "123456"); + + Assert.True(PasswordResetCodeSecrets.Matches(PasswordResetCodeSecrets.DeriveKey(new string('a', 64)), salt, "123456", hash)); + Assert.False(PasswordResetCodeSecrets.Matches(PasswordResetCodeSecrets.DeriveKey(new string('b', 64)), salt, "123456", hash)); + } + + [Fact] + public async Task Reset_queue_and_throttle_are_process_wide_and_the_sender_runs() + { + await using var host = await PasswordResetTestHost.StartAsync(); + + object Resolve() where T : notnull + { + using var scope = host.Services.CreateScope(); + return scope.ServiceProvider.GetRequiredService(); + } + + Assert.Same(Resolve(), Resolve()); + Assert.Same(Resolve(), Resolve()); + Assert.Same(Resolve(), Resolve()); + Assert.Contains(host.Services.GetServices(), service => service is PasswordResetEmailSenderHostedService); + } + + private static string WrongCode(string code) => + ((int.Parse(code) + 1) % 1_000_000).ToString("D6"); + + private sealed class BlockingEmailSender : IEmailSender + { + private readonly TaskCompletionSource _gate = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public bool Completed { get; private set; } + + public void Release() => _gate.TrySetResult(); + + public async Task SendEmailAsync(string emailTo, string subject, string body) + { + await _gate.Task; + Completed = true; + return true; + } + } +} diff --git a/SeaHavenIndustries.Tests/PasswordResetFlowTests.cs b/SeaHavenIndustries.Tests/PasswordResetFlowTests.cs index 51f3c70..fe4984d 100644 --- a/SeaHavenIndustries.Tests/PasswordResetFlowTests.cs +++ b/SeaHavenIndustries.Tests/PasswordResetFlowTests.cs @@ -5,6 +5,7 @@ using Api.SeaHavenIndustries.Controllers; using Api.SeaHavenIndustries.Infrastructure; using Microsoft.Extensions.DependencyInjection; using SeaHaven.DataServices.Interfaces; +using SeaHaven.Services.Helpers; using SeaHaven.Services.Interfaces; namespace SeaHavenIndustries.Tests; @@ -64,7 +65,7 @@ public sealed class PasswordResetFlowTests var message = Assert.Single(host.Sent.Messages); Assert.Equal(Alice, message.To); - Assert.Single(await host.PendingCodesAsync()); + Assert.Single(await host.PendingCodesAsync(), row => row.UserId.Length > 0); } [Fact] @@ -84,7 +85,7 @@ public sealed class PasswordResetFlowTests } [Fact] - public async Task Stored_code_is_a_salted_hash_and_the_plaintext_is_only_in_the_email() + public async Task Stored_code_is_keyed_with_a_server_secret_and_the_plaintext_is_only_in_the_email() { await using var host = await PasswordResetTestHost.StartAsync(); await host.AddUserAsync(Alice, OldPassword); @@ -96,8 +97,11 @@ public sealed class PasswordResetFlowTests Assert.Equal(string.Empty, row.Code); Assert.Matches("^[0-9a-f]{32}$", row.CodeSalt); Assert.Matches("^[0-9a-f]{64}$", row.CodeHash); - Assert.NotEqual(Sha256Hex(code), row.CodeHash); - Assert.Equal(Sha256Hex(row.CodeSalt + ":" + code), row.CodeHash); + // Salt and hash from the row alone are not enough to test a guess offline. + Assert.NotEqual(Sha256Hex(row.CodeSalt + ":" + code), row.CodeHash); + Assert.Equal( + PasswordResetCodeSecrets.Hash(PasswordResetCodeSecrets.DeriveKey(PasswordResetTestHost.JwtSecret), row.CodeSalt, code), + row.CodeHash); Assert.DoesNotContain(code, string.Join("|", row.Code, row.CodeHash, row.CodeSalt, row.Email, row.UserId)); } @@ -289,18 +293,25 @@ public sealed class PasswordResetFlowTests } [Fact] - public async Task VerificationCode_and_ForgetPassword_still_accept_the_query_string_form() + public async Task Email_and_code_in_the_query_string_are_ignored() { await using var host = await PasswordResetTestHost.StartAsync(); await host.AddUserAsync(Alice, OldPassword); - var requested = await host.Client.SendAsync(PasswordResetTestHost.Post($"api/Authentication/ForgetPassword?Email={Uri.EscapeDataString(Alice)}")); - Assert.Equal(HttpStatusCode.OK, requested.StatusCode); - var code = host.Sent.LatestCodeFor(Alice); + var queryRequest = await host.Client.SendAsync(PasswordResetTestHost.Post($"api/Authentication/ForgetPassword?Email={Uri.EscapeDataString(Alice)}")); + await host.WaitForEmailDrainAsync(); + var bodyRequest = await host.ForgetPasswordAsync("nobody@example.com"); + Assert.Equal(HttpStatusCode.OK, queryRequest.StatusCode); + Assert.Equal(await bodyRequest.Content.ReadAsStringAsync(), await queryRequest.Content.ReadAsStringAsync()); + Assert.Empty(host.Sent.Messages); + await host.ForgetPasswordAsync(Alice); + var code = host.Sent.LatestCodeFor(Alice); var verified = await host.Client.SendAsync(PasswordResetTestHost.Post( $"api/Authentication/VerificationCode?email={Uri.EscapeDataString(Alice)}&code={code}")); - Assert.Equal(HttpStatusCode.OK, verified.StatusCode); + + Assert.Equal(HttpStatusCode.BadRequest, verified.StatusCode); + Assert.Equal(CodeNotMatched, await verified.Content.ReadAsStringAsync()); } [Fact] @@ -367,6 +378,7 @@ public sealed class PasswordResetFlowTests var program = File.ReadAllText(Path.Combine(RepoRoot(), "Api.SeaHavenIndustries", "Program.cs")); Assert.Contains("builder.Services.AddPasswordResetRateLimiting();", program); + Assert.Contains("builder.Services.AddPasswordResetEmailDelivery();", program); var build = program.IndexOf("builder.Build()", StringComparison.Ordinal); var forwarded = program.IndexOf("app.UseForwardedHeaders()", StringComparison.Ordinal); var firstMiddleware = program.IndexOf("app.Use", build, StringComparison.Ordinal); diff --git a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs index 6904954..a16774e 100644 --- a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs +++ b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs @@ -2,6 +2,7 @@ using System.Collections.Concurrent; using System.Net.Http.Json; using System.Text.RegularExpressions; using Api.SeaHavenIndustries.Controllers; +using Api.SeaHavenIndustries.HostedServices; using Api.SeaHavenIndustries.Infrastructure; using Data.SeaHavenIndustries; using Microsoft.AspNetCore.Builder; @@ -32,6 +33,8 @@ namespace SeaHavenIndustries.Tests; /// internal sealed class PasswordResetTestHost : IAsyncDisposable { + public static readonly string JwtSecret = new('k', 64); + private readonly WebApplication _app; private readonly string _databasePath; @@ -61,7 +64,7 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable builder.WebHost.UseUrls("http://127.0.0.1:0"); builder.Configuration.AddInMemoryCollection(new Dictionary { - ["JWT:Secret"] = new string('k', 64), + ["JWT:Secret"] = JwtSecret, ["JWT:ValidIssuer"] = "issuer", ["JWT:ValidAudience"] = "audience" }); @@ -90,6 +93,7 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable manager.FeatureProviders.Add(new OnlyAuthenticationController()); }); builder.Services.AddPasswordResetRateLimiting(); + builder.Services.AddPasswordResetEmailDelivery(); var app = builder.Build(); app.UseForwardedHeaders(); @@ -141,7 +145,7 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable /// Stores a second pending code for the email directly, as two concurrent first /// requests could, and returns it. The new row is the newest one. /// - public async Task AddSiblingCodeAsync(string email, string userId) + public async Task AddSiblingCodeAsync(string email, string userId, bool keyedHash = true) { const string code = "424242"; const string salt = "0123456789abcdef0123456789abcdef"; @@ -152,7 +156,9 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable Email = email, UserId = userId, CodeSalt = salt, - CodeHash = SeaHaven.Services.Helpers.PasswordResetCodeSecrets.Hash(salt, code), + CodeHash = keyedHash + ? SeaHaven.Services.Helpers.PasswordResetCodeSecrets.Hash(SeaHaven.Services.Helpers.PasswordResetCodeSecrets.DeriveKey(JwtSecret), salt, code) + : Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(salt + ":" + code))).ToLowerInvariant(), ExpiresAtUtc = Time.GetUtcNow().UtcDateTime.AddMinutes(15) }); await context.SaveChangesAsync(); @@ -169,8 +175,25 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable return request; } - public Task ForgetPasswordAsync(string email, string? clientIp = null) => - Client.SendAsync(Post("api/Authentication/ForgetPassword", new { email }, clientIp)); + /// Requests a code and waits until any queued email has been handed to the sender. + public async Task ForgetPasswordAsync(string email, string? clientIp = null) + { + var response = await Client.SendAsync(Post("api/Authentication/ForgetPassword", new { email }, clientIp)); + await WaitForEmailDrainAsync(); + return response; + } + + public async Task WaitForEmailDrainAsync() + { + var channel = _app.Services.GetRequiredService(); + var deadline = DateTime.UtcNow.AddSeconds(10); + while (channel.Pending > 0) + { + if (DateTime.UtcNow > deadline) + throw new TimeoutException("Queued password reset emails were not sent."); + await Task.Delay(10); + } + } public Task VerifyAsync(string email, string code, string? clientIp = null) => Client.SendAsync(Post("api/Authentication/VerificationCode", new { email, code }, clientIp)); From a6dd40b972c2257cd3f0560702c22330cfb20180 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 13:12:49 -0300 Subject: [PATCH 2/8] fix(auth): only count real guesses, drop codes that cannot be emailed, trace reset email sends --- .../AuthenticationServiceTests.cs | 21 +++++++++++++ .../SentryPipelineContractTests.cs | 1 + .../PasswordResetEmailDelivery.cs | 31 ++++++++++++++++--- .../Helpers/InMemoryPasswordResetThrottle.cs | 11 +++++++ .../Implementation/AuthenticationService.cs | 14 +++++++-- .../Interfaces/IPasswordResetThrottle.cs | 8 ++++- .../PasswordResetAbuseLimitsTests.cs | 28 +++++++++++++++++ .../PasswordResetTestHost.cs | 1 + 8 files changed, 107 insertions(+), 8 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs index 6b539d2..0ac9cde 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs @@ -138,6 +138,7 @@ public class AuthenticationServiceTests var registered = new Mock(); var unregistered = new Mock(); var email = new Mock(); + email.Setup(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny())).Returns(true); await NewService(userData, registered, email, out _, out _).ForgetPasswordAsync("alice@example.com", CancellationToken.None); await NewService(userData, unregistered, email, out _, out _).ForgetPasswordAsync("nope@example.com", CancellationToken.None); @@ -149,6 +150,26 @@ public class AuthenticationServiceTests email.Verify(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } + [Fact] + public async Task ForgetPassword_EmailThatCannotBeQueued_DropsTheCodeAndDoesNotCountTheRequest() + { + var user = IdentityTestHelpers.User(); + var userData = new Mock(); + userData.Setup(u => u.GetByEmailNormalizedAsync(It.IsAny(), It.IsAny())).ReturnsAsync(user); + var forget = new Mock(); + var email = new Mock(); + email.SetupSequence(e => e.TryEnqueue(user.Email!, It.IsAny(), It.IsAny())) + .Returns(false).Returns(false).Returns(false).Returns(true); + + var service = NewService(userData, forget, email, out _, out _); + for (var request = 0; request < 4; request++) + await service.ForgetPasswordAsync("alice@example.com", CancellationToken.None); + + // Three undelivered codes were removed, and they did not use up the hourly limit of three. + forget.Verify(f => f.RemoveByEmailAsync(user.Email!, It.IsAny()), Times.Exactly(3)); + email.Verify(e => e.TryEnqueue(user.Email!, It.IsAny(), It.IsAny()), Times.Exactly(4)); + } + [Fact] public async Task ForgetPassword_DeletedAccount_IsTreatedAsUnregistered() { diff --git a/Api.SeaHavenIndustries.Tests/SentryPipelineContractTests.cs b/Api.SeaHavenIndustries.Tests/SentryPipelineContractTests.cs index b5ff0d3..90f539b 100644 --- a/Api.SeaHavenIndustries.Tests/SentryPipelineContractTests.cs +++ b/Api.SeaHavenIndustries.Tests/SentryPipelineContractTests.cs @@ -61,6 +61,7 @@ public class SentryPipelineContractTests [InlineData("Api.SeaHavenIndustries/HostedServices/WorkOrderWeekRolledHostedService.cs", "workorders.week-rolled-job")] [InlineData("Api.SeaHavenIndustries/HostedServices/PastDueCacheHostedService.cs", "workorders.past-due-cache-job")] [InlineData("Api.SeaHavenIndustries/HostedServices/UpliftLifecycleHostedService.cs", "uplifts.lifecycle-sweep")] + [InlineData("Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs", "auth.password-reset-email")] public void Workers_UseSharedBackgroundTransactionHelper_WithStableNames(string relativePath, string transactionName) { var source = File.ReadAllText(Path.Combine(RepoRoot(), relativePath)); diff --git a/Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs b/Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs index 348073f..5b2b7af 100644 --- a/Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs +++ b/Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs @@ -1,5 +1,7 @@ using System.Threading.Channels; +using Api.SeaHavenIndustries.Observability; using SeaHaven.Services.Interfaces; +using Sentry; namespace Api.SeaHavenIndustries.HostedServices { @@ -62,32 +64,43 @@ namespace Api.SeaHavenIndustries.HostedServices private readonly PasswordResetEmailChannel _channel; private readonly IServiceScopeFactory _scopeFactory; private readonly ILogger _logger; + private readonly IHub _sentryHub; public PasswordResetEmailSenderHostedService( PasswordResetEmailChannel channel, IServiceScopeFactory scopeFactory, - ILogger logger) + ILogger logger, + IHub sentryHub) { _channel = channel; _scopeFactory = scopeFactory; _logger = logger; + _sentryHub = sentryHub; } protected override async Task ExecuteAsync(CancellationToken stoppingToken) { await foreach (var email in _channel.Reader.ReadAllAsync(stoppingToken)) { + using var transaction = SentryObservability.BeginBackgroundTransaction( + _sentryHub, + "auth.password-reset-email", + $"{nameof(PasswordResetEmailSenderHostedService)}.{nameof(SendAsync)}"); try { - await using var scope = _scopeFactory.CreateAsyncScope(); - var sender = scope.ServiceProvider.GetRequiredService(); - if (!await sender.SendEmailAsync(email.EmailTo, email.Subject, email.Body)) - _logger.LogWarning("Password reset email was not accepted by the mail provider."); + await SendAsync(email); + transaction.FinishOk(); + } + catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) + { + transaction.FinishCancelled(); + throw; } catch (Exception ex) { // The message can echo the recipient or the body, so only the type is logged. _logger.LogError("Password reset email failed with {ExceptionType}.", ex.GetType().FullName); + transaction.FinishError(ex); } finally { @@ -95,5 +108,13 @@ namespace Api.SeaHavenIndustries.HostedServices } } } + + private async Task SendAsync(PasswordResetEmail email) + { + await using var scope = _scopeFactory.CreateAsyncScope(); + var sender = scope.ServiceProvider.GetRequiredService(); + if (!await sender.SendEmailAsync(email.EmailTo, email.Subject, email.Body)) + _logger.LogWarning("Password reset email was not accepted by the mail provider."); + } } } diff --git a/SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs b/SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs index 00ad7aa..0502f0c 100644 --- a/SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs +++ b/SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs @@ -46,6 +46,17 @@ namespace SeaHaven.Services.Helpers } } + public void ReleaseCodeRequest(string email) + { + var now = _timeProvider.GetUtcNow(); + lock (_gate) + { + var account = AccountFor(email, now); + if (account.Requests.Count > 0) + account.Requests.RemoveAt(account.Requests.Count - 1); + } + } + public bool TryReserveCheck(string email) { var now = _timeProvider.GetUtcNow(); diff --git a/SeaHaven.Services/Implementation/AuthenticationService.cs b/SeaHaven.Services/Implementation/AuthenticationService.cs index 94e0833..32cc06c 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -162,7 +162,12 @@ namespace SeaHaven.Services.Implementation if (active) { var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; - _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); + if (!_resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body)) + { + // The code will never reach the user: drop it and do not count the request. + await _forgetPasswordDataService.RemoveByEmailAsync(user.Email!, cancellationToken); + _resetThrottle.ReleaseCodeRequest(requested); + } } } @@ -219,17 +224,22 @@ namespace SeaHaven.Services.Implementation // The per-account budget spans every code the account is sent, so asking for // new codes does not buy more guesses. A slot is reserved before comparing and - // given back only when the code matches. + // given back when the code matches or there is no live code to guess at. if (!_resetThrottle.TryReserveCheck(email)) return null; var pending = await _forgetPasswordDataService.GetByEmailAsync(email, cancellationToken); if (pending == null) + { + _resetThrottle.ReleaseCheck(email); return null; + } var nowUtc = _timeProvider.GetUtcNow().UtcDateTime; if (!await _forgetPasswordDataService.TryConsumeAttemptAsync(pending.Id, MaxCodeAttempts, nowUtc, cancellationToken)) { + // Expired or out of attempts: nothing was compared, so no guess is counted. + _resetThrottle.ReleaseCheck(email); await _forgetPasswordDataService.RemoveByEmailAsync(pending.Email, cancellationToken); return null; } diff --git a/SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs b/SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs index 8cdcc2e..5484994 100644 --- a/SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs +++ b/SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs @@ -12,13 +12,19 @@ namespace SeaHaven.Services.Interfaces /// bool TryAcceptCodeRequest(string email); + /// Gives back an accepted code request whose email could not be queued. + void ReleaseCodeRequest(string email); + /// /// Reserves one failed check for the email before a code is compared. Returns /// false once the account has used its failed checks for the window. /// bool TryReserveCheck(string email); - /// Gives back the reservation of a check whose code matched. + /// + /// Gives back a reservation that did not become a failed guess: the code matched, + /// or there was no live code to compare it with. + /// void ReleaseCheck(string email); } } diff --git a/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs b/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs index 2431cc4..9fd8388 100644 --- a/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs +++ b/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs @@ -92,6 +92,34 @@ public sealed class PasswordResetAbuseLimitsTests Assert.Equal(HttpStatusCode.OK, (await host.ResetAsync(Alice, host.Sent.LatestCodeFor(Alice), NewPassword, NextClient())).StatusCode); } + [Fact] + public async Task Checks_without_a_live_code_do_not_use_up_the_account_budget() + { + await using var host = await PasswordResetTestHost.StartAsync(); + await host.AddUserAsync(Alice, OldPassword); + + // No code issued yet. + for (var check = 0; check < 10; check++) + { + await host.VerifyAsync(Alice, "123456", NextClient()); + await host.ResetAsync(Alice, "123456", NewPassword, NextClient()); + } + + // A code that has expired. + await host.ForgetPasswordAsync(Alice, NextClient()); + var expired = host.Sent.LatestCodeFor(Alice); + host.Time.Advance(TimeSpan.FromMinutes(16)); + for (var check = 0; check < 5; check++) + await host.VerifyAsync(Alice, expired, NextClient()); + + await host.ForgetPasswordAsync(Alice, NextClient()); + var live = host.Sent.LatestCodeFor(Alice); + + Assert.Equal(HttpStatusCode.OK, (await host.VerifyAsync(Alice, live, NextClient())).StatusCode); + Assert.Equal(HttpStatusCode.OK, (await host.ResetAsync(Alice, live, NewPassword, NextClient())).StatusCode); + Assert.Equal(HttpStatusCode.OK, (await host.LoginAsync(Alice, NewPassword)).StatusCode); + } + [Fact] public void Concurrent_checks_can_never_exceed_the_account_budget() { diff --git a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs index a16774e..1199080 100644 --- a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs +++ b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs @@ -93,6 +93,7 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable manager.FeatureProviders.Add(new OnlyAuthenticationController()); }); builder.Services.AddPasswordResetRateLimiting(); + builder.Services.AddSingleton(Sentry.Extensibility.HubAdapter.Instance); builder.Services.AddPasswordResetEmailDelivery(); var app = builder.Build(); From 57af8a12068a4bc04e9294da556784c7cde261e3 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 13:38:11 -0300 Subject: [PATCH 3/8] fix(auth): store an unmatchable code when the reset email cannot be queued, keeping data calls identical --- .../AuthenticationServiceTests.cs | 44 +++++++++++++++++-- .../Implementation/AuthenticationService.cs | 24 +++++----- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs index 0ac9cde..2fd6382 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs @@ -157,17 +157,53 @@ public class AuthenticationServiceTests var userData = new Mock(); userData.Setup(u => u.GetByEmailNormalizedAsync(It.IsAny(), It.IsAny())).ReturnsAsync(user); var forget = new Mock(); + var stored = new List<(string Hash, string Salt)>(); + forget.Setup(f => f.ReplaceCodeAsync(user.Email!, user.Id, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((_, _, h, s, _, _, _) => stored.Add((h, s))) + .Returns(Task.CompletedTask); var email = new Mock(); - email.SetupSequence(e => e.TryEnqueue(user.Email!, It.IsAny(), It.IsAny())) - .Returns(false).Returns(false).Returns(false).Returns(true); + var bodies = new List(); + // The queue is full for the first three requests. + email.Setup(e => e.TryEnqueue(user.Email!, It.IsAny(), It.IsAny())) + .Returns((_, _, b) => + { + bodies.Add(b); + return bodies.Count > 3; + }); var service = NewService(userData, forget, email, out _, out _); for (var request = 0; request < 4; request++) await service.ForgetPasswordAsync("alice@example.com", CancellationToken.None); - // Three undelivered codes were removed, and they did not use up the hourly limit of three. - forget.Verify(f => f.RemoveByEmailAsync(user.Email!, It.IsAny()), Times.Exactly(3)); + // The three undelivered requests did not use up the hourly limit of three. email.Verify(e => e.TryEnqueue(user.Email!, It.IsAny(), It.IsAny()), Times.Exactly(4)); + stored.Should().HaveCount(4); + for (var request = 0; request < 3; request++) + { + var code = System.Text.RegularExpressions.Regex.Match(bodies[request], @"Your Password Reset Code is: (\d{6})").Groups[1].Value; + PasswordResetCodeSecrets.Matches(ResetKey, stored[request].Salt, code, stored[request].Hash).Should().BeFalse(); + } + var delivered = System.Text.RegularExpressions.Regex.Match(bodies[3], @"Your Password Reset Code is: (\d{6})").Groups[1].Value; + PasswordResetCodeSecrets.Matches(ResetKey, stored[3].Salt, delivered, stored[3].Hash).Should().BeTrue(); + forget.Verify(f => f.RemoveByEmailAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task ForgetPassword_EmailThatCannotBeQueued_MakesTheSameDataCallsAsAnUnknownEmail() + { + var userData = new Mock(); + userData.Setup(u => u.GetByEmailNormalizedAsync("alice@example.com", It.IsAny())).ReturnsAsync(IdentityTestHelpers.User()); + var registered = new Mock(); + var unregistered = new Mock(); + var email = new Mock(); + email.Setup(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny())).Returns(false); + + await NewService(userData, registered, email, out _, out _).ForgetPasswordAsync("alice@example.com", CancellationToken.None); + await NewService(userData, unregistered, email, out _, out _).ForgetPasswordAsync("nope@example.com", CancellationToken.None); + + registered.Invocations.Select(call => call.Method.Name) + .Should().Equal(unregistered.Invocations.Select(call => call.Method.Name)) + .And.Equal(nameof(IForgetPasswordDataService.ReplaceCodeAsync)); } [Fact] diff --git a/SeaHaven.Services/Implementation/AuthenticationService.cs b/SeaHaven.Services/Implementation/AuthenticationService.cs index 32cc06c..f07ff28 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -150,25 +150,25 @@ namespace SeaHaven.Services.Implementation var hash = PasswordResetCodeSecrets.Hash(ResetCodeKey, salt, code); var active = user != null && user.IsDeleted != true && !string.IsNullOrWhiteSpace(user.Email); + var queued = false; + if (active) + { + var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; + queued = _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); + + // A code that cannot be emailed is stored unmatchable and the request is not counted. + if (!queued) + _resetThrottle.ReleaseCodeRequest(requested); + } + await _forgetPasswordDataService.ReplaceCodeAsync( active ? user!.Email! : requested, active ? user!.Id : string.Empty, - active ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), + queued ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), salt, nowUtc.Add(ResetCodeLifetime), nowUtc, cancellationToken); - - if (active) - { - var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; - if (!_resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body)) - { - // The code will never reach the user: drop it and do not count the request. - await _forgetPasswordDataService.RemoveByEmailAsync(user.Email!, cancellationToken); - _resetThrottle.ReleaseCodeRequest(requested); - } - } } public async Task VerifyCodeAsync(string? email, string? code, CancellationToken cancellationToken) From fe5f27c8e1ac9aa11d47f9d1f2e83ebeb35d396d Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 14:08:30 -0300 Subject: [PATCH 4/8] test(auth): build the password-policy service with the reset queue, throttle and keyed hash --- Api.SeaHavenIndustries.Tests/PasswordPolicyTests.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/PasswordPolicyTests.cs b/Api.SeaHavenIndustries.Tests/PasswordPolicyTests.cs index 8183cf3..6e3a85e 100644 --- a/Api.SeaHavenIndustries.Tests/PasswordPolicyTests.cs +++ b/Api.SeaHavenIndustries.Tests/PasswordPolicyTests.cs @@ -133,7 +133,8 @@ public sealed class PasswordPolicyTests : IAsyncDisposable Microsoft.Extensions.Options.Options.Create(new JwtOptions { Secret = new string('x', 64) }), Mock.Of(), Mock.Of(), - Mock.Of(), + Mock.Of(), + new InMemoryPasswordResetThrottle(TimeProvider.System), TimeProvider.System); var result = await service.ChangePasswordAsync(user.Id, CurrentPassword, "Next2@x", CancellationToken.None); @@ -179,7 +180,7 @@ public sealed class PasswordPolicyTests : IAsyncDisposable Email = user.Email!, UserId = user.Id, CodeSalt = salt, - CodeHash = PasswordResetCodeSecrets.Hash(salt, "123456"), + CodeHash = PasswordResetCodeSecrets.Hash(PasswordResetCodeSecrets.DeriveKey(new string('x', 64)), salt, "123456"), ExpiresAtUtc = DateTime.UtcNow.AddMinutes(10) }); forget.Setup(f => f.TryConsumeAttemptAsync(7, It.IsAny(), It.IsAny(), It.IsAny())) @@ -220,7 +221,8 @@ public sealed class PasswordPolicyTests : IAsyncDisposable Microsoft.Extensions.Options.Options.Create(new JwtOptions { Secret = new string('x', 64) }), Mock.Of(), (forget ?? new Mock()).Object, - Mock.Of(), + Mock.Of(), + new InMemoryPasswordResetThrottle(TimeProvider.System), TimeProvider.System); public async ValueTask DisposeAsync() From f0dcba63fde8637d67cae33ec4efff342fdec111 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 17:38:51 -0300 Subject: [PATCH 5/8] fix(auth): give back reset check and request slots when a data call fails or is cancelled --- .../AuthenticationServiceTests.cs | 78 +++++++++++++++++ .../Implementation/AuthenticationService.cs | 87 +++++++++++-------- 2 files changed, 128 insertions(+), 37 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs index 4f6a138..e64014f 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs @@ -271,6 +271,84 @@ public class AuthenticationServiceTests store.Verify(s => s.FindByIdAsync(It.IsAny(), It.IsAny()), Times.Never); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task VerifyCode_FailedOrCancelledLookups_LeaveTheAccountCheckBudgetUntouched(bool cancelled) + { + var pending = new ForgetPasswordCode { Id = 7, Email = "a@b.com", UserId = "u1", CodeSalt = "s", CodeHash = PasswordResetCodeSecrets.Hash(ResetKey, "s", "123456") }; + var forget = new Mock(); + var lookups = 0; + forget.Setup(f => f.GetByEmailAsync("a@b.com", It.IsAny())) + .Returns(() => ++lookups <= InMemoryPasswordResetThrottle.FailedChecksPerDay + ? Task.FromException(cancelled ? new OperationCanceledException() : new InvalidOperationException("database unavailable")) + : Task.FromResult(pending)); + forget.Setup(f => f.TryConsumeAttemptAsync(7, AuthenticationService.MaxCodeAttempts, It.IsAny(), It.IsAny())).ReturnsAsync(true); + var service = NewService(new Mock(), forget, new Mock(), out _, out _); + + for (var check = 0; check < InMemoryPasswordResetThrottle.FailedChecksPerDay; check++) + { + var act = () => service.VerifyCodeAsync("a@b.com", "123456", CancellationToken.None); + await act.Should().ThrowAsync(); + } + + (await service.VerifyCodeAsync("a@b.com", "123456", CancellationToken.None)).Should().BeTrue(); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task VerifyCode_FailedOrCancelledAttemptConsumes_LeaveTheAccountCheckBudgetUntouched(bool cancelled) + { + var pending = new ForgetPasswordCode { Id = 7, Email = "a@b.com", UserId = "u1", CodeSalt = "s", CodeHash = PasswordResetCodeSecrets.Hash(ResetKey, "s", "123456") }; + var forget = new Mock(); + forget.Setup(f => f.GetByEmailAsync("a@b.com", It.IsAny())).ReturnsAsync(pending); + var consumes = 0; + forget.Setup(f => f.TryConsumeAttemptAsync(7, AuthenticationService.MaxCodeAttempts, It.IsAny(), It.IsAny())) + .Returns(() => ++consumes <= InMemoryPasswordResetThrottle.FailedChecksPerDay + ? Task.FromException(cancelled ? new OperationCanceledException() : new InvalidOperationException("database unavailable")) + : Task.FromResult(true)); + var service = NewService(new Mock(), forget, new Mock(), out _, out _); + + for (var check = 0; check < InMemoryPasswordResetThrottle.FailedChecksPerDay; check++) + { + var act = () => service.VerifyCodeAsync("a@b.com", "123456", CancellationToken.None); + await act.Should().ThrowAsync(); + } + + (await service.VerifyCodeAsync("a@b.com", "123456", CancellationToken.None)).Should().BeTrue(); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ForgetPassword_FailedOrCancelledWrites_DoNotUseUpTheEmailRequestLimit(bool cancelled) + { + var user = IdentityTestHelpers.User(); + var userData = new Mock(); + userData.Setup(u => u.GetByEmailNormalizedAsync(It.IsAny(), It.IsAny())).ReturnsAsync(user); + var forget = new Mock(); + var writes = 0; + forget.Setup(f => f.ReplaceCodeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(() => ++writes <= InMemoryPasswordResetThrottle.CodeRequestsPerHour + ? Task.FromException(cancelled ? new OperationCanceledException() : new InvalidOperationException("database unavailable")) + : Task.CompletedTask); + var email = new Mock(); + email.Setup(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny())).Returns(true); + var service = NewService(userData, forget, email, out _, out _); + + for (var request = 0; request < InMemoryPasswordResetThrottle.CodeRequestsPerHour; request++) + { + var act = () => service.ForgetPasswordAsync("alice@example.com", CancellationToken.None); + await act.Should().ThrowAsync(); + } + + await service.ForgetPasswordAsync("alice@example.com", CancellationToken.None); + + writes.Should().Be(InMemoryPasswordResetThrottle.CodeRequestsPerHour + 1); + forget.Verify(f => f.PurgeExpiredAsync(It.IsAny(), It.IsAny()), Times.Never); + } + [Theory] [InlineData("123456", "123456", true)] [InlineData("123456", " 123456 ", true)] diff --git a/SeaHaven.Services/Implementation/AuthenticationService.cs b/SeaHaven.Services/Implementation/AuthenticationService.cs index c00c3c2..73b43d2 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -164,25 +164,34 @@ namespace SeaHaven.Services.Implementation var hash = PasswordResetCodeSecrets.Hash(ResetCodeKey, salt, code); var active = user != null && user.IsDeleted != true && !string.IsNullOrWhiteSpace(user.Email); - var queued = false; - if (active) + // The request stays counted only once its row is stored. A code that cannot be + // emailed is stored unmatchable and not counted, and a failed or cancelled + // write is not counted either. + var counted = false; + try { - var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; - queued = _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); + var queued = false; + if (active) + { + var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; + queued = _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); + } - // A code that cannot be emailed is stored unmatchable and the request is not counted. - if (!queued) + await _forgetPasswordDataService.ReplaceCodeAsync( + active ? user!.Email! : requested, + active ? user!.Id : string.Empty, + queued ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), + salt, + nowUtc.Add(ResetCodeLifetime), + nowUtc, + cancellationToken); + counted = queued || !active; + } + finally + { + if (!counted) _resetThrottle.ReleaseCodeRequest(requested); } - - await _forgetPasswordDataService.ReplaceCodeAsync( - active ? user!.Email! : requested, - active ? user!.Id : string.Empty, - queued ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), - salt, - nowUtc.Add(ResetCodeLifetime), - nowUtc, - cancellationToken); } public async Task VerifyCodeAsync(string? email, string? code, CancellationToken cancellationToken) @@ -242,33 +251,37 @@ namespace SeaHaven.Services.Implementation if (!_resetThrottle.TryReserveCheck(email)) return null; - var pending = await _forgetPasswordDataService.GetByEmailAsync(email, cancellationToken); - if (pending == null) + // Only a wrong code actually compared keeps the slot. No live code, an expired or + // used-up code, a match, and a failed or cancelled call all give it back. + var wrongGuess = false; + try { - _resetThrottle.ReleaseCheck(email); + var pending = await _forgetPasswordDataService.GetByEmailAsync(email, cancellationToken); + if (pending == null) + return null; + + var nowUtc = _timeProvider.GetUtcNow().UtcDateTime; + // Deletes below stop at this code's id: a code issued by a concurrent request + // after this one was read belongs to that request and must survive. + if (!await _forgetPasswordDataService.TryConsumeAttemptAsync(pending.Id, MaxCodeAttempts, nowUtc, cancellationToken)) + { + await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); + return null; + } + + if (PasswordResetCodeSecrets.Matches(ResetCodeKey, pending.CodeSalt, code, pending.CodeHash)) + return pending; + + wrongGuess = true; + if (pending.FailedAttempts + 1 >= MaxCodeAttempts) + await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); return null; } - - var nowUtc = _timeProvider.GetUtcNow().UtcDateTime; - // Deletes below stop at this code's id: a code issued by a concurrent request - // after this one was read belongs to that request and must survive. - if (!await _forgetPasswordDataService.TryConsumeAttemptAsync(pending.Id, MaxCodeAttempts, nowUtc, cancellationToken)) + finally { - // Expired or out of attempts: nothing was compared, so no guess is counted. - _resetThrottle.ReleaseCheck(email); - await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); - return null; + if (!wrongGuess) + _resetThrottle.ReleaseCheck(email); } - - if (PasswordResetCodeSecrets.Matches(ResetCodeKey, pending.CodeSalt, code, pending.CodeHash)) - { - _resetThrottle.ReleaseCheck(email); - return pending; - } - - if (pending.FailedAttempts + 1 >= MaxCodeAttempts) - await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); - return null; } private JwtSecurityToken GetToken(List authClaims) From 5eb13234196a482c6381dfacb7327c8e9aaaa909 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 17:51:26 -0300 Subject: [PATCH 6/8] test(auth): give the reset test host's helpers names of their own --- .../PasswordResetAbuseLimitsTests.cs | 4 ++-- .../PasswordResetTestHost.cs | 22 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs b/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs index 9fd8388..9835a85 100644 --- a/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs +++ b/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs @@ -123,7 +123,7 @@ public sealed class PasswordResetAbuseLimitsTests [Fact] public void Concurrent_checks_can_never_exceed_the_account_budget() { - var throttle = new InMemoryPasswordResetThrottle(new ManualTimeProvider()); + var throttle = new InMemoryPasswordResetThrottle(new ResetTestClock()); var reserved = 0; Parallel.For(0, 64, _ => @@ -138,7 +138,7 @@ public sealed class PasswordResetAbuseLimitsTests [Fact] public void A_matching_check_gives_its_reservation_back() { - var throttle = new InMemoryPasswordResetThrottle(new ManualTimeProvider()); + var throttle = new InMemoryPasswordResetThrottle(new ResetTestClock()); for (var check = 0; check < 50; check++) { diff --git a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs index 58ea020..e8e6812 100644 --- a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs +++ b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs @@ -46,9 +46,9 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable } public HttpClient Client { get; } - public CapturingEmailSender Sent { get; private init; } = null!; - public ManualTimeProvider Time { get; private init; } = null!; - public CapturingLoggerProvider Logged { get; private init; } = null!; + public ResetEmailOutbox Sent { get; private init; } = null!; + public ResetTestClock Time { get; private init; } = null!; + public ResetLogCapture Logged { get; private init; } = null!; public IServiceProvider Services => _app.Services; public static async Task StartAsync(IEmailSender? emailSender = null, Action? configureServices = null) @@ -56,9 +56,9 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable var databasePath = Path.Combine(Path.GetTempPath(), $"password-reset-{Guid.NewGuid():N}.db"); var connectionString = new SqliteConnectionStringBuilder { DataSource = databasePath, DefaultTimeout = 30 }.ToString(); - var sent = new CapturingEmailSender(); - var time = new ManualTimeProvider(); - var logged = new CapturingLoggerProvider(); + var sent = new ResetEmailOutbox(); + var time = new ResetTestClock(); + var logged = new ResetLogCapture(); var builder = WebApplication.CreateBuilder(new WebApplicationOptions { EnvironmentName = "Testing" }); builder.WebHost.UseUrls("http://127.0.0.1:0"); @@ -258,7 +258,7 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable } } -internal sealed class ManualTimeProvider : TimeProvider +internal sealed class ResetTestClock : TimeProvider { private DateTimeOffset _now = new(2026, 9, 25, 12, 0, 0, TimeSpan.Zero); @@ -267,7 +267,7 @@ internal sealed class ManualTimeProvider : TimeProvider public void Advance(TimeSpan by) => _now = _now.Add(by); } -internal sealed class CapturingEmailSender : IEmailSender +internal sealed class ResetEmailOutbox : IEmailSender { private static readonly Regex CodePattern = new(@"Your Password Reset Code is: (\d{6})", RegexOptions.CultureInvariant); @@ -286,7 +286,7 @@ internal sealed class CapturingEmailSender : IEmailSender } } -internal sealed class CapturingLoggerProvider : ILoggerProvider +internal sealed class ResetLogCapture : ILoggerProvider { public ConcurrentQueue Lines { get; } = new(); @@ -298,10 +298,10 @@ internal sealed class CapturingLoggerProvider : ILoggerProvider private sealed class CapturingLogger : ILogger { - private readonly CapturingLoggerProvider _owner; + private readonly ResetLogCapture _owner; private readonly string _category; - public CapturingLogger(CapturingLoggerProvider owner, string category) + public CapturingLogger(ResetLogCapture owner, string category) { _owner = owner; _category = category; From 2f23f6fadca25540e65c5995768c324962b0f2b6 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 18:10:10 -0300 Subject: [PATCH 7/8] test(team-members): register reset email delivery in the invite test host --- SeaHavenIndustries.Tests/TeamMemberInviteTestHost.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/SeaHavenIndustries.Tests/TeamMemberInviteTestHost.cs b/SeaHavenIndustries.Tests/TeamMemberInviteTestHost.cs index a2e5a1a..82ec3e7 100644 --- a/SeaHavenIndustries.Tests/TeamMemberInviteTestHost.cs +++ b/SeaHavenIndustries.Tests/TeamMemberInviteTestHost.cs @@ -1,6 +1,7 @@ using System.Collections.Concurrent; using System.Security.Claims; using System.Text.RegularExpressions; +using Api.SeaHavenIndustries.HostedServices; using Api.SeaHavenIndustries.Infrastructure; using Data.SeaHavenIndustries; using Microsoft.Data.Sqlite; @@ -79,6 +80,9 @@ internal sealed class TeamMemberInviteTestHost : IAsyncDisposable services.AddSingleton(fakes.Sent); services.AddDataServices(); services.AddBusinessServices(configuration); + // Registration signs the member in through the authentication service, which + // queues password reset email; the API host registers the same delivery. + services.AddPasswordResetEmailDelivery(); configure?.Invoke(services); return services.BuildServiceProvider(); } From ca9322fa605ad3772be506a9c9a6bfdb37a72d3b Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 18:29:33 -0300 Subject: [PATCH 8/8] fix(auth): queue the reset email only after its code is stored --- .../AuthenticationServiceTests.cs | 31 +++++++++++++++---- .../Implementation/AuthenticationService.cs | 24 +++++++------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs index e64014f..045acea 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs @@ -151,7 +151,7 @@ public class AuthenticationServiceTests } [Fact] - public async Task ForgetPassword_EmailThatCannotBeQueued_DropsTheCodeAndDoesNotCountTheRequest() + public async Task ForgetPassword_EmailThatCannotBeQueued_DoesNotCountTheRequest() { var user = IdentityTestHelpers.User(); var userData = new Mock(); @@ -175,19 +175,38 @@ public class AuthenticationServiceTests for (var request = 0; request < 4; request++) await service.ForgetPasswordAsync("alice@example.com", CancellationToken.None); - // The three undelivered requests did not use up the hourly limit of three. + // The three undelivered requests did not use up the hourly limit of three, and every + // email carries the code stored just before it was queued. email.Verify(e => e.TryEnqueue(user.Email!, It.IsAny(), It.IsAny()), Times.Exactly(4)); stored.Should().HaveCount(4); - for (var request = 0; request < 3; request++) + for (var request = 0; request < 4; request++) { var code = System.Text.RegularExpressions.Regex.Match(bodies[request], @"Your Password Reset Code is: (\d{6})").Groups[1].Value; - PasswordResetCodeSecrets.Matches(ResetKey, stored[request].Salt, code, stored[request].Hash).Should().BeFalse(); + PasswordResetCodeSecrets.Matches(ResetKey, stored[request].Salt, code, stored[request].Hash).Should().BeTrue(); } - var delivered = System.Text.RegularExpressions.Regex.Match(bodies[3], @"Your Password Reset Code is: (\d{6})").Groups[1].Value; - PasswordResetCodeSecrets.Matches(ResetKey, stored[3].Salt, delivered, stored[3].Hash).Should().BeTrue(); forget.Verify(f => f.RemoveByEmailAsync(It.IsAny(), It.IsAny()), Times.Never); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ForgetPassword_WriteThatFailsOrIsCancelled_QueuesNoEmail(bool cancelled) + { + var user = IdentityTestHelpers.User(); + var userData = new Mock(); + userData.Setup(u => u.GetByEmailNormalizedAsync(It.IsAny(), It.IsAny())).ReturnsAsync(user); + var forget = new Mock(); + forget.Setup(f => f.ReplaceCodeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.FromException(cancelled ? new OperationCanceledException() : new InvalidOperationException("database unavailable"))); + var email = new Mock(); + email.Setup(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny())).Returns(true); + + var act = () => NewService(userData, forget, email, out _, out _).ForgetPasswordAsync("alice@example.com", CancellationToken.None); + + await act.Should().ThrowAsync(); + email.Verify(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + [Fact] public async Task ForgetPassword_EmailThatCannotBeQueued_MakesTheSameDataCallsAsAnUnknownEmail() { diff --git a/SeaHaven.Services/Implementation/AuthenticationService.cs b/SeaHaven.Services/Implementation/AuthenticationService.cs index fbc7a2e..1b982d0 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -170,12 +170,22 @@ namespace SeaHaven.Services.Implementation var hash = PasswordResetCodeSecrets.Hash(ResetCodeKey, salt, code); var active = user != null && user.IsDeleted != true && !string.IsNullOrWhiteSpace(user.Email); - // The request stays counted only once its row is stored. A code that cannot be - // emailed is stored unmatchable and not counted, and a failed or cancelled - // write is not counted either. + // The email is queued only after its code is stored, so a failed or cancelled + // write never sends a code that cannot be used. The request stays counted only + // when the row is stored and, for an account, its email was queued; when the + // queue is full the stored code is left unsent and the user can ask again. var counted = false; try { + await _forgetPasswordDataService.ReplaceCodeAsync( + active ? user!.Email! : requested, + active ? user!.Id : string.Empty, + active ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), + salt, + nowUtc.Add(ResetCodeLifetime), + nowUtc, + cancellationToken); + var queued = false; if (active) { @@ -183,14 +193,6 @@ namespace SeaHaven.Services.Implementation queued = _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); } - await _forgetPasswordDataService.ReplaceCodeAsync( - active ? user!.Email! : requested, - active ? user!.Id : string.Empty, - queued ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), - salt, - nowUtc.Add(ResetCodeLifetime), - nowUtc, - cancellationToken); counted = queued || !active; } finally