From 0be3d5fb993df681b7f53b07e93fc37f096a4627 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 14 Sep 2026 08:09:21 +0200 Subject: [PATCH] fix(secrets): close post-merge tenancy gaps (#8140) Closes #8103. --- .../Repositories/EFCoreSecretRepository.cs | 54 ++- src/modules/Elsa.Secrets/Elsa.Secrets.csproj | 1 + .../Extensions/ServiceCollectionExtensions.cs | 2 + .../Repositories/FileSecretRepository.cs | 97 ++++-- .../Repositories/InMemorySecretRepository.cs | 63 +++- .../Repositories/SecretRepositoryTenant.cs | 25 +- .../EFCoreSecretRepositoryTests.cs | 328 ++++++++++++++++++ .../SecretRepositoryTenantIsolationTests.cs | 195 ++++++++++- .../SecretStoreTests.cs | 29 ++ 9 files changed, 726 insertions(+), 68 deletions(-) diff --git a/src/modules/Elsa.Secrets.Persistence.EFCore/Repositories/EFCoreSecretRepository.cs b/src/modules/Elsa.Secrets.Persistence.EFCore/Repositories/EFCoreSecretRepository.cs index 4e5f75693..7a7daf257 100644 --- a/src/modules/Elsa.Secrets.Persistence.EFCore/Repositories/EFCoreSecretRepository.cs +++ b/src/modules/Elsa.Secrets.Persistence.EFCore/Repositories/EFCoreSecretRepository.cs @@ -2,12 +2,25 @@ using Elsa.Common.Multitenancy; using Elsa.Persistence.EFCore; using Elsa.Secrets.Contracts; using Elsa.Secrets.Models; +using Elsa.Tenants.Options; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Options; namespace Elsa.Secrets.Persistence.EFCore.Repositories; -public class EFCoreSecretRepository(Store store, ISecretNameValidator secretNameValidator) : ISecretRepository +public class EFCoreSecretRepository( + Store store, + ISecretNameValidator secretNameValidator, + IOptions? tenantsOptions = null) : ISecretRepository { + private readonly bool? _tenancyEnabled = tenantsOptions?.Value.IsEnabled; + + // Keep the pre-tenancy constructor in the public binary surface. + public EFCoreSecretRepository(Store store, ISecretNameValidator secretNameValidator) + : this(store, secretNameValidator, null) + { + } + public async Task GetAsync(string normalizedName, CancellationToken cancellationToken = default) { await using var dbContext = await store.CreateDbContextAsync(cancellationToken); @@ -45,6 +58,7 @@ public class EFCoreSecretRepository(Store store, I { await using var dbContext = await store.CreateDbContextAsync(cancellationToken); AssignDefaultTenantId(secret, dbContext); + var tenancyEnabled = IsTenancyEnabled(dbContext); var existingSecret = await FindByNameAsync(dbContext, secret.Name, cancellationToken); if (existingSecret == null) @@ -58,6 +72,12 @@ public class EFCoreSecretRepository(Store store, I if (existingSecret.Status != SecretStatus.Deleted) return false; + var incomingTenantId = secret.TenantId ?? dbContext.TenantId; + if (tenancyEnabled && !TenantVisibility.CanReplaceOwnedRow(existingSecret.TenantId, incomingTenantId, dbContext.TenantId ?? Tenant.DefaultTenantId)) + return false; + + if (tenancyEnabled) + secret.TenantId = existingSecret.TenantId; await using var transaction = await dbContext.Database.BeginTransactionAsync(cancellationToken); dbContext.Secrets.Remove(existingSecret); await dbContext.SaveChangesAsync(cancellationToken); @@ -76,6 +96,7 @@ public class EFCoreSecretRepository(Store store, I { await using var dbContext = await store.CreateDbContextAsync(cancellationToken); AssignDefaultTenantId(secret, dbContext); + var tenancyEnabled = IsTenancyEnabled(dbContext); var existingSecret = await FindByNameAsync(dbContext, secret.Name, cancellationToken); if (existingSecret == null) @@ -86,7 +107,11 @@ public class EFCoreSecretRepository(Store store, I } else { - Copy(secret, existingSecret); + var incomingTenantId = secret.TenantId ?? dbContext.TenantId; + if (tenancyEnabled && !TenantVisibility.CanReplaceOwnedRow(existingSecret.TenantId, incomingTenantId, dbContext.TenantId ?? Tenant.DefaultTenantId)) + throw new InvalidOperationException($"A secret named '{secret.Name}' belongs to another tenant."); + + Copy(secret, existingSecret, tenancyEnabled); SetNormalizedName(dbContext, existingSecret); SecretSerialization.StoreSerializedProperties(dbContext, existingSecret); } @@ -94,8 +119,11 @@ public class EFCoreSecretRepository(Store store, I await SaveChangesAsync(dbContext, secret.Name, cancellationToken); } - private static void Copy(Secret source, Secret target) + private static void Copy(Secret source, Secret target, bool tenancyEnabled) { + if (!tenancyEnabled) + target.TenantId = source.TenantId; + target.Name = source.Name; target.DisplayName = source.DisplayName; target.Description = source.Description; @@ -115,17 +143,27 @@ public class EFCoreSecretRepository(Store store, I return dbContext.Secrets.FirstOrDefaultAsync(x => EF.Property(x, SecretShadowPropertyNames.NormalizedName) == normalizedName, cancellationToken); } + private bool IsTenancyEnabled(SecretsElsaDbContext dbContext) + { + if (_tenancyEnabled.HasValue) + return _tenancyEnabled.Value; + + var entityType = dbContext.Model.FindEntityType(typeof(Secret)); +#if NET10_0_OR_GREATER + return entityType?.GetDeclaredQueryFilters().Any() == true; +#else + return entityType?.FindAnnotation("QueryFilter")?.Value is not null; +#endif + } + private static Task ExistsByNormalizedNameAsync(SecretsElsaDbContext dbContext, string normalizedName, CancellationToken cancellationToken) { return dbContext.Secrets.AnyAsync(x => EF.Property(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. + // unique index. Default-tenant writes are stamped with an empty TenantId before saving so the index + // provides the same concurrency backstop when multitenancy is disabled. See doc/migrations/secrets-tenancy.md. private async Task SaveChangesAsync(SecretsElsaDbContext dbContext, string name, CancellationToken cancellationToken) { try diff --git a/src/modules/Elsa.Secrets/Elsa.Secrets.csproj b/src/modules/Elsa.Secrets/Elsa.Secrets.csproj index ee8a03d51..f13fb653c 100644 --- a/src/modules/Elsa.Secrets/Elsa.Secrets.csproj +++ b/src/modules/Elsa.Secrets/Elsa.Secrets.csproj @@ -15,6 +15,7 @@ + diff --git a/src/modules/Elsa.Secrets/Extensions/ServiceCollectionExtensions.cs b/src/modules/Elsa.Secrets/Extensions/ServiceCollectionExtensions.cs index 396a263ed..70e7faf50 100644 --- a/src/modules/Elsa.Secrets/Extensions/ServiceCollectionExtensions.cs +++ b/src/modules/Elsa.Secrets/Extensions/ServiceCollectionExtensions.cs @@ -4,6 +4,7 @@ using Elsa.Secrets.Repositories; using Elsa.Secrets.Services; using Elsa.Secrets.Stores; using Elsa.Secrets.Types; +using Elsa.Tenants.Options; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -17,6 +18,7 @@ public static class ServiceCollectionExtensions services.Configure(configureOptions); services.AddOptions(); + services.AddOptions(); services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); diff --git a/src/modules/Elsa.Secrets/Repositories/FileSecretRepository.cs b/src/modules/Elsa.Secrets/Repositories/FileSecretRepository.cs index e31b4caf8..fb681317f 100644 --- a/src/modules/Elsa.Secrets/Repositories/FileSecretRepository.cs +++ b/src/modules/Elsa.Secrets/Repositories/FileSecretRepository.cs @@ -1,23 +1,61 @@ using System.Text.Json; using System.Text.Json.Serialization; using Elsa.Common.Multitenancy; +using Elsa.Secrets.Contracts; +using Elsa.Secrets.Services; +using Elsa.Tenants.Options; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Elsa.Secrets.Repositories; -public class FileSecretRepository( - IOptions options, - ILogger? logger = null, - ITenantAccessor? tenantAccessor = null) : ISecretRepository +public class FileSecretRepository : ISecretRepository { - // Keep the pre-tenancy constructor in the public binary surface. Optional parameters only preserve - // source compatibility; existing binaries still look for this exact two-argument constructor. - public FileSecretRepository(IOptions options, ILogger? logger) + private readonly IOptions options; + private readonly ILogger? logger; + private readonly ITenantAccessor? tenantAccessor; + private readonly bool tenancyEnabled; + private readonly ISecretNameValidator nameValidator; + + public FileSecretRepository( + IOptions options, + ILogger? logger) : this(options, logger, null) { } + public FileSecretRepository( + IOptions options, + ILogger? logger = null, + ITenantAccessor? tenantAccessor = null) + : this(options, logger, tenantAccessor, tenantAccessor is not null, new DefaultSecretNameValidator()) + { + } + + public FileSecretRepository( + IOptions options, + IOptions tenantsOptions, + ISecretNameValidator nameValidator, + ILogger? logger = null, + ITenantAccessor? tenantAccessor = null) + : this(options, logger, tenantAccessor, tenantsOptions.Value.IsEnabled, nameValidator) + { + } + + private FileSecretRepository( + IOptions options, + ILogger? logger, + ITenantAccessor? tenantAccessor, + bool tenancyEnabled, + ISecretNameValidator nameValidator) + { + this.options = options; + this.logger = logger; + this.tenantAccessor = tenantAccessor; + this.tenancyEnabled = tenancyEnabled; + this.nameValidator = nameValidator; + } + private readonly SemaphoreSlim _lock = new(1, 1); private readonly JsonSerializerOptions _jsonOptions = new(JsonSerializerDefaults.Web) { @@ -28,26 +66,26 @@ public class FileSecretRepository( public async Task GetAsync(string normalizedName, CancellationToken cancellationToken = default) { var secrets = await ReadAllAsync(cancellationToken); - return secrets.FirstOrDefault(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor) && SecretRepositoryTenant.HasName(x, normalizedName)); + return secrets.FirstOrDefault(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor, tenancyEnabled) && SecretRepositoryTenant.HasName(x, normalizedName, nameValidator)); } public async Task> ListAsync(CancellationToken cancellationToken = default) { - return (await ReadAllAsync(cancellationToken)).Where(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor)).ToList(); + return (await ReadAllAsync(cancellationToken)).Where(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor, tenancyEnabled)).ToList(); } public async Task AddAsync(Secret secret, CancellationToken cancellationToken = default) { - SecretRepositoryTenant.Stamp(secret, tenantAccessor); + SecretRepositoryTenant.Stamp(secret, tenantAccessor, tenancyEnabled); await _lock.WaitAsync(cancellationToken); try { var secrets = await ReadAllUnsafeAsync(cancellationToken); - if (secrets.Any(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor) && SecretRepositoryTenant.HasName(x, secret.Name))) + if (secrets.Any(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor, tenancyEnabled) && SecretRepositoryTenant.HasName(x, secret.Name, nameValidator))) throw new InvalidOperationException($"A secret named '{secret.Name}' already exists."); - if (secrets.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret))) + if (secrets.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret, nameValidator, tenancyEnabled))) throw new InvalidOperationException($"A secret named '{secret.Name}' already exists."); if (secrets.Any(x => x.Id == secret.Id)) @@ -64,29 +102,29 @@ public class FileSecretRepository( public async Task TryAddOrReplaceDeletedAsync(Secret secret, CancellationToken cancellationToken = default) { - SecretRepositoryTenant.Stamp(secret, tenantAccessor); + SecretRepositoryTenant.Stamp(secret, tenantAccessor, tenancyEnabled); await _lock.WaitAsync(cancellationToken); try { var secrets = await ReadAllUnsafeAsync(cancellationToken); - var index = secrets.FindIndex(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor) && SecretRepositoryTenant.HasName(x, secret.Name)); + var index = secrets.FindIndex(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor, tenancyEnabled) && SecretRepositoryTenant.HasName(x, secret.Name, nameValidator)); if (index >= 0) { if (secrets[index].Status != SecretStatus.Deleted) return false; - if (!SecretRepositoryTenant.CanReplace(secrets[index], secret, tenantAccessor)) + if (!SecretRepositoryTenant.CanReplace(secrets[index], secret, tenantAccessor, tenancyEnabled)) return false; if (secrets.Where((_, i) => i != index).Any(x => x.Id == secret.Id)) return false; - secrets[index] = ReplaceTenantOwnedSecret(secrets[index], secret); + secrets[index] = ReplaceTenantOwnedSecret(secrets[index], secret, tenancyEnabled); } else { - if (secrets.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret))) + if (secrets.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret, nameValidator, tenancyEnabled))) return false; if (secrets.Any(x => x.Id == secret.Id)) @@ -106,16 +144,16 @@ public class FileSecretRepository( public async Task SaveAsync(Secret secret, CancellationToken cancellationToken = default) { - SecretRepositoryTenant.Stamp(secret, tenantAccessor); + SecretRepositoryTenant.Stamp(secret, tenantAccessor, tenancyEnabled); await _lock.WaitAsync(cancellationToken); try { var secrets = await ReadAllUnsafeAsync(cancellationToken); - var index = secrets.FindIndex(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor) && SecretRepositoryTenant.HasName(x, secret.Name)); + var index = secrets.FindIndex(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor, tenancyEnabled) && SecretRepositoryTenant.HasName(x, secret.Name, nameValidator)); if (index < 0) { - if (secrets.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret))) + if (secrets.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret, nameValidator, tenancyEnabled))) throw new InvalidOperationException($"A secret named '{secret.Name}' already exists."); if (secrets.Any(x => x.Id == secret.Id)) @@ -125,10 +163,10 @@ public class FileSecretRepository( } else { - if (!SecretRepositoryTenant.CanReplace(secrets[index], secret, tenantAccessor)) + if (!SecretRepositoryTenant.CanReplace(secrets[index], secret, tenantAccessor, tenancyEnabled)) throw new InvalidOperationException($"A secret named '{secret.Name}' belongs to another tenant."); - secrets[index] = ReplaceIdentityAndTenant(secrets[index], secret); + secrets[index] = ReplaceIdentityAndTenant(secrets[index], secret, tenancyEnabled); } await WriteAllUnsafeAsync(secrets, cancellationToken); @@ -191,18 +229,23 @@ public class FileSecretRepository( } /// - /// Updates the aggregate payload while retaining the row identity and tenant ownership. + /// Updates the aggregate payload while retaining the row identity and, when enabled, tenant ownership. /// - private static Secret ReplaceTenantOwnedSecret(Secret existing, Secret incoming) + private static Secret ReplaceTenantOwnedSecret(Secret existing, Secret incoming, bool tenancyEnabled) { - incoming.TenantId = existing.TenantId; + if (tenancyEnabled) + incoming.TenantId = existing.TenantId; + return incoming; } - private static Secret ReplaceIdentityAndTenant(Secret existing, Secret incoming) + private static Secret ReplaceIdentityAndTenant(Secret existing, Secret incoming, bool tenancyEnabled) { incoming.Id = existing.Id; - incoming.TenantId = existing.TenantId; + + if (tenancyEnabled) + incoming.TenantId = existing.TenantId; + return incoming; } diff --git a/src/modules/Elsa.Secrets/Repositories/InMemorySecretRepository.cs b/src/modules/Elsa.Secrets/Repositories/InMemorySecretRepository.cs index 2cbea4166..074705bf3 100644 --- a/src/modules/Elsa.Secrets/Repositories/InMemorySecretRepository.cs +++ b/src/modules/Elsa.Secrets/Repositories/InMemorySecretRepository.cs @@ -1,4 +1,8 @@ using Elsa.Common.Multitenancy; +using Elsa.Secrets.Contracts; +using Elsa.Secrets.Services; +using Elsa.Tenants.Options; +using Microsoft.Extensions.Options; namespace Elsa.Secrets.Repositories; @@ -11,14 +15,38 @@ namespace Elsa.Secrets.Repositories; /// . This is deliberately the same shape as the persisted contract rather /// than a test-only global name dictionary. /// -public class InMemorySecretRepository(ITenantAccessor? tenantAccessor = null) : ISecretRepository +public class InMemorySecretRepository : ISecretRepository { + private readonly ITenantAccessor? tenantAccessor; + private readonly bool tenancyEnabled; + private readonly ISecretNameValidator nameValidator; + + public InMemorySecretRepository(ITenantAccessor? tenantAccessor = null) + : this(tenantAccessor, tenantAccessor is not null, new DefaultSecretNameValidator()) + { + } + // Keep the pre-tenancy parameterless constructor in the public binary surface. Optional parameters do // not emit a zero-argument constructor for existing binaries to bind to. public InMemorySecretRepository() : this(null) { } + public InMemorySecretRepository( + IOptions tenantsOptions, + ISecretNameValidator nameValidator, + ITenantAccessor? tenantAccessor = null) + : this(tenantAccessor, tenantsOptions.Value.IsEnabled, nameValidator) + { + } + + private InMemorySecretRepository(ITenantAccessor? tenantAccessor, bool tenancyEnabled, ISecretNameValidator nameValidator) + { + this.tenantAccessor = tenantAccessor; + this.tenancyEnabled = tenancyEnabled; + this.nameValidator = nameValidator; + } + private readonly Dictionary _secrets = new(StringComparer.Ordinal); private readonly object _sync = new(); @@ -36,7 +64,7 @@ public class InMemorySecretRepository(ITenantAccessor? tenantAccessor = null) : lock (_sync) { var secrets = _secrets.Values - .Where(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor)) + .Where(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor, tenancyEnabled)) .Select(Clone) .ToList(); return Task.FromResult>(secrets); @@ -45,14 +73,14 @@ public class InMemorySecretRepository(ITenantAccessor? tenantAccessor = null) : public Task AddAsync(Secret secret, CancellationToken cancellationToken = default) { - SecretRepositoryTenant.Stamp(secret, tenantAccessor); + SecretRepositoryTenant.Stamp(secret, tenantAccessor, tenancyEnabled); lock (_sync) { if (FindVisible(secret.Name) is not null) throw new InvalidOperationException($"A secret named '{secret.Name}' already exists."); - if (_secrets.Values.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret))) + if (_secrets.Values.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret, nameValidator, tenancyEnabled))) throw new InvalidOperationException($"A secret named '{secret.Name}' already exists."); EnsureIdAvailable(secret); @@ -64,18 +92,19 @@ public class InMemorySecretRepository(ITenantAccessor? tenantAccessor = null) : public Task TryAddOrReplaceDeletedAsync(Secret secret, CancellationToken cancellationToken = default) { - SecretRepositoryTenant.Stamp(secret, tenantAccessor); + SecretRepositoryTenant.Stamp(secret, tenantAccessor, tenancyEnabled); lock (_sync) { var existing = FindVisible(secret.Name); if (existing is not null) { - if (existing.Status != SecretStatus.Deleted || !SecretRepositoryTenant.CanReplace(existing, secret, tenantAccessor)) + if (existing.Status != SecretStatus.Deleted || !SecretRepositoryTenant.CanReplace(existing, secret, tenantAccessor, tenancyEnabled)) return Task.FromResult(false); var replacement = Clone(secret); - replacement.TenantId = existing.TenantId; + if (tenancyEnabled) + replacement.TenantId = existing.TenantId; // Validate before removing the deleted row. Reusing its own ID is valid; another row's ID // is a collision and must leave the deleted row untouched when validation fails. @@ -87,10 +116,13 @@ public class InMemorySecretRepository(ITenantAccessor? tenantAccessor = null) : return Task.FromResult(true); } - if (_secrets.Values.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret))) + if (_secrets.Values.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret, nameValidator, tenancyEnabled))) + return Task.FromResult(false); + + // Insert-side ID collisions must have the same non-mutating Try contract as the file repository. + if (_secrets.ContainsKey(secret.Id)) return Task.FromResult(false); - EnsureIdAvailable(secret); _secrets.Add(secret.Id, Clone(secret)); return Task.FromResult(true); } @@ -98,25 +130,28 @@ public class InMemorySecretRepository(ITenantAccessor? tenantAccessor = null) : public Task SaveAsync(Secret secret, CancellationToken cancellationToken = default) { - SecretRepositoryTenant.Stamp(secret, tenantAccessor); + SecretRepositoryTenant.Stamp(secret, tenantAccessor, tenancyEnabled); lock (_sync) { var existing = FindVisible(secret.Name); if (existing is not null) { - if (!SecretRepositoryTenant.CanReplace(existing, secret, tenantAccessor)) + if (!SecretRepositoryTenant.CanReplace(existing, secret, tenantAccessor, tenancyEnabled)) throw new InvalidOperationException($"A secret named '{secret.Name}' belongs to another tenant."); // EF updates the row found by name, retaining its primary key and tenant ownership. var replacement = Clone(secret); replacement.Id = existing.Id; - replacement.TenantId = existing.TenantId; + + if (tenancyEnabled) + replacement.TenantId = existing.TenantId; + _secrets[existing.Id] = replacement; return Task.CompletedTask; } - if (_secrets.Values.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret))) + if (_secrets.Values.Any(x => SecretRepositoryTenant.HasSameTenantName(x, secret, nameValidator, tenancyEnabled))) throw new InvalidOperationException($"A secret named '{secret.Name}' already exists."); EnsureIdAvailable(secret); @@ -127,7 +162,7 @@ public class InMemorySecretRepository(ITenantAccessor? tenantAccessor = null) : } private Secret? FindVisible(string name) => - _secrets.Values.FirstOrDefault(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor) && SecretRepositoryTenant.HasName(x, name)); + _secrets.Values.FirstOrDefault(x => SecretRepositoryTenant.IsVisible(x, tenantAccessor, tenancyEnabled) && SecretRepositoryTenant.HasName(x, name, nameValidator)); private void EnsureIdAvailable(Secret secret) { diff --git a/src/modules/Elsa.Secrets/Repositories/SecretRepositoryTenant.cs b/src/modules/Elsa.Secrets/Repositories/SecretRepositoryTenant.cs index 7c39440e5..70b73f1cf 100644 --- a/src/modules/Elsa.Secrets/Repositories/SecretRepositoryTenant.cs +++ b/src/modules/Elsa.Secrets/Repositories/SecretRepositoryTenant.cs @@ -1,4 +1,5 @@ using Elsa.Common.Multitenancy; +using Elsa.Secrets.Contracts; namespace Elsa.Secrets.Repositories; @@ -11,27 +12,27 @@ internal static class SecretRepositoryTenant public static string CurrentTenantId(ITenantAccessor? tenantAccessor) => tenantAccessor?.TenantId ?? Tenant.DefaultTenantId; - public static void Stamp(Secret secret, ITenantAccessor? tenantAccessor) + public static void Stamp(Secret secret, ITenantAccessor? tenantAccessor, bool tenancyEnabled) { - if (tenantAccessor is null || secret.TenantId == Tenant.AgnosticTenantId) + if (!tenancyEnabled || tenantAccessor is null || secret.TenantId == Tenant.AgnosticTenantId) return; secret.TenantId ??= tenantAccessor.TenantId; } - public static bool IsVisible(Secret secret, ITenantAccessor? tenantAccessor) => - TenantVisibility.IsVisible(secret.TenantId, CurrentTenantId(tenantAccessor)); + public static bool IsVisible(Secret secret, ITenantAccessor? tenantAccessor, bool tenancyEnabled) => + !tenancyEnabled || TenantVisibility.IsVisible(secret.TenantId, CurrentTenantId(tenantAccessor)); - public static bool CanReplace(Secret existing, Secret incoming, ITenantAccessor? tenantAccessor) => - TenantVisibility.CanReplaceOwnedRow(existing.TenantId, incoming.TenantId, CurrentTenantId(tenantAccessor)); + public static bool CanReplace(Secret existing, Secret incoming, ITenantAccessor? tenantAccessor, bool tenancyEnabled) => + !tenancyEnabled || TenantVisibility.CanReplaceOwnedRow(existing.TenantId, incoming.TenantId, CurrentTenantId(tenantAccessor)); - public static bool HasName(Secret secret, string name) => - string.Equals(secret.Name, name, StringComparison.OrdinalIgnoreCase); + public static bool HasName(Secret secret, string name, ISecretNameValidator nameValidator) => + string.Equals(nameValidator.Normalize(secret.Name), nameValidator.Normalize(name), StringComparison.Ordinal); - public static bool HasSameTenantName(Secret existing, Secret incoming) => - string.Equals( + public static bool HasSameTenantName(Secret existing, Secret incoming, ISecretNameValidator nameValidator, bool tenancyEnabled) => + HasName(existing, incoming.Name, nameValidator) + && (!tenancyEnabled || string.Equals( existing.TenantId ?? Tenant.DefaultTenantId, incoming.TenantId ?? Tenant.DefaultTenantId, - StringComparison.Ordinal) - && HasName(existing, incoming.Name); + StringComparison.Ordinal)); } diff --git a/test/unit/Elsa.Secrets.UnitTests/EFCoreSecretRepositoryTests.cs b/test/unit/Elsa.Secrets.UnitTests/EFCoreSecretRepositoryTests.cs index 27b6692dc..08884687e 100644 --- a/test/unit/Elsa.Secrets.UnitTests/EFCoreSecretRepositoryTests.cs +++ b/test/unit/Elsa.Secrets.UnitTests/EFCoreSecretRepositoryTests.cs @@ -1,5 +1,6 @@ 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; @@ -7,8 +8,10 @@ using Elsa.Secrets.Persistence.EFCore; using Elsa.Secrets.Persistence.EFCore.Repositories; using Elsa.Secrets.Persistence.EFCore.Sqlite.Extensions; using Elsa.Secrets.Services; +using Elsa.Tenants.Options; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Xunit; namespace Elsa.Secrets.UnitTests; @@ -46,6 +49,18 @@ public class EFCoreSecretRepositoryTests : IAsyncLifetime File.Delete(_databasePath); } + [Fact] + public void Repository_RetainsPreTenancyConstructorShape() + { + Assert.NotNull(typeof(EFCoreSecretRepository).GetConstructor([ + typeof(Store), + typeof(ISecretNameValidator)])); + Assert.NotNull(typeof(EFCoreSecretRepository).GetConstructor([ + typeof(Store), + typeof(ISecretNameValidator), + typeof(IOptions)])); + } + [Fact] public async Task SecretNamesAreUniquePerTenantRatherThanGlobally() { @@ -175,6 +190,319 @@ public class EFCoreSecretRepositoryTests : IAsyncLifetime Assert.Equal(SecretStatus.Active, reloaded.Status); } + [Fact] + public async Task TryAddOrReplaceDeletedAsync_WhenIncomingTenantDiffers_PreservesExistingTenant() + { + await WithTenantAwareRepositoryAsync(async (repository, tenantAccessor) => + { + using (UseTenant(tenantAccessor, "tenant-a")) + { + await repository.SaveAsync(new Secret + { + Id = "old", + Name = "smtp:password", + DisplayName = "Deleted password", + Status = SecretStatus.Deleted + }); + + var replacement = new Secret + { + Id = "new", + Name = "SMTP:PASSWORD", + DisplayName = "Replacement password", + TenantId = "tenant-b" + }; + var result = await repository.TryAddOrReplaceDeletedAsync(replacement); + var reloaded = await repository.GetAsync("smtp:password"); + + Assert.True(result); + Assert.Equal("tenant-a", replacement.TenantId); + Assert.NotNull(reloaded); + Assert.Equal("new", reloaded!.Id); + Assert.Equal("Replacement password", reloaded.DisplayName); + Assert.Equal("tenant-a", reloaded.TenantId); + } + + using (UseTenant(tenantAccessor, "tenant-b")) + Assert.Null(await repository.GetAsync("smtp:password")); + }); + } + + [Fact] + public async Task SaveAsync_WhenIncomingTenantDiffers_PreservesExistingTenant() + { + await WithTenantAwareRepositoryAsync(async (repository, tenantAccessor) => + { + using (UseTenant(tenantAccessor, "tenant-a")) + { + await repository.SaveAsync(new Secret + { + Id = "old", + Name = "smtp:password", + DisplayName = "Original" + }); + + await repository.SaveAsync(new Secret + { + Id = "new", + Name = "SMTP:PASSWORD", + DisplayName = "Updated", + TenantId = "tenant-b" + }); + + var reloaded = await repository.GetAsync("smtp:password"); + Assert.NotNull(reloaded); + Assert.Equal("old", reloaded!.Id); + Assert.Equal("Updated", reloaded.DisplayName); + Assert.Equal("tenant-a", reloaded.TenantId); + } + + using (UseTenant(tenantAccessor, "tenant-b")) + Assert.Null(await repository.GetAsync("smtp:password")); + }); + } + + [Fact] + public async Task TryAddOrReplaceDeletedAsync_WhenTenancyIsDisabled_PreservesLegacyReplacementBehavior() + { + await using var scope = _serviceProvider.CreateAsyncScope(); + var repository = scope.ServiceProvider.GetRequiredService(); + await repository.SaveAsync(new Secret + { + Id = "old", + Name = "smtp:password", + DisplayName = "Deleted password", + Status = SecretStatus.Deleted, + TenantId = Tenant.AgnosticTenantId + }); + + var replacement = new Secret + { + Id = "new", + Name = "SMTP:PASSWORD", + DisplayName = "Replacement password", + TenantId = "tenant-b" + }; + + Assert.True(await repository.TryAddOrReplaceDeletedAsync(replacement)); + var reloaded = await repository.GetAsync("smtp:password"); + Assert.NotNull(reloaded); + Assert.Equal("tenant-b", reloaded!.TenantId); + Assert.Equal("new", reloaded.Id); + } + + [Fact] + public async Task SaveAsync_WhenTenancyIsDisabled_KeepsStoredIdButAcceptsIncomingTenant() + { + await using var scope = _serviceProvider.CreateAsyncScope(); + var repository = scope.ServiceProvider.GetRequiredService(); + await repository.AddAsync(new Secret + { + Id = "stored-id", + Name = "save:secret", + DisplayName = "Original", + TenantId = "tenant-a" + }); + + var incoming = new Secret + { + Id = "incoming-id", + Name = " SAVE:SECRET ", + DisplayName = "Updated", + TenantId = "tenant-b" + }; + await repository.SaveAsync(incoming); + + var stored = await repository.GetAsync("save:secret"); + Assert.NotNull(stored); + Assert.Equal("stored-id", stored!.Id); + Assert.Equal("tenant-b", stored.TenantId); + } + + [Fact] + public async Task SaveAsync_RejectsNamedWriterUpdatingAgnosticSecretWithoutMutation() + { + await WithTenantAwareRepositoryAsync(async (repository, tenantAccessor) => + { + using (UseTenant(tenantAccessor, Tenant.AgnosticTenantId)) + { + await repository.SaveAsync(new Secret + { + Id = "agnostic-id", + Name = "smtp:password", + DisplayName = "Agnostic secret", + TenantId = Tenant.AgnosticTenantId + }); + } + + using (UseTenant(tenantAccessor, "tenant-a")) + { + var exception = await Assert.ThrowsAsync(() => repository.SaveAsync(new Secret + { + Id = "replacement-id", + Name = "SMTP:PASSWORD", + DisplayName = "Forged update" + })); + + Assert.Equal("A secret named 'SMTP:PASSWORD' belongs to another tenant.", exception.Message); + var unchanged = await repository.GetAsync("smtp:password"); + Assert.NotNull(unchanged); + Assert.Equal("agnostic-id", unchanged!.Id); + Assert.Equal("Agnostic secret", unchanged.DisplayName); + Assert.Equal(Tenant.AgnosticTenantId, unchanged.TenantId); + } + }); + } + + [Fact] + public async Task TryAddOrReplaceDeletedAsync_RejectsNamedWriterReplacingAgnosticSecretWithoutMutation() + { + await WithTenantAwareRepositoryAsync(async (repository, tenantAccessor) => + { + using (UseTenant(tenantAccessor, Tenant.AgnosticTenantId)) + { + await repository.SaveAsync(new Secret + { + Id = "agnostic-id", + Name = "smtp:password", + DisplayName = "Deleted agnostic secret", + Status = SecretStatus.Deleted, + TenantId = Tenant.AgnosticTenantId + }); + } + + using (UseTenant(tenantAccessor, "tenant-a")) + { + var result = await repository.TryAddOrReplaceDeletedAsync(new Secret + { + Id = "replacement-id", + Name = "SMTP:PASSWORD", + DisplayName = "Forged replacement" + }); + + Assert.False(result); + var unchanged = await repository.GetAsync("smtp:password"); + Assert.NotNull(unchanged); + Assert.Equal("agnostic-id", unchanged!.Id); + Assert.Equal("Deleted agnostic secret", unchanged.DisplayName); + Assert.Equal(SecretStatus.Deleted, unchanged.Status); + Assert.Equal(Tenant.AgnosticTenantId, unchanged.TenantId); + } + }); + } + + [Fact] + public async Task TryAddOrReplaceDeletedAsync_AllowsAgnosticWriterReplacingAgnosticSecret() + { + await WithTenantAwareRepositoryAsync(async (repository, tenantAccessor) => + { + using (UseTenant(tenantAccessor, Tenant.AgnosticTenantId)) + { + await repository.SaveAsync(new Secret + { + Id = "agnostic-id", + Name = "smtp:password", + DisplayName = "Deleted agnostic secret", + Status = SecretStatus.Deleted, + TenantId = Tenant.AgnosticTenantId + }); + + var result = await repository.TryAddOrReplaceDeletedAsync(new Secret + { + Id = "replacement-id", + Name = "SMTP:PASSWORD", + DisplayName = "Replacement agnostic secret" + }); + + Assert.True(result); + var replacement = await repository.GetAsync("smtp:password"); + Assert.NotNull(replacement); + Assert.Equal("replacement-id", replacement!.Id); + Assert.Equal("Replacement agnostic secret", replacement.DisplayName); + Assert.Equal(Tenant.AgnosticTenantId, replacement.TenantId); + } + }); + } + + [Fact] + public async Task PreTenancyConstructor_UsesTenantFilteredModelForOwnershipChecks() + { + await WithTenantAwareRepositoryAsync(async (repository, tenantAccessor) => + { + using (UseTenant(tenantAccessor, Tenant.AgnosticTenantId)) + { + await repository.SaveAsync(new Secret + { + Id = "agnostic-id", + Name = "smtp:password", + DisplayName = "Deleted agnostic secret", + Status = SecretStatus.Deleted, + TenantId = Tenant.AgnosticTenantId + }); + } + + using (UseTenant(tenantAccessor, "tenant-a")) + { + var result = await repository.TryAddOrReplaceDeletedAsync(new Secret + { + Id = "replacement-id", + Name = "SMTP:PASSWORD", + DisplayName = "Forged replacement" + }); + + Assert.False(result); + var unchanged = await repository.GetAsync("smtp:password"); + Assert.NotNull(unchanged); + Assert.Equal("agnostic-id", unchanged!.Id); + Assert.Equal(SecretStatus.Deleted, unchanged.Status); + } + }, useLegacyConstructor: true); + } + + private static async Task WithTenantAwareRepositoryAsync(Func test, bool useLegacyConstructor = false) + { + var databasePath = Path.Join(Path.GetTempPath(), $"elsa-secrets-tenant-{Guid.NewGuid():N}.db"); + var tenantAccessor = new DefaultTenantAccessor(); + var services = new ServiceCollection() + .AddSqliteEntityModelCreatingHandlers() + .AddSingleton(tenantAccessor) + .Configure(options => options.IsEnabled = true) + .AddScoped() + .AddScoped() + .AddDbContextFactory(builder => builder + .EnableServiceProviderCaching(false) + .UseElsaSqlite(typeof(SqliteSecretsPersistenceFeatureExtensions).Assembly, $"Data Source={databasePath}")) + .Decorate, TenantAwareDbContextFactory>() + .AddSingleton() + .AddScoped>() + .AddScoped() + .BuildServiceProvider(); + + try + { + await using var scope = services.CreateAsyncScope(); + var factory = scope.ServiceProvider.GetRequiredService>(); + await using (var dbContext = await factory.CreateDbContextAsync()) + await dbContext.Database.MigrateAsync(); + + var repository = useLegacyConstructor + ? new EFCoreSecretRepository( + scope.ServiceProvider.GetRequiredService>(), + scope.ServiceProvider.GetRequiredService()) + : scope.ServiceProvider.GetRequiredService(); + await test(repository, tenantAccessor); + } + finally + { + await services.DisposeAsync(); + if (File.Exists(databasePath)) + File.Delete(databasePath); + } + } + + private static IDisposable UseTenant(DefaultTenantAccessor tenantAccessor, string tenantId) => + tenantAccessor.PushContext(new Tenant { Id = tenantId, Name = tenantId }); + private static async Task InsertSecretAsync(SecretsElsaDbContext dbContext, string id, string name, string tenantId) { var secret = new Secret diff --git a/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryTenantIsolationTests.cs b/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryTenantIsolationTests.cs index 576247f9b..4ba1b1c70 100644 --- a/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryTenantIsolationTests.cs +++ b/test/unit/Elsa.Secrets.UnitTests/SecretRepositoryTenantIsolationTests.cs @@ -1,8 +1,11 @@ using Elsa.Common.Multitenancy; using Elsa.Secrets.Contracts; +using Elsa.Secrets.Extensions; using Elsa.Secrets.Models; using Elsa.Secrets.Options; using Elsa.Secrets.Repositories; +using Elsa.Tenants.Options; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -17,15 +20,33 @@ public class SecretRepositoryTenantIsolationTests [Fact] public void Repositories_RetainPreTenancyConstructorShapes() { + var options = Microsoft.Extensions.Options.Options.Create(new SecretsOptions()); + _ = new FileSecretRepository(options, null); + _ = new FileSecretRepository(options, tenantAccessor: new DefaultTenantAccessor()); + + var binaryConstructor = typeof(FileSecretRepository).GetConstructor([ + typeof(IOptions), + typeof(ILogger)])!; + Assert.False(binaryConstructor.GetParameters()[1].IsOptional); + + var compatibilityConstructor = typeof(FileSecretRepository).GetConstructor([ + typeof(IOptions), + typeof(ILogger), + typeof(ITenantAccessor)])!; + Assert.True(compatibilityConstructor.GetParameters()[1].IsOptional); + Assert.True(compatibilityConstructor.GetParameters()[2].IsOptional); Assert.NotNull(typeof(FileSecretRepository).GetConstructor([ typeof(IOptions), - typeof(ILogger)])); - Assert.NotNull(typeof(FileSecretRepository).GetConstructor([ - typeof(IOptions), + typeof(IOptions), + typeof(ISecretNameValidator), typeof(ILogger), typeof(ITenantAccessor)])); Assert.NotNull(typeof(InMemorySecretRepository).GetConstructor(Type.EmptyTypes)); Assert.NotNull(typeof(InMemorySecretRepository).GetConstructor([typeof(ITenantAccessor)])); + Assert.NotNull(typeof(InMemorySecretRepository).GetConstructor([ + typeof(IOptions), + typeof(ISecretNameValidator), + typeof(ITenantAccessor)])); } [Fact] @@ -256,7 +277,7 @@ public class SecretRepositoryTenantIsolationTests { var accessor = new MutableTenantAccessor(null); var options = Microsoft.Extensions.Options.Options.Create(new SecretsOptions { RepositoryFilePath = path }); - await AssertNullAndEmptyAreDuplicatesAsync(accessor, new FileSecretRepository(options, tenantAccessor: accessor)); + await AssertNullAndEmptyAreDuplicatesAsync(accessor, new FileSecretRepository(options, null, accessor)); } finally { @@ -265,6 +286,136 @@ public class SecretRepositoryTenantIsolationTests } } + [Fact] + public async Task Repositories_NormalizeNamesForLookupAndUniqueness() + { + await ForEachRepositoryAsync(async (tenantAccessor, repository) => + { + using (UseTenant(tenantAccessor, "tenant-a")) + { + await repository.AddAsync(new Secret { Name = " smtp:password ", DisplayName = "Original" }); + Assert.Equal("Original", (await repository.GetAsync(" SMTP:PASSWORD "))!.DisplayName); + + await Assert.ThrowsAsync(() => repository.AddAsync(new Secret + { + Name = "SMTP:PASSWORD", + DisplayName = "Duplicate" + })); + + await repository.SaveAsync(new Secret { Name = " SMTP:PASSWORD ", DisplayName = "Updated" }); + Assert.Equal("Updated", (await repository.GetAsync("smtp:password"))!.DisplayName); + Assert.False(await repository.TryAddOrReplaceDeletedAsync(new Secret + { + Name = " smtp:password ", + DisplayName = "Active duplicate" + })); + } + + using (UseTenant(tenantAccessor, "tenant-b")) + { + await repository.AddAsync(new Secret { Name = "SMTP:PASSWORD", DisplayName = "Tenant B" }); + Assert.Equal("Tenant B", (await repository.GetAsync(" smtp:password "))!.DisplayName); + } + }); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Repositories_CreatedThroughDiFollowTenancyOption(bool tenancyEnabled) + { + await ForEachDiRepositoryAsync(tenancyEnabled, async (tenantAccessor, repository) => + { + using (UseTenant(tenantAccessor, "tenant-a")) + await repository.AddAsync(new Secret { Name = "smtp:password", DisplayName = "Tenant A" }); + + using (UseTenant(tenantAccessor, "tenant-b")) + { + if (tenancyEnabled) + { + Assert.Null(await repository.GetAsync("smtp:password")); + await repository.AddAsync(new Secret { Name = "SMTP:PASSWORD", DisplayName = "Tenant B" }); + } + else + { + Assert.Equal("Tenant A", (await repository.GetAsync("smtp:password"))!.DisplayName); + await Assert.ThrowsAsync(() => repository.AddAsync(new Secret + { + Name = "SMTP:PASSWORD", + DisplayName = "Duplicate" + })); + + await repository.AddAsync(new Secret + { + Name = "explicit:secret", + DisplayName = "Explicit Tenant A", + TenantId = "tenant-a" + }); + + Assert.Equal("Explicit Tenant A", (await repository.GetAsync("EXPLICIT:SECRET"))!.DisplayName); + Assert.Contains(await repository.ListAsync(), x => x.Name == "explicit:secret"); + await Assert.ThrowsAsync(() => repository.AddAsync(new Secret + { + Name = " explicit:secret ", + DisplayName = "Duplicate Explicit", + TenantId = "tenant-b" + })); + } + } + }); + } + + [Fact] + public async Task Repositories_WhenTenancyIsDisabledPreserveIncomingTenantOnReplacement() + { + await ForEachDiRepositoryAsync(false, async (_, repository) => + { + await repository.AddAsync(new Secret + { + Id = "save-existing", + Name = "save:secret", + DisplayName = "Original", + TenantId = "tenant-a" + }); + + await repository.SaveAsync(new Secret + { + Id = "save-incoming", + Name = "SAVE:SECRET", + DisplayName = "Updated", + TenantId = "tenant-b" + }); + + var saved = await repository.GetAsync("save:secret"); + Assert.NotNull(saved); + Assert.Equal("save-existing", saved!.Id); + Assert.Equal("tenant-b", saved.TenantId); + + await repository.AddAsync(new Secret + { + Id = "deleted-existing", + Name = "replace:secret", + DisplayName = "Deleted", + Status = SecretStatus.Deleted, + TenantId = "tenant-a" + }); + + var replaced = await repository.TryAddOrReplaceDeletedAsync(new Secret + { + Id = "replace-incoming", + Name = "REPLACE:SECRET", + DisplayName = "Replacement", + TenantId = "tenant-b" + }); + + Assert.True(replaced); + var replacement = await repository.GetAsync("replace:secret"); + Assert.NotNull(replacement); + Assert.Equal("replace-incoming", replacement!.Id); + Assert.Equal("tenant-b", replacement.TenantId); + }); + } + [Fact] public async Task FileRepository_DefaultTenantKeepsReadingLegacyNullTenantRows() { @@ -276,12 +427,12 @@ public class SecretRepositoryTenantIsolationTests await legacyRepository.AddAsync(new Secret { Name = "legacy:secret", DisplayName = "Legacy" }); var namedAccessor = new DefaultTenantAccessor(); - var namedRepository = new FileSecretRepository(options, tenantAccessor: namedAccessor); + var namedRepository = new FileSecretRepository(options, null, namedAccessor); using (UseTenant(namedAccessor, "tenant-a")) Assert.Empty(await namedRepository.ListAsync()); var defaultAccessor = new DefaultTenantAccessor(); - var defaultRepository = new FileSecretRepository(options, tenantAccessor: defaultAccessor); + var defaultRepository = new FileSecretRepository(options, null, defaultAccessor); Assert.Equal("Legacy", (await defaultRepository.GetAsync("legacy:secret"))!.DisplayName); Assert.Null((await defaultRepository.GetAsync("legacy:secret"))!.TenantId); } @@ -322,7 +473,7 @@ public class SecretRepositoryTenantIsolationTests { var fileAccessor = new DefaultTenantAccessor(); var options = Microsoft.Extensions.Options.Options.Create(new SecretsOptions { RepositoryFilePath = path }); - await test(fileAccessor, new FileSecretRepository(options, tenantAccessor: fileAccessor)); + await test(fileAccessor, new FileSecretRepository(options, null, fileAccessor)); } finally { @@ -331,6 +482,36 @@ public class SecretRepositoryTenantIsolationTests } } + private static async Task ForEachDiRepositoryAsync(bool tenancyEnabled, Func test) + { + await using (var inMemoryProvider = CreateDiRepository(tenancyEnabled)) + await test(inMemoryProvider.GetRequiredService(), inMemoryProvider.GetRequiredService()); + + var path = Path.Join(Path.GetTempPath(), $"elsa-secrets-{Guid.NewGuid():N}.json"); + try + { + await using var fileProvider = CreateDiRepository(tenancyEnabled, path); + await test(fileProvider.GetRequiredService(), fileProvider.GetRequiredService()); + } + finally + { + if (File.Exists(path)) + File.Delete(path); + } + } + + private static ServiceProvider CreateDiRepository(bool tenancyEnabled, string? repositoryFilePath = null) + where TRepository : class, ISecretRepository + { + return new ServiceCollection() + .AddSingleton() + .AddSingleton(services => services.GetRequiredService()) + .AddSecretsServices(options => options.RepositoryFilePath = repositoryFilePath) + .Configure(options => options.IsEnabled = tenancyEnabled) + .AddSingleton() + .BuildServiceProvider(); + } + private sealed class MutableTenantAccessor(string? tenantId) : ITenantAccessor { public string TenantId { get; set; } = tenantId!; diff --git a/test/unit/Elsa.Secrets.UnitTests/SecretStoreTests.cs b/test/unit/Elsa.Secrets.UnitTests/SecretStoreTests.cs index 41804452d..c8dabe12d 100644 --- a/test/unit/Elsa.Secrets.UnitTests/SecretStoreTests.cs +++ b/test/unit/Elsa.Secrets.UnitTests/SecretStoreTests.cs @@ -1,3 +1,4 @@ +using Elsa.Secrets.Contracts; using Elsa.Secrets.Models; using Elsa.Secrets.Options; using Elsa.Secrets.Repositories; @@ -174,6 +175,12 @@ public class SecretStoreTests }); } + [Fact] + public async Task FileRepository_TryAddOrReplaceDeletedAsync_RejectsCollidingIdWhenInserting() + { + await WithFileRepositoryAsync(async (repository, _) => await AssertInsertIdCollisionRejectedAsync(repository)); + } + [Fact] public async Task FileRepository_TryAddOrReplaceDeletedAsync_RejectsCollidingIdWithoutMutatingFile() { @@ -239,6 +246,12 @@ public class SecretStoreTests Assert.Equal("owned-id", (await repository.GetAsync("other:secret"))!.Id); } + [Fact] + public async Task InMemoryRepository_TryAddOrReplaceDeletedAsync_RejectsCollidingIdWhenInserting() + { + await AssertInsertIdCollisionRejectedAsync(new InMemorySecretRepository()); + } + [Fact] public async Task FileRepository_RecoversFromCorruptJson() { @@ -291,4 +304,20 @@ public class SecretStoreTests File.Delete(path); } } + + private static async Task AssertInsertIdCollisionRejectedAsync(ISecretRepository repository) + { + await repository.AddAsync(new Secret { Id = "existing-id", Name = "existing:secret", DisplayName = "Existing" }); + + var result = await repository.TryAddOrReplaceDeletedAsync(new Secret + { + Id = "existing-id", + Name = "new:secret", + DisplayName = "New" + }); + + Assert.False(result); + Assert.Equal("Existing", (await repository.GetAsync("existing:secret"))!.DisplayName); + Assert.Null(await repository.GetAsync("new:secret")); + } }