Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DownstreamWebApiOptions.Clone does not include CustomizeHttpRequestMessage #1970

Closed
rvplauborg opened this issue Nov 18, 2022 · 5 comments
Closed
Labels
bug Something isn't working P1 question Further information is requested

Comments

@rvplauborg
Copy link
Contributor

Microsoft.Identity.Web Library

Microsoft.Identity.Web

Microsoft.Identity.Web version

1.25.5

Web app

Sign-in users and call web APIs

Web API

Protected web APIs call downstream web APIs

Token cache serialization

In-memory caches

Description

DownstreamWebApi.CallWebApiForUserAsync implementation currently merges options from initial configuration and calledApiOptionsOverride by cloning the initial options and then invoking the override action:

            DownstreamWebApiOptions clonedOptions = options.Clone();
            calledApiOptionsOverride?.Invoke(clonedOptions);

The problem is that Clone does not include the CustomizeHttpRequestMessage property on DownstreamWebApiOptions, which means that it is just null and you thereby lose the original configuration for that property.
If users do not want to use the strongly typed extensions because they need more control, they then cannot use the CustomizeHttpRequestMessage setting at all unfortunately.

Reproduction steps

  1. Initial DI configuration:
builder.AddDownstreamWebApi(serviceName, options =>
            {
               options.CustomizeHttpRequestMessage =
                    message =>
                    {
                        message.Headers.Add("key", "value");
                    };
            });
  1. Inject downstream web api, and use it:
await _downstreamWebApi.CallWebApiForUserAsync(ServiceName(), null, options =>
            {
                // Here the options.CustomizeHttpRequestMessage  is null unfortunately
                options.RelativePath = "some-url";
                options.HttpMethod = HttpMethod.Get;
            });

Error message

No response

Id Web logs

No response

Relevant code snippets

Included in repro steps

Regression

No response

Expected behavior

Expected the CustomizeHttpRequestMessage setting to behave as all other settings and not be cleared.

@rvplauborg rvplauborg added the question Further information is requested label Nov 18, 2022
@jmprieur jmprieur added bug Something isn't working P1 labels Nov 19, 2022
@jmprieur
Copy link
Collaborator

Thanks @vplauborg

@rvplauborg
Copy link
Contributor Author

Thanks for merging my fix so quickly. When do you reckon the change will be released just out of curiosity?

@jmprieur
Copy link
Collaborator

We'll try to release it tomorrow, @rvplauborg

@rvplauborg
Copy link
Contributor Author

@jmprieur amazing. Thanks!

@jennyf19
Copy link
Collaborator

Included in 1.25.8 thanks for the contribution @rvplauborg
We will also include this in 2.x-preview in the next release, either today or tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1 question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants