fix(identity): tenant-safe role stores for Dapper and Mongo (elsa-core#8615)

src/modules/persistence/Elsa.Persistence.Dapper.Migrations/Identity/V3_10.cs

@@ -0,0 +1,128 @@
+using System.Data;
+using System.Diagnostics.CodeAnalysis;
+using System.Text;
+using FluentMigrator;
+using JetBrains.Annotations;
+using Elsa.Persistence.Dapper.Migrations;
+
+namespace Elsa.Persistence.Dapper.Migrations.Identity;
+
+/// <summary>
+/// Adds a unique index on <c>Roles (TenantId, Name)</c> so two tenants can share a role name
+/// while a single tenant cannot (#282 / elsa-core#8615).
+/// </summary>
+/// <remarks>
+/// 3.9 databases have no name uniqueness, so this migration never rewrites or deletes rows.
+/// Same-tenant duplicate names (including case-insensitive pairs, which RoleManager treats
+/// as the same role) fail the migration with the colliding ids. The operator must resolve
+/// those rows and re-run. <c>NULL</c> and <c>''</c> tenant ids are treated as the default
+/// tenant for that check, matching Dapper's read filter. The index itself is on the stored
+/// columns. SQL Server treats NULLs as equal in a unique index (one <c>(NULL, Name)</c>
+/// per name). SQLite, PostgreSQL, MySQL and Oracle treat NULLs as distinct, so a later
+/// <c>NULL</c>/<c>''</c> pair is not rejected by the index (elsa-extensions#245 / #242
+/// normalisation). The index follows the database collation: typically case-insensitive
+/// on SQL Server, case-sensitive on SQLite / PostgreSQL / MySQL / Oracle. Case variants
+/// on SQLite and PostgreSQL therefore rely on core's <c>OrdinalIgnoreCase</c> pre-save
+/// check; the index only rejects exact stored names there.
+/// </remarks>
+[Migration(30005, "Elsa:Identity:V3.10")]
+[PublicAPI]
+[SuppressMessage("ReSharper", "InconsistentNaming")]
+public class V3_10 : Migration
+{
+    /// <summary>
+    /// Unique index on <c>Roles (TenantId, Name)</c>.
+    /// </summary>
+    public const string TenantIdNameUniqueIndex = "IX_Roles_TenantId_Name";
+
+    /// <inheritdoc />
+    public override void Up()
+    {
+        if (!Schema.Table("Roles").Exists() || !Schema.Table("Roles").Column("TenantId").Exists())
+            return;
+
+        if (Schema.Table("Roles").Index(TenantIdNameUniqueIndex).Exists())
+            return;
+
+        IfDatabase(MigrationDatabases.QuotedIdentifiers)
+            .Execute.WithConnection((connection, transaction) =>
+                ThrowIfDuplicateTenantRoleNames(connection, transaction, quoted: true));
+        IfDatabase(MigrationDatabases.UnquotedIdentifiers)
+            .Execute.WithConnection((connection, transaction) =>
+                ThrowIfDuplicateTenantRoleNames(connection, transaction, quoted: false));
+
+        Create.Index(TenantIdNameUniqueIndex)
+            .OnTable("Roles")
+            .OnColumn("TenantId").Ascending()
+            .OnColumn("Name").Ascending()
+            .WithOptions().Unique();
+    }
+
+    /// <inheritdoc />
+    public override void Down()
+    {
+        if (Schema.Table("Roles").Index(TenantIdNameUniqueIndex).Exists())
+            Delete.Index(TenantIdNameUniqueIndex).OnTable("Roles");
+    }
+
+    internal static void ThrowIfDuplicateTenantRoleNames(IDbConnection connection, IDbTransaction? transaction, bool quoted = false)
+    {
+        var roles = ReadRoles(connection, transaction, quoted);
+        var duplicates = roles
+            .GroupBy(role => (TenantKey: NormalizeTenantKey(role.TenantId), NameKey: role.Name.ToLowerInvariant()))
+            .Where(group => group.Count() > 1)
+            .OrderBy(group => group.Key.TenantKey)
+            .ThenBy(group => group.Key.NameKey)
+            .ToList();
+
+        if (duplicates.Count == 0)
+            return;
+
+        var message = new StringBuilder();
+        message.Append("Cannot create unique index ")
+            .Append(TenantIdNameUniqueIndex)
+            .Append(" on Roles (TenantId, Name): the table already has same-tenant duplicate names. ")
+            .Append("No rows were changed. Rename or delete the extra rows and re-run the migration.");
+
+        foreach (var group in duplicates)
+        {
+            message.AppendLine()
+                .Append("  tenant ")
+                .Append(FormatTenant(group.Key.TenantKey))
+                .Append(", name(s) ")
+                .Append(string.Join(" / ", group.Select(role => $"'{role.Name}'").Distinct(StringComparer.Ordinal)))
+                .Append(": ")
+                .Append(string.Join(", ", group.Select(role => $"Id={role.Id}")));
+        }
+
+        throw new InvalidOperationException(message.ToString());
+    }
+
+    private static List<RoleNameRow> ReadRoles(IDbConnection connection, IDbTransaction? transaction, bool quoted)
+    {
+        var rows = new List<RoleNameRow>();
+        using var command = connection.CreateCommand();
+        command.Transaction = transaction;
+        command.CommandText = quoted
+            ? "SELECT \"Id\", \"TenantId\", \"Name\" FROM \"Roles\""
+            : "SELECT Id, TenantId, Name FROM Roles";
+        using var reader = command.ExecuteReader();
+        while (reader.Read())
+        {
+            rows.Add(new RoleNameRow(
+                reader.GetString(0),
+                reader.IsDBNull(1) ? null : reader.GetString(1),
+                reader.GetString(2)));
+        }
+
+        return rows;
+    }
+
+    private static string NormalizeTenantKey(string? tenantId) =>
+        string.IsNullOrEmpty(tenantId) ? string.Empty : tenantId;
+
+    private static string FormatTenant(string tenantKey) =>
+        tenantKey.Length == 0 ? "(default)" : $"'{tenantKey}'";
+
+    private sealed record RoleNameRow(string Id, string? TenantId, string Name);
+}

src/modules/persistence/Elsa.Persistence.Dapper.Migrations/MigrationDatabases.cs

@@ -32,4 +32,25 @@ internal static class MigrationDatabases
         "PostgreSQL",
         "PostgreSQL92"
     ];
+
+    /// <summary>
+    /// Providers whose FluentMigrator tables use quoted identifiers (PostgreSQL ForceQuote, Oracle).
+    /// </summary>
+    public static readonly string[] QuotedIdentifiers =
+    [
+        "Oracle",
+        "Postgres",
+        "PostgreSQL",
+        "PostgreSQL92"
+    ];
+
+    /// <summary>
+    /// Providers whose FluentMigrator tables use unquoted identifiers.
+    /// </summary>
+    public static readonly string[] UnquotedIdentifiers =
+    [
+        "Sqlite",
+        "SqlServer",
+        "MySql"
+    ];
 }

src/modules/persistence/Elsa.Persistence.Dapper/Modules/Identity/Stores/RoleStore.cs

@@ -1,3 +1,4 @@
+using Elsa.Common.Multitenancy;
 using Elsa.Persistence.Dapper.Extensions;
 using Elsa.Persistence.Dapper.Models;
 using Elsa.Persistence.Dapper.Modules.Identity.Records;
@@ -15,10 +16,35 @@ namespace Elsa.Persistence.Dapper.Modules.Identity.Stores;
 internal class DapperRoleStore(Store<RoleRecord> store) : IRoleStore
 {
     /// <inheritdoc />
-    public async Task SaveAsync(Role application, CancellationToken cancellationToken = default)
+    public async Task SaveAsync(Role role, CancellationToken cancellationToken = default)
     {
-        var record = Map(application);
-        await store.SaveAsync(record, cancellationToken);
+        var record = Map(role);
+
+        // Store.SaveAsync upserts on Id alone (SQLite INSERT OR REPLACE / SQL Server MERGE).
+        // That re-homes another tenant's row when ids collide, which is what happened when
+        // role ids were derived from the name. Only update a row this tenant already owns;
+        // otherwise insert, or refuse if the id belongs to a different tenant.
+        var owned = await store.FindAsync(q => q.Is(nameof(RoleRecord.Id), record.Id), tenantAgnostic: false, cancellationToken);
+        if (owned != null)
+        {
+            await store.UpdateAsync(
+                record,
+                [x => x.Name, x => x.Permissions],
+                q => q.Is(nameof(RoleRecord.Id), record.Id),
+                cancellationToken);
+            return;
+        }
+
+        var existing = await store.FindAsync(q => q.Is(nameof(RoleRecord.Id), record.Id), tenantAgnostic: true, cancellationToken);
+        if (existing != null)
+        {
+            var message = string.Equals(existing.TenantId, Tenant.AgnosticTenantId, StringComparison.Ordinal)
+                ? $"A role with ID '{record.Id}' already exists as a shared ('*') role."
+                : $"A role with ID '{record.Id}' already exists in another tenant.";
+            throw new InvalidOperationException(message);
+        }
+
+        await store.AddAsync(record, cancellationToken);
     }
 
     /// <inheritdoc />
@@ -52,8 +78,8 @@ private void ApplyFilter(ParameterizedQuery query, RoleFilter filter)
     {
         query
             .Is(nameof(RoleRecord.Id), filter.Id)
-            .In(nameof(RoleRecord.Name), filter.Ids)
-            ;   
+            .In(nameof(RoleRecord.Id), filter.Ids)
+            ;
     }
 
     private RoleRecord Map(Role source)
@@ -77,4 +103,4 @@ private Role Map(RoleRecord source)
             TenantId = source.TenantId
         };
     }
-}
+}

src/modules/persistence/Elsa.Persistence.MongoDb/Elsa.Persistence.MongoDb.csproj

@@ -28,4 +28,8 @@
         <ProjectReference Include="..\..\..\..\..\elsa-core\src\modules\Elsa.Workflows.Runtime\Elsa.Workflows.Runtime.csproj"/>
     </ItemGroup>
 
+    <ItemGroup>
+        <InternalsVisibleTo Include="Elsa.MongoDb.UnitTests" />
+    </ItemGroup>
+
 </Project>

src/modules/persistence/Elsa.Persistence.MongoDb/Modules/Identity/CreateIndices.cs

@@ -1,11 +1,28 @@
+using System.Linq;
 using Elsa.Identity.Entities;
 using Elsa.Persistence.MongoDb.Helpers;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
+using MongoDB.Bson;
 using MongoDB.Driver;
 
 namespace Elsa.Persistence.MongoDb.Modules.Identity;
 
+internal static class IdentityRoleIndexes
+{
+    /// <summary>
+    /// Store-wide unique index on <see cref="Role.Name"/> created by 3.9.0 and earlier.
+    /// </summary>
+    public const string LegacyNameUnique = "Name_1";
+
+    /// <summary>
+    /// Per-tenant unique index on (TenantId, Name).
+    /// </summary>
+    public const string TenantIdNameUnique = "TenantId_1_Name_1";
+}
+
 internal class CreateIndices(IServiceProvider serviceProvider) : IHostedService
 {
     public Task StartAsync(CancellationToken cancellationToken)
@@ -74,19 +91,61 @@ private static Task CreateRoleIndices(IServiceScope serviceScope, CancellationTo
         var roleCollection = serviceScope.ServiceProvider.GetService<IMongoCollection<Role>>();
         if (roleCollection == null) return Task.CompletedTask;
 
+        var logger = serviceScope.ServiceProvider.GetService<ILogger<CreateIndices>>() ?? NullLogger<CreateIndices>.Instance;
+
         return IndexHelpers.CreateAsync(
             roleCollection,
             async (collection, indexBuilder) =>
-                await collection.Indexes.CreateManyAsync(
-                    new List<CreateIndexModel<Role>>
-                    {
-                        new(indexBuilder.Ascending(x => x.Name),
+            {
+                var existingNames = await ListIndexNamesAsync(collection, cancellationToken);
+                if (existingNames.Contains(IdentityRoleIndexes.TenantIdNameUnique))
+                {
+                    logger.LogDebug("Role unique index '{IndexName}' is already present.", IdentityRoleIndexes.TenantIdNameUnique);
+                }
+                else
+                {
+                    await collection.Indexes.CreateOneAsync(
+                        new CreateIndexModel<Role>(
+                            indexBuilder.Ascending(x => x.TenantId).Ascending(x => x.Name),
                             new CreateIndexOptions
                             {
-                                Unique = true
+                                Unique = true,
+                                Name = IdentityRoleIndexes.TenantIdNameUnique
                             }),
+                        cancellationToken: cancellationToken);
+                    logger.LogDebug("Created Role unique index '{IndexName}' on (TenantId, Name).", IdentityRoleIndexes.TenantIdNameUnique);
+                }
+
+                // Create the compound unique index before dropping Name_1 so two nodes
+                // starting together never leave the collection without name uniqueness,
+                // and a second DropOne of Name_1 is IndexNotFound (code 27), not startup failure.
+                try
+                {
+                    await collection.Indexes.DropOneAsync(IdentityRoleIndexes.LegacyNameUnique, cancellationToken);
+                    logger.LogInformation("Dropped Role unique index '{IndexName}'.", IdentityRoleIndexes.LegacyNameUnique);
+                }
+                catch (MongoCommandException exception) when (exception.Code == 27 || exception.InnerException is MongoCommandException { Code: 27 })
+                {
+                    logger.LogDebug("Role unique index '{IndexName}' was not found.", IdentityRoleIndexes.LegacyNameUnique);
+                }
+
+                await collection.Indexes.CreateManyAsync(
+                    new List<CreateIndexModel<Role>>
+                    {
                         new(indexBuilder.Ascending(x => x.TenantId))
                     },
-                    cancellationToken));
+                    cancellationToken);
+            });
+    }
+
+    private static async Task<HashSet<string>> ListIndexNamesAsync<T>(IMongoCollection<T> collection, CancellationToken cancellationToken)
+    {
+        var names = new HashSet<string>(StringComparer.Ordinal);
+        using var cursor = await collection.Indexes.ListAsync(cancellationToken);
+        foreach (var index in (await cursor.ToListAsync(cancellationToken))
+                 .Where(index => index.TryGetValue("name", out var name) && name.BsonType == BsonType.String))
+            names.Add(index["name"].AsString);
+
+        return names;
     }
-}
+}

test/modules/persistence/Elsa.MongoDb.UnitTests/Elsa.MongoDb.UnitTests.csproj

@@ -5,6 +5,8 @@
     </PropertyGroup>
 
 	<ItemGroup>
+	  <PackageReference Include="Microsoft.Extensions.DependencyInjection" />
+	  <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
 	  <PackageReference Include="Testcontainers.MongoDb" />
 	  <ProjectReference Include="..\..\..\..\src\modules\persistence\Elsa.Persistence.MongoDb\Elsa.Persistence.MongoDb.csproj" />
 	</ItemGroup>