From 372cf33cae32a93dd835f2b0feaef7a670d40ec8 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 25 Aug 2026 22:58:16 +0200 Subject: [PATCH] refactor(auth): remove the vestigial per-author script permission plumbing (#7990) * refactor(auth): remove the vestigial per-author script permission plumbing #7975 is closed won't-do: authoring a workflow is a trusted act, and a per-author gate would not change what a script can do once it runs. The host switch stays the control, and it is per language, so an untrusted author gets a host with the switch off rather than a permission. That settles what the code was still half-carrying. WorkflowDefinitionScriptAuthorizationService took a ClaimsPrincipal it never read, and could return a MissingPermission reason nothing produced; two call sites branched on that reason to send a 403 that could not happen. The expression-descriptor endpoint kept a map from expression type to per-author permission whose values went unused even before the permissions were retired -- it only ever tested membership, and the decision was always IsBrowsable. Each of these reads as an authorization gate to anyone scanning the file, and none of them is one. The principal, the unreachable reason, and both dead branches are gone. The map becomes a set of the expression types the host can switch off, which is what it was actually being used as. Behaviour is unchanged: the only failure is a language the host disabled, which is a property of the deployment and so a 400 naming the switch, never a 403. PermissionNames loses ExecuteCSharpExpressions and ExecutePythonExpressions, which existed only for that map and the test mirroring it. Five other legacy constants there are also unreferenced but belong to other modules; they are left alone rather than swept up here. Two tests asserting the host-and-user case were exact duplicates of the host-only case once the principal stopped mattering, so they go with it. The migration guide said deployments lose per-author granularity "until #7975 lands" and advised disabling host code until then. That promise is withdrawn and replaced with the actual guidance. Closes #7975 Co-Authored-By: Claude Opus 5 * docs(wiki): drop the retired exec:* permissions from the scripting guide Review found doc/wiki/expressions-and-scripting.md still telling operators that API callers "must have the exec:csharp-expressions permission" to author, publish, dispatch or execute workflows containing C#, and the same for Python. Those permissions no longer exist, so the instruction cannot be followed and describes a gate that is not there. Both sections now say what is actually true: the host switch is the whole control, there is no per-caller permission because a workflow runs under the server's authority rather than the caller's, and an untrusted author gets a host with the switch off. The switches are noted as independent, since enabling Python while leaving C# off is a real posture. My earlier sweep searched for the issue number rather than the permission strings, which is why this file was missed. Refs #7975 Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- doc/migrations/authorization-model.md | 10 ++- doc/wiki/expressions-and-scripting.md | 4 +- src/common/Elsa.Api.Common/PermissionNames.cs | 9 --- .../ExpressionDescriptors/List/Endpoint.cs | 26 ++++--- .../BulkDispatch/Endpoint.cs | 4 +- .../BulkPublish/Endpoint.cs | 4 +- .../WorkflowDefinitions/Dispatch/Endpoint.cs | 4 +- .../Execute/WorkflowExecutionHelper.cs | 10 +-- .../WorkflowDefinitions/Import/Endpoint.cs | 4 +- .../ImportFiles/Endpoint.cs | 4 +- .../WorkflowDefinitions/Post/Endpoint.cs | 4 +- .../WorkflowDefinitions/Publish/Endpoint.cs | 4 +- ...lowDefinitionScriptAuthorizationFailure.cs | 9 +-- ...lowDefinitionScriptAuthorizationService.cs | 30 ++++---- ...finitionScriptAuthorizationServiceTests.cs | 77 ++++++------------- 15 files changed, 81 insertions(+), 122 deletions(-) diff --git a/doc/migrations/authorization-model.md b/doc/migrations/authorization-model.md index ff7a5829a..304d2a293 100644 --- a/doc/migrations/authorization-model.md +++ b/doc/migrations/authorization-model.md @@ -39,7 +39,11 @@ Today they are literal claim values, not patterns: `read:*` authorizes only the - Where host code is **disabled**, nothing changes. - Where host code is **enabled**, any author who may write workflow definitions may use C# and Python, and the editor offers those expression types to every such author. -Deployments that enabled host code while trusting only *some* authors lose that granularity until [#7975](https://github.com/elsa-workflows/elsa-core/issues/7975) lands. If that matters to you, disable host code until then. +Deployments that enabled host code while trusting only *some* authors lose that granularity, and this is now the +intended posture rather than a gap awaiting a fix: [#7975](https://github.com/elsa-workflows/elsa-core/issues/7975) +was closed as won't-do. Authoring a workflow is a trusted act, and a per-author gate would not have changed what a +script can do once it runs. If some of your authors are not trusted with host code, give them a host with the +switch off; the switch is per language, so C# and Python can be decided separately. ## Revocation @@ -146,8 +150,8 @@ If you have duplicate names across tenants today, they were impossible to create | `read:output-converters` | `workflows/descriptors/output-converters:view` | | `read:workflow-activation-strategies` | `workflows/descriptors/activation-strategies:view` | | `read:javascript-type-definitions` | `workflows/scripting/javascript:view` | -| `exec:csharp-expressions` | *removed* — see #7975 | -| `exec:python-expressions` | *removed* — see #7975 | +| `exec:csharp-expressions` | *removed* — the host switch is the control; see #7975 | +| `exec:python-expressions` | *removed* — the host switch is the control; see #7975 | | `read:user` | `identity/users:view` | | `create:user` | `identity/users:create` | | `update:user` | `identity/users:update` | diff --git a/doc/wiki/expressions-and-scripting.md b/doc/wiki/expressions-and-scripting.md index f49072967..32060eb45 100644 --- a/doc/wiki/expressions-and-scripting.md +++ b/doc/wiki/expressions-and-scripting.md @@ -61,13 +61,13 @@ elsa.UseCSharp(options => }); ``` -Roslyn C# scripting is privileged host-code execution, not a sandbox. Hosts must explicitly set `CSharpOptions.AllowHostCodeExecution` to `true` before C# expressions or `RunCSharp` can be authored or executed. API callers that author, publish, dispatch, or directly execute workflows containing C# must have the `exec:csharp-expressions` permission. +Roslyn C# scripting is privileged host-code execution, not a sandbox. Hosts must explicitly set `CSharpOptions.AllowHostCodeExecution` to `true` before C# expressions or `RunCSharp` can be authored or executed. That switch is the whole control: there is no per-caller permission, because a workflow runs under the server's authority rather than the caller's, so gating the caller never constrained what a script could do. Any author who may write workflow definitions may use C# where the switch is on. If only some of your authors are trusted with host code, give the others a host with the switch off — see [#7975](https://github.com/elsa-workflows/elsa-core/issues/7975). ## Python [PythonFeature](../../src/modules/Elsa.Expressions.Python/Features/PythonFeature.cs) registers pythonnet-based evaluation and configures `PythonGlobalInterpreterManager` as a hosted service. Python.NET execution is privileged host-code execution, not a sandbox. Python code can access host process capabilities through pythonnet and must only be enabled for trusted workflow authors. -Hosts must explicitly set `PythonOptions.AllowHostCodeExecution` to `true` before Python expressions or `RunPython` can be authored or executed. API callers that author, publish, dispatch, or directly execute workflows containing Python must have the `exec:python-expressions` permission. Hosts must also configure the Python DLL path or set `PYTHONNET_PYDLL`. +Hosts must explicitly set `PythonOptions.AllowHostCodeExecution` to `true` before Python expressions or `RunPython` can be authored or executed. As with C#, that switch is the whole control and there is no per-caller permission; the switches are independent, so Python can be enabled while C# stays off. Hosts must also configure the Python DLL path or set `PYTHONNET_PYDLL`. The reference server binds `Scripting:Python` configuration in [Program.cs](../../src/apps/Elsa.Server.Web/Program.cs). diff --git a/src/common/Elsa.Api.Common/PermissionNames.cs b/src/common/Elsa.Api.Common/PermissionNames.cs index 951010aa9..09021a161 100644 --- a/src/common/Elsa.Api.Common/PermissionNames.cs +++ b/src/common/Elsa.Api.Common/PermissionNames.cs @@ -5,15 +5,6 @@ public static class PermissionNames public const string All = "*"; public const string ClaimType = "permissions"; - /// - /// Permission required to author or execute C# workflow expressions. - /// - public const string ExecuteCSharpExpressions = "exec:csharp-expressions"; - - /// - /// Permission required to author or execute Python.NET workflow expressions. - /// - public const string ExecutePythonExpressions = "exec:python-expressions"; /// /// Permission required to pause, resume, or force-drain the workflow runtime. diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/Scripting/ExpressionDescriptors/List/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/Scripting/ExpressionDescriptors/List/Endpoint.cs index 07a7eadbd..ffd032d00 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/Scripting/ExpressionDescriptors/List/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/Scripting/ExpressionDescriptors/List/Endpoint.cs @@ -14,11 +14,18 @@ namespace Elsa.Workflows.Api.Endpoints.Scripting.ExpressionDescriptors.List; [UsedImplicitly] internal class List(IExpressionDescriptorRegistry expressionDescriptorRegistry) : ElsaEndpointWithoutRequest> { - private static readonly IReadOnlyDictionary PrivilegedExpressionPermissions = new Dictionary - { - ["CSharp"] = PermissionNames.ExecuteCSharpExpressions, - ["Python"] = PermissionNames.ExecutePythonExpressions - }.ToFrozenDictionary(StringComparer.Ordinal); + /// + /// The expression types the host can switch off, which are omitted entirely rather than listed as + /// non-browsable so a disabled language does not appear in the editor at all. Every other type is listed + /// and carries IsBrowsable for the client to act on. + /// + /// + /// This was a map from expression type to a per-author permission. The permission was only ever read to + /// test membership -- the value went unused even before it was retired -- and the decision has always + /// been the host switch, surfaced as IsBrowsable. A set says that without implying a check that + /// does not happen. Per-author script trust was considered and declined in #7975: the host switch is the control. + /// + private static readonly FrozenSet HostCodeExpressionTypes = new[] { "CSharp", "Python" }.ToFrozenSet(StringComparer.Ordinal); /// public override void Configure() @@ -36,13 +43,8 @@ internal class List(IExpressionDescriptorRegistry expressionDescriptorRegistry) return Send.OkAsync(response, cancellationToken); } - private bool CanListDescriptor(ExpressionDescriptor descriptor) - { - if (!PrivilegedExpressionPermissions.TryGetValue(descriptor.Type, out var permission)) - return true; - - return descriptor.IsBrowsable; - } + private static bool CanListDescriptor(ExpressionDescriptor descriptor) => + !HostCodeExpressionTypes.Contains(descriptor.Type) || descriptor.IsBrowsable; private static IEnumerable Map(List descriptors) => descriptors.Select(Map); diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkDispatch/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkDispatch/Endpoint.cs index 22d324158..71b90b1c2 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkDispatch/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkDispatch/Endpoint.cs @@ -36,10 +36,10 @@ internal class Endpoint( return; } - var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, User, cancellationToken); + var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, cancellationToken); if (!scriptAuthorizationResult.Succeeded) { - await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, Send.ForbiddenAsync, message => AddError(message), Send.ErrorsAsync, cancellationToken); + await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, message => AddError(message), Send.ErrorsAsync, cancellationToken); return; } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Endpoint.cs index 9549f38df..1864ca4a0 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Endpoint.cs @@ -80,10 +80,10 @@ internal class BulkPublish( foreach (var (_, definition) in publishableDefinitions) { var workflowGraph = await workflowDefinitionService.MaterializeWorkflowAsync(definition, cancellationToken); - var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, User, cancellationToken); + var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, cancellationToken); if (!scriptAuthorizationResult.Succeeded) { - await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, Send.ForbiddenAsync, message => AddError(message), Send.ErrorsAsync, cancellationToken); + await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, message => AddError(message), Send.ErrorsAsync, cancellationToken); return null!; } } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Dispatch/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Dispatch/Endpoint.cs index 6c1dea319..affe0169d 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Dispatch/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Dispatch/Endpoint.cs @@ -34,10 +34,10 @@ internal class Endpoint( return; } - var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, User, cancellationToken); + var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, cancellationToken); if (!scriptAuthorizationResult.Succeeded) { - await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, Send.ForbiddenAsync, message => AddError(message), Send.ErrorsAsync, cancellationToken); + await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, message => AddError(message), Send.ErrorsAsync, cancellationToken); return; } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Execute/WorkflowExecutionHelper.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Execute/WorkflowExecutionHelper.cs index fc6d1da8f..84b3f0f68 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Execute/WorkflowExecutionHelper.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Execute/WorkflowExecutionHelper.cs @@ -31,7 +31,7 @@ internal static class WorkflowExecutionHelper return; } - var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, httpContext.User, cancellationToken); + var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, cancellationToken); if (!scriptAuthorizationResult.Succeeded) { await SendScriptAuthorizationFailureAsync(httpContext, scriptAuthorizationResult, cancellationToken); @@ -99,12 +99,8 @@ internal static class WorkflowExecutionHelper private static async Task SendScriptAuthorizationFailureAsync(HttpContext httpContext, WorkflowDefinitionScriptAuthorizationResult result, CancellationToken cancellationToken) { - if (result.FailureReason == WorkflowDefinitionScriptAuthorizationFailureReason.MissingPermission) - { - await httpContext.Response.SendForbiddenAsync(cancellation: cancellationToken); - return; - } - + // Only one failure is possible: the host has the language switched off. That is a property of the + // deployment rather than of the caller, so it is a 400 explaining what is disabled, never a 403. httpContext.Response.StatusCode = StatusCodes.Status400BadRequest; await httpContext.Response.WriteAsync(result.Message ?? "Workflow script authorization failed.", cancellationToken); } 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 87d251372..ff8c12486 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Import/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Import/Endpoint.cs @@ -49,10 +49,10 @@ internal class Import : ElsaEndpoint var definitionId = model.DefinitionId; var isNew = string.IsNullOrWhiteSpace(definitionId); - var scriptAuthorizationResult = await _scriptAuthorizationService.AuthorizeAsync(model, User, cancellationToken); + var scriptAuthorizationResult = await _scriptAuthorizationService.AuthorizeAsync(model, cancellationToken); if (!scriptAuthorizationResult.Succeeded) { - await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, Send.ForbiddenAsync, message => AddError(message), Send.ErrorsAsync, cancellationToken); + await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, message => AddError(message), Send.ErrorsAsync, cancellationToken); return; } 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 c56b5ae7f..fb395a44f 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportFiles/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/ImportFiles/Endpoint.cs @@ -81,10 +81,10 @@ internal class ImportFiles : ElsaEndpoint { foreach (var model in models) { - var scriptAuthorizationResult = await _scriptAuthorizationService.AuthorizeAsync(model, User, cancellationToken); + var scriptAuthorizationResult = await _scriptAuthorizationService.AuthorizeAsync(model, cancellationToken); if (!scriptAuthorizationResult.Succeeded) { - await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, Send.ForbiddenAsync, message => AddError(message), Send.ErrorsAsync, cancellationToken); + await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, message => AddError(message), Send.ErrorsAsync, cancellationToken); return false; } } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Endpoint.cs index e20344f9b..4aed98ee5 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Endpoint.cs @@ -67,10 +67,10 @@ internal class Post( return; } - var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(model, User, cancellationToken); + var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(model, cancellationToken); if (!scriptAuthorizationResult.Succeeded) { - await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, Send.ForbiddenAsync, message => AddError(message), Send.ErrorsAsync, cancellationToken); + await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, message => AddError(message), Send.ErrorsAsync, cancellationToken); return; } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Endpoint.cs index a8b2eae79..344b84b35 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Endpoint.cs @@ -52,10 +52,10 @@ internal class Publish( } var workflowGraph = await workflowDefinitionService.MaterializeWorkflowAsync(definition, cancellationToken); - var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, User, cancellationToken); + var scriptAuthorizationResult = await scriptAuthorizationService.AuthorizeAsync(workflowGraph.Workflow, cancellationToken); if (!scriptAuthorizationResult.Succeeded) { - await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, Send.ForbiddenAsync, message => AddError(message), Send.ErrorsAsync, cancellationToken); + await WorkflowDefinitionScriptAuthorizationFailure.SendAsync(scriptAuthorizationResult, message => AddError(message), Send.ErrorsAsync, cancellationToken); return; } diff --git a/src/modules/Elsa.Workflows.Api/Security/WorkflowDefinitionScriptAuthorizationFailure.cs b/src/modules/Elsa.Workflows.Api/Security/WorkflowDefinitionScriptAuthorizationFailure.cs index 445ff6050..80bc75952 100644 --- a/src/modules/Elsa.Workflows.Api/Security/WorkflowDefinitionScriptAuthorizationFailure.cs +++ b/src/modules/Elsa.Workflows.Api/Security/WorkflowDefinitionScriptAuthorizationFailure.cs @@ -4,17 +4,12 @@ internal static class WorkflowDefinitionScriptAuthorizationFailure { public static async Task SendAsync( WorkflowDefinitionScriptAuthorizationResult result, - Func sendForbiddenAsync, Action addError, Func sendErrorsAsync, CancellationToken cancellationToken) { - if (result.FailureReason == WorkflowDefinitionScriptAuthorizationFailureReason.MissingPermission) - { - await sendForbiddenAsync(cancellationToken); - return; - } - + // Only one failure is possible: the host has the language switched off. That is a property of the + // deployment rather than of the caller, so it is a 400 explaining what is disabled, never a 403. addError(result.Message ?? "Workflow script authorization failed."); await sendErrorsAsync(400, cancellationToken); } diff --git a/src/modules/Elsa.Workflows.Api/Security/WorkflowDefinitionScriptAuthorizationService.cs b/src/modules/Elsa.Workflows.Api/Security/WorkflowDefinitionScriptAuthorizationService.cs index 355443786..eb0819cf6 100644 --- a/src/modules/Elsa.Workflows.Api/Security/WorkflowDefinitionScriptAuthorizationService.cs +++ b/src/modules/Elsa.Workflows.Api/Security/WorkflowDefinitionScriptAuthorizationService.cs @@ -1,4 +1,3 @@ -using System.Security.Claims; using Elsa.Expressions.Contracts; using Elsa.Extensions; using Elsa.Workflows.Activities; @@ -22,20 +21,20 @@ internal class WorkflowDefinitionScriptAuthorizationService( "Python.NET workflow expression execution is disabled by the host. Set PythonOptions.AllowHostCodeExecution to true only for trusted workflow authors; Python.NET is not a sandbox.") ]; - public async Task AuthorizeAsync(WorkflowDefinitionModel model, ClaimsPrincipal user, CancellationToken cancellationToken = default) + public async Task AuthorizeAsync(WorkflowDefinitionModel model, CancellationToken cancellationToken = default) { if (model.Root == null) return WorkflowDefinitionScriptAuthorizationResult.Allowed(); - return await AuthorizeAsync(model.Root, user, cancellationToken); + return await AuthorizeAsync(model.Root, cancellationToken); } - public async Task AuthorizeAsync(IActivity root, ClaimsPrincipal user, CancellationToken cancellationToken = default) + public async Task AuthorizeAsync(IActivity root, CancellationToken cancellationToken = default) { var scriptUsages = await GetUsedScriptPoliciesAsync(root, cancellationToken); var failure = scriptUsages - .Select(policy => AuthorizeScriptUsage(policy, user)) + .Select(AuthorizeScriptUsage) .FirstOrDefault(result => result is { Succeeded: false }); if (failure.FailureReason.HasValue) @@ -44,21 +43,23 @@ internal class WorkflowDefinitionScriptAuthorizationService( return WorkflowDefinitionScriptAuthorizationResult.Allowed(); } - public async Task AuthorizeAsync(Workflow workflow, ClaimsPrincipal user, CancellationToken cancellationToken = default) + public async Task AuthorizeAsync(Workflow workflow, CancellationToken cancellationToken = default) { - return await AuthorizeAsync((IActivity)workflow, user, cancellationToken); + return await AuthorizeAsync((IActivity)workflow, cancellationToken); } - private WorkflowDefinitionScriptAuthorizationResult AuthorizeScriptUsage(ScriptPolicy policy, ClaimsPrincipal user) + private WorkflowDefinitionScriptAuthorizationResult AuthorizeScriptUsage(ScriptPolicy policy) { // Language-specific options live in optional modules. Workflows.Api observes the descriptor state projected by those module providers. if (expressionDescriptorRegistry.Find(policy.ExpressionType)?.IsBrowsable != true) return WorkflowDefinitionScriptAuthorizationResult.HostDisabled(policy.HostDisabledMessage); - // The host switch is the single control. The former per-author permission conflated an incoherent - // execution-side gate -- a workflow runs under the server's authority, not the caller's, so the - // check never constrained what a script could do -- with a meaningful authoring-side one. Per-author - // script trust is redesigned separately; see #7975. + // The host switch is the only control, so there is nothing left to decide once it is on. The former + // per-author permission conflated an incoherent execution-side gate -- a workflow runs under the + // server's authority, not the caller's, so the check never constrained what a script could do -- + // with a meaningful authoring-side one. Neither the caller nor a failure reason for a denied caller + // is modelled here any more, because nothing produces one. Per-author script trust was considered and + // declined in #7975, so this is the settled shape rather than a stop on the way to one. return WorkflowDefinitionScriptAuthorizationResult.Allowed(); } @@ -87,12 +88,9 @@ internal readonly record struct WorkflowDefinitionScriptAuthorizationResult(bool public static WorkflowDefinitionScriptAuthorizationResult Allowed() => new(true, null, null); public static WorkflowDefinitionScriptAuthorizationResult HostDisabled(string message) => new(false, WorkflowDefinitionScriptAuthorizationFailureReason.HostDisabled, message); - - public static WorkflowDefinitionScriptAuthorizationResult MissingPermission() => new(false, WorkflowDefinitionScriptAuthorizationFailureReason.MissingPermission, null); } internal enum WorkflowDefinitionScriptAuthorizationFailureReason { - HostDisabled, - MissingPermission + HostDisabled } diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Security/WorkflowDefinitionScriptAuthorizationServiceTests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Security/WorkflowDefinitionScriptAuthorizationServiceTests.cs index ce997e76e..f06b84a70 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Security/WorkflowDefinitionScriptAuthorizationServiceTests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Security/WorkflowDefinitionScriptAuthorizationServiceTests.cs @@ -1,4 +1,3 @@ -using System.Security.Claims; using Elsa.Expressions.Contracts; using Elsa.Expressions.Models; using Elsa.Workflows.Activities; @@ -14,9 +13,6 @@ namespace Elsa.Workflows.IntegrationTests.Security; public class WorkflowDefinitionScriptAuthorizationServiceTests { - private static readonly ClaimsPrincipal UserWithCSharpPermission = CreateUser(PermissionNames.ExecuteCSharpExpressions); - private static readonly ClaimsPrincipal UserWithPythonPermission = CreateUser(PermissionNames.ExecutePythonExpressions); - private static readonly ClaimsPrincipal UserWithoutScriptPermission = CreateUser("workflows/definitions:write"); [Fact] public async Task AuthorizeAsync_BlocksCSharpExpression_WhenHostHasNotOptedIn() @@ -24,38 +20,28 @@ public class WorkflowDefinitionScriptAuthorizationServiceTests var service = CreateService(hostAllowsCSharp: false, hostAllowsPython: true); var model = CreateModelWithCSharpExpression(); - var result = await service.AuthorizeAsync(model, UserWithCSharpPermission); + var result = await service.AuthorizeAsync(model); Assert.Equal(WorkflowDefinitionScriptAuthorizationFailureReason.HostDisabled, result.FailureReason); Assert.Contains("CSharpOptions.AllowHostCodeExecution", result.Message); } [Fact] - public async Task AuthorizeAsync_AllowsCSharpExpression_WhenHostOptedIn_RegardlessOfUserPermissions() + public async Task AuthorizeAsync_AllowsCSharpExpression_WhenHostOptedIn() { var service = CreateService(hostAllowsCSharp: true, hostAllowsPython: true); var model = CreateModelWithCSharpExpression(); - var result = await service.AuthorizeAsync(model, UserWithoutScriptPermission); + var result = await service.AuthorizeAsync(model); - // The per-author permission was removed: a workflow runs under the server's authority, not the - // caller's, so gating execution on the caller never constrained what a script could do. The host - // switch is the only control now. Per-author script trust is redesigned in #7975. + // The host switch is the only control. The per-author permission was removed because a workflow runs + // under the server's authority, not the caller's, so gating on the caller never constrained what a + // script could do. The service no longer takes a principal at all, and #7975 closed won't-do, so + // this is the settled behaviour rather than an interim state. Assert.True(result.Succeeded); Assert.Null(result.FailureReason); } - [Fact] - public async Task AuthorizeAsync_AllowsCSharpExpression_WhenHostAndUserAllowIt() - { - var service = CreateService(hostAllowsCSharp: true, hostAllowsPython: true); - var model = CreateModelWithCSharpExpression(); - - var result = await service.AuthorizeAsync(model, UserWithCSharpPermission); - - Assert.True(result.Succeeded); - } - [Fact] public async Task AuthorizeAsync_AllowsWorkflowWithoutScriptUsage() { @@ -65,7 +51,7 @@ public class WorkflowDefinitionScriptAuthorizationServiceTests Root = new WriteLine("hello") }; - var result = await service.AuthorizeAsync(model, UserWithoutScriptPermission); + var result = await service.AuthorizeAsync(model); Assert.True(result.Succeeded); } @@ -82,11 +68,12 @@ public class WorkflowDefinitionScriptAuthorizationServiceTests } }; - var result = await service.AuthorizeAsync(model, UserWithoutScriptPermission); + var result = await service.AuthorizeAsync(model); - // The per-author permission was removed: a workflow runs under the server's authority, not the - // caller's, so gating execution on the caller never constrained what a script could do. The host - // switch is the only control now. Per-author script trust is redesigned in #7975. + // The host switch is the only control. The per-author permission was removed because a workflow runs + // under the server's authority, not the caller's, so gating on the caller never constrained what a + // script could do. The service no longer takes a principal at all, and #7975 closed won't-do, so + // this is the settled behaviour rather than an interim state. Assert.True(result.Succeeded); Assert.Null(result.FailureReason); } @@ -97,38 +84,28 @@ public class WorkflowDefinitionScriptAuthorizationServiceTests var service = CreateService(hostAllowsCSharp: true, hostAllowsPython: false); var model = CreateModelWithPythonExpression(); - var result = await service.AuthorizeAsync(model, UserWithPythonPermission); + var result = await service.AuthorizeAsync(model); Assert.Equal(WorkflowDefinitionScriptAuthorizationFailureReason.HostDisabled, result.FailureReason); Assert.Contains("PythonOptions.AllowHostCodeExecution", result.Message); } [Fact] - public async Task AuthorizeAsync_AllowsPythonExpression_WhenHostOptedIn_RegardlessOfUserPermissions() + public async Task AuthorizeAsync_AllowsPythonExpression_WhenHostOptedIn() { var service = CreateService(hostAllowsCSharp: true, hostAllowsPython: true); var model = CreateModelWithPythonExpression(); - var result = await service.AuthorizeAsync(model, UserWithoutScriptPermission); + var result = await service.AuthorizeAsync(model); - // The per-author permission was removed: a workflow runs under the server's authority, not the - // caller's, so gating execution on the caller never constrained what a script could do. The host - // switch is the only control now. Per-author script trust is redesigned in #7975. + // The host switch is the only control. The per-author permission was removed because a workflow runs + // under the server's authority, not the caller's, so gating on the caller never constrained what a + // script could do. The service no longer takes a principal at all, and #7975 closed won't-do, so + // this is the settled behaviour rather than an interim state. Assert.True(result.Succeeded); Assert.Null(result.FailureReason); } - [Fact] - public async Task AuthorizeAsync_AllowsPythonExpression_WhenHostAndUserAllowIt() - { - var service = CreateService(hostAllowsCSharp: true, hostAllowsPython: true); - var model = CreateModelWithPythonExpression(); - - var result = await service.AuthorizeAsync(model, UserWithPythonPermission); - - Assert.True(result.Succeeded); - } - [Fact] public async Task AuthorizeAsync_TreatsRunPythonActivityAsPythonUsage() { @@ -141,11 +118,12 @@ public class WorkflowDefinitionScriptAuthorizationServiceTests } }; - var result = await service.AuthorizeAsync(model, UserWithoutScriptPermission); + var result = await service.AuthorizeAsync(model); - // The per-author permission was removed: a workflow runs under the server's authority, not the - // caller's, so gating execution on the caller never constrained what a script could do. The host - // switch is the only control now. Per-author script trust is redesigned in #7975. + // The host switch is the only control. The per-author permission was removed because a workflow runs + // under the server's authority, not the caller's, so gating on the caller never constrained what a + // script could do. The service no longer takes a principal at all, and #7975 closed won't-do, so + // this is the settled behaviour rather than an interim state. Assert.True(result.Succeeded); Assert.Null(result.FailureReason); } @@ -206,9 +184,4 @@ public class WorkflowDefinitionScriptAuthorizationServiceTests return new(visitor, registry); } - private static ClaimsPrincipal CreateUser(params string[] permissions) - { - var identity = new ClaimsIdentity(permissions.Select(x => new Claim("permissions", x)), "Test"); - return new(identity); - } }