[Bug] Signing in users breaks when using backchannel proxy

[mjnorman]

Which version of Microsoft Identity Web are you using?
Note that to get help, you need to run the latest version.
0.3.1-preview

Where is the issue?

• Web app
  • Sign-in users
  • Sign-in users and call web APIs
• Web API
  • Protected web APIs (validating tokens)
  • Protected web APIs (validating scopes)
  • Protected web APIs call downstream web APIs
• Token cache serialization
  • In-memory caches
  • Session caches
  • Distributed caches
• Other (please describe)

Is this a new or an existing app?

a. The app is in production and I have upgraded to a new version of Microsoft Identity Web.

Repro
I have a public repro which is mainly a new project created with dotnet new mvc2 -singleauth template, then modified to handle Cloud Foundry variable parsing to obtain proxy information, and create the back channel proxy. Relevant code for Proxy is below. -- repro here

I am not sure why, but simply changing back to .AddAzureAD, changing options to OpenIdConnectOptions, and no other changes, the back channel communication works properly.

        public void PostConfigure(string name, MicrosoftIdentityOptions options)
        {
            Log.Debug("Starting PostConfigure");
            var proxyURI = VCAPHelper.GetSquidUri();
            Log.Debug("Setting Proxy URI to {Proxy}", proxyURI);

            if (!string.IsNullOrEmpty(proxyURI))
            {
                var webProxy = new WebProxy()
                {
                    Address = new Uri($"http://{VCAPHelper.GetSquidHost()}:{VCAPHelper.GetSquidPort()}"),
                    BypassProxyOnLocal = true,
                    UseDefaultCredentials = false,
                    Credentials = new NetworkCredential(VCAPHelper.GetSquidUsername(), VCAPHelper.GetSquidPassword())
                };
                var httpClientHandler = new HttpClientHandler()
                {
                    Proxy = webProxy,
                    UseDefaultCredentials = true,
                    UseProxy = true
                };
                options.Backchannel = new HttpClient(httpClientHandler, true);
            }

Expected behavior
I would expect back channel behavior to work normally when access through a proxy is required to access metadata at login.microsoft.com.

Actual behavior
Receive an error below presumably due to some communication on the back channel not working through the proxy.

IOException: IDX20804: Unable to retrieve document from: 'https://login.microsoftonline.com/common/discovery/instance?authorization_endpoint=https://login.microsoftonline.com/common/oauth2/v2.0/authorize&api-version=1.1'.

Additional context / logs / screenshots
There was an issue open under aspnetcore last year where I got the AzureAD piece working, that issue is dotnet/aspnetcore#20000, for reference.

[jmprieur]

@mjnorman : did you read https://docs.microsoft.com/en-us/aspnet/core/host-and-deploy/proxy-load-balancer?

[mjnorman]

Yes, we are set up correctly on the repro. Also recall that simply moving back to AzureAd method and to OpenIdConnectOptions with no other changes works perfectly.

[pmaytak]

@mjnorman

1. What if you move the AddAuth lines before the Configure method.
   (You would have to change your events to += context =>)
2. Also you can try changing
   services.Configure<MicrosoftIdentityOptions>(options =>
   to
   services.Configure<OpenIdConnectOptions>(OpenIdConnectDefaults.AuthenticationScheme, options =>

[mjnorman]

It does not make a difference. However, we did originally have that ordering, but after re-reading tratcher's comments from last year on the old issue mentioned above, it is clear that Configure has to come first because the backchannel HttpClient is consumed when the Options are initialized. Here if memory serves,
https://github.com/dotnet/aspnetcore/blob/1e7d6d56a5ae6b9ddfc8680d6f4f8282a8f17400/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectPostConfigureOptions.cs#L63

[mjnorman]

@pmaytak Missed your second point. If OpenIdConnectOptions is used, the Configure code is not even hit.

[jmprieur]

@Tratcher do you have an idea?

[Tratcher]

Note to self: This is likely part of the problem.

microsoft-identity-web/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs

Line 242 in c83cb5d

PopulateOpenIdOptionsFromMicrosoftIdentityOptions(options, microsoftIdentityOptions.Value);

[Tratcher]

No, here:

microsoft-identity-web/src/Microsoft.Identity.Web/Resource/AadIssuerValidator.cs

Line 25 in d33ab13

private static readonly ConfigurationManager<IssuerMetadata> s_configManager = new ConfigurationManager<IssuerMetadata>(Constants.AzureADIssuerMetadataUrl, new IssuerConfigurationRetriever());

It's not using the back channel. Statics are bad...

[Tratcher]

@mjnorman For now you'll need to set the proxy here:
https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.defaultproxy?view=netcore-3.1

Do that really early in Startup or Main so that the statics pick it up.

There should also be a way to set this from a config file, but you'd have to ask about that over at https://github.com/dotnet/runtime.

@jmprieur the outbound network communication needs to be configurable for scenarios like this, we'll need to find a replacement for this static.

[mjnorman]

@Tratcher This worked perfectly. I almost want to say this is might be a preferred method when everything outbound would need to go through a proxy such as CF instances with no internet connectivity, so you don't have to configure options individually, although I agree with your statement that it should be configurable in these options as well.

        public void ConfigureServices(IServiceCollection services)
        {
            Environment.SetEnvironmentVariable(HTTP_PROXY, GetProxyUri());
            Environment.SetEnvironmentVariable(HTTPS_PROXY, GetProxyUri());
            ...
        }

[jmprieur]

@Tratcher ; would removing the static be solved by this issue?: #402 ?

[Tratcher]

Sure, something like that.

[liiri]

@Tratcher can you please explain why that is the root cause and how the fix would allow us to use the backchannel configuration?

[Tratcher]

The ConfigurationManager I linked to uses its own HttpClient instance with only the default proxy settings.
https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/91acf95dcbba555064d089363b97dbf5d211e30a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs#L89

It needs to be replaced by one that allows you to pass in the HttpClient like this:
https://github.com/dotnet/aspnetcore/blob/3fdbc2e90d9762324ef3809098c9ca25b0cb3284/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectPostConfigureOptions.cs#L99-L100

Until then the above workarounds that let you configure the default proxy for the whole application should work.

[jmprieur]

@jennyf19 : moving this to an enhancement as @Tratcher provided more context about what needs to be done.
@Tratcher : would inject an IHttpClient be a god solution?
It might be worth a quick sync to be sure we are not iterating towards the solution :)

[Tratcher]

I'd suggest using the same pattern as AspNet, injecting IOptions<AadIssuerValidatorOptions> so the developer can do something like this in startup:

services.Configure<AadIssuerValidatorOptions>(options =>
{
  options.Backchallel = ...
  or 
  options.Configuration = ...
});

[liiri]

For reference, I ended up having to use the following violent fix, since I cannot set proxy globally, most of my requests are done in the organization network and shouldn't use proxy.

var configurationManagerFieldInfo = typeof(AadIssuerValidator).GetField("s_configManager", BindingFlags.Static | BindingFlags.NonPublic);
var configurationManager = configurationManagerFieldInfo?.GetValue(null);
var documentRetrieverFieldInfo = configurationManager?.GetType()
    .GetField("_docRetriever", BindingFlags.Instance | BindingFlags.NonPublic);
var httpClientHandler = new HttpClientHandler();
// configure your httpClientHandler ...
documentRetrieverFieldInfo?.SetValue(configurationManager, new HttpDocumentRetriever(new HttpClient(httpClientHandler)));

[mjnorman]

violent fix

🤣

I have the same issue. A "less violent fix" is just to set NO_PROXY for your internal hosts. Comma delimited name of hosts to bypass proxy. Albeit a pain to manage that, but at any rate--

                        System.Environment.SetEnvironmentVariable(NO_PROXY, BuildNoProxyList());

[jmprieur]

@jennyf19
My understanding is that we could:

• have MicrosoftIdentityIssuerValidatorFactory provide a public constructor that takes an IHttpClientFactory as a parameter (Is constructed by DI anyway)
• in this constructor to build configManager use the override of the constructor which takes an IHttpClient, which would be be from the HttpClientFactory

@Tratcher : what options would you see for AadIssuerValidatorOptions. Isn't it better to inject an HttpClient and let customers provide their factory if they want to?

[Tratcher]

Fair point, the backchannel options pattern pre-dates IHttpClientFactory and we've never reconciled them.

Even when using IHttpClientFactory you'll want to expose a configurable name option to pass to CreateClient.

Also, IHttpClientFactory isn't a default service, if you add it as a required constructor parameter then you'll need to also add it as a package dependency and register it in DI.

Note there is no IHttpClient, only HttpClient (right?).

[jennyf19]

@Tratcher do you mind taking a look at the linked draft? thanks.

[jennyf19]

Included in 1.2.0 release.