Skip to content
This repository has been archived by the owner on Jun 30, 2023. It is now read-only.

Custom HttpClient not used for every call to the service #1488

Closed
bgavrilMS opened this issue Jan 24, 2019 · 12 comments
Closed

Custom HttpClient not used for every call to the service #1488

bgavrilMS opened this issue Jan 24, 2019 · 12 comments
Assignees
Milestone

Comments

@bgavrilMS
Copy link
Member

bgavrilMS commented Jan 24, 2019

Which Version of ADAL are you using ?
4.5

Which platform has the issue?
all

Repro
Use the new AuthenticationContext constructor that takes in an HttpClient. Some flows use a new HttpClient everytime.

Expected behavior
the passed in httpclient should be used everywhere

Actual behavior
the httpClient is not used in some calls, such as the auth code request.
the custom httpClient is used for calls to the MEX endpoint.

Note: this is where we create a new HttpClient https://github.com:443/AzureAD/azure-activedirectory-library-for-dotnet.git/blob/dev/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Http/HttpClientWrapper.cs#L66
This is also bad for perf, because we should reuse this httpClient.

@GroupeElanz
Copy link

Same issue here.
Using a proxy is mandatory for us, so we use a custom HttpClient to specify the proxy (as described here: https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/wiki/with-proxy) but not all the calls use this proxy.
Is there a workaround except setting a proxy at system level?

@bgavrilMS
Copy link
Member Author

bgavrilMS commented Feb 14, 2019 via email

@GroupeElanz
Copy link

Hello, this works and that's the same behavior as setting the proxy on web.config, but our customer wants to isolate only the ADAL calls through a specific proxy with specific credentials.

@jmprieur jmprieur added this to the 4.5.2 milestone Feb 18, 2019
@nikitinvalera
Copy link

nikitinvalera commented Feb 19, 2019

Hi Bogdan,
WebRequest.defaultwebproxy will not help for .NET core project, so I wonder if ALL Handlers (like AcquireTokenForClientHandler) will ever (will they?) use custom configured HttpClient?

This is a problem for us, since we're behind corporate firewall.

@GroupeElanz
Copy link

Hello,
Is there already a planned date for the release of a version which includes the correction?

@jmprieur jmprieur modified the milestones: 4.5.2, 5.01, 5.0.1 Mar 7, 2019
@javisst
Copy link

javisst commented Mar 11, 2019

Hi, we are also behind a corporate proxy and very interested in a solution to that.

@bgavrilMS
Copy link
Member Author

This is being actively worked on.

@vadirajgk
Copy link

We are also facing same issue in our test server, I think ADAL API is not allowing it to read the proxy values passed in the HttpClient object for the AuthenticationContext call. It will work fine when we add proxy in IE for that user account login only but we can not do this. Also we can not add proxy on system level.

Please let us know the ETA for this. This is impacting by holding the multiple application's production release from one month.

@henrik-me
Copy link
Contributor

PR: #1530

@zapalap
Copy link

zapalap commented Mar 31, 2019

It looks like the custom IHttpClientFactory passed to the new AuthenticationContext constructor is not actually used further.

It's because of this code:

internal static IServiceBundle CreateWithHttpClientFactory(IHttpClientFactory httpClientFactory)
{
if (httpClientFactory != null)
{
return new ServiceBundle(new HttpClientFactory());
}
else
{
return new ServiceBundle();
}
}

So even if the httpClientFactory parameter is not null it's not being passed to the ServiceBundle constructor.

@jennyf19
Copy link
Contributor

jennyf19 commented Apr 1, 2019

@zapalap this has been fixed and will be in 5.0.1-preview release. fix is merged into dev. thank you.

@jennyf19
Copy link
Contributor

jennyf19 commented Apr 2, 2019

included in ADAL 5.0.1-preview

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

10 participants