Skip to content

Conversation

@hakenr
Copy link
Member

@hakenr hakenr commented Nov 7, 2024

This entire section seems more confusing than helpful:

Don't attempt to resolve AuthenticationStateProvider within a custom scope because it results in the creation of a new instance of the AuthenticationStateProvider that isn't correctly initialized.

What exactly do we mean by "custom scope" here? The article is focused on server-side Blazor, where the default scope is the circuit. Any additional scope, such as OwningComponentBase, is simply a nested scope and will resolve the correct AuthenticationStateProvider instance.

Yes, we could construct an edge case - like using a BackgroundService in the same process, where there is no circuit scope available. But in such scenarios, there's no user identity available for that service workload anyway, and passing an AuthenticationStateProvider from a component to a background service method doesn't make sense.

ExampleService.cs

Register the service as scoped. In a server-side Blazor app, scoped services have a lifetime equal to the duration of the client connection [circuit].

Firstly, the service itself is stateless (i.e., it has no instance data fields), which makes the choice of lifetime irrelevant.

Secondly, if both the service and AuthenticationStateProvider have lifetimes tied to the client connection circuit, why are we passing AuthenticationStateProvider as a method parameter? It seems unnecessary if they share the same scope.

Thirdly, in the example below (InjectAuthStateProvider.razor), a component scope is used to resolve ExampleService, which contradicts the earlier statements about circuit-scoped services.

Overall, this section introduces too much confusion and should probably be removed. The basic principles of scoped services and OwningComponentBase are already well-explained in the Dependency Injection documentation, and they apply just as well to AuthenticationStateProvider.

If there's a specific behavior of AuthenticationStateProvider that needs to be covered here, we should rewrite the section with clear examples that illustrate this behavior effectively.


Internal previews

📄 File 🔗 Preview link
aspnetcore/blazor/security/server/index.md Secure ASP.NET Core server-side Blazor apps

…vices scoped to a component - section removal
@hakenr hakenr requested a review from guardrex as a code owner November 7, 2024 00:48
@guardrex
Copy link
Collaborator

guardrex commented Nov 7, 2024

This entire section seems more confusing than helpful:

Actually, the entire Blazor Security and Identity node is due for a major overhaul. However, it won't be possible this year. It will likely occur in 25Q1.

This section came in from the product unit for publication, so we'll get a technical analysis from them on your remarks and desire to remove it. However, we have the .NET 9 release next week, and I know for sure that they can't look at this right now. I'll mention this to Artak on Friday, and he'll likely place it on his schedule for a look ASAP.

@guardrex guardrex self-assigned this Nov 7, 2024
@guardrex
Copy link
Collaborator

guardrex commented Nov 8, 2024

Took a closer look at this. Unfortunately, I only see where I brought it in and a PU discussion where I mentioned it. I don't see the exact source.

What exactly do we mean by "custom scope" here?

It probably should read "service scoped to a component" there. This section is telling devs not to inject an AuthenticationStateProvider directly into such a service. One should inject it into the component that uses the service, and then they can pass it as the section describes.

This might be an Javier example. Sometimes, he posts them in a personal repository, and I pull them from there. That might explain why I'm having a hard time tracking down the original code example. I'm going to go ahead and shoot him an email and ask if he has time to put an 👁️ on it right now.

@guardrex
Copy link
Collaborator

Moving to #34134 for work on the section.

@guardrex guardrex closed this Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants