SemianiakaVY/elsa-core
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

Fix MemoryRoleStore global role ID uniqueness (#8127)

Browse source 
* fix(identity): enforce global role ID uniqueness

* fix(identity): preserve role tenant on updates

* test(identity): cover role tenant rehoming
This commit is contained in:
Sipke Schoorstra 2026-09-13 23:28:38 +02:00 committed by GitHub
parent d4c00f13ed
commit 815d7b7f70
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 239 additions and 99 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
src/modules/Elsa.ExternalAuthentication/Services/ExternalAuthenticationRoleDeletionDependencyContributor.cs
View file

@ -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 <c>MemoryRoleStore</c> 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
/// <c>MemoryRoleStore</c>) is rejected as not agnostic rather than guessed at, so it is reported as
/// <c>replacement_role_unavailable_or_unauthorized</c> instead of surfacing as an exception.
/// In EF Core persistence and <c>MemoryRoleStore</c>, 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 <see cref="IRoleStore"/> 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 <c>replacement_role_unavailable_or_unauthorized</c>
/// instead of surfacing as an exception.
/// </remarks>
public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
IIdentityProviderConnectionStore store,
@ -262,13 +261,13 @@ public sealed class ExternalAuthenticationRoleDeletionDependencyContributor(
/// <summary>
/// Resolves the tenant context for the role being deleted: the role's own <c>TenantId</c>
/// (<see cref="Tenant.AgnosticTenantId"/> 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
/// <c>Roles</c> table keys on <c>Id</c> alone), so this lookup resolves to at most one role there. Only
/// <c>MemoryRoleStore</c> 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 <c>MemoryRoleStore</c>, a role ID is unique
/// across all tenants (the <c>Roles</c> table and in-memory store both key on <c>Id</c> 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 <see cref="ITenantAccessor"/>,
/// 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(
/// <summary>
/// Resolves whether a candidate role ID (typically a replacement role) is itself tenant-agnostic. Unlike
/// <see cref="ResolveRoleTenantIdAsync"/>, an ambiguous candidate -- a role ID that resolves to more than one
/// role, which only <c>MemoryRoleStore</c> 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.
/// </summary>
private async ValueTask<bool> IsAgnosticRoleAsync(string? roleId, CancellationToken cancellationToken)
{

51
src/modules/Elsa.Identity/Services/MemoryRoleStore.cs
View file

@ -26,15 +26,24 @@ public class MemoryRoleStore : IRoleStore, IRoleStoreWithAtomicDelete
/// <inheritdoc />
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;
}
/// <inheritdoc />
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
/// </remarks>
public Task<bool> 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);
}
}
/// <inheritdoc />
@ -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}'.");
}
}
/// <summary>
/// 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 <c>TenantId</c>.
/// </summary>
private bool CanReplace(Role existing, Role replacement) =>
string.Equals(existing.TenantId, replacement.TenantId, StringComparison.Ordinal) &&
TenantVisibility.IsVisible(existing.TenantId, _tenantAccessor.TenantId);
/// <inheritdoc />
public Task<Role?> 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}";
}

121
test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationRoleDeletionDependencyContributorTests.cs
View file

@ -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<InvalidOperationException>(() => 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<RoleReferenceRemovalValidationResult.Forbidden>(validation);
Assert.Equal("replacement_role_unavailable_or_unauthorized", forbidden.Code);
var result = await contributor.RemoveEditableReferencesAsync(request);
var failed = Assert.IsType<RoleReferenceRemovalResult.Failed>(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<RoleReferenceRemovalValidationResult.Forbidden>(validation);
Assert.Equal("replacement_role_unavailable_or_unauthorized", forbidden.Code);
var result = await contributor.RemoveEditableReferencesAsync(request);
var failed = Assert.IsType<RoleReferenceRemovalResult.Failed>(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<IdentityProviderConnection> configuredConnections,
params IdentityProviderConnection[] databaseConnections) =>
@ -671,18 +671,22 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
IdentityProviderConnection[] databaseConnections,
IReadOnlyCollection<Role>? additionalRoles = null,
ITenantAccessor? tenantAccessor = null,
Func<InMemoryIdentityProviderConnectionStore, IIdentityProviderConnectionStore>? decorateStore = null)
Func<InMemoryIdentityProviderConnectionStore, IIdentityProviderConnectionStore>? decorateStore = null,
IRoleStore? roleStoreOverride = null)
{
var store = new InMemoryIdentityProviderConnectionStore();
foreach (var connection in databaseConnections)
Assert.IsType<ConnectionMutationResult.Created>(await store.CreateAsync(connection));
var accessor = tenantAccessor ?? TestTenantAccessor.Default;
var roleStore = new MemoryRoleStore(new MemoryStore<Role>(), 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<Role>(), 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
/// <summary>
/// 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. <see cref="MemoryRoleStore"/> 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. <see cref="MemoryRoleStore"/>
/// cannot exercise those scenarios because it keys rows by ID and filters by the ambient tenant itself.
/// </summary>
private sealed class RoleStoreWithoutAmbientTenantFilter(IReadOnlyCollection<Role> roles) : IRoleStore
{
@ -789,10 +794,12 @@ public class ExternalAuthenticationRoleDeletionDependencyContributorTests
public Task SaveAsync(Role role, CancellationToken cancellationToken = default) => throw new NotSupportedException();
public Task<Role?> FindAsync(RoleFilter filter, CancellationToken cancellationToken = default) =>
Task.FromResult(roles.FirstOrDefault(x => x.Id == filter.Id));
Task.FromResult(ApplyFilter(filter).FirstOrDefault());
public Task<IEnumerable<Role>> FindManyAsync(RoleFilter filter, CancellationToken cancellationToken = default) =>
Task.FromResult(roles.Where(x => x.Id == filter.Id));
Task.FromResult<IEnumerable<Role>>(ApplyFilter(filter).ToArray());
private IQueryable<Role> ApplyFilter(RoleFilter filter) => filter.Apply(roles.AsQueryable());
}
private sealed class RoleStoreThatRemovesReplacementAfterContributorValidation(

117
test/unit/Elsa.Identity.UnitTests/Services/MemoryRoleStoreUniquenessTests.cs
View file

@ -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;
/// <summary>
/// Memory must enforce the same per-tenant role name uniqueness that EF Core
/// already enforces via <c>PerTenantIdentityUniqueness</c> on <c>(TenantId, Name)</c>.
/// Memory must enforce the same global role ID and per-tenant role name uniqueness
/// that EF Core already enforces on the Roles table.
/// </summary>
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<Role>();
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<ArgumentException>(() =>
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<Exception?> TryAddAsync(MemoryRoleStore store, Role role)
{
try
{
await store.AddAsync(role);
return null;
}
catch (ArgumentException exception)
{
return exception;
}
}
var backing = new MemoryStore<Role>();
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<ArgumentException>(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<Role>();
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<InvalidOperationException>(() =>
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<InvalidOperationException>(() => tenantB.SaveAsync(roleTaggedAsOwner));
var roleRehomedFromTenantA = CreateRole("shared-role", "Rehomed role", "tenant-b");
roleRehomedFromTenantA.Permissions = ["tenant-b:permission"];
await Assert.ThrowsAsync<InvalidOperationException>(() => tenantA.SaveAsync(roleRehomedFromTenantA));
var roleMadeAgnostic = CreateRole("shared-role", "Agnostic role", Tenant.AgnosticTenantId);
roleMadeAgnostic.Permissions = ["tenant-b:permission"];
await Assert.ThrowsAsync<InvalidOperationException>(() => 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<Role>();
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")]

10
test/unit/Elsa.Identity.UnitTests/Services/RoleManagerTests.cs
View file

@ -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);
}