diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationControllerTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationControllerTests.cs index 544aa1d..ac2d358 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationControllerTests.cs @@ -237,8 +237,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; @@ -259,7 +259,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); @@ -283,7 +283,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) { @@ -302,16 +302,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] @@ -322,7 +322,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 37e1b01..9da31d9 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs @@ -19,7 +19,7 @@ public class AuthenticationServiceTests private static AuthenticationService NewService( Mock userData, Mock forget, - Mock email, + Mock email, out Mock> store, out Mock> hasher) { @@ -28,7 +28,7 @@ public class AuthenticationServiceTests hasher = h; var jwtOptions = Microsoft.Extensions.Options.Options.Create(JwtOptions); var sessionStamps = new SessionStampService(userData.Object, new InMemorySessionStampCache(TimeProvider.System), jwtOptions); - return new AuthenticationService(manager, jwtOptions, userData.Object, forget.Object, email.Object, TimeProvider.System, sessionStamps); + return new AuthenticationService(manager, jwtOptions, userData.Object, forget.Object, email.Object, new InMemoryPasswordResetThrottle(TimeProvider.System), TimeProvider.System, sessionStamps); } private static JwtOptions JwtOptions => new() @@ -41,7 +41,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); @@ -53,7 +53,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()); @@ -67,7 +67,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>() @@ -90,7 +90,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"); @@ -101,22 +101,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; @@ -126,25 +128,103 @@ 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); + 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(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); + + 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] + public async Task ForgetPassword_EmailThatCannotBeQueued_DoesNotCountTheRequest() + { + 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(); + 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(); + 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); - await service.ForgetPasswordAsync("nope@example.com", CancellationToken.None); + // 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 < 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().BeTrue(); + } + forget.Verify(f => f.RemoveByEmailAsync(It.IsAny(), It.IsAny()), Times.Never); + } - 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); + [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() + { + 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] @@ -154,13 +234,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] @@ -173,7 +253,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); @@ -186,7 +266,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); @@ -198,12 +278,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); @@ -212,6 +292,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)] @@ -219,11 +377,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.Tests/PasswordPolicyTests.cs b/Api.SeaHavenIndustries.Tests/PasswordPolicyTests.cs index b028f78..117e3a7 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, Mock.Of()); @@ -180,7 +181,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())) @@ -221,7 +222,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, Mock.Of()); diff --git a/Api.SeaHavenIndustries.Tests/PasswordResetEmailChannelTests.cs b/Api.SeaHavenIndustries.Tests/PasswordResetEmailChannelTests.cs new file mode 100644 index 0000000..5366173 --- /dev/null +++ b/Api.SeaHavenIndustries.Tests/PasswordResetEmailChannelTests.cs @@ -0,0 +1,43 @@ +using Api.SeaHavenIndustries.HostedServices; +using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace Api.SeaHavenIndustries.Tests; + +public class PasswordResetEmailChannelTests +{ + [Fact] + public void A_full_queue_refuses_the_email_instead_of_dropping_it_silently() + { + var channel = new PasswordResetEmailChannel(NullLogger.Instance); + for (var i = 0; i < PasswordResetEmailChannel.Capacity; i++) + channel.TryEnqueue($"user{i}@example.com", "subject", "body").Should().BeTrue(); + + var accepted = channel.TryEnqueue("late@example.com", "subject", "body"); + + accepted.Should().BeFalse(); + channel.Pending.Should().Be(PasswordResetEmailChannel.Capacity); + } + + [Fact] + public async Task A_refused_email_is_never_delivered_and_the_queue_accepts_again_once_drained() + { + var channel = new PasswordResetEmailChannel(NullLogger.Instance); + for (var i = 0; i < PasswordResetEmailChannel.Capacity; i++) + channel.TryEnqueue($"user{i}@example.com", "subject", "body"); + channel.TryEnqueue("late@example.com", "subject", "body").Should().BeFalse(); + + var delivered = new List(); + while (channel.Reader.TryRead(out var email)) + { + delivered.Add(email.EmailTo); + channel.MarkHandled(); + } + + delivered.Should().HaveCount(PasswordResetEmailChannel.Capacity).And.NotContain("late@example.com"); + channel.Pending.Should().Be(0); + channel.TryEnqueue("retry@example.com", "subject", "body").Should().BeTrue(); + (await channel.Reader.ReadAsync()).EmailTo.Should().Be("retry@example.com"); + } +} diff --git a/Api.SeaHavenIndustries.Tests/PasswordResetEmailSenderTracingTests.cs b/Api.SeaHavenIndustries.Tests/PasswordResetEmailSenderTracingTests.cs new file mode 100644 index 0000000..93c912a --- /dev/null +++ b/Api.SeaHavenIndustries.Tests/PasswordResetEmailSenderTracingTests.cs @@ -0,0 +1,55 @@ +using Api.SeaHavenIndustries.HostedServices; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Sentry; +using SeaHaven.Services.Interfaces; +using Xunit; + +namespace Api.SeaHavenIndustries.Tests; + +public class PasswordResetEmailSenderTracingTests +{ + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Each_send_finishes_its_transaction_as_ok_only_when_the_provider_accepts_it(bool accepted) + { + var transaction = new Mock(); + var hub = new Mock(); + hub.Setup(h => h.PushScope()).Returns(Mock.Of()); + hub.Setup(h => h.StartTransaction(It.IsAny(), It.IsAny>())) + .Returns(transaction.Object); + + var sender = new Mock(); + sender.Setup(s => s.SendEmailAsync(It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(accepted); + var services = new ServiceCollection().AddSingleton(sender.Object).BuildServiceProvider(); + var channel = new PasswordResetEmailChannel(NullLogger.Instance); + var worker = new PasswordResetEmailSenderHostedService( + channel, + services.GetRequiredService(), + NullLogger.Instance, + hub.Object); + + await worker.StartAsync(CancellationToken.None); + Assert.True(channel.TryEnqueue("user@example.com", "subject", "Your code is 123456")); + var deadline = DateTime.UtcNow.AddSeconds(10); + while (channel.Pending > 0 && DateTime.UtcNow < deadline) + await Task.Delay(10); + await worker.StopAsync(CancellationToken.None); + + Assert.Equal(0, channel.Pending); + if (accepted) + { + transaction.Verify(t => t.Finish(SpanStatus.Ok), Times.Once); + transaction.Verify(t => t.Finish(It.IsAny(), It.IsAny()), Times.Never); + } + else + { + transaction.Verify(t => t.Finish(SpanStatus.Ok), Times.Never); + transaction.Verify(t => t.Finish( + It.Is(ex => !ex.Message.Contains("user@example.com") && !ex.Message.Contains("123456")), + SpanStatus.InternalError), Times.Once); + } + } +} 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/Controllers/AuthenticationController.cs b/Api.SeaHavenIndustries/Controllers/AuthenticationController.cs index 2d711fd..c7751b0 100644 --- a/Api.SeaHavenIndustries/Controllers/AuthenticationController.cs +++ b/Api.SeaHavenIndustries/Controllers/AuthenticationController.cs @@ -115,20 +115,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) { @@ -146,13 +145,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..172f678 --- /dev/null +++ b/Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs @@ -0,0 +1,129 @@ +using System.Threading.Channels; +using Api.SeaHavenIndustries.Observability; +using SeaHaven.Services.Interfaces; +using Sentry; + +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) + { + // Wait, not DropWrite: with DropWrite, TryWrite reports success and discards + // the email, so a full queue would still count the request. TryWrite never + // blocks; under Wait it returns false when the queue is full. + FullMode = BoundedChannelFullMode.Wait, + 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; + private readonly IHub _sentryHub; + + public PasswordResetEmailSenderHostedService( + PasswordResetEmailChannel channel, + IServiceScopeFactory scopeFactory, + 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 + { + if (await SendAsync(email)) + transaction.FinishOk(); + else + transaction.FinishError(new InvalidOperationException("The mail provider did not accept the password reset email.")); + } + 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 + { + _channel.MarkHandled(); + } + } + } + + /// True when the mail provider accepted the email. + 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)) + return true; + + _logger.LogWarning("Password reset email was not accepted by the mail provider."); + return false; + } + } +} diff --git a/Api.SeaHavenIndustries/Program.cs b/Api.SeaHavenIndustries/Program.cs index f82c666..d7c3d30 100644 --- a/Api.SeaHavenIndustries/Program.cs +++ b/Api.SeaHavenIndustries/Program.cs @@ -57,6 +57,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 fe3bd1d..4d625c6 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 1c7f0ad..f1035f2 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 1ef9e77..1fc488a 100644 --- a/SeaHaven.Services/DependencyInjection/ServicesModule.cs +++ b/SeaHaven.Services/DependencyInjection/ServicesModule.cs @@ -77,6 +77,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..0502f0c --- /dev/null +++ b/SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs @@ -0,0 +1,130 @@ +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 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(); + 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 db30bca..60448cc 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -19,19 +19,25 @@ 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 readonly ISessionStampService _sessionStamps; + 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, ISessionStampService sessionStamps) { @@ -39,11 +45,14 @@ namespace SeaHaven.Services.Implementation _jwtOptions = jwtOptions.Value; _userDataService = userDataService; _forgetPasswordDataService = forgetPasswordDataService; - _emailSender = emailSender; + _resetEmails = resetEmails; + _resetThrottle = resetThrottle; _timeProvider = timeProvider; _sessionStamps = sessionStamps; } + private byte[] ResetCodeKey => _resetCodeKey ??= PasswordResetCodeSecrets.DeriveKey(_jwtOptions.Secret); + public async Task LoginAsync(string? username, string? password, CancellationToken cancellationToken) { var user = await _userManager.FindByNameAsync(username ?? ""); @@ -156,26 +165,58 @@ 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); + + // 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) + { + var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; + queued = _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); + } + + counted = queued || !active; + } + finally + { + if (!counted) + _resetThrottle.ReleaseCodeRequest(requested); + } } public async Task VerifyCodeAsync(string? email, string? code, CancellationToken cancellationToken) @@ -231,25 +272,43 @@ namespace SeaHaven.Services.Implementation if (string.IsNullOrWhiteSpace(email) || string.IsNullOrWhiteSpace(code)) return null; - var pending = await _forgetPasswordDataService.GetByEmailAsync(email, cancellationToken); - if (pending == 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 when the code matches or there is no live code to guess at. + if (!_resetThrottle.TryReserveCheck(email)) 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)) + // 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 { - await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); + 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; } - - if (PasswordResetCodeSecrets.Matches(pending.CodeSalt, code, pending.CodeHash)) - return pending; - - if (pending.FailedAttempts + 1 >= MaxCodeAttempts) - await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); - return null; + finally + { + if (!wrongGuess) + _resetThrottle.ReleaseCheck(email); + } } private JwtSecurityToken GetToken(List authClaims) 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..5484994 --- /dev/null +++ b/SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs @@ -0,0 +1,30 @@ +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); + + /// 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 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 new file mode 100644 index 0000000..9835a85 --- /dev/null +++ b/SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs @@ -0,0 +1,269 @@ +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 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() + { + var throttle = new InMemoryPasswordResetThrottle(new ResetTestClock()); + + 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 ResetTestClock()); + + 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/PasswordResetRaceTests.cs b/SeaHavenIndustries.Tests/PasswordResetRaceTests.cs index 20a89f2..1ae3937 100644 --- a/SeaHavenIndustries.Tests/PasswordResetRaceTests.cs +++ b/SeaHavenIndustries.Tests/PasswordResetRaceTests.cs @@ -92,8 +92,11 @@ public sealed class PasswordResetRaceTests _race = race; } - public Task ReplaceCodeAsync(string email, string userId, string codeHash, string codeSalt, DateTime expiresAtUtc, CancellationToken cancellationToken) => - _inner.ReplaceCodeAsync(email, userId, codeHash, codeSalt, expiresAtUtc, cancellationToken); + public Task ReplaceCodeAsync(string email, string userId, string codeHash, string codeSalt, DateTime expiresAtUtc, DateTime nowUtc, CancellationToken cancellationToken) => + _inner.ReplaceCodeAsync(email, userId, codeHash, codeSalt, expiresAtUtc, nowUtc, cancellationToken); + + public Task PurgeExpiredAsync(DateTime nowUtc, CancellationToken cancellationToken) => + _inner.PurgeExpiredAsync(nowUtc, cancellationToken); public Task GetByEmailAsync(string email, CancellationToken cancellationToken) => _inner.GetByEmailAsync(email, cancellationToken); diff --git a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs index 323453f..e8e6812 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" }); @@ -91,6 +94,8 @@ 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(); app.UseForwardedHeaders(); @@ -142,7 +147,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"; @@ -153,7 +158,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(); @@ -170,8 +177,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)); diff --git a/SeaHavenIndustries.Tests/SessionTestHost.cs b/SeaHavenIndustries.Tests/SessionTestHost.cs index 0dca587..a78bef3 100644 --- a/SeaHavenIndustries.Tests/SessionTestHost.cs +++ b/SeaHavenIndustries.Tests/SessionTestHost.cs @@ -5,6 +5,7 @@ using System.Text; using System.Text.Json; using System.Text.RegularExpressions; using Api.SeaHavenIndustries.Controllers; +using Api.SeaHavenIndustries.HostedServices; using Api.SeaHavenIndustries.Infrastructure; using Data.SeaHavenIndustries; using Microsoft.AspNetCore.Builder; @@ -83,6 +84,9 @@ internal sealed class SessionTestHost : IAsyncDisposable builder.Services.AddSingleton(sent); builder.Services.AddDataServices(); builder.Services.AddBusinessServices(builder.Configuration); + // Forgot Password queues its email; the API host registers the same delivery. + builder.Services.AddSingleton(Sentry.Extensibility.HubAdapter.Instance); + builder.Services.AddPasswordResetEmailDelivery(); builder.Services.AddSeaHavenJwtAuthentication(builder.Configuration); builder.Services.AddControllers() .AddApplicationPart(typeof(AuthenticationController).Assembly) 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(); }