Thread.CurrentPrincipal does not flow with ExecutionContext

[AndreasHeisel]

We have a problem with System.Threading.ExecutionContext and System.Threading.Thread.CurrentPrincipal on .NET Core 2.1.

We want to implement a custom asynchronous point. According to docs.microsoft.com System.Threading.ExecutionContext is the way to go:

The ExecutionContext class provides the functionality for user code to capture and transfer this context across user-defined asynchronous points.

Test:

using System;
using System.Security.Claims;
using System.Threading;
using System.Threading.Tasks;

class Program
{
	static void Main()
	{
		var principal = new ClaimsPrincipal();

		Thread.CurrentPrincipal = principal;
		var context = ExecutionContext.Capture();
		Thread.CurrentPrincipal = null;

		ExecutionContext.Run
		(
			context,
			state =>
			{
				Console.WriteLine("Thread.CurrentPrincipal: {0}", Thread.CurrentPrincipal);
			},
			null
		);
	}
}

Using .NET Framework 4.7.1 I get:
Thread.CurrentPrincipal: System.Security.Claims.ClaimsPrincipal
Wich is the expected behavior, according to the documentation

The ExecutionContext class provides a single container for all information relevant to a logical thread of execution. This includes security context, call context, and synchronization context.

Using .NET Core 2.1 I get:
Thread.CurrentPrincipal:

I tested the behavior with the runtime provided asynchronoues point "await" and "Task.Run". In both cases it worked as expected.

Is this behavioral change intended?

[MarcoRossignoli]

I think so. AFAIK in .NET core context is a list of AsyncLocal<T> added through Value set
I don't see any code on Thread.CurrentPrincipal to flow it to context.
On full .NET Principal is added to context https://referencesource.microsoft.com/#mscorlib/system/threading/thread.cs,1369
Here one sample of AsyncLocal used for impersonation https://github.com/dotnet/corefx/blob/master/src/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs#L715
Maybe guide should be updated https://docs.microsoft.com/en-us/dotnet/api/system.threading.executioncontext?view=netcore-2.2#remarks

/cc @stephentoub

[MarcoRossignoli]

I tested the behavior with the runtime provided asynchronoues point "await" and "Task.Run". In both cases it worked as expected.

I did some test and on simple console app 2.2 principal seems not flow(maybe I misunderstood):

 internal class Program
    {
        private static async Task Main(string[] args)
        {
            await Context1();
            Console.WriteLine("Test end");
            Console.ReadKey();
        }

        public static async Task Context1()
        {
            ClaimsPrincipal principal = new ClaimsPrincipal();

            Debug.Assert(Thread.CurrentPrincipal == null);
            Thread.CurrentPrincipal = principal;
            Debug.Assert(Thread.CurrentPrincipal != null);
            Debug.Assert(Thread.CurrentPrincipal is ClaimsPrincipal);

            Console.WriteLine(Thread.CurrentPrincipal);
            Console.WriteLine(Thread.CurrentThread.ManagedThreadId);

            await Task.Run(async () => await Context2());
        }

        public static async Task Context2()
        {
            Debug.Assert(Thread.CurrentPrincipal == null); // empty Thread.CurrentPrincipal
            Console.WriteLine(Thread.CurrentPrincipal);
            Console.WriteLine(Thread.CurrentThread.ManagedThreadId);
        }
    }

[AndreasHeisel]

You are right, my tests were wrong. Await and Task.Run also doen't work on .NET Core.

This is a big issue for us, and it doesn't answer the question if it is a bug or a feature. Other contextual data like CurrentCulture and CurrentUICulture flow as they were ported to AsyncLocal.

[MarcoRossignoli]

I think this is a feature...context flow is a key piece of async/await machinery...however I cannot answer to this we need to wait team guys /cc @stephentoub @jkotas

[jkotas]

If the CurrentPrincipal was part of ExecutionContext on desktop, it makes sense to flow it in .NET Core as well for better compatibility.

It should be done lazily - create the AsyncLocal only once somebody actually sets the principal.

[jkotas]

cc @danmosemsft

[AndreasHeisel]

It's good to see this on the to do list.

@jkotas This is not only for completeness and compatibility. There is a real need for the feature. It enables authorisation downstream in an asynchronous path if you don't control all the code on the way.

[MarcoRossignoli]

I think this is a feature...context flow is a key piece of async/await machinery

I'm surprised that this issue doesn't come out before.

[benaadams]

cc @blowdart

[jkotas]

I'm surprised that this issue doesn't come out before.

It has come up a few times before, e.g. aspnet/Security#322 or dotnet/aspnetcore#481. Depending on ambient statics for security-related stuff is not very robust pattern as multiple people pointed out in the linked issues. It is easy to make a subtle mistake and get these ambient statics mixed up or escape unexpectedly.

[MarcoRossignoli]

Thank's Jan for pointing issues....my "surprise" come right from "web apps"

[jkotas]

@MarcoRossignoli Interested in grabbing this one? This one should be pretty straightforward and non-controversial.

[MarcoRossignoli]

Sure @jkotas I will grab this ASAP, thank you for your trust.

[gulbanana]

This change is a bit worrying. What if people have existing .NET Core ambient-security code which really does expect Thread.CurrentPrincipal to mean the thread's principal, rather than the principal of an execution context? This situation can happen when you dispatch work into some sort of pool of worker threads, or when you lazily set up per-thread security contexts.

[jkotas]

Right, any change has potential to break somebody. We try to make sure that benefits outweigh the risks.

@gulbanana Do you have a real example of a program that would be broken by this change?

This property is unlikely to be used in .NET Core code written from scratch. It is not used by ASP.NET Core. This property is most likely going to be used in code being ported from .NET Framework where it was flowing with the execution context.