[Blazor][Server-side]Issue with individual auth and Azure Signalr Service

[javiercn]

The issue is the HttpContext is null when connected through the SignalR service.

We notify the client of the error in an asynchronous way, that's why we see the two error messages.

An exception is thrown trying to initialize the circuit.
The circuit never gets initialized.
We continue because we return without throwing from StartCircuit and then try to invoke EndInvokeJSFromDotNet with the circuit being initialized.
The changes that we added are good because we caught a real issue through the logs.

We should additionally do a couple of things.
Change StartCircuit to return a result type instead of the Circuit ID and to notify clients of the errors synchronously so that they stop processing calls.
On the client side, stop processing interop and other calls after we detect the first error.

/cc @SteveSandersonMS

[javiercn]

The HttpContext is null in the IHttpContextAccessor, the fix is likely to set it there.
We can do this ourselves, but I think the right fix is to do it at the Azure SignalR Service level.

/cc: @anurse

[analogrelay]

The HttpContext is null in the IHttpContextAccessor, the fix is likely to set it there.
We can do this ourselves, but I think the right fix is to do it at the Azure SignalR Service level.

There is no way to have an HttpContext when using Azure SignalR. There is no HTTP request.

[javiercn]

@anurse That's not technically true. There is a "fake" HttpContext that gets populated. I'm saying that context should be set in the IHttpContextAccessor. Without it, things that depend on that don't work when using the SignalR service, like Identity.

That's why I think it should be set there. Blazor could set it https://github.com/aspnet/AspNetCore/blob/master/src/Components/Server/src/ComponentHub.cs#L97 but it is not the right fix, as that only solves the problem for Blazor and not in general for Hubs using the Azure Signalr Service

[analogrelay]

The IHttpContextAccessor doesn't even function correctly in standard SignalR contexts IIRC since we don't execute Hub methods in the context of any HTTP request. Changing that behavior is a much bigger issue we'll have to discuss in depth.

[javiercn]

@anurse I'm not suggesting we change the behavior, I'm suggesting we keep it inline with the current expectations.

When I execute a method on a Hub, I expect the HttpContext to be the one that was there when we set up the connection.

I'm saying the answer should be the same when using the SignalR service. We already setup the user, the request url, etc in the signalr service, why wouldn't we set the fabricated HttpContext into the HttpContextAccessor for things to work transparently, to the extent they do when using the Azure SignalR Service?

[analogrelay]

The fact that IHttpContextAccessor works at all in non-Azure SignalR scenarios is questionable. For example, in Long Polling I'd anticipate IHttpContextAccessor.HttpContext would return a broken HttpContext much of the time since we don't replace it with our fabricated long-living HttpContext so it will be disposed after the first poll ends. About the only place you could see IHttpContextAccessor "work" is WebSockets.

That's why I think this is a larger issue than just setting IHttpContextAccessor. I think that setting it to whatever value is used in ConnectionContext makes sense, but there are a few things that will change.

In general, usage of HttpContext at all within SignalR is super rough. I wish we hadn't enabled it quite so easily since there is no correlation between when you Hub executes and any single HTTP request. Everything we've done with it so far is a leaky abstraction that falls apart when you poke at it. It creates a false sense that it's working. (cc @rynowak @davidfowl)

Anyway, what's the impact of this particular issue? Is this blocking a core Blazor scenario? If so, we need to work out how to unblock that with as little churn as possible since we're in ask-mode now.

It looks like you're calling in to Identity or Auth somewhere in the SignalR context and that's using IHttpContextAccessor? You're trying to read the User off the HttpContext associated with the initial request? Is it possible to read this off ConnectionContext.GetHttpContext().User?

[javiercn]

It’s inside an Identity abstraction. SignInManager that uses IHttpContextAccessor.

I’m not an expert on that area (@SteveSandersonMS is) so I don’t feel comfortable recommending an alternative solution.

Ultimately it’s likely we can set this on the ComponentHub but it’s not something I feel we should be doing.

I think it is one of our services in DI that uses the SignInManager and that in turn uses the IHttpContextAccessor, so I’m not sure there’s anything we can do.

[rynowak]

We should try to understand if there's something more granular than sign-in manager we should be using. If it's coupled to the idea of request/response, it's probably bad.

[SteveSandersonMS]

In Preview 8 in most cases (identity being the exception) we get the principal from HubContext. That will work ok with SignalR service, right?

One of the outstanding P9 issues is to use the same mechanism in the Identity template too, not SignInManager.

[analogrelay]

In Preview 8 in most cases (identity being the exception) we get the principal from HubContext. That will work ok with SignalR service, right?

Correct. The Principal on HubContext is fine. We intentionally set that up appropriately. It's IHttpContextAccessor that is super-borken

[analogrelay]

Right now, IHttpContextAccessor works "OK" on WebSockets and SSE because the Hub is executed in the context of the long-running SSE/WS request. In Azure SignalR (ASRS) and Long Polling, that isn't the case. Moving forward, as we consider transparent reconnect, all transports would manifest this problem.

[analogrelay]

We should try to understand if there's something more granular than sign-in manager we should be using. If it's coupled to the idea of request/response, it's probably bad

Agreed. If your goal is to read the connection's active user, that's what ConnectionContext.User is for. We could have a IConnectionContextAccessor I gueeeeeesssss (he says very tentatively)

[javiercn]

@danroth27 Should we just punt this to preview9 (seems like @SteveSandersonMS has a fix in mind) and create an item in the known issues for preview8?

[mkArtakMSFT]

We've discussed this and it seems this will be resolved as part of #12692.
In the meantime, customers can disable re-validation by removing the RevalidatingAuthenticationStateProvider registration from the Startup.ConfigureServices method. This will bring them to the same place as before Preview7 (no re-validation).