dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.43k stars 10.02k forks source link

Problem providing Access Token to HttpClient in Interactive Server mode #52390

Closed RobJDavey closed 10 months ago

RobJDavey commented 11 months ago

Is there an existing issue for this?

Describe the bug

When making authenticated requests in InteractiveServer mode, a user's access token is required to talk to an external service. As the guidance is that it is not safe to use the IHttpContextAccessor when in server interactive mode, I have been following the documentation to try and add a token provider servicer.

I've followed the documentation guides below, however the access token on the token provider in interactive server mode always comes through as null. I'm unsure if I have misunderstood or missed something in these guides or if the guides do not currently lead to a complete solution.

The sample project I have attached tries two different approaches to get this access token.

The first is the inject the TokenProvider into the WeatherService, and grab the token from it there. This works fine when using server side rendering, but the token is null when in interactive server mode.

The second is to try use a circuit handler to set the correct services for the circuit, allowing it to access the TokenProvider from inside other services by injecting the CircuitServicesAccessor. The circuit handler however never seems to get the CreateInboundActivityHandler, and so I have been unable to test whether the TokenProvider it would provide would contain a null access token or not.

Expected Behavior

When a HttpClient request is made in InteractiveServer mode, the TokenProvider configured in the App component should be passed to the WeatherService class so it can be used to set the Authentication header. Alternatively, the AuthenticationStateHandler class should get the CircuitServicesAccessor set by the ServicesAccessorCircuitHandler, which can then be used to access the TokenProvider class to get the token.

Steps To Reproduce

I have created a sample solution which shows what I have attempted so far: https://github.com/RobJDavey/BlazorTokenIssue

The README explains how to run the solution. While there are 3 projects in the solution, 2 of them are purely there to support the demonstration, it's only the BlazorApp service that is at issue.

To authenticate, please user the either username and password alice/alice or bob/bob as these are the test users configured.

The SSR page always loads the data from the external service fine, however the Interactive Server page fails due to the missing token.

Exceptions (if any)

A 401 is returned by the service when no valid access token is attached.

System.Net.Http.HttpRequestException: Response status code does not indicate success: 401 (Unauthorized).
   at System.Net.Http.HttpResponseMessage.EnsureSuccessStatusCode()
   at BlazorApp.Services.WeatherForecastService.GetForecastAsync(Int32 skip, Int32 take) in ./BlazorApp/Services/WeatherForecastService.cs:line 34
   at BlazorApp.Components.Pages.WeatherInteractive.<OnInitialized>b__8_0(GridItemsProviderRequest`1 request) in ./BlazorApp/Components/Pages/WeatherInteractive.razor:line 47

.NET Version

8.0.100

Anything else?

cc: @guardrex https://github.com/dotnet/AspNetCore.Docs/issues/31113

gragra33 commented 11 months ago

Have you looked at using a DelegatingHandler to manage this? Here is a YouTube video by @m-jovanovic that shows you how: Middleware Pattern For HttpClient With Delegating Handlers.

RobJDavey commented 11 months ago

@gragra33 yes I do have a DelegatingHandler in the project: https://github.com/RobJDavey/BlazorTokenIssue/blob/main/BlazorApp/AuthenticationStateHandler.cs

This does not appear to get given the configured TokenProvider when running in interactive mode though.

Based on this guidance, as this handler could be called from an interactive context, it cannot just grab the token off the HttpContext which is why the TokenProvider approach appears to be necessary.

gragra33 commented 11 months ago

@RobJDavey based on your implementation TokenProvider will always have null values.

namespace BlazorApp.Services;

public class TokenProvider
{
    public string? AccessToken { get; set; }
    public string? RefreshToken { get; set; }
}
builder.Services.AddScoped<TokenProvider>();

You should have a TokenProviderFactory class to populate and return a TokenProvider, typically, values are stored in the appsettings.json. This is mentioned, in a more simpler way, in the above YouTube video. Here is the timestamped position in that video: Middleware Pattern For HttpClient With Delegating Handlers @ 3:10

Here is what your implementation should be:

public class TokenProviderFactory
{

    private IServiceProvider _serviceProvider;

    public TokenProviderFactory(IServiceProvider serviceProvider)
        => _serviceProvider = serviceProvider;

    public GenerateTokenProvider<TSettings>() where TSettings : IProviderSettings
    {
        var settings = _serviceProvider.GetRequiredService<IOptions<TSettings>>().Value;
        return new TokenProvider()
            {
                AccessToken  = settings.AccessToken;
                RefreshToken = settings.RefreshToken;
            };
    }
}

remove:

builder.Services.AddScoped<TokenProvider>();

and replace with:

builder.Services.AddScoped<TokenProviderFactory>();

now update:

var tokenProvider = _circuitServicesAccessor.Services?.GetRequiredService<TokenProviderFactory>().GenerateTokenProvider<GitHubSettings>();

*Note: the above code is typed freehand here and not tested but should work as-is. I am sure that there is enough there to help solve your issue.

RobJDavey commented 11 months ago

@gragra33 the token provider is not for client access tokens, but user tokens retrieved at login via OpenIDConnect, so they cannot be stored in settings.

The token provider is configured to have the current user's access token in the App component which is based on this guide.

This works fine in server side scenarios, but not in interactive.

gragra33 commented 11 months ago

@RobJDavey I have not run your code, so I did not dig that deeply. Now I have.

However, have you set breakpoints in:

  1. WeatherForecastService - when the TokenPrivder is being populated
  2. AuthenticationStateHandler - when the TokenPrivder is being used

Is the TokenProvider being set before being used in AuthenticationStateHandler?

I would also add a constructor in the TokenProvider, with a breakpoint, to check when it is being created and if, and when, it is occurring more than once.

RobJDavey commented 11 months ago

@gragra33 the construction of the TokenProvider is occurring twice, which is part of the issue.

The first time is for the server side rendered version of the page which is getting the access token set correctly, and is successfully making the HTTP call to the external service.

The second time is for the interactive client services, which as I understood it is what the circuit services accessor is supposed to help with. Maybe I have misunderstood what this is for. As this is creating a new version of the TokenProvider, rather than using the existing one that was populated in the App component, it provides a null access token and gets a 401 (Unauthorized) back from the external service. As I mentioned in the issue though, I am never seeing the CreateInboundActivityHandler method called so I may be doing something wrong here.

Ideally when the Blazor Hub receives a message I'd be able to just grab the token from the user's HttpContext, but the documentation makes it clear this is not safe to do in an interactive context. This is why I'm trying to follow the guide to get the TokenProvider working.

One approach I have tried which does seem to work is using the IHttpContextAccessor in the CircuitHandler.OnCircuitOpenedAsync to populate the TokenProvider, however I am not sure from the documentation about IHttpContextAccessor whether this is considered safe to do so ideally would like some clarification from someone very familiar with the Blazor hub if this is a safe thing to do or not.

gragra33 commented 11 months ago

@RobJDavey I noticed that @mkArtakMSFT tagged your issue with Doc. It made me curious, so I downloaded your solution and ran it.

I set a breakpoint in the OnInitializedAsync method in the App.razor component, OnInitialized method in the WeatherInteractive.razor component, and in the GetForecastAsync method in WeatherForecastService. I then navigated to the page. and observe the order of breakpoints being hit.

What I observed was that the component was created twice. The first time, your token was passed correctly, the OnInitialized was called, and the data was collected from your server via the WeatherForecastService. The second time, the OnInitialized was called, as it was a new instance, no token was passed to the new WeatherForecastService instance, and you obviously received a 401 response, as you should. This is because prerendering is enabled by default (more information on render modes here: ASP.NET Core Blazor render modes)

So when you navigate to the WeatherInteractive page, I am seeing the following sequence of breakpoints being hit:

App.razor                // new instance and sets token
WeatherInteractive.razor // initialised
WeatherForecastService   // set token is passed
WeatherInteractive.razor // initialised (again) / new instance
WeatherForecastService   // new instance and empty token is passed

This is happening when NavManager is used (via the NavLink component) and also with a regular anchor html element.

This does not happen with StreamRendering. So when you navigate to the WeatherSsr page, I am seeing the following sequence of breakpoints being hit:

App.razor              // new instance and sets token
WeatherSsr.razor       // initialised
WeatherForecastService // set token is passed

If I set the render mode to no prerender:

@rendermode @(new InteractiveServerRenderMode(prerender: false))

... then I am seeing the following sequence of breakpoints being hit:

App.razor                // new instance and sets token
WeatherInteractive.razor // initialised
WeatherForecastService   // new instance and empty token is passed

I have not isolated why this is happening with your code and there is no simple work-around that I can see. I am not sure if this is a bug or by-design. Someone from Microsoft will have to chime in.

wesleyscaldwell commented 11 months ago

@gragra33 the construction of the TokenProvider is occurring twice, which is part of the issue.

The first time is for the server side rendered version of the page which is getting the access token set correctly, and is successfully making the HTTP call to the external service.

The second time is for the interactive client services, which as I understood it is what the circuit services accessor is supposed to help with. Maybe I have misunderstood what this is for. As this is creating a new version of the TokenProvider, rather than using the existing one that was populated in the App component, it provides a null access token and gets a 401 (Unauthorized) back from the external service. As I mentioned in the issue though, I am never seeing the CreateInboundActivityHandler method called so I may be doing something wrong here.

Ideally when the Blazor Hub receives a message I'd be able to just grab the token from the user's HttpContext, but the documentation makes it clear this is not safe to do in an interactive context. This is why I'm trying to follow the guide to get the TokenProvider working.

One approach I have tried which does seem to work is using the IHttpContextAccessor in the CircuitHandler.OnCircuitOpenedAsync to populate the TokenProvider, however I am not sure from the documentation about IHttpContextAccessor whether this is considered safe to do so ideally would like some clarification from someone very familiar with the Blazor hub if this is a safe thing to do or not.

This is the exact issue I am having. And I am new to blazor, not new to asp.net or DI. but it also seems like a lot of this is very new in Blazor so finding information has been difficult. The result i was hoping for is that the scope of the prerender or the scope of the http request that runs through the middleware would be the same scope that is then used on the interactive rendering side. But that is not the case.

Currently, I'm thinking the only option I have is to register a singleton token class keyed on some value from the user or cookie to then retrieve the token instance based on that key value. But obviously any custom token handle carries security risks.

Let me know if y'all found any solutions to this.

gragra33 commented 11 months ago

@RobJDavey @wesleyscaldwell There is this recent blog post Per-User Blazor 8 State by @rockfordlhotka. I have not tried this, due to a recent emergency that we are still resolving, however this looks like a possible interim solution until Microsoft releases one.

wesleyscaldwell commented 11 months ago

I haven't read the full post but my understanding from the intro is that if I disable the prerender and use only interactive I should be good.

This is a start up project and I'm just trying to get it going. Once we have more usage I can focus on optimizations.

If this works I'll report back.

wesleyscaldwell commented 11 months ago

@gragra33 I tested this by disabling prerendering in hopes of a quick solution and all that did was disable the rendering portion, but the underlying DI scopes remain the same. The initial scope is created and return the UI and then the socket scope starts up without the context.

So the Per-User Blazor 8 State by @rockfordlhotka option is exactly what i was thinking, but he has clearly covered this in a much more extensive way.

But after reading it all, I find that when I'm this far down a rabbit hole on something that I have to imagine would be a common request, I have to think i'm missing something.

Is there any chance we have all missed some other option that makes it possible to share the scope between the prerender and the interactive session?

In the meantime, I am going to make use of what @rockfordlhotka has started on and store user properties in a singleton.

pockets3407 commented 11 months ago

I just implemented a simplified version of Per-User Blazor 8 State (I am not using wasm) and it seems to be working fine. I agree with @wesleyscaldwell that it feels like we are missing something about sharing scope between prerender and the interactive session.

wesleyscaldwell commented 11 months ago

I implemented a similar process. But I decided to add unique session id user claim on the OIDC OnUserInformationReceived Event. This then enables to me get the session from the session service as in the repo you referenced.

Having a unique session ID added by default would have saved a lot of the difficulty of this task.

My implementation is buried deep now in other code, but i can strip it out if anyone thinks it would be helpful to see.

halter73 commented 10 months ago

We'll look into how we can make it easier to retrieve saved access tokens from interactive server components in .NET 9, but I do think that reading the access token from the HttpContext using the IHttpContextAccessor is a reasonable approach for getting the access token during interactive server rendering. It won't update if the user authenticates after the circuit is established since the HttpContext is captured at the start of the SignalR connection, and its use of async local means that you have to be careful not to lose the execution context before reading it, but otherwise it should work fine.

I'll also look into updating the https://learn.microsoft.com/en-us/aspnet/core/blazor/security/server/additional-scenarios?view=aspnetcore-8.0#pass-tokens-to-a-server-side-blazor-app doc to use IHttpContextAccessor with caveats and/or use PersistentComponentState to transfer the token retrieved from the cascading HttpContext during static rendering to the interactively rendered components.

wesleyscaldwell commented 10 months ago

My opinion is that having a session token to be referenced between the signalr connection and the httpcontext would enable a lot of use cases beyond just the access token.

In my case I wanted the session ID for an authenticated user. But I could even see an instance where you want a session ID synced between the two for non authenticated users.

And I could also see the signalr connection starting before the authenticated context is available. But I might be recalling that you need to reload the UI after authenticating.

I am relatively new to blazor. So I be overlooking some basic details in my response.

rsandbach commented 10 months ago

@RobJDavey / @halter73

Is not being able access the TokenProvider instance created during prerender related to CreateInboundActivityHandler not being being dispatched (#51934) and (#52379)? If the override was being called, couldn't the same pattern used by AuthenticationStateProvider to get the AuthenticationStateProvider instance be used to get the initial TokenProvider instance?

rockfordlhotka commented 10 months ago

@RobJDavey / @halter73

Is not being able access the TokenProvider instance created during prerender related to CreateInboundActivityHandler not being being dispatched (#51934) and (#52379)? If the override was being called, couldn't the same pattern used by AuthenticationStateProvider to get the AuthenticationStateProvider instance be used to get the initial TokenProvider instance?

Right now AuthenticationStateProvider isn't available (or usable) when using server-rendering. A runtime exception is thrown when you try to use this service in that context.

halter73 commented 10 months ago

Right now AuthenticationStateProvider isn't available (or usable) when using server-rendering. A runtime exception is thrown when you try to use this service in that context.

If you're migrating from AddServerSideBlazor() to .AddRazorComponents().AddInteractiveServerComponents(), this is probably happening because .AddRazorComponents() doesn't call services.TryAddScoped<AuthenticationStateProvider, ServerAuthenticationStateProvider>() for you like AddServerSideBlazor() does.

You can just call builder.Services.TryAddScoped<AuthenticationStateProvider, ServerAuthenticationStateProvider>() yourself. ServerAuthenticationStateProvider is a public type. That's what the templates do for local auth and static rendering only. Although the templates use custom AuthenticationStateProviders for interactive scenarios so it can revalidate the security stamp while the SignalR circuit is active and transfer AuthenticationState to the WASM renderer.

https://github.com/dotnet/aspnetcore/blob/1ee90c23a8f97065ea9eb60cca5271c4ff3845f9/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWeb-CSharp/Program.cs#L39-L47

rockfordlhotka commented 10 months ago

@halter73 then there's something missing in the Blazor Web App template, because it does set up server-rendered, server-interactive, and wasm-interactive models. And in that template, if you try to access the AuthenticationStateProvider from server-rendered code you get a runtime exception saying that you can't use that type if you aren't in a Blazor interactive context.

My assumption, therefore, is that Microsoft intends for us to use HttpContext to access the current identity when in server-rendered mode, and AuthenticationStateProvider when in server-interactive or wasm-interactive.

halter73 commented 10 months ago

@rockfordlhotka Are you trying to access the AuthenticationStateProvider outside of a component? I'm confident "Microsoft" intends for you to use the AuthenticationStateProvider everywhere you need the ClaimsPrincipal inside a Blazor component, even during prerendering. If there's a bug in the Blazor web app template, please file a new issue with the full exception type, message and stack trace pointing to where in the template you see this runtime exception.

rockfordlhotka commented 10 months ago

Yes, like many (most?) people, I have assemblies that include business rules that depend on having access to the current user identity.

Putting such business logic directly in the UI is what has gotten us in trouble since VB3, and is a bad practice.

Until now, it has been possible to gain access to the current user from any code in an app - via the current thread, HttpContext, ApplicationStateProvider, or other means depending on which .NET environment is hosting the code.

This includes Blazor interactive modes, where AuthenticationStateProvider is a service that can be injected into any service to gain access to the user identity.

Right now, in Blazor 8, it appears that the solution (for server code) is to detect whether a circuit is active, so AuthenticationStateProvider is available, and to fall back to using HttpContext otherwise.

This is my prototype solution:

https://github.com/MarimerLLC/csla/blob/6d159d2d8bf1f515646caaac88971d053a3af3ba/Source/Csla.AspNetCore/Blazor/ApplicationContextManagerBlazor.cs#L69

ghost commented 10 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

mkArtakMSFT commented 10 months ago

Closing as a dupe of https://github.com/dotnet/aspnetcore/issues/52379