fix(auth): only count real guesses, drop codes that cannot be emailed, trace reset email sends

Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs

@@ -138,6 +138,7 @@ public class AuthenticationServiceTests
         var registered = new Mock<IForgetPasswordDataService>();
         var unregistered = new Mock<IForgetPasswordDataService>();
         var email = new Mock<IPasswordResetEmailQueue>();
+        email.Setup(e => e.TryEnqueue(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>())).Returns(true);
 
         await NewService(userData, registered, email, out _, out _).ForgetPasswordAsync("alice@example.com", CancellationToken.None);
         await NewService(userData, unregistered, email, out _, out _).ForgetPasswordAsync("nope@example.com", CancellationToken.None);
@@ -149,6 +150,26 @@ public class AuthenticationServiceTests
         email.Verify(e => e.TryEnqueue(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()), Times.Once);
     }
 
+    [Fact]
+    public async Task ForgetPassword_EmailThatCannotBeQueued_DropsTheCodeAndDoesNotCountTheRequest()
+    {
+        var user = IdentityTestHelpers.User();
+        var userData = new Mock<IUserDataService>();
+        userData.Setup(u => u.GetByEmailNormalizedAsync(It.IsAny<string>(), It.IsAny<CancellationToken>())).ReturnsAsync(user);
+        var forget = new Mock<IForgetPasswordDataService>();
+        var email = new Mock<IPasswordResetEmailQueue>();
+        email.SetupSequence(e => e.TryEnqueue(user.Email!, It.IsAny<string>(), It.IsAny<string>()))
+            .Returns(false).Returns(false).Returns(false).Returns(true);
+
+        var service = NewService(userData, forget, email, out _, out _);
+        for (var request = 0; request < 4; request++)
+            await service.ForgetPasswordAsync("alice@example.com", CancellationToken.None);
+
+        // Three undelivered codes were removed, and they did not use up the hourly limit of three.
+        forget.Verify(f => f.RemoveByEmailAsync(user.Email!, It.IsAny<CancellationToken>()), Times.Exactly(3));
+        email.Verify(e => e.TryEnqueue(user.Email!, It.IsAny<string>(), It.IsAny<string>()), Times.Exactly(4));
+    }
+
     [Fact]
     public async Task ForgetPassword_DeletedAccount_IsTreatedAsUnregistered()
     {

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));

Api.SeaHavenIndustries/HostedServices/PasswordResetEmailDelivery.cs

@@ -1,5 +1,7 @@
 using System.Threading.Channels;
+using Api.SeaHavenIndustries.Observability;
 using SeaHaven.Services.Interfaces;
+using Sentry;
 
 namespace Api.SeaHavenIndustries.HostedServices
 {
@@ -62,32 +64,43 @@ namespace Api.SeaHavenIndustries.HostedServices
         private readonly PasswordResetEmailChannel _channel;
         private readonly IServiceScopeFactory _scopeFactory;
         private readonly ILogger<PasswordResetEmailSenderHostedService> _logger;
+        private readonly IHub _sentryHub;
 
         public PasswordResetEmailSenderHostedService(
             PasswordResetEmailChannel channel,
             IServiceScopeFactory scopeFactory,
-            ILogger<PasswordResetEmailSenderHostedService> logger)
+            ILogger<PasswordResetEmailSenderHostedService> logger,
+            IHub sentryHub)
         {
             _channel = channel;
             _scopeFactory = scopeFactory;
             _logger = logger;
+            _sentryHub = sentryHub;
         }
 
         protected override async Task ExecuteAsync(CancellationToken stoppingToken)
         {
             await foreach (var email in _channel.Reader.ReadAllAsync(stoppingToken))
             {
+                using var transaction = SentryObservability.BeginBackgroundTransaction(
+                    _sentryHub,
+                    "auth.password-reset-email",
+                    $"{nameof(PasswordResetEmailSenderHostedService)}.{nameof(SendAsync)}");
                 try
                 {
-                    await using var scope = _scopeFactory.CreateAsyncScope();
+                    await SendAsync(email);
-                    var sender = scope.ServiceProvider.GetRequiredService<IEmailSender>();
+                    transaction.FinishOk();
-                    if (!await sender.SendEmailAsync(email.EmailTo, email.Subject, email.Body))
+                }
-                        _logger.LogWarning("Password reset email was not accepted by the mail provider.");
+                catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
+                {
+                    transaction.FinishCancelled();
+                    throw;
                 }
                 catch (Exception ex)
                 {
                     // The message can echo the recipient or the body, so only the type is logged.
                     _logger.LogError("Password reset email failed with {ExceptionType}.", ex.GetType().FullName);
+                    transaction.FinishError(ex);
                 }
                 finally
                 {
@@ -95,5 +108,13 @@ namespace Api.SeaHavenIndustries.HostedServices
                 }
             }
         }
+
+        private async Task SendAsync(PasswordResetEmail email)
+        {
+            await using var scope = _scopeFactory.CreateAsyncScope();
+            var sender = scope.ServiceProvider.GetRequiredService<IEmailSender>();
+            if (!await sender.SendEmailAsync(email.EmailTo, email.Subject, email.Body))
+                _logger.LogWarning("Password reset email was not accepted by the mail provider.");
+        }
     }
 }

SeaHaven.Services/Helpers/InMemoryPasswordResetThrottle.cs

@@ -46,6 +46,17 @@ namespace SeaHaven.Services.Helpers
             }
         }
 
+        public void ReleaseCodeRequest(string email)
+        {
+            var now = _timeProvider.GetUtcNow();
+            lock (_gate)
+            {
+                var account = AccountFor(email, now);
+                if (account.Requests.Count > 0)
+                    account.Requests.RemoveAt(account.Requests.Count - 1);
+            }
+        }
+
         public bool TryReserveCheck(string email)
         {
             var now = _timeProvider.GetUtcNow();

SeaHaven.Services/Implementation/AuthenticationService.cs

@@ -162,7 +162,12 @@ namespace SeaHaven.Services.Implementation
             if (active)
             {
                 var body = $"Your Password Reset Code is: {code}. It expires in {(int)ResetCodeLifetime.TotalMinutes} minutes.";
-                _resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body);
+                if (!_resetEmails.TryEnqueue(user!.Email!, "Forget Password Request.", body))
+                {
+                    // The code will never reach the user: drop it and do not count the request.
+                    await _forgetPasswordDataService.RemoveByEmailAsync(user.Email!, cancellationToken);
+                    _resetThrottle.ReleaseCodeRequest(requested);
+                }
             }
         }
 
@@ -219,17 +224,22 @@ namespace SeaHaven.Services.Implementation
 
             // The per-account budget spans every code the account is sent, so asking for
             // new codes does not buy more guesses. A slot is reserved before comparing and
-            // given back only when the code matches.
+            // given back when the code matches or there is no live code to guess at.
             if (!_resetThrottle.TryReserveCheck(email))
                 return null;
 
             var pending = await _forgetPasswordDataService.GetByEmailAsync(email, cancellationToken);
             if (pending == null)
+            {
+                _resetThrottle.ReleaseCheck(email);
                 return null;
+            }
 
             var nowUtc = _timeProvider.GetUtcNow().UtcDateTime;
             if (!await _forgetPasswordDataService.TryConsumeAttemptAsync(pending.Id, MaxCodeAttempts, nowUtc, cancellationToken))
             {
+                // Expired or out of attempts: nothing was compared, so no guess is counted.
+                _resetThrottle.ReleaseCheck(email);
                 await _forgetPasswordDataService.RemoveByEmailAsync(pending.Email, cancellationToken);
                 return null;
             }

SeaHaven.Services/Interfaces/IPasswordResetThrottle.cs

@@ -12,13 +12,19 @@ namespace SeaHaven.Services.Interfaces
         /// </summary>
         bool TryAcceptCodeRequest(string email);
 
+        /// <summary>Gives back an accepted code request whose email could not be queued.</summary>
+        void ReleaseCodeRequest(string email);
+
         /// <summary>
         /// 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.
         /// </summary>
         bool TryReserveCheck(string email);
 
-        /// <summary>Gives back the reservation of a check whose code matched.</summary>
+        /// <summary>
+        /// Gives back a reservation that did not become a failed guess: the code matched,
+        /// or there was no live code to compare it with.
+        /// </summary>
         void ReleaseCheck(string email);
     }
 }

SeaHavenIndustries.Tests/PasswordResetAbuseLimitsTests.cs

@@ -92,6 +92,34 @@ public sealed class PasswordResetAbuseLimitsTests
         Assert.Equal(HttpStatusCode.OK, (await host.ResetAsync(Alice, host.Sent.LatestCodeFor(Alice), NewPassword, NextClient())).StatusCode);
     }
 
+    [Fact]
+    public async Task Checks_without_a_live_code_do_not_use_up_the_account_budget()
+    {
+        await using var host = await PasswordResetTestHost.StartAsync();
+        await host.AddUserAsync(Alice, OldPassword);
+
+        // No code issued yet.
+        for (var check = 0; check < 10; check++)
+        {
+            await host.VerifyAsync(Alice, "123456", NextClient());
+            await host.ResetAsync(Alice, "123456", NewPassword, NextClient());
+        }
+
+        // A code that has expired.
+        await host.ForgetPasswordAsync(Alice, NextClient());
+        var expired = host.Sent.LatestCodeFor(Alice);
+        host.Time.Advance(TimeSpan.FromMinutes(16));
+        for (var check = 0; check < 5; check++)
+            await host.VerifyAsync(Alice, expired, NextClient());
+
+        await host.ForgetPasswordAsync(Alice, NextClient());
+        var live = host.Sent.LatestCodeFor(Alice);
+
+        Assert.Equal(HttpStatusCode.OK, (await host.VerifyAsync(Alice, live, NextClient())).StatusCode);
+        Assert.Equal(HttpStatusCode.OK, (await host.ResetAsync(Alice, live, NewPassword, NextClient())).StatusCode);
+        Assert.Equal(HttpStatusCode.OK, (await host.LoginAsync(Alice, NewPassword)).StatusCode);
+    }
+
     [Fact]
     public void Concurrent_checks_can_never_exceed_the_account_budget()
     {

SeaHavenIndustries.Tests/PasswordResetTestHost.cs

@@ -93,6 +93,7 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable
                 manager.FeatureProviders.Add(new OnlyAuthenticationController());
             });
         builder.Services.AddPasswordResetRateLimiting();
+        builder.Services.AddSingleton<Sentry.IHub>(Sentry.Extensibility.HubAdapter.Instance);
         builder.Services.AddPasswordResetEmailDelivery();
 
         var app = builder.Build();