From 168a8c76f0b0f93b60e9669a21d5fd8dfb32b275 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 27 Aug 2026 11:45:54 +0200 Subject: [PATCH] fix(auth): validate wildcard permission patterns and warn on deny-list stripping (#7997) * fix(auth): validate wildcard permission patterns and warn on deny-list stripping Permission.IsValidPattern rejects inert wildcard spellings (such as "workflows*:delete") that parse but can never match. The grant boundary, stored-permission, and external-authentication options validators reject them at authoring time, and PermissionGrantValidator applies the same check to incoming grants. ExternalAuthenticationOptionsValidator now warns (never fails) when DeniedPermissions is non-empty, because any non-empty deny list refuses every wildcard grant that could reach a denied permission -- including the seeded administrator role's "*". The validator takes an ILogger, and AddExternalAuthenticationServices registers logging alongside its other framework dependencies (TryAdd-based, so host logging configuration wins). The operational consequence is recorded in the authorization-model migration guide. Co-Authored-By: Claude Fable 5 * fix(auth): report subtree grants whose verb nothing under them supports 'workflows/*:frobnicate' reached a non-empty subtree and was therefore treated as resolved, so the startup audit stayed silent about a grant that cannot authorize anything. Require at least one reached descriptor to support a concrete verb; verb wildcards keep the reach-only check. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Fable 5 --- doc/migrations/authorization-model.md | 5 +- .../Authorization/Permission.cs | 26 ++++++ .../Permissions/PermissionGrantValidator.cs | 8 ++ .../Extensions/ServiceCollectionExtensions.cs | 4 + .../ExternalAuthenticationOptionsValidator.cs | 24 ++++- .../StoredPermissionValidator.cs | 18 +++- .../Authorization/PermissionTests.cs | 17 ++++ .../PermissionGrantValidatorTests.cs | 15 +++ .../ExternalAuthenticationOptionsTests.cs | 3 +- ...rnalAuthenticationOptionsValidatorTests.cs | 83 ++++++++++++++++- ...ExternalAuthenticationHandleHasherTests.cs | 3 +- .../StoredPermissionValidatorTests.cs | 91 +++++++++++++++++++ 12 files changed, 286 insertions(+), 11 deletions(-) create mode 100644 test/unit/Elsa.Identity.UnitTests/HostedServices/StoredPermissionValidatorTests.cs diff --git a/doc/migrations/authorization-model.md b/doc/migrations/authorization-model.md index 78f02d45d..13d7a3330 100644 --- a/doc/migrations/authorization-model.md +++ b/doc/migrations/authorization-model.md @@ -61,7 +61,10 @@ by exact string, so they read the way a role does. connection granting `workflows/*:delete` is denied by a deny list naming only `workflows/definitions:delete`. Before this release both comparisons were exact, so either spelling slipped past the other and a deployment's deny list did not hold. If you carried a deny list across the upgrade, re-read it: it may now deny more than it - used to, which is the intent. + used to, which is the intent. A consequence to plan for: any non-empty deny list refuses every wildcard grant + that could reach a denied permission, and `*` (which parses to `*:*`) reaches all of them — so a role holding + `*`, including the seeded administrator role, will not survive external issuance. Operators using + `DeniedPermissions` must give externally-authenticating administrators enumerated grants instead of `*`. - **Allowed** is matched one way: an allow entry must cover the whole grant. `workflows/*:delete` admits `workflows/definitions:delete`, but an allow list naming only `workflows/definitions:delete` refuses a `workflows/*:delete` grant rather than admitting the part that overlaps. diff --git a/src/common/Elsa.Api.Common/Authorization/Permission.cs b/src/common/Elsa.Api.Common/Authorization/Permission.cs index 64867674d..5aeeb0bf6 100644 --- a/src/common/Elsa.Api.Common/Authorization/Permission.cs +++ b/src/common/Elsa.Api.Common/Authorization/Permission.cs @@ -37,6 +37,32 @@ public readonly record struct Permission(string Resource, string Verb) /// Whether either axis carries a wildcard. public bool HasWildcard => IsResourceWildcard || IsVerbWildcard || IsSubtree; + /// + /// Whether every * this permission carries sits where the matcher gives it meaning: the entire + /// resource (*), a trailing /* subtree segment with no other *, or the entire verb. + /// + /// + /// stays lenient because stored roles may hold historical strings, so a stray + /// wildcard such as workflows* or work*/foo parses yet can never match anything. Validation + /// paths use this check to surface those entries instead of letting them silently match nothing — which + /// in a deny list would mean silently not denying. + /// + public bool IsValidPattern + { + get + { + if (!IsVerbWildcard && Verb.Contains(Wildcard, StringComparison.Ordinal)) + return false; + + var first = Resource.IndexOf(Wildcard, StringComparison.Ordinal); + + if (first < 0) + return true; + + return first == Resource.LastIndexOf(Wildcard, StringComparison.Ordinal) && (IsResourceWildcard || IsSubtree); + } + } + /// /// Parses , returning false when it is not a well-formed permission. /// diff --git a/src/common/Elsa.Api.Common/Permissions/PermissionGrantValidator.cs b/src/common/Elsa.Api.Common/Permissions/PermissionGrantValidator.cs index 317b6db95..69ac2cb3d 100644 --- a/src/common/Elsa.Api.Common/Permissions/PermissionGrantValidator.cs +++ b/src/common/Elsa.Api.Common/Permissions/PermissionGrantValidator.cs @@ -48,6 +48,14 @@ public sealed class PermissionGrantValidator(IPermissionDescriptorRegistry regis continue; } + // An entry like 'workflows*:delete' parses but the matcher never satisfies it, so it would + // be persisted as a grant that silently reaches nothing. + if (!permission.IsValidPattern) + { + errors.Add(new(value, "Places '*' where it has no meaning and would match nothing. A wildcard may only be the entire resource ('*'), a trailing '/*' segment ('workflows/*'), or the entire verb ('workflows/definitions:*').")); + continue; + } + if (permission.IsResourceWildcard || permission.IsSubtree) continue; diff --git a/src/modules/Elsa.ExternalAuthentication/Extensions/ServiceCollectionExtensions.cs b/src/modules/Elsa.ExternalAuthentication/Extensions/ServiceCollectionExtensions.cs index 9af9ba70b..f8988cb57 100644 --- a/src/modules/Elsa.ExternalAuthentication/Extensions/ServiceCollectionExtensions.cs +++ b/src/modules/Elsa.ExternalAuthentication/Extensions/ServiceCollectionExtensions.cs @@ -49,6 +49,10 @@ public static class ServiceCollectionExtensions services.AddExternalAuthenticationExtension(ExternalAuthenticationExtensionKind.PermissionGrantSource, ClaimMappingPermissionGrantSource.SourceType); services.AddExternalAuthenticationExtension(ExternalAuthenticationExtensionKind.PermissionGrantSource, GroupMappingPermissionGrantSource.SourceType); services.AddExternalAuthenticationExtension(ExternalAuthenticationExtensionKind.PermissionGrantSource, ClaimPassThroughPermissionGrantSource.SourceType); + // The validator warns about grant-boundary configuration, and ValidateOnStart resolves it on any + // IOptions access, so a logger has to be resolvable even on a bare service collection. AddLogging is + // TryAdd-based, so a host that already configured logging keeps its own. + services.AddLogging(); services.TryAddEnumerable(ServiceDescriptor.Singleton, ExternalAuthenticationOptionsValidator>()); services.AddDataProtection(); services.AddRateLimiter(_ => { }); diff --git a/src/modules/Elsa.ExternalAuthentication/Validation/ExternalAuthenticationOptionsValidator.cs b/src/modules/Elsa.ExternalAuthentication/Validation/ExternalAuthenticationOptionsValidator.cs index 5b4cb7c9b..441d7c90c 100644 --- a/src/modules/Elsa.ExternalAuthentication/Validation/ExternalAuthenticationOptionsValidator.cs +++ b/src/modules/Elsa.ExternalAuthentication/Validation/ExternalAuthenticationOptionsValidator.cs @@ -2,6 +2,7 @@ using Elsa.Authorization; using Elsa.ExternalAuthentication.Models; using Elsa.ExternalAuthentication.Options; using Elsa.ExternalAuthentication.Policies; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Elsa.ExternalAuthentication.Validation; @@ -10,7 +11,8 @@ namespace Elsa.ExternalAuthentication.Validation; /// Validates the deployment-owned External Authentication configuration before it is used by the broker. /// public sealed class ExternalAuthenticationOptionsValidator( - IOptions extensionOptions) : IValidateOptions + IOptions extensionOptions, + ILogger logger) : IValidateOptions { public ValidateOptionsResult Validate(string? name, ExternalAuthenticationOptions options) { @@ -48,19 +50,33 @@ public sealed class ExternalAuthenticationOptionsValidator( /// quietly stop denying what it names. Failing startup puts the mistake in front of whoever can fix it /// instead of leaving it to be discovered from an issued token. /// - private static void ValidatePermissionGrantBoundary(PermissionGrantOptions? permissionGrants, ICollection failures) + private void ValidatePermissionGrantBoundary(PermissionGrantOptions? permissionGrants, ICollection failures) { if (permissionGrants is null) return; ValidatePermissionPatterns(permissionGrants.AllowedPermissions, "AllowedPermissions", failures); ValidatePermissionPatterns(permissionGrants.DeniedPermissions, "DeniedPermissions", failures); + + // Deny entries match in both directions, so a wildcard grant that could reach a denied permission is + // refused whole rather than narrowed — a role holding '*' does not survive external token issuance. + // That is intended, but surprising enough at sign-in time to be worth announcing at startup. + if (permissionGrants.DeniedPermissions is { Count: > 0 }) + logger.LogWarning( + "ExternalAuthentication:PermissionGrants:DeniedPermissions is configured. Deny entries match wildcard grants in both directions, so any grant that could reach a denied permission is refused entirely — a role holding '*' will carry no permissions into an externally issued token. Give externally-authenticating administrators enumerated grants instead of '*'."); } private static void ValidatePermissionPatterns(IEnumerable? permissions, string listName, ICollection failures) { - foreach (var permission in (permissions ?? []).Where(x => !Permission.TryParse(x, out _))) - failures.Add($"'{permission}' in ExternalAuthentication:PermissionGrants:{listName} is not a well-formed permission. Expected '{{resource}}:{{verb}}', for example 'workflows/*:delete'."); + foreach (var permission in permissions ?? []) + { + if (!Permission.TryParse(permission, out var parsed)) + failures.Add($"'{permission}' in ExternalAuthentication:PermissionGrants:{listName} is not a well-formed permission. Expected '{{resource}}:{{verb}}', for example 'workflows/*:delete'."); + // An entry like 'workflows*:delete' parses but the matcher never satisfies it, so in a deny + // list it would silently stop denying what it names. + else if (!parsed.IsValidPattern) + failures.Add($"'{permission}' in ExternalAuthentication:PermissionGrants:{listName} places '*' where it has no meaning and would match nothing. A wildcard may only be the entire resource ('*'), a trailing '/*' segment ('workflows/*'), or the entire verb ('workflows/definitions:*')."); + } } private static void ValidateExternalCallbackBaseUri(RedirectValidationOptions? redirects, ICollection failures) diff --git a/src/modules/Elsa.Identity/HostedServices/StoredPermissionValidator.cs b/src/modules/Elsa.Identity/HostedServices/StoredPermissionValidator.cs index 64ed4de8c..4081f4493 100644 --- a/src/modules/Elsa.Identity/HostedServices/StoredPermissionValidator.cs +++ b/src/modules/Elsa.Identity/HostedServices/StoredPermissionValidator.cs @@ -69,12 +69,26 @@ public class StoredPermissionValidator(IServiceScopeFactory scopeFactory, ILogge private static bool Resolves(IPermissionDescriptorRegistry registry, string value) { - if (!Permission.TryParse(value, out var permission)) + if (!Permission.TryParse(value, out var permission) || !permission.IsValidPattern) return false; - if (permission.IsResourceWildcard || permission.IsSubtree) + if (permission.IsResourceWildcard) return true; + // A subtree grant reaching nothing is far more likely a typo ('workflow/*') than a grant for a + // module yet to be installed, so it is reported rather than assumed forward-reaching. + if (permission.IsSubtree) + { + var reached = registry.Reach(permission.Resource); + + // A concrete verb is only resolved when something under the subtree actually supports it: + // 'workflows/*:frobnicate' reaches plenty and authorizes nothing, which is the same inert + // grant an unreachable subtree is, and deserves the same warning. + return permission.IsVerbWildcard + ? reached.Count > 0 + : reached.Any(x => registry.Find(x)?.Supports(permission.Verb) == true); + } + var descriptor = registry.Find(permission.Resource); return descriptor is not null && (permission.IsVerbWildcard || descriptor.Supports(permission.Verb)); diff --git a/test/unit/Elsa.Api.Common.UnitTests/Authorization/PermissionTests.cs b/test/unit/Elsa.Api.Common.UnitTests/Authorization/PermissionTests.cs index 1cb70e7d3..f6f013a9a 100644 --- a/test/unit/Elsa.Api.Common.UnitTests/Authorization/PermissionTests.cs +++ b/test/unit/Elsa.Api.Common.UnitTests/Authorization/PermissionTests.cs @@ -63,4 +63,21 @@ public class PermissionTests Assert.Equal(subtree, permission.IsSubtree); Assert.Equal(resourceWildcard || verbWildcard || subtree, permission.HasWildcard); } + + [Theory] + [InlineData("workflows/definitions:view", true)] + [InlineData("workflows/*:view", true)] + [InlineData("workflows/definitions:*", true)] + [InlineData("*:*", true)] + [InlineData("workflows*:delete", false)] // missing slash: not a subtree pattern + [InlineData("work*/foo:view", false)] // embedded wildcard mid-resource + [InlineData("work*/definitions/*:view", false)] // trailing '/*' does not redeem an embedded '*' + [InlineData("workflows/*/versions:view", false)] // '*' as a middle segment + [InlineData("workflows:del*", false)] // embedded wildcard in the verb + public void RecognizesWildcardsTheMatcherNeverSatisfies(string value, bool valid) + { + // Such strings parse — TryParse stays lenient for stored roles — but validation paths reject them, + // because a pattern that matches nothing in a deny list silently stops denying. + Assert.Equal(valid, Permission.Parse(value).IsValidPattern); + } } diff --git a/test/unit/Elsa.Api.Common.UnitTests/Permissions/PermissionGrantValidatorTests.cs b/test/unit/Elsa.Api.Common.UnitTests/Permissions/PermissionGrantValidatorTests.cs index b1edf5635..a12e50a6c 100644 --- a/test/unit/Elsa.Api.Common.UnitTests/Permissions/PermissionGrantValidatorTests.cs +++ b/test/unit/Elsa.Api.Common.UnitTests/Permissions/PermissionGrantValidatorTests.cs @@ -66,6 +66,21 @@ public class PermissionGrantValidatorTests Assert.Contains("view, write", result.Errors.Single().Reason); } + [Theory] + [InlineData("workflows*:delete")] + [InlineData("work*/foo/*:view")] + [InlineData("workflows/*/instances:view")] + [InlineData("workflows/definitions:vi*w")] + public void RejectsWildcardsTheMatcherNeverSatisfies(string permission) + { + // These parse, but the matcher never satisfies them; accepting them would persist a grant + // that silently reaches nothing. + var result = Validator.Validate([permission]); + + Assert.False(result.IsValid); + Assert.Contains("would match nothing", result.Errors.Single().Reason); + } + [Theory] [InlineData("not a permission")] [InlineData("workflows/definitions")] diff --git a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationOptionsTests.cs b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationOptionsTests.cs index 8144668d4..8660379d9 100644 --- a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationOptionsTests.cs +++ b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationOptionsTests.cs @@ -2,6 +2,7 @@ using Elsa.ExternalAuthentication.Contracts; using Elsa.ExternalAuthentication.Models; using Elsa.ExternalAuthentication.Options; using Elsa.ExternalAuthentication.Validation; +using Microsoft.Extensions.Logging.Abstractions; using Elsa.Extensions; using Microsoft.Extensions.Configuration; @@ -206,7 +207,7 @@ public class ExternalAuthenticationOptionsTests extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.Adapter, adapter.Type)); extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.UnlinkedIdentityPolicy, "reject")); extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.PermissionGrantSource, "elsa-roles")); - return new(Microsoft.Extensions.Options.Options.Create(extensions)); + return new(Microsoft.Extensions.Options.Options.Create(extensions), NullLogger.Instance); } private sealed class TestAdapter(string type) : IExternalAuthenticationAdapter diff --git a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationOptionsValidatorTests.cs b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationOptionsValidatorTests.cs index 15fcedbdd..77e0ec428 100644 --- a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationOptionsValidatorTests.cs +++ b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/ExternalAuthenticationOptionsValidatorTests.cs @@ -1,11 +1,14 @@ using Elsa.ExternalAuthentication.Models; using Elsa.ExternalAuthentication.Options; using Elsa.ExternalAuthentication.Validation; +using Microsoft.Extensions.Logging; namespace Elsa.ExternalAuthentication.UnitTests.Foundational; public class ExternalAuthenticationOptionsValidatorTests { + private readonly CapturingLogger _logger = new(); + [Theory] // The legacy spelling carries two colons, so it parses as nothing and would silently stop bounding anything. [InlineData("external-authentication:connections:read")] @@ -27,6 +30,29 @@ public class ExternalAuthenticationOptionsValidatorTests Assert.Contains(deniedResult.Failures!, x => x.Contains("DeniedPermissions") && x.Contains("well-formed permission")); } + [Theory] + // These parse, so they slipped past well-formedness — yet the matcher never satisfies them. In a deny + // list that is a silent un-denying, exactly what boundary validation exists to prevent. + [InlineData("workflows*:delete")] + [InlineData("work*/foo:view")] + [InlineData("work*/definitions/*:view")] + [InlineData("workflows:del*")] + public void RejectsAGrantBoundaryEntryWithAWildcardTheMatcherNeverSatisfies(string permission) + { + var allowed = new ExternalAuthenticationOptions(); + allowed.PermissionGrants.AllowedPermissions = [permission]; + var denied = new ExternalAuthenticationOptions(); + denied.PermissionGrants.DeniedPermissions = [permission]; + + var allowedResult = CreateValidator().Validate(null, allowed); + var deniedResult = CreateValidator().Validate(null, denied); + + Assert.False(allowedResult.Succeeded); + Assert.Contains(allowedResult.Failures!, x => x.Contains("AllowedPermissions") && x.Contains("would match nothing")); + Assert.False(deniedResult.Succeeded); + Assert.Contains(deniedResult.Failures!, x => x.Contains("DeniedPermissions") && x.Contains("would match nothing")); + } + [Fact] public void AcceptsAGrantBoundaryOfWildcardPatterns() { @@ -39,6 +65,37 @@ public class ExternalAuthenticationOptionsValidatorTests Assert.DoesNotContain(result.Failures ?? [], x => x.Contains("well-formed permission")); } + /// + /// A deny list is not a failure, but it costs a wildcard grant everything it could have conferred: deny + /// matches in both directions, so '*' satisfies every deny entry and an externally-authenticated superuser + /// silently loses it while local login keeps working. The warning names that at startup rather than leaving + /// it to be diagnosed from an issued token. + /// + [Fact] + public void WarnsThatANonEmptyDenyListRefusesWildcardGrantsWhole() + { + var options = new ExternalAuthenticationOptions(); + options.PermissionGrants.DeniedPermissions = ["workflows/*:delete"]; + + var result = CreateValidator().Validate(null, options); + + Assert.True(result.Succeeded); + var warning = Assert.Single(_logger.Entries, x => x.Level == LogLevel.Warning); + Assert.Contains("DeniedPermissions", warning.Message); + Assert.Contains("refused entirely", warning.Message); + } + + [Fact] + public void DoesNotWarnWhenNoPermissionsAreDenied() + { + var options = new ExternalAuthenticationOptions(); + options.PermissionGrants.AllowedPermissions = ["workflows/*:delete"]; + + CreateValidator().Validate(null, options); + + Assert.DoesNotContain(_logger.Entries, x => x.Level == LogLevel.Warning); + } + [Fact] public void RejectsDuplicateInstalledAdapterTypes() { @@ -187,7 +244,7 @@ public class ExternalAuthenticationOptionsValidatorTests Assert.True(result.Succeeded); } - private static ExternalAuthenticationOptionsValidator CreateValidator(IEnumerable? adapters = null) + private ExternalAuthenticationOptionsValidator CreateValidator(IEnumerable? adapters = null) { var extensions = new ExternalAuthenticationExtensionOptions(); foreach (var adapter in adapters ?? [new StubAdapter("oidc")]) @@ -198,6 +255,28 @@ public class ExternalAuthenticationOptionsValidatorTests extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.PermissionGrantSource, "claim-mapping")); extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.PermissionGrantSource, "group-mapping")); extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.PermissionGrantSource, "claim-pass-through")); - return new(Microsoft.Extensions.Options.Options.Create(extensions)); + return new(Microsoft.Extensions.Options.Options.Create(extensions), _logger); + } + + private sealed class CapturingLogger : ILogger + { + public List Entries { get; } = []; + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => + Entries.Add(new(logLevel, formatter(state, exception))); + } + + private sealed record LogEntry(LogLevel Level, string Message); + + private sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + + public void Dispose() + { + } } } diff --git a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/HmacExternalAuthenticationHandleHasherTests.cs b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/HmacExternalAuthenticationHandleHasherTests.cs index 58e9d0960..6b2ce9f3b 100644 --- a/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/HmacExternalAuthenticationHandleHasherTests.cs +++ b/test/unit/Elsa.ExternalAuthentication.UnitTests/Foundational/HmacExternalAuthenticationHandleHasherTests.cs @@ -1,6 +1,7 @@ using Elsa.ExternalAuthentication.Options; using Elsa.ExternalAuthentication.Services; using Elsa.ExternalAuthentication.Validation; +using Microsoft.Extensions.Logging.Abstractions; namespace Elsa.ExternalAuthentication.UnitTests.Foundational; @@ -51,7 +52,7 @@ public class HmacExternalAuthenticationHandleHasherTests extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.PermissionGrantSource, "claim-mapping")); extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.PermissionGrantSource, "group-mapping")); extensions.Registrations.Add(new(ExternalAuthenticationExtensionKind.PermissionGrantSource, "claim-pass-through")); - var validator = new ExternalAuthenticationOptionsValidator(Microsoft.Extensions.Options.Options.Create(extensions)); + var validator = new ExternalAuthenticationOptionsValidator(Microsoft.Extensions.Options.Options.Create(extensions), NullLogger.Instance); var result = validator.Validate(null, options); diff --git a/test/unit/Elsa.Identity.UnitTests/HostedServices/StoredPermissionValidatorTests.cs b/test/unit/Elsa.Identity.UnitTests/HostedServices/StoredPermissionValidatorTests.cs new file mode 100644 index 000000000..e94cfcf82 --- /dev/null +++ b/test/unit/Elsa.Identity.UnitTests/HostedServices/StoredPermissionValidatorTests.cs @@ -0,0 +1,91 @@ +using Elsa.Identity.Contracts; +using Elsa.Identity.Entities; +using Elsa.Identity.HostedServices; +using Elsa.Identity.Models; +using Elsa.Permissions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Elsa.Identity.UnitTests.HostedServices; + +public class StoredPermissionValidatorTests +{ + private readonly CapturingLogger _logger = new(); + + [Theory] + [InlineData("*")] + [InlineData("workflows/definitions:view")] + [InlineData("workflows/definitions:*")] + [InlineData("workflows/*:view")] // reaches 'workflows/definitions' + [InlineData("workflows/definitions/*:view")] // reaches the prefix resource itself + public async Task DoesNotWarnAboutAPermissionThatResolves(string permission) + { + await StartAsync(permission); + + Assert.DoesNotContain(_logger.Entries, x => x.Level == LogLevel.Warning); + } + + [Theory] + [InlineData("workflow/*:view")] // typo'd subtree: reaches nothing, silently authorizes nothing + [InlineData("secrets/*:view")] // subtree over an unregistered branch + [InlineData("workflows*:delete")] // embedded wildcard: parses, but the matcher never satisfies it + [InlineData("work*/foo/*:view")] + [InlineData("workflows/definitions:frobnicate")] + [InlineData("workflows/*:frobnicate")] // reaches 'workflows/definitions', which supports no such verb + public async Task WarnsAboutAPermissionThatDoesNotResolve(string permission) + { + await StartAsync(permission); + + var warning = _logger.Entries.First(x => x.Level == LogLevel.Warning); + Assert.Contains("editors", warning.Message); + Assert.Contains(permission, warning.Message); + } + + private async Task StartAsync(string permission) + { + var role = new Role { Id = "role-1", Name = "editors", Permissions = [permission] }; + var registry = new DefaultPermissionDescriptorRegistry([new StubDescriptorProvider()]); + + var services = new ServiceCollection() + .AddSingleton(new StubRoleProvider(role)) + .AddSingleton(registry) + .BuildServiceProvider(); + + var validator = new StoredPermissionValidator(services.GetRequiredService(), _logger); + await validator.StartAsync(CancellationToken.None); + } + + private sealed class StubDescriptorProvider : IPermissionDescriptorProvider + { + public IEnumerable GetDescriptors() => + [new("workflows/definitions", ["view"], "Workflow definitions", "Workflow definitions.", "Workflows")]; + } + + private sealed class StubRoleProvider(params Role[] roles) : IRoleProvider + { + public ValueTask> FindManyAsync(RoleFilter filter, CancellationToken cancellationToken = default) => + ValueTask.FromResult>(roles); + } + + private sealed class CapturingLogger : ILogger + { + public List Entries { get; } = []; + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => + Entries.Add(new(logLevel, formatter(state, exception))); + } + + private sealed record LogEntry(LogLevel Level, string Message); + + private sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + + public void Dispose() + { + } + } +}