diff --git a/docs/usage_guidance.md b/docs/usage_guidance.md index 42cd3e3a3b..f9c993d7cd 100644 --- a/docs/usage_guidance.md +++ b/docs/usage_guidance.md @@ -17,12 +17,6 @@ There are two ways to convert an `Microsoft.AspNetCore.Http.HttpContext` to a `S **Recommendation**: For the most cases, implicit casting should be preferred as this will cache the created instance and ensure only a single `System.Web.HttpContext` per request. -## `System.Threading.Thread.CurrentPrincipal` not supported - -In ASP.NET Framework, `System.Threading.Thread.CurrentPrincipal` would be set to the current user. This is not available on ASP.NET Core. - -**Recommendation**: Use the property `HttpContext.User` instead. - ## `CultureInfo.CurrentCulture` is not set by default In ASP.NET Framework, `CultureInfo.Current` was set for a request, but this is not done automatically in ASP.NET Core. Instead, you must add the appropriate middleware to your pipeline. @@ -35,12 +29,26 @@ Simplest way to enable this with similar behavior as ASP.NET Framework would be app.UseRequestLocalization(); ``` +## `System.Threading.Thread.CurrentPrincipal` + +In ASP.NET Framework, `System.Threading.Thread.CurrentPrincipal` and `System.Security.Claims.ClaimsPrincipal.Current` would be set to the current user. This is not available on ASP.NET Core out of the box. Support for this is available with these adapters by adding the `ISetThreadCurrentPrincipal` to the endpoint (available to controllers via the `SetThreadCurrentPrincipalAttribute`). However, it should only be used if the code cannot be refactored to remove usage. + +**Recommendation**: If possible, use the property `HttpContext.User` instead by passing it through to the call site. If not possible, enable setting the current user and also consider setting the request to be a logical single thread (see below for details). + ## Request thread does not exist in ASP.NET Core In ASP.NET Framework, a request had thread-affinity and `HttpContext.Current` would only be available if on that thread. ASP.NET Core does not have this guarantee so `HttpContext.Current` will be available within the same async context, but no guarantees about threads are made. -**Recommendation**: If reading/writing to the `HttpContext`, you must ensure you are doing so in a single-threaded way. +**Recommendation**: If reading/writing to the `HttpContext`, you must ensure you are doing so in a single-threaded way. You can force a request to never run concurrently on any async context by setting the `ISingleThreadedRequestMetadata`. This will have performance implications and should only be used if you can't refactor usage to ensure non-concurrent access. There is an implementation available to add to controllers with `SingleThreadedRequestAttribute`: + +```csharp +[SingleThreadedRequest] +public class SomeController : Controller +{ + ... +} +``` ## `HttpContext.Request` may need to be prebuffered diff --git a/samples/MvcCoreApp/Program.cs b/samples/MvcCoreApp/Program.cs index fd6c5ee0e1..352eee49aa 100644 --- a/samples/MvcCoreApp/Program.cs +++ b/samples/MvcCoreApp/Program.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using Microsoft.AspNetCore.SystemWebAdapters; var builder = WebApplication.CreateBuilder(); @@ -36,6 +37,23 @@ void ConfigureRemoteServiceOptions(RemoteServiceOptions options) app.UseSystemWebAdapters(); +app.MapGet("/current-principals-with-metadata", (HttpContext ctx) => +{ + var user1 = Thread.CurrentPrincipal; + var user2 = ClaimsPrincipal.Current; + + return "done"; +}).WithMetadata(new SetThreadCurrentPrincipalAttribute(), new SingleThreadedRequestAttribute()); + + +app.MapGet("/current-principals-no-metadata", (HttpContext ctx) => +{ + var user1 = Thread.CurrentPrincipal; + var user2 = ClaimsPrincipal.Current; + + return "done"; +}); + app.UseEndpoints(endpoints => { app.MapDefaultControllerRoute(); diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/CurrentPrincipalMiddleware.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/CurrentPrincipalMiddleware.cs new file mode 100644 index 0000000000..df4a558c6c --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/CurrentPrincipalMiddleware.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +internal partial class CurrentPrincipalMiddleware +{ + [LoggerMessage(0, LogLevel.Trace, "Thread.CurrentPrincipal has been set with the current user")] + private partial void LogCurrentPrincipalUsage(); + + private readonly RequestDelegate _next; + private readonly ILogger _logger; + + public CurrentPrincipalMiddleware(RequestDelegate next, ILogger logger) + { + _next = next; + _logger = logger; + } + + public Task InvokeAsync(HttpContext context) + => context.GetEndpoint()?.Metadata.GetMetadata() is { IsEnabled: true } ? SetUserAsync(context) : _next(context); + + private async Task SetUserAsync(HttpContext context) + { + LogCurrentPrincipalUsage(); + + var current = Thread.CurrentPrincipal; + + try + { + Thread.CurrentPrincipal = context.User; + + await _next(context); + } + finally + { + Thread.CurrentPrincipal = current; + } + } +} diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/ISetThreadCurrentPrincipal.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/ISetThreadCurrentPrincipal.cs new file mode 100644 index 0000000000..8326e9f5d5 --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/ISetThreadCurrentPrincipal.cs @@ -0,0 +1,9 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +public interface ISetThreadCurrentPrincipal +{ + bool IsEnabled { get; } +} diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/ISingleThreadedRequestMetadata.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/ISingleThreadedRequestMetadata.cs new file mode 100644 index 0000000000..cc95e5d3f2 --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/ISingleThreadedRequestMetadata.cs @@ -0,0 +1,9 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +public interface ISingleThreadedRequestMetadata +{ + bool IsEnabled { get; } +} diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SetThreadCurrentPrincipalAttribute.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SetThreadCurrentPrincipalAttribute.cs new file mode 100644 index 0000000000..bd61c7819f --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SetThreadCurrentPrincipalAttribute.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] +public sealed class SetThreadCurrentPrincipalAttribute : Attribute, ISetThreadCurrentPrincipal, ISingleThreadedRequestMetadata +{ + public bool IsEnabled { get; set; } = true; +} diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SingleThreadedRequestAttribute.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SingleThreadedRequestAttribute.cs new file mode 100644 index 0000000000..64d2481602 --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SingleThreadedRequestAttribute.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] +public sealed class SingleThreadedRequestAttribute : Attribute, ISingleThreadedRequestMetadata +{ + public bool IsEnabled { get; set; } = true; +} diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SingleThreadedRequestMiddleware.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SingleThreadedRequestMiddleware.cs new file mode 100644 index 0000000000..a26d04fb77 --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SingleThreadedRequestMiddleware.cs @@ -0,0 +1,26 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +internal class SingleThreadedRequestMiddleware +{ + private readonly RequestDelegate _next; + + public SingleThreadedRequestMiddleware(RequestDelegate next) => _next = next; + + public Task InvokeAsync(HttpContextCore context) + => context.GetEndpoint()?.Metadata.GetMetadata() is { IsEnabled: true } + ? EnsureSingleThreaded(context) + : _next(context); + + private Task EnsureSingleThreaded(HttpContextCore context) + { + var schedule = new ConcurrentExclusiveSchedulerPair(TaskScheduler.Default, 1); + + return Task.Factory.StartNew(() => _next(context), context.RequestAborted, TaskCreationOptions.DenyChildAttach, schedule.ExclusiveScheduler).Unwrap(); + } +} diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SystemWebAdaptersExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SystemWebAdaptersExtensions.cs index d01a15c6af..943e819283 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SystemWebAdaptersExtensions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SystemWebAdaptersExtensions.cs @@ -32,6 +32,8 @@ public static void UseSystemWebAdapters(this IApplicationBuilder app) app.UseMiddleware(); app.UseMiddleware(); app.UseMiddleware(); + app.UseMiddleware(); + app.UseMiddleware(); } ///