diff --git a/src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs b/src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs
index 7cf02be7c..c659a4131 100644
--- a/src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs
+++ b/src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs
@@ -39,15 +39,14 @@ namespace Elsa.ExternalAuthentication.Services;
/// replacement role, however, is still performed through the ambient tenant's role services, so when the
/// deletion target is agnostic the replacement role must itself be agnostic; a tenant-scoped replacement is
/// rejected rather than being authorized in one tenant and written into every tenant's connections.
-/// In EF Core persistence a role's primary key is its ID alone, so a role ID is unique across all tenants there
-/// and an agnostic/tenant-scoped collision cannot exist. Only MemoryRoleStore can hold two roles that
-/// share an ID (its storage key includes the tenant); resolving a role ID against it can then be genuinely
-/// ambiguous. That ambiguity is never resolved by guessing: widening a tenant-scoped deletion would expose
-/// another tenant's references, and narrowing an agnostic deletion would leave an agnostic role's references
-/// dangling. It fails closed instead. The same collision makes a replacement candidate ambiguous too: a
-/// replacement ID that resolves to more than one role (an ambient match and an agnostic match, under
-/// MemoryRoleStore) is rejected as not agnostic rather than guessed at, so it is reported as
-/// replacement_role_unavailable_or_unauthorized instead of surfacing as an exception.
+/// In EF Core persistence and MemoryRoleStore, a role's key is its ID alone, so a role ID is unique across
+/// all tenants and an agnostic/tenant-scoped collision cannot exist. A custom can still
+/// violate that invariant, so resolving a role ID against it can be genuinely ambiguous. That ambiguity is never
+/// resolved by guessing: widening a tenant-scoped deletion would expose another tenant's references, and narrowing
+/// an agnostic deletion would leave an agnostic role's references dangling. It fails closed instead. The same
+/// defensive rule applies to a replacement candidate: an ID that resolves to more than one role is rejected as not
+/// agnostic rather than guessed at, so it is reported as replacement_role_unavailable_or_unauthorized
+/// instead of surfacing as an exception.
///
public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
IIdentityProviderConnectionStore store,
@@ -262,13 +261,13 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
///
/// Resolves the tenant context for the role being deleted: the role's own TenantId
/// ( normalized when the role is tenant-agnostic, in which case its
- /// tenant context is every tenant). In EF Core persistence a role ID is unique across all tenants (the
- /// Roles table keys on Id alone), so this lookup resolves to at most one role there. Only
- /// MemoryRoleStore can hold two roles that share an ID because its storage key includes the tenant;
- /// if the ID resolves to more than one role, which tenant's role the coordinator's own delete actually
- /// targets is already ambiguous, and this method cannot make the operation consistent by guessing in either
- /// direction -- widening would expose another tenant's references for what may be a tenant-scoped deletion,
- /// and narrowing would leave an agnostic role's references dangling. It fails closed instead.
+ /// tenant context is every tenant). In EF Core persistence and MemoryRoleStore, a role ID is unique
+ /// across all tenants (the Roles table and in-memory store both key on Id alone), so this lookup
+ /// resolves to at most one role there. If a custom store violates that invariant and the ID resolves to more
+ /// than one role, which tenant's role the coordinator's own delete actually targets is already ambiguous, and
+ /// this method cannot make the operation consistent by guessing in either direction -- widening would expose
+ /// another tenant's references for what may be a tenant-scoped deletion, and narrowing would leave an agnostic
+ /// role's references dangling. It fails closed instead.
/// A missing store or no matching role falls back to the ambient tenant on ,
/// which is the only case where the ambient tenant is trusted: the role cannot be resolved at all, so there
/// is no resolved tenant to prefer over it.
@@ -288,10 +287,10 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
///
/// Resolves whether a candidate role ID (typically a replacement role) is itself tenant-agnostic. Unlike
/// , an ambiguous candidate -- a role ID that resolves to more than one
- /// role, which only MemoryRoleStore can produce -- is not the coordinator's own deletion target, so
- /// there is no operation to fail closed on by throwing; it is instead treated the same as an unresolved
- /// candidate and reported as not agnostic, since there is no single resolved role to trust as safe to write
- /// into every tenant's connections.
+ /// role, which a custom store can produce despite the global ID invariant of the built-in stores -- is not the
+ /// coordinator's own deletion target, so there is no operation to fail closed on by throwing; it is instead
+ /// treated the same as an unresolved candidate and reported as not agnostic, since there is no single resolved
+ /// role to trust as safe to write into every tenant's connections.
///
private async ValueTask IsAgnosticRoleAsync(string? roleId, CancellationToken cancellationToken)
{
diff --git a/src/modules/Elsa.Identity/Services/MemoryRoleStore.cs b/src/modules/Elsa.Identity/Services/MemoryRoleStore.cs
index 8f0522057..aa23328e9 100644
--- a/src/modules/Elsa.Identity/Services/MemoryRoleStore.cs
+++ b/src/modules/Elsa.Identity/Services/MemoryRoleStore.cs
@@ -26,15 +26,24 @@ public class MemoryRoleStore : IRoleStore, IRoleStoreWithAtomicDelete
///
public Task AddAsync(Role role, CancellationToken cancellationToken = default)
{
- Save(role);
+ lock (_store.Sync)
+ {
+ MemoryIdentityUniqueness.EnsureAvailable(_store, role, x => x.Name, "name");
+ _store.Add(role, x => x.Id);
+ }
+
return Task.CompletedTask;
}
///
public Task DeleteAsync(RoleFilter filter, CancellationToken cancellationToken = default)
{
- var roles = _store.Query(query => Filter(query, filter)).ToList();
- _store.DeleteMany(roles, GetStorageKey);
+ lock (_store.Sync)
+ {
+ var roles = _store.Query(query => Filter(query, filter)).ToList();
+ _store.DeleteMany(roles, x => x.Id);
+ }
+
return Task.CompletedTask;
}
@@ -46,10 +55,13 @@ public class MemoryRoleStore : IRoleStore, IRoleStoreWithAtomicDelete
///
public Task TryDeleteAsync(string roleId, CancellationToken cancellationToken = default)
{
- var role = _store.Query(query => Filter(query, new RoleFilter { Id = roleId })).FirstOrDefault();
- var deleted = role is not null && _store.Delete(GetStorageKey(role));
+ lock (_store.Sync)
+ {
+ var role = _store.Query(query => Filter(query, new RoleFilter { Id = roleId })).FirstOrDefault();
+ var deleted = role is not null && _store.Delete(role.Id);
- return Task.FromResult(deleted);
+ return Task.FromResult(deleted);
+ }
}
///
@@ -63,11 +75,31 @@ public class MemoryRoleStore : IRoleStore, IRoleStoreWithAtomicDelete
{
lock (_store.Sync)
{
+ EnsureIdIsAvailable(role);
MemoryIdentityUniqueness.EnsureAvailable(_store, role, x => x.Name, "name");
- _store.Save(role, GetStorageKey);
+ _store.Save(role, x => x.Id);
}
}
+ private void EnsureIdIsAvailable(Role role)
+ {
+ var existing = _store.Find(x => x.Id == role.Id);
+ if (existing is not null && !CanReplace(existing, role))
+ {
+ throw new InvalidOperationException(
+ $"A role already exists with ID '{role.Id}' in tenant '{existing.TenantId}'.");
+ }
+ }
+
+ ///
+ /// A role may be replaced only when its existing row is visible to the ambient tenant and the incoming role
+ /// retains that row's tenant marker. This matches the durable store's query-filtered ID lookup while preventing
+ /// a caller from re-homing a globally keyed row by changing its TenantId.
+ ///
+ private bool CanReplace(Role existing, Role replacement) =>
+ string.Equals(existing.TenantId, replacement.TenantId, StringComparison.Ordinal) &&
+ TenantVisibility.IsVisible(existing.TenantId, _tenantAccessor.TenantId);
+
///
public Task FindAsync(RoleFilter filter, CancellationToken cancellationToken = default)
{
@@ -104,9 +136,4 @@ public class MemoryRoleStore : IRoleStore, IRoleStoreWithAtomicDelete
TenantId = role.TenantId,
Permissions = role.Permissions.ToList()
};
-
- private static string GetStorageKey(Role role) => GetStorageKey(role.TenantId, role.Id);
-
- private static string GetStorageKey(string? tenantId, string roleId) =>
- $"{tenantId?.Length ?? -1}:{tenantId}{roleId.Length}:{roleId}";
}
diff --git a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs
index d8f913aa8..adca0325b 100644
--- a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs
+++ b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs
@@ -471,20 +471,22 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
}
[Fact]
- public async Task RoleIdThatResolvesToMoreThanOneRoleAcrossTenantScopesFailsClosed()
+ public async Task RoleIdThatResolvesToMoreThanOneRoleInACustomStoreFailsClosed()
{
var ownConnection = Connection("own-connection", CreateUserPolicy("workflow-user"), TenantA);
var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("workflow-user"), TenantB);
var (contributor, _, _) = await CreateContributorAsync(
[],
[ownConnection, otherTenantConnection],
- additionalRoles: [new Role { Id = "workflow-user", Name = "Agnostic workflow user", TenantId = Tenant.AgnosticTenantId, Permissions = [] }],
- tenantAccessor: new TestTenantAccessor(TenantA));
+ tenantAccessor: new TestTenantAccessor(TenantA),
+ roleStoreOverride: new RoleStoreWithoutAmbientTenantFilter(
+ [
+ new Role { Id = "workflow-user", Name = "Tenant A workflow user", TenantId = TenantA, Permissions = [] },
+ new Role { Id = "workflow-user", Name = "Agnostic workflow user", TenantId = Tenant.AgnosticTenantId, Permissions = [] }
+ ]));
- // Tenant A's own "workflow-user" role and an agnostic role sharing that same ID both exist in the
- // in-memory role store, so the deletion target is ambiguous: the contributor cannot determine whether
- // to scope its inspection and remediation to tenant A alone or to every tenant, and must fail closed
- // rather than guess in either direction.
+ // The built-in role stores use the role ID as their key and cannot produce this state. A custom store can
+ // still violate that contract, so the contributor must not guess which tenant scope the deletion targets.
await Assert.ThrowsAsync(() => contributor.InspectAsync("workflow-user").AsTask());
var request = new RoleReferenceRemovalRequest(
@@ -529,6 +531,44 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
AssertDefaultRoleIds(await store.FindByIdAsync(otherTenantConnection.Id));
}
+ [Fact]
+ public async Task ReplacementRoleIdThatResolvesToMoreThanOneRoleInACustomStoreIsRejectedRatherThanThrowing()
+ {
+ var ownConnection = Connection("own-connection", CreateUserPolicy("agnostic-role"), TenantA);
+ var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("agnostic-role"), TenantB);
+ var (contributor, store, _) = await CreateContributorAsync(
+ [],
+ [ownConnection, otherTenantConnection],
+ tenantAccessor: new TestTenantAccessor(TenantA),
+ roleStoreOverride: new RoleStoreWithoutAmbientTenantFilter(
+ [
+ new Role { Id = "agnostic-role", Name = "Agnostic role", TenantId = Tenant.AgnosticTenantId, Permissions = [] },
+ new Role { Id = "ambiguous-replacement", Name = "Tenant A replacement", TenantId = TenantA, Permissions = [] },
+ new Role { Id = "ambiguous-replacement", Name = "Agnostic replacement", TenantId = Tenant.AgnosticTenantId, Permissions = [] }
+ ]));
+ var snapshot = await contributor.InspectAsync("agnostic-role");
+ var request = new RoleReferenceRemovalRequest("agnostic-role", Administrator(), snapshot.Version, snapshot.Dependencies)
+ {
+ SelectedReferences = snapshot.Dependencies
+ .Select(x => new RoleDeletionReferenceSelection(ExternalAuthenticationRoleDeletionDependencyContributor.SourceName, x.OwnerId))
+ .ToArray(),
+ ReplacementRoleId = "ambiguous-replacement"
+ };
+
+ // An ambiguous replacement cannot be proven agnostic, so it must be reported as unavailable instead of
+ // letting whichever role FindAsync returns authorize a write into every tenant's connection.
+ var validation = await contributor.ValidateRemovalAsync(request);
+ var forbidden = Assert.IsType(validation);
+ Assert.Equal("replacement_role_unavailable_or_unauthorized", forbidden.Code);
+
+ var result = await contributor.RemoveEditableReferencesAsync(request);
+ var failed = Assert.IsType(result);
+ Assert.Equal("replacement_role_unavailable_or_unauthorized", failed.Code);
+ Assert.Empty(failed.ChangedOwnerIds);
+ AssertDefaultRoleIds(await store.FindByIdAsync(ownConnection.Id), "agnostic-role");
+ AssertDefaultRoleIds(await store.FindByIdAsync(otherTenantConnection.Id), "agnostic-role");
+ }
+
[Fact]
public async Task RemediationOfAnAgnosticRoleRejectsATenantScopedReplacementRole()
{
@@ -621,46 +661,6 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
AssertDefaultRoleIds(await store.FindByIdAsync(hostConnection.Id), "other-role");
}
- [Fact]
- public async Task ReplacementRoleIdThatResolvesToBothATenantRoleAndAnAgnosticRoleIsRejectedRatherThanThrowing()
- {
- var ownConnection = Connection("own-connection", CreateUserPolicy("agnostic-role"), TenantA);
- var otherTenantConnection = Connection("other-tenant-connection", CreateUserPolicy("agnostic-role"), TenantB);
- var (contributor, store, _) = await CreateContributorAsync(
- [],
- [ownConnection, otherTenantConnection],
- additionalRoles:
- [
- new Role { Id = "agnostic-role", Name = "Agnostic role", TenantId = Tenant.AgnosticTenantId, Permissions = [] },
- new Role { Id = "ambiguous-replacement", Name = "Ambiguous replacement (tenant A)", TenantId = TenantA, Permissions = [] },
- new Role { Id = "ambiguous-replacement", Name = "Ambiguous replacement (agnostic)", TenantId = Tenant.AgnosticTenantId, Permissions = [] }
- ],
- tenantAccessor: new TestTenantAccessor(TenantA));
- var snapshot = await contributor.InspectAsync("agnostic-role");
- var request = new RoleReferenceRemovalRequest("agnostic-role", Administrator(), snapshot.Version, snapshot.Dependencies)
- {
- SelectedReferences = snapshot.Dependencies
- .Select(x => new RoleDeletionReferenceSelection(ExternalAuthenticationRoleDeletionDependencyContributor.SourceName, x.OwnerId))
- .ToArray(),
- ReplacementRoleId = "ambiguous-replacement"
- };
-
- // The replacement ID resolves to two roles in the in-memory store (a tenant-A role and an agnostic role
- // sharing the same ID), which is exactly the collision ResolveRoleTenantIdAsync fails closed on for a
- // deletion target. A replacement candidate is not the coordinator's own deletion target, so this must be
- // reported as an ordinary validation failure rather than escape as an exception.
- var validation = await contributor.ValidateRemovalAsync(request);
- var forbidden = Assert.IsType(validation);
- Assert.Equal("replacement_role_unavailable_or_unauthorized", forbidden.Code);
-
- var result = await contributor.RemoveEditableReferencesAsync(request);
- var failed = Assert.IsType(result);
- Assert.Equal("replacement_role_unavailable_or_unauthorized", failed.Code);
- Assert.Empty(failed.ChangedOwnerIds);
- AssertDefaultRoleIds(await store.FindByIdAsync(ownConnection.Id), "agnostic-role");
- AssertDefaultRoleIds(await store.FindByIdAsync(otherTenantConnection.Id), "agnostic-role");
- }
-
private static Task<(ExternalAuthenticationRoleDeletionDependencyContributor Contributor, InMemoryIdentityProviderConnectionStore Store, InMemoryConnectionRegistryVersionStore Versions)> CreateContributorAsync(
IReadOnlyCollection configuredConnections,
params IdentityProviderConnection[] databaseConnections) =>
@@ -671,18 +671,22 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
IdentityProviderConnection[] databaseConnections,
IReadOnlyCollection? additionalRoles = null,
ITenantAccessor? tenantAccessor = null,
- Func? decorateStore = null)
+ Func? decorateStore = null,
+ IRoleStore? roleStoreOverride = null)
{
var store = new InMemoryIdentityProviderConnectionStore();
foreach (var connection in databaseConnections)
Assert.IsType(await store.CreateAsync(connection));
var accessor = tenantAccessor ?? TestTenantAccessor.Default;
- var roleStore = new MemoryRoleStore(new MemoryStore(), accessor);
- await roleStore.SaveAsync(new Role { Id = "workflow-user", Name = "Workflow user", TenantId = accessor.TenantId, Permissions = [] });
- await roleStore.SaveAsync(new Role { Id = "other-role", Name = "Other role", TenantId = accessor.TenantId, Permissions = [] });
- foreach (var role in additionalRoles ?? [])
- await roleStore.SaveAsync(role);
+ var roleStore = roleStoreOverride ?? new MemoryRoleStore(new MemoryStore(), accessor);
+ if (roleStoreOverride is null)
+ {
+ await roleStore.SaveAsync(new Role { Id = "workflow-user", Name = "Workflow user", TenantId = accessor.TenantId, Permissions = [] });
+ await roleStore.SaveAsync(new Role { Id = "other-role", Name = "Other role", TenantId = accessor.TenantId, Permissions = [] });
+ foreach (var role in additionalRoles ?? [])
+ await roleStore.SaveAsync(role);
+ }
var versions = new InMemoryConnectionRegistryVersionStore();
var services = new ServiceCollection().BuildServiceProvider();
var contributor = new ExternalAuthenticationRoleDeletionDependencyContributor(
@@ -777,8 +781,9 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
///
/// Resolves roles by ID alone, regardless of the ambient tenant, standing in for the EF Core role store
/// with multitenancy disabled: it installs no tenant query filter and can resolve a tenant-owned role by
- /// ID no matter which tenant is ambient. cannot exercise that scenario
- /// because it always filters by the ambient tenant itself.
+ /// ID no matter which tenant is ambient. It intentionally permits duplicate IDs so the external-auth tests
+ /// can retain coverage for a custom store that violates the durable global-ID invariant.
+ /// cannot exercise those scenarios because it keys rows by ID and filters by the ambient tenant itself.
///
private sealed class RoleStoreWithoutAmbientTenantFilter(IReadOnlyCollection roles) : IRoleStore
{
@@ -789,10 +794,12 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
public Task SaveAsync(Role role, CancellationToken cancellationToken = default) => throw new NotSupportedException();
public Task FindAsync(RoleFilter filter, CancellationToken cancellationToken = default) =>
- Task.FromResult(roles.FirstOrDefault(x => x.Id == filter.Id));
+ Task.FromResult(ApplyFilter(filter).FirstOrDefault());
public Task> FindManyAsync(RoleFilter filter, CancellationToken cancellationToken = default) =>
- Task.FromResult(roles.Where(x => x.Id == filter.Id));
+ Task.FromResult>(ApplyFilter(filter).ToArray());
+
+ private IQueryable ApplyFilter(RoleFilter filter) => filter.Apply(roles.AsQueryable());
}
private sealed class RoleStoreThatRemovesReplacementAfterContributorValidation(
diff --git a/test/unit/Elsa.Identity.UnitTests/Services/MemoryRoleStoreUniquenessTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/MemoryRoleStoreUniquenessTests.cs
index 034543e8f..126d3ff02 100644
--- a/test/unit/Elsa.Identity.UnitTests/Services/MemoryRoleStoreUniquenessTests.cs
+++ b/test/unit/Elsa.Identity.UnitTests/Services/MemoryRoleStoreUniquenessTests.cs
@@ -1,3 +1,4 @@
+using Elsa.Common.Multitenancy;
using Elsa.Common.Services;
using Elsa.Identity.Entities;
using Elsa.Identity.Models;
@@ -7,8 +8,8 @@ using Elsa.Testing.Shared.Multitenancy;
namespace Elsa.Identity.UnitTests.Services;
///
-/// Memory must enforce the same per-tenant role name uniqueness that EF Core
-/// already enforces via PerTenantIdentityUniqueness on (TenantId, Name).
+/// Memory must enforce the same global role ID and per-tenant role name uniqueness
+/// that EF Core already enforces on the Roles table.
///
public class MemoryRoleStoreUniquenessTests
{
@@ -40,6 +41,110 @@ public class MemoryRoleStoreUniquenessTests
Assert.Equal("role-1", Assert.Single(stored).Id);
}
+ [Fact(DisplayName = "AddAsync rejects a role ID already used by another tenant")]
+ public async Task AddAsync_WhenIdExistsInAnotherTenant_Throws()
+ {
+ var backing = new MemoryStore();
+ var tenantA = new MemoryRoleStore(backing, new TestTenantAccessor("tenant-a"));
+ var tenantB = new MemoryRoleStore(backing, new TestTenantAccessor("tenant-b"));
+
+ await tenantA.AddAsync(CreateRole("shared-role", "Tenant A role", "tenant-a"));
+
+ await Assert.ThrowsAsync(() =>
+ tenantB.AddAsync(CreateRole("shared-role", "Tenant B role", "tenant-b")));
+
+ Assert.Equal("tenant-a", (await tenantA.FindAsync(new RoleFilter { Id = "shared-role" }))!.TenantId);
+ Assert.Null(await tenantB.FindAsync(new RoleFilter { Id = "shared-role" }));
+ }
+
+ [Fact(DisplayName = "AddAsync admits only one concurrent writer for a globally unique role ID")]
+ public async Task AddAsync_WhenConcurrentWritersUseTheSameId_AllButOneFailAtomically()
+ {
+ static async Task TryAddAsync(MemoryRoleStore store, Role role)
+ {
+ try
+ {
+ await store.AddAsync(role);
+ return null;
+ }
+ catch (ArgumentException exception)
+ {
+ return exception;
+ }
+ }
+
+ var backing = new MemoryStore();
+ var stores = new[]
+ {
+ new MemoryRoleStore(backing, new TestTenantAccessor("tenant-a")),
+ new MemoryRoleStore(backing, new TestTenantAccessor("tenant-b"))
+ };
+
+ var attempts = Enumerable.Range(0, 32)
+ .Select(index => Task.Run(() => TryAddAsync(
+ stores[index % stores.Length],
+ CreateRole("concurrent-role", $"Role {index}", $"tenant-{(char)('a' + index % stores.Length)}"))))
+ .ToArray();
+ var exceptions = await Task.WhenAll(attempts);
+
+ Assert.Single(exceptions, exception => exception is null);
+ Assert.All(exceptions.Where(exception => exception is not null), exception => Assert.IsType(exception));
+ Assert.Single(backing.List());
+ }
+
+ [Fact(DisplayName = "SaveAsync rejects a role ID already used by another tenant")]
+ public async Task SaveAsync_WhenIdExistsInAnotherTenant_Throws()
+ {
+ var backing = new MemoryStore();
+ var tenantA = new MemoryRoleStore(backing, new TestTenantAccessor("tenant-a"));
+ var tenantB = new MemoryRoleStore(backing, new TestTenantAccessor("tenant-b"));
+
+ var tenantARole = CreateRole("shared-role", "Tenant A role", "tenant-a");
+ tenantARole.Permissions = ["tenant-a:permission"];
+ await tenantA.SaveAsync(tenantARole);
+
+ var exception = await Assert.ThrowsAsync(() =>
+ tenantB.SaveAsync(CreateRole("shared-role", "Tenant B role", "tenant-b")));
+
+ Assert.Contains("already exists", exception.Message);
+ Assert.Equal("tenant-a", (await tenantA.FindAsync(new RoleFilter { Id = "shared-role" }))!.TenantId);
+ Assert.Null(await tenantB.FindAsync(new RoleFilter { Id = "shared-role" }));
+
+ var roleTaggedAsOwner = CreateRole("shared-role", "Tenant A replacement", "tenant-a");
+ roleTaggedAsOwner.Permissions = ["tenant-b:permission"];
+ await Assert.ThrowsAsync(() => tenantB.SaveAsync(roleTaggedAsOwner));
+
+ var roleRehomedFromTenantA = CreateRole("shared-role", "Rehomed role", "tenant-b");
+ roleRehomedFromTenantA.Permissions = ["tenant-b:permission"];
+ await Assert.ThrowsAsync(() => tenantA.SaveAsync(roleRehomedFromTenantA));
+
+ var roleMadeAgnostic = CreateRole("shared-role", "Agnostic role", Tenant.AgnosticTenantId);
+ roleMadeAgnostic.Permissions = ["tenant-b:permission"];
+ await Assert.ThrowsAsync(() => tenantA.SaveAsync(roleMadeAgnostic));
+
+ var unchanged = await tenantA.FindAsync(new RoleFilter { Id = "shared-role" });
+ Assert.Equal("Tenant A role", unchanged!.Name);
+ Assert.Equal("tenant-a", unchanged.TenantId);
+ Assert.Equal(["tenant-a:permission"], unchanged.Permissions);
+ }
+
+ [Fact(DisplayName = "SaveAsync allows a named tenant to update a visible agnostic role")]
+ public async Task SaveAsync_WhenAgnosticRoleIsVisibleToNamedTenant_UpdatesExistingRole()
+ {
+ var backing = new MemoryStore();
+ var agnostic = new MemoryRoleStore(backing, new TestTenantAccessor(Tenant.AgnosticTenantId));
+ var tenantA = new MemoryRoleStore(backing, new TestTenantAccessor("tenant-a"));
+
+ await agnostic.SaveAsync(CreateRole("shared-role", "Shared role", Tenant.AgnosticTenantId));
+
+ await tenantA.SaveAsync(CreateRole("shared-role", "Updated shared role", Tenant.AgnosticTenantId));
+
+ var stored = await agnostic.FindAsync(new RoleFilter { Id = "shared-role" });
+ Assert.Equal("Updated shared role", stored!.Name);
+ Assert.Equal(Tenant.AgnosticTenantId, stored.TenantId);
+ Assert.Equal("Updated shared role", (await tenantA.FindAsync(new RoleFilter { Id = "shared-role" }))!.Name);
+ }
+
[Fact(DisplayName = "SaveAsync allows the same Id to update its own name")]
public async Task SaveAsync_WhenSameIdUpdatesName_Succeeds()
{
@@ -59,11 +164,11 @@ public class MemoryRoleStoreUniquenessTests
var tenantA = new MemoryRoleStore(backing, new TestTenantAccessor("tenant-a"));
var tenantB = new MemoryRoleStore(backing, new TestTenantAccessor("tenant-b"));
- await tenantA.SaveAsync(CreateRole("operators", "Operators", "tenant-a"));
- await tenantB.SaveAsync(CreateRole("operators", "Operators", "tenant-b"));
+ await tenantA.SaveAsync(CreateRole("role-a", "Operators", "tenant-a"));
+ await tenantB.SaveAsync(CreateRole("role-b", "Operators", "tenant-b"));
- Assert.Equal("Operators", (await tenantA.FindAsync(new RoleFilter { Id = "operators" }))!.Name);
- Assert.Equal("Operators", (await tenantB.FindAsync(new RoleFilter { Id = "operators" }))!.Name);
+ Assert.Equal("Operators", (await tenantA.FindAsync(new RoleFilter { Id = "role-a" }))!.Name);
+ Assert.Equal("Operators", (await tenantB.FindAsync(new RoleFilter { Id = "role-b" }))!.Name);
}
[Fact(DisplayName = "SaveAsync leaves the stored name unchanged when a Find result is renamed onto a collision")]
diff --git a/test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs b/test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs
index ff8609a99..37db1c3ab 100644
--- a/test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs
+++ b/test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs
@@ -23,16 +23,18 @@ public class RoleManagerTests
[Fact]
public async Task CreateListUpdateAndDeleteAreIsolatedForRolesWithTheSameNameAcrossTenants()
{
- var roleA = await _manager.CreateRoleAsync("Operators", ["tenant-a:permission"]);
+ var roleA = await _manager.CreateRoleAsync("Operators", ["tenant-a:permission"], "operators-a");
Assert.Equal("tenant-a", roleA.Role.TenantId);
Assert.Single(await _roleStore.FindManyAsync(new() { TenantId = "tenant-a" }));
+ var roleBId = string.Empty;
using (_tenantAccessor.PushContext(new Tenant { Id = "tenant-b", Name = "Tenant B" }))
{
- var roleB = await _manager.CreateRoleAsync("Operators", ["tenant-b:permission"]);
+ var roleB = await _manager.CreateRoleAsync("Operators", ["tenant-b:permission"], "operators-b");
+ roleBId = roleB.Role.Id;
- Assert.Equal(roleA.Role.Id, roleB.Role.Id);
+ Assert.NotEqual(roleA.Role.Id, roleB.Role.Id);
Assert.Equal("tenant-b", roleB.Role.TenantId);
Assert.Equal(["tenant-b:permission"], roleB.Role.Permissions);
@@ -60,7 +62,7 @@ public class RoleManagerTests
using (_tenantAccessor.PushContext(new Tenant { Id = "tenant-b", Name = "Tenant B" }))
{
- var remainingTenantBRole = await _roleStore.FindAsync(new() { Id = roleA.Role.Id });
+ var remainingTenantBRole = await _roleStore.FindAsync(new() { Id = roleBId });
Assert.NotNull(remainingTenantBRole);
Assert.Equal("Operators B", remainingTenantBRole.Name);
}