diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs index 4f6a138..e64014f 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs @@ -271,6 +271,84 @@ public class AuthenticationServiceTests store.Verify(s => s.FindByIdAsync(It.IsAny(), It.IsAny()), Times.Never); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task VerifyCode_FailedOrCancelledLookups_LeaveTheAccountCheckBudgetUntouched(bool cancelled) + { + var pending = new ForgetPasswordCode { Id = 7, Email = "a@b.com", UserId = "u1", CodeSalt = "s", CodeHash = PasswordResetCodeSecrets.Hash(ResetKey, "s", "123456") }; + var forget = new Mock(); + var lookups = 0; + forget.Setup(f => f.GetByEmailAsync("a@b.com", It.IsAny())) + .Returns(() => ++lookups <= InMemoryPasswordResetThrottle.FailedChecksPerDay + ? Task.FromException(cancelled ? new OperationCanceledException() : new InvalidOperationException("database unavailable")) + : Task.FromResult(pending)); + forget.Setup(f => f.TryConsumeAttemptAsync(7, AuthenticationService.MaxCodeAttempts, It.IsAny(), It.IsAny())).ReturnsAsync(true); + var service = NewService(new Mock(), forget, new Mock(), out _, out _); + + for (var check = 0; check < InMemoryPasswordResetThrottle.FailedChecksPerDay; check++) + { + var act = () => service.VerifyCodeAsync("a@b.com", "123456", CancellationToken.None); + await act.Should().ThrowAsync(); + } + + (await service.VerifyCodeAsync("a@b.com", "123456", CancellationToken.None)).Should().BeTrue(); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task VerifyCode_FailedOrCancelledAttemptConsumes_LeaveTheAccountCheckBudgetUntouched(bool cancelled) + { + var pending = new ForgetPasswordCode { Id = 7, Email = "a@b.com", UserId = "u1", CodeSalt = "s", CodeHash = PasswordResetCodeSecrets.Hash(ResetKey, "s", "123456") }; + var forget = new Mock(); + forget.Setup(f => f.GetByEmailAsync("a@b.com", It.IsAny())).ReturnsAsync(pending); + var consumes = 0; + forget.Setup(f => f.TryConsumeAttemptAsync(7, AuthenticationService.MaxCodeAttempts, It.IsAny(), It.IsAny())) + .Returns(() => ++consumes <= InMemoryPasswordResetThrottle.FailedChecksPerDay + ? Task.FromException(cancelled ? new OperationCanceledException() : new InvalidOperationException("database unavailable")) + : Task.FromResult(true)); + var service = NewService(new Mock(), forget, new Mock(), out _, out _); + + for (var check = 0; check < InMemoryPasswordResetThrottle.FailedChecksPerDay; check++) + { + var act = () => service.VerifyCodeAsync("a@b.com", "123456", CancellationToken.None); + await act.Should().ThrowAsync(); + } + + (await service.VerifyCodeAsync("a@b.com", "123456", CancellationToken.None)).Should().BeTrue(); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ForgetPassword_FailedOrCancelledWrites_DoNotUseUpTheEmailRequestLimit(bool cancelled) + { + var user = IdentityTestHelpers.User(); + var userData = new Mock(); + userData.Setup(u => u.GetByEmailNormalizedAsync(It.IsAny(), It.IsAny())).ReturnsAsync(user); + var forget = new Mock(); + var writes = 0; + forget.Setup(f => f.ReplaceCodeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(() => ++writes <= InMemoryPasswordResetThrottle.CodeRequestsPerHour + ? Task.FromException(cancelled ? new OperationCanceledException() : new InvalidOperationException("database unavailable")) + : Task.CompletedTask); + var email = new Mock(); + email.Setup(e => e.TryEnqueue(It.IsAny(), It.IsAny(), It.IsAny())).Returns(true); + var service = NewService(userData, forget, email, out _, out _); + + for (var request = 0; request < InMemoryPasswordResetThrottle.CodeRequestsPerHour; request++) + { + var act = () => service.ForgetPasswordAsync("alice@example.com", CancellationToken.None); + await act.Should().ThrowAsync(); + } + + await service.ForgetPasswordAsync("alice@example.com", CancellationToken.None); + + writes.Should().Be(InMemoryPasswordResetThrottle.CodeRequestsPerHour + 1); + forget.Verify(f => f.PurgeExpiredAsync(It.IsAny(), It.IsAny()), Times.Never); + } + [Theory] [InlineData("123456", "123456", true)] [InlineData("123456", " 123456 ", true)] diff --git a/SeaHaven.Services/Implementation/AuthenticationService.cs b/SeaHaven.Services/Implementation/AuthenticationService.cs index c00c3c2..73b43d2 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -164,25 +164,34 @@ namespace SeaHaven.Services.Implementation var hash = PasswordResetCodeSecrets.Hash(ResetCodeKey, salt, code); var active = user != null && user.IsDeleted != true && !string.IsNullOrWhiteSpace(user.Email); - var queued = false; - if (active) + // The request stays counted only once its row is stored. A code that cannot be + // emailed is stored unmatchable and not counted, and a failed or cancelled + // write is not counted either. + var counted = false; + try { - var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; - queued = _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); + var queued = false; + if (active) + { + var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes."; + queued = _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body); + } - // A code that cannot be emailed is stored unmatchable and the request is not counted. - if (!queued) + await _forgetPasswordDataService.ReplaceCodeAsync( + active ? user!.Email! : requested, + active ? user!.Id : string.Empty, + queued ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), + salt, + nowUtc.Add(ResetCodeLifetime), + nowUtc, + cancellationToken); + counted = queued || !active; + } + finally + { + if (!counted) _resetThrottle.ReleaseCodeRequest(requested); } - - await _forgetPasswordDataService.ReplaceCodeAsync( - active ? user!.Email! : requested, - active ? user!.Id : string.Empty, - queued ? hash : PasswordResetCodeSecrets.NewUnmatchableHash(), - salt, - nowUtc.Add(ResetCodeLifetime), - nowUtc, - cancellationToken); } public async Task VerifyCodeAsync(string? email, string? code, CancellationToken cancellationToken) @@ -242,33 +251,37 @@ namespace SeaHaven.Services.Implementation if (!_resetThrottle.TryReserveCheck(email)) return null; - var pending = await _forgetPasswordDataService.GetByEmailAsync(email, cancellationToken); - if (pending == null) + // Only a wrong code actually compared keeps the slot. No live code, an expired or + // used-up code, a match, and a failed or cancelled call all give it back. + var wrongGuess = false; + try { - _resetThrottle.ReleaseCheck(email); + var pending = await _forgetPasswordDataService.GetByEmailAsync(email, cancellationToken); + if (pending == null) + return null; + + var nowUtc = _timeProvider.GetUtcNow().UtcDateTime; + // Deletes below stop at this code's id: a code issued by a concurrent request + // after this one was read belongs to that request and must survive. + if (!await _forgetPasswordDataService.TryConsumeAttemptAsync(pending.Id, MaxCodeAttempts, nowUtc, cancellationToken)) + { + await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); + return null; + } + + if (PasswordResetCodeSecrets.Matches(ResetCodeKey, pending.CodeSalt, code, pending.CodeHash)) + return pending; + + wrongGuess = true; + if (pending.FailedAttempts + 1 >= MaxCodeAttempts) + await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); return null; } - - var nowUtc = _timeProvider.GetUtcNow().UtcDateTime; - // Deletes below stop at this code's id: a code issued by a concurrent request - // after this one was read belongs to that request and must survive. - if (!await _forgetPasswordDataService.TryConsumeAttemptAsync(pending.Id, MaxCodeAttempts, nowUtc, cancellationToken)) + finally { - // Expired or out of attempts: nothing was compared, so no guess is counted. - _resetThrottle.ReleaseCheck(email); - await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); - return null; + if (!wrongGuess) + _resetThrottle.ReleaseCheck(email); } - - if (PasswordResetCodeSecrets.Matches(ResetCodeKey, pending.CodeSalt, code, pending.CodeHash)) - { - _resetThrottle.ReleaseCheck(email); - return pending; - } - - if (pending.FailedAttempts + 1 >= MaxCodeAttempts) - await _forgetPasswordDataService.RemoveIssuedThroughAsync(pending.Email, pending.Id, cancellationToken); - return null; } private JwtSecurityToken GetToken(List authClaims)