docs(secrets): document null-tenant index gaps and the MySql TFM pin (#7998)
* docs(secrets): document null-tenant index gaps and the MySql TFM pin The per-tenant unique indexes only backstop rows whose TenantId is non-null, so single-tenant deployments and pre-upgrade rows fall back to the pre-save existence checks for name uniqueness. Recorded in secrets-tenancy.md and the authorization-model guide, with a comment at the EFCore secret repository's write path. Also documents why Elsa.Secrets.Persistence.EFCore.MySql stays pinned to net8.0/net9.0: Pomelo.EntityFrameworkCore.MySql tops out at EF Core 9, and the project references Elsa.Persistence.EFCore.MySql which carries the same pin. Comment-only csproj change; no behavior changes anywhere in this commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(secrets): say plainly that the null-tenant backfill is not a fix The guide implied backfilling TenantId to "" restored the uniqueness guarantee in single-tenant mode. It does not: disabled-mode writes keep persisting null, so new rows still land outside the index and two concurrent creates can still commit the same name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
80a5892ecd
commit
7e8e7aa012
|
|
@ -130,6 +130,8 @@ Included in the same release: `User.Name`, `Role.Name`, `Application.Name` and `
|
|||
|
||||
If you have duplicate names across tenants today, they were impossible to create, so no data conflict can arise. Going the other way — downgrading — will fail if duplicates exist by then.
|
||||
|
||||
One caveat: the composite indexes only cover rows whose `TenantId` is non-null (SQL Server filters null rows out of the index; SQLite, PostgreSQL and MySQL treat nulls as distinct — Oracle alone still rejects null-tenant duplicates). `TenantId` is only assigned when multitenancy is enabled, so in a single-tenant deployment every row keeps null and user, role and application name uniqueness becomes application-enforced rather than schema-enforced: the pre-save existence checks block sequential duplicates, but the database no longer backstops concurrent ones. Likewise, rows written before the upgrade keep a null `TenantId`, and in a multi-tenant deployment's default tenant those legacy rows and new `""`-tenant rows are distinct index keys, so the index cannot catch a name collision between them. See the same caveat, with the reasoning, in [secrets-tenancy.md](secrets-tenancy.md).
|
||||
|
||||
## Full mapping
|
||||
|
||||
| Legacy permission | Replacement |
|
||||
|
|
|
|||
|
|
@ -46,6 +46,45 @@ 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
|
||||
|
||||
"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.
|
||||
|
||||
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.
|
||||
|
||||
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.)
|
||||
|
||||
## The MySQL provider ships for net8.0 and net9.0 only
|
||||
|
||||
`Elsa.Secrets.Persistence.EFCore.MySql` targets `net8.0;net9.0`, while the Sqlite, SQL Server, PostgreSQL and
|
||||
Oracle secrets providers also target `net10.0`. That is a dependency constraint, not an oversight:
|
||||
`Pomelo.EntityFrameworkCore.MySql` tops out at 9.0.0, built for EF Core 9, so there is no net10.0 provider to
|
||||
build against. Every MySQL project in the repository carries the same pin, and the secrets one additionally
|
||||
inherits it by referencing `Elsa.Persistence.EFCore.MySql`.
|
||||
|
||||
A net10.0 host referencing the MySQL secrets provider resolves the net9.0 asset and runs normally, including
|
||||
the `SecretTenancy` migration — migrations are ordinary C# and do not depend on the host framework. The pins
|
||||
come out together once Pomelo ships for EF Core 10.
|
||||
|
||||
## The VNext persistence provider does not support this
|
||||
|
||||
`Elsa.Secrets.Persistence.VNext` stores documents keyed by secret name alone, and `Elsa.Persistence.VNext` has
|
||||
|
|
|
|||
|
|
@ -1,7 +1,14 @@
|
|||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
|
||||
<PropertyGroup>
|
||||
<!-- Pomelo targets EF Core up to 9.x, so exclude MySQL provider for net10.0. -->
|
||||
<!--
|
||||
Pomelo targets EF Core up to 9.x, so there is no net10.0 MySQL provider to build against.
|
||||
Elsa.Persistence.EFCore.MySql is pinned for that reason and this project references it, so it
|
||||
could not target net10.0 even without this line. The sibling secrets providers (Sqlite, SqlServer,
|
||||
PostgreSql, Oracle) inherit the repo default and do include net10.0; the divergence is the
|
||||
dependency, not an oversight. A net10.0 host binds the net9.0 asset. Remove both pins together
|
||||
once Pomelo ships for EF Core 10.
|
||||
-->
|
||||
<TargetFrameworks>net8.0;net9.0</TargetFrameworks>
|
||||
<Description>
|
||||
Provides MySQL EF Core migrations for Elsa secrets.
|
||||
|
|
|
|||
|
|
@ -116,6 +116,12 @@ public class EFCoreSecretRepository(Store<SecretsElsaDbContext, Secret> store, I
|
|||
return dbContext.Secrets.AnyAsync(x => EF.Property<string>(x, SecretShadowPropertyNames.NormalizedName) == normalizedName, cancellationToken);
|
||||
}
|
||||
|
||||
// The DbUpdateException-to-name-conflict translation below relies on the (TenantId, NormalizedName)
|
||||
// unique index, which only covers rows with a non-null TenantId (SQL Server filters null rows out of the
|
||||
// index; SQLite/PostgreSQL/MySQL treat nulls as distinct — Oracle alone rejects null-tenant duplicates).
|
||||
// With multitenancy disabled nothing assigns a TenantId, so this backstop never fires there and
|
||||
// uniqueness rests solely on the FindByNameAsync/ExistsByNormalizedNameAsync pre-checks — two concurrent
|
||||
// creates racing past the pre-check both commit. See doc/migrations/secrets-tenancy.md.
|
||||
private async Task SaveChangesAsync(SecretsElsaDbContext dbContext, string name, CancellationToken cancellationToken)
|
||||
{
|
||||
try
|
||||
|
|
|
|||
Loading…
Reference in a new issue