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

[Documentation] Use of ConfidentialClientApplication and TokenStore #1342

Closed
3 of 5 tasks
techgeek03 opened this issue Aug 15, 2019 · 11 comments
Closed
3 of 5 tasks

[Documentation] Use of ConfidentialClientApplication and TokenStore #1342

techgeek03 opened this issue Aug 15, 2019 · 11 comments

Comments

@techgeek03
Copy link

Documentation Related To Component:

ConfidentialClientApplication and TokenStore

Please check those that apply

  • typo
  • documentation doesn't exist
  • documentation needs clarification
  • error(s) in example
  • needs example

Description Of The Issue

I have a question regarding the use of the ConfidentialClientApplication and token caching that is not very clear in the documentation.

In my application, I'm using the on behalf of flow to call downstream API from my ASP.NET Core Web API and I would like to leverage token caching for users.

  1. The current documentation for MSAL states that there is a default token caching implemented, however it is not very clear in the following cases:
  • How and when should the ConfidentialClientApplication be instantiated. When ConfidentialClientApplication is registered as a singleton in the DI service provider the default token store can be used as expected and subsequent calls from the same user will result in only one call to Azure AD (when using the AcquireTokenSilent on the ConfidentialClientApplication). However registering it as a scoped service the cache is only available per request. My question is: Is it safe to register ConfidentialClientApplication as singleton? Can you please update and elaborate on this in the https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/Client-Applications with this information?
  • Is the default token store handling removal of accounts and tokens when they expire?
  • On the other hand, for the on behalf of flow the documentation states that you should consider implementing your own cache. Can you please elaborate the reasons for this recommendation? It will be much easier to implement it if we can understand the reasons and use cases.
  1. There is a proposal for creating Microsoft.Identity.Client.Extensions.Web as NuGet package (https://github.com/AzureAD/microsoft-authentication-extensions-for-dotnet), the code base has examples on how to implement token store per user, but the implementation is not following some of the best practices from the ASP.Net Core team (the code base leaks lot of framework dependencies like the IHttpContextAccessor, HttpContext and forces use of ISession). There is a lot of repeating code when using the MSAL library and it is good that such library be created, but it does not look that is production ready.

Looking forward on your feedback! Thank you! :)

@techgeek03 techgeek03 changed the title [Documentation] ConfidentialClientApplication [Documentation] Use of ConfidentialClientApplication and TokenStore Aug 15, 2019
@bgavrilMS
Copy link
Member

bgavrilMS commented Aug 15, 2019

Great questions.

  1. You should have 1 Confidential Client Application for each token cache. And we recommend that you have 1 token cache per session, so there should be 1 CCA per session.

Creation pattern in web app sample.

Testing caveat: You may think that using DI is also gonna make things more testable. Sadly, it's difficult (impossible) to mock the builder pattern, so I would actually suggest to wrap the creation and invocation in a class of your own.

  1. Default token cache is a non-static in-memory implementation (think of it as a dictionary). There is one for each ConfidentialClientApplication you create. If you create multiple applications, you'd need to attach the same cache to each of them (some of our samples do that)

The issues with the in-memory cache are:

  • Availability - in memory token caches are fine, but when ASP renews the process, it will kill the cache. I'm not sure how much control you have over that. Now OBO itself is a silent flow, so there is no user impact if you just get a fresh token from AAD. However, getting the original token via the authorization flow is impactful - your users will need to re-login. The solution is to use a persistent cache like a Redis or SQL cache

  • Security - although we test MSAL as best we can to protect it against it retrieving the token of the wrong user, a token cache implementation where you store only 1 user per cache is more secure by default.

Scale - a distributed cache scales better. A rough example of how much space is needed - around 50kb for some 3 users in an OBO like scenario.

Microsoft.Identity.Client.Extensions.Web has a bunch of TokenCache implementation that you can use.

  1. We're still talking internall about the best way to productize Microsoft.Identity.Client.Extensions.Web. For now, we recommend that you just copy the code.

CC @kalyankrishna1 @TiagoBrenck @jmprieur

@techgeek03
Copy link
Author

Hi,
Thanks a lot for your answers, this clears things :) It would be good if the docs are updated as well with this info :)

Regarding availability: as you said OBO flow is silent, and getting the original token require re-login. However, is it correct to say that, from the standpoint of the Web API you will most likely never perform re-login? You usually have a SAP app in front the Web API that will perform the login and store the original token in local storage; and use OBO to call the Web API, which then will again use OBO flow to call other Web API (internal or external). So thus using an in-memory cache could be sufficient for a smaller applications.

Regarding Microsoft.Identity.Client.Extensions.Web it is good that you are rethinking that library. I would personally not use the TokenCache implementations from there as they are implemented now. The IHttpContextAccessor and HttpContex is all over the place and I think you can get rid of it easily :)

@jmprieur
Copy link
Contributor

@techgeek03 : you are right that the Web API would not perform re-login, but it would send back to its caller (the SAP front end app in your case) information so that the user relogs-in in case the refresh token has expired, or the user needs to consent to more scopes;
This is explained in the following sample: https://github.com/Azure-Samples/active-directory-dotnet-native-aspnetcore-v2/tree/master/2.%20Web%20API%20now%20calls%20Microsoft%20Graph part of the Web API tutorial

Regarding your comment on the fact that we'd want to get rid of IHttpContextAccessor: how would you propose we do?

@bgavrilMS
Copy link
Member

@techgeek03 - I updated the guidance on the creation pattern above (point 1).

@MarkZuber
Copy link
Contributor

@jmprieur -- I understand the abstraction hiding of IHttpContextAccessor that is being discussed here. When you're back from vacation, let's discuss.

@jennyf19 jennyf19 added this to the 4.3.2 milestone Aug 22, 2019
@jmprieur
Copy link
Contributor

@MarkZuber : ready to discuss

@AquilaSands
Copy link

Hi,
I've also been looking for something similar but for service to service auth using a bearer token and have been trying to figure out if it is thread safe. Based on the response from @bgavrilMS

You should have 1 Confidential Client Application for each token cache. And we recommend that you have 1 token cache per session, so there should be 1 CCA per session.

is the following implementation for getting the token safe to register as a singleton in .net core?

public class AADConfidentialClient : IServiceApiAuthorizer
{
    private readonly IConfidentialClientApplication _confidentialClient;
    public AADConfidentialClient(IOptions<ConfidentialClientApplicationOptions> options)
    {
        _confidentialClient = ConfidentialClientApplicationBuilder
                    .CreateWithApplicationOptions(options.Value)
                    .Build();
    }

    public async Task<string> GetTokenAsync(IReadOnlyCollection<string> scopes)
    {
        var result = await _confidentialClient.AcquireTokenForClient(scopes).ExecuteAsync();

        return result.AccessToken;
    }
}

@bgavrilMS
Copy link
Member

bgavrilMS commented Aug 27, 2019

@AquilaSands - yes, for Client Credentials flow a singleton should work fine. You are requesting tokens for an application, not for a user. There will only be 1 access token in the in-memory cache (for the app), irrespective of how many users and sessions there are

Note that you're not providing a token cache implementation, which means that MSAL will cache tokens in memory (in this case the one access token). When you restart the app / service, MSAL will have to contact AAD for a new access token, so there will be a small delay. But in any case, MSAL will have to go to AAD every ~1h or so to fetch a new AT as they expire in 1h. So the perf profile of your service varies a bit (~1s) when MSAL cannot serve ATs from its cache. Let me know if this is a concern and I can suggest solutions.

@AquilaSands
Copy link

@bgavrilMS excellent, thanks for the quick reply. I think the perf will be fine if it does turn out to be an issue I can always add something to warm up the token cache and refresh the value periodically and/or stick it in Redis.

@bgavrilMS bgavrilMS removed their assignment Sep 4, 2019
@abatishchev
Copy link
Contributor

Hi! I found this issue while searching and see it's open since 2019. Should it be finished or closed?

@bgavrilMS
Copy link
Member

Hi yes, it is finished. https://github.com/AzureAD/microsoft-identity-web has GA-ed and it offers all the best practices for this scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants