From d0d85665eba4a0a9760bce9edee3fa58637f89ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Can=20Karag=C3=B6z?= Date: Tue, 27 Oct 2026 22:30:41 +0300 Subject: [PATCH] Fix EF Core transaction execution for user deletion and refresh tokens --- .../Stores/EfCoreRefreshTokenStore.cs | 81 ++++--- .../Services/UserApplicationService.cs | 168 +++++++++---- .../EfCoreTokenStoreTests.cs | 227 +++++++++++++++++- .../Users/UserApplicationServiceTests.cs | 169 +++++++++++++ 4 files changed, 556 insertions(+), 89 deletions(-) diff --git a/src/tokens/CodeBeam.UltimateAuth.Tokens.EntityFrameworkCore/Stores/EfCoreRefreshTokenStore.cs b/src/tokens/CodeBeam.UltimateAuth.Tokens.EntityFrameworkCore/Stores/EfCoreRefreshTokenStore.cs index 328f69eb..2c0744ee 100644 --- a/src/tokens/CodeBeam.UltimateAuth.Tokens.EntityFrameworkCore/Stores/EfCoreRefreshTokenStore.cs +++ b/src/tokens/CodeBeam.UltimateAuth.Tokens.EntityFrameworkCore/Stores/EfCoreRefreshTokenStore.cs @@ -21,66 +21,73 @@ public EfCoreRefreshTokenStore(TDbContext db, TenantExecutionContext tenant) private DbSet DbSet => _db.Set(); // TODO: Review execution strategy retries and commit-unknown scenarios. - // Ensure failed transaction attempts do not retain stale tracked entities, - // and evaluate recovery when the database commit succeeds but the client - // does not receive the replacement refresh token. - public async Task ExecuteAsync(Func action, CancellationToken ct = default) + // Ensure failed transaction attempts do not retain stale tracked entities and evaluate recovery when the database commit succeeds but the client does not receive the replacement refresh token. + public Task ExecuteAsync(Func action, CancellationToken ct = default) { - var strategy = _db.Database.CreateExecutionStrategy(); - - await strategy.ExecuteAsync(async () => - { - await using var tx = await _db.Database.BeginTransactionAsync(ct); + ArgumentNullException.ThrowIfNull(action); - _inTransaction = true; - - try + return ExecuteAsync( + async innerCt => { - await action(ct); - await _db.SaveChangesAsync(ct); - await tx.CommitAsync(ct); - } - catch - { - await tx.RollbackAsync(ct); - throw; - } - finally - { - _inTransaction = false; - } - }); + await action(innerCt); + return null; + }, + ct); } public async Task ExecuteAsync(Func> action, CancellationToken ct = default) { + ArgumentNullException.ThrowIfNull(action); + ct.ThrowIfCancellationRequested(); + + // Participate in the transaction owned by the atomic executor. + if (_db.Database.CurrentTransaction is not null) + return await ExecuteCoreAsync(action, ct); + var strategy = _db.Database.CreateExecutionStrategy(); return await strategy.ExecuteAsync(async () => { - await using var tx = await _db.Database.BeginTransactionAsync(ct); - - _inTransaction = true; + await using var transaction = + await _db.Database.BeginTransactionAsync(ct); try { - var result = await action(ct); - await _db.SaveChangesAsync(ct); - await tx.CommitAsync(ct); + var result = await ExecuteCoreAsync(action, ct); + + await transaction.CommitAsync(ct); + return result; } catch { - await tx.RollbackAsync(ct); + await transaction.RollbackAsync(CancellationToken.None); throw; } - finally - { - _inTransaction = false; - } }); } + private async Task ExecuteCoreAsync(Func> action, CancellationToken ct) + { + if (_inTransaction) + throw new InvalidOperationException("A refresh token store execution is already active."); + + _inTransaction = true; + + try + { + var result = await action(ct); + + await _db.SaveChangesAsync(ct); + + return result; + } + finally + { + _inTransaction = false; + } + } + private void EnsureTransaction() { if (!_inTransaction) diff --git a/src/users/CodeBeam.UltimateAuth.Users.Reference/Services/UserApplicationService.cs b/src/users/CodeBeam.UltimateAuth.Users.Reference/Services/UserApplicationService.cs index f5e72748..12036f54 100644 --- a/src/users/CodeBeam.UltimateAuth.Users.Reference/Services/UserApplicationService.cs +++ b/src/users/CodeBeam.UltimateAuth.Users.Reference/Services/UserApplicationService.cs @@ -222,91 +222,156 @@ public async Task ChangeUserStatusAsync(AccessContext co return await _accessOrchestrator.ExecuteAsync(context, command, ct); } - public async Task DeleteMeAsync(AccessContext context, CancellationToken ct = default) { var command = new AccessCommand(async innerCt => { - var userKey = context.GetTargetUserKey(); - var now = _clock.UtcNow; + await _atomicExecutor.ExecuteAsync(async atomicCt => + { + var userKey = context.GetTargetUserKey(); + var now = _clock.UtcNow; - var lifecycleKey = new UserLifecycleKey(context.ResourceTenant, userKey); - var lifecycleStore = _lifecycleStoreFactory.Create(context.ResourceTenant); - var lifecycle = await lifecycleStore.GetAsync(lifecycleKey, innerCt); + var lifecycleKey = new UserLifecycleKey(context.ResourceTenant, userKey); - if (lifecycle is null) - throw new UAuthNotFoundException(); + var lifecycleStore = _lifecycleStoreFactory.Create(context.ResourceTenant); - var profileStore = _profileStoreFactory.Create(context.ResourceTenant); - var identifierStore = _identifierStoreFactory.Create(context.ResourceTenant); + var lifecycle = await lifecycleStore.GetAsync(lifecycleKey, atomicCt); - await lifecycleStore.DeleteAsync(lifecycleKey, lifecycle.Version, DeleteMode.Soft, now, innerCt); - await identifierStore.DeleteByUserAsync(userKey, DeleteMode.Soft, now, innerCt); + if (lifecycle is null) + throw new UAuthNotFoundException(); - var profiles = await profileStore.GetAllProfilesByUserAsync(userKey, innerCt); + var profileStore = _profileStoreFactory.Create(context.ResourceTenant); - foreach (var profile in profiles) - { - var key = new UserProfileKey(context.ResourceTenant, userKey, profile.ProfileKey); - await profileStore.DeleteAsync(key, profile.Version, DeleteMode.Soft, now, innerCt); - } + var identifierStore = _identifierStoreFactory.Create(context.ResourceTenant); - foreach (var integration in _integrations) - { - await integration.OnUserDeletedAsync(context.ResourceTenant, userKey, DeleteMode.Soft, innerCt); - } + await lifecycleStore.DeleteAsync(lifecycleKey, lifecycle.Version, DeleteMode.Soft, now, atomicCt); + + await identifierStore.DeleteByUserAsync(userKey, DeleteMode.Soft, now, atomicCt); - var sessionStore = _sessionStoreFactory.Create(context.ResourceTenant); - await sessionStore.RevokeAllChainsAsync(userKey, now, innerCt); + var profiles = await profileStore.GetAllProfilesByUserAsync(userKey, atomicCt); + + foreach (var profile in profiles) + { + var key = new UserProfileKey( + context.ResourceTenant, + userKey, + profile.ProfileKey); + + await profileStore.DeleteAsync( + key, + profile.Version, + DeleteMode.Soft, + now, + atomicCt); + } + + foreach (var integration in _integrations) + { + await integration.OnUserDeletedAsync( + context.ResourceTenant, + userKey, + DeleteMode.Soft, + atomicCt); + } + + var sessionStore = _sessionStoreFactory.Create( + context.ResourceTenant); + + await sessionStore.ExecuteAsync( + sessionCt => sessionStore.RevokeAllChainsAsync( + userKey, + now, + sessionCt), + atomicCt); + }, innerCt); }); await _accessOrchestrator.ExecuteAsync(context, command, ct); } - public async Task DeleteUserAsync(AccessContext context, DeleteUserRequest request, CancellationToken ct = default) { var command = new AccessCommand(async innerCt => { - var targetUserKey = context.GetTargetUserKey(); - var now = _clock.UtcNow; - var userLifecycleKey = new UserLifecycleKey(context.ResourceTenant, targetUserKey); - var lifecycleStore = _lifecycleStoreFactory.Create(context.ResourceTenant); - var lifecycle = await lifecycleStore.GetAsync(userLifecycleKey, innerCt); + return await _atomicExecutor.ExecuteAsync(async atomicCt => + { + var targetUserKey = context.GetTargetUserKey(); + var now = _clock.UtcNow; - if (lifecycle is null) - throw new UAuthNotFoundException(); + var userLifecycleKey = new UserLifecycleKey(context.ResourceTenant, targetUserKey); - var profileStore = _profileStoreFactory.Create(context.ResourceTenant); - var identifierStore = _identifierStoreFactory.Create(context.ResourceTenant); + var lifecycleStore = _lifecycleStoreFactory.Create(context.ResourceTenant); - await lifecycleStore.DeleteAsync(userLifecycleKey, lifecycle.Version, request.Mode, now, innerCt); - await identifierStore.DeleteByUserAsync(targetUserKey, request.Mode, now, innerCt); - var profiles = await profileStore.GetAllProfilesByUserAsync(targetUserKey, innerCt); + var lifecycle = await lifecycleStore.GetAsync(userLifecycleKey, atomicCt); - foreach (var profile in profiles) - { - var key = new UserProfileKey(context.ResourceTenant, profile.UserKey, profile.ProfileKey); + if (lifecycle is null) + throw new UAuthNotFoundException(); - await profileStore.DeleteAsync(key, profile.Version, request.Mode, now, innerCt); - } + var profileStore = _profileStoreFactory.Create(context.ResourceTenant); - foreach (var integration in _integrations) - { - await integration.OnUserDeletedAsync(context.ResourceTenant, targetUserKey, request.Mode, innerCt); - } + var identifierStore = _identifierStoreFactory.Create( context.ResourceTenant); + + await lifecycleStore.DeleteAsync( + userLifecycleKey, + lifecycle.Version, + request.Mode, + now, + atomicCt); + + await identifierStore.DeleteByUserAsync( + targetUserKey, + request.Mode, + now, + atomicCt); - var sessionStore = _sessionStoreFactory.Create(context.ResourceTenant); + var profiles = await profileStore.GetAllProfilesByUserAsync( + targetUserKey, + atomicCt); - await sessionStore.RevokeAllChainsAsync(targetUserKey, now, innerCt); + foreach (var profile in profiles) + { + var key = new UserProfileKey( + context.ResourceTenant, + profile.UserKey, + profile.ProfileKey); + + await profileStore.DeleteAsync( + key, + profile.Version, + request.Mode, + now, + atomicCt); + } - return UserDeleteResult.Success(request.Mode); + foreach (var integration in _integrations) + { + await integration.OnUserDeletedAsync( + context.ResourceTenant, + targetUserKey, + request.Mode, + atomicCt); + } + + var sessionStore = _sessionStoreFactory.Create( + context.ResourceTenant); + + await sessionStore.ExecuteAsync( + sessionCt => sessionStore.RevokeAllChainsAsync( + targetUserKey, + now, + sessionCt), + atomicCt); + + return UserDeleteResult.Success(request.Mode); + }, innerCt); }); - return await _accessOrchestrator.ExecuteAsync(context, command, ct); + return await _accessOrchestrator.ExecuteAsync( + context, + command, + ct); } - #endregion @@ -819,6 +884,7 @@ public async Task CheckIdentifierAvailabilityA #endregion + public async Task> QueryUsersAsync(AccessContext context, UserQuery query, CancellationToken ct = default) { var command = new AccessCommand>( diff --git a/tests/CodeBeam.UltimateAuth.Tests.Unit/EntityFrameworkCore/EfCoreTokenStoreTests.cs b/tests/CodeBeam.UltimateAuth.Tests.Unit/EntityFrameworkCore/EfCoreTokenStoreTests.cs index 87445ef0..7e36b9ff 100644 --- a/tests/CodeBeam.UltimateAuth.Tests.Unit/EntityFrameworkCore/EfCoreTokenStoreTests.cs +++ b/tests/CodeBeam.UltimateAuth.Tests.Unit/EntityFrameworkCore/EfCoreTokenStoreTests.cs @@ -1,8 +1,10 @@ -using CodeBeam.UltimateAuth.Core.Domain; +using CodeBeam.UltimateAuth.Core.Abstractions; +using CodeBeam.UltimateAuth.Core.Domain; using CodeBeam.UltimateAuth.Core.MultiTenancy; using CodeBeam.UltimateAuth.Tests.Unit.Helpers; using CodeBeam.UltimateAuth.Tokens.EntityFrameworkCore; using Microsoft.Data.Sqlite; +using Microsoft.Extensions.DependencyInjection; namespace CodeBeam.UltimateAuth.Tests.Unit; @@ -119,4 +121,227 @@ public async Task Store_Outside_Transaction_Should_Throw() await Assert.ThrowsAsync(() => store.StoreAsync(token)); } + + [Fact] + public async Task ExecuteAsync_InsideAtomicExecutor_ShouldJoinTransaction() + { + using var connection = CreateOpenConnection(); + + var tenant = TenantKeys.Single; + const string hash = "atomic-token-commit"; + + await using (var db = CreateDb(connection)) + { + var store = new EfCoreRefreshTokenStore( + db, + new TenantExecutionContext(tenant)); + + await ExecuteAtomicallyAsync(db, async executor => + { + await executor.ExecuteAsync(async ct => + { + Assert.NotNull(db.Database.CurrentTransaction); + + await store.ExecuteAsync(async tokenCt => + { + await store.StoreAsync( + CreateToken(hash, tenant), + tokenCt); + }, ct); + + Assert.NotNull(db.Database.CurrentTransaction); + }); + }); + } + + await using (var db = CreateDb(connection)) + { + var store = new EfCoreRefreshTokenStore( + db, + new TenantExecutionContext(tenant)); + + var persisted = await store.FindByHashAsync(hash); + + Assert.NotNull(persisted); + } + } + + [Fact] + public async Task ExecuteAsync_InsideAtomicExecutor_ShouldRollbackWithOuterTransaction() + { + using var connection = CreateOpenConnection(); + + var tenant = TenantKeys.Single; + const string hash = "atomic-token-rollback"; + + await using (var db = CreateDb(connection)) + { + var store = new EfCoreRefreshTokenStore( + db, + new TenantExecutionContext(tenant)); + + await ExecuteAtomicallyAsync(db, async executor => + { + await Assert.ThrowsAsync( + () => executor.ExecuteAsync(async ct => + { + await store.ExecuteAsync(async tokenCt => + { + await store.StoreAsync( + CreateToken(hash, tenant), + tokenCt); + }, ct); + + throw new InvalidOperationException( + "Simulated outer transaction failure"); + })); + }); + } + + await using (var db = CreateDb(connection)) + { + var store = new EfCoreRefreshTokenStore( + db, + new TenantExecutionContext(tenant)); + + var persisted = await store.FindByHashAsync(hash); + + Assert.Null(persisted); + } + } + + [Fact] + public async Task ExecuteAsync_Generic_InsideAtomicExecutor_ShouldJoinTransaction() + { + using var connection = CreateOpenConnection(); + + var tenant = TenantKeys.Single; + const string hash = "atomic-token-generic"; + + await using (var db = CreateDb(connection)) + { + var store = new EfCoreRefreshTokenStore( + db, + new TenantExecutionContext(tenant)); + + await ExecuteAtomicallyAsync(db, async executor => + { + await executor.ExecuteAsync(async ct => + { + var result = await store.ExecuteAsync(async tokenCt => + { + await store.StoreAsync( + CreateToken(hash, tenant), + tokenCt); + + return 42; + }, ct); + + Assert.Equal(42, result); + }); + }); + } + + await using (var db = CreateDb(connection)) + { + var store = new EfCoreRefreshTokenStore( + db, + new TenantExecutionContext(tenant)); + + Assert.NotNull(await store.FindByHashAsync(hash)); + } + } + + [Fact] + public async Task ExecuteAsync_AfterFailure_ShouldResetExecutionState() + { + using var connection = CreateOpenConnection(); + await using var db = CreateDb(connection); + + var tenant = TenantKeys.Single; + var store = new EfCoreRefreshTokenStore( + db, + new TenantExecutionContext(tenant)); + + await Assert.ThrowsAsync( + () => store.ExecuteAsync(_ => + throw new InvalidOperationException("Simulated failure"))); + + await store.ExecuteAsync(async ct => + { + await store.StoreAsync( + CreateToken("after-failure", tenant), + ct); + }); + + Assert.NotNull( + await store.FindByHashAsync("after-failure")); + } + + [Fact] + public async Task ExecuteAsync_NestedOnSameStore_ShouldThrow() + { + using var connection = CreateOpenConnection(); + await using var db = CreateDb(connection); + + var tenant = TenantKeys.Single; + var store = new EfCoreRefreshTokenStore( + db, + new TenantExecutionContext(tenant)); + + await Assert.ThrowsAsync( + () => store.ExecuteAsync(async ct => + { + await store.ExecuteAsync( + _ => Task.CompletedTask, + ct); + })); + + // The failed nested call must not leave the store locked. + await store.ExecuteAsync(async ct => + { + await store.StoreAsync( + CreateToken("after-nested-failure", tenant), + ct); + }); + + Assert.NotNull( + await store.FindByHashAsync("after-nested-failure")); + } + + private static RefreshToken CreateToken(string hash, TenantKey tenant) + { + AuthSessionId.TryCreate(ValidRaw, out var sessionId); + + var now = DateTimeOffset.UtcNow; + + return RefreshToken.Create( + TokenId.From(Guid.NewGuid()), + hash, + tenant, + UserKey.FromGuid(Guid.NewGuid()), + sessionId, + null, + now, + now.AddHours(1)); + } + + private static async Task ExecuteAtomicallyAsync(UAuthTokenDbContext db, Func action) + { + var services = new ServiceCollection(); + + services.AddSingleton(db); + + CodeBeam.UltimateAuth.EntityFrameworkCore.Extensions + .ServiceCollectionExtensions + .AddUltimateAuthEntityFrameworkCore(services); + + await using var provider = services.BuildServiceProvider(); + await using var scope = provider.CreateAsyncScope(); + + var executor = scope.ServiceProvider + .GetRequiredService(); + + await action(executor); + } } diff --git a/tests/CodeBeam.UltimateAuth.Tests.Unit/Users/UserApplicationServiceTests.cs b/tests/CodeBeam.UltimateAuth.Tests.Unit/Users/UserApplicationServiceTests.cs index 9685b2c7..2087b2c4 100644 --- a/tests/CodeBeam.UltimateAuth.Tests.Unit/Users/UserApplicationServiceTests.cs +++ b/tests/CodeBeam.UltimateAuth.Tests.Unit/Users/UserApplicationServiceTests.cs @@ -447,6 +447,16 @@ public async Task DeleteMeAsync_SoftDeletesUserData_AndRevokesAllChains() Now, It.IsAny()), Times.Once); + + f.Atomic.Verify(x => x.ExecuteAsync( + It.IsAny>(), + It.IsAny()), + Times.Once); + + f.SessionStore.Verify(x => x.ExecuteAsync( + It.IsAny>(), + It.IsAny()), + Times.Once); } [Fact] @@ -576,6 +586,144 @@ await f.Sut.DeleteUserAsync( Now, It.IsAny()), Times.Once); + + f.Atomic.Verify(x => x.ExecuteAsync( + It.IsAny>>(), + It.IsAny()), + Times.Once); + + f.SessionStore.Verify(x => x.ExecuteAsync( + It.IsAny>(), + It.IsAny()), + Times.Once); + + f.SessionStore.Verify(x => x.RevokeAllChainsAsync( + target, + Now, + It.IsAny()), + Times.Once); + } + + [Fact] + public async Task DeleteMeAsync_WhenSessionRevocationFails_PropagatesFailure() + { + var f = CreateFixture(); + var user = UserKey.New(); + + var context = CreateContext( + actorUserKey: user, + targetUserKey: user, + action: UAuthActions.Users.DeleteSelf); + + var lifecycle = UserLifecycle.Create( + context.ResourceTenant, + user, + Now.AddDays(-1)); + + SetupDeleteSelfPersistence(f, user, lifecycle); + + f.SessionStore + .Setup(x => x.RevokeAllChainsAsync( + user, + Now, + It.IsAny())) + .ThrowsAsync(new InvalidOperationException( + "Simulated session revocation failure")); + + var act = () => f.Sut.DeleteMeAsync(context); + + await act.Should() + .ThrowAsync() + .WithMessage("Simulated session revocation failure"); + + f.Atomic.Verify(x => x.ExecuteAsync( + It.IsAny>(), + It.IsAny()), + Times.Once); + + f.SessionStore.Verify(x => x.ExecuteAsync( + It.IsAny>(), + It.IsAny()), + Times.Once); + } + + [Theory] + [InlineData(DeleteMode.Soft)] + [InlineData(DeleteMode.Hard)] + public async Task DeleteUserAsync_WhenSessionRevocationFails_PropagatesFailure(DeleteMode mode) + { + var f = CreateFixture(); + + var actor = UserKey.New(); + var target = UserKey.New(); + + var context = CreateContext( + actorUserKey: actor, + targetUserKey: target, + action: UAuthActions.Users.DeleteAdmin); + + var lifecycle = UserLifecycle.Create( + context.ResourceTenant, + target, + Now.AddDays(-1)); + + f.LifecycleStore + .Setup(x => x.GetAsync( + It.IsAny(), + It.IsAny())) + .ReturnsAsync(lifecycle); + + f.LifecycleStore + .Setup(x => x.DeleteAsync( + It.IsAny(), + lifecycle.Version, + mode, + Now, + It.IsAny())) + .Returns(Task.CompletedTask); + + f.IdentifierStore + .Setup(x => x.DeleteByUserAsync( + target, + mode, + Now, + It.IsAny())) + .Returns(Task.CompletedTask); + + f.ProfileStore + .Setup(x => x.GetAllProfilesByUserAsync( + target, + It.IsAny())) + .ReturnsAsync(Array.Empty()); + + f.SessionStore + .Setup(x => x.RevokeAllChainsAsync( + target, + Now, + It.IsAny())) + .ThrowsAsync(new InvalidOperationException( + "Simulated session revocation failure")); + + var request = new DeleteUserRequest + { + Mode = mode + }; + + var act = () => f.Sut.DeleteUserAsync(context, request); + + await act.Should() + .ThrowAsync() + .WithMessage("Simulated session revocation failure"); + + f.Atomic.Verify(x => x.ExecuteAsync( + It.IsAny>>(), + It.IsAny()), + Times.Once); + + f.SessionStore.Verify(x => x.ExecuteAsync( + It.IsAny>(), + It.IsAny()), + Times.Once); } // ============================================================ @@ -1939,6 +2087,13 @@ private static Fixture CreateFixture(params IUserLifecycleIntegration[] integrat .Setup(x => x.Create(It.IsAny())) .Returns(sessionStore.Object); + sessionStore + .Setup(x => x.ExecuteAsync( + It.IsAny>(), + It.IsAny())) + .Returns, CancellationToken>( + (operation, ct) => operation(ct)); + /* * Unit-test the service command body without mocking its contents. * Authorization itself belongs to IAccessOrchestrator tests. @@ -2011,6 +2166,20 @@ private static Fixture CreateFixture(params IUserLifecycleIntegration[] integrat .Returns>, CancellationToken>( (operation, ct) => operation(ct)); + atomic + .Setup(x => x.ExecuteAsync( + It.IsAny>(), + It.IsAny())) + .Returns, CancellationToken>( + (operation, ct) => operation(ct)); + + atomic + .Setup(x => x.ExecuteAsync( + It.IsAny>>(), + It.IsAny())) + .Returns>, CancellationToken>( + (operation, ct) => operation(ct)); + var options = Options.Create( new UAuthServerOptions());