From 77a10e38caa6b292fe5e0e9fccb3816eb4012745 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 13:00:03 -0300 Subject: [PATCH] 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));