From f4762308a269dc6a616dd36d429db0f814fcb6e2 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sun, 13 Sep 2026 02:56:01 -0700 Subject: [PATCH] fix(persistence): honor tenant isolation in Memory workflow stores (#8100) * fix(persistence): honor tenant isolation in Memory workflow stores Apply the EF SetTenantIdFilter admission rule on Memory definition, trigger, and bookmark query paths so ambient tenant and TenantAgnostic match IgnoreQueryFilters instead of leaking cross-tenant rows. Co-authored-by: Sipke Schoorstra * fix(persistence): stop Memory definition delete from wiping other tenants DeleteAsync collected logical DefinitionIds from tenant-visible rows, then removed every in-memory row with those IDs. Shared DefinitionIds across tenants therefore deleted tenant B when tenant A deleted. Keep the all-versions-of-DefinitionId Memory delete, but apply the same TenantVisibility rule (or TenantAgnostic bypass) to the final removal. Co-authored-by: Sipke Schoorstra --------- Co-authored-by: Cursor Agent --- .../Multitenancy/TenantVisibility.cs | 35 ++++ .../Stores/MemoryWorkflowDefinitionStore.cs | 27 ++- .../Stores/MemoryBookmarkStore.cs | 12 +- .../Stores/MemoryTriggerStore.cs | 19 +- .../Multitenancy/TenantVisibilityTests.cs | 79 ++++++++ ...flowDefinitionStoreTenantIsolationTests.cs | 179 ++++++++++++++++++ ...MemoryBookmarkStoreTenantIsolationTests.cs | 114 +++++++++++ .../MemoryTriggerStoreTenantIsolationTests.cs | 107 +++++++++++ .../Stores/MemoryTriggerStoreTests.cs | 2 +- 9 files changed, 561 insertions(+), 13 deletions(-) create mode 100644 src/modules/Elsa.Common/Multitenancy/TenantVisibility.cs create mode 100644 test/unit/Elsa.Common.UnitTests/Multitenancy/TenantVisibilityTests.cs create mode 100644 test/unit/Elsa.Workflows.Management.UnitTests/Stores/MemoryWorkflowDefinitionStoreTenantIsolationTests.cs create mode 100644 test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryBookmarkStoreTenantIsolationTests.cs create mode 100644 test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryTriggerStoreTenantIsolationTests.cs diff --git a/src/modules/Elsa.Common/Multitenancy/TenantVisibility.cs b/src/modules/Elsa.Common/Multitenancy/TenantVisibility.cs new file mode 100644 index 000000000..a6b602367 --- /dev/null +++ b/src/modules/Elsa.Common/Multitenancy/TenantVisibility.cs @@ -0,0 +1,35 @@ +using Elsa.Common.Entities; + +namespace Elsa.Common.Multitenancy; + +/// +/// Memory-store counterpart of EF Core SetTenantIdFilter. +/// A row is visible when its matches the ambient tenant, +/// is , or is null while the ambient tenant is the default. +/// +public static class TenantVisibility +{ + /// + /// Returns whether is visible under . + /// + public static bool IsVisible(string? entityTenantId, string ambientTenantId) => + entityTenantId == ambientTenantId + || entityTenantId == Tenant.AgnosticTenantId + || entityTenantId is null && ambientTenantId == Tenant.DefaultTenantId; + + /// + /// Restricts to rows visible to , + /// unless is set (EF IgnoreQueryFilters). + /// + public static IQueryable WhereVisibleToTenant(this IQueryable queryable, string ambientTenantId, bool tenantAgnostic = false) + where T : Entity + { + if (tenantAgnostic) + return queryable; + + return queryable.Where(entity => + entity.TenantId == ambientTenantId + || entity.TenantId == Tenant.AgnosticTenantId + || entity.TenantId == null && ambientTenantId == Tenant.DefaultTenantId); + } +} diff --git a/src/modules/Elsa.Workflows.Management/Stores/MemoryWorkflowDefinitionStore.cs b/src/modules/Elsa.Workflows.Management/Stores/MemoryWorkflowDefinitionStore.cs index ad702e3c1..d55d76123 100644 --- a/src/modules/Elsa.Workflows.Management/Stores/MemoryWorkflowDefinitionStore.cs +++ b/src/modules/Elsa.Workflows.Management/Stores/MemoryWorkflowDefinitionStore.cs @@ -1,4 +1,5 @@ using Elsa.Common.Models; +using Elsa.Common.Multitenancy; using Elsa.Common.Services; using Elsa.Extensions; using Elsa.Workflows.Management.Entities; @@ -10,7 +11,7 @@ namespace Elsa.Workflows.Management.Stores; /// /// A memory implementation of . /// -public class MemoryWorkflowDefinitionStore(MemoryStore store) : IWorkflowDefinitionStore +public class MemoryWorkflowDefinitionStore(MemoryStore store, ITenantAccessor? tenantAccessor = null) : IWorkflowDefinitionStore { /// public Task FindAsync(WorkflowDefinitionFilter filter, CancellationToken cancellationToken = default) @@ -147,7 +148,9 @@ public class MemoryWorkflowDefinitionStore(MemoryStore store lock (store.Sync) { var workflowDefinitionIds = store.Query(query => Filter(query, filter)).Select(x => x.DefinitionId).Distinct().ToList(); - store.DeleteWhere(x => workflowDefinitionIds.Contains(x.DefinitionId)); + store.DeleteWhere(x => + workflowDefinitionIds.Contains(x.DefinitionId) + && (filter.TenantAgnostic || TenantVisibility.IsVisible(x.TenantId, CurrentTenantId))); return Task.FromResult(workflowDefinitionIds.LongCount()); } } @@ -162,17 +165,31 @@ public class MemoryWorkflowDefinitionStore(MemoryStore store /// public Task CountDistinctAsync(CancellationToken cancellationToken = default) { - return Task.FromResult(store.Count(x => true, x => x.DefinitionId)); + var count = store.Query(query => query.WhereVisibleToTenant(CurrentTenantId)) + .Select(x => x.DefinitionId) + .Distinct() + .LongCount(); + return Task.FromResult(count); } /// public Task GetIsNameUnique(string name, string? definitionId = default, CancellationToken cancellationToken = default) { - var exists = store.Any(x => x.Name == name && x.DefinitionId != definitionId); + var exists = store.Any(x => + x.Name == name + && x.DefinitionId != definitionId + && TenantVisibility.IsVisible(x.TenantId, CurrentTenantId)); return Task.FromResult(!exists); } - private IQueryable Filter(IQueryable queryable, WorkflowDefinitionFilter filter) => filter.Apply(queryable); + /// + /// Ambient tenant is applied here rather than in . + /// EF owns that via SetTenantIdFilter / IgnoreQueryFilters; Memory must compensate. + /// + private IQueryable Filter(IQueryable queryable, WorkflowDefinitionFilter filter) => + filter.Apply(queryable.WhereVisibleToTenant(CurrentTenantId, filter.TenantAgnostic)); + + private string CurrentTenantId => tenantAccessor?.TenantId ?? Tenant.DefaultTenantId; private string GetId(WorkflowDefinition workflowDefinition) => workflowDefinition.Id; } \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Runtime/Stores/MemoryBookmarkStore.cs b/src/modules/Elsa.Workflows.Runtime/Stores/MemoryBookmarkStore.cs index b2115e28e..552a7be67 100644 --- a/src/modules/Elsa.Workflows.Runtime/Stores/MemoryBookmarkStore.cs +++ b/src/modules/Elsa.Workflows.Runtime/Stores/MemoryBookmarkStore.cs @@ -1,4 +1,5 @@ using Elsa.Common.Models; +using Elsa.Common.Multitenancy; using Elsa.Common.Services; using Elsa.Extensions; using Elsa.Workflows.Runtime.Entities; @@ -9,7 +10,7 @@ namespace Elsa.Workflows.Runtime.Stores; /// [UsedImplicitly] -public class MemoryBookmarkStore(MemoryStore store) : IBookmarkStore +public class MemoryBookmarkStore(MemoryStore store, ITenantAccessor? tenantAccessor = null) : IBookmarkStore { /// public ValueTask SaveAsync(StoredBookmark record, CancellationToken cancellationToken = default) @@ -54,5 +55,12 @@ public class MemoryBookmarkStore(MemoryStore store) : IBookmarkS return store.DeleteMany(ids); } - private static IQueryable Filter(IQueryable query, BookmarkFilter filter) => filter.Apply(query); + /// + /// Ambient tenant is applied here rather than in . + /// EF owns that via SetTenantIdFilter / IgnoreQueryFilters; Memory must compensate. + /// + private IQueryable Filter(IQueryable query, BookmarkFilter filter) => + filter.Apply(query.WhereVisibleToTenant(CurrentTenantId, filter.TenantAgnostic)); + + private string CurrentTenantId => tenantAccessor?.TenantId ?? Tenant.DefaultTenantId; } \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Runtime/Stores/MemoryTriggerStore.cs b/src/modules/Elsa.Workflows.Runtime/Stores/MemoryTriggerStore.cs index 55aee2ad7..96fa0abcc 100644 --- a/src/modules/Elsa.Workflows.Runtime/Stores/MemoryTriggerStore.cs +++ b/src/modules/Elsa.Workflows.Runtime/Stores/MemoryTriggerStore.cs @@ -63,14 +63,14 @@ public class MemoryTriggerStore : ITriggerStore /// public ValueTask FindAsync(TriggerFilter filter, CancellationToken cancellationToken = default) { - var entity = _store.Query(filter.Apply).FirstOrDefault(); + var entity = _store.Query(query => Filter(query, filter)).FirstOrDefault(); return new(entity); } /// public ValueTask> FindManyAsync(TriggerFilter filter, CancellationToken cancellationToken = default) { - var entities = _store.Query(filter.Apply); + var entities = _store.Query(query => Filter(query, filter)); return new(entities); } @@ -81,8 +81,8 @@ public class MemoryTriggerStore : ITriggerStore public ValueTask> FindManyAsync(TriggerFilter filter, PageArgs pageArgs, StoredTriggerOrder order, CancellationToken cancellationToken = default) { - var count = _store.Query(filter.Apply).LongCount(); - var result = _store.Query(query => filter.Apply(query).OrderBy(order).Paginate(pageArgs)).ToList(); + var count = _store.Query(query => Filter(query, filter)).LongCount(); + var result = _store.Query(query => Filter(query, filter).OrderBy(order).Paginate(pageArgs)).ToList(); return ValueTask.FromResult(Page.Of(result, count)); } @@ -121,11 +121,20 @@ public class MemoryTriggerStore : ITriggerStore { lock (_store.Sync) { - var ids = _store.Query(filter.Apply).Select(x => x.Id).ToList(); + var ids = _store.Query(query => Filter(query, filter)).Select(x => x.Id).ToList(); return new(_store.DeleteMany(ids)); } } + /// + /// Ambient tenant is applied here rather than in . + /// EF owns that via SetTenantIdFilter / IgnoreQueryFilters; Memory must compensate. + /// + private IQueryable Filter(IQueryable queryable, TriggerFilter filter) => + filter.Apply(queryable.WhereVisibleToTenant(CurrentTenantId, filter.TenantAgnostic)); + + private string CurrentTenantId => _tenantAccessor?.TenantId ?? Tenant.DefaultTenantId; + private void EnsureLogicalKeyAvailable(StoredTrigger record) { var logicalKey = GetLogicalKey(record); diff --git a/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantVisibilityTests.cs b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantVisibilityTests.cs new file mode 100644 index 000000000..07ff5b958 --- /dev/null +++ b/test/unit/Elsa.Common.UnitTests/Multitenancy/TenantVisibilityTests.cs @@ -0,0 +1,79 @@ +using Elsa.Common.Entities; +using Elsa.Common.Multitenancy; + +namespace Elsa.Common.UnitTests.Multitenancy; + +/// +/// Mirrors EF SetTenantIdFilter: ambient match, *, and null-on-default-tenant. +/// +public class TenantVisibilityTests +{ + [Theory] + [InlineData("tenant-a", "tenant-a", true)] + [InlineData("tenant-a", "tenant-b", false)] + [InlineData(Tenant.AgnosticTenantId, "tenant-a", true)] + [InlineData(Tenant.AgnosticTenantId, Tenant.DefaultTenantId, true)] + [InlineData(null, Tenant.DefaultTenantId, true)] + [InlineData(null, "tenant-a", false)] + [InlineData(Tenant.DefaultTenantId, Tenant.DefaultTenantId, true)] + [InlineData(Tenant.DefaultTenantId, "tenant-a", false)] + public void IsVisible_MatchesSetTenantIdFilter(string? entityTenantId, string ambientTenantId, bool expected) + { + Assert.Equal(expected, TenantVisibility.IsVisible(entityTenantId, ambientTenantId)); + } + + [Fact] + public void WhereVisibleToTenant_WhenNotAgnostic_HidesOtherTenants() + { + var queryable = new[] + { + Entity("tenant-a"), + Entity("tenant-b"), + Entity(Tenant.AgnosticTenantId), + Entity(null) + }.AsQueryable(); + + var visible = queryable.WhereVisibleToTenant("tenant-a").Select(x => x.TenantId).ToList(); + + Assert.Equal(2, visible.Count); + Assert.Contains("tenant-a", visible); + Assert.Contains(Tenant.AgnosticTenantId, visible); + } + + [Fact] + public void WhereVisibleToTenant_WhenAgnostic_ReturnsAllRows() + { + var queryable = new[] + { + Entity("tenant-a"), + Entity("tenant-b") + }.AsQueryable(); + + var visible = queryable.WhereVisibleToTenant("tenant-a", tenantAgnostic: true).ToList(); + + Assert.Equal(2, visible.Count); + } + + [Fact] + public void WhereVisibleToTenant_WhenAmbientIsDefault_IncludesNullTenantId() + { + var queryable = new[] + { + Entity(null), + Entity("tenant-a") + }.AsQueryable(); + + var visible = queryable.WhereVisibleToTenant(Tenant.DefaultTenantId).ToList(); + + Assert.Single(visible); + Assert.Null(visible[0].TenantId); + } + + private static TestEntity Entity(string? tenantId) => new() + { + Id = tenantId ?? "null", + TenantId = tenantId + }; + + private sealed class TestEntity : Entity; +} diff --git a/test/unit/Elsa.Workflows.Management.UnitTests/Stores/MemoryWorkflowDefinitionStoreTenantIsolationTests.cs b/test/unit/Elsa.Workflows.Management.UnitTests/Stores/MemoryWorkflowDefinitionStoreTenantIsolationTests.cs new file mode 100644 index 000000000..09446cb7a --- /dev/null +++ b/test/unit/Elsa.Workflows.Management.UnitTests/Stores/MemoryWorkflowDefinitionStoreTenantIsolationTests.cs @@ -0,0 +1,179 @@ +using Elsa.Common.Multitenancy; +using Elsa.Common.Services; +using Elsa.Testing.Shared.Multitenancy; +using Elsa.Workflows.Management.Entities; +using Elsa.Workflows.Management.Filters; +using Elsa.Workflows.Management.Stores; + +namespace Elsa.Workflows.Management.UnitTests.Stores; + +/// +/// Memory must honor ambient tenant + +/// the same way EF does via SetTenantIdFilter / IgnoreQueryFilters. +/// +public class MemoryWorkflowDefinitionStoreTenantIsolationTests +{ + [Fact(DisplayName = "FindManyAsync hides other tenants and keeps * visible")] + public async Task FindManyAsync_WhenNotTenantAgnostic_HidesOtherTenants() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = (await store.FindManyAsync(new WorkflowDefinitionFilter())).ToList(); + + Assert.Equal(2, found.Count); + Assert.Contains(found, x => x.Id == "def-a"); + Assert.Contains(found, x => x.Id == "def-star"); + Assert.DoesNotContain(found, x => x.Id == "def-b"); + } + + [Fact(DisplayName = "FindManyAsync with TenantAgnostic returns every tenant")] + public async Task FindManyAsync_WhenTenantAgnostic_ReturnsAllTenants() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = (await store.FindManyAsync(new WorkflowDefinitionFilter { TenantAgnostic = true })).ToList(); + + Assert.Equal(3, found.Count); + Assert.Contains(found, x => x.Id == "def-a"); + Assert.Contains(found, x => x.Id == "def-b"); + Assert.Contains(found, x => x.Id == "def-star"); + } + + [Fact(DisplayName = "FindAsync does not return another tenant's row by Id")] + public async Task FindAsync_WhenOtherTenant_ReturnsNull() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = await store.FindAsync(new WorkflowDefinitionFilter { Id = "def-b" }); + + Assert.Null(found); + } + + [Fact(DisplayName = "AnyAsync is false when only another tenant matches")] + public async Task AnyAsync_WhenOnlyOtherTenantMatches_ReturnsFalse() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var exists = await store.AnyAsync(new WorkflowDefinitionFilter { Id = "def-b" }); + + Assert.False(exists); + } + + [Fact(DisplayName = "CountDistinctAsync counts only the current tenant's definition IDs")] + public async Task CountDistinctAsync_CountsOnlyVisibleDefinitions() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var count = await store.CountDistinctAsync(); + + Assert.Equal(2, count); + } + + [Fact(DisplayName = "GetIsNameUnique allows the same name in another tenant")] + public async Task GetIsNameUnique_AllowsSameNameInAnotherTenant() + { + var store = CreateStore("tenant-a"); + await store.SaveAsync(Definition("def-b", "order", "tenant-b")); + + var unique = await store.GetIsNameUnique("order"); + + Assert.True(unique); + } + + [Fact(DisplayName = "GetIsNameUnique is false when the current tenant already has the name")] + public async Task GetIsNameUnique_WhenCurrentTenantOwnsTheName_ReturnsFalse() + { + var store = CreateStore("tenant-a"); + await store.SaveAsync(Definition("def-a", "order", "tenant-a")); + + var unique = await store.GetIsNameUnique("order"); + + Assert.False(unique); + } + + [Fact(DisplayName = "DeleteAsync does not remove another tenant's rows")] + public async Task DeleteAsync_DoesNotDeleteOtherTenantRows() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var deleted = await store.DeleteAsync(new WorkflowDefinitionFilter()); + var remaining = (await store.FindManyAsync(new WorkflowDefinitionFilter { TenantAgnostic = true })).ToList(); + + Assert.Equal(2, deleted); + Assert.Single(remaining); + Assert.Equal("def-b", remaining[0].Id); + } + + [Fact(DisplayName = "DeleteAsync with a shared DefinitionId leaves the other tenant's versions")] + public async Task DeleteAsync_WhenDefinitionIdIsShared_LeavesOtherTenantRows() + { + var backing = new MemoryStore(); + var tenantA = new MemoryWorkflowDefinitionStore(backing, new TestTenantAccessor("tenant-a")); + var tenantB = new MemoryWorkflowDefinitionStore(backing, new TestTenantAccessor("tenant-b")); + await tenantA.SaveAsync(Definition("def-a", "Order", "tenant-a", definitionId: "order")); + await tenantB.SaveAsync(Definition("def-b", "Order", "tenant-b", definitionId: "order")); + + var deleted = await tenantA.DeleteAsync(new WorkflowDefinitionFilter { DefinitionId = "order" }); + var remainingForA = (await tenantA.FindManyAsync(new WorkflowDefinitionFilter { DefinitionId = "order" })).ToList(); + var remainingForB = (await tenantB.FindManyAsync(new WorkflowDefinitionFilter { DefinitionId = "order" })).ToList(); + + Assert.Equal(1, deleted); + Assert.Empty(remainingForA); + Assert.Single(remainingForB); + Assert.Equal("def-b", remainingForB[0].Id); + } + + [Fact(DisplayName = "FindManyAsync on the default tenant includes null TenantId rows")] + public async Task FindManyAsync_WhenAmbientIsDefault_IncludesNullTenantId() + { + var store = CreateStore(Tenant.DefaultTenantId); + await store.SaveAsync(Definition("def-null", "Null", tenantId: null)); + await store.SaveAsync(Definition("def-a", "A", "tenant-a")); + + var found = (await store.FindManyAsync(new WorkflowDefinitionFilter())).ToList(); + + Assert.Single(found); + Assert.Equal("def-null", found[0].Id); + } + + [Fact(DisplayName = "FindManyAsync on a named tenant hides null TenantId rows")] + public async Task FindManyAsync_WhenAmbientIsNamed_HidesNullTenantId() + { + var store = CreateStore("tenant-a"); + await store.SaveAsync(Definition("def-null", "Null", tenantId: null)); + await store.SaveAsync(Definition("def-a", "A", "tenant-a")); + + var found = (await store.FindManyAsync(new WorkflowDefinitionFilter())).ToList(); + + Assert.Single(found); + Assert.Equal("def-a", found[0].Id); + } + + private static MemoryWorkflowDefinitionStore CreateStore(string tenantId) => + new(new MemoryStore(), new TestTenantAccessor(tenantId)); + + private static async Task SeedMixedTenantsAsync(MemoryWorkflowDefinitionStore store) + { + await store.SaveAsync(Definition("def-a", "A", "tenant-a")); + await store.SaveAsync(Definition("def-b", "B", "tenant-b")); + await store.SaveAsync(Definition("def-star", "Star", Tenant.AgnosticTenantId)); + } + + private static WorkflowDefinition Definition(string id, string name, string? tenantId, string? definitionId = null) => + new() + { + Id = id, + DefinitionId = definitionId ?? id, + Name = name, + TenantId = tenantId, + Version = 1, + IsLatest = true, + MaterializerName = "Json" + }; +} diff --git a/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryBookmarkStoreTenantIsolationTests.cs b/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryBookmarkStoreTenantIsolationTests.cs new file mode 100644 index 000000000..2271344a4 --- /dev/null +++ b/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryBookmarkStoreTenantIsolationTests.cs @@ -0,0 +1,114 @@ +using Elsa.Common.Multitenancy; +using Elsa.Common.Services; +using Elsa.Testing.Shared.Multitenancy; +using Elsa.Workflows.Runtime.Entities; +using Elsa.Workflows.Runtime.Filters; +using Elsa.Workflows.Runtime.Stores; + +namespace Elsa.Workflows.Runtime.UnitTests.Stores; + +/// +/// Memory must honor ambient tenant + +/// the same way EF does via SetTenantIdFilter / IgnoreQueryFilters. +/// +public class MemoryBookmarkStoreTenantIsolationTests +{ + [Fact(DisplayName = "FindManyAsync hides other tenants and keeps * visible")] + public async Task FindManyAsync_WhenNotTenantAgnostic_HidesOtherTenants() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = (await store.FindManyAsync(new BookmarkFilter())).ToList(); + + Assert.Equal(2, found.Count); + Assert.Contains(found, x => x.Id == "bm-a"); + Assert.Contains(found, x => x.Id == "bm-star"); + Assert.DoesNotContain(found, x => x.Id == "bm-b"); + } + + [Fact(DisplayName = "FindManyAsync with TenantAgnostic returns every tenant")] + public async Task FindManyAsync_WhenTenantAgnostic_ReturnsAllTenants() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = (await store.FindManyAsync(new BookmarkFilter { TenantAgnostic = true })).ToList(); + + Assert.Equal(3, found.Count); + Assert.Contains(found, x => x.Id == "bm-a"); + Assert.Contains(found, x => x.Id == "bm-b"); + Assert.Contains(found, x => x.Id == "bm-star"); + } + + [Fact(DisplayName = "FindAsync does not return another tenant's row by Id")] + public async Task FindAsync_WhenOtherTenant_ReturnsNull() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = await store.FindAsync(new BookmarkFilter { BookmarkId = "bm-b" }); + + Assert.Null(found); + } + + [Fact(DisplayName = "DeleteAsync does not remove another tenant's rows")] + public async Task DeleteAsync_DoesNotDeleteOtherTenantRows() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var deleted = await store.DeleteAsync(new BookmarkFilter()); + var remaining = (await store.FindManyAsync(new BookmarkFilter { TenantAgnostic = true })).ToList(); + + Assert.Equal(2, deleted); + Assert.Single(remaining); + Assert.Equal("bm-b", remaining[0].Id); + } + + [Fact(DisplayName = "FindManyAsync on the default tenant includes null TenantId rows")] + public async Task FindManyAsync_WhenAmbientIsDefault_IncludesNullTenantId() + { + var store = CreateStore(Tenant.DefaultTenantId); + await store.SaveAsync(Bookmark("bm-null", tenantId: null)); + await store.SaveAsync(Bookmark("bm-a", tenantId: "tenant-a")); + + var found = (await store.FindManyAsync(new BookmarkFilter())).ToList(); + + Assert.Single(found); + Assert.Equal("bm-null", found[0].Id); + } + + [Fact(DisplayName = "FindManyAsync on a named tenant hides null TenantId rows")] + public async Task FindManyAsync_WhenAmbientIsNamed_HidesNullTenantId() + { + var store = CreateStore("tenant-a"); + await store.SaveAsync(Bookmark("bm-null", tenantId: null)); + await store.SaveAsync(Bookmark("bm-a", tenantId: "tenant-a")); + + var found = (await store.FindManyAsync(new BookmarkFilter())).ToList(); + + Assert.Single(found); + Assert.Equal("bm-a", found[0].Id); + } + + private static MemoryBookmarkStore CreateStore(string tenantId) => + new(new MemoryStore(), new TestTenantAccessor(tenantId)); + + private static async Task SeedMixedTenantsAsync(MemoryBookmarkStore store) + { + await store.SaveAsync(Bookmark("bm-a", tenantId: "tenant-a")); + await store.SaveAsync(Bookmark("bm-b", tenantId: "tenant-b")); + await store.SaveAsync(Bookmark("bm-star", tenantId: Tenant.AgnosticTenantId)); + } + + private static StoredBookmark Bookmark(string id, string? tenantId) => + new() + { + Id = id, + TenantId = tenantId, + Hash = id, + WorkflowInstanceId = "instance-1", + Name = "Elsa.HttpEndpoint" + }; +} diff --git a/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryTriggerStoreTenantIsolationTests.cs b/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryTriggerStoreTenantIsolationTests.cs new file mode 100644 index 000000000..e9077a0ce --- /dev/null +++ b/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryTriggerStoreTenantIsolationTests.cs @@ -0,0 +1,107 @@ +using Elsa.Common.Multitenancy; +using Elsa.Common.Services; +using Elsa.Testing.Shared.Multitenancy; +using Elsa.Workflows.Runtime.Entities; +using Elsa.Workflows.Runtime.Filters; +using Elsa.Workflows.Runtime.Stores; + +namespace Elsa.Workflows.Runtime.UnitTests.Stores; + +/// +/// Memory must honor ambient tenant + +/// the same way EF does via SetTenantIdFilter / IgnoreQueryFilters. +/// +public class MemoryTriggerStoreTenantIsolationTests +{ + [Fact(DisplayName = "FindManyAsync hides other tenants and keeps * visible")] + public async Task FindManyAsync_WhenNotTenantAgnostic_HidesOtherTenants() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = (await store.FindManyAsync(new TriggerFilter())).ToList(); + + Assert.Equal(2, found.Count); + Assert.Contains(found, x => x.Id == "id-a"); + Assert.Contains(found, x => x.Id == "id-star"); + Assert.DoesNotContain(found, x => x.Id == "id-b"); + } + + [Fact(DisplayName = "FindManyAsync with TenantAgnostic returns every tenant")] + public async Task FindManyAsync_WhenTenantAgnostic_ReturnsAllTenants() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = (await store.FindManyAsync(new TriggerFilter { TenantAgnostic = true })).ToList(); + + Assert.Equal(3, found.Count); + Assert.Contains(found, x => x.Id == "id-a"); + Assert.Contains(found, x => x.Id == "id-b"); + Assert.Contains(found, x => x.Id == "id-star"); + } + + [Fact(DisplayName = "FindAsync does not return another tenant's row by Id")] + public async Task FindAsync_WhenOtherTenant_ReturnsNull() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var found = await store.FindAsync(new TriggerFilter { Id = "id-b" }); + + Assert.Null(found); + } + + [Fact(DisplayName = "DeleteManyAsync does not remove another tenant's rows")] + public async Task DeleteManyAsync_DoesNotDeleteOtherTenantRows() + { + var store = CreateStore("tenant-a"); + await SeedMixedTenantsAsync(store); + + var deleted = await store.DeleteManyAsync(new TriggerFilter()); + var remaining = (await store.FindManyAsync(new TriggerFilter { TenantAgnostic = true })).ToList(); + + Assert.Equal(2, deleted); + Assert.Single(remaining); + Assert.Equal("id-b", remaining[0].Id); + } + + [Fact(DisplayName = "FindManyAsync on the default tenant includes null TenantId rows")] + public async Task FindManyAsync_WhenAmbientIsDefault_IncludesNullTenantId() + { + var store = new MemoryTriggerStore(new MemoryStore()); + var unassigned = Trigger("id-null"); + unassigned.TenantId = null; + var other = Trigger("id-a", hash: "hash-a"); + other.TenantId = "tenant-a"; + await store.SaveAsync(unassigned); + await store.SaveAsync(other); + + var found = (await store.FindManyAsync(new TriggerFilter())).ToList(); + + Assert.Single(found); + Assert.Equal("id-null", found[0].Id); + } + + private static MemoryTriggerStore CreateStore(string tenantId) => + new(new MemoryStore(), new TestTenantAccessor(tenantId)); + + private static async Task SeedMixedTenantsAsync(MemoryTriggerStore store) + { + await store.SaveAsync(Trigger("id-a", tenantId: "tenant-a", hash: "hash-a")); + await store.SaveAsync(Trigger("id-b", tenantId: "tenant-b", hash: "hash-b")); + await store.SaveAsync(Trigger("id-star", tenantId: Tenant.AgnosticTenantId, hash: "hash-star")); + } + + private static StoredTrigger Trigger(string id, string? tenantId = null, string hash = "hash-1") => + new() + { + Id = id, + TenantId = tenantId, + WorkflowDefinitionId = "workflow-1", + WorkflowDefinitionVersionId = "v1", + ActivityId = id, + Hash = hash, + Name = "Elsa.HttpEndpoint" + }; +} diff --git a/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryTriggerStoreTests.cs b/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryTriggerStoreTests.cs index 624c2f4bc..d8b3dde43 100644 --- a/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryTriggerStoreTests.cs +++ b/test/unit/Elsa.Workflows.Runtime.UnitTests/Stores/MemoryTriggerStoreTests.cs @@ -96,7 +96,7 @@ public class MemoryTriggerStoreTests await store.ReplaceAsync([], [tenantA, tenantB]); - var stored = (await store.FindManyAsync(new TriggerFilter())).ToList(); + var stored = (await store.FindManyAsync(new TriggerFilter { TenantAgnostic = true })).ToList(); Assert.Equal(2, stored.Count); Assert.Contains(stored, x => x.Id == "id-a"); Assert.Contains(stored, x => x.Id == "id-b");