SemianiakaVY/elsa-core
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

Enhance HTTP Endpoint + Flowchart fix (#4469)

Browse source 
* Fix flowchart next activity scheduling bug

This fixes a bug where the Flowchart activity would schedule connected activities regardless of the outcome of the completed activity.

* Cancel activity execution when cancellation token is triggered.

* Enable user to handle HTTP endpoint validation failures as outcomes

* Don't respond with full details in case of HTTP Endpoint faults for security
This commit is contained in:
Sipke Schoorstra 2023-09-22 18:45:18 +02:00 committed by GitHub
parent a201672584
commit a5d13e4e9b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 307 additions and 78 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
src/common/Elsa.Testing.Shared/TestApplicationBuilder.cs
View file

@ -100,6 +100,14 @@ public class TestApplicationBuilder
return ConfigureElsa(elsa => elsa.AddWorkflow<T>());
}
/// <summary>
/// Adds activities from the assembly containing the specified type.
/// </summary>
public TestApplicationBuilder AddActivitiesFrom<T>()
{
return ConfigureElsa(elsa => elsa.AddActivitiesFrom<T>());
}
/// <summary>
/// Add workflows from the specified relative directory.
/// </summary>

174
src/modules/Elsa.Http/Activities/HttpEndpoint.cs
View file

@ -92,6 +92,30 @@ public class HttpEndpoint : Trigger<HttpRequest>
[Input(Description = "Only MIME types in this list are allowed. Leave empty to allow all types", Category = "Upload", UIHint = InputUIHints.MultiText)]
public Input<ICollection<string>> AllowedMimeTypes { get; set; } = default!;
/// <summary>
/// A value indicating whether to expose the "Request too large" outcome.
/// </summary>
[Input(Description = "A value indicating whether to expose the \"Request too large\" outcome.", Category = "Outcomes")]
public bool ExposeRequestTooLargeOutcome { get; set; }
/// <summary>
/// A value indicating whether to expose the "File too large" outcome.
/// </summary>
[Input(Description = "A value indicating whether to expose the \"File too large\" outcome.", Category = "Outcomes")]
public bool ExposeFileTooLargeOutcome { get; set; }
/// <summary>
/// A value indicating whether to expose the "Invalid file extension" outcome.
/// </summary>
[Input(Description = "A value indicating whether to expose the \"Invalid file extension\" outcome.", Category = "Outcomes")]
public bool ExposeInvalidFileExtensionOutcome { get; set; }
/// <summary>
/// A value indicating whether to expose the "Invalid file MIME type" outcome.
/// </summary>
[Input(Description = "A value indicating whether to expose the \"Invalid file MIME type\" outcome.", Category = "Outcomes")]
public bool ExposeInvalidFileMimeTypeOutcome { get; set; }
/// <summary>
/// The parsed request content, if any.
/// </summary>
@ -184,23 +208,45 @@ public class HttpEndpoint : Trigger<HttpRequest>
context.Set(RouteData, routeDictionary);
context.Set(QueryStringData, queryStringDictionary);
context.Set(Headers, headersDictionary);
// Validate request size.
if (!ValidateRequestSize(context, httpContext))
{
await HandleRequestTooLargeAsync(context, httpContext);
return;
}
// Read files, if any.
var files = ReadFilesAsync(context, request);
if (!await ValidateFileSizesAsync(context, httpContext, files))
return;
if (files.Any())
{
if (!ValidateFileSizes(context, httpContext, files))
{
await HandleFileSizeTooLargeAsync(context, httpContext);
return;
}
if (!await ValidateFileExtensionWhitelistAsync(context, httpContext, files))
return;
if (!ValidateFileExtensionWhitelist(context, httpContext, files))
{
await HandleInvalidFileExtensionWhitelistAsync(context, httpContext);
return;
}
if (!await ValidateFileExtensionBlacklistAsync(context, httpContext, files))
return;
if (!ValidateFileExtensionBlacklist(context, httpContext, files))
{
await HandleInvalidFileExtensionBlacklistAsync(context, httpContext);
return;
}
if (!await ValidateFileMimeTypesAsync(context, httpContext, files))
return;
if (!ValidateFileMimeTypes(context, httpContext, files))
{
await HandleInvalidFileMimeTypesAsync(context, httpContext);
return;
}
Files.Set(context, files.ToArray());
Files.Set(context, files.ToArray());
}
// Read content, if any.
var content = await ParseContentAsync(context, request);
@ -215,9 +261,39 @@ public class HttpEndpoint : Trigger<HttpRequest>
return request.HasFormContentType ? request.Form.Files : new FormFileCollection();
}
private async Task<bool> ValidateFileSizesAsync(ActivityExecutionContext context, HttpContext httpContext, IFormFileCollection files)
private bool ValidateRequestSize(ActivityExecutionContext context, HttpContext httpContext)
{
var requestSizeLimit = RequestSizeLimit.GetOrDefault(context);
if (!requestSizeLimit.HasValue)
return true;
var requestSize = httpContext.Request.ContentLength ?? 0;
return requestSize <= requestSizeLimit;
}
private async Task HandleRequestTooLargeAsync(ActivityExecutionContext context, HttpContext httpContext)
{
var exposeRequestTooLargeOutcome = ExposeRequestTooLargeOutcome;
if (exposeRequestTooLargeOutcome)
{
await context.CompleteActivityWithOutcomesAsync("Request too large");
}
else
{
var response = httpContext.Response;
response.StatusCode = StatusCodes.Status413PayloadTooLarge;
await response.WriteAsJsonAsync(new
{
Message = $"The maximum request size allowed is {RequestSizeLimit.Get(context)} bytes."
});
await response.Body.FlushAsync();
}
}
private bool ValidateFileSizes(ActivityExecutionContext context, HttpContext httpContext, IFormFileCollection files)
{
// Validate individual file sizes.
var fileSizeLimit = FileSizeLimit.GetOrDefault(context);
if (!fileSizeLimit.HasValue)
@ -226,18 +302,30 @@ public class HttpEndpoint : Trigger<HttpRequest>
if (!files.Any(file => file.Length > fileSizeLimit.Value))
return true;
var response = httpContext.Response;
response.StatusCode = StatusCodes.Status413PayloadTooLarge;
await response.WriteAsJsonAsync(new
{
Message = $"The maximum file size allowed is {fileSizeLimit} bytes."
});
await response.Body.FlushAsync();
return false;
}
private async Task<bool> ValidateFileExtensionWhitelistAsync(ActivityExecutionContext context, HttpContext httpContext, IFormFileCollection files)
private async Task HandleFileSizeTooLargeAsync(ActivityExecutionContext context, HttpContext httpContext)
{
var exposeFileTooLargeOutcome = ExposeFileTooLargeOutcome;
if (exposeFileTooLargeOutcome)
{
await context.CompleteActivityWithOutcomesAsync("File too large");
}
else
{
var response = httpContext.Response;
response.StatusCode = StatusCodes.Status413PayloadTooLarge;
await response.WriteAsJsonAsync(new
{
Message = $"The maximum file size allowed is {FileSizeLimit.Get(context)} bytes."
});
await response.Body.FlushAsync();
}
}
private bool ValidateFileExtensionWhitelist(ActivityExecutionContext context, HttpContext httpContext, IFormFileCollection files)
{
var allowedFileExtensions = AllowedFileExtensions.GetOrDefault(context);
@ -247,18 +335,28 @@ public class HttpEndpoint : Trigger<HttpRequest>
if (files.All(file => allowedFileExtensions.Contains(System.IO.Path.GetExtension(file.FileName), StringComparer.OrdinalIgnoreCase)))
return true;
return false;
}
private async Task HandleInvalidFileExtensionWhitelistAsync(ActivityExecutionContext context, HttpContext httpContext)
{
if (ExposeInvalidFileExtensionOutcome)
{
await context.CompleteActivityWithOutcomesAsync("Invalid file extension");
return;
}
var response = httpContext.Response;
var allowedFileExtensions = AllowedFileExtensions.GetOrDefault(context)!;
response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
await response.WriteAsJsonAsync(new
{
Message = $"Only the following file extensions are allowed: {string.Join(", ", allowedFileExtensions)}"
});
await response.Body.FlushAsync();
return false;
}
private async Task<bool> ValidateFileExtensionBlacklistAsync(ActivityExecutionContext context, HttpContext httpContext, IFormFileCollection files)
private bool ValidateFileExtensionBlacklist(ActivityExecutionContext context, HttpContext httpContext, IFormFileCollection files)
{
var blockedFileExtensions = BlockedFileExtensions.GetOrDefault(context);
@ -268,6 +366,18 @@ public class HttpEndpoint : Trigger<HttpRequest>
if (!files.Any(file => blockedFileExtensions.Contains(System.IO.Path.GetExtension(file.FileName), StringComparer.OrdinalIgnoreCase)))
return true;
return false;
}
private async Task HandleInvalidFileExtensionBlacklistAsync(ActivityExecutionContext context, HttpContext httpContext)
{
if (ExposeInvalidFileExtensionOutcome)
{
await context.CompleteActivityWithOutcomesAsync("Invalid file extension");
return;
}
var blockedFileExtensions = BlockedFileExtensions.GetOrDefault(context)!;
var response = httpContext.Response;
response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
await response.WriteAsJsonAsync(new
@ -275,11 +385,9 @@ public class HttpEndpoint : Trigger<HttpRequest>
Message = $"The following file extensions are not allowed: {string.Join(", ", blockedFileExtensions)}"
});
await response.Body.FlushAsync();
return false;
}
private async Task<bool> ValidateFileMimeTypesAsync(ActivityExecutionContext context, HttpContext httpContext, IFormFileCollection files)
private bool ValidateFileMimeTypes(ActivityExecutionContext context, HttpContext httpContext, IFormFileCollection files)
{
var allowedMimeTypes = AllowedMimeTypes.GetOrDefault(context);
@ -289,6 +397,18 @@ public class HttpEndpoint : Trigger<HttpRequest>
if (files.All(file => allowedMimeTypes.Contains(file.ContentType, StringComparer.OrdinalIgnoreCase)))
return true;
return false;
}
private async Task HandleInvalidFileMimeTypesAsync(ActivityExecutionContext context, HttpContext httpContext)
{
if(ExposeInvalidFileMimeTypeOutcome)
{
await context.CompleteActivityWithOutcomesAsync("Invalid file MIME type");
return;
}
var allowedMimeTypes = AllowedMimeTypes.GetOrDefault(context)!;
var response = httpContext.Response;
response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
await response.WriteAsJsonAsync(new
@ -296,8 +416,6 @@ public class HttpEndpoint : Trigger<HttpRequest>
Message = $"Only the following MIME types are allowed: {string.Join(", ", allowedMimeTypes)}"
});
await response.Body.FlushAsync();
return false;
}
private async Task<object?> ParseContentAsync(ActivityExecutionContext context, HttpRequest httpRequest)

23
src/modules/Elsa.Http/Handlers/DefaultHttpEndpointFaultHandler.cs
View file

@ -6,35 +6,20 @@ using Elsa.Workflows.Core.Contracts;
namespace Elsa.Http.Handlers;
// ReSharper disable once ClassWithVirtualMembersNeverInherited.Global
/// <summary>
/// A default fault handler that writes information about the fault to the <see cref="HttpResponse"/>.
/// </summary>
public class DefaultHttpEndpointFaultHandler : IHttpEndpointFaultHandler
public sealed class DefaultHttpEndpointFaultHandler : IHttpEndpointFaultHandler
{
private readonly IApiSerializer _apiSerializer;
/// <summary>
/// Initializes a new instance of the <see cref="DefaultHttpEndpointFaultHandler"/> class.
/// </summary>
public DefaultHttpEndpointFaultHandler(IApiSerializer apiSerializer)
{
_apiSerializer = apiSerializer;
}
/// <inheritdoc />
public virtual async ValueTask HandleAsync(HttpEndpointFaultContext context)
public ValueTask HandleAsync(HttpEndpointFaultContext context)
{
var httpContext = context.HttpContext;
var workflowState = context.WorkflowState;
var isTimeoutIncident = GetIsTimeoutFault(context);
var statusCode = isTimeoutIncident ? StatusCodes.Status408RequestTimeout : StatusCodes.Status500InternalServerError;
httpContext.Response.ContentType = MediaTypeNames.Application.Json;
httpContext.Response.StatusCode = statusCode;
var faultedResponse = _apiSerializer.Serialize(workflowState);
await httpContext.Response.WriteAsync(faultedResponse, context.CancellationToken);
return ValueTask.CompletedTask;
}
private bool GetIsTimeoutFault(HttpEndpointFaultContext context)

47
src/modules/Elsa.Http/Handlers/DetailedHttpEndpointFaultHandler.cs Normal file
View file

@ -0,0 +1,47 @@
using System.Net.Mime;
using Elsa.Http.Contracts;
using Elsa.Http.Models;
using Elsa.Workflows.Core.Contracts;
using Microsoft.AspNetCore.Http;
namespace Elsa.Http.Handlers;
/// <summary>
/// A fault handler that writes detailed information about the fault to the <see cref="HttpResponse"/>.
/// </summary>
public sealed class DetailedHttpEndpointFaultHandler : IHttpEndpointFaultHandler
{
private readonly IApiSerializer _apiSerializer;
/// <summary>
/// Initializes a new instance of the <see cref="DefaultHttpEndpointFaultHandler"/> class.
/// </summary>
public DetailedHttpEndpointFaultHandler(IApiSerializer apiSerializer)
{
_apiSerializer = apiSerializer;
}
/// <inheritdoc />
public async ValueTask HandleAsync(HttpEndpointFaultContext context)
{
var httpContext = context.HttpContext;
var workflowState = context.WorkflowState;
var isTimeoutIncident = GetIsTimeoutFault(context);
var statusCode = isTimeoutIncident ? StatusCodes.Status408RequestTimeout : StatusCodes.Status500InternalServerError;
httpContext.Response.ContentType = MediaTypeNames.Application.Json;
httpContext.Response.StatusCode = statusCode;
var faultedResponse = _apiSerializer.Serialize(workflowState);
await httpContext.Response.WriteAsync(faultedResponse, context.CancellationToken);
}
private bool GetIsTimeoutFault(HttpEndpointFaultContext context)
{
var workflowState = context.WorkflowState;
var exceptionTypes = new[] { typeof(OperationCanceledException), typeof(TaskCanceledException), typeof(TimeoutException) };
var timeoutIncident = workflowState.Incidents.FirstOrDefault(x => exceptionTypes.Contains(x.Exception?.Type));
return timeoutIncident != null;
}
}

18
src/modules/Elsa.Http/Middleware/WorkflowsMiddleware.cs
View file

@ -132,23 +132,7 @@ public class WorkflowsMiddleware
// Get settings from the bookmark payload.
var foundBookmarkPayload = matchedWorkflow.Payload as HttpEndpointBookmarkPayload;
// Get the configured request size limit, if any.
var requestSizeLimit = foundBookmarkPayload?.RequestSizeLimit;
if (requestSizeLimit != null)
{
// If a request size limit was configured, check if the request size exceeds the limit.
var requestSize = request.ContentLength ?? 0;
if (requestSize > requestSizeLimit)
{
httpContext.Response.StatusCode = 413; // Payload Too Large.
await httpContext.Response.WriteAsJsonAsync(new { message = $"The request size exceeds the configured limit of {requestSizeLimit} bytes." }, cancellationToken: cancellationToken);
return;
}
}
// Get the configured request timeout, if any.
var requestTimeout = foundBookmarkPayload?.RequestTimeout;

22
src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs
View file

@ -180,17 +180,13 @@ public class Flowchart : Container
var result = context.Result;
logger.LogDebug("Child activity {ActivityId} completed with status {ActivityStatus}", completedActivity.Id, completedActivityContext.Status);
// If the complete activity's status is anything but "Completed", do not schedule its outbound activities.
var scheduleChildren = completedActivityContext.Status == ActivityStatus.Completed;
// If specific outcomes were provided by the completed activity, use them to find the connection to the next activity.
Func<Connection, bool> outboundConnectionsQuery = result is Outcomes outcomes
? connection => connection.Source.Activity == completedActivity && outcomes.Names.Contains(connection.Source.Port)
: connection => connection.Source.Activity == completedActivity;
var outcomeNames = result is Outcomes outcomes ? outcomes.Names : new[] { default(string), "Done" };
// Only query the outbound connections if the completed activity wasn't already completed.
var outboundConnections = Connections.Where(outboundConnectionsQuery).ToList();
var outboundConnections = Connections.Where(connection => connection.Source.Activity == completedActivity && outcomeNames.Contains(connection.Source.Port)).ToList();
var children = outboundConnections.Select(x => x.Target.Activity).ToList();
var scope = flowchartContext.GetProperty(ScopeProperty, () => new FlowScope());
@ -206,11 +202,11 @@ public class Flowchart : Container
{
if (children.Any())
{
if(children.Count == 1)
if (children.Count == 1)
logger.LogDebug("Found 1 child for activity {ActivityId}: {ChildActivityId}", completedActivity.Id, children.First().Id);
else
logger.LogDebug("Found {Count} children for activity {ActivityId}: {ChildActivityIds}", children.Count, completedActivity.Id, children.Select(x => x.Id).ToList());
scope.AddActivities(children);
// Schedule each child, but only if all of its left inbound activities have already executed.
@ -248,12 +244,12 @@ public class Flowchart : Container
ReuseActivityExecutionContextId = joinContext?.Id,
PreventDuplicateScheduling = true
};
if(joinContext != null)
if (joinContext != null)
logger.LogDebug("Next activity {ChildActivityId} is a join activity. Attaching to existing context {JoinContext}", activity.Id, joinContext.Id);
else
logger.LogDebug("Next activity {ChildActivityId} is a join activity", activity.Id);
logger.LogDebug("Scheduling child activity {ChildActivityId}", activity.Id);
await flowchartContext.ScheduleActivityAsync(activity, scheduleWorkOptions);
}
@ -263,7 +259,7 @@ public class Flowchart : Container
if (!children.Any())
{
logger.LogDebug("No children found for activity {ActivityId}", completedActivity.Id);
// If there is no pending work, complete the flowchart activity.
var hasPendingWork = HasPendingWork(flowchartContext);

5
src/modules/Elsa.Workflows.Core/Activities/Flowchart/Serialization/ConnectionComparer.cs
View file

@ -20,6 +20,9 @@ public class ConnectionComparer : IEqualityComparer<Connection>
/// <inheritdoc />
public int GetHashCode(Connection obj)
{
return HashCode.Combine(obj.Source.Activity.Id, obj.Target.Activity.Id, obj.Source.Port, obj.Target.Port);
// ReSharper disable ConditionalAccessQualifierIsNonNullableAccordingToAPIContract
// Justification: These can be null when the designer is in an invalid state. For example, if it used a NotFoundActivity that no longer has the same outcomes.
return HashCode.Combine(obj.Source?.Activity?.Id, obj.Target?.Activity?.Id, obj.Source?.Port, obj.Target?.Port);
// ReSharper restore ConditionalAccessQualifierIsNonNullableAccordingToAPIContract
}
}

2
src/modules/Elsa.Workflows.Core/Middleware/Activities/DefaultActivityInvokerMiddleware.cs
View file

@ -35,6 +35,8 @@ public class DefaultActivityInvokerMiddleware : IActivityExecutionMiddleware
/// <inheritdoc />
public async ValueTask InvokeAsync(ActivityExecutionContext context)
{
context.CancellationToken.ThrowIfCancellationRequested();
var workflowExecutionContext = context.WorkflowExecutionContext;
// Evaluate input properties.

2
test/integration/Elsa.IntegrationTests/Elsa.IntegrationTests.csproj
View file

@ -122,6 +122,8 @@
</ItemGroup>

14
test/integration/Elsa.IntegrationTests/Scenarios/FlowchartNextActivity/Activities/CustomActivity.cs Normal file
View file

@ -0,0 +1,14 @@
using System;
using System.Threading.Tasks;
using Elsa.Extensions;
using Elsa.Workflows.Core;
namespace Elsa.IntegrationTests.Scenarios.FlowchartNextActivity.Activities;
public class CustomActivity : CodeActivity
{
protected override async ValueTask ExecuteAsync(ActivityExecutionContext context)
{
await context.CompleteActivityAsync();
}
}

37
test/integration/Elsa.IntegrationTests/Scenarios/FlowchartNextActivity/Tests.cs Normal file
View file

@ -0,0 +1,37 @@
using System;
using System.Linq;
using System.Threading.Tasks;
using Elsa.IntegrationTests.Scenarios.FlowchartNextActivity.Workflows;
using Elsa.Testing.Shared;
using Elsa.Workflows.Core.Contracts;
using Microsoft.Extensions.DependencyInjection;
using Xunit;
using Xunit.Abstractions;
namespace Elsa.IntegrationTests.Scenarios.FlowchartNextActivity;
public class FlowchartNextActivityTests
{
private readonly CapturingTextWriter _capturingTextWriter = new();
private readonly IServiceProvider _services;
private readonly IWorkflowRunner _workflowRunner;
public FlowchartNextActivityTests(ITestOutputHelper testOutputHelper)
{
_services = new TestApplicationBuilder(testOutputHelper)
.WithCapturingTextWriter(_capturingTextWriter)
.AddActivitiesFrom<FlowchartNextActivityTests>()
.Build();
_workflowRunner = _services.GetRequiredService<IWorkflowRunner>();
}
[Fact(DisplayName = "Flowchart only schedules next activity connected to outcome of previous activity.")]
public async Task Test1()
{
await _services.PopulateRegistriesAsync();
await _workflowRunner.RunAsync<FlowchartWorkflow>();
var lines = _capturingTextWriter.Lines.ToList();
Assert.Equal(new[] { "Line 1" }, lines);
}
}

33
test/integration/Elsa.IntegrationTests/Scenarios/FlowchartNextActivity/Workflows/FlowchartWorkflow.cs Normal file
View file

@ -0,0 +1,33 @@
using Elsa.IntegrationTests.Scenarios.FlowchartNextActivity.Activities;
using Elsa.Workflows.Core;
using Elsa.Workflows.Core.Activities;
using Elsa.Workflows.Core.Activities.Flowchart.Activities;
using Elsa.Workflows.Core.Activities.Flowchart.Models;
using Elsa.Workflows.Core.Contracts;
namespace Elsa.IntegrationTests.Scenarios.FlowchartNextActivity.Workflows;
public class FlowchartWorkflow : WorkflowBase
{
protected override void Build(IWorkflowBuilder builder)
{
var customActivity = new CustomActivity();
var writeLine1 = new WriteLine("Line 1");
var writeLine2 = new WriteLine("Line 2");
builder.Root = new Flowchart
{
Activities =
{
customActivity,
writeLine1,
writeLine2
},
Connections =
{
new Connection(new Endpoint(customActivity, "Done"), new Endpoint(writeLine1)),
new Connection(new Endpoint(customActivity, "Fake"), new Endpoint(writeLine2)),
}
};
}
}