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

Implement consistent Token Caching support that builds on MSAL cache #25361

Closed
christothes opened this issue Nov 16, 2021 · 25 comments
Closed

Implement consistent Token Caching support that builds on MSAL cache #25361

christothes opened this issue Nov 16, 2021 · 25 comments
Assignees
Labels
Azure.Identity feature-request This issue requires a new behavior in the product in order be resolved.

Comments

@christothes
Copy link
Member

MSAL's cache implements some fairly complex logic that we do not ever intend to duplicate in Azure.Identity.

Ideally we would utilize the underlying MSAL library's cache to offer a consistent caching experience. MSAL currently it does not expose its cache directly. This may be something that becomes available in the future as a feature enhancement.

@mikequ-taggysoft
Copy link

mikequ-taggysoft commented Nov 17, 2021

This is a key feature and without it, we cannot comfortably migrate from AppAuthentication to Azure.Identity for Azure apps that are not natively integrated with the latter SDK.

This includes using managed identity with things like various Azure PaaS database services (Azure SQL, Azure PostgreSQL) or authenticating against Azure AD protected web apps/function apps etc.

Hopefully it will be implemented in Azure.Identity soon and then this library can truly and totally replace the legacy one.

@cadwal
Copy link

cadwal commented Nov 19, 2021

Whether you roll your own cache or borrow from BearerTokenAuthenticationPolicy in some way there is a problem with freely choosing the tokenRefreshOffset. BearerTokenAuthenticationPolicy has a default of 5m which happens to be the same as what some of the the underlying TokenCredential variants uses. Those with MSAL i guess, but yes, that code was complex enough that I could not find where its equivalent to tokenRefreshOffset was set or what it was set to, I only observed the behaviour...

If you choose something longer than 5m (like I happened to do with 10m) you end up with the code trying to refresh the token but getting the same token back from the TokenCredential so for the difference of the duration your cache is suddenly "read-through" since the newly fetched token has the same Expiry time as the old. With the logic in BearerTokenAuthenticationPolicy this does not affect the caller with a performance hit, but some additional CPU will be used and if you have AppInsights active you will get a lot of unexpected logging from the TokenCredential code.

Can someone add some advice on how to best choose the tokenRefreshOffset or should we just assume 5m and hope for the best in the future?

@christothes
Copy link
Member Author

Requesting a token more frequently from AAD does not guarantee that a new token will be received since it does its own caching as well. The only benefit that BearerTokenAuthenticationPolicy attempts to provide is to reduce the chance that a network call is required in the hot path of TokenCredential.GetToken.

@cadwal
Copy link

cadwal commented Nov 19, 2021

I would actually be very surprised if two requests for tokens from AAD returned the same token unless I hammered it hard with requests and hit an output cache or something like that. More likely to get a 429 response in that case would be my guess.
For the kind of access token this concerns and for requests 50m apart it seems very unlikely. But of course anything is possible.

@MatthewSteeples
Copy link
Contributor

@cadwal In that case be very surprised. Managed Identity returns the same token for up to 24 hours, which has been a real pain when making changes to roles as they don't take effect until a new token is generated.

@cadwal
Copy link

cadwal commented Nov 19, 2021

No, that is not really a surprise.

I assume you are talking about AppServices, Functions or AKS Linux Pods here and to aquire a token for the MI:s involved there, the library you use have to (the last time I looked at least) do a network call to the sidecar that has access to the MI and its secret (I don't think the MSI_SECRET env variable is the actual MI secret at least) and can do the actual AAD call to aquire the token. There might be more levels of libraries involved in the sidecar that I don't know about since that code is not exactly public as far as I know. All that code might do whatever it wants in the way of caching causing that behaviour, but it also means that those MI:s have a different token policy with a longer lifetime than the usual 1h, wonder why that was done and if one can change that... Might there also be a difference between System Assigned and User Assigned?

Though I also wonder if the innermost layer there uses MSAL and has that same issue with that cache not allowing for new tokens until 5m before expiry. Using Windows AKS Pods here that don't have Pod Indentities yet so have not come across more details on that...

But essentially I guess then there is no known better choice than 5m as a RefreshOffset for any top level cache as discussed here.

@Meertman
Copy link

@christothes, I think this one is also related to: AzureAD/microsoft-authentication-library-for-dotnet#2597

@eluchsinger
Copy link

eluchsinger commented Feb 14, 2022

The lack of this feature makes a migration away from AppAuthentication unreasonable for at least SQL Server Authentication use cases.

The migration below causes 3 calls to the token endpoint, to check for migrations and apply them if needed and each call is 10 seconds right now (I'm not sure why it's taking so long though).

image

Legacy

var tokenProvider = new AzureServiceTokenProvider();
return await tokenProvider.GetAccessTokenAsync(ResourceId);

New

var context = new TokenRequestContext(new[] { ResourceId + "/.default" });
var credential = new DefaultAzureCredential();
var accessToken = await credential.GetTokenAsync(context);
return accessToken.Token;

@christothes
Copy link
Member Author

The migration below causes 3 calls to the token endpoint, to check for migrations and apply them if needed and each call is 10 seconds right now (I'm not sure why it's taking so long though).

The delay you are experiencing is a known issue unrelated to caching - see #24767

@eluchsinger
Copy link

@christothes it did sound like it was unrelated to caching. Still, the migration is not an option for an app that should scale. Unless someone wants to build a custom caching mechanism.

@christothes
Copy link
Member Author

Thanks for the feedback. We're also working on a caching strategy for ManagedIdentityCredential involving improved support from MSAL. I don't have an exact timeframe to share, but it is on the radar for sure.

@cwe1ss
Copy link

cwe1ss commented Mar 17, 2022

This missing cache also prevents us from fully migrating away from AppAuthentication.

I'd love to also have an option in the cache that automatically refreshes the tokens in the background before they expire, so that no actual user-facing request ever faces the penalty of waiting for the token response.

We're currently doing this with AppAuthentication where a IHostedService just calls GetAccessTokenAsync(resource, forceRefresh: true) on each dependant AAD resource every 45 min. This helped a lot with making our response times more consistent.

@Rookian
Copy link

Rookian commented Jul 1, 2022

@eluchsinger

The lack of this feature makes a migration away from AppAuthentication unreasonable for at least SQL Server Authentication use cases.

The migration below causes 3 calls to the token endpoint, to check for migrations and apply them if needed and each call is 10 seconds right now (I'm not sure why it's taking so long though).

For SQL Server authentication you don't need to get the token by yourself anymore, but use the new Active Directory Default authentication provider, which can you specify in the connection string like this
Authentication=Active Directory Default;

@Tiberriver256
Copy link

If anyone lands here and needs a band-aid solution you can use this to wrap any TokenCredential.

using System.Text.Json;
using Azure.Core;
using Microsoft.Extensions.Caching.Memory;

public class CachedTokenCredential : TokenCredential {
    private readonly IMemoryCache cache;
    private readonly TokenCredential credential;

    public CachedTokenCredential(IMemoryCache cache, TokenCredential credential) {
        this.cache = cache;
        this.credential = credential;
    }

    public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken) {
        if (!this.cache.TryGetValue(GetCacheKey(requestContext), out string token)) {
            AccessToken tokenModel = this.credential.GetToken(
                requestContext,
                cancellationToken
            );

            MemoryCacheEntryOptions? options = new MemoryCacheEntryOptions().SetAbsoluteExpiration(
                tokenModel.ExpiresOn.AddMinutes(-1)
            );

            token = JsonSerializer.Serialize(tokenModel);

            this.cache.Set(GetCacheKey(requestContext), token, options);
        }

        return JsonSerializer.Deserialize<AccessToken>(token);
    }

    public override async ValueTask<AccessToken> GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken) {
        if (!this.cache.TryGetValue(GetCacheKey(requestContext), out string token)) {
            AccessToken tokenModel = await this.credential.GetTokenAsync(
                requestContext,
                cancellationToken
            );

            MemoryCacheEntryOptions? options = new MemoryCacheEntryOptions().SetAbsoluteExpiration(
                tokenModel.ExpiresOn.AddMinutes(-1)
            );

            token = JsonSerializer.Serialize(tokenModel);

            this.cache.Set(GetCacheKey(requestContext), token, options);
        }

        return JsonSerializer.Deserialize<AccessToken>(token);
    }

    private static string GetCacheKey(TokenRequestContext tokenRequestContext) =>
        $"{tokenRequestContext.TenantId}{string.Join('-', tokenRequestContext.Scopes)}";
}

@scottaddie
Copy link
Member

Thanks for the feedback. We're also working on a caching strategy for ManagedIdentityCredential involving improved support from MSAL. I don't have an exact timeframe to share, but it is on the radar for sure.

It seems there's a lot of interest in Chris' comment, so I want to provide an update. The ManagedIdentityCredential token caching feature shipped in the 1.7.0-beta.1 release of Azure.Identity. We expect to GA it in October.

@amirschw
Copy link

It seems there's a lot of interest in Chris' comment, so I want to provide an update. The ManagedIdentityCredential token caching feature shipped in the 1.7.0-beta.1 release of Azure.Identity. We expect to GA it in October.

Hey @scottaddie, I see that the token caching feature was removed from the 1.7.0 release of Azure.Identity. Is it still planned to be GA'ed in October?

@scottaddie
Copy link
Member

scottaddie commented Sep 23, 2022

@amirschw The 1.8.0 release will GA in October and will include token caching for ManagedIdentityCredential. We had to ship the 1.7.0 release sooner than expected to include this feature for multi-tenant apps. The 1.8.0-beta.1 release, which reintroduces token caching, is expected early next week.

@scottaddie
Copy link
Member

UPDATE: The 1.8.0 GA release has slipped to November.

Why? The Azure SDK team strives to achieve feature parity across the most popular programming languages we support. We have numerous customers who use the Azure Identity library across multiple languages. Consistency is a key ingredient in their developer experience. In this situation, our Azure Identity for JavaScript library's unit tests for Managed Identity token caching still need some work. And we don't want to rush a feature out the door that doesn't meet our quality bar. Thanks for your patience.

@rguthriemsft
Copy link

Any plans to also support token caching with InteractiveBrowserCredential? Our scenario is console app that uses Azure SDK and we want to be able to have similar expereince to az cli where user logs in and can execute mutliple CLI commands without needing to re-authenticate. We could implement our own tokencredential that does aquiresilent and acquire interactive with MSAL but we don't want to take on the burden of securing/maintaining secure implementation of token cache.

@christothes
Copy link
Member Author

Any plans to also support token caching with InteractiveBrowserCredential? Our scenario is console app that uses Azure SDK and we want to be able to have similar expereince to az cli where user logs in and can execute mutliple CLI commands without needing to re-authenticate. We could implement our own tokencredential that does aquiresilent and acquire interactive with MSAL but we don't want to take on the burden of securing/maintaining secure implementation of token cache.

Does the following sample work for your scenario? https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/identity/Azure.Identity/samples/TokenCache.md

@scottaddie
Copy link
Member

UPDATE: The 1.8.0 GA release shipped today!

@amirschw
Copy link

amirschw commented Nov 9, 2022

@scottaddie the release notes don't say anything about Managed Identity token caching 🧐

@scottaddie
Copy link
Member

scottaddie commented Nov 9, 2022

@amirschw It's mentioned in the changelog entry for the initial 1.8.0 Beta release: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/identity/Azure.Identity/CHANGELOG.md#180-beta1-2022-10-13

@kaledkowski
Copy link

Could anyone explain how to use this feature with DefaultAzureCredential? Obviously, the DefaultAzureCredentialOptions does not implement ITokenCacheOptions

@christothes
Copy link
Member Author

Hi @kaledkowski - The feature is to enable caching where there was previously none. Managed Identity does not support the advanced cache features related to ITokenCacheOptions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity feature-request This issue requires a new behavior in the product in order be resolved.
Projects
None yet
Development

No branches or pull requests