Add support in DiscoveryCache to HttpClients with BaseAddress specified

[josephdecock]

Migrated from DuendeArchive/IdentityModel#599

Original Description

In my application, I have a HttpClientFactory configured to point at the authority

builder.Services.AddHttpClient("idp", http =>
{
    http.BaseAddress = new Uri("https://idp");
});

I want to use that client configuration to request the discovery document from the cache.

Currently all DiscoveryCache constructors require the authority as string but it's not required to have a value if the HttpClient produced by the delegate has a BaseAddress.

The snippet below works, but the null! passed as argument really hurts my eyes.

builder.Services.AddSingleton<IDiscoveryCache>(sp =>
{
    var policy = new DiscoveryPolicy
    {
       // some policy settings
    };
    
    return new DiscoveryCache(null!, () => sp.GetRequiredService<IHttpClientFactory>().CreateClient("idp"), policy);
});

I'm open to work on a PR if there is an agreed-upon solution to work on!
In my application, I have a HttpClientFactory configured to point at the authority

builder.Services.AddHttpClient("idp", http =>
{
    http.BaseAddress = new Uri("https://idp");
});

I want to use that client configuration to request the discovery document from the cache.

Currently all DiscoveryCache constructors require the authority as string but it's not required to have a value if the HttpClient produced by the delegate has a BaseAddress.

The snippet below works, but the null! passed as argument really hurts my eyes.

builder.Services.AddSingleton<IDiscoveryCache>(sp =>
{
    var policy = new DiscoveryPolicy
    {
       // some policy settings
    };
    
    return new DiscoveryCache(null!, () => sp.GetRequiredService<IHttpClientFactory>().CreateClient("idp"), policy);
});

I'm open to work on a PR if there is an agreed-upon solution to work on!

My thoughts

My first thought is to have a constructor overload like this:

public DiscoveryCache(Func<HttpMessageInvoker> httpClientFunc, DiscoveryPolicy? policy = null)

And then _authority's type would change to allow nulls.

The only downside that I see is that it is possible to misuse this API, but we have a runtime check already to ensure that there is a base address, and a helpful error message. We should just make sure to include xmldoc that explains the requirements of the new constructor really clearly.