diff --git a/src/modules/Elsa.ExternalAuthentication.Persistence.EFCore/Stores/EFCoreExternalIdentityProvisioner.cs b/src/modules/Elsa.ExternalAuthentication.Persistence.EFCore/Stores/EFCoreExternalIdentityProvisioner.cs index 79ed00cce..88decb574 100644 --- a/src/modules/Elsa.ExternalAuthentication.Persistence.EFCore/Stores/EFCoreExternalIdentityProvisioner.cs +++ b/src/modules/Elsa.ExternalAuthentication.Persistence.EFCore/Stores/EFCoreExternalIdentityProvisioner.cs @@ -63,26 +63,42 @@ public sealed class EFCoreExternalIdentityProvisioner( CreatedAt = clock.UtcNow }; + var saveAttempted = false; try { await using var dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); dbContext.ExternalIdentityLinks.Add(link); + saveAttempted = true; await dbContext.SaveChangesAsync(cancellationToken); - await EnsureLinkedUserStillExistsAsync(dbContext, link, user, wasCreated, cancellationToken); - return new ProvisioningResult(user.Id, ToModel(link), wasCreated, true); } catch (DbUpdateException linkException) { // IX_ExternalIdentityLink_Identity arbitrates concurrent first sign-ins for the same identity tuple. - var winner = await FindLinkAsync(request.TenantId, request.ConnectionKey, request.Identity, cancellationToken); + var winner = await FindLinkAsync(request.TenantId, request.ConnectionKey, request.Identity, CancellationToken.None); if (winner?.Id == link.Id) + { + await EnsureLinkedUserStillExistsAsync(link, user, wasCreated, CancellationToken.None); return new ProvisioningResult(user.Id, winner, wasCreated, true); + } if (wasCreated) - await RemoveStrandedUserAsync(user, linkException, cancellationToken); + await RemoveStrandedUserAsync(user, linkException, CancellationToken.None); if (winner is null) throw; return new ProvisioningResult(winner.UserId, winner, false); } + catch (Exception linkException) + { + if (saveAttempted && await LinkExistsAsync(link.Id, CancellationToken.None)) + await EnsureLinkedUserStillExistsAsync(link, user, wasCreated, CancellationToken.None); + else if (wasCreated) + await RemoveStrandedUserAsync(user, linkException, CancellationToken.None); + + throw; + } + + // Once the link is durable, cancellation must not interrupt the complementary user check and cleanup. + await EnsureLinkedUserStillExistsAsync(link, user, wasCreated, CancellationToken.None); + return new ProvisioningResult(user.Id, ToModel(link), wasCreated, true); } public async ValueTask ReplaceAsync(ExternalIdentityLinkReplaceRequest request, CancellationToken cancellationToken = default) @@ -91,12 +107,17 @@ public sealed class EFCoreExternalIdentityProvisioner( var normalizedConnectionKey = ConnectionRevisionCalculator.NormalizeKey(request.ConnectionKey); var subjectHash = handleHasher.Hash(request.Identity.Subject); ExternalIdentityLink? oldLink = null; + PersistedExternalIdentityLink? oldEntity = null; + PersistedExternalIdentityLink? replacementEntity = null; + User? replacementUser = null; + var commitAttempted = false; + var replacementCommitted = false; try { await using var dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); await using var transaction = await dbContext.Database.BeginTransactionAsync(cancellationToken); - var oldEntity = await dbContext.ExternalIdentityLinks.AsNoTracking().SingleOrDefaultAsync( + oldEntity = await dbContext.ExternalIdentityLinks.AsNoTracking().SingleOrDefaultAsync( x => x.Id == request.LinkId && x.TenantId == request.TenantId, cancellationToken); if (oldEntity is null) @@ -115,7 +136,7 @@ public sealed class EFCoreExternalIdentityProvisioner( // The identity aggregate lives in its own store, so the target user is verified through its contract. // Checked here rather than up front to preserve the "unknown link wins over unknown user" result ordering. - var (user, _) = await _userProvisioningService.ResolveAsync( + (replacementUser, _) = await _userProvisioningService.ResolveAsync( new ProvisioningRequest(request.TenantId, normalizedConnectionKey, request.Identity, null, request.UserId), cancellationToken: cancellationToken); @@ -125,37 +146,48 @@ public sealed class EFCoreExternalIdentityProvisioner( if (deleted == 0) return new ExternalIdentityLinkReplaceResult.NotFound(); - var replacementEntity = new PersistedExternalIdentityLink + replacementEntity = new PersistedExternalIdentityLink { Id = identityGenerator.GenerateId(), TenantId = request.TenantId, ConnectionKey = normalizedConnectionKey, Issuer = request.Identity.Issuer, SubjectHash = subjectHash, - UserId = user.Id, + UserId = replacementUser.Id, CreatedAt = clock.UtcNow }; dbContext.ExternalIdentityLinks.Add(replacementEntity); await dbContext.SaveChangesAsync(cancellationToken); - await transaction.CommitAsync(cancellationToken); - if (!await _userProvisioningService.ExistsAsync(user, false, cancellationToken)) - { - await CompensateReplacementAsync(oldEntity, replacementEntity, cancellationToken); - throw new InvalidOperationException("The Elsa user was deleted while its external identity link was being replaced."); - } + commitAttempted = true; + // The replacement write has succeeded; cancellation must not make commit outcome ambiguous. + await transaction.CommitAsync(CancellationToken.None); + replacementCommitted = true; + + await EnsureReplacementUserStillExistsAsync(oldEntity, replacementEntity, replacementUser, CancellationToken.None); return new ExternalIdentityLinkReplaceResult.Success(oldLink, ToModel(replacementEntity)); } - catch (DbUpdateConcurrencyException) + catch (DbUpdateConcurrencyException) when (!commitAttempted) { return new ExternalIdentityLinkReplaceResult.NotFound(); } - catch (DbUpdateException) when (oldLink is not null) + catch (DbUpdateException) when (!commitAttempted && oldLink is not null) { - var conflictingLink = await FindLinkAsync(request.TenantId, normalizedConnectionKey, request.Identity, cancellationToken); + var conflictingLink = await FindLinkAsync(request.TenantId, normalizedConnectionKey, request.Identity, CancellationToken.None); if (conflictingLink is not null && !string.Equals(conflictingLink.Id, request.LinkId, StringComparison.Ordinal)) return new ExternalIdentityLinkReplaceResult.Conflict(oldLink, conflictingLink); throw; } + catch (Exception) when (commitAttempted && !replacementCommitted && oldLink is not null && oldEntity is not null && replacementEntity is not null && replacementUser is not null) + { + // The transaction scope has been disposed before this handler runs, so probing cannot contend with it. + var durableLink = await FindLinkAsync(request.TenantId, normalizedConnectionKey, request.Identity, CancellationToken.None); + if (durableLink?.Id != replacementEntity.Id) + throw; + + replacementCommitted = true; + await EnsureReplacementUserStillExistsAsync(oldEntity, replacementEntity, replacementUser, CancellationToken.None); + return new ExternalIdentityLinkReplaceResult.Success(oldLink, durableLink); + } } public async ValueTask> FindAsync(ExternalIdentityLinkFilter filter, CancellationToken cancellationToken = default) @@ -182,7 +214,6 @@ public sealed class EFCoreExternalIdentityProvisioner( } private async ValueTask EnsureLinkedUserStillExistsAsync( - ExternalAuthenticationElsaDbContext dbContext, PersistedExternalIdentityLink link, User user, bool wasCreated, @@ -191,15 +222,30 @@ public sealed class EFCoreExternalIdentityProvisioner( if (await _userProvisioningService.ExistsAsync(user, wasCreated, cancellationToken)) return; + await using var dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); await dbContext.ExternalIdentityLinks.Where(x => x.Id == link.Id).ExecuteDeleteAsync(cancellationToken); throw new InvalidOperationException("The Elsa user was deleted while its external identity link was being created."); } + private async ValueTask EnsureReplacementUserStillExistsAsync( + PersistedExternalIdentityLink oldLink, + PersistedExternalIdentityLink replacementLink, + User replacementUser, + CancellationToken cancellationToken) + { + if (await _userProvisioningService.ExistsAsync(replacementUser, false, cancellationToken)) + return; + + await CompensateReplacementAsync(oldLink, replacementLink, cancellationToken); + throw new InvalidOperationException("The Elsa user was deleted while its external identity link was being replaced."); + } + private async ValueTask CompensateReplacementAsync( PersistedExternalIdentityLink oldLink, PersistedExternalIdentityLink replacementLink, CancellationToken cancellationToken) { + var commitAttempted = false; try { await using var dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); @@ -218,35 +264,70 @@ public sealed class EFCoreExternalIdentityProvisioner( LastSignedInAt = oldLink.LastSignedInAt }); await dbContext.SaveChangesAsync(cancellationToken); + commitAttempted = true; await transaction.CommitAsync(cancellationToken); - - var previousUser = new User { Id = oldLink.UserId, TenantId = oldLink.TenantId }; - if (!await _userProvisioningService.ExistsAsync(previousUser, false, cancellationToken)) - { - await using var cleanupContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - await cleanupContext.ExternalIdentityLinks.Where(x => x.Id == oldLink.Id).ExecuteDeleteAsync(cancellationToken); - } } catch (Exception compensationException) + { + // The transaction scope has been disposed before this handler runs. A lost commit acknowledgement must + // not turn a successfully restored previous link into data loss. + var restorationCommitted = commitAttempted && + await LinkExistsAsync(oldLink.Id, cancellationToken) && + !await LinkExistsAsync(replacementLink.Id, cancellationToken); + if (!restorationCommitted) + { + await RemoveReplacementLinksOrThrowAsync(oldLink.Id, replacementLink.Id, compensationException, cancellationToken); + throw new InvalidOperationException( + "The replacement link was removed after its target user was deleted, but the previous link could not be restored.", + compensationException); + } + } + + // An indeterminate user-directory failure must not be mistaken for a failed link restoration. + // Only remove the restored link when the directory positively reports that its user is gone. + var previousUser = new User { Id = oldLink.UserId, TenantId = oldLink.TenantId }; + if (!await _userProvisioningService.ExistsAsync(previousUser, false, cancellationToken)) { try { await using var cleanupContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - await cleanupContext.ExternalIdentityLinks - .Where(x => x.Id == replacementLink.Id || x.Id == oldLink.Id) - .ExecuteDeleteAsync(cancellationToken); + await cleanupContext.ExternalIdentityLinks.Where(x => x.Id == oldLink.Id).ExecuteDeleteAsync(cancellationToken); } catch (Exception cleanupException) { - throw new AggregateException( - "A replacement-compensation link refers to a deleted user and could not be removed. No credentials were issued.", - compensationException, + await RemoveReplacementLinksOrThrowAsync(oldLink.Id, replacementLink.Id, cleanupException, cancellationToken); + throw new InvalidOperationException( + "The restored previous link was removed after its user was deleted, but its first cleanup attempt failed.", cleanupException); } + } + } - throw new InvalidOperationException( - "The replacement link was removed after its target user was deleted, but the previous link could not be restored.", - compensationException); + private async ValueTask LinkExistsAsync(string linkId, CancellationToken cancellationToken) + { + await using var dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + return await dbContext.ExternalIdentityLinks.AsNoTracking().AnyAsync(x => x.Id == linkId, cancellationToken); + } + + private async ValueTask RemoveReplacementLinksOrThrowAsync( + string oldLinkId, + string replacementLinkId, + Exception operationException, + CancellationToken cancellationToken) + { + try + { + await using var cleanupContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + await cleanupContext.ExternalIdentityLinks + .Where(x => x.Id == replacementLinkId || x.Id == oldLinkId) + .ExecuteDeleteAsync(cancellationToken); + } + catch (Exception cleanupException) + { + throw new AggregateException( + "A replacement-compensation link refers to a deleted user and could not be removed. No credentials were issued.", + operationException, + cleanupException); } } diff --git a/src/modules/Elsa.ExternalAuthentication/Services/ExternalIdentityUserProvisioningService.cs b/src/modules/Elsa.ExternalAuthentication/Services/ExternalIdentityUserProvisioningService.cs index 0bc3e1e6b..e536e4cd6 100644 --- a/src/modules/Elsa.ExternalAuthentication/Services/ExternalIdentityUserProvisioningService.cs +++ b/src/modules/Elsa.ExternalAuthentication/Services/ExternalIdentityUserProvisioningService.cs @@ -64,6 +64,9 @@ public sealed class ExternalIdentityUserProvisioningService( } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { + var persistedUser = await userStore.FindAsync(new UserFilter { Id = user.Id }, CancellationToken.None); + if (persistedUser is not null) + await userStore.DeleteAsync(new UserFilter { Id = user.Id }, CancellationToken.None); throw; } catch diff --git a/src/modules/Elsa.ExternalAuthentication/Services/InMemoryExternalIdentityProvisioner.cs b/src/modules/Elsa.ExternalAuthentication/Services/InMemoryExternalIdentityProvisioner.cs index d61d12508..4c5799a75 100644 --- a/src/modules/Elsa.ExternalAuthentication/Services/InMemoryExternalIdentityProvisioner.cs +++ b/src/modules/Elsa.ExternalAuthentication/Services/InMemoryExternalIdentityProvisioner.cs @@ -66,7 +66,7 @@ public sealed class InMemoryExternalIdentityProvisioner( clock.UtcNow, null); state.Links[key] = link; - if (!await _userProvisioningService.ExistsAsync(user, wasCreated, cancellationToken)) + if (!await _userProvisioningService.ExistsAsync(user, wasCreated, CancellationToken.None)) { state.Links.Remove(key); throw new InvalidOperationException("The Elsa user was deleted while its external identity link was being created."); @@ -114,10 +114,13 @@ public sealed class InMemoryExternalIdentityProvisioner( null); state.Links.Remove(oldEntry.Key); state.Links[replacementKey] = replacement; - if (!await _userProvisioningService.ExistsAsync(user, false, cancellationToken)) + if (!await _userProvisioningService.ExistsAsync(user, false, CancellationToken.None)) { state.Links.Remove(replacementKey); state.Links[oldEntry.Key] = oldEntry.Value; + var previousUser = new User { Id = oldEntry.Value.UserId, TenantId = oldEntry.Value.TenantId }; + if (!await _userProvisioningService.ExistsAsync(previousUser, false, CancellationToken.None)) + state.Links.Remove(oldEntry.Key); throw new InvalidOperationException("The Elsa user was deleted while its external identity link was being replaced."); } return new ExternalIdentityLinkReplaceResult.Success(oldEntry.Value, replacement); diff --git a/src/modules/Elsa.Identity/Services/UserDeletionCoordinator.cs b/src/modules/Elsa.Identity/Services/UserDeletionCoordinator.cs index da37cef7a..753d90c85 100644 --- a/src/modules/Elsa.Identity/Services/UserDeletionCoordinator.cs +++ b/src/modules/Elsa.Identity/Services/UserDeletionCoordinator.cs @@ -32,13 +32,13 @@ public sealed class UserDeletionCoordinator( } catch { - await userStore.SaveAsync(user, cancellationToken); + await userStore.SaveAsync(user, CancellationToken.None); throw; } if (dependencies.Count > 0) { - await userStore.SaveAsync(user, cancellationToken); + await userStore.SaveAsync(user, CancellationToken.None); return new UserDeletionOperationResult.Blocked(dependencies); } diff --git a/test/integration/Elsa.ExternalAuthentication.IntegrationTests/Persistence/ExternalAuthenticationPersistenceTests.cs b/test/integration/Elsa.ExternalAuthentication.IntegrationTests/Persistence/ExternalAuthenticationPersistenceTests.cs index 82ca84f4f..7af5acfaa 100644 --- a/test/integration/Elsa.ExternalAuthentication.IntegrationTests/Persistence/ExternalAuthenticationPersistenceTests.cs +++ b/test/integration/Elsa.ExternalAuthentication.IntegrationTests/Persistence/ExternalAuthenticationPersistenceTests.cs @@ -1,3 +1,4 @@ +using System.Data.Common; using System.Text.Json; using Elsa.Common; using Elsa.Common.Services; @@ -311,6 +312,27 @@ public sealed class ExternalAuthenticationPersistenceTests : IAsyncLifetime Assert.Empty(await _userStore.FindManyAsync(new UserFilter())); } + [Fact] + public async Task ProvisionerRemovesTheJustInTimeUserWhenPublicationIsCancelled() + { + using var cancellationTokenSource = new CancellationTokenSource(); + var users = new CancelAfterSaveUserStore(new MemoryUserStore(new MemoryStore()), cancellationTokenSource); + using var hasher = new HmacExternalAuthenticationHandleHasher(); + var provisioner = CreateProvisioner(hasher, userStore: users); + var request = new ProvisioningRequest( + "tenant-a", + "connection-a", + new ExternalIdentity("https://issuer.example", "subject-cancelled-publication", EmptyClaims), + new UserCreationProposal("external")); + + await Assert.ThrowsAnyAsync(() => + provisioner.CreateLinkOrGetExistingAsync(request, cancellationTokenSource.Token).AsTask()); + + Assert.Empty(await users.FindManyAsync(new UserFilter())); + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + Assert.Empty(await dbContext.ExternalIdentityLinks.ToListAsync()); + } + [Fact] public async Task ProvisionerFailsWhenAJustInTimeUserCannotBeCompensated() { @@ -360,6 +382,30 @@ public sealed class ExternalAuthenticationPersistenceTests : IAsyncLifetime Assert.Empty(await dbContext.ExternalIdentityLinks.ToListAsync()); } + [Fact] + public async Task ProvisionerReconcilesAmbiguousPublicationForExistingUser() + { + await _userStore.SaveAsync(new User { Id = "user-a", Name = "alice", TenantId = "tenant-a" }); + var options = new DbContextOptionsBuilder() + .UseSqlite(_connection) + .AddInterceptors(new DeleteLinkedUserAfterSaveAndThrowInterceptor(_userStore)) + .Options; + using var hasher = new HmacExternalAuthenticationHandleHasher(); + var provisioner = CreateProvisioner(hasher, new TestDbContextFactory(options, _services)); + var request = new ProvisioningRequest( + "tenant-a", + "connection-a", + new ExternalIdentity("https://issuer.example", "subject-ambiguous-existing-user", EmptyClaims), + null, + "user-a"); + + await Assert.ThrowsAsync(() => provisioner.CreateLinkOrGetExistingAsync(request).AsTask()); + + Assert.Null(await _userStore.FindAsync(new UserFilter { Id = "user-a" })); + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + Assert.Empty(await dbContext.ExternalIdentityLinks.ToListAsync()); + } + [Fact] public async Task ProvisionerAtomicallyReplacesLinksAndPreservesTheOldLinkOnConflict() { @@ -453,6 +499,150 @@ public sealed class ExternalAuthenticationPersistenceTests : IAsyncLifetime Assert.Empty(await dbContext.ExternalIdentityLinks.ToListAsync()); } + [Fact] + public async Task ProvisionerPreservesRestoredLinkWhenPreviousUserLookupFails() + { + await _userStore.SaveAsync(new User { Id = "user-a", Name = "alice", TenantId = "tenant-a" }); + await _userStore.SaveAsync(new User { Id = "user-b", Name = "bob", TenantId = "tenant-a" }); + using var hasher = new HmacExternalAuthenticationHandleHasher(); + var originalProvisioner = CreateProvisioner(hasher); + var old = (await originalProvisioner.CreateLinkOrGetExistingAsync( + new ProvisioningRequest("tenant-a", "contoso", new ExternalIdentity("https://issuer.example", "subject-old", EmptyClaims), null, "user-a"))).Link; + var racingProvider = new DeleteThenThrowUserProvider(new StoreBasedUserProvider(_userStore), _userStore); + var racingProvisioner = CreateProvisioner(hasher, userProvider: racingProvider); + + var exception = await Assert.ThrowsAsync(() => racingProvisioner.ReplaceAsync( + new ExternalIdentityLinkReplaceRequest( + "tenant-a", + old.Id, + "user-b", + "contoso", + new ExternalIdentity("https://issuer.example", "subject-new", EmptyClaims))).AsTask()); + + Assert.Contains("lookup failure", exception.Message, StringComparison.Ordinal); + Assert.NotNull(await _userStore.FindAsync(new UserFilter { Id = "user-a" })); + Assert.Null(await _userStore.FindAsync(new UserFilter { Id = "user-b" })); + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + var durableLink = Assert.Single(await dbContext.ExternalIdentityLinks.ToListAsync()); + Assert.Equal(old.Id, durableLink.Id); + Assert.Equal("user-a", durableLink.UserId); + } + + [Fact] + public async Task ProvisionerFallsBackWhenInvalidRestoredLinkCleanupFailsOnce() + { + await _userStore.SaveAsync(new User { Id = "user-a", Name = "alice", TenantId = "tenant-a" }); + await _userStore.SaveAsync(new User { Id = "user-b", Name = "bob", TenantId = "tenant-a" }); + var options = new DbContextOptionsBuilder() + .UseSqlite(_connection) + .AddInterceptors(new FailSelectedLinkDeleteInterceptor(3)) + .Options; + using var hasher = new HmacExternalAuthenticationHandleHasher(); + var factory = new TestDbContextFactory(options, _services); + var originalProvisioner = CreateProvisioner(hasher, factory); + var old = (await originalProvisioner.CreateLinkOrGetExistingAsync( + new ProvisioningRequest("tenant-a", "contoso", new ExternalIdentity("https://issuer.example", "subject-old", EmptyClaims), null, "user-a"))).Link; + var racingProvider = new DeleteOnSelectedFindUserProvider(new StoreBasedUserProvider(_userStore), _userStore, 2, 3); + var racingProvisioner = CreateProvisioner(hasher, factory, userProvider: racingProvider); + + await Assert.ThrowsAsync(() => racingProvisioner.ReplaceAsync( + new ExternalIdentityLinkReplaceRequest( + "tenant-a", + old.Id, + "user-b", + "contoso", + new ExternalIdentity("https://issuer.example", "subject-new", EmptyClaims))).AsTask()); + + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + Assert.Empty(await dbContext.ExternalIdentityLinks.ToListAsync()); + } + + [Fact] + public async Task ProvisionerDoesNotMisclassifyPostCommitUserLookupFailureAsConflict() + { + await _userStore.SaveAsync(new User { Id = "user-a", Name = "alice", TenantId = "tenant-a" }); + await _userStore.SaveAsync(new User { Id = "user-b", Name = "bob", TenantId = "tenant-a" }); + using var hasher = new HmacExternalAuthenticationHandleHasher(); + var originalProvisioner = CreateProvisioner(hasher); + var old = (await originalProvisioner.CreateLinkOrGetExistingAsync( + new ProvisioningRequest("tenant-a", "contoso", new ExternalIdentity("https://issuer.example", "subject-old", EmptyClaims), null, "user-a"))).Link; + var failingProvider = new ThrowOnSelectedFindUserProvider(new StoreBasedUserProvider(_userStore), 2); + var failingProvisioner = CreateProvisioner(hasher, userProvider: failingProvider); + + await Assert.ThrowsAsync(() => failingProvisioner.ReplaceAsync( + new ExternalIdentityLinkReplaceRequest( + "tenant-a", + old.Id, + "user-b", + "contoso", + new ExternalIdentity("https://issuer.example", "subject-new", EmptyClaims))).AsTask()); + + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + var durableLink = Assert.Single(await dbContext.ExternalIdentityLinks.ToListAsync()); + Assert.NotEqual(old.Id, durableLink.Id); + Assert.Equal("user-b", durableLink.UserId); + } + + [Fact] + public async Task ProvisionerReconcilesReplacementWhenCommitAcknowledgementIsLost() + { + await _userStore.SaveAsync(new User { Id = "user-a", Name = "alice", TenantId = "tenant-a" }); + await _userStore.SaveAsync(new User { Id = "user-b", Name = "bob", TenantId = "tenant-a" }); + using var hasher = new HmacExternalAuthenticationHandleHasher(); + var originalProvisioner = CreateProvisioner(hasher); + var old = (await originalProvisioner.CreateLinkOrGetExistingAsync( + new ProvisioningRequest("tenant-a", "contoso", new ExternalIdentity("https://issuer.example", "subject-old", EmptyClaims), null, "user-a"))).Link; + var options = new DbContextOptionsBuilder() + .UseSqlite(_connection) + .AddInterceptors(new ThrowAfterSelectedCommitInterceptor(1)) + .Options; + var provisioner = CreateProvisioner(hasher, new TestDbContextFactory(options, _services)); + + var result = Assert.IsType(await provisioner.ReplaceAsync( + new ExternalIdentityLinkReplaceRequest( + "tenant-a", + old.Id, + "user-b", + "contoso", + new ExternalIdentity("https://issuer.example", "subject-new", EmptyClaims)))); + + Assert.NotEqual(old.Id, result.NewLink.Id); + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + var durableLink = Assert.Single(await dbContext.ExternalIdentityLinks.ToListAsync()); + Assert.Equal(result.NewLink.Id, durableLink.Id); + Assert.Equal("user-b", durableLink.UserId); + } + + [Fact] + public async Task ProvisionerPreservesRestoredLinkWhenCompensationCommitAcknowledgementIsLost() + { + await _userStore.SaveAsync(new User { Id = "user-a", Name = "alice", TenantId = "tenant-a" }); + await _userStore.SaveAsync(new User { Id = "user-b", Name = "bob", TenantId = "tenant-a" }); + using var hasher = new HmacExternalAuthenticationHandleHasher(); + var originalProvisioner = CreateProvisioner(hasher); + var old = (await originalProvisioner.CreateLinkOrGetExistingAsync( + new ProvisioningRequest("tenant-a", "contoso", new ExternalIdentity("https://issuer.example", "subject-old", EmptyClaims), null, "user-a"))).Link; + var options = new DbContextOptionsBuilder() + .UseSqlite(_connection) + .AddInterceptors(new ThrowAfterSelectedCommitInterceptor(2)) + .Options; + var racingProvider = new DeleteOnSelectedFindUserProvider(new StoreBasedUserProvider(_userStore), _userStore, 2); + var provisioner = CreateProvisioner(hasher, new TestDbContextFactory(options, _services), userProvider: racingProvider); + + await Assert.ThrowsAsync(() => provisioner.ReplaceAsync( + new ExternalIdentityLinkReplaceRequest( + "tenant-a", + old.Id, + "user-b", + "contoso", + new ExternalIdentity("https://issuer.example", "subject-new", EmptyClaims))).AsTask()); + + await using var dbContext = await _dbContextFactory.CreateDbContextAsync(); + var durableLink = Assert.Single(await dbContext.ExternalIdentityLinks.ToListAsync()); + Assert.Equal(old.Id, durableLink.Id); + Assert.Equal("user-a", durableLink.UserId); + } + [Fact] public async Task DurableConcurrentReplacementUsesTheOldLinkIdAsAnAtomicGuard() { @@ -597,6 +787,20 @@ public sealed class ExternalAuthenticationPersistenceTests : IAsyncLifetime } } + private sealed class DeleteLinkedUserAfterSaveAndThrowInterceptor(IUserStore users) : SaveChangesInterceptor + { + public override async ValueTask SavedChangesAsync( + SaveChangesCompletedEventData eventData, + int result, + CancellationToken cancellationToken = default) + { + var userId = eventData.Context!.ChangeTracker.Entries() + .Single().Entity.UserId; + await users.DeleteAsync(new UserFilter { Id = userId }, CancellationToken.None); + throw new InvalidOperationException("Simulated ambiguous post-save failure."); + } + } + private sealed class DeleteFailingUserStore(IUserStore inner) : IUserStore { public Task SaveAsync(User user, CancellationToken cancellationToken = default) => inner.SaveAsync(user, cancellationToken); @@ -605,6 +809,53 @@ public sealed class ExternalAuthenticationPersistenceTests : IAsyncLifetime public Task FindAsync(UserFilter filter, CancellationToken cancellationToken = default) => inner.FindAsync(filter, cancellationToken); } + private sealed class CancelAfterSaveUserStore(IUserStore inner, CancellationTokenSource cancellationTokenSource) : IUserStore + { + public async Task SaveAsync(User user, CancellationToken cancellationToken = default) + { + await inner.SaveAsync(user, cancellationToken); + cancellationTokenSource.Cancel(); + } + + public Task DeleteAsync(UserFilter filter, CancellationToken cancellationToken = default) => inner.DeleteAsync(filter, cancellationToken); + public Task> FindManyAsync(UserFilter filter, CancellationToken cancellationToken = default) => inner.FindManyAsync(filter, cancellationToken); + public Task FindAsync(UserFilter filter, CancellationToken cancellationToken = default) => inner.FindAsync(filter, cancellationToken); + } + + private sealed class FailSelectedLinkDeleteInterceptor(int failureCount) : DbCommandInterceptor + { + private int _deleteCount; + + public override ValueTask> NonQueryExecutingAsync( + DbCommand command, + CommandEventData eventData, + InterceptionResult result, + CancellationToken cancellationToken = default) + { + if (command.CommandText.Contains("DELETE FROM \"ExternalIdentityLinks\"", StringComparison.Ordinal) && + Interlocked.Increment(ref _deleteCount) == failureCount) + throw new InvalidOperationException("Simulated external identity link cleanup failure."); + + return ValueTask.FromResult(result); + } + } + + private sealed class ThrowAfterSelectedCommitInterceptor(int failureCount) : DbTransactionInterceptor + { + private int _commitCount; + + public override Task TransactionCommittedAsync( + DbTransaction transaction, + TransactionEndEventData eventData, + CancellationToken cancellationToken = default) + { + if (Interlocked.Increment(ref _commitCount) == failureCount) + throw new InvalidOperationException("Simulated lost transaction commit acknowledgement."); + + return Task.CompletedTask; + } + } + private sealed class DeleteOnSelectedFindUserProvider(IUserProvider inner, IUserStore users, params int[] deletionCounts) : IUserProvider { private readonly HashSet _deletionCounts = deletionCounts.ToHashSet(); @@ -622,4 +873,38 @@ public sealed class ExternalAuthenticationPersistenceTests : IAsyncLifetime return user; } } + + private sealed class DeleteThenThrowUserProvider(IUserProvider inner, IUserStore users) : IUserProvider + { + private int _findCount; + + public async Task FindAsync(UserFilter filter, CancellationToken cancellationToken = default) + { + var user = await inner.FindAsync(filter, cancellationToken); + var findCount = Interlocked.Increment(ref _findCount); + if (user is not null && findCount == 2) + { + await users.DeleteAsync(new UserFilter { Id = user.Id }, cancellationToken); + return null; + } + + if (findCount == 3) + throw new InvalidOperationException("Simulated user-directory lookup failure."); + + return user; + } + } + + private sealed class ThrowOnSelectedFindUserProvider(IUserProvider inner, int failureCount) : IUserProvider + { + private int _findCount; + + public async Task FindAsync(UserFilter filter, CancellationToken cancellationToken = default) + { + var user = await inner.FindAsync(filter, cancellationToken); + if (Interlocked.Increment(ref _findCount) == failureCount) + throw new DbUpdateException("Simulated post-commit user-directory failure."); + return user; + } + } } diff --git a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/InMemoryExternalIdentityProvisionerTests.cs b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/InMemoryExternalIdentityProvisionerTests.cs index bf1eb63bc..da297f283 100644 --- a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/InMemoryExternalIdentityProvisionerTests.cs +++ b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/InMemoryExternalIdentityProvisionerTests.cs @@ -1,4 +1,5 @@ using Elsa.Common.Services; +using Elsa.ExternalAuthentication.Contracts; using Elsa.ExternalAuthentication.Models; using Elsa.ExternalAuthentication.Services; using Elsa.Identity.Contracts; @@ -41,6 +42,47 @@ public class InMemoryExternalIdentityProvisionerTests Assert.Empty((await provisioner.FindAsync(new ExternalIdentityLinkFilter { TenantId = "tenant-a" })).Items); } + [Fact] + public async Task RemovesRestoredLinkWhenBothReplacementUsersAreDeleted() + { + var users = new MemoryUserStore(new MemoryStore()); + await users.SaveAsync(new User { Id = "user-a", Name = "alice", TenantId = "tenant-a" }); + await users.SaveAsync(new User { Id = "user-b", Name = "bob", TenantId = "tenant-a" }); + using var hasher = new HmacExternalAuthenticationHandleHasher(); + var state = new InMemoryExternalIdentityProvisionerState(); + var originalProvisioner = CreateProvisioner(users, new StoreBasedUserProvider(users), hasher, state); + var identity = new ExternalIdentity("https://issuer.example", "subject-a", new Dictionary>()); + var oldLink = (await originalProvisioner.CreateLinkOrGetExistingAsync( + new ProvisioningRequest("tenant-a", "contoso", identity, null, "user-a"))).Link; + var racingProvider = new DeleteOnSelectedFindUserProvider(new StoreBasedUserProvider(users), users, 2, 3); + var racingProvisioner = CreateProvisioner(users, racingProvider, hasher, state); + + await Assert.ThrowsAsync(() => racingProvisioner.ReplaceAsync( + new ExternalIdentityLinkReplaceRequest( + "tenant-a", + oldLink.Id, + "user-b", + "contoso", + new ExternalIdentity("https://issuer.example", "subject-b", new Dictionary>()))).AsTask()); + + Assert.Null(await users.FindAsync(new UserFilter { Id = "user-a" })); + Assert.Null(await users.FindAsync(new UserFilter { Id = "user-b" })); + Assert.Empty((await racingProvisioner.FindAsync(new ExternalIdentityLinkFilter { TenantId = "tenant-a" })).Items); + } + + private static InMemoryExternalIdentityProvisioner CreateProvisioner( + IUserStore users, + IUserProvider provider, + IExternalAuthenticationHandleHasher hasher, + InMemoryExternalIdentityProvisionerState state) => new( + users, + provider, + Substitute.For(), + new GuidIdentityGenerator(), + new TestSystemClock(DateTimeOffset.UtcNow), + hasher, + state); + private sealed class DeleteAfterResolveUserProvider(IUserProvider inner, IUserStore users) : IUserProvider { private int _findCount; @@ -53,4 +95,22 @@ public class InMemoryExternalIdentityProvisionerTests return user; } } + + private sealed class DeleteOnSelectedFindUserProvider(IUserProvider inner, IUserStore users, params int[] deletionCounts) : IUserProvider + { + private readonly HashSet _deletionCounts = deletionCounts.ToHashSet(); + private int _findCount; + + public async Task FindAsync(UserFilter filter, CancellationToken cancellationToken = default) + { + var user = await inner.FindAsync(filter, cancellationToken); + if (user is not null && _deletionCounts.Contains(Interlocked.Increment(ref _findCount))) + { + await users.DeleteAsync(new UserFilter { Id = user.Id }, cancellationToken); + return null; + } + + return user; + } + } } diff --git a/test/unit/Elsa.Identity.UnitTests/Services/UserDeletionCoordinatorTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/UserDeletionCoordinatorTests.cs new file mode 100644 index 000000000..a259e8d32 --- /dev/null +++ b/test/unit/Elsa.Identity.UnitTests/Services/UserDeletionCoordinatorTests.cs @@ -0,0 +1,72 @@ +using Elsa.Identity.Contracts; +using Elsa.Identity.Entities; +using Elsa.Identity.Models; +using Elsa.Identity.Services; + +namespace Elsa.Identity.UnitTests.Services; + +public class UserDeletionCoordinatorTests +{ + [Fact] + public async Task CancellationAfterDeletionDoesNotCancelUserRestoration() + { + using var cancellationTokenSource = new CancellationTokenSource(); + var user = new User { Id = "user-a", Name = "alice", TenantId = "tenant-a" }; + var store = new CancellationAwareUserStore(user); + var coordinator = new UserDeletionCoordinator(store, [new CancelAfterDeletionContributor(cancellationTokenSource)]); + + await Assert.ThrowsAnyAsync(() => + coordinator.DeleteAsync(user.Id, cancellationTokenSource.Token).AsTask()); + + Assert.Same(user, store.User); + Assert.Equal(CancellationToken.None, store.RestorationToken); + } + + private sealed class CancelAfterDeletionContributor(CancellationTokenSource cancellationTokenSource) : IUserDeletionDependencyContributor + { + private int _inspectionCount; + + public string Source => "test"; + + public ValueTask InspectAsync(User user, CancellationToken cancellationToken = default) + { + if (Interlocked.Increment(ref _inspectionCount) == 2) + { + cancellationTokenSource.Cancel(); + throw new OperationCanceledException(cancellationTokenSource.Token); + } + + return ValueTask.FromResult(null); + } + } + + private sealed class CancellationAwareUserStore(User user) : IUserStore + { + public User? User { get; private set; } = user; + public CancellationToken? RestorationToken { get; private set; } + + public Task SaveAsync(User userToSave, CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + User = userToSave; + RestorationToken = cancellationToken; + return Task.CompletedTask; + } + + public Task DeleteAsync(UserFilter filter, CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + User = null; + return Task.CompletedTask; + } + + public Task> FindManyAsync(UserFilter filter, CancellationToken cancellationToken = default) => + Task.FromResult(User is null ? Enumerable.Empty() : [User]); + + public Task FindAsync(UserFilter filter, CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); + return Task.FromResult(User); + } + } +}