microsoft-identity-web icon indicating copy to clipboard operation
microsoft-identity-web copied to clipboard

Allow configuration of HttpClients used by DownstreamWebApi

Open h3rmanj opened this issue 2 years ago • 3 comments

Addresses #1740

The MS docs recommends implementing resilient http requests using libraries like Polly. At the same time, the recommended way to make authenticated api calls is through the IDownstreamWebApi interface. This PR closes the gaps in the docs and allows the consumer access to the IHttpClientBuilder, allowing them to add Polly policies for their downstream apis etc.

Changed to add named HttpClients, which DownstreamWebApi then gets from IHttpClientFactory, allowing the consumer to optionally configure HttpClient per downstream service.

Example usage with this:

builder.Services.AddMicrosoftIdentityWebApi(builder.Configuration)
  .EnableTokenAcquisitionToCallDownstreamApi()
    .AddDownstreamWebApi(
      "DownstreamApi",
      builder.Configuration.GetSection("DownstreamApi"),
      httpClientBuilder => httpClientBuilder
        .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]
        {
          TimeSpan.FromSeconds(1),
          TimeSpan.FromSeconds(5),
          TimeSpan.FromSeconds(10)
        })))
    .AddDownstreamWebApi(
      "AnotherApi",
      builder.Configuration.GetSection("AnotherApi"),
      httpClientBuilder => httpClientBuilder
        .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]
        {
          TimeSpan.FromSeconds(1),
          TimeSpan.FromSeconds(2)
        })))
  .AddDistributedTokenCaches();

h3rmanj avatar May 12 '22 14:05 h3rmanj

CLA assistant check
All CLA requirements met.

ghost avatar May 12 '22 14:05 ghost

Alternatively, if changing the API of AddDownstreamWebApi isn't desirable, I could just drop it. Multiple calls to AddHttpClient with the same client name applies all options to the same client. So we could add a named HttpClient with the serviceName internally, and the consumer could then AddHttpClient(serviceName) as well if they need it. It would be a little bit more of a hidden feature, but would work.

builder.Services.AddMicrosoftIdentityWebApi(builder.Configuration)
  .EnableTokenAcquisitionToCallDownstreamApi()
    .AddDownstreamWebApi("DownstreamApi", builder.Configuration.GetSection("DownstreamApi"))
    .AddDownstreamWebApi("AnotherApi", builder.Configuration.GetSection("AnotherApi"))
  .AddDistributedTokenCaches();

builder.Services.AddHttpClient("DownstreamApi")
  .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]
  {
    TimeSpan.FromSeconds(1),
    TimeSpan.FromSeconds(5),
    TimeSpan.FromSeconds(10)
  }));
      
builder.Services.AddHttpClient("AnotherApi")
  .AddTransientHttpErrorPolicy(builder => builder.WaitAndRetryAsync(new[]
  {
    TimeSpan.FromSeconds(1),
    TimeSpan.FromSeconds(2)
  }));

h3rmanj avatar Jun 10 '22 20:06 h3rmanj

@h3rmanj Let's us get back to you on this one. We are considering having a new interface (IDownstreamRestApi) which therefore would not have breaking changes (as it would be new).

@jennyf19: let's work on collecting all the issues and PRs around IDownstreamWebApi and add the right design for IDownstreamRestApi

jmprieur avatar Jun 13 '22 01:06 jmprieur

@jmprieur @jennyf19 I see some work has been done in the rel/v2 branch, but I'm not sure if it's complete. What is the status of this? Will there be a chance to review before a final release?

h3rmanj avatar Nov 08 '22 10:11 h3rmanj

@h3rmanj : what you wrote in https://github.com/AzureAD/microsoft-identity-web/pull/1735#issuecomment-1152699911 would work in Microsoft.Identity.Web 2.0.5-preview (with AddDownstrreamRestApi)

jmprieur avatar Nov 09 '22 00:11 jmprieur

Implemented in 2.5.0

h3rmanj avatar Feb 28 '23 09:02 h3rmanj