fix(identity): keep the profile ETag stable across a failed sign-in

src/Modules/Identity/Modules.Identity.Contracts/DTOs/UserDto.cs

@@ -26,10 +26,10 @@ public class UserDto
     public bool TwoFactorEnabled { get; set; }
 
     /// <summary>
-    /// The stored optimistic-concurrency token for this user, populated only by the self-profile
-    /// read. It never reaches the response body — <c>GET /identity/profile</c> turns it into the
-    /// response's <c>ETag</c>, and that header is the token clients echo back in <c>If-Match</c>.
+    /// A hash of the profile fields <c>PUT /identity/profile</c> writes, populated only by the
+    /// self-profile read. It never reaches the response body — <c>GET /identity/profile</c> turns it
+    /// into the response's <c>ETag</c>, and that header is the token clients echo back in <c>If-Match</c>.
     /// </summary>
     [JsonIgnore]
-    public string? ConcurrencyStamp { get; set; }
+    public string? ProfileVersion { get; set; }
 }

src/Modules/Identity/Modules.Identity.Contracts/Services/IUserProfileService.cs

@@ -26,7 +26,7 @@ public interface IUserProfileService
     /// <summary>
     /// Updates a user's profile. When <paramref name="expectedConcurrencyStamps"/> is non-null the
     /// update is rejected with <see cref="System.Net.HttpStatusCode.PreconditionFailed"/> unless the
-    /// stored concurrency token matches one of the entries — the caller edited a stale copy.
+    /// current profile version matches one of the entries — the caller edited a stale copy.
     /// </summary>
     Task UpdateAsync(string userId, string firstName, string lastName, string phoneNumber, FileUploadRequest image, bool deleteCurrentImage, IReadOnlyList<string>? expectedConcurrencyStamps, CancellationToken cancellationToken = default);
 

src/Modules/Identity/Modules.Identity.Contracts/v1/Users/UpdateUser/UpdateUserCommand.cs

@@ -18,7 +18,7 @@ public class UpdateUserCommand : ICommand<Unit>
     /// Concurrency tokens the caller is willing to overwrite, taken from the request's
     /// <c>If-Match</c> header by the endpoint. <see langword="null"/> means the caller sent no
     /// precondition and accepts whatever version is stored; a non-null list means the update
-    /// only proceeds when the stored token matches one of the entries.
+    /// only proceeds when the current profile version matches one of the entries.
     /// </summary>
     /// <remarks>
     /// Header-derived, never read from the request body — the endpoint always overwrites it.

src/Modules/Identity/Modules.Identity/Features/v1/Users/GetUserProfile/GetUserProfileEndpoint.cs

@@ -26,12 +26,9 @@ internal static RouteHandlerBuilder MapGetMeEndpoint(this IEndpointRouteBuilder
 
             // The profile is a full-representation resource: PUT /profile rewrites every field, so
             // a caller editing a stale copy would blank whatever changed meanwhile. Publishing the
-            // stored concurrency token as a strong ETag lets that caller echo it back in If-Match
-            // and have the server reject the stale write.
-            if (!string.IsNullOrEmpty(profile.ConcurrencyStamp))
-            {
-                response.Headers.ETag = new EntityTagHeaderValue($"\"{profile.ConcurrencyStamp}\"", isWeak: false).ToString();
-            }
+            // profile version as a strong ETag lets that caller echo it back in If-Match and have
+            // the server reject the stale write.
+            response.Headers.ETag = new EntityTagHeaderValue($"\"{profile.ProfileVersion}\"", isWeak: false).ToString();
 
             return TypedResults.Ok(profile);
         })

src/Modules/Identity/Modules.Identity/Features/v1/Users/UpdateUser/UpdateUserEndpoint.cs

@@ -69,7 +69,7 @@ internal static RouteHandlerBuilder MapUpdateUserEndpoint(this IEndpointRouteBui
         }
 
         // If-Match mandates the strong comparison function, so a weak validator can never match.
-        // Dropping the weak entries leaves a list no stored token matches, which is exactly the
+        // Dropping the weak entries leaves a list no profile version matches, which is exactly the
         // 412 the RFC asks for.
         return entityTags
             .Where(entityTag => !entityTag.IsWeak)

src/Modules/Identity/Modules.Identity/Services/UserProfileService.cs

@@ -10,6 +10,9 @@
 using Microsoft.AspNetCore.Identity;
 using Microsoft.EntityFrameworkCore;
 using System.Net;
+using System.Security.Cryptography;
+using System.Text;
+using System.Text.Json;
 
 namespace FSH.Modules.Identity.Services;
 
@@ -44,7 +47,7 @@ public async Task<UserDto> GetAsync(string userId, CancellationToken cancellatio
             EmailConfirmed = user.EmailConfirmed,
             PhoneNumber = user.PhoneNumber,
             TwoFactorEnabled = user.TwoFactorEnabled,
-            ConcurrencyStamp = user.ConcurrencyStamp,
+            ProfileVersion = ComputeProfileVersion(user),
         };
     }
 
@@ -82,7 +85,7 @@ public async Task UpdateAsync(string userId, string firstName, string lastName,
         // silently blank whatever changed since. The precondition is checked here, before the
         // storage calls below: a rejected update must not leave an orphan upload behind, and on
         // the deleteCurrentImage path it must not remove the avatar with no database change.
-        EnsureConcurrencyStampMatches(user, expectedConcurrencyStamps);
+        EnsureProfileUnchanged(user, expectedConcurrencyStamps);
 
         // The old blob is only deleted once the database write has gone through. UpdateAsync can
         // still lose a race here — the If-Match check above is not the last word, because another
@@ -120,9 +123,11 @@ public async Task UpdateAsync(string userId, string firstName, string lastName,
         if (!result.Succeeded)
         {
             // Identity's store answers a lost race with ConcurrencyFailure instead of throwing,
-            // so it would otherwise surface as a generic 500. It is the same condition the
-            // If-Match check above reports, just detected one layer down: another writer landed
-            // between our read and our save.
+            // so it would otherwise surface as a generic 500. The store compares its own
+            // ConcurrencyStamp, which every write to the row rotates, so a failed sign-in landing
+            // between our read and our save also ends here, as a 412 with the profile untouched.
+            // Accepted: the window is this request's own, and re-reading and saving again recovers
+            // exactly as it does from a real conflict.
             if (result.Errors.Any(error => string.Equals(error.Code, errorDescriber.ConcurrencyFailure().Code, StringComparison.Ordinal)))
             {
                 throw StaleProfileException();
@@ -139,7 +144,7 @@ public async Task UpdateAsync(string userId, string firstName, string lastName,
         await signInManager.RefreshSignInAsync(user);
     }
 
-    private static void EnsureConcurrencyStampMatches(FshUser user, IReadOnlyList<string>? expectedConcurrencyStamps)
+    private static void EnsureProfileUnchanged(FshUser user, IReadOnlyList<string>? expectedConcurrencyStamps)
     {
         // A null list means the caller sent no If-Match and accepts the stored version as-is.
         // ponytail: keep the precondition optional for backward compatibility; a future major can
@@ -149,13 +154,25 @@ private static void EnsureConcurrencyStampMatches(FshUser user, IReadOnlyList<st
             return;
         }
 
-        var storedStamp = user.ConcurrencyStamp;
-        if (storedStamp is null || !expectedConcurrencyStamps.Contains(storedStamp, StringComparer.Ordinal))
+        if (!expectedConcurrencyStamps.Contains(ComputeProfileVersion(user), StringComparer.Ordinal))
         {
             throw StaleProfileException();
         }
     }
 
+    // Covers only the fields PUT /profile writes. Identity's ConcurrencyStamp also rotates on
+    // writes that leave the profile alone, such as the failed-sign-in counter, and would turn a
+    // wrong password typed anywhere into a 412 on an untouched form. Keyed with the SecurityStamp,
+    // which never leaves the server, because the tag travels in a header proxies and APM log and a
+    // plain hash of name plus phone is brute-forceable over the small phone-number space.
+    // ponytail: the stamp also rotates on a password or 2FA change, so those move the tag too;
+    // swap for a server-wide HMAC key if that ever needs to stop.
+    private static string ComputeProfileVersion(FshUser user) =>
+        Convert.ToHexStringLower(HMACSHA256.HashData(
+            Encoding.UTF8.GetBytes(user.SecurityStamp ?? string.Empty),
+            JsonSerializer.SerializeToUtf8Bytes(
+                new[] { user.FirstName, user.LastName, user.PhoneNumber, user.ImageUrl?.OriginalString })));
+
     private static CustomException StaleProfileException() =>
         new(
             "The profile changed since you loaded it. Reload it and apply your changes again.",

src/Tests/Integration.Tests/Tests/Users/UserProfileTests.cs

@@ -134,6 +134,33 @@ public async Task GetProfile_Should_ReturnStrongETag_When_ProfileIsRead()
         response.Headers.ETag.Tag.ShouldEndWith("\"");
     }
 
+    [Fact]
+    public async Task GetProfile_Should_ReturnDifferentETags_When_TwoUsersHoldTheSameProfileValues()
+    {
+        // Arrange — the tag goes out in a header, so it must not be a bare hash of the profile:
+        // that would let anyone holding a logged tag confirm a guessed name and phone offline.
+        using var adminClient = await _auth.CreateRootAdminClientAsync();
+        var first = await IdentityUserSeeder.CreateLoginableUserAsync(_factory, adminClient, "etag-key-a");
+        var second = await IdentityUserSeeder.CreateLoginableUserAsync(_factory, adminClient, "etag-key-b");
+        using var firstClient = await _auth.CreateAuthenticatedClientAsync(first.Email, first.Password);
+        using var secondClient = await _auth.CreateAuthenticatedClientAsync(second.Email, second.Password);
+
+        var sameValues = new { firstName = "Same", lastName = "Person", phoneNumber = "5550002222" };
+        (await PutProfileAsync(firstClient, sameValues, ifMatch: null)).StatusCode.ShouldBe(HttpStatusCode.OK);
+        (await PutProfileAsync(secondClient, sameValues, ifMatch: null)).StatusCode.ShouldBe(HttpStatusCode.OK);
+
+        // Act
+        var firstRead = await firstClient.GetAsync($"{TestConstants.IdentityBasePath}/profile");
+        var secondRead = await secondClient.GetAsync($"{TestConstants.IdentityBasePath}/profile");
+
+        // Assert
+        var firstDto = await firstRead.DeserializeAsync<UserDto>();
+        var secondDto = await secondRead.DeserializeAsync<UserDto>();
+        (firstDto.FirstName, firstDto.LastName, firstDto.PhoneNumber, firstDto.ImageUrl)
+            .ShouldBe((secondDto.FirstName, secondDto.LastName, secondDto.PhoneNumber, secondDto.ImageUrl));
+        firstRead.Headers.ETag!.ToString().ShouldNotBe(secondRead.Headers.ETag!.ToString());
+    }
+
     [Fact]
     public async Task UpdateProfile_Should_PersistAndRotateETag_When_IfMatchMatches()
     {
@@ -193,6 +220,62 @@ public async Task UpdateProfile_Should_Return412AndKeepConcurrentChange_When_IfM
         dto.PhoneNumber.ShouldBe("5550001111");
     }
 
+    [Fact]
+    public async Task UpdateProfile_Should_Succeed_When_SignInAttemptsLandAfterRead()
+    {
+        // Arrange — a wrong password bumps AccessFailedCount and the next good sign-in resets it.
+        // Both write the user row, but neither touches the profile, so neither may turn the
+        // caller's open form into a 412.
+        using var adminClient = await _auth.CreateRootAdminClientAsync();
+        var user = await IdentityUserSeeder.CreateLoginableUserAsync(_factory, adminClient, "etag-signin");
+        using var userClient = await _auth.CreateAuthenticatedClientAsync(user.Email, user.Password);
+        var etag = await ReadProfileETagAsync(userClient);
+
+        var failedSignIn = await AttemptSignInAsync(user.Email, "Wrong-Password-1!");
+        failedSignIn.StatusCode.ShouldBe(HttpStatusCode.Unauthorized);
+        (await ReadProfileETagAsync(userClient)).ShouldBe(etag);
+
+        var succeededSignIn = await AttemptSignInAsync(user.Email, user.Password);
+        succeededSignIn.StatusCode.ShouldBe(HttpStatusCode.OK);
+        (await ReadProfileETagAsync(userClient)).ShouldBe(etag);
+
+        // Act
+        var response = await PutProfileAsync(userClient, new { firstName = "AfterSignIn" }, etag);
+
+        // Assert
+        response.StatusCode.ShouldBe(HttpStatusCode.OK);
+
+        var profile = await userClient.GetAsync($"{TestConstants.IdentityBasePath}/profile");
+        var dto = await profile.DeserializeAsync<UserDto>();
+        dto.FirstName.ShouldBe("AfterSignIn");
+    }
+
+    [Fact]
+    public async Task UpdateProfile_Should_Return412_When_AvatarChangedAfterRead()
+    {
+        // Arrange — the avatar has its own endpoint, but PUT /profile writes it too, so a change
+        // made there must still invalidate a form loaded before it.
+        using var adminClient = await _auth.CreateRootAdminClientAsync();
+        var user = await IdentityUserSeeder.CreateLoginableUserAsync(_factory, adminClient, "etag-avatar");
+        using var userClient = await _auth.CreateAuthenticatedClientAsync(user.Email, user.Password);
+        var staleETag = await ReadProfileETagAsync(userClient);
+
+        const string imageUrl = "https://cdn.example.com/avatars/set-elsewhere.png";
+        var setImage = await userClient.PutAsJsonAsync(
+            $"{TestConstants.IdentityBasePath}/profile/image", new { imageUrl });
+        setImage.StatusCode.ShouldBe(HttpStatusCode.NoContent);
+
+        // Act
+        var response = await PutProfileAsync(userClient, new { firstName = "Stale", deleteCurrentImage = true }, staleETag);
+
+        // Assert
+        response.StatusCode.ShouldBe(HttpStatusCode.PreconditionFailed);
+
+        var profile = await userClient.GetAsync($"{TestConstants.IdentityBasePath}/profile");
+        var dto = await profile.DeserializeAsync<UserDto>();
+        dto.ImageUrl.ShouldBe(imageUrl);
+    }
+
     [Fact]
     public async Task UpdateProfile_Should_Succeed_When_IfMatchIsAny()
     {
@@ -341,6 +424,15 @@ private static async Task<string> ReadProfileETagAsync(HttpClient client)
         return response.Headers.ETag.ToString();
     }
 
+    private async Task<HttpResponseMessage> AttemptSignInAsync(string email, string password)
+    {
+        using var client = _factory.CreateClient();
+        using var request = new HttpRequestMessage(HttpMethod.Post, $"{TestConstants.IdentityBasePath}/token/issue");
+        request.Headers.Add("tenant", TestConstants.RootTenantId);
+        request.Content = JsonContent.Create(new { email, password });
+        return await client.SendAsync(request);
+    }
+
     private static async Task<HttpResponseMessage> PutProfileAsync(HttpClient client, object body, string? ifMatch)
     {
         using var request = new HttpRequestMessage(