Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of HttpContextAccessor in Blazor Interactive Server(?) #258

Open
nwoolls opened this issue Apr 3, 2024 · 9 comments
Open

Use of HttpContextAccessor in Blazor Interactive Server(?) #258

nwoolls opened this issue Apr 3, 2024 · 9 comments
Assignees
Labels
Not triaged Awaiting review

Comments

@nwoolls
Copy link
Contributor

nwoolls commented Apr 3, 2024

Describe the issue

From what I've read, using HttpContext and HttpContextAccessor via Dependency Injection with Blazor Interactive Server is inadvisable, as the HttpContext used when serving HTML is not the same as the one used by Web Sockets / SignalR. There are numerous posts, GitHub issues, and threads on this:

However, the OIDC BFF sample in this repo is using IHttpContextAccessor to get the access token.

To Reproduce

Steps to reproduce the behavior:

  1. Attempt to follow best practices while adopting the OIDC BFF sample contained in this repo.

Expected behavior

No use of IHttpContextAccessor - instead using circuit handlers. e.g.:

Additional context

This is all pretty new to me. I'm working through some different POCs for Interactive Server, Auto Render Mode, etc. I could very much be mistaken above. But in my work on the Interactive Server POC, I used (had to use?) circuit handlers and not IHttpContextAccessor. So I was confused to see it in the BFF sample in this repo. Any advise would be appreciated.


Issue Details

Do not edit this section. It is required for issue processing.

@nwoolls nwoolls added the Not triaged Awaiting review label Apr 3, 2024
@guardrex
Copy link
Collaborator

guardrex commented Apr 3, 2024

Hello @nwoolls ... It's not used for interactively-rendered components, only static SSR components or statically-rendered root components (e.g., App component), so it's safe. We have some guidance on that aspect that appears in two places three places (it's the same INCLUDE file used in both spots) ...

... and in the spot that you cross-linked.

@guardrex guardrex closed this as completed Apr 3, 2024
@guardrex
Copy link
Collaborator

guardrex commented Apr 3, 2024

I'm going to open an issue to explain that it's ok in this case for these sample apps.

@nwoolls
Copy link
Contributor Author

nwoolls commented Apr 3, 2024

Sorry if the line of question is a basic one - I'm coming from MVC and this all seems a bit more complicated.

I get that HttpContext is safe in static rendering and in e.g. the App.razor component as that is where I used it alone with circuit handlers to get access to an access token in a delegating handler.

In this case of the OIDC BFF sample, my understanding is that this uses the new Auto Render Mode, meaning that WASM is used once downloaded, and until then Interactive Server (not Static Server / SSR, which would be pre-rendering) is used.

If that is the case, wouldn't injecting IWeatherForcaster into Weather.razor, during the initial download, function in Interactive Server mode, thus using ServerWeatherForecaster from SignalR? I think I must be missing a key aspect of this because the OIDC BFF sample does seem to function if I change App.razor to use @rendermode="InteractiveServer".

Thanks in advance for the feedback!

@guardrex
Copy link
Collaborator

guardrex commented Apr 3, 2024

The HttpContext isn't being used in that case. Just because the app has the registered services for HttpContext doesn't mean that they're used because the app adopts an interactive render mode. If you try and consume the service in an interactively-rendered component, that's where things go wrong 😈🐉. In this case, the context isn't used directly by any component that's rendering interactively, so it's all good.

... and keep in mind that when you set the render mode there in the App component, you aren't actually making the App component itself interactive. It can't be interactive. It's assigning it to the Routes and HeadOutlet components, and it goes downstream from there via inheritance.

@nwoolls
Copy link
Contributor Author

nwoolls commented Apr 3, 2024

In this case, the context isn't used directly by any component that's rendering interactively, so it's all good.

I guess I'm not clear on why the HttpContext isn't being used in Interactive Server mode here.

I've updated App.razor so that the various other pages & components use Interactive Server mode. I can see that this is the case as the Web Socket is being used and I have interactivity.

The service - in this case ServerWeatherForecaster - is being consumed by a page, Weather.razor, that is running in Interactive Server mode.

I must be misunderstanding something above as it seems to me like HttpContext is being used by an interactively rendered component.

@bpsc-wkubis
Copy link

bpsc-wkubis commented Apr 18, 2024

Hey @nwoolls I find that very confusing too, did you find any thread that explains this concept more deeply?

@guardrex
Copy link
Collaborator

My bad! Sorry ... I was only thinking in terms of server-side static SSR, probably focusing on security scenarios. I didn't realize (or LOOK 👀🙈) at the ServerWeatherForecaster when you directly asked about it.

https://github.com/dotnet/blazor-samples/blob/main/8.0/BlazorWebAppOidcBff/BlazorWebAppOidc/ServerWeatherForecaster.cs

We need help from @halter73 to explain ...

@halter73, the ServerWeatherForecaster is obtaining an HttpContext from IHttpContextAccessor. Our guidance is NOT to do that for interactive SSR. What's the situation with it here? Why is it ok in this case?

@guardrex guardrex reopened this Apr 18, 2024
@guardrex
Copy link
Collaborator

I still don't have an answer on this. I'll try pinging Stephen offline again.

@guardrex
Copy link
Collaborator

guardrex commented Apr 29, 2024

Ok --- I actually messaged Dan and Artak on this one. I think they'll know how the 🧀 moved. The updates to the INCLUDE to address the use of IHttpContextAccessor went in at the height of the doc churn to address 8.0 BWA content. It's possible that Steve Sanderson's work in RC2 for using HttpContext as a cascading parameter in statically-rendered root components was updated correctly, but IHttpContextAccessor guidance wasn't updated correctly. They'll probably spot that fairly quickly, and I'll get an update rushed into the documents quickly 🏃‍♂️ to address it. Stand-by ........................................

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not triaged Awaiting review
Projects
None yet
Development

No branches or pull requests

3 participants