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

[Enhancement] Initialize confidentialClientApplicationOptions properties from microsoftIdentityOptions when possible needed #742

Closed
radistmorse opened this issue Nov 3, 2020 · 6 comments
Assignees
Labels
answered enhancement New feature or request fixed question Further information is requested supportability
Milestone

Comments

@radistmorse
Copy link
Contributor

I use Identity.Web/Identity.Web.MicrosoftGraph v1.2 & MS Graph SDK v3.19 in an asp core 3.1 web api application.

I use graph in AppOnly mode. This is my configuration:

services.AddAuthentication()
    .AddMicrosoftIdentityWebApp(opts =>
    {
        opts.Instance = "https://login.microsoftonline.com/";
        opts.TenantId = "B2CTenantId";
        opts.ClientId = "B2CClientId";
        opts.ClientSecret = "B2CClientSecret";
        opts.BackchannelHttpHandler = new HttpClientHandler
        {
            UseProxy = true,
            Proxy = new System.Net.WebProxy { Address = new System.Uri("myproxy") }
        };
    })
        .EnableTokenAcquisitionToCallDownstreamApi(opts =>
        {
            opts.Instance = "https://login.microsoftonline.com/";
            opts.TenantId = "B2CTenantId";
            opts.ClientId = "B2CClientId";
        })
            .AddMicrosoftGraphAppOnly(provider => new GraphServiceClient(GraphClientFactory.Create(provider, proxy: new System.Net.WebProxy { Address = new System.Uri("myproxy") })))
    .AddInMemoryTokenCaches();

The first and obvious question is: is this the correct way? I never found any examples with the simple confidential client configuration.

The second question is: why does it require AddAuthentication? I add authentication to my app anyway, so it's not a problem for me, but still. As far as I understood, the main difference between AddApp and AddApi is that the former doesn't add the authentication schema, and doesn't authorize the api users. So it shouldn't require the AuthenticationBuilder, should it?

The third question is about Instance. Why do I need to explicitly provide it? Shouldn't https://login.microsoftonline.com/ be the obvious default? Could you at least provide it in some public constant somewhere, so that I did not need to hardcode it myself.

And the last question is about duplication. I need to explicitly provide both MicrosoftIdentityOptions and ConfidentialClientApplicationOptions with the same set of parameters. Why confidential client setup doesn't copy the parameters from microsoft identity? It does copy ClientSecret, but nothing else. Why?

@jmprieur jmprieur added the question Further information is requested label Nov 4, 2020
@jmprieur
Copy link
Collaborator

jmprieur commented Nov 4, 2020

@radistmorse

  1. If you want to use this code, you'd need to provide the default Authentication scheme: AddAuthentication(AzureADDefaults.AuthenticationScheme)

However, I'm pretty confused that you want to call Graph with a B2C settings. Or is it to provision users? (in that case it's considered as an AAD application - you won't pass-in any policy?)

  1. However, you could do simpler by leveraging the configuration for the instance, clientId, client secret:

    services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
         .AddMicrosoftIdentityWebApp(opts =>
    {
     Configuration.Bind("AzureADB2C", opts );
     opts .BackchannelHttpHandler = new HttpClientHandler
     {
      UseProxy = true,
      Proxy = new System.Net.WebProxy { Address = new System.Uri("myproxy") }
      };
    })
    .EnableTokenAcquisitionToCallDownstreamApi(opts => { Configuration.Bind("AzureADB2C", opts )})
    .AddMicrosoftGraphAppOnly(provider => new GraphServiceClient(GraphClientFactory.Create(provider, proxy: new System.Net.WebProxy { Address = new System.Uri("myproxy") })))
     .AddInMemoryTokenCaches();
  2. Or even simpler (and does not have any repeat)

    services.AddMicrosoftIdentityWebAppAuthentication(Configuration.GetSection("AzureAdB2C"))
                .EnableTokenAcquisitionToCallDownstreamApi()
                .AddMicrosoftGraphAppOnly(provider => new GraphServiceClient(GraphClientFactory.Create(provider, proxy: new System.Net.WebProxy { Address = new System.Uri("myproxy") })))
     .AddInMemoryTokenCaches();
    
     services.Configure<OpenIdConnectOptions>(OpenIdConnectDefaults.AuthenticationScheme, opts=>
     {
       opts.BackchannelHttpHandler = new HttpClientHandler
         {
             UseProxy = true,
             Proxy = new System.Net.WebProxy { Address = new System.Uri("myproxy") }
         };
     });
  3. Finally, I believe that you could even get rid of the initialization of the proxy in these lines altogether by just setting environment variables. Not pretty, but affects all the HttpClients, which is probably what you want.

    System.Environment.SetEnvironmentVariable(HTTP_PROXY, proxy);
    System.Environment.SetEnvironmentVariable(HTTPS_PROXY, proxy);
    
    services.AddMicrosoftIdentityWebAppAuthentication(Configuration.GetSection("AzureAdB2C"))
                .EnableTokenAcquisitionToCallDownstreamApi()
                .AddMicrosoftGraphAppOnly()
                .AddInMemoryTokenCaches();

@radistmorse
Copy link
Contributor Author

Hi @jmprieur

Thanks for your answer, but I still don't fully understand that.

  1. I'm not actually familiar with this terminology. We only need graph to remove the users when their access rights expire. And I added all the authentication schemes manually, since we have many sources of JWT tokens, and map them to several authorization policies. It seems to work without the "default authentication schema", yet you say that it is needed. What for? Also, I still don't understand why it requires the authentication schema at all. I only need this to get the app-only security token from azure, why does it care about my own schemes?

2 & 3. I noticed that in all the examples and tutorials the focus is on the Configuration system. We store all the configuration in the DB, so it's easier for me to provide all the values manually, rather than through Bind or GetSection, since I'll need to set the values in there first. Is it not the intended way? How GetSection is different from the manual setting? Why does it not require a duplication, when every other method does?

  1. No, it is not what I want. We have a proxy for "external" resources only. All the internal calls should not use it. So we need to explicitly set the proxy where we need it. BTW, did you know that it is impossible to set the proxy for MS ApplicationInsights through public API? Because it is :)

@jmprieur
Copy link
Collaborator

jmprieur commented Nov 5, 2020

@radistmorse. Thanks for sharing your scenario with us. Pretty advanced compared to what most of our customers do.

  1. that's fine if you have mapped all your controller actions to an authentication schema. No need to add a default one. (it's just that not many customers do that) 👍
  2. I see, you need to use the overrides with the delegates. The thing is the same section is used to initialize several data structures (the OpenIdConnect options = MicrosoftIdentityOptions (which inherit from the OpendIdConnectOptions), and the ConfidentialClientApplicationOptions (for MSAL.NET). In the case with the configuration section, we keep it in the builder returned by AddMicrosoftIdentityWebAppAuthentication / AddMicrosoftIdentityWebApp, and reapply the delegates ourselves to EnableTokenAcquisitionToCallDownstreamApi. When you use the overrides with the delegates, you need to do it yourself for the moment (some properties are the same and we could do a better job here, but some properties are different)
  3. thanks for explaining. Again a very advanced scenario.

@jmprieur
Copy link
Collaborator

jmprieur commented Nov 5, 2020

some properties are the same and we could do a better job here

@jennyf19 : do you think we'd want to take an enhancement to avoid having to pass-in the clientID, tenantId, Instance, ClientSecret in the call to EnableTokenAcquisitionToCallDownstreamApi?

@jmprieur jmprieur added the enhancement New feature or request label Nov 26, 2020
@jmprieur
Copy link
Collaborator

@jennyf19, what do you think of this improvement:

We could set ConfidentialClientApplicationOptions from MicrosoftIdentityOptions when not overriden. Something like:

confidentialClientApplicationOptions.ClientId  ??= microsoftIdentityOptions.ClientId;
// etc

@jmprieur jmprieur added this to the 1.4.2 milestone Dec 15, 2020
@jmprieur jmprieur modified the milestones: 1.5.0, 1.6.0 Jan 14, 2021
@jmprieur jmprieur changed the title [Question] The proper way to configure a simple confidential client for MS graph [Enhancement] Initialize confidentialClientApplicationOptions properties from microsoftIdentityOptions when possible needed Jan 14, 2021
@jennyf19 jennyf19 added the fixed label Feb 1, 2021
@jennyf19
Copy link
Collaborator

Included in 1.6.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered enhancement New feature or request fixed question Further information is requested supportability
Projects
None yet
Development

No branches or pull requests

3 participants