diff --git a/doc/migrations/secrets-tenancy.md b/doc/migrations/secrets-tenancy.md index 5b7f4112a..d33c64139 100644 --- a/doc/migrations/secrets-tenancy.md +++ b/doc/migrations/secrets-tenancy.md @@ -9,8 +9,10 @@ against which tenant owns the secret, so any caller holding it reached the whole ## What you have to do -Apply the `SecretTenancy` migration for your provider. There is no data step, and nothing else is required for -a single-tenant deployment. +Apply the `SecretTenancy` and `SecretDefaultTenantUniqueness` migrations for your provider. The second +migration stamps leftover null `TenantId` values to `""` (the default tenant) and rebuilds the unique +index. It does **not** delete duplicate names — if two leftover rows share a name in the same tenant, +the upgrade aborts and lists the keys. Reconcile those rows, then re-run. ## Existing secrets @@ -46,32 +48,28 @@ the first tenant to claim a name took it globally. Downgrading recreates the global unique index and **will fail if two tenants hold the same secret name by then**. Reconcile the duplicates first. -### Null-tenant rows sit outside the index +### Default-tenant rows use `""` -"Unique per tenant" is enforced by the database only for rows whose `TenantId` is non-null. SQL Server -creates the composite index with a `[TenantId] IS NOT NULL` filter, and SQLite, PostgreSQL and MySQL treat -nulls as distinct in unique indexes — either way, null-tenant rows never collide in it. Only Oracle, where -equal nulls do count as duplicates in a composite unique index, still rejects them. +The default tenant id is `""` (`Tenant.DefaultTenantId`), the same sentinel Labels uses. New EF writes that +would have left `TenantId` null now persist `""`, so they participate in `(TenantId, NormalizedName)`. -This matters more than it sounds, because null is the common case. With multitenancy **disabled** — the -default single-tenant deployment — nothing ever assigns a `TenantId`, so every row keeps null and the schema -no longer enforces secret-name uniqueness at all. Uniqueness then rests on the repository's read-before-write -check, which blocks sequential duplicates but not two concurrent creates racing past it. The old global index -was the backstop for exactly that race; accepting its loss for null rows is a consequence of the no-backfill -decision above. The same gap applies in a multi-tenant deployment's default tenant: pre-upgrade null rows and -new `""`-tenant rows are distinct index keys, so the index cannot stop a new default-tenant secret from -colliding by name with a legacy row. +`SecretDefaultTenantUniqueness` stamps leftover nulls from `SecretTenancy` to `""` and rebuilds that unique +index. SQL Server still creates the composite index with a `TenantId IS NOT NULL` filter; after the stamp +those rows are non-null, so they collide in it. SQLite, PostgreSQL and MySQL treat nulls as distinct — the +stamp is what brings them into the index. -Backfilling `TenantId` to `""` yourself does **not** restore the database guarantee, and it is worth being -precise about why. The backfill indexes the rows that exist when you run it, but nothing changes what happens -afterwards: with multitenancy disabled no `TenantId` is ever assigned, so every subsequent write still lands -as a null row outside the index. Two concurrent creates can still both pass the repository's read-before-write -check and commit the same name. Restoring the guarantee for real would mean making disabled-mode writes use -the same non-null sentinel the index is built on — Elsa does not do that, and the backfill alone does not -substitute for it. Until it does, treat the read-before-write check as the only protection in single-tenant -mode and serialize secret creation if you cannot tolerate the race. (The `SetTenantIdFilter` -null-compatibility clause keeps backfilled and straggler rows visible either way, so a backfill is still -useful for de-duplicating what you already have.) +Oracle is different: it stores `''` as `NULL`, so the stamp is a no-op and a `TenantId IS NOT NULL` filter +would leave every default-tenant row outside the index. That provider creates a function-based unique index +on `NVL(TenantId, CHR(1))` plus `NormalizedName` instead. `CHR(1)` is not a tenant id; it only exists so +NULL default-tenant names share one index key. The EF model snapshot cannot express `NVL`, so it looks like +the other unfiltered unique indexes; the physical Oracle index is the function-based one from this migration. + +The migration does not delete duplicates: it lists leftover `(TenantId, NormalizedName)` keys and aborts, +then the unique index fails loudly if any remain. + +`SetTenantIdFilter`'s null-compatibility clause still treats a stray null as the default tenant, so a row +that somehow remains unstamped stays visible there. File and InMemory already treat null and `""` as the +same tenant for uniqueness. ## The MySQL provider ships for net8.0 and net9.0 only diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.MySql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.MySql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs new file mode 100644 index 000000000..ea80de906 --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.MySql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs @@ -0,0 +1,113 @@ +// +using System; +using Elsa.Secrets.Persistence.EFCore; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.MySql.Migrations.Secrets +{ + [DbContext(typeof(SecretsElsaDbContext))] + [Migration("20260914120000_SecretDefaultTenantUniqueness")] + partial class SecretDefaultTenantUniqueness + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasDefaultSchema("Elsa") + .HasAnnotation("ProductVersion", "9.0.17") + .HasAnnotation("Relational:MaxIdentifierLength", 64); + + MySqlModelBuilderExtensions.AutoIncrementColumns(modelBuilder); + + modelBuilder.Entity("Elsa.Secrets.Models.Secret", b => + { + b.Property("Id") + .HasColumnType("varchar(255)"); + + b.Property("CreatedAt") + .HasColumnType("datetime(6)"); + + b.Property("Description") + .HasColumnType("longtext"); + + b.Property("DisplayName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("varchar(200)"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("varchar(200)"); + + b.Property("NormalizedName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("varchar(200)"); + + b.Property("Scope") + .HasMaxLength(200) + .HasColumnType("varchar(200)"); + + b.Property("SerializedTags") + .IsRequired() + .HasColumnType("longtext") + .HasColumnName("Tags"); + + b.Property("SerializedVersions") + .IsRequired() + .HasColumnType("longtext") + .HasColumnName("Versions"); + + b.Property("Status") + .IsRequired() + .HasMaxLength(32) + .HasColumnType("varchar(32)"); + + b.Property("StoreName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("varchar(100)"); + + b.Property("TenantId") + .HasColumnType("varchar(255)"); + + b.Property("TypeName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("varchar(100)"); + + b.Property("UpdatedAt") + .HasColumnType("datetime(6)"); + + b.HasKey("Id"); + + b.HasIndex("Scope") + .HasDatabaseName("IX_Secret_Scope"); + + b.HasIndex("Status") + .HasDatabaseName("IX_Secret_Status"); + + b.HasIndex("StoreName") + .HasDatabaseName("IX_Secret_StoreName"); + + b.HasIndex("TypeName") + .HasDatabaseName("IX_Secret_TypeName"); + + b.HasIndex("TenantId", "NormalizedName") + .IsUnique() + .HasDatabaseName("IX_Secret_TenantId_NormalizedName"); + + b.ToTable("Secrets", "Elsa"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.MySql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.MySql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs new file mode 100644 index 000000000..bff67db1c --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.MySql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs @@ -0,0 +1,87 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.MySql.Migrations.Secrets +{ + /// + public partial class SecretDefaultTenantUniqueness : Migration + { + private readonly Elsa.Persistence.EFCore.IElsaDbContextSchema _schema; + + /// + public SecretDefaultTenantUniqueness(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) + { + _schema = schema; + } + + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + // Default tenant is "" (not null). Stamp leftover nulls so the unique index covers them. + migrationBuilder.Sql($""" + UPDATE `{_schema.Schema}`.`Secrets` + SET `TenantId` = '' + WHERE `TenantId` IS NULL; + """); + + // No silent dedupe. List leftover keys and abort; operators must resolve them before upgrading. + // SIGNAL MESSAGE_TEXT is limited to 128 characters, so the listed keys are truncated. + migrationBuilder.Sql($""" + SET @duplicate_keys := ( + SELECT GROUP_CONCAT(CONCAT('(', IFNULL(`TenantId`, ''), ', ', `NormalizedName`, ')') SEPARATOR '; ') + FROM ( + SELECT `TenantId`, `NormalizedName` + FROM `{_schema.Schema}`.`Secrets` + GROUP BY `TenantId`, `NormalizedName` + HAVING COUNT(*) > 1 + ) AS Duplicates + ); + + SET @sql := IF( + @duplicate_keys IS NOT NULL, + CONCAT( + 'SIGNAL SQLSTATE ''45000'' SET MESSAGE_TEXT = ''', + LEFT(REPLACE(CONCAT( + 'Duplicate keys. Resolve before upgrade: ', + @duplicate_keys + ), '''', ''), 128), + '''' + ), + 'SELECT 1' + ); + PREPARE stmt FROM @sql; + EXECUTE stmt; + DEALLOCATE PREPARE stmt; + """); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true); + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs new file mode 100644 index 000000000..71a875241 --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs @@ -0,0 +1,113 @@ +// +using System; +using Elsa.Secrets.Persistence.EFCore; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; +using Oracle.EntityFrameworkCore.Metadata; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.Oracle.Migrations.Secrets +{ + [DbContext(typeof(SecretsElsaDbContext))] + [Migration("20260914120000_SecretDefaultTenantUniqueness")] + partial class SecretDefaultTenantUniqueness + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasDefaultSchema("Elsa") + .HasAnnotation("ProductVersion", "10.0.9") + .HasAnnotation("Relational:MaxIdentifierLength", 128); + + OracleModelBuilderExtensions.UseIdentityColumns(modelBuilder); + + modelBuilder.Entity("Elsa.Secrets.Models.Secret", b => + { + b.Property("Id") + .HasColumnType("NVARCHAR2(450)"); + + b.Property("CreatedAt") + .HasColumnType("TIMESTAMP(7) WITH TIME ZONE"); + + b.Property("Description") + .HasColumnType("NVARCHAR2(2000)"); + + b.Property("DisplayName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("NVARCHAR2(200)"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("NVARCHAR2(200)"); + + b.Property("NormalizedName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("NVARCHAR2(200)"); + + b.Property("Scope") + .HasMaxLength(200) + .HasColumnType("NVARCHAR2(200)"); + + b.Property("SerializedTags") + .IsRequired() + .HasColumnType("NCLOB") + .HasColumnName("Tags"); + + b.Property("SerializedVersions") + .IsRequired() + .HasColumnType("NCLOB") + .HasColumnName("Versions"); + + b.Property("Status") + .IsRequired() + .HasMaxLength(32) + .HasColumnType("NVARCHAR2(32)"); + + b.Property("StoreName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("NVARCHAR2(100)"); + + b.Property("TenantId") + .HasColumnType("NVARCHAR2(450)"); + + b.Property("TypeName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("NVARCHAR2(100)"); + + b.Property("UpdatedAt") + .HasColumnType("TIMESTAMP(7) WITH TIME ZONE"); + + b.HasKey("Id"); + + b.HasIndex("Scope") + .HasDatabaseName("IX_Secret_Scope"); + + b.HasIndex("Status") + .HasDatabaseName("IX_Secret_Status"); + + b.HasIndex("StoreName") + .HasDatabaseName("IX_Secret_StoreName"); + + b.HasIndex("TypeName") + .HasDatabaseName("IX_Secret_TypeName"); + + b.HasIndex("TenantId", "NormalizedName") + .IsUnique() + .HasDatabaseName("IX_Secret_TenantId_NormalizedName"); + + b.ToTable("Secrets", "Elsa"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs new file mode 100644 index 000000000..ff94cfe76 --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs @@ -0,0 +1,98 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.Oracle.Migrations.Secrets +{ + /// + public partial class SecretDefaultTenantUniqueness : Migration + { + private readonly Elsa.Persistence.EFCore.IElsaDbContextSchema _schema; + + /// + public SecretDefaultTenantUniqueness(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) + { + _schema = schema; + } + + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + // Oracle persists '' as NULL, so this stamp is a no-op. It is kept so leftover + // default-tenant rows stay in one GROUP BY bucket for the fail-loud preflight. + // Uniqueness is the NVL unique index below, not a TenantId IS NOT NULL filter. + migrationBuilder.Sql($""" + UPDATE "{_schema.Schema}"."Secrets" + SET "TenantId" = '' + WHERE "TenantId" IS NULL; + """); + + // No silent dedupe. List leftover keys and abort; operators must resolve them before upgrading. + migrationBuilder.Sql($""" + DECLARE + duplicate_keys VARCHAR2(4000); + duplicate_count NUMBER; + BEGIN + SELECT COUNT(*) + INTO duplicate_count + FROM ( + SELECT "TenantId", "NormalizedName" + FROM "{_schema.Schema}"."Secrets" + GROUP BY "TenantId", "NormalizedName" + HAVING COUNT(*) > 1 + ); + + IF duplicate_count > 0 THEN + SELECT LISTAGG('(' || NVL("TenantId", '') || ', ' || "NormalizedName" || ')', '; ' ON OVERFLOW TRUNCATE) + WITHIN GROUP (ORDER BY "NormalizedName") + INTO duplicate_keys + FROM ( + SELECT "TenantId", "NormalizedName" + FROM "{_schema.Schema}"."Secrets" + GROUP BY "TenantId", "NormalizedName" + HAVING COUNT(*) > 1 + ); + + RAISE_APPLICATION_ERROR( + -20001, + SUBSTR( + 'Cannot create unique index IX_Secret_TenantId_NormalizedName because leftover duplicate (TenantId, NormalizedName) rows exist. Operators must resolve leftover (TenantId, NormalizedName) rows before upgrade. Duplicate keys: ' || duplicate_keys, + 1, + 2048 + ) + ); + END IF; + END; + """); + + // CHR(1) is not a tenant id. NVL maps Oracle's NULL (from '') onto one index key + // so two default-tenant rows with the same NormalizedName collide. + migrationBuilder.Sql($""" + CREATE UNIQUE INDEX "{_schema.Schema}"."IX_Secret_TenantId_NormalizedName" + ON "{_schema.Schema}"."Secrets" (NVL("TenantId", CHR(1)), "NormalizedName"); + """); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true, + filter: "\"TenantId\" IS NOT NULL"); + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/SecretsElsaDbContextModelSnapshot.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/SecretsElsaDbContextModelSnapshot.cs index babf70075..f365b296a 100644 --- a/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/SecretsElsaDbContextModelSnapshot.cs +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.Oracle/Migrations/Secrets/SecretsElsaDbContextModelSnapshot.cs @@ -100,8 +100,7 @@ namespace Elsa.Secrets.Persistence.EFCore.Oracle.Migrations.Secrets b.HasIndex("TenantId", "NormalizedName") .IsUnique() - .HasDatabaseName("IX_Secret_TenantId_NormalizedName") - .HasFilter("\"TenantId\" IS NOT NULL"); + .HasDatabaseName("IX_Secret_TenantId_NormalizedName"); b.ToTable("Secrets", "Elsa"); }); diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.PostgreSql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.PostgreSql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs new file mode 100644 index 000000000..b3976f33f --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.PostgreSql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs @@ -0,0 +1,113 @@ +// +using System; +using Elsa.Secrets.Persistence.EFCore; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; +using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.PostgreSql.Migrations.Secrets +{ + [DbContext(typeof(SecretsElsaDbContext))] + [Migration("20260914120000_SecretDefaultTenantUniqueness")] + partial class SecretDefaultTenantUniqueness + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasDefaultSchema("Elsa") + .HasAnnotation("ProductVersion", "10.0.9") + .HasAnnotation("Relational:MaxIdentifierLength", 63); + + NpgsqlModelBuilderExtensions.UseIdentityByDefaultColumns(modelBuilder); + + modelBuilder.Entity("Elsa.Secrets.Models.Secret", b => + { + b.Property("Id") + .HasColumnType("text"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("Description") + .HasColumnType("text"); + + b.Property("DisplayName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("NormalizedName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("Scope") + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("SerializedTags") + .IsRequired() + .HasColumnType("text") + .HasColumnName("Tags"); + + b.Property("SerializedVersions") + .IsRequired() + .HasColumnType("text") + .HasColumnName("Versions"); + + b.Property("Status") + .IsRequired() + .HasMaxLength(32) + .HasColumnType("character varying(32)"); + + b.Property("StoreName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("TenantId") + .HasColumnType("text"); + + b.Property("TypeName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.HasKey("Id"); + + b.HasIndex("Scope") + .HasDatabaseName("IX_Secret_Scope"); + + b.HasIndex("Status") + .HasDatabaseName("IX_Secret_Status"); + + b.HasIndex("StoreName") + .HasDatabaseName("IX_Secret_StoreName"); + + b.HasIndex("TypeName") + .HasDatabaseName("IX_Secret_TypeName"); + + b.HasIndex("TenantId", "NormalizedName") + .IsUnique() + .HasDatabaseName("IX_Secret_TenantId_NormalizedName"); + + b.ToTable("Secrets", "Elsa"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.PostgreSql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.PostgreSql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs new file mode 100644 index 000000000..c6827765e --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.PostgreSql/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs @@ -0,0 +1,77 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.PostgreSql.Migrations.Secrets +{ + /// + public partial class SecretDefaultTenantUniqueness : Migration + { + private readonly Elsa.Persistence.EFCore.IElsaDbContextSchema _schema; + + /// + public SecretDefaultTenantUniqueness(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) + { + _schema = schema; + } + + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + // Default tenant is "" (not null). Stamp leftover nulls so the unique index covers them. + migrationBuilder.Sql($""" + UPDATE "{_schema.Schema}"."Secrets" + SET "TenantId" = '' + WHERE "TenantId" IS NULL; + """); + + // No silent dedupe. List leftover keys and abort; operators must resolve them before upgrading. + migrationBuilder.Sql($""" + DO $$ + DECLARE keys text; + BEGIN + SELECT string_agg(format('(%s, %s)', "TenantId", "NormalizedName"), ', ') + INTO keys + FROM ( + SELECT "TenantId", "NormalizedName" + FROM "{_schema.Schema}"."Secrets" + GROUP BY "TenantId", "NormalizedName" + HAVING COUNT(*) > 1 + ) d; + + IF keys IS NOT NULL THEN + RAISE EXCEPTION 'Cannot apply IX_Secret_TenantId_NormalizedName. Operators must resolve leftover (TenantId, NormalizedName) rows before upgrade. Duplicate keys: %', keys; + END IF; + END $$; + """); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true); + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.SqlServer/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.SqlServer/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs new file mode 100644 index 000000000..8f87dae71 --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.SqlServer/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs @@ -0,0 +1,114 @@ +// +using System; +using Elsa.Secrets.Persistence.EFCore; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.SqlServer.Migrations.Secrets +{ + [DbContext(typeof(SecretsElsaDbContext))] + [Migration("20260914120000_SecretDefaultTenantUniqueness")] + partial class SecretDefaultTenantUniqueness + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasDefaultSchema("Elsa") + .HasAnnotation("ProductVersion", "10.0.9") + .HasAnnotation("Relational:MaxIdentifierLength", 128); + + SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder); + + modelBuilder.Entity("Elsa.Secrets.Models.Secret", b => + { + b.Property("Id") + .HasColumnType("nvarchar(450)"); + + b.Property("CreatedAt") + .HasColumnType("datetimeoffset"); + + b.Property("Description") + .HasColumnType("nvarchar(max)"); + + b.Property("DisplayName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("nvarchar(200)"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("nvarchar(200)"); + + b.Property("NormalizedName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("nvarchar(200)"); + + b.Property("Scope") + .HasMaxLength(200) + .HasColumnType("nvarchar(200)"); + + b.Property("SerializedTags") + .IsRequired() + .HasColumnType("nvarchar(max)") + .HasColumnName("Tags"); + + b.Property("SerializedVersions") + .IsRequired() + .HasColumnType("nvarchar(max)") + .HasColumnName("Versions"); + + b.Property("Status") + .IsRequired() + .HasMaxLength(32) + .HasColumnType("nvarchar(32)"); + + b.Property("StoreName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("nvarchar(100)"); + + b.Property("TenantId") + .HasColumnType("nvarchar(450)"); + + b.Property("TypeName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("nvarchar(100)"); + + b.Property("UpdatedAt") + .HasColumnType("datetimeoffset"); + + b.HasKey("Id"); + + b.HasIndex("Scope") + .HasDatabaseName("IX_Secret_Scope"); + + b.HasIndex("Status") + .HasDatabaseName("IX_Secret_Status"); + + b.HasIndex("StoreName") + .HasDatabaseName("IX_Secret_StoreName"); + + b.HasIndex("TypeName") + .HasDatabaseName("IX_Secret_TypeName"); + + b.HasIndex("TenantId", "NormalizedName") + .IsUnique() + .HasDatabaseName("IX_Secret_TenantId_NormalizedName") + .HasFilter("[TenantId] IS NOT NULL"); + + b.ToTable("Secrets", "Elsa"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.SqlServer/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.SqlServer/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs new file mode 100644 index 000000000..d9aa9d316 --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.SqlServer/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs @@ -0,0 +1,89 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.SqlServer.Migrations.Secrets +{ + /// + public partial class SecretDefaultTenantUniqueness : Migration + { + private readonly Elsa.Persistence.EFCore.IElsaDbContextSchema _schema; + + /// + public SecretDefaultTenantUniqueness(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) + { + _schema = schema; + } + + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + // Default tenant is "" (not null). Stamp leftover nulls so the filtered unique + // index (TenantId IS NOT NULL) covers them. Duplicates are not deleted. + migrationBuilder.Sql($""" + UPDATE [{_schema.Schema}].[Secrets] + SET [TenantId] = N'' + WHERE [TenantId] IS NULL; + """); + + // No silent dedupe. List leftover keys and abort; operators must resolve them before upgrading. + migrationBuilder.Sql($""" + IF EXISTS ( + SELECT 1 + FROM [{_schema.Schema}].[Secrets] + GROUP BY [TenantId], [NormalizedName] + HAVING COUNT(*) > 1 + ) + BEGIN + DECLARE @DuplicateKeys nvarchar(max); + + SELECT @DuplicateKeys = STRING_AGG( + CAST(CONCAT(N'(', ISNULL([TenantId], N''), N', ', [NormalizedName], N')') AS nvarchar(max)), + N'; ' + ) + FROM ( + SELECT [TenantId], [NormalizedName] + FROM [{_schema.Schema}].[Secrets] + GROUP BY [TenantId], [NormalizedName] + HAVING COUNT(*) > 1 + ) AS Duplicates; + + THROW 50001, CONCAT( + N'Cannot create unique index IX_Secret_TenantId_NormalizedName because leftover duplicate (TenantId, NormalizedName) rows exist. Operators must resolve leftover (TenantId, NormalizedName) rows before upgrade. Duplicate keys: ', + LEFT(@DuplicateKeys, 1500) + ), 1; + END + """); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true, + filter: "[TenantId] IS NOT NULL"); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true, + filter: "[TenantId] IS NOT NULL"); + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.Sqlite/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.Sqlite/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs new file mode 100644 index 000000000..e495b51ee --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.Sqlite/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.Designer.cs @@ -0,0 +1,109 @@ +// +using Elsa.Secrets.Persistence.EFCore; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.Sqlite.Migrations.Secrets +{ + [DbContext(typeof(SecretsElsaDbContext))] + [Migration("20260914120000_SecretDefaultTenantUniqueness")] + partial class SecretDefaultTenantUniqueness + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasDefaultSchema("Elsa") + .HasAnnotation("ProductVersion", "10.0.9"); + + modelBuilder.Entity("Elsa.Secrets.Models.Secret", b => + { + b.Property("Id") + .HasColumnType("TEXT"); + + b.Property("CreatedAt") + .IsRequired() + .HasColumnType("TEXT"); + + b.Property("Description") + .HasColumnType("TEXT"); + + b.Property("DisplayName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("TEXT"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("TEXT"); + + b.Property("NormalizedName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("TEXT"); + + b.Property("Scope") + .HasMaxLength(200) + .HasColumnType("TEXT"); + + b.Property("SerializedTags") + .IsRequired() + .HasColumnType("TEXT") + .HasColumnName("Tags"); + + b.Property("SerializedVersions") + .IsRequired() + .HasColumnType("TEXT") + .HasColumnName("Versions"); + + b.Property("Status") + .IsRequired() + .HasMaxLength(32) + .HasColumnType("TEXT"); + + b.Property("StoreName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("TEXT"); + + b.Property("TenantId") + .HasColumnType("TEXT"); + + b.Property("TypeName") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("TEXT"); + + b.Property("UpdatedAt") + .HasColumnType("TEXT"); + + b.HasKey("Id"); + + b.HasIndex("Scope") + .HasDatabaseName("IX_Secret_Scope"); + + b.HasIndex("Status") + .HasDatabaseName("IX_Secret_Status"); + + b.HasIndex("StoreName") + .HasDatabaseName("IX_Secret_StoreName"); + + b.HasIndex("TypeName") + .HasDatabaseName("IX_Secret_TypeName"); + + b.HasIndex("TenantId", "NormalizedName") + .IsUnique() + .HasDatabaseName("IX_Secret_TenantId_NormalizedName"); + + b.ToTable("Secrets", "Elsa"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore.Sqlite/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs b/src/modules/Elsa.Secrets.Persistence.EFCore.Sqlite/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs new file mode 100644 index 000000000..6d349b50c --- /dev/null +++ b/src/modules/Elsa.Secrets.Persistence.EFCore.Sqlite/Migrations/Secrets/20260914120000_SecretDefaultTenantUniqueness.cs @@ -0,0 +1,61 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Elsa.Secrets.Persistence.EFCore.Sqlite.Migrations.Secrets +{ + /// + public partial class SecretDefaultTenantUniqueness : Migration + { + private readonly Elsa.Persistence.EFCore.IElsaDbContextSchema _schema; + + /// + public SecretDefaultTenantUniqueness(Elsa.Persistence.EFCore.IElsaDbContextSchema schema) + { + _schema = schema; + } + + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + // SQLite ignores schemas; the table is created as "Secrets". + const string table = "\"Secrets\""; + + // Default tenant is "" (not null). Stamp leftover nulls so the unique index covers them. + // Leftover duplicates fail at CreateIndex; SQLite cannot abort from a standalone SELECT. + migrationBuilder.Sql($""" + UPDATE {table} + SET "TenantId" = '' + WHERE "TenantId" IS NULL; + """); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets"); + + migrationBuilder.CreateIndex( + name: "IX_Secret_TenantId_NormalizedName", + schema: _schema.Schema, + table: "Secrets", + columns: new[] { "TenantId", "NormalizedName" }, + unique: true); + } + } +} diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore/Repositories/EFCoreSecretRepository.cs b/src/modules/Elsa.Secrets.Persistence.EFCore/Repositories/EFCoreSecretRepository.cs index 2f3a19980..4e5f75693 100644 --- a/src/modules/Elsa.Secrets.Persistence.EFCore/Repositories/EFCoreSecretRepository.cs +++ b/src/modules/Elsa.Secrets.Persistence.EFCore/Repositories/EFCoreSecretRepository.cs @@ -1,3 +1,4 @@ +using Elsa.Common.Multitenancy; using Elsa.Persistence.EFCore; using Elsa.Secrets.Contracts; using Elsa.Secrets.Models; @@ -29,6 +30,7 @@ public class EFCoreSecretRepository(Store store, I public async Task AddAsync(Secret secret, CancellationToken cancellationToken = default) { await using var dbContext = await store.CreateDbContextAsync(cancellationToken); + AssignDefaultTenantId(secret, dbContext); var normalizedName = secretNameValidator.Normalize(secret.Name); if (await ExistsByNormalizedNameAsync(dbContext, normalizedName, cancellationToken)) throw new InvalidOperationException($"A secret named '{secret.Name}' already exists."); @@ -42,6 +44,7 @@ public class EFCoreSecretRepository(Store store, I public async Task TryAddOrReplaceDeletedAsync(Secret secret, CancellationToken cancellationToken = default) { await using var dbContext = await store.CreateDbContextAsync(cancellationToken); + AssignDefaultTenantId(secret, dbContext); var existingSecret = await FindByNameAsync(dbContext, secret.Name, cancellationToken); if (existingSecret == null) @@ -72,6 +75,7 @@ public class EFCoreSecretRepository(Store store, I public async Task SaveAsync(Secret secret, CancellationToken cancellationToken = default) { await using var dbContext = await store.CreateDbContextAsync(cancellationToken); + AssignDefaultTenantId(secret, dbContext); var existingSecret = await FindByNameAsync(dbContext, secret.Name, cancellationToken); if (existingSecret == null) @@ -163,4 +167,19 @@ public class EFCoreSecretRepository(Store store, I { dbContext.Entry(secret).Property(SecretShadowPropertyNames.NormalizedName).CurrentValue = secretNameValidator.Normalize(secret.Name); } + + /// + /// Default-tenant uniqueness uses (""), not null. + /// Named and agnostic ambient tenants are left for ApplyTenantId. + /// + private static void AssignDefaultTenantId(Secret secret, SecretsElsaDbContext dbContext) + { + if (secret.TenantId is not null) + return; + + if (!string.IsNullOrEmpty(dbContext.TenantId) && dbContext.TenantId != Tenant.DefaultTenantId) + return; + + secret.TenantId = Tenant.DefaultTenantId; + } } diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore/SecretConfiguration.cs b/src/modules/Elsa.Secrets.Persistence.EFCore/SecretConfiguration.cs index 9f3de5d39..f340ad9c5 100644 --- a/src/modules/Elsa.Secrets.Persistence.EFCore/SecretConfiguration.cs +++ b/src/modules/Elsa.Secrets.Persistence.EFCore/SecretConfiguration.cs @@ -21,9 +21,11 @@ internal class SecretConfiguration : IEntityTypeConfiguration builder.Property(x => x.StoreName).HasMaxLength(100).IsRequired(); builder.Property(x => x.Scope).HasMaxLength(200); builder.Property(x => x.Status).HasConversion().HasMaxLength(32).IsRequired(); - // Uniqueness is per tenant, matching what User, Role and Application moved to in the same release. - // A global unique index would make the name a shared resource across tenants: the second tenant to - // want a secret called "smtp-password" simply could not create one. + // Uniqueness is per tenant, matching Labels (and User, Role, Application). + // A global unique index would make the name a shared resource across tenants. + // SQL Server still emits a filtered unique index (TenantId IS NOT NULL); leftover + // nulls are stamped to "" first. Oracle stores '' as NULL, so that provider uses + // a function-based unique index on NVL(TenantId, CHR(1)) instead of a filter. builder.HasIndex("TenantId", SecretShadowPropertyNames.NormalizedName).HasDatabaseName($"IX_{nameof(Secret)}_TenantId_{SecretShadowPropertyNames.NormalizedName}").IsUnique(); builder.HasIndex(x => x.TypeName).HasDatabaseName($"IX_{nameof(Secret)}_{nameof(Secret.TypeName)}"); builder.HasIndex(x => x.StoreName).HasDatabaseName($"IX_{nameof(Secret)}_{nameof(Secret.StoreName)}"); diff --git a/test/unit/Elsa.Secrets.UnitTests/EFCoreSecretRepositoryTests.cs b/test/unit/Elsa.Secrets.UnitTests/EFCoreSecretRepositoryTests.cs index ddc54ff25..27b6692dc 100644 --- a/test/unit/Elsa.Secrets.UnitTests/EFCoreSecretRepositoryTests.cs +++ b/test/unit/Elsa.Secrets.UnitTests/EFCoreSecretRepositoryTests.cs @@ -1,3 +1,4 @@ +using Elsa.Common.Multitenancy; using Elsa.Persistence.EFCore; using Elsa.Persistence.EFCore.Extensions; using Elsa.Secrets.Contracts; @@ -66,11 +67,10 @@ public class EFCoreSecretRepositoryTests : IAsyncLifetime } [Fact] - public async Task ExistingSecretsCarryNoTenantUntilOneIsAssigned() + public async Task DefaultTenantWritesUseEmptyTenantId() { - // The upgrade adds the column nullable with no backfill, so rows written before it stay null. That is - // what SetTenantIdFilter's "null counts as the default tenant" clause is for, and it is why the - // migration needs no data step. + // Default-tenant uniqueness uses "" so the composite unique index covers these rows. + // Leftover nulls from SecretTenancy are stamped by SecretDefaultTenantUniqueness. await using var scope = _serviceProvider.CreateAsyncScope(); var repository = scope.ServiceProvider.GetRequiredService(); @@ -78,7 +78,21 @@ public class EFCoreSecretRepositoryTests : IAsyncLifetime var stored = await repository.GetAsync("legacy:secret"); Assert.NotNull(stored); - Assert.Null(stored!.TenantId); + Assert.Equal(string.Empty, stored!.TenantId); + } + + [Fact] + public async Task UniqueIndexRejectsDuplicateDefaultTenantNames() + { + await using var scope = _serviceProvider.CreateAsyncScope(); + var factory = scope.ServiceProvider.GetRequiredService>(); + await using var dbContext = await factory.CreateDbContextAsync(); + + await InsertSecretAsync(dbContext, "first", "smtp:password", Tenant.DefaultTenantId); + await dbContext.SaveChangesAsync(); + + await InsertSecretAsync(dbContext, "second", "smtp:password", Tenant.DefaultTenantId); + await Assert.ThrowsAsync(() => dbContext.SaveChangesAsync()); } [Fact] @@ -160,4 +174,20 @@ public class EFCoreSecretRepositoryTests : IAsyncLifetime Assert.Equal("Replacement password", reloaded.DisplayName); Assert.Equal(SecretStatus.Active, reloaded.Status); } + + private static async Task InsertSecretAsync(SecretsElsaDbContext dbContext, string id, string name, string tenantId) + { + var secret = new Secret + { + Id = id, + Name = name, + DisplayName = name, + TenantId = tenantId + }; + + await dbContext.Secrets.AddAsync(secret); + dbContext.Entry(secret).Property(SecretShadowPropertyNames.NormalizedName).CurrentValue = name; + dbContext.Entry(secret).Property(SecretShadowPropertyNames.SerializedTags).CurrentValue = "[]"; + dbContext.Entry(secret).Property(SecretShadowPropertyNames.SerializedVersions).CurrentValue = "[]"; + } } diff --git a/test/unit/Elsa.Secrets.UnitTests/SecretDefaultTenantUniquenessMigrationTests.cs b/test/unit/Elsa.Secrets.UnitTests/SecretDefaultTenantUniquenessMigrationTests.cs new file mode 100644 index 000000000..2f07188cf --- /dev/null +++ b/test/unit/Elsa.Secrets.UnitTests/SecretDefaultTenantUniquenessMigrationTests.cs @@ -0,0 +1,94 @@ +namespace Elsa.Secrets.UnitTests; + +/// +/// SecretDefaultTenantUniqueness must stamp null TenantId to "" and must not +/// auto-delete duplicate secrets. A preflight lists leftover keys and aborts; +/// CreateIndex then fails loudly if any remain. SQL Server keeps the filtered +/// unique index (TenantId IS NOT NULL) after leftover nulls become "". Oracle +/// stores '' as NULL, so it uses NVL(TenantId, CHR(1)) instead of a filter. +/// +public class SecretDefaultTenantUniquenessMigrationTests +{ + [Theory] + [InlineData("Elsa.Secrets.Persistence.EFCore.Sqlite")] + [InlineData("Elsa.Secrets.Persistence.EFCore.SqlServer")] + [InlineData("Elsa.Secrets.Persistence.EFCore.PostgreSql")] + [InlineData("Elsa.Secrets.Persistence.EFCore.MySql")] + [InlineData("Elsa.Secrets.Persistence.EFCore.Oracle")] + public void SecretDefaultTenantUniqueness_StampsNullTenantIdAndDoesNotDeleteDuplicates(string providerProject) + { + var migration = FindMigration(providerProject); + + Assert.Contains("TenantId", migration, StringComparison.Ordinal); + Assert.Contains("IS NULL", migration, StringComparison.Ordinal); + Assert.DoesNotContain("DELETE FROM", migration, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("DeleteData", migration, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("COALESCE", migration, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void SecretDefaultTenantUniqueness_SqliteFailsLoudlyAtCreateIndex() + { + // SQLite cannot abort from a standalone SELECT, so CreateIndex unique is the fail-loud path. + var migration = FindMigration("Elsa.Secrets.Persistence.EFCore.Sqlite"); + Assert.DoesNotContain("RAISE", migration, StringComparison.Ordinal); + Assert.Contains("CreateIndex", migration, StringComparison.Ordinal); + Assert.Contains("unique: true", migration, StringComparison.Ordinal); + } + + [Theory] + [InlineData("Elsa.Secrets.Persistence.EFCore.SqlServer", "THROW")] + [InlineData("Elsa.Secrets.Persistence.EFCore.PostgreSql", "RAISE EXCEPTION")] + [InlineData("Elsa.Secrets.Persistence.EFCore.MySql", "SIGNAL")] + [InlineData("Elsa.Secrets.Persistence.EFCore.Oracle", "RAISE_APPLICATION_ERROR")] + public void SecretDefaultTenantUniqueness_PreflightsDuplicateKeys(string providerProject, string abortKeyword) + { + var migration = FindMigration(providerProject); + + Assert.Contains(abortKeyword, migration, StringComparison.Ordinal); + Assert.Contains("COUNT(*)", migration, StringComparison.Ordinal); + Assert.Contains("Duplicate keys", migration, StringComparison.OrdinalIgnoreCase); + Assert.Contains("HAVING COUNT(*) > 1", migration, StringComparison.Ordinal); + } + + [Fact] + public void SecretDefaultTenantUniqueness_SqlServerKeepsFilteredUniqueIndex() + { + var migration = FindMigration("Elsa.Secrets.Persistence.EFCore.SqlServer"); + Assert.Contains("[TenantId] IS NOT NULL", migration, StringComparison.Ordinal); + } + + [Fact] + public void SecretDefaultTenantUniqueness_OracleUsesNvlBecauseEmptyStringIsNull() + { + var migration = FindMigration("Elsa.Secrets.Persistence.EFCore.Oracle"); + Assert.Contains("NVL(\"TenantId\", CHR(1))", migration, StringComparison.Ordinal); + Assert.Contains("CREATE UNIQUE INDEX", migration, StringComparison.Ordinal); + } + + private static string FindMigration(string providerProject) + { + if (Path.IsPathRooted(providerProject)) + throw new ArgumentException("Provider project must be a relative path.", nameof(providerProject)); + + var repoRoot = FindRepoRoot(); + var secretsDir = Path.Combine(repoRoot, "src", "modules", providerProject, "Migrations", "Secrets"); + var path = Directory.GetFiles(secretsDir, "*SecretDefaultTenantUniqueness.cs") + .Single(file => !file.EndsWith(".Designer.cs", StringComparison.Ordinal)); + return File.ReadAllText(path); + } + + private static string FindRepoRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory is not null) + { + if (File.Exists(Path.Join(directory.FullName, "Elsa.sln"))) + return directory.FullName; + + directory = directory.Parent; + } + + throw new InvalidOperationException("Could not find Elsa.sln from the test output directory."); + } +} diff --git a/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryConformanceTests.cs b/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryConformanceTests.cs new file mode 100644 index 000000000..ffe9df6de --- /dev/null +++ b/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryConformanceTests.cs @@ -0,0 +1,155 @@ +using Elsa.Common.Multitenancy; +using Elsa.Secrets.Models; + +namespace Elsa.Secrets.UnitTests; + +/// +/// Shared File / InMemory / EF Core store-contract assertions for secret repositories. +/// Covers per-tenant name uniqueness and ambient-tenant isolation on the existing +/// surface. +/// +public abstract class SecretRepositoryConformanceTests +{ + protected abstract Task CreateScenarioAsync(); + + [Fact] + public async Task TwoTenantsCanOwnTheSameSecretName() + { + await using var scenario = await CreateScenarioAsync(); + + using (scenario.UseTenant("tenant-a")) + { + var secretA = CreateSecret("SMTP:PASSWORD", "Tenant A"); + await scenario.Repository.AddAsync(secretA); + + Assert.Equal("tenant-a", secretA.TenantId); + await Assert.ThrowsAsync(() => + scenario.Repository.AddAsync(CreateSecret("smtp:password", "Duplicate A"))); + } + + using (scenario.UseTenant("tenant-b")) + { + var secretB = CreateSecret("smtp:password", "Tenant B"); + await scenario.Repository.AddAsync(secretB); + + Assert.Equal("tenant-b", secretB.TenantId); + Assert.Equal("Tenant B", (await scenario.Repository.GetAsync("SMTP:PASSWORD"))!.DisplayName); + } + + using (scenario.UseTenant("tenant-a")) + Assert.Equal("Tenant A", (await scenario.Repository.GetAsync("smtp:password"))!.DisplayName); + } + + [Fact] + public async Task GetDoesNotCrossReadAnotherTenant() + { + await using var scenario = await CreateScenarioAsync(); + + using (scenario.UseTenant("tenant-a")) + await scenario.Repository.AddAsync(CreateSecret("smtp:password", "Tenant A")); + + using (scenario.UseTenant("tenant-b")) + { + Assert.Null(await scenario.Repository.GetAsync("smtp:password")); + await scenario.Repository.AddAsync(CreateSecret("smtp:password", "Tenant B")); + } + + using (scenario.UseTenant("tenant-a")) + { + var loaded = await scenario.Repository.GetAsync("SMTP:PASSWORD"); + Assert.NotNull(loaded); + Assert.Equal("Tenant A", loaded.DisplayName); + Assert.Equal("tenant-a", loaded.TenantId); + } + } + + [Fact] + public async Task ListIsTenantScoped() + { + await using var scenario = await CreateScenarioAsync(); + + using (scenario.UseTenant("tenant-a")) + { + await scenario.Repository.AddAsync(CreateSecret("smtp:password", "Tenant A")); + await scenario.Repository.AddAsync(CreateSecret("api:key", "Key A")); + } + + using (scenario.UseTenant("tenant-b")) + { + await scenario.Repository.AddAsync(CreateSecret("smtp:password", "Tenant B")); + var listedB = await scenario.Repository.ListAsync(); + Assert.Equal("Tenant B", Assert.Single(listedB).DisplayName); + } + + using (scenario.UseTenant("tenant-a")) + { + var listedA = (await scenario.Repository.ListAsync()).OrderBy(x => x.Name).ToList(); + Assert.Equal(["api:key", "smtp:password"], listedA.Select(x => x.Name).ToList()); + Assert.All(listedA, secret => Assert.Equal("tenant-a", secret.TenantId)); + } + } + + [Fact] + public async Task DeletedReplaceStaysPerTenant() + { + await using var scenario = await CreateScenarioAsync(); + + using (scenario.UseTenant("tenant-a")) + { + await scenario.Repository.SaveAsync(CreateSecret("smtp:password", "Deleted A", SecretStatus.Deleted)); + Assert.True(await scenario.Repository.TryAddOrReplaceDeletedAsync(CreateSecret("SMTP:PASSWORD", "Replacement A"))); + Assert.Equal("Replacement A", (await scenario.Repository.GetAsync("smtp:password"))!.DisplayName); + } + + using (scenario.UseTenant("tenant-b")) + { + Assert.True(await scenario.Repository.TryAddOrReplaceDeletedAsync(CreateSecret("smtp:password", "Tenant B"))); + Assert.Equal("Tenant B", (await scenario.Repository.GetAsync("smtp:password"))!.DisplayName); + } + + using (scenario.UseTenant("tenant-a")) + Assert.Equal("Replacement A", (await scenario.Repository.GetAsync("smtp:password"))!.DisplayName); + } + + [Fact] + public async Task DefaultTenantRejectsDuplicateNames() + { + await using var scenario = await CreateScenarioAsync(); + + using (scenario.UseTenant(Tenant.DefaultTenantId)) + { + var secret = CreateSecret("smtp:password", "Default"); + await scenario.Repository.AddAsync(secret); + Assert.Equal(Tenant.DefaultTenantId, secret.TenantId); + + await Assert.ThrowsAsync(() => + scenario.Repository.AddAsync(CreateSecret("SMTP:PASSWORD", "Duplicate"))); + } + } + + private static Secret CreateSecret(string name, string displayName, SecretStatus status = SecretStatus.Active) => + new() + { + Name = name, + DisplayName = displayName, + Status = status + }; +} + +public sealed class InMemorySecretRepositoryConformanceTests : SecretRepositoryConformanceTests +{ + protected override Task CreateScenarioAsync() => + SecretRepositoryScenario.CreateInMemoryAsync(); +} + +public sealed class FileSecretRepositoryConformanceTests : SecretRepositoryConformanceTests +{ + protected override Task CreateScenarioAsync() => + SecretRepositoryScenario.CreateFileAsync(); +} + +public sealed class SqliteSecretRepositoryConformanceTests : SecretRepositoryConformanceTests +{ + protected override Task CreateScenarioAsync() => + SecretRepositoryScenario.CreateSqliteAsync(); +} diff --git a/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryScenario.cs b/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryScenario.cs new file mode 100644 index 000000000..cf60b38ec --- /dev/null +++ b/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryScenario.cs @@ -0,0 +1,134 @@ +using Elsa.Common.Multitenancy; +using Elsa.Persistence.EFCore; +using Elsa.Persistence.EFCore.EntityHandlers; +using Elsa.Persistence.EFCore.Extensions; +using Elsa.Secrets.Contracts; +using Elsa.Secrets.Models; +using Elsa.Secrets.Options; +using Elsa.Secrets.Persistence.EFCore; +using Elsa.Secrets.Persistence.EFCore.Repositories; +using Elsa.Secrets.Persistence.EFCore.Sqlite.Extensions; +using Elsa.Secrets.Repositories; +using Elsa.Secrets.Services; +using Elsa.Tenants.Options; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Secrets.UnitTests; + +/// +/// One File, InMemory, or EF/SQLite plus the ambient tenant it reads. +/// +public sealed class SecretRepositoryScenario( + ITenantAccessor tenantAccessor, + ISecretRepository repository, + Func disposeAsync) : IAsyncDisposable +{ + public ITenantAccessor TenantAccessor { get; } = tenantAccessor; + public ISecretRepository Repository { get; } = repository; + + public IDisposable UseTenant(string tenantId) => + TenantAccessor.PushContext(tenantId == Tenant.DefaultTenantId + ? Tenant.Default + : new Tenant + { + Id = tenantId, + Name = tenantId + }); + + public ValueTask DisposeAsync() => disposeAsync(); + + public static Task CreateInMemoryAsync() + { + var tenantAccessor = new DefaultTenantAccessor(); + return Task.FromResult(new SecretRepositoryScenario( + tenantAccessor, + new InMemorySecretRepository(tenantAccessor), + () => ValueTask.CompletedTask)); + } + + public static Task CreateFileAsync() + { + var path = Path.Join(Path.GetTempPath(), $"elsa-secrets-conformance-{Guid.NewGuid():N}.json"); + var tenantAccessor = new DefaultTenantAccessor(); + var options = Microsoft.Extensions.Options.Options.Create(new SecretsOptions + { + RepositoryFilePath = path + }); + + return Task.FromResult(new SecretRepositoryScenario( + tenantAccessor, + new FileSecretRepository(options, tenantAccessor: tenantAccessor), + () => + { + if (File.Exists(path)) + File.Delete(path); + return ValueTask.CompletedTask; + })); + } + + public static async Task CreateSqliteAsync() + { + var databasePath = Path.Join(Path.GetTempPath(), $"elsa-secrets-conformance-{Guid.NewGuid():N}.db"); + var tenantAccessor = new DefaultTenantAccessor(); + ServiceProvider? services = null; + IServiceScope? scope = null; + + try + { + var migrationsAssembly = typeof(SqliteSecretsPersistenceFeatureExtensions).Assembly; + services = new ServiceCollection() + .AddSingleton(tenantAccessor) + .Configure(options => options.IsEnabled = true) + .AddScoped() + .AddScoped() + .AddSqliteEntityModelCreatingHandlers() + .AddDbContextFactory((_, builder) => + builder.UseElsaSqlite(migrationsAssembly, $"Data Source={databasePath}") + .ReplaceService()) + .Decorate, TenantAwareDbContextFactory>() + .AddSingleton() + .AddScoped>() + .AddScoped() + .BuildServiceProvider(); + + await using (var dbContext = await services.GetRequiredService>().CreateDbContextAsync()) + await dbContext.Database.MigrateAsync(); + + scope = services.CreateScope(); + return new( + tenantAccessor, + scope.ServiceProvider.GetRequiredService(), + async () => + { + scope.Dispose(); + await services.DisposeAsync(); + SqliteConnection.ClearAllPools(); + if (File.Exists(databasePath)) + File.Delete(databasePath); + }); + } + catch + { + scope?.Dispose(); + if (services is not null) + await services.DisposeAsync(); + SqliteConnection.ClearAllPools(); + if (File.Exists(databasePath)) + File.Delete(databasePath); + throw; + } + } +} + +/// +/// Keeps the tenant-aware Secrets model out of the default EF cache shared with +/// EFCoreSecretRepositoryTests, which builds the same context type without a query filter. +/// +file sealed class TenantAwareSecretModelCacheKeyFactory : IModelCacheKeyFactory +{ + public object Create(DbContext context, bool designTime) => + (context.GetType(), designTime, "secrets-tenant-aware"); +}