address pr review comments

- fix Include() being dropped under Select() projection in
  CreatePasswordResetFlowAsync and CreateEmailChangeFlowAsync;
  deactivation was never detected. Project IsDeactivated explicitly.
- race-safe complete/verify via ExecuteUpdateAsync gated on the
  SecurityStamp predicate so two concurrent valid tokens cannot both
  win and clobber the user's password/email.
- add deprecated aliases POST /reset-password and POST /verify-email
  delegating to the renamed routes.
- ChangeEmail: return AccountOAuthOnly (401) for users without a
  password, instead of the misleading PasswordChangeInvalidPassword.
- fix unresolved cref to OpenShock.Common.Constants.Duration in
  UserPasswordReset and UserEmailChange xmldocs.
- clarify User.SecurityStamp doc to make explicit that re-hashing the
  same password is not a rotation.
- update stale EmailVersion comment in MailTests.

Author: hhvrc

API.IntegrationTests/Tests/MailTests.cs

@@ -439,7 +439,7 @@ public async Task ChangeEmailFlow_SecondPendingRequest_InvalidatedAfterFirstComp
         var firstVerify = await anonClient.PostAsync($"/1/account/email-change/verify?token={firstToken}", null);
         await Assert.That(firstVerify.StatusCode).IsEqualTo(HttpStatusCode.OK);
 
-        // Second pending request is now invalid: its EmailVersionAtCreate snapshot no longer matches User.EmailVersion.
+        // Second pending request is now invalid: its SecurityStampAtCreate snapshot no longer matches User.SecurityStamp.
         var secondVerify = await anonClient.PostAsync($"/1/account/email-change/verify?token={secondToken}", null);
         await Assert.That(secondVerify.StatusCode).IsEqualTo(HttpStatusCode.BadRequest);
 

API/Controller/Account/Authenticated/ChangeEmail.cs

@@ -19,12 +19,18 @@ public sealed partial class AuthenticatedAccountController
     [Consumes(MediaTypeNames.Application.Json)]
     [ProducesResponseType(StatusCodes.Status200OK)]
     [ProducesResponseType<OpenShockProblem>(StatusCodes.Status400BadRequest, MediaTypeNames.Application.ProblemJson)] // EmailChangeUnchanged
+    [ProducesResponseType<OpenShockProblem>(StatusCodes.Status401Unauthorized, MediaTypeNames.Application.ProblemJson)] // AccountOAuthOnly
     [ProducesResponseType<OpenShockProblem>(StatusCodes.Status403Forbidden, MediaTypeNames.Application.ProblemJson)] // PasswordChangeInvalidPassword
     [ProducesResponseType<OpenShockProblem>(StatusCodes.Status409Conflict, MediaTypeNames.Application.ProblemJson)] // EmailChangeAlreadyInUse
     [ProducesResponseType<OpenShockProblem>(StatusCodes.Status429TooManyRequests, MediaTypeNames.Application.ProblemJson)] // EmailChangeTooMany
     public async Task<IActionResult> ChangeEmail([FromBody] ChangeEmailRequest body)
     {
-        if (string.IsNullOrEmpty(CurrentUser.PasswordHash) || !HashingUtils.VerifyPassword(body.CurrentPassword, CurrentUser.PasswordHash).Verified)
+        if (string.IsNullOrEmpty(CurrentUser.PasswordHash))
+        {
+            return Problem(AccountError.AccountOAuthOnly);
+        }
+
+        if (!HashingUtils.VerifyPassword(body.CurrentPassword, CurrentUser.PasswordHash).Verified)
         {
             return Problem(AccountError.PasswordChangeInvalidPassword);
         }

API/Controller/Account/PasswordResetInitiateV2.cs

@@ -1,4 +1,5 @@
-using System.Net;
+using System;
+using System.Net;
 using System.Net.Mime;
 using Microsoft.AspNetCore.Mvc;
 using Asp.Versioning;
@@ -23,7 +24,24 @@ public sealed partial class AccountController
     [ProducesResponseType(StatusCodes.Status200OK)]
     [ProducesResponseType<OpenShockProblem>(StatusCodes.Status403Forbidden, MediaTypeNames.Application.ProblemJson)]
     [MapToApiVersion("2")]
-    public async Task<IActionResult> PasswordResetInitiateV2([FromBody] PasswordResetRequestV2 body, [FromServices] ICloudflareTurnstileService turnstileService, CancellationToken cancellationToken)
+    public Task<IActionResult> PasswordResetInitiateV2([FromBody] PasswordResetRequestV2 body, [FromServices] ICloudflareTurnstileService turnstileService, CancellationToken cancellationToken)
+        => PasswordResetInitiate(body, turnstileService, cancellationToken);
+
+    /// <summary>
+    /// Initiate a password reset. Deprecated: use POST /password-reset instead.
+    /// </summary>
+    /// <response code="200">Password reset email sent if the email is associated to an registered account</response>
+    [Obsolete("Use POST /password-reset instead.")]
+    [HttpPost("reset-password")]
+    [EnableRateLimiting("auth")]
+    [Consumes(MediaTypeNames.Application.Json)]
+    [ProducesResponseType(StatusCodes.Status200OK)]
+    [ProducesResponseType<OpenShockProblem>(StatusCodes.Status403Forbidden, MediaTypeNames.Application.ProblemJson)]
+    [MapToApiVersion("2")]
+    public Task<IActionResult> PasswordResetInitiateV2Legacy([FromBody] PasswordResetRequestV2 body, [FromServices] ICloudflareTurnstileService turnstileService, CancellationToken cancellationToken)
+        => PasswordResetInitiate(body, turnstileService, cancellationToken);
+
+    private async Task<IActionResult> PasswordResetInitiate(PasswordResetRequestV2 body, ICloudflareTurnstileService turnstileService, CancellationToken cancellationToken)
     {
         var turnStile = await turnstileService.VerifyUserResponseTokenAsync(body.TurnstileResponse, HttpContext.GetRemoteIP(), cancellationToken);
         if (!turnStile.IsT0)
@@ -34,9 +52,9 @@ public async Task<IActionResult> PasswordResetInitiateV2([FromBody] PasswordRese
 
             return Problem(new OpenShockProblem("InternalServerError", "Internal Server Error", HttpStatusCode.InternalServerError));
         }
-        
+
         await _accountService.CreatePasswordResetFlowAsync(body.Email);
-        
+
         return Ok();
     }
 }

API/Controller/Account/VerifyEmail.cs

@@ -1,4 +1,5 @@
-using System.Net.Mime;
+using System;
+using System.Net.Mime;
 using Microsoft.AspNetCore.Mvc;
 using Asp.Versioning;
 using OpenShock.Common.Errors;
@@ -19,7 +20,25 @@ public sealed partial class AccountController
     [ProducesResponseType<OpenShockProblem>(StatusCodes.Status400BadRequest, MediaTypeNames.Application.ProblemJson)]
     [ProducesResponseType<OpenShockProblem>(StatusCodes.Status409Conflict, MediaTypeNames.Application.ProblemJson)]
     [MapToApiVersion("1")]
-    public async Task<IActionResult> EmailVerify([FromQuery(Name = "token")] string token, CancellationToken cancellationToken)
+    public Task<IActionResult> EmailVerify([FromQuery(Name = "token")] string token, CancellationToken cancellationToken)
+        => VerifyPendingEmailChange(token, cancellationToken);
+
+    /// <summary>
+    /// Verify a pending email change. Deprecated: use POST /email-change/verify instead.
+    /// </summary>
+    /// <response code="200">Email change verified and applied</response>
+    /// <response code="400">Token is invalid, already used, or the request has expired</response>
+    /// <response code="409">The new email address was claimed by another account before verification completed</response>
+    [Obsolete("Use POST /email-change/verify instead.")]
+    [HttpPost("verify-email")]
+    [ProducesResponseType(StatusCodes.Status200OK)]
+    [ProducesResponseType<OpenShockProblem>(StatusCodes.Status400BadRequest, MediaTypeNames.Application.ProblemJson)]
+    [ProducesResponseType<OpenShockProblem>(StatusCodes.Status409Conflict, MediaTypeNames.Application.ProblemJson)]
+    [MapToApiVersion("1")]
+    public Task<IActionResult> EmailVerifyLegacy([FromQuery(Name = "token")] string token, CancellationToken cancellationToken)
+        => VerifyPendingEmailChange(token, cancellationToken);
+
+    private async Task<IActionResult> VerifyPendingEmailChange(string token, CancellationToken cancellationToken)
     {
         var result = await _accountService.TryVerifyEmailAsync(token, cancellationToken);
 

API/Services/Account/AccountService.cs

@@ -400,16 +400,16 @@ public async Task<OneOf<Success, TooManyPasswordResets, AccountNotActivated, Acc
         var lowerCaseEmail = email.ToLowerInvariant();
         var user = await _db.Users
             .Where(x => x.Email == lowerCaseEmail)
-            .Include(x => x.UserDeactivation)
             .Select(x => new
             {
                 User = x,
+                IsDeactivated = x.UserDeactivation != null,
                 PasswordResetCount = x.PasswordResets.Count(y => y.UsedAt == null && y.CreatedAt >= validSince)
             })
             .FirstOrDefaultAsync();
         if (user is null) return new NotFound();
         if (user.User.ActivatedAt is null) return new AccountNotActivated();
-        if (user.User.UserDeactivation is not null) return new AccountDeactivated();
+        if (user.IsDeactivated) return new AccountDeactivated();
         if (user.PasswordResetCount >= 3) return new TooManyPasswordResets();
 
         var token = CryptoUtils.RandomAlphaNumericString(AuthConstants.GeneratedTokenLength);
@@ -436,21 +436,39 @@ public async Task<OneOf<Success, NotFound, AccountNotActivated, AccountDeactivat
         var validSince = DateTime.UtcNow - Duration.PasswordResetRequestLifetime;
 
         var reset = await _db.UserPasswordResets
-            .Include(x => x.User)
-            .Include(x => x.User.UserDeactivation)
-            .FirstOrDefaultAsync(x => x.Id == passwordResetId && x.UsedAt == null && x.CreatedAt >= validSince
-                && x.SecurityStampAtCreate == x.User.SecurityStamp);
+            .Select(x => new
+            {
+                Reset = x,
+                UserActivatedAt = x.User.ActivatedAt,
+                IsDeactivated = x.User.UserDeactivation != null,
+                UserSecurityStamp = x.User.SecurityStamp
+            })
+            .FirstOrDefaultAsync(x => x.Reset.Id == passwordResetId && x.Reset.UsedAt == null && x.Reset.CreatedAt >= validSince
+                && x.Reset.SecurityStampAtCreate == x.UserSecurityStamp);
         if (reset is null) return new NotFound();
-        if (reset.User.ActivatedAt is null) return new AccountNotActivated();
-        if (reset.User.UserDeactivation is not null) return new AccountDeactivated();
+        if (reset.UserActivatedAt is null) return new AccountNotActivated();
+        if (reset.IsDeactivated) return new AccountDeactivated();
 
-        var result = HashingUtils.VerifyToken(secret, reset.TokenHash);
+        var result = HashingUtils.VerifyToken(secret, reset.Reset.TokenHash);
         if (!result.Verified) return new SecretInvalid();
 
-        reset.UsedAt = DateTime.UtcNow;
-        reset.User.PasswordHash = HashingUtils.HashPassword(newPassword);
-        reset.User.SecurityStamp = Guid.CreateVersion7(); // Rotates the stamp; every other pending reset/email-change for this user is now invalid by predicate.
-        await _db.SaveChangesAsync();
+        // Race-safe consume + apply: only updates if SecurityStamp still matches the snapshot.
+        // If a sibling reset (or a separate password/email change) completed since the read above,
+        // the stamp has rotated and the predicate matches zero rows.
+        var newPasswordHash = HashingUtils.HashPassword(newPassword);
+        var newStamp = Guid.CreateVersion7();
+        var userRows = await _db.Users
+            .Where(u => u.Id == reset.Reset.UserId && u.SecurityStamp == reset.Reset.SecurityStampAtCreate)
+            .ExecuteUpdateAsync(s => s
+                .SetProperty(u => u.PasswordHash, newPasswordHash)
+                .SetProperty(u => u.SecurityStamp, newStamp));
+        if (userRows == 0) return new NotFound();
+
+        var now = DateTime.UtcNow;
+        await _db.UserPasswordResets
+            .Where(r => r.Id == reset.Reset.Id && r.UsedAt == null)
+            .ExecuteUpdateAsync(s => s.SetProperty(r => r.UsedAt, now));
+
         return new Success();
     }
 
@@ -537,16 +555,16 @@ public async Task<OneOf<Success, EmailAlreadyInUse, EmailUnchanged, TooManyEmail
 
         var data = await _db.Users
             .Where(x => x.Id == userId)
-            .Include(x => x.UserDeactivation)
             .Select(x => new
             {
                 User = x,
+                IsDeactivated = x.UserDeactivation != null,
                 PendingCount = x.EmailChanges.Count(y => y.UsedAt == null && y.CreatedAt >= validSince)
             })
             .FirstOrDefaultAsync();
         if (data is null) return new NotFound();
         if (data.User.ActivatedAt is null) return new AccountNotActivated();
-        if (data.User.UserDeactivation is not null) return new AccountDeactivated();
+        if (data.IsDeactivated) return new AccountDeactivated();
         if (string.Equals(data.User.Email, lowerCaseEmail, StringComparison.Ordinal)) return new EmailUnchanged();
         if (data.PendingCount >= 3) return new TooManyEmailChanges();
 
@@ -594,28 +612,45 @@ public async Task<OneOf<Success, NotFound, EmailAlreadyInUse>> TryVerifyEmailAsy
         var validSince = DateTime.UtcNow - Duration.EmailChangeRequestLifetime;
 
         var change = await _db.UserEmailChanges
-            .Include(x => x.User).ThenInclude(u => u.UserDeactivation)
-            .FirstOrDefaultAsync(x => x.TokenHash == hash && x.UsedAt == null && x.CreatedAt >= validSince
+            .Where(x => x.TokenHash == hash && x.UsedAt == null && x.CreatedAt >= validSince
                 && x.SecurityStampAtCreate == x.User.SecurityStamp
-                && x.User.UserDeactivation == null && x.User.ActivatedAt != null, cancellationToken);
+                && x.User.UserDeactivation == null && x.User.ActivatedAt != null)
+            .Select(x => new
+            {
+                ChangeId = x.Id,
+                UserId = x.UserId,
+                x.NewEmail,
+                x.SecurityStampAtCreate
+            })
+            .FirstOrDefaultAsync(cancellationToken);
 
         if (change is null) return new NotFound();
 
-        change.UsedAt = DateTime.UtcNow;
-        change.User.Email = change.NewEmail;
-        change.User.SecurityStamp = Guid.CreateVersion7(); // Rotates the stamp; every other pending reset/email-change for this user is now invalid by predicate.
-
+        // Race-safe consume + apply: only updates if SecurityStamp still matches the snapshot, so
+        // sibling email changes / password resets that completed since the read above cleanly lose.
+        var newStamp = Guid.CreateVersion7();
         try
         {
-            await _db.SaveChangesAsync(cancellationToken);
-            return new Success();
+            var userRows = await _db.Users
+                .Where(u => u.Id == change.UserId && u.SecurityStamp == change.SecurityStampAtCreate)
+                .ExecuteUpdateAsync(s => s
+                    .SetProperty(u => u.Email, change.NewEmail)
+                    .SetProperty(u => u.SecurityStamp, newStamp), cancellationToken);
+            if (userRows == 0) return new NotFound();
         }
         catch (DbUpdateException ex) when (ex.InnerException is PostgresException { SqlState: "23505" })
         {
             // Another account claimed this email between request creation and verification.
             // The pending row stays as-is (not marked used) so it can expire naturally.
             return new EmailAlreadyInUse();
         }
+
+        var now = DateTime.UtcNow;
+        await _db.UserEmailChanges
+            .Where(c => c.Id == change.ChangeId && c.UsedAt == null)
+            .ExecuteUpdateAsync(s => s.SetProperty(c => c.UsedAt, now), cancellationToken);
+
+        return new Success();
     }
 
     private async Task<bool> CheckPassword(string password, User user)

Common/OpenShockDb/User.cs

@@ -13,10 +13,11 @@ public sealed class User
     public string? PasswordHash { get; set; }
 
     /// <summary>
-    /// Opaque value rotated whenever a security-sensitive field on this user changes
-    /// (currently <see cref="PasswordHash"/> and <see cref="Email"/>). Snapshotted onto pending
-    /// password resets and email changes so that any rotation silently invalidates every
-    /// outstanding request for this user, regardless of which code path applied the change.
+    /// Opaque value rotated whenever the user's password or email <em>value</em> changes
+    /// (re-hashing the same password with a stronger algorithm does not rotate it).
+    /// Snapshotted onto pending password resets and email changes so that any rotation
+    /// silently invalidates every outstanding request for this user, regardless of which
+    /// code path applied the change.
     /// </summary>
     public Guid SecurityStamp { get; set; }
 

Common/OpenShockDb/UserEmailChange.cs

@@ -1,4 +1,6 @@
-namespace OpenShock.Common.OpenShockDb;
+using OpenShock.Common.Constants;
+
+namespace OpenShock.Common.OpenShockDb;
 
 /// <summary>
 /// A pending or completed email-change request. The row is created when an authenticated user
@@ -50,7 +52,7 @@ public sealed class UserEmailChange
     public DateTime? UsedAt { get; set; }
 
     /// <summary>
-    /// When the request was created. Combined with <see cref="Constants.Duration.EmailChangeRequestLifetime"/>
+    /// When the request was created. Combined with <see cref="Duration.EmailChangeRequestLifetime"/>
     /// to enforce the link's expiry.
     /// </summary>
     public DateTime CreatedAt { get; set; }

Common/OpenShockDb/UserPasswordReset.cs

@@ -1,4 +1,6 @@
-namespace OpenShock.Common.OpenShockDb;
+using OpenShock.Common.Constants;
+
+namespace OpenShock.Common.OpenShockDb;
 
 /// <summary>
 /// A pending or completed password reset request, created when a user (or someone with knowledge
@@ -37,7 +39,7 @@ public sealed class UserPasswordReset
     public DateTime? UsedAt { get; set; }
 
     /// <summary>
-    /// When the reset was created. Combined with <see cref="Constants.Duration.PasswordResetRequestLifetime"/>
+    /// When the reset was created. Combined with <see cref="Duration.PasswordResetRequestLifetime"/>
     /// to enforce the link's expiry.
     /// </summary>
     public DateTime CreatedAt { get; set; }