From 37c02c74ff901dfa3479242e378ce6a414dc5c77 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 20 May 2026 23:29:35 +0200 Subject: [PATCH 1/3] [codex] Authorize workflow imports before persistence (#7510) * Authorize workflow imports before persistence * Address import authorization review feedback * Fix import authorization test fixture data --- .../WorkflowDefinitions/Import/Endpoint.cs | 13 +-- .../ImportAuthorizationExtensions.cs | 92 +++++++++++++++ .../ImportFiles/Endpoint.cs | 53 +++++---- .../Import/ImportAuthorizationTests.cs | 108 ++++++++++++++++++ 4 files changed, 237 insertions(+), 29 deletions(-) create mode 100644 src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportAuthorizationExtensions.cs create mode 100644 test/component/Elsa.Workflows.ComponentTests/Scenarios/RestApis/Endpoints/WorkflowDefinitions/Import/ImportAuthorizationTests.cs diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Import/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Import/Endpoint.cs index 2f12131f2..2d3eec31f 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Import/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Import/Endpoint.cs @@ -1,6 +1,4 @@ using Elsa.Abstractions; -using Elsa.Workflows.Api.Constants; -using Elsa.Workflows.Api.Requirements; using Elsa.Workflows.Api.Security; using Elsa.Workflows.Management; using Elsa.Workflows.Management.Models; @@ -15,6 +13,7 @@ namespace Elsa.Workflows.Api.Endpoints.WorkflowDefinitions.Import; [PublicAPI] internal class Import : ElsaEndpoint { + private readonly IWorkflowDefinitionStore _workflowDefinitionStore; private readonly IWorkflowDefinitionImporter _workflowDefinitionImporter; private readonly IWorkflowDefinitionLinker _linker; private readonly IAuthorizationService _authorizationService; @@ -22,11 +21,13 @@ internal class Import : ElsaEndpoint /// public Import( + IWorkflowDefinitionStore workflowDefinitionStore, IWorkflowDefinitionImporter workflowDefinitionImporter, IWorkflowDefinitionLinker linker, IAuthorizationService authorizationService, PythonWorkflowDefinitionAuthorizationService pythonAuthorizationService) { + _workflowDefinitionStore = workflowDefinitionStore; _workflowDefinitionImporter = workflowDefinitionImporter; _linker = linker; _authorizationService = authorizationService; @@ -54,10 +55,7 @@ internal class Import : ElsaEndpoint return; } - var result = await ImportSingleWorkflowDefinitionAsync(model, cancellationToken); - var definition = result.WorkflowDefinition; - - var authorizationResult = await _authorizationService.AuthorizeAsync(User, new NotReadOnlyResource(definition), AuthorizationPolicies.NotReadOnlyPolicy); + var authorizationResult = await _authorizationService.AuthorizeWorkflowDefinitionImportAsync(User, _workflowDefinitionStore, model, cancellationToken); if (!authorizationResult.Succeeded) { @@ -65,6 +63,8 @@ internal class Import : ElsaEndpoint return; } + var result = await ImportSingleWorkflowDefinitionAsync(model, cancellationToken); + var definition = result.WorkflowDefinition; var updatedModel = await _linker.MapAsync(definition, cancellationToken); if (result.Succeeded) @@ -98,5 +98,4 @@ internal class Import : ElsaEndpoint return result; } - } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportAuthorizationExtensions.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportAuthorizationExtensions.cs new file mode 100644 index 000000000..bbe3cb9dc --- /dev/null +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportAuthorizationExtensions.cs @@ -0,0 +1,92 @@ +using System.Security.Claims; +using Elsa.Common.Models; +using Elsa.Workflows.Api.Constants; +using Elsa.Workflows.Api.Requirements; +using Elsa.Workflows.Management; +using Elsa.Workflows.Management.Entities; +using Elsa.Workflows.Management.Filters; +using Elsa.Workflows.Management.Models; +using Microsoft.AspNetCore.Authorization; + +namespace Elsa.Workflows.Api.Endpoints.WorkflowDefinitions; + +internal static class ImportAuthorizationExtensions +{ + public static async Task AuthorizeWorkflowDefinitionImportAsync( + this IAuthorizationService authorizationService, + ClaimsPrincipal user, + IWorkflowDefinitionStore workflowDefinitionStore, + WorkflowDefinitionModel model, + CancellationToken cancellationToken) + { + var definition = await FindExistingDefinitionAsync(workflowDefinitionStore, model.DefinitionId, cancellationToken); + return await authorizationService.AuthorizeAsync(user, new NotReadOnlyResource(definition), AuthorizationPolicies.NotReadOnlyPolicy); + } + + public static async Task AuthorizeWorkflowDefinitionImportsAsync( + this IAuthorizationService authorizationService, + ClaimsPrincipal user, + IWorkflowDefinitionStore workflowDefinitionStore, + IEnumerable models, + CancellationToken cancellationToken) + { + var modelList = models.ToList(); + + if (modelList.Count == 0) + return await authorizationService.AuthorizeAsync(user, new NotReadOnlyResource(), AuthorizationPolicies.NotReadOnlyPolicy); + + var definitions = await FindExistingDefinitionsAsync(workflowDefinitionStore, modelList, cancellationToken); + + foreach (var model in modelList) + { + definitions.TryGetValue(model.DefinitionId ?? string.Empty, out var definition); + var authorizationResult = await authorizationService.AuthorizeAsync(user, new NotReadOnlyResource(definition), AuthorizationPolicies.NotReadOnlyPolicy); + + if (!authorizationResult.Succeeded) + return authorizationResult; + } + + return AuthorizationResult.Success(); + } + + private static async Task FindExistingDefinitionAsync( + IWorkflowDefinitionStore workflowDefinitionStore, + string? definitionId, + CancellationToken cancellationToken) + { + if (string.IsNullOrWhiteSpace(definitionId)) + return null; + + return await workflowDefinitionStore.FindAsync(new WorkflowDefinitionFilter + { + DefinitionId = definitionId, + VersionOptions = VersionOptions.Latest + }, cancellationToken); + } + + private static async Task> FindExistingDefinitionsAsync( + IWorkflowDefinitionStore workflowDefinitionStore, + IEnumerable models, + CancellationToken cancellationToken) + { + var definitionIds = models + .Select(x => x.DefinitionId) + .Where(x => !string.IsNullOrWhiteSpace(x)) + .Select(x => x!) + .Distinct(StringComparer.Ordinal) + .ToList(); + + if (definitionIds.Count == 0) + return new Dictionary(); + + var definitions = await workflowDefinitionStore.FindManyAsync(new WorkflowDefinitionFilter + { + DefinitionIds = definitionIds, + VersionOptions = VersionOptions.Latest + }, cancellationToken); + + return definitions + .GroupBy(x => x.DefinitionId, StringComparer.Ordinal) + .ToDictionary(x => x.Key, x => x.First(), StringComparer.Ordinal); + } +} diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportFiles/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportFiles/Endpoint.cs index 61334a825..99bce9e04 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportFiles/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportFiles/Endpoint.cs @@ -1,9 +1,6 @@ using Elsa.Abstractions; -using Elsa.Workflows.Api.Constants; -using Elsa.Workflows.Api.Requirements; using Elsa.Workflows.Api.Security; using Elsa.Workflows.Management; -using Elsa.Workflows.Management.Mappers; using Elsa.Workflows.Management.Models; using JetBrains.Annotations; using Microsoft.AspNetCore.Authorization; @@ -17,25 +14,22 @@ namespace Elsa.Workflows.Api.Endpoints.WorkflowDefinitions.ImportFiles; [PublicAPI] internal class ImportFiles : ElsaEndpoint { - private readonly IWorkflowDefinitionService _workflowDefinitionService; + private readonly IWorkflowDefinitionStore _workflowDefinitionStore; private readonly IWorkflowDefinitionImporter _workflowDefinitionImporter; - private readonly WorkflowDefinitionMapper _workflowDefinitionMapper; private readonly IApiSerializer _apiSerializer; private readonly IAuthorizationService _authorizationService; private readonly PythonWorkflowDefinitionAuthorizationService _pythonAuthorizationService; /// public ImportFiles( - IWorkflowDefinitionService workflowDefinitionService, + IWorkflowDefinitionStore workflowDefinitionStore, IWorkflowDefinitionImporter workflowDefinitionImporter, - WorkflowDefinitionMapper workflowDefinitionMapper, IApiSerializer apiSerializer, IAuthorizationService authorizationService, PythonWorkflowDefinitionAuthorizationService pythonAuthorizationService) { - _workflowDefinitionService = workflowDefinitionService; + _workflowDefinitionStore = workflowDefinitionStore; _workflowDefinitionImporter = workflowDefinitionImporter; - _workflowDefinitionMapper = workflowDefinitionMapper; _apiSerializer = apiSerializer; _authorizationService = authorizationService; _pythonAuthorizationService = pythonAuthorizationService; @@ -52,17 +46,21 @@ internal class ImportFiles : ElsaEndpoint /// public override async Task HandleAsync(WorkflowDefinitionModel model, CancellationToken cancellationToken) { - var authorizationResult = await _authorizationService.AuthorizeAsync(User, new NotReadOnlyResource(), AuthorizationPolicies.NotReadOnlyPolicy); - - if (!authorizationResult.Succeeded) - { - await Send.ForbiddenAsync(cancellationToken); - return; - } - if (Files.Any()) { - var count = await ImportFilesAsync(Files, cancellationToken); + var models = await ReadWorkflowDefinitionModelsAsync(Files, cancellationToken); + if (!await AuthorizePythonUsageAsync(models, cancellationToken)) + return; + + var authorizationResult = await _authorizationService.AuthorizeWorkflowDefinitionImportsAsync(User, _workflowDefinitionStore, models, cancellationToken); + + if (!authorizationResult.Succeeded) + { + await Send.ForbiddenAsync(cancellationToken); + return; + } + + var count = await ImportWorkflowDefinitionsAsync(models, cancellationToken); if (!ValidationFailed && !HttpContext.Response.HasStarted) await Send.OkAsync(new { Count = count }, cancellationToken); @@ -72,28 +70,39 @@ internal class ImportFiles : ElsaEndpoint await Send.ErrorsAsync(400, cancellationToken); } - private async Task ImportFilesAsync(IFormFileCollection files, CancellationToken cancellationToken) + private async Task> ReadWorkflowDefinitionModelsAsync(IFormFileCollection files, CancellationToken cancellationToken) { var models = await WorkflowDefinitionImportFileReader.ReadAsync(files, _apiSerializer, () => HttpContext.Response.HasStarted, cancellationToken); + return models.ToList(); + } + private async Task AuthorizePythonUsageAsync(IEnumerable models, CancellationToken cancellationToken) + { foreach (var model in models) { var pythonAuthorizationResult = await _pythonAuthorizationService.AuthorizeAsync(model, User, cancellationToken); if (pythonAuthorizationResult != PythonWorkflowDefinitionAuthorizationResult.Allowed) { await PythonWorkflowDefinitionAuthorizationFailure.SendAsync(pythonAuthorizationResult, Send.ForbiddenAsync, message => AddError(message), Send.ErrorsAsync, cancellationToken); - return 0; + return false; } } + return true; + } + + private async Task ImportWorkflowDefinitionsAsync(IEnumerable models, CancellationToken cancellationToken) + { var count = 0; foreach (var model in models) { if (HttpContext.Response.HasStarted) return count; - await ImportSingleWorkflowDefinitionAsync(model, cancellationToken); - count++; + var result = await ImportSingleWorkflowDefinitionAsync(model, cancellationToken); + + if (result.Succeeded) + count++; } return count; diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/RestApis/Endpoints/WorkflowDefinitions/Import/ImportAuthorizationTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/RestApis/Endpoints/WorkflowDefinitions/Import/ImportAuthorizationTests.cs new file mode 100644 index 000000000..611b389bd --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/RestApis/Endpoints/WorkflowDefinitions/Import/ImportAuthorizationTests.cs @@ -0,0 +1,108 @@ +using System.Net; +using System.Text; +using System.Text.Json; +using Elsa.Api.Client.Resources.WorkflowDefinitions.Contracts; +using Elsa.Api.Client.Resources.WorkflowDefinitions.Models; +using Elsa.Workflows; +using Elsa.Workflows.Activities; +using Elsa.Workflows.ComponentTests.Abstractions; +using Elsa.Workflows.ComponentTests.Fixtures; +using Elsa.Workflows.Management; +using Elsa.Workflows.Management.Filters; +using Elsa.Workflows.Management.Materializers; +using Microsoft.Extensions.DependencyInjection; +using Refit; +using WorkflowDefinitionEntity = Elsa.Workflows.Management.Entities.WorkflowDefinition; + +namespace Elsa.Workflows.ComponentTests.Scenarios.RestApis.Endpoints.WorkflowDefinitions.Import; + +public class ImportAuthorizationTests : AppComponentTest +{ + private readonly IWorkflowDefinitionStore _store; + private readonly IActivitySerializer _activitySerializer; + private readonly IWorkflowDefinitionsApi _client; + + public ImportAuthorizationTests(App app) : base(app) + { + _store = Scope.ServiceProvider.GetRequiredService(); + _activitySerializer = Scope.ServiceProvider.GetRequiredService(); + _client = WorkflowServer.CreateApiClient(); + } + + [Fact] + public async Task ImportExistingReadOnlyDefinition_ShouldReturnForbiddenAndLeaveStorageUnchanged() + { + var definitionId = $"readonly-import-{Guid.NewGuid():N}"; + await SaveDefinitionAsync(definitionId, "Original", isReadonly: true); + + var exception = await Assert.ThrowsAsync(() => _client.ImportAsync(CreateImportModel(definitionId, "Updated"))); + + Assert.Equal(HttpStatusCode.Forbidden, exception.StatusCode); + await AssertDefinitionUnchangedAsync(definitionId, "Original", isReadonly: true); + } + + [Fact] + public async Task ImportFilesWithReadOnlyTarget_ShouldReturnForbiddenAndLeaveStorageUnchanged() + { + var writableDefinitionId = $"writable-import-files-{Guid.NewGuid():N}"; + var readOnlyDefinitionId = $"readonly-import-files-{Guid.NewGuid():N}"; + await SaveDefinitionAsync(writableDefinitionId, "Writable Original"); + await SaveDefinitionAsync(readOnlyDefinitionId, "ReadOnly Original", isReadonly: true); + + await using var writableStream = CreateImportStream(writableDefinitionId, "Writable Updated"); + await using var readOnlyStream = CreateImportStream(readOnlyDefinitionId, "ReadOnly Updated"); + var files = new List + { + new(writableStream, "writable.json", "application/json"), + new(readOnlyStream, "readonly.json", "application/json") + }; + + var exception = await Assert.ThrowsAsync(() => _client.ImportFilesAsync(files)); + + Assert.Equal(HttpStatusCode.Forbidden, exception.StatusCode); + await AssertDefinitionUnchangedAsync(writableDefinitionId, "Writable Original"); + await AssertDefinitionUnchangedAsync(readOnlyDefinitionId, "ReadOnly Original", isReadonly: true); + } + + private async Task SaveDefinitionAsync(string definitionId, string name, bool isReadonly = false) + { + await _store.SaveAsync(new WorkflowDefinitionEntity + { + Id = Guid.NewGuid().ToString("N"), + DefinitionId = definitionId, + Name = name, + CreatedAt = DateTimeOffset.UtcNow, + IsLatest = true, + IsReadonly = isReadonly, + MaterializerName = JsonWorkflowMaterializer.MaterializerName, + StringData = _activitySerializer.Serialize(new Sequence()) + }); + } + + private async Task AssertDefinitionUnchangedAsync(string definitionId, string expectedName, bool isReadonly = false) + { + var definitions = (await _store.FindManyAsync(new WorkflowDefinitionFilter + { + DefinitionId = definitionId + })).ToList(); + + var definition = Assert.Single(definitions); + Assert.Equal(expectedName, definition.Name); + Assert.Equal(isReadonly, definition.IsReadonly); + } + + private static WorkflowDefinitionModel CreateImportModel(string definitionId, string name) + { + return new() + { + DefinitionId = definitionId, + Name = name + }; + } + + private static MemoryStream CreateImportStream(string definitionId, string name) + { + var json = JsonSerializer.Serialize(CreateImportModel(definitionId, name), new JsonSerializerOptions(JsonSerializerDefaults.Web)); + return new(Encoding.UTF8.GetBytes(json)); + } +} From 3fc87945fd6a7fefc030c5e82373ac82744db625 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 21 May 2026 00:31:34 +0200 Subject: [PATCH 2/3] Stabilize bulk dispatch fire-and-forget component test (#7520) * Stabilize bulk dispatch fire-and-forget test * Improve bulk dispatch timeout diagnostics * Stabilize bulk dispatch polling test * Reduce bulk dispatch test polling pressure --- .../BulkDispatchWorkflowsTests.cs | 61 +++++++++++++++---- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/Composition/BulkDispatchWorkflows/BulkDispatchWorkflowsTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/Composition/BulkDispatchWorkflows/BulkDispatchWorkflowsTests.cs index 928bc317a..5f4f636b3 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/Composition/BulkDispatchWorkflows/BulkDispatchWorkflowsTests.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/Activities/Composition/BulkDispatchWorkflows/BulkDispatchWorkflowsTests.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using Elsa.Common.Models; using Elsa.Testing.Shared; using Elsa.Testing.Shared.Models; @@ -7,6 +8,8 @@ using Elsa.Workflows.ComponentTests.Abstractions; using Elsa.Workflows.ComponentTests.Fixtures; using Elsa.Workflows.ComponentTests.Scenarios.Activities.Composition.BulkDispatchWorkflows.Workflows; using Elsa.Workflows.Management; +using Elsa.Workflows.Management.Entities; +using Elsa.Workflows.Management.Filters; using Elsa.Workflows.Models; using Elsa.Workflows.State; using Microsoft.Extensions.DependencyInjection; @@ -16,11 +19,17 @@ namespace Elsa.Workflows.ComponentTests.Scenarios.Activities.Composition.BulkDis public class BulkDispatchWorkflowsTests : AppComponentTest { private const int ChildWorkflowTimeoutSeconds = 30; + private const int InitialChildWorkflowPollingIntervalMilliseconds = 250; + private const int MaxChildWorkflowPollingIntervalMilliseconds = 1000; + // The runtime persists this marker under the activity property name; resume handlers read the same key from WorkflowState.Properties. + private const string WaitForCompletionPropertyName = nameof(Elsa.Workflows.Runtime.Activities.BulkDispatchWorkflows.WaitForCompletion); private readonly AsyncWorkflowRunner _workflowRunner; + private readonly IWorkflowInstanceStore _workflowInstanceStore; public BulkDispatchWorkflowsTests(App app) : base(app) { _workflowRunner = Scope.ServiceProvider.GetRequiredService(); + _workflowInstanceStore = Scope.ServiceProvider.GetRequiredService(); } [Fact(DisplayName = "BulkDispatchWorkflows should wait for all child workflows to complete")] @@ -37,21 +46,19 @@ public class BulkDispatchWorkflowsTests : AppComponentTest { var expectedChildCount = 3; - // Run the main workflow and wait for child workflows to complete - var (result, completedChildWorkflows) = await RunWorkflowAndWaitForChildWorkflowsAsync( - BulkDispatchFireAndForgetWorkflow.DefinitionId, + var result = await RunWorkflowAsync(BulkDispatchFireAndForgetWorkflow.DefinitionId); + + AssertWorkflowFinished(result); + var childWorkflowInstances = await WaitForChildWorkflowInstancesAsync( + result.WorkflowExecutionContext.Id, SlowBulkChildWorkflow.DefinitionId, expectedChildCount); - AssertWorkflowFinished(result); - var mainWorkflowCompletedAt = result.WorkflowExecutionContext.UpdatedAt; - - // Assert that all child workflows completed after the main workflow - Assert.Equal(expectedChildCount, completedChildWorkflows.Count); - foreach (var childContext in completedChildWorkflows) + Assert.Equal(expectedChildCount, childWorkflowInstances.Count); + foreach (var childWorkflowInstance in childWorkflowInstances) { - Assert.True(childContext.UpdatedAt > mainWorkflowCompletedAt, - $"Child workflow should complete after main workflow. Main: {mainWorkflowCompletedAt}, Child: {childContext.UpdatedAt}"); + Assert.Equal(result.WorkflowExecutionContext.Id, childWorkflowInstance.ParentWorkflowInstanceId); + Assert.False(childWorkflowInstance.WorkflowState.Properties.ContainsKey(WaitForCompletionPropertyName)); } } @@ -176,4 +183,36 @@ public class BulkDispatchWorkflowsTests : AppComponentTest workflowEvents.WorkflowStateCommitted -= OnWorkflowStateCommitted; } } + + private async Task> WaitForChildWorkflowInstancesAsync( + string parentWorkflowInstanceId, + string childWorkflowDefinitionId, + int expectedChildCount, + CancellationToken cancellationToken = default) + { + var timeout = TimeSpan.FromSeconds(ChildWorkflowTimeoutSeconds); + var pollingInterval = TimeSpan.FromMilliseconds(InitialChildWorkflowPollingIntervalMilliseconds); + var maxPollingInterval = TimeSpan.FromMilliseconds(MaxChildWorkflowPollingIntervalMilliseconds); + var stopwatch = Stopwatch.StartNew(); + var filter = new WorkflowInstanceFilter + { + DefinitionId = childWorkflowDefinitionId, + ParentWorkflowInstanceIds = [parentWorkflowInstanceId] + }; + var actualChildCount = 0; + + while (stopwatch.Elapsed < timeout) + { + var instances = (await _workflowInstanceStore.FindManyAsync(filter, cancellationToken)).ToList(); + actualChildCount = instances.Count; + + if (instances.Count >= expectedChildCount) + return instances; + + await Task.Delay(pollingInterval, cancellationToken); + pollingInterval = TimeSpan.FromMilliseconds(Math.Min(pollingInterval.TotalMilliseconds * 2, maxPollingInterval.TotalMilliseconds)); + } + + throw new TimeoutException($"Expected {expectedChildCount} child workflow instances of definition '{childWorkflowDefinitionId}' for parent '{parentWorkflowInstanceId}', but found {actualChildCount}."); + } } From d23e61e9be472269b8e40e2790ae9a4b2446f7fc Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 21 May 2026 00:40:57 +0200 Subject: [PATCH 3/3] [codex] Require opt-in for localhost authorization grants (#7498) * Require opt-in for localhost auth grant * Preserve custom authorization configuration * Address localhost auth review feedback * Address localhost bootstrap review comments --- .../LocalHostPermissionRequirementOptions.cs | 12 ++ .../LocalHostPermissionRequirement.cs | 52 ++++++-- .../Features/DefaultAuthenticationFeature.cs | 63 ++++++++- src/modules/Elsa.Identity/README.md | 1 + .../DefaultAuthenticationFeature.cs | 12 +- .../DefaultAuthenticationFeatureTests.cs | 85 ++++++++++++ ...alHostPermissionRequirementHandlerTests.cs | 124 ++++++++++++++++++ 7 files changed, 330 insertions(+), 19 deletions(-) create mode 100644 src/common/Elsa.Api.Common/Options/LocalHostPermissionRequirementOptions.cs create mode 100644 test/unit/Elsa.Identity.UnitTests/Features/DefaultAuthenticationFeatureTests.cs create mode 100644 test/unit/Elsa.Identity.UnitTests/Requirements/LocalHostPermissionRequirementHandlerTests.cs diff --git a/src/common/Elsa.Api.Common/Options/LocalHostPermissionRequirementOptions.cs b/src/common/Elsa.Api.Common/Options/LocalHostPermissionRequirementOptions.cs new file mode 100644 index 000000000..0dda8b1cc --- /dev/null +++ b/src/common/Elsa.Api.Common/Options/LocalHostPermissionRequirementOptions.cs @@ -0,0 +1,12 @@ +namespace Elsa.Options; + +/// +/// Options for the localhost permission requirement. +/// +public class LocalHostPermissionRequirementOptions +{ + /// + /// Gets or sets whether localhost requests may satisfy the security-root permission requirement without other credentials. + /// + public bool EnableLocalHostPermissionGrant { get; set; } +} diff --git a/src/common/Elsa.Api.Common/Requirements/LocalHostPermissionRequirement.cs b/src/common/Elsa.Api.Common/Requirements/LocalHostPermissionRequirement.cs index 7cf7f9906..883662d0e 100644 --- a/src/common/Elsa.Api.Common/Requirements/LocalHostPermissionRequirement.cs +++ b/src/common/Elsa.Api.Common/Requirements/LocalHostPermissionRequirement.cs @@ -1,14 +1,16 @@ using System.Security.Claims; using Elsa.Extensions; +using Elsa.Options; using JetBrains.Annotations; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Options; namespace Elsa.Requirements; /// -/// Add the "create:application" permission to the current user if the request is local. +/// Adds security-root bootstrap permissions to the current user when explicit localhost permission grants are enabled and the request is local. /// public class LocalHostPermissionRequirement : IAuthorizationRequirement { @@ -18,32 +20,58 @@ public class LocalHostPermissionRequirement : IAuthorizationRequirement [PublicAPI] public class LocalHostPermissionRequirementHandler : AuthorizationHandler { + private static readonly string[] BootstrapPermissions = + [ + "create:application", + "create:user", + "create:role" + ]; + private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IOptions _options; /// - public LocalHostPermissionRequirementHandler(IHttpContextAccessor httpContextAccessor) + public LocalHostPermissionRequirementHandler(IHttpContextAccessor httpContextAccessor) : this( + httpContextAccessor, + Microsoft.Extensions.Options.Options.Create(new LocalHostPermissionRequirementOptions())) + { + } + + /// + public LocalHostPermissionRequirementHandler(IHttpContextAccessor httpContextAccessor, IOptions options) { _httpContextAccessor = httpContextAccessor; + _options = options; } /// protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, LocalHostPermissionRequirement requirement) { - if (_httpContextAccessor.HttpContext?.Request.IsLocal() == false) + if (!_options.Value.EnableLocalHostPermissionGrant) return Task.CompletedTask; - - var currentIdentity = context.User.Identity; - if (currentIdentity?.IsAuthenticated == false) + if (_httpContextAccessor.HttpContext?.Request.IsLocal() != true) + return Task.CompletedTask; + + if (context.User.Identities.Any(x => x.IsAuthenticated)) { - var identity = new ClaimsIdentity(JwtBearerDefaults.AuthenticationScheme); - identity.AddClaim(new Claim("permissions", "create:application")); - identity.AddClaim(new Claim("permissions", "create:user")); - identity.AddClaim(new Claim("permissions", "create:role")); - context.User.AddIdentity(identity); + if (HasBootstrapPermissions(context.User)) + context.Succeed(requirement); + + return Task.CompletedTask; } + var identity = new ClaimsIdentity(JwtBearerDefaults.AuthenticationScheme); + identity.AddClaims(BootstrapPermissions.Select(permission => new Claim(PermissionNames.ClaimType, permission))); + context.User.AddIdentity(identity); + context.Succeed(requirement); return Task.CompletedTask; } -} \ No newline at end of file + + private static bool HasBootstrapPermissions(ClaimsPrincipal user) + { + var permissions = user.FindAll(PermissionNames.ClaimType).Select(x => x.Value).ToHashSet(StringComparer.Ordinal); + return permissions.Contains(PermissionNames.All) || BootstrapPermissions.All(permissions.Contains); + } +} diff --git a/src/modules/Elsa.Identity/Features/DefaultAuthenticationFeature.cs b/src/modules/Elsa.Identity/Features/DefaultAuthenticationFeature.cs index 6aadbafd8..0f19a1413 100644 --- a/src/modules/Elsa.Identity/Features/DefaultAuthenticationFeature.cs +++ b/src/modules/Elsa.Identity/Features/DefaultAuthenticationFeature.cs @@ -6,6 +6,7 @@ using Elsa.Features.Services; using Elsa.Identity.Constants; using Elsa.Identity.Options; using Elsa.Identity.Providers; +using Elsa.Options; using Elsa.Requirements; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.JwtBearer; @@ -22,17 +23,32 @@ public class DefaultAuthenticationFeature : FeatureBase { private const string MultiScheme = "Jwt-or-ApiKey"; private Func _configureApiKeyAuthorization = builder => builder.AddApiKeyInAuthorizationHeader(); + private Action? _configureAuthorizationOptions; /// public DefaultAuthenticationFeature(IModule module) : base(module) { + ConfigureAuthorizationOptions = ConfigureDefaultSecurityRootPolicy; } /// /// Gets or sets the . /// public Type ApiKeyProviderType { get; set; } = typeof(DefaultApiKeyProvider); - public Action ConfigureAuthorizationOptions { get; set; } = options => options.AddPolicy(IdentityPolicyNames.SecurityRoot, policy => policy.AddRequirements(new LocalHostPermissionRequirement())); + + /// + /// Gets or sets the authorization options configuration. + /// + public Action ConfigureAuthorizationOptions + { + get => _configureAuthorizationOptions ?? ConfigureDefaultSecurityRootPolicy; + set => _configureAuthorizationOptions = value ?? ConfigureDefaultSecurityRootPolicy; + } + + /// + /// Gets or sets whether localhost requests may satisfy the security-root permission requirement without other credentials. + /// + public bool EnableLocalHostPermissionGrant { get; set; } /// /// Configures the API key provider type. @@ -79,23 +95,40 @@ public class DefaultAuthenticationFeature : FeatureBase /// /// The current . public DefaultAuthenticationFeature UseDevelopmentAdminApiKey() => UseAdminApiKey(AdminApiKeyProvider.DevelopmentApiKey); - + /// - /// Disables the local host requirement for the security root policy. - /// This is useful when privileged identity bootstrap is handled through features such as . + /// Enables the legacy localhost permission grant for the security root policy. /// - public DefaultAuthenticationFeature DisableLocalHostRequirement() + public DefaultAuthenticationFeature EnableLocalHostPermissionGrantForSecurityRoot() { - ConfigureAuthorizationOptions = options => options.AddPolicy(IdentityPolicyNames.SecurityRoot, policy => policy.RequireAuthenticatedUser()); + EnableLocalHostPermissionGrant = true; return this; } + /// + /// Disables the localhost permission grant for the security root policy. + /// This is useful when privileged identity bootstrap is handled through features such as . + /// + public DefaultAuthenticationFeature DisableLocalHostPermissionGrantForSecurityRoot() + { + EnableLocalHostPermissionGrant = false; + return this; + } + + /// + /// Disables the legacy localhost permission grant for the security root policy. + /// This is useful when privileged identity bootstrap is handled through features such as . + /// + [Obsolete("Use DisableLocalHostPermissionGrantForSecurityRoot instead.")] + public DefaultAuthenticationFeature DisableLocalHostRequirement() => DisableLocalHostPermissionGrantForSecurityRoot(); + /// public override void Apply() { Services.ConfigureOptions(); Services.Configure(_ => { }); Services.AddIdentityTokenOptionsValidation(); + Services.Configure(options => options.EnableLocalHostPermissionGrant = EnableLocalHostPermissionGrant); var authBuilder = Services .AddAuthentication(MultiScheme) @@ -119,4 +152,22 @@ public class DefaultAuthenticationFeature : FeatureBase Services.AddScoped(sp => (IApiKeyProvider)sp.GetRequiredService(ApiKeyProviderType)); Services.AddAuthorization(ConfigureAuthorizationOptions); } + + private static void ConfigureAuthenticatedSecurityRootPolicy(AuthorizationOptions options) + { + options.AddPolicy(IdentityPolicyNames.SecurityRoot, policy => policy.RequireAuthenticatedUser()); + } + + private void ConfigureDefaultSecurityRootPolicy(AuthorizationOptions options) + { + if (EnableLocalHostPermissionGrant) + ConfigureLocalHostSecurityRootPolicy(options); + else + ConfigureAuthenticatedSecurityRootPolicy(options); + } + + private static void ConfigureLocalHostSecurityRootPolicy(AuthorizationOptions options) + { + options.AddPolicy(IdentityPolicyNames.SecurityRoot, policy => policy.AddRequirements(new LocalHostPermissionRequirement())); + } } diff --git a/src/modules/Elsa.Identity/README.md b/src/modules/Elsa.Identity/README.md index 5f1c793ff..481894504 100644 --- a/src/modules/Elsa.Identity/README.md +++ b/src/modules/Elsa.Identity/README.md @@ -83,3 +83,4 @@ identity.UseDefaultAdmin("admin", "REPLACE_WITH_SECURE_BOOTSTRAP_PASSWORD", "adm - Do not keep development defaults in production. - Prefer environment variables or a secret manager for admin credentials. - After first bootstrap, rotate credentials according to your security policy. +- Localhost requests no longer satisfy `SecurityRoot` by default. Legacy localhost bootstrap requires an explicit opt-in: call `EnableLocalHostPermissionGrantForSecurityRoot()` in code-first configuration or set `EnableLocalHostPermissionGrant` on the shell `DefaultAuthentication` feature; prefer `DefaultAdminUser` instead. diff --git a/src/modules/Elsa.Identity/ShellFeatures/DefaultAuthenticationFeature.cs b/src/modules/Elsa.Identity/ShellFeatures/DefaultAuthenticationFeature.cs index 51f892a61..57d8ed1c1 100644 --- a/src/modules/Elsa.Identity/ShellFeatures/DefaultAuthenticationFeature.cs +++ b/src/modules/Elsa.Identity/ShellFeatures/DefaultAuthenticationFeature.cs @@ -4,6 +4,7 @@ using Elsa.Extensions; using Elsa.Identity.Constants; using Elsa.Identity.Options; using Elsa.Identity.Providers; +using Elsa.Options; using Elsa.PackageManifest.Generator.Hints; using Elsa.Requirements; using JetBrains.Annotations; @@ -53,6 +54,11 @@ public class DefaultAuthenticationFeature : IShellFeature RestartRequired = true)] public bool UseDevelopmentAdminApiKey { get; set; } + /// + /// Gets or sets whether localhost requests may satisfy the security-root permission requirement without other credentials. + /// + public bool EnableLocalHostPermissionGrant { get; set; } + public void ConfigureServices(IServiceCollection services) { var resolvedAdminApiKey = UseDevelopmentAdminApiKey ? AdminApiKeyProvider.DevelopmentApiKey : AdminApiKey; @@ -61,6 +67,7 @@ public class DefaultAuthenticationFeature : IShellFeature services.ConfigureOptions(); services.AddIdentityTokenOptionsValidation(); + services.Configure(options => options.EnableLocalHostPermissionGrant = EnableLocalHostPermissionGrant); services.Configure(options => { options.ApiKey = resolvedAdminApiKey; @@ -93,7 +100,10 @@ public class DefaultAuthenticationFeature : IShellFeature services.AddAuthorization(options => { - options.AddPolicy(IdentityPolicyNames.SecurityRoot, policy => policy.RequireAuthenticatedUser()); + if (EnableLocalHostPermissionGrant) + options.AddPolicy(IdentityPolicyNames.SecurityRoot, policy => policy.AddRequirements(new LocalHostPermissionRequirement())); + else + options.AddPolicy(IdentityPolicyNames.SecurityRoot, policy => policy.RequireAuthenticatedUser()); }); } } diff --git a/test/unit/Elsa.Identity.UnitTests/Features/DefaultAuthenticationFeatureTests.cs b/test/unit/Elsa.Identity.UnitTests/Features/DefaultAuthenticationFeatureTests.cs new file mode 100644 index 000000000..8568eeaef --- /dev/null +++ b/test/unit/Elsa.Identity.UnitTests/Features/DefaultAuthenticationFeatureTests.cs @@ -0,0 +1,85 @@ +using Elsa.Features.Services; +using Elsa.Identity.Features; +using Elsa.Requirements; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Authorization.Infrastructure; +using NSubstitute; + +namespace Elsa.Identity.UnitTests.Features; + +public class DefaultAuthenticationFeatureTests +{ + [Fact] + public void DefaultSecurityRootPolicyRequiresAuthenticatedUser() + { + var feature = new DefaultAuthenticationFeature(Substitute.For()); + var options = new AuthorizationOptions(); + + feature.ConfigureAuthorizationOptions(options); + + var policy = options.GetPolicy(IdentityPolicyNames.SecurityRoot); + + Assert.NotNull(policy); + Assert.Contains(policy.Requirements, requirement => requirement is DenyAnonymousAuthorizationRequirement); + Assert.DoesNotContain(policy.Requirements, requirement => requirement is LocalHostPermissionRequirement); + } + + [Fact] + public void EnableLocalHostPermissionGrantForSecurityRootConfiguresExplicitLocalhostPolicy() + { + var feature = new DefaultAuthenticationFeature(Substitute.For()); + var options = new AuthorizationOptions(); + + feature.EnableLocalHostPermissionGrantForSecurityRoot(); + feature.ConfigureAuthorizationOptions(options); + + var policy = options.GetPolicy(IdentityPolicyNames.SecurityRoot); + + Assert.True(feature.EnableLocalHostPermissionGrant); + Assert.NotNull(policy); + Assert.Contains(policy.Requirements, requirement => requirement is LocalHostPermissionRequirement); + } + + [Fact] + public void EnableLocalHostPermissionGrantDoesNotOverwriteCustomAuthorizationConfiguration() + { + var feature = new DefaultAuthenticationFeature(Substitute.For()); + feature.ConfigureAuthorizationOptions = options => options.AddPolicy("Custom", policy => policy.RequireAuthenticatedUser()); + + feature.EnableLocalHostPermissionGrantForSecurityRoot(); + var options = new AuthorizationOptions(); + feature.ConfigureAuthorizationOptions(options); + + Assert.True(feature.EnableLocalHostPermissionGrant); + Assert.NotNull(options.GetPolicy("Custom")); + Assert.Null(options.GetPolicy(IdentityPolicyNames.SecurityRoot)); + } + + [Fact] + public void NullConfigureAuthorizationOptionsFallsBackToDefaultSecurityRootPolicy() + { + var feature = new DefaultAuthenticationFeature(Substitute.For()) + { + ConfigureAuthorizationOptions = null! + }; + var options = new AuthorizationOptions(); + + feature.ConfigureAuthorizationOptions(options); + + var policy = options.GetPolicy(IdentityPolicyNames.SecurityRoot); + + Assert.NotNull(policy); + Assert.Contains(policy.Requirements, requirement => requirement is DenyAnonymousAuthorizationRequirement); + } + + [Fact] + public void DisableLocalHostPermissionGrantForSecurityRootClearsOptInFlag() + { + var feature = new DefaultAuthenticationFeature(Substitute.For()); + + feature.EnableLocalHostPermissionGrantForSecurityRoot(); + feature.DisableLocalHostPermissionGrantForSecurityRoot(); + + Assert.False(feature.EnableLocalHostPermissionGrant); + } +} diff --git a/test/unit/Elsa.Identity.UnitTests/Requirements/LocalHostPermissionRequirementHandlerTests.cs b/test/unit/Elsa.Identity.UnitTests/Requirements/LocalHostPermissionRequirementHandlerTests.cs new file mode 100644 index 000000000..970387f02 --- /dev/null +++ b/test/unit/Elsa.Identity.UnitTests/Requirements/LocalHostPermissionRequirementHandlerTests.cs @@ -0,0 +1,124 @@ +using System.Net; +using System.Security.Claims; +using Elsa; +using Elsa.Options; +using Elsa.Requirements; +using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using OptionsFactory = Microsoft.Extensions.Options.Options; + +namespace Elsa.Identity.UnitTests.Requirements; + +public class LocalHostPermissionRequirementHandlerTests +{ + [Fact] + public async Task DoesNotGrantPermissionsToLocalhostRequestsByDefault() + { + var context = await AuthorizeAsync(enableLocalHostPermissionGrant: false, isLocal: true); + + Assert.False(context.HasSucceeded); + Assert.Empty(context.User.FindAll(PermissionNames.ClaimType)); + } + + [Fact] + public async Task GrantsPermissionsToLocalhostRequestsWhenExplicitlyEnabled() + { + var context = await AuthorizeAsync(enableLocalHostPermissionGrant: true, isLocal: true); + var permissions = context.User.FindAll(PermissionNames.ClaimType).Select(x => x.Value).ToList(); + + Assert.True(context.HasSucceeded); + Assert.Contains("create:application", permissions); + Assert.Contains("create:user", permissions); + Assert.Contains("create:role", permissions); + } + + [Fact] + public async Task DoesNotGrantPermissionsToAuthenticatedLocalhostRequestsWhenExplicitlyEnabled() + { + var context = await AuthorizeAsync(enableLocalHostPermissionGrant: true, isLocal: true, isAuthenticated: true); + + Assert.False(context.HasSucceeded); + Assert.True(context.User.Identity?.IsAuthenticated); + Assert.Empty(context.User.FindAll(PermissionNames.ClaimType)); + } + + [Fact] + public async Task SucceedsForAuthenticatedLocalhostRequestsWithExistingBootstrapPermissionsWhenExplicitlyEnabled() + { + var context = await AuthorizeAsync(enableLocalHostPermissionGrant: true, isLocal: true, isAuthenticated: true, permissions: BootstrapPermissions); + var permissions = context.User.FindAll(PermissionNames.ClaimType).Select(x => x.Value).ToList(); + + Assert.True(context.HasSucceeded); + Assert.True(context.User.Identity?.IsAuthenticated); + Assert.Equal(3, permissions.Count); + Assert.Contains("create:application", permissions); + Assert.Contains("create:user", permissions); + Assert.Contains("create:role", permissions); + } + + [Fact] + public async Task DoesNotDuplicateBootstrapPermissionsAcrossMultipleEvaluations() + { + var requirement = new LocalHostPermissionRequirement(); + var user = new ClaimsPrincipal(new ClaimsIdentity()); + var authorizationContext = new AuthorizationHandlerContext(new[] { requirement }, user, null); + var handler = CreateHandler(enableLocalHostPermissionGrant: true, isLocal: true); + + await handler.HandleAsync(authorizationContext); + await handler.HandleAsync(authorizationContext); + + Assert.True(authorizationContext.HasSucceeded); + Assert.Equal(1, authorizationContext.User.Identities.Count(x => x.AuthenticationType == JwtBearerDefaults.AuthenticationScheme)); + Assert.Equal(3, authorizationContext.User.FindAll(PermissionNames.ClaimType).Count()); + } + + [Fact] + public async Task DoesNotGrantPermissionsToRemoteRequestsWhenExplicitlyEnabled() + { + var context = await AuthorizeAsync(enableLocalHostPermissionGrant: true, isLocal: false); + + Assert.False(context.HasSucceeded); + Assert.Empty(context.User.FindAll(PermissionNames.ClaimType)); + } + + private static readonly string[] BootstrapPermissions = + [ + "create:application", + "create:user", + "create:role" + ]; + + private static async Task AuthorizeAsync(bool enableLocalHostPermissionGrant, bool isLocal, bool isAuthenticated = false, params string[] permissions) + { + var requirement = new LocalHostPermissionRequirement(); + var user = new ClaimsPrincipal(new ClaimsIdentity(isAuthenticated ? "Test" : null)); + user.Identities.First().AddClaims(permissions.Select(x => new Claim(PermissionNames.ClaimType, x))); + var authorizationContext = new AuthorizationHandlerContext(new[] { requirement }, user, null); + var handler = CreateHandler(enableLocalHostPermissionGrant, isLocal); + + await handler.HandleAsync(authorizationContext); + + return authorizationContext; + } + + private static LocalHostPermissionRequirementHandler CreateHandler(bool enableLocalHostPermissionGrant, bool isLocal) + { + var httpContext = CreateHttpContext(isLocal); + var httpContextAccessor = new HttpContextAccessor { HttpContext = httpContext }; + var options = OptionsFactory.Create(new LocalHostPermissionRequirementOptions + { + EnableLocalHostPermissionGrant = enableLocalHostPermissionGrant + }); + + return new LocalHostPermissionRequirementHandler(httpContextAccessor, options); + } + + private static HttpContext CreateHttpContext(bool isLocal) + { + var httpContext = new DefaultHttpContext(); + httpContext.Connection.LocalIpAddress = IPAddress.Loopback; + httpContext.Connection.RemoteIpAddress = isLocal ? IPAddress.Loopback : IPAddress.Parse("10.0.0.1"); + return httpContext; + } +}