feat: optimistic concurrency for UserStore.UpdateAsync (#70)

THREAT-MODEL.md

@@ -148,7 +148,9 @@ Stated honestly so you can make an informed risk decision.
   `UpdateAsync` **cannot** be guarded there: ASP.NET Identity drives them through
   anonymous flows (registration, password reset, email confirmation, lockout), so
   their authorization remains the reset/confirmation **token** and the UI-layer guard.
-  A stale full-state `UpdateAsync` overwrite is tracked separately in **#70**.
+  `UpdateAsync` is now protected against silently reverting a concurrent change by an
+  optimistic-concurrency (content-version) check that rejects a stale write while still
+  merging the auto-managed lockout counters forward (#70).
 - **First-run setup is an unauthenticated bootstrap** by design — see the host
   obligation above. The actual `/Setup` page is host-provided and outside this
   library's control.

src/AndreGoepel.Marten.Identity.Abstractions/Users/Events/UserUpdated.cs

@@ -22,4 +22,12 @@ public record UserUpdated(UserId UserId)
     /// cookies stop revalidating, signing the user out everywhere.
     /// </summary>
     public string? SecurityStamp { get; init; }
+
+    /// <summary>
+    /// True when this update only carries auto-managed lockout state (failed-count /
+    /// lockout window) and no user-visible content change. The projection skips bumping
+    /// <see cref="Users.User.ContentVersion" /> for these, so lockout increments do not
+    /// trigger optimistic-concurrency conflicts on the generic update path (#70).
+    /// </summary>
+    public bool LockoutOnly { get; init; }
 }

src/AndreGoepel.Marten.Identity/Users/User.cs

@@ -20,6 +20,14 @@ public Guid StreamId
     public bool RootUser { get; set; }
     public bool Deletable { get; set; } = true;
     public bool Deleted { get; set; }
+
+    /// <summary>
+    /// Optimistic-concurrency token for the user's non-lockout content (#70). Bumped by
+    /// the projection on every content-changing event (create excluded) but <b>not</b> by
+    /// the auto-managed lockout increments, so a stale generic update is detected while
+    /// concurrent failed-login counting never triggers a spurious conflict.
+    /// </summary>
+    public int ContentVersion { get; set; }
     public string? AuthenticatorKey { get; set; }
     public string? RecoveryCodes { get; set; }
 

src/AndreGoepel.Marten.Identity/Users/UserProjection.cs

@@ -75,6 +75,9 @@ public void Apply(UserRestored @event, User user)
 
         if (@event.SecurityStamp is not null)
             user.SecurityStamp = @event.SecurityStamp;
+
+        // Restoring changes content — advance the optimistic-concurrency token (#70).
+        user.ContentVersion++;
     }
 
     [SuppressMessage(
@@ -126,6 +129,13 @@ public void Apply(UserUpdated @event, User user)
 
         user.ChangedBy = @event.UpdatedBy;
         user.ChangedAt = @event.UpdatedAt;
+
+        // Advance the optimistic-concurrency token for genuine content changes only;
+        // lockout-only updates (failed-count / lockout window) must not bump it, or
+        // concurrent failed-login counting would spuriously conflict with a profile
+        // update (#70).
+        if (!@event.LockoutOnly)
+            user.ContentVersion++;
     }
 
     [SuppressMessage(

src/AndreGoepel.Marten.Identity/Users/UserStore.cs

@@ -43,6 +43,15 @@ private static IdentityResult NotAuthorized(string description) =>
             new IdentityError { Code = "NotAuthorized", Description = description }
         );
 
+    private static IdentityResult ConcurrencyFailure() =>
+        IdentityResult.Failed(
+            new IdentityError
+            {
+                Code = "ConcurrencyFailure",
+                Description = "The user was modified concurrently; reload and try again.",
+            }
+        );
+
     public IQueryable<TUser> Users => querySession.Query<TUser>();
 
     public Task<string> GetUserIdAsync(TUser user, CancellationToken cancellationToken) =>
@@ -220,9 +229,23 @@ public async Task<IdentityResult> UpdateAsync(TUser user, CancellationToken canc
                     user.Deletable = false;
             }
 
-            // Nothing changed — release the lock without churning the stream.
+            // Nothing changed — release the lock without churning the stream. Keep the
+            // caller's content version in step with the projection so a follow-up update
+            // on this same instance is not spuriously rejected.
             if (existingUser != null && existingUser.AreEqual(user))
+            {
+                user.ContentVersion = existingUser.ContentVersion;
                 return IdentityResult.Success;
+            }
+
+            // Optimistic concurrency (#70): the non-lockout content is versioned. If it
+            // advanced since the caller loaded, this full-state write is based on a stale
+            // snapshot and would silently revert the concurrent change (e.g. a 2FA toggle
+            // overwritten by a stale profile save) — reject so the caller reloads and
+            // retries. Lockout increments do not bump ContentVersion, so concurrent
+            // failed-login counting never triggers a spurious conflict here.
+            if (existingUser != null && user.ContentVersion != existingUser.ContentVersion)
+                return ConcurrencyFailure();
 
             session.Events.Append(
                 userId.Value,
@@ -244,6 +267,11 @@ public async Task<IdentityResult> UpdateAsync(TUser user, CancellationToken canc
                 }
             );
             await session.SaveChangesAsync(cancellationToken);
+
+            // The inline projection just advanced the persisted content version; mirror it
+            // so repeated updates on the same instance (some ASP.NET Identity flows call
+            // UpdateAsync more than once) keep matching instead of self-conflicting.
+            user.ContentVersion = (existingUser?.ContentVersion ?? 0) + 1;
             return IdentityResult.Success;
         }
         catch (Exception ex)
@@ -893,6 +921,9 @@ CancellationToken cancellationToken
                 LockoutEnabled = current.LockoutEnabled,
                 LockoutEnd = lockoutEnd,
                 AccessFailedCount = accessFailedCount,
+                // Lockout counting is auto-managed; don't advance the content-version
+                // token, or it would spuriously conflict with a concurrent update (#70).
+                LockoutOnly = true,
             }
         );
         await session.SaveChangesAsync(cancellationToken);

tests/AndreGoepel.Marten.Identity.IntegrationTests/Users/UserStoreTests.cs

@@ -384,6 +384,113 @@ public async Task UpdateAsync_ConcurrentWithLockoutIncrements_LosesNoIncrements(
 
     #endregion
 
+    #region Optimistic concurrency (#70)
+
+    [Fact]
+    public async Task UpdateAsync_StaleSnapshot_IsRejectedWithConcurrencyFailure()
+    {
+        // A stale full-state update must not silently revert a change committed after the
+        // caller loaded — it is rejected so the caller reloads and retries.
+        // Arrange
+        var store = UserStoreTestHelpers.BuildStore(fixture.Store);
+        var user = UserStoreTestHelpers.NewUser();
+        await store.CreateAsync(user, Ct);
+        var first = await store.FindByIdAsync(user.Id, Ct);
+        var second = await store.FindByIdAsync(user.Id, Ct); // same content version
+
+        // Act — the first update commits; the second is now stale.
+        first!.PhoneNumber = "+49 111 1111";
+        Assert.True((await store.UpdateAsync(first, Ct)).Succeeded);
+
+        second!.PhoneNumber = "+49 222 2222";
+        var result = await store.UpdateAsync(second, Ct);
+
+        // Assert — rejected, and the first writer's change survives.
+        Assert.False(result.Succeeded);
+        Assert.Contains(result.Errors, e => e.Code == "ConcurrencyFailure");
+        var persisted = await store.FindByIdAsync(user.Id, Ct);
+        Assert.Equal("+49 111 1111", persisted!.PhoneNumber);
+    }
+
+    [Fact]
+    public async Task UpdateAsync_StaleProfileEdit_DoesNotClobberConcurrent2faEnable()
+    {
+        // The headline scenario: a stale profile edit must not turn 2FA back off after it
+        // was enabled concurrently.
+        // Arrange
+        var store = UserStoreTestHelpers.BuildStore(fixture.Store);
+        var user = UserStoreTestHelpers.NewUser();
+        await store.CreateAsync(user, Ct);
+        var editor = await store.FindByIdAsync(user.Id, Ct);
+        var securitySnapshot = await store.FindByIdAsync(user.Id, Ct);
+
+        // Act — 2FA is enabled; then the stale profile edit lands.
+        securitySnapshot!.TwoFactorEnabled = true;
+        Assert.True((await store.UpdateAsync(securitySnapshot, Ct)).Succeeded);
+
+        editor!.PhoneNumber = "+49 555 5555";
+        var result = await store.UpdateAsync(editor, Ct);
+
+        // Assert — the stale edit is rejected and 2FA stays enabled.
+        Assert.False(result.Succeeded);
+        var persisted = await store.FindByIdAsync(user.Id, Ct);
+        Assert.True(persisted!.TwoFactorEnabled);
+    }
+
+    [Fact]
+    public async Task UpdateAsync_SequentialUpdatesOnSameInstance_BothSucceed()
+    {
+        // ASP.NET Identity flows (e.g. ResetAuthenticator) call UpdateAsync more than once
+        // on the same in-memory instance without reloading; the content version must be
+        // refreshed after each write so the second call is not self-rejected.
+        // Arrange
+        var store = UserStoreTestHelpers.BuildStore(fixture.Store);
+        var user = UserStoreTestHelpers.NewUser();
+        await store.CreateAsync(user, Ct);
+        var loaded = await store.FindByIdAsync(user.Id, Ct);
+
+        // Act
+        loaded!.TwoFactorEnabled = true;
+        Assert.True((await store.UpdateAsync(loaded, Ct)).Succeeded);
+        loaded.PhoneNumber = "+49 333 3333";
+        Assert.True((await store.UpdateAsync(loaded, Ct)).Succeeded);
+
+        // Assert
+        var persisted = await store.FindByIdAsync(user.Id, Ct);
+        Assert.True(persisted!.TwoFactorEnabled);
+        Assert.Equal("+49 333 3333", persisted.PhoneNumber);
+    }
+
+    [Fact]
+    public async Task UpdateAsync_AfterConcurrentLockoutIncrement_StillSucceeds()
+    {
+        // A concurrent failed-login increment must not be seen as a content conflict:
+        // lockout-only updates do not advance the content version.
+        // Arrange
+        var store = UserStoreTestHelpers.BuildStore(fixture.Store);
+        var user = UserStoreTestHelpers.NewUser();
+        await store.CreateAsync(user, Ct);
+        var loaded = await store.FindByIdAsync(user.Id, Ct);
+
+        await using (var session = fixture.Store.QuerySession())
+        {
+            var other = await session.LoadAsync<User>(user.UserId.Value, Ct);
+            await store.IncrementAccessFailedCountAsync(other!, Ct);
+        }
+
+        // Act
+        loaded!.PhoneNumber = "+49 444 4444";
+        var result = await store.UpdateAsync(loaded, Ct);
+
+        // Assert — the profile update commits, and the increment is preserved.
+        Assert.True(result.Succeeded);
+        var persisted = await store.FindByIdAsync(user.Id, Ct);
+        Assert.Equal("+49 444 4444", persisted!.PhoneNumber);
+        Assert.Equal(1, persisted.AccessFailedCount);
+    }
+
+    #endregion
+
     #region Security stamp (session invalidation)
 
     [Fact]