ClaimsPrincipal.Current does not return the ClaimsPrincipal of the caller.

[brentschmaltz]

In previous runtimes ClaimsPrincipal.Current returned the ClaimsPrincipal that was created by the authentication layer. This provided a consistent programming model.

[kevinchalet]

/cc @davidfowl, as we chatted about this specific feature on JabbR.

[vibronet]

This is a key feature. Developers should be able to find the identity of the caller in the same property regardless of host type and past/present/future Dev stack. The manipulation of claims should not be coupled to how the claims themselves were obtained, which is the promise that .net made from 4.5 onward.

[davidfowl]

@vibronet Totally agree its just the implementation in .NET 4.5 doesn't have everything required to make this work properly.

[leastprivilege]

Really? Are we reviving that?

I thought you gonna deprecate global mutable statics in favour of modern techniques like DI and better testability.

Are you also planning to bring HttpContext.Current back? :p

[davidfowl]

This is about compatibility on .NET 4.x. It's also not going to work on .NET core AFAIK. I generally hate the statics but its the pattern the identity team has been pushing. There's actually a problem with the implementation in NET 4.x that makes it hard to implement this properly thats why it's not working right now.

HttpContext.Current isn't coming back as a static 😄

[leastprivilege]

Right - as you are saying -

• it does not work reliably right now.
• It won't go forward in .NET Core.
• It is bad style

So why care at all in ASP.NET 5?

[brentschmaltz]

@leastprivilege can you be specific as to what doesn't work reliably right now?
@davidfowl there is a solution that would work in most scenarios
Let's separate this into parts:

• ClaimsPrincipal.Current
  • The Identity Team is encouraging ClaimsPrincipal.Current which worked fine with previous runtimes.
• Implementation
  • The Identity Team is not pushing statics or any other implementation.
    Currently HttpContext.User returns the Identity that the AuthenticationLayer and runtime have created. It is possible that ClaimsPrincipal.Current could return the same value.
• Going forward
  • We are going to suggest changes in .Net to improve the feature.

[leastprivilege]

Thread.CurrentPrincipal (and thus ClaimsPrincipal.Current) comes from a time where

• people did not care about unit tests and global static mutables
• where the mental model was that a principal is assigned/attached to one thread of execution

Both has clearly changed.

There have been a couple of of inconsistencies with using Thread.CurrentPrincipal and async/await in the past - e.g. with russian doll style pipelines (e.g. Web API and I guess Katana and ASP.NET 5 have the same issue).

When e.g. one piece in the pipeline sets Thread.CurrentPrincipal on the way in - on the way out plumbing that runs "later" then the code that set the principal in the first place does not see the right principal due to async/await cleaning up the call stack. This happened in Web API with a message handler that set the principal on the way in - but the media type formatter ran much later on the way out.

I do remember other inconsistencies with Web API and the Katana self host where after creating a bunch of threads the current principal was not in sync anymore with the Web API request context. Generally using the request context in Web API was the "officially" recommended way of accessing the current principal.

Syncing and propagating the principal to new threads seems complex and error prone. That combined with statics makes this approach a thing of the past. ASP.NET 5 is a thing of the future.

my 2c.

[damianh]

Mutable static state 🔫

Sure, may as well bring in CommonServiceLocator while we're at it.

Sorry for the sarcastic tone but I strongly feel this is a Bad Idea.

[brentschmaltz]

@leastprivilege Interesting observations, I agree with most of them.
@damianh The identity of the caller is fixed one authentication is complete w.r.t. a call.

Some history.
Back in 2010 when we started the move of putting ClaimsPrincipal in .Net 4.5. I argued that Thread.CurrentPrincipal was not the right model. So I invented ClaimsPrincipal.Current that contains a settable delegate, ClaimsPrincipalSelector, that will return the CurrentPrincipal. ClaimsPrincipal.Current -> ClaimsPrincipalSelector -> CP. I was trying to prepare for a world post Thread.CurrentPrincipal, which seemed too Windowish.

Since at the time the runtimes of interest were WCF and Asp.Net, returning Thread.CurrentPrincipal seemed the right default. It is and was my belief that the runtime that is responsible for creating the ClaimsPrincipal should be responsible for crafting the ClaimsPrincipalSelector.

So conceptually, I think we are on the same page. In multi-threaded, async, apppool, queued apps, there has to be a deterministic model for mapping back to context that the call arrived. Cannot ClaimsPrincipalSelector map back to the correct CP.

@lodejard suggested perhaps we are missing the API: CurrentPrincipal {set;}

Any ideas you have to arrive at the correct API are welcome.

[leastprivilege]

ASP.NET 5 is DI enabled all over - the current principal should be injectable.

[markrendle]

Is it even possible to create a static ClaimsPrincipalSelector which is able to extract the "current" ClaimsPrincipal from the active HttpContext, given that there is no HttpContext.Current.

Consistent programming models are all very well and good when they make sense in all the contexts across which they are consistent, but to expect all models to be applicable across all the contexts in which C# and .NET can be used is unrealistic.

[HaoK]

Well the IHttpContextAccessor service already basically provides this via context.User

[brentschmaltz]

@HaoK the idea was to create a selector the mimic'd IHttpContextAccessor.
@markrendle there has to be a way to get to the 'context' that represents the associated inbound message. Otherwise how would IsInRole("admin") work?

[Eilon]

We will revisit this when an appropriate facility exists to set the current thread's principal.

[StevenVandenBroeck]

Any news on this ?
Is there already something that can be injected and contains the current principal ?

[davidfowl]

You can get it from the IHttpContextAccessor

[StevenVandenBroeck]

Thanks, that works.
Will this be the only way in the future or will there be a more generic object (like IPrincipalContext or something) ? It feels kinda strange having to inject a http-ish object in my business classes.

[davidfowl]

You should pass in the ClaimsPrincipal directly then 😄

[forforeach]

What are the current best practices to get the identity of a caller in the business logic layers of an aspcore application? It seems strange to pass around the ClaimsPrincipal.

[leastprivilege]

Why does that feel strange? It's an input to your logic, right? Making the identity an input parameter (as everything else) is the only sane way to test your code.

[fredgate]

I agree that ClaimsPrincipal should be injected in the business layer, even if ThreadPrincipal did not change much to write tests.
So how to configure the IoC container. I had the below code (with Autofac) :

var builder = new ContainerBuilder();
builder.Register(c => c.Resolve<IHttpContextAccessor>().HttpContext.User).As<ClaimsPrincipal>();

It worked fine. But since I migrated to Asp.net Core MVC 1.0.1, this fails with the exception

The requested service 'Microsoft.AspNetCore.Http.IHttpContextAccessor' has not been registered. To avoid this exception, either register a component to provide the service, check for service registration using IsRegistered(), or use the ResolveOptional() method to resolve an optional dependency.

What need to be done to have IHttpContextAccessor available in the container ?

[dougbu]

What need to be done to have IHttpContextAccessor available in the container ?

Add the following to your Startup.ConfigureServices() method:

services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();

[nathannoble]

@davidfowl, @dougbu That worked. Thanks!

[barynov]

It seems strange to pass around the ClaimsPrincipal

I agree. I used Web API 2 on my previous project and it was quite convenient to have identity as an ambient dependency. I used asyncs a lot and did not experience mentioned problems with identity loss. On a new project I tried to apply the same pattern using ASP.NET Core and faced this issue. Yes, it is certainly possible to pass the identity as a parameter to the core, but that would only pollute the layer contract. Internally the identity is passed explicitly to sub-components when needed but this is hidden from core users.

[barynov]

After all, Thread.CurrentPrincipal already exists in BCL, it is worse not to keep it up to date. Those, who decide to pass the principal explicitly, can do that anyway.

[davidfowl]

@barynov then inject it as a dependency into your components.

[barynov]

@davidfowl I ended up with an action filter that sets Thread.CurrentPrincipal correctly.

[bm2yogi]

@nathannoble @dougbu @davidfowl

Add the following to your Startup.ConfigureServices() method:

services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();

Is there any concern that the HttpContext created by the HttpContextAccessor might also be a singleton, or is the HttpContext certain to be scoped to each request?

[Tratcher]

HttpContextAccessor does not create the HttpContext, it only stores the current one for a given request.

[jonas-stjernquist]

@forforeach

What are the current best practices to get the identity of a caller in the business logic layers of an aspcore application? It seems strange to pass around the ClaimsPrincipal.

AFAIK the best practice is to use HttpContext.User in the BLL layer, which also can be used in Controller classes if you need to apply the same kind of logic outside of your BLL layer.
In a frontend razor page you have the nifty User property of the RazorPage class.

[martino01]

Could the aspnet.core framework not register an HttpContext instance into the request scope when it is created? The request scope seems to me the ideal place to register instances (singletons) that are ambient to the request handling logic. The request scope guarantees the instance is per request, and all request processing logic can access it simply through DI. IMHO using a singleton with AsyncLocal smells a lot like global mutable state.