IdentityServer / IdentityServer4.AccessTokenValidation

IdentityServer Access Token Validation for ASP.NET Core
Apache License 2.0
544 stars 214 forks source link

Support interop with ASP.NET Core 2.1 HttpClientFactory #105

Closed martincostello closed 5 years ago

martincostello commented 6 years ago

TL;DR - Please make IdentityServer4.AccessTokenValidation DI-aware for HttpClient usage.

Details

I'm updating an application targeting ASP.NET Core 2.0 to ASP.NET Core 2.1. As part of this, I'm integrating HttpClientFactory into the application when creating HttpClient instances as it allows delegating message handlers etc. to be easily setup for HTTP calls.

In the 2.0 and 2.1 implementations I intercept HTTP calls to return canned-responses for in-memory testing of dependencies. As part of the 2.0 implementation I use the JwtBackChannelHandler property to provide an HttpMessageHandler implementation for use by IdentityServer so that I can return an in-memory generated configuration for JWTs, rather than need to host an endpoint to do so.

However as part of moving this HTTP-related code to use HttpClientFactory for 2.1, I've found an issue where the two implementations don't easily interop with each other.

HttpClientFactory works at the HttpClient level and provides no way to return an HttpMessageHandler that leverages all the other benefits of the factory, but IdentityServer4.AccessTokenValidation provides extensibility at the HttpMessageHandler level and creates its own HttpClient instances.

I tried wrapping the HttpClient from HttpClientFactory with a custom HttpMessageHandler implementation, but due to HttpClient being used internally, this fails as the HttpRequestMessage is marked internally as sent before the wrapped code is invoked, and the operation fails.

I've raised a separate issue in the HttpClientFactory repo (aspnet/HttpClientFactory#110) about a future release exposing the ability to access message handlers as well, but making IdentityServer4.AccessTokenValidation support DI-awareness/injection of HttpClient somehow would make this scenario easier to achieve.

As a workaround for now, I'm creating an HttpClient through the factory, extracting the inner message handler from its private field using reflection, and then assigning that as the value of JwtBackChannelHandler, as the implementation used by the factory wraps the real handler and does lifetime tracking. For example:

var factory = serviceProvider.GetRequiredService<IHttpClientFactory>();
HttpClient client = factory.CreateClient("MyClient");

FieldInfo field = typeof(HttpMessageInvoker).GetField(
    "_handler",
    BindingFlags.GetField | BindingFlags.Instance | BindingFlags.NonPublic);

options.JwtBackChannelHandler = field.GetValue(client) as HttpMessageHandler;
leastprivilege commented 6 years ago

We also like the new HttpClient factory. We'll look into it.

brockallen commented 6 years ago

Should this be in 2.3 or 2.3.x?

leastprivilege commented 6 years ago

I deliberately put that in 2.3.x - we first need to move core to 2.1 - then we can assume presence of things like the http client factory.

paulomorgado commented 6 years ago

@leastprivilege, isn't HTTP Client Factory .NET Standard 2.0?

https://github.com/aspnet/HttpClientFactory/blob/master/src/Microsoft.Extensions.Http/Microsoft.Extensions.Http.csproj#L9

https://www.nuget.org/packages/Microsoft.Extensions.Http/2.1.1

leastprivilege commented 6 years ago

Sure . but all dependencies are >= 2.1.0

paulomorgado commented 6 years ago

But still .NET Standard 2.0. IT's a tricky one, I know.

fuzzzerd commented 6 years ago

Any chance this will address integration test issues, such as here: https://github.com/IdentityServer/IdentityServer4/issues/685 ?

martincostello commented 6 years ago

@fuzzzerd It should do, in fact doing exactly that is what spurred this issue into existence.

jingliancui commented 5 years ago

Facing the same problem with the cross two factory issue. mvcWebapplicationFactory can not communicate to idserverWebApplication factory. Hope,this issue fixed quickly.

jingliancui commented 5 years ago

Hi,I have start a new issue for the integration test feature here

because I don't know this issue is about the integration test ,if yes ,please close #2861

leastprivilege commented 5 years ago

@martincostello Hi - so now IdentityServer is on 2.1 - how would an ideal HttpClientFactory integration look to you?

martincostello commented 5 years ago

So for 2.1.x, I think this would be nice and easy to do by accepting an HttpClient as the type of the extensibility point on the options, rather than HttpMessageHandler, as that's what IHttpClientFactory provides as the output type for things that leverage the pooling/caching etc.

With 2.2.x, no additional work is actually needed because of the addition of the new IHttpMessageHandlerFactory interface (see https://github.com/aspnet/HttpClientFactory/pull/118), meaning you can now access the handlers directly.

I guess the options now are either:

  1. Add a way to inject the HttpClient and forward the existing HttpMessageHandler property to it so either approach works, at the expense of adding a new member to the options.
  2. Do nothing - applications targeting .NET Core 2.2.0 and later (even if this library is targeting 2.1.x) can use the new IHttpMessageHandlerFactory interface to support the interop scenario.

Here's a snippet from an application I've updated for ASP.NET Core 2.2.0 that uses the second approach:

var factory = serviceProvider.GetRequiredService<IHttpMessageHandlerFactory>();
options.JwtBackChannelHandler = factory.CreateHandler("Authorize");
leastprivilege commented 5 years ago

I think I will change the handler to use this pattern:

https://github.com/aspnet/aspnetcore/blob/master/src/Security/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectPostConfigureOptions.cs#L63-L69

Out of curiosity - from where are you calling the code from your last comment? In a PostConfigure?

martincostello commented 5 years ago

It’s in a lambda that runs as part of resolving services from IServiceProvider, so it runs “just in time” when IdentityServer types are first needed.

leastprivilege commented 5 years ago

Could you elaborate?

martincostello commented 5 years ago

So we've got something a bit like this going on in our application using some custom code that we wrap around IdentityService functionality so that common setup for our apps is hidden away in our wrapper library:

public static IServiceCollection UseCustomAuthorization(this IServiceCollection services)
{
    var settings = new MyAuthorizationSettings()
    {
        ClientSecret = "ClientSecretFromConfig",
        ClientId = "MyClientIdFromConfig",
    };

    settings.OnBuildServerOptions = (serviceProvider, options) =>
    {
        var factory = serviceProvider.GetRequiredService<IHttpMessageHandlerFactory>();
        options.JwtBackChannelHandler = factory.CreateHandler("Authorize");
    };
}
leastprivilege commented 5 years ago

OK - I finally looked into this.

This handler is really a decorator over our OAuth2 introspection handler and the Microsoft JWT handler.

Our handler is now HTTP client factory friendly (preview is here: https://www.nuget.org/packages/IdentityModel.AspNetCore.OAuth2Introspection/4.0.0-preview.6). Everything else is Microsoft code that I cannot change.