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

ServiceClient excessively acquires tokens when specifying AccessTokenProviderFunctionAsync and ExternalTokenManagement type #377

Closed
jimmy3912msncom opened this issue Jun 1, 2023 · 5 comments · Fixed by #378

Comments

@jimmy3912msncom
Copy link

jimmy3912msncom commented Jun 1, 2023

We noticed in our logs after switching over to use the ServiceClient that for some reason it seems to excessively call to acquire an access token. It seems to call the AccessTokenProviderFunctionAsync once when instantiating the serviceClient, to acquire a token, then when you try to execute a request it seems to acquire access tokens again but this times 6 times for each request.

This means if we use the client to make:

  1. 1 API call it will try to make 7 token request calls
  2. 2 API calls it will try to make 13 token request calls
  3. ...

Client details: Microsoft.PowerPlatform.Dataverse.Client Version=[1.0.39]
Target framework: .NET 6.0

We log each of our token requests and although it uses a cache it does add confusion to our service. Below is a sample of code that can be put together to reproduce this.

Also this is a gif of what I see:
dvserviceclientexcesstokens

...
        private int _authTokenAcquiryCount = 0;
        public async Task<string> GetTokenForInstanceAsync(Uri uri)
        {
            _authTokenAcquiryCount++;
            Console.WriteLine("coming in to acquire auth again " + uri);
            return "your token logic";
        }

        protected async Task AnalyzeAsync(bool withRecordingIngestion)
        {
            var targetUri = new Uri("orgUrl");
            var serviceClient = new PowerPlatform.Dataverse.Client.ServiceClient(
                new PowerPlatform.Dataverse.Client.Model.ConnectionOptions
                {
                    AccessTokenProviderFunctionAsync = _ => GetTokenForInstanceAsync(targetUri),
                    AuthenticationType = PowerPlatform.Dataverse.Client.AuthenticationType.ExternalTokenManagement,
                    ServiceUri = targetUri,
                    ClientId = "your client id",
                },
                false,
                new PowerPlatform.Dataverse.Client.Model.ConfigurationOptions()
                {
                    UseWebApi = true
                });

            await serviceClient.ExecuteAsync(new RetrieveMultipleRequest() { Query = new QueryExpression("usersettings") });

            await serviceClient.ExecuteAsync(new RetrieveMultipleRequest() { Query = new QueryExpression("usersettings") });

            await serviceClient.ExecuteAsync(new RetrieveMultipleRequest() { Query = new QueryExpression("usersettings") });
        }
...
@rajyraman
Copy link

Might be related to #161

@jimmy3912msncom
Copy link
Author

@rajyraman Thanks. That issue looks right. In my case we do handle caching but it stuck out as we logged every token access request even on caching.

The thing I don't understand is where @MattB-msft mentioned: "Yes, this is by design,
Each time the client needs to use the token for something, or its accessed in a way that could contact Dataverse, it requests the current access token of the token provider.
During initialization, several calls are made to support authentication, version detection and endpoints, Client identification, and accessing user identification. Collectively these calls setup any version specific handling of subsequent calls, and populate properties for the client."

If you see in my example, on serviceclient instantiation it calls token acquisition once. On all subsequent calls, even the same kind of request it seems to make 6 additional calls as mentioned in the original issue. What instantiation is it doing that would require it to fire on every request though?

If this is truly needed, I would suggest adding documentation to make it clear that this is the expected behavior as none of the documentation related to this called this out to me: connectionoptions.accesstokenproviderfunctionasync. It looks like in the other issue the issuer opener also called out the same thing and this issue is fairly old.

@MattB-msft
Copy link
Member

The service client acquires a new access token on each use of the internal IOrganziation service. (acquiring it for information for example). In the onboard implementation, the service client handles the token access by determining the lifecycle of the token and returning it w/out asking MSAL for it.
It does this because it knows the use case / scenario in play ( User Auth / or Client Cert | secret auth )

In the case of External authentication, the service client doesn't understand the scenario, thus token caching and returning of tokens is left entirely up to the external authentication handler. The reason that we don't make assumptions About the external authentication handler is that there exists scenarios where tokens need to be evaluated on a request by request basis. In such situations simply evaluating the token life cycle and returning the cached copy without sending the request to the external authentication handler, would create a situation where the client breaks the behavior required by the external authentication provider. Should 'you' ( the external auth provider ) want to do it the same way that the Service Client does it, your welcome to copy the code we use to your provider.

@jimmy3912msncom
Copy link
Author

Thank you for the explanation. I still think it would be good to document this for the client when using it so that this is not a surprise for users

@MattB-msft
Copy link
Member

Thats fair, we can discuss with docs team.
Also we are going to push an update to reduce the number of times we ask for the token.

@MattB-msft MattB-msft mentioned this issue Jun 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants