Security implications of using IHttpContextAccessor in Blazor Server

[Bouke]

(I originally posted this question on the Docs repository, but I was told to post it here instead.)

The documentation note the following on IHttpContextAccessor:

Additionally, again for security reasons, you must not use IHttpContextAccessor within Blazor apps. Blazor apps run outside of the context of the ASP.NET Core pipeline. The HttpContext isn't guaranteed to be available within the IHttpContextAccessor, nor is it guaranteed to be holding the context that started the Blazor app.

The default HttpContextAccessor is implemented using AsyncLocal:

https://github.com/dotnet/aspnetcore/blob/589aa11b5c631ce719e0530d66be8324a6d79169/src/Http/Http/src/HttpContextAccessor.cs#LL11C108-L11C108

I fail to understand the security implications of using IHttpContextAccessor in Blazor Server. AsyncLocal in general must flow with the user's SignalR connection†. So that would also be the case for the HttpContext stored in this AsyncLocal. This is also what we're seeing in our tests.

• Under what situations would IHttpContextAccessor return a different HttpContext than the one that started the Blazor app? (i.e. another user)
• How is this use of AsyncLocal any different from other uses of AsyncLocal and (apparently) a security consideration?
• How can we pass the originating HttpContext from the "context of the ASP.NET Core pipeline" safely to the Blazor Server app?

† If that isn't the case, then all bets are off.

cc: @guardrex dotnet/AspNetCore.Docs#27944

[guardrex]

Thanks, @Bouke. I just found the following open issue to work further on this subject in SignalR docs. It wasn't apparent to me at first because it's labeled for the SignalR docs, not the Blazor docs and not tracked by the Blazor project. It might provide some helpful info to you while you wait for a response here ...

dotnet/AspNetCore.Docs#14974

Javier ... when you see this ... let me know if you'd like any additional guidance added to that bit in the Blazor doc. 👂

Also, the issue ☝️ has been languishing since 2019 without an assignee (but it is on Brady's master list of things to do). What I think will happen is that this will get covered in the SignalR docs. I'll cross-link to the new coverage from relevant Blazor-node docs. If that's not the plan, let me know what you'd like to do. 👂

[davidfowl]

I believe the issue is that Blazor server circuits survive across multiple SignalR connections. This is the same reason why you have to get the HttpContext within SignalR using HubConnectionContext.GetHttpContext(). The accessor doesn't work well because SignalR connection can persist across multiple http requests. In the case of Blazor server, the circuit is the source of truth and that can also be handled across multiple http requests and SignalR connections. What you don't want to happen is for the user captured in a service to be the wrong one because the user context changed after a reconnect but your code didn't notice.

The IHttpContextAccessor can be made to "work" somewhat by either:

• Nulling it out so that you see null within these contexts when you access the HttpContext
• Trying to make it work whenever we update a view of the HttpContext in these frameworks. Though its unclear to me where the framework would do this.

[Bouke]

So the security implication being referred to in the documentation would be the following:

• User A starts the Blazor application, and thus their user is captured in HttpContext.User
• From within the Blazor application User A is signed out and User B is signed in (all the while maintaining their SignalR connection)
• Now HttpContext.User still refers to User A?
  Meaning that if my Blazor application doesn't handle sign-ins, is there still a security implication that I need to consider?

So coming back to the circuit being the source of truth, I would still expect AsyncLocal to correctly flow; is this not the case?

What you don't want to happen is for the user captured in a service to be the wrong one because the user context changed after a reconnect but your code didn't notice.

Can you clarify what you mean by this, is this the scenario that I described above?

---

For some background: I have a design where my business layer receives an IUserProvider that has a sole method IPrincipal GetUser(). Depending on the type of application, I would inject in web contexts a HttpContextUserProvider that relies on IHttpContextAccessor, or in non-web contexts a ThreadUserProvider that uses Thread.CurrentPrincipal. The advertised way to get a user in Blazor is with AuthenticationStateProvider, but I cannot plug that into my existing design. I need to register something with the DI that isn't async and works both in MVC and Blazor. In my testing IHttpContextAccessor appears to be working fine, except the very first render following a hot reload, which is how I came to further investigate IHttpContextAccessor's trade-offs.

[davidfowl]

From within the Blazor application User A is signed out and User B is signed in (all the while maintaining their SignalR connection)

Right, this is the same reason why it's unsafe to do use the IHttpContextAccessor in SignalR. The user attached to the Blazor circuit is updated but the IHttpContextAccessor never gets updated again.

Now HttpContext.User still refers to User A?
Meaning that if my Blazor application doesn't handle sign-ins, is there still a security implication that I need to consider?

Right, the async local captured by the initial SignalR connection is what will flow to the blazor circuit and subsequent requests, or connections will not update the async local you captured previously.

PS: I wrote a things that can go bad with async locals article.

In my testing IHttpContextAccessor appears to be working fine, except the very first render following a hot reload, which is how I came to further investigate IHttpContextAccessor's trade-offs.

Try the scenario you listen above. If it's "working", it's definitely by chance and not intended 😄 (which means it's not really working). The HttpContext pointed to by the accessor gets disposed/reset after the request is over. There are many IHttpContextAccesor.HttpContext instances over the lifetime of the circuit, so it's very unsafe in that sense.

I have a design where my business layer receives an IUserProvider that has a sole method IPrincipal GetUser()

To make it safe, it needs to be updated with the "right" instance of the HttpContext at the appropriate time (I haven't spent much time trying to understand what that is). Maybe you can update it in a circuit handler or in a hub filter.

BTW if you just need the user an not the HttpContext, its cleaner to set Thread.CurrentPrincipal in all the right places.

[guardrex]

I'm going to expand what we have in the Blazor Threat Mitigation doc and probably add a bit on this in the Blazor-SignalR Fundamentals doc.

There's a bit on this in the HttpContext doc. The best content to cross-link will be whatever is placed for dotnet/AspNetCore.Docs#14974. I'll cross-link to whatever they place.

[javiercn]

All that @davidfowl mentions here is correct.

I have a design where my business layer receives an IUserProvider that has a sole method IPrincipal GetUser()

Use a CircuitHandler capture the User there from the AuthenticationStateProvider and set that user in your service. If you want to get updates to the user, register a callback to AuthenticationStateChanged and queue a Task to get the new user and update your service.

[ghost]

Hi @Bouke. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[davidfowl]

@javiercn I think we should write a full example.

[javiercn]

@davidfowl do you mean for what I suggested?

[davidfowl]

yep

[javiercn]

@davidfowl @Bouke here is a sample.

javiercn/BlazorServerCaptureUser@2e34b74

[davidfowl]

@javiercn to make it a bit more realistic. The current user service needs to work for both MVC and Blazor server in the same application and the user service can't be coupled to ASP.NET Core at all (if I understand correctly).

[javiercn]

@davidfowl that would be no problem, you could set the user inside an action filter in MVC and it will work there too. There is no coupling to ASP.NET Core at all.

[davidfowl]

@Bouke does this example help? Can you show an example of your user service definition?

[Bouke]

Thank you all so far for your investigation and feedback. The suggested solution doesn't cover ASP.NET Core, so further work would be needed in order to implement the IPrincipalProvider I mentioned before. That implementation should work regardless of being used in the ASP.NET Core or Blazor context. See also #3 below. Some questions/remarks:

1. How is @javiercn's solution preferable over just setting Thread.CurrentPrincipal from inside OnCircuitOpenedAsync -- update: Thread.CurrentPrincipal will be null following a hot reload; I don't know if there are any hooks available to restore state?

2. Making the user available to ASP.NET Core (both MVC + Razor Pages + ...) could work something like this? Note that this doesn't handle any changes to the current user during the request (e.g. sign-in/sign-out).

   app.UseMiddleware<UserServiceMiddleware>(); // after app.UseAuthorization();
   
   public class UserServiceMiddleware
   {
       private readonly RequestDelegate next;
   
       public UserServiceMiddleware(RequestDelegate next)
       {
           this.next = next ?? throw new ArgumentNullException(nameof(next));
       }
   
       public async Task InvokeAsync(HttpContext context, UserService service)
       {
           service.SetUser(context.User);
           await next(context);
       }
   }

3. My testing inside your demo application works. However in my application I'm using SimpleInjector as my DI container and I have some issues with getting the same scoped instance of UserService back. So the middleware sets the user on scoped instance 1, but resolving uses scoped instance 2. This is unrelated to this issue, but something I need to resolve before I can further investigate the suggested approach.

During the holiday season I might not be able to respond in a timely matter. Happ 🎄 ☃️!

[javiercn]

1. How is @javiercn's solution preferable over just setting Thread.CurrentPrincipal from inside OnCircuitOpenedAsync -- update: Thread.CurrentPrincipal will be null following a hot reload; I don't know if there are any hooks available to restore state?

You might end up with a User for a different circuit if you use Thread.CurrentPrincipal. Blazor circuits outlive AsyncLocals and ThreadStatics. Same reason why in principle at least originally you could not use Thread.CurrentPrincipal with async (although this might have changed).

2. Making the user available to ASP.NET Core (both MVC + Razor Pages + ...) could work something like this? Note that this doesn't handle any changes to the current user during the request (e.g. sign-in/sign-out).

Using a middleware will work for anything that is based on request/response. However, note that Blazor Server apps are not tied to a single websocket connection or HTTP request, so this won't work there.

3. My testing inside your demo application works. However in my application I'm using SimpleInjector as my DI container and I have some issues with getting the same scoped instance of UserService back. So the middleware sets the user on scoped instance 1, but resolving uses scoped instance 2. This is unrelated to this issue, but something I need to resolve before I can further investigate the suggested approach.

For clarity, Blazor Server creates its own scope, so do not expect that you are able to get the same service on a regular HTTP request and inside of a circuit. You would need a different solution for that. As with regards to SimpleInjector, I don't know about it, so we can't help there.

[Bouke]

I don't think I follow. The UserService needs to work in both Blazor and ASP.NET Core context:

@javiercn to make it a bit more realistic. The current user service needs to work for both MVC and Blazor server in the same application and the user service can't be coupled to ASP.NET Core at all (if I understand correctly).

However the provided example doesn't work in ASP.NET Core, as SetUser() isn't called on any path. So my suggested fix for this is to actually call SetUser() from a middleware, but your response to this is "this won't work here". So what would an actual UserService look like then?

In the end I want a UserService that can return the current user. My application is using ASP.NET Core for authentication, so inherently coupled to the cookies of the original request. The user is not going to change during the application's (circuit) lifetime, other than maybe ending the circuit when the user logged out in a different tab (although I currently have no idea how that will work).

I think integrating into ASP.NET Core like this is a rather common scenario, it is even documented here:

The built-in AuthenticationStateProvider service for Blazor Server apps obtains authentication state data from ASP.NET Core's HttpContext.User. This is how authentication state integrates with existing ASP.NET Core authentication mechanisms.

As we've established, the HttpContext of the original request is disposed after starting Blazor. So once the user is copied HttpContext.User into AuthenticationStateProvider, I don't see how the current user could change after all. So how does that tie into the bigger context of this issue?

• I cannot use IHttpContextAccessor because the original context will be disposed at some point.
• The default AuthenticationStateProvider doesn't (?) implement changing the current user
• So using a fixed user for the duration of the circuit would be fine
• Given these constraints: how can I resolve the current user in my UserService (in both ASP.NET Core + Blazor)?

[davidfowl]

• The UserService should be a scoped service instead with a field that isn't async local based.
• It should be set in middleware after the authentication middleware runs, or it could be set with an IClaimsTransformation implementation.
• For Blazor server, use a circuit handler implementation to set the current user (much what Javier showed in his example)

This should work in both cases since there are 2 different scopes, so you'll get 2 different instances of the service.

If you need to access the current user from a singleton service, then this won't work.

[javiercn]

However the provided example doesn't work in ASP.NET Core, as SetUser() isn't called on any path. So my suggested fix for this is to actually call SetUser() from a middleware, but your response to this is "this won't work here". So what would an actual UserService look like then?

I leave that as an exercise to you. In the end it comes down to registering a scoped service and setting the Value for it to the current user in the appropriate context before your application logic runs. That happens as described:

• A circuit handler for Blazor as the sample demonstrates.
• Your own middleware for MVC, etc. as @davidfowl suggests.

• I cannot use IHttpContextAccessor because the original context will be disposed at some point.
• The default AuthenticationStateProvider doesn't (?) implement changing the current user
• So using a fixed user for the duration of the circuit would be fine
• Given these constraints: how can I resolve the current user in my UserService (in both ASP.NET Core + Blazor)?

If you do not need to update the user after the app has started, the only code you need is inside OnConnectionUpAsync:

public override async Task OnConnectionUpAsync(Circuit circuit, CancellationToken cancellationToken)
{
    // This gets called every time the circuit reconnects, setting the user for the lifetime of the connection.
    // Unless you implement updates.
    var state = await _authenticationStateProvider.GetAuthenticationStateAsync();
    _userService.SetUser(state.User);
}

[ghost]

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

[davidfowl]

I'd like @Bouke to confirm this works (so we can document it). Let's leave this open until after the holidays.

[ghost]

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

[ghost]

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

[ghost]

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

[ghost]

Hi @Bouke. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[ghost]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.