dotnet / dotnet/aspnetcore

Use IHttpClientFactory in RemoteAuthenticationOptions

Open
#10,542 10 comments 7 reactions 0 assignees View on GitHub
affected-medium area-auth enhancement severity-minor
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 6h
Merged PRs (30d)
290

Description

### Is your feature request related to a problem? Please describe.
ASP.NET Core 2.1 introduced IHttpClientFactory to solve the [problems with creating and disposing HttpClient with every call](https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/), and the [problems with creating a long-lived HttpClient](https://github.com/dotnet/corefx/issues/11224).

However, long-lived HttpClient is still created within the framework. Specifically, in [RemoteAuthenticationOptions.Backchannel](https://github.com/aspnet/AspNetCore/blob/master/src/Security/Authentication/Core/src/RemoteAuthenticationOptions.cs#L79) and [RemoteAuthenticationOptions.BackchannelHttpHandler](https://github.com/aspnet/AspNetCore/blob/master/src/Security/Authentication/Core/src/RemoteAuthenticationOptions.cs#L74), these have effectively application lifetime because instances of AuthenticationOptions are cached in the singleton IOptionsMonitorCache.

### Describe the solution you'd like
Replace RemoteAuthenticationOptions.Backchannel and RemoteAuthenticationOptions.BackchannelHttpHandler with RemoteAuthenticationOptions.BackchannelFactory, so that whenever an HttpClient is needed, it is created from the BackchannelFactory and disposed.

This would also require a new [HttpDocumentRetriever](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/master/src/Microsoft.IdentityModel.Protocols/Configuration/HttpDocumentRetriever.cs) constructor that takes an IHttpClientFactory, because [JwtBearerPostConfigureOptions](https://github.com/aspnet/AspNetCore/blob/master/src/Security/Authentication/JwtBearer/src/JwtBearerPostConfigureOptions.cs#L58), [WsFederationPostConfigureOptions](https://github.com/aspnet/AspNetCore/blob/master/src/Security/Authentication/WsFederation/src/WsFederationPostConfigureOptions.cs#L84) and [OpenIdConnectPostConfigureOptions](https://github.com/aspnet/AspNetCore/blob/master/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectPostConfigureOptions.cs#L96) initializes a long-lived ConfigurationManager.

### Describe alternatives you've considered
- Do not store AuthenticationOptions in IOptionsMonitorCache and make it scoped per request. However, it means that JwtBearer, OpenIdConnect and WsFederation (or 3rd party Saml2 protocol handler) would require fetching metadata on every request, which would slow down every request.

- Roll my own ConfigurationManager and HttpDocumentRetriever, which would bypass any kind of niceities that `.AddJwtBearer`, `.AddOpenIdConnect` extension methods provide.

### Additional context
[The Grand Auth Redesign of 2017](https://github.com/aspnet/Security/issues/1179) in Core 2.0 predates IHttpClientFactory in Core 2.1.

In Core 2.0, the new [SocketsHttpHandler does not solve DNS problem](https://github.com/aspnet/HttpClientFactory/issues/10#issuecomment-398511183)

[Exploring DNS issue](https://github.com/aspnet/HttpClientFactory/wiki/Exploring-DNS-issues)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.