From bcc9b6d7a247a02f2ef4bc700a20396fba3f9c7f Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Fri, 25 Sep 2026 12:31:17 -0300 Subject: [PATCH] fix(auth): invalidate every pending reset code for an email together Two concurrent first requests can leave two pending codes for one email. Exhausting or using one now deletes all of them, so a sibling code cannot become live afterwards. --- .../AuthenticationServiceTests.cs | 2 +- .../ForgetPasswordDataService.cs | 7 ----- .../Interfaces/IForgetPasswordDataService.cs | 1 - .../Implementation/AuthenticationService.cs | 8 ++--- .../PasswordResetFlowTests.cs | 31 +++++++++++++++++++ .../PasswordResetTestHost.cs | 22 +++++++++++++ 6 files changed, 58 insertions(+), 13 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs index 1e1c1f0..ef8a297 100644 --- a/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/AuthenticationServiceTests.cs @@ -206,7 +206,7 @@ public class AuthenticationServiceTests var result = await service.ResetPasswordAsync("a@b.com", "123456", "New@67890", CancellationToken.None); result.Should().BeFalse(); - forget.Verify(f => f.RemoveAsync(7, It.IsAny()), Times.Once); + forget.Verify(f => f.RemoveByEmailAsync("a@b.com", It.IsAny()), Times.Once); store.Verify(s => s.FindByIdAsync(It.IsAny(), It.IsAny()), Times.Never); } diff --git a/SeaHaven.DataServices/Implementation/ForgetPasswordDataService.cs b/SeaHaven.DataServices/Implementation/ForgetPasswordDataService.cs index 7eec75f..7e3b3c4 100644 --- a/SeaHaven.DataServices/Implementation/ForgetPasswordDataService.cs +++ b/SeaHaven.DataServices/Implementation/ForgetPasswordDataService.cs @@ -67,13 +67,6 @@ namespace SeaHaven.DataServices.Implementation cancellationToken); } - public async Task RemoveAsync(int id, CancellationToken cancellationToken) - { - await _context.ForgetPasswordCodes - .Where(u => u.Id == id) - .ExecuteDeleteAsync(cancellationToken); - } - public async Task RemoveByEmailAsync(string email, CancellationToken cancellationToken) { var normalizedEmail = Normalize(email); diff --git a/SeaHaven.DataServices/Interfaces/IForgetPasswordDataService.cs b/SeaHaven.DataServices/Interfaces/IForgetPasswordDataService.cs index e145aa9..a7de61e 100644 --- a/SeaHaven.DataServices/Interfaces/IForgetPasswordDataService.cs +++ b/SeaHaven.DataServices/Interfaces/IForgetPasswordDataService.cs @@ -19,7 +19,6 @@ namespace SeaHaven.DataServices.Interfaces /// Gives back an attempt consumed by a check that matched. Task RefundAttemptAsync(int id, CancellationToken cancellationToken); - Task RemoveAsync(int id, CancellationToken cancellationToken); Task RemoveByEmailAsync(string email, CancellationToken cancellationToken); } } diff --git a/SeaHaven.Services/Implementation/AuthenticationService.cs b/SeaHaven.Services/Implementation/AuthenticationService.cs index fa90dff..1a0d263 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -164,7 +164,7 @@ namespace SeaHaven.Services.Implementation var user = await _userManager.FindByIdAsync(pending.UserId); if (user == null || user.IsDeleted == true) { - await _forgetPasswordDataService.RemoveAsync(pending.Id, cancellationToken); + await _forgetPasswordDataService.RemoveByEmailAsync(pending.Email, cancellationToken); return false; } @@ -178,7 +178,7 @@ namespace SeaHaven.Services.Implementation return false; } - await _forgetPasswordDataService.RemoveAsync(pending.Id, cancellationToken); + await _forgetPasswordDataService.RemoveByEmailAsync(pending.Email, cancellationToken); return true; } @@ -199,7 +199,7 @@ namespace SeaHaven.Services.Implementation var nowUtc = _timeProvider.GetUtcNow().UtcDateTime; if (!await _forgetPasswordDataService.TryConsumeAttemptAsync(pending.Id, MaxCodeAttempts, nowUtc, cancellationToken)) { - await _forgetPasswordDataService.RemoveAsync(pending.Id, cancellationToken); + await _forgetPasswordDataService.RemoveByEmailAsync(pending.Email, cancellationToken); return null; } @@ -207,7 +207,7 @@ namespace SeaHaven.Services.Implementation return pending; if (pending.FailedAttempts + 1 >= MaxCodeAttempts) - await _forgetPasswordDataService.RemoveAsync(pending.Id, cancellationToken); + await _forgetPasswordDataService.RemoveByEmailAsync(pending.Email, cancellationToken); return null; } diff --git a/SeaHavenIndustries.Tests/PasswordResetFlowTests.cs b/SeaHavenIndustries.Tests/PasswordResetFlowTests.cs index 97b3630..51f3c70 100644 --- a/SeaHavenIndustries.Tests/PasswordResetFlowTests.cs +++ b/SeaHavenIndustries.Tests/PasswordResetFlowTests.cs @@ -163,6 +163,37 @@ public sealed class PasswordResetFlowTests Assert.Equal(HttpStatusCode.OK, (await host.ResetAsync(Alice, host.Sent.LatestCodeFor(Alice), NewPassword)).StatusCode); } + [Fact] + public async Task Exhausting_a_code_invalidates_every_pending_code_for_that_email() + { + await using var host = await PasswordResetTestHost.StartAsync(); + var user = await host.AddUserAsync(Alice, OldPassword); + await host.ForgetPasswordAsync(Alice); + var older = host.Sent.LatestCodeFor(Alice); + var newest = await host.AddSiblingCodeAsync(Alice, user.Id); + + for (var attempt = 0; attempt < 5; attempt++) + await host.VerifyAsync(Alice, WrongCode(newest)); + + Assert.Empty(await host.PendingCodesAsync()); + Assert.Equal(ResetFailed, await (await host.ResetAsync(Alice, older, NewPassword)).Content.ReadAsStringAsync()); + } + + [Fact] + public async Task A_successful_reset_invalidates_every_pending_code_for_that_email() + { + await using var host = await PasswordResetTestHost.StartAsync(); + var user = await host.AddUserAsync(Alice, OldPassword); + await host.ForgetPasswordAsync(Alice); + var older = host.Sent.LatestCodeFor(Alice); + var newest = await host.AddSiblingCodeAsync(Alice, user.Id); + + Assert.Equal(HttpStatusCode.OK, (await host.ResetAsync(Alice, newest, NewPassword)).StatusCode); + + Assert.Empty(await host.PendingCodesAsync()); + Assert.Equal(ResetFailed, await (await host.ResetAsync(Alice, older, "Other@24680")).Content.ReadAsStringAsync()); + } + [Fact] public async Task Four_wrong_attempts_then_the_right_code_still_resets() { diff --git a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs index bbfe044..6904954 100644 --- a/SeaHavenIndustries.Tests/PasswordResetTestHost.cs +++ b/SeaHavenIndustries.Tests/PasswordResetTestHost.cs @@ -137,6 +137,28 @@ internal sealed class PasswordResetTestHost : IAsyncDisposable .ForgetPasswordCodes.AsNoTracking().OrderBy(code => code.Id).ToListAsync(); } + /// + /// 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) + { + const string code = "424242"; + const string salt = "0123456789abcdef0123456789abcdef"; + await using var scope = _app.Services.CreateAsyncScope(); + var context = scope.ServiceProvider.GetRequiredService(); + context.ForgetPasswordCodes.Add(new ForgetPasswordCode + { + Email = email, + UserId = userId, + CodeSalt = salt, + CodeHash = SeaHaven.Services.Helpers.PasswordResetCodeSecrets.Hash(salt, code), + ExpiresAtUtc = Time.GetUtcNow().UtcDateTime.AddMinutes(15) + }); + await context.SaveChangesAsync(); + return code; + } + public static HttpRequestMessage Post(string path, object? json = null, string? clientIp = null) { var request = new HttpRequestMessage(HttpMethod.Post, path);