microsoftgraph / msgraph-sdk-dotnet-auth

Archived - use the TokenCredential classes provided by Azure.Identity. https://docs.microsoft.com/en-us/dotnet/api/overview/azure/identity-readme
https://graph.microsoft.com
MIT License
78 stars 19 forks source link

Cannot get cache tokens of confidential client application according to the current ClientCredentialProvider implementation, #65

Closed asith-w closed 3 years ago

asith-w commented 4 years ago

As below implementation ,There are 5 tenants and need to retrieve graph users of each tenants.
because ConfidentialClientApplication is created with tenant id and app retrieves graph users again for same tenants ,it generates another new token instead of using cache token
is there ClientCredentialProvider implementation to solve this issue ?

below code in gist

public async Task ExecuteAsync( )
 {  

var tenantIds = new List<string>{
     "1448f51b-234d-123345-86c8-0f6a634bbbb1",
     "2448f51b-234d-123345-86c8-0f6a634bbbb2",
     "3448f51b-234d-123345-86c8-0f6a634bbbb3",
      "4448f51b-234d-123345-86c8-0f6a634bbbb4",
      "5448f51b-234d-123345-86c8-0f6a634bbbb5"
};  

IConfidentialClientApplication confidentialClientApplication;
//1st requests
foreach (var tenantId in tenantIds)
{
    confidentialClientApplication = ConfidentialClientApplicationBuilder
                                    .Create(_azureADConfiguration.ClientId)
                                    .WithTenantId(tenantId)
                                    .WithClientSecret(_azureADConfiguration.ClientSecret)
                                    .Build();

    var clientCredentialProvider = new ClientCredentialProvider(confidentialClientApplication);
    var graphServiceClient = new GraphServiceClient(clientCredentialProvider);
    var users = await graphServiceClient.Users
                                        .Request()
                                        .GetAsync();

}
//2st requests
foreach (var tenantId in tenantIds)
{

    confidentialClientApplication = ConfidentialClientApplicationBuilder
                                        .Create(_azureADConfiguration.ClientId)
                                        .WithTenantId(tenantId)
                                        .WithClientSecret(_azureADConfiguration.ClientSecret)
                                        .Build();

    var clientCredentialProvider = new ClientCredentialProvider(confidentialClientApplication);
    var graphServiceClient = new GraphServiceClient(clientCredentialProvider);
    var users = await graphServiceClient.Users
                                        .Request()
                                        .GetAsync();
//two tokens are generated for each tenant.
}

AB#7210

MIchaelMainer commented 4 years ago

A ClientCredentialProvider instance is intended to authenticate per single tenant. We recommend that you create a dictionary of GraphServiceClient instances by tenant ID, and reuse those GraphServiceClient instances across your application. Don't re-instantiate GraphServiceClient per tenant.

asith-w commented 4 years ago

will dictionary of GraphServiceClient instances manage cache tokens ? following way is managed cache tokens properly https://stackoverflow.com/questions/61802091/how-to-handle-appcachetokens-of-multi-tenant-daemon-service-application-using-ms

MIchaelMainer commented 4 years ago

Got it. MSAL provides a mechanism to manage tokens for different tenants with the same client instance (and cache) which would have allowed a single GraphServiceClient to work across all tenants. We didn't expose WithAuthority (or it wasn't available when we did the work).

At this point, you'll have to have one client per tenant and allow each client + its MSAL manage token cache.

For us, we should verify that having x-tenant tokens managed in a shared token cache is what we want. If so, then the next auth integration+iteration should expose WithAuthority.

cc: @darrelmiller

maisarissi commented 3 years ago

Hi @asith-w

Thank you for reaching out and opening this issue. This client library will not leave the preview state. Microsoft.Graph v4 now integrates with Azure.Identity which supports a wide variety of authentication flows out of the box. We suggest that you migrate to v4 + Azure.Identity. Read more about it in this issue.

This issue won't be fixed, and the repository will be archived.