Skip to content

Commit

Permalink
Disable ADAL cache (#2309)
Browse files Browse the repository at this point in the history
* Add public API method to disable ADAL cache. Add if checks around ADAL cache operations.

* Update if AdalCacheEnabled checks.

* Add benchmark for SaveToken method.

* Renaming and comment updates. Add app option.

* Add GetRefreshToken test.

* Fix FindToken test. Move helper methods into LegacyTokenCacheHelper for reusability.

* Cleanup.

* Add RemoveUser and GetAllUsers benchmarks.

* Add unit tests.

* Update XML comment.

* Rename AdalCache to LegacyCache
  • Loading branch information
pmaytak committed Jan 9, 2021
1 parent ab9c52d commit 6b1cff8
Show file tree
Hide file tree
Showing 14 changed files with 419 additions and 169 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,24 @@ internal T WithUserTokenCacheInternalForTest(ITokenCacheInternal tokenCacheInter
return (T)this;
}

/// <summary>
/// Enables legacy ADAL cache serialization and deserialization.
/// </summary>
/// <param name="enableLegacyCacheCompatibility">Enable legacy ADAL cache compatibility.</param>
/// <returns>The builder to chain the .With methods.</returns>
/// <remarks>
/// ADAL is a previous legacy generation of MSAL.NET authentication library.
/// If you don't use <c>.WithLegacyCacheCompatibility(false)</c>, then by default, the ADAL cache is used
/// (along with MSAL cache). <c>true</c> flag is only needed for specific migration scenarios
/// from ADAL.NET to MSAL.NET when both library versions are running side-by-side.
/// To improve performance add <c>.WithLegacyCacheCompatibility(false)</c> unless you care about migration scenarios.
/// </remarks>
public T WithLegacyCacheCompatibility(bool enableLegacyCacheCompatibility = true)
{
Config.LegacyCacheCompatibilityEnabled = enableLegacyCacheCompatibility;
return (T)this;
}

/// <summary>
/// Sets the logging callback. For details see https://aka.ms/msal-net-logging
/// </summary>
Expand Down Expand Up @@ -356,6 +374,7 @@ protected T WithOptions(ApplicationOptions applicationOptions)
WithClientName(applicationOptions.ClientName);
WithClientVersion(applicationOptions.ClientVersion);
WithClientCapabilities(applicationOptions.ClientCapabilities);
WithLegacyCacheCompatibility(applicationOptions.LegacyCacheCompatibilityEnabled);

WithLogging(
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ public string ClientVersion
public bool MergeWithDefaultClaims { get; internal set; }
internal int ConfidentialClientCredentialCount;

public bool LegacyCacheCompatibilityEnabled { get; internal set; } = true;

#region Authority

public InstanceDiscoveryResponse CustomInstanceDiscoveryMetadata { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,5 +121,10 @@ public abstract class ApplicationOptions
/// For more details see https://aka.ms/msal-net-claims-request
/// </remarks>
public IEnumerable<string> ClientCapabilities { get; set; }

/// <summary>
/// Enables legacy ADAL cache serialization and deserialization.
/// </summary>
public bool LegacyCacheCompatibilityEnabled { get; set; }
}
}
4 changes: 4 additions & 0 deletions src/client/Microsoft.Identity.Client/AppConfig/IAppConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ public interface IAppConfig
/// </remarks>
IEnumerable<string> ClientCapabilities { get; }

/// <summary>
/// Enables legacy ADAL cache serialization and deserialization.
/// </summary>
bool LegacyCacheCompatibilityEnabled { get; }

/// <summary>
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,9 @@ public sealed partial class TokenCache : ITokenCacheInternal

UpdateAppMetadata(requestParams.ClientId, instanceDiscoveryMetadata.PreferredCache, response.FamilyId);

// Do not save RT in ADAL cache for confidential client or B2C
if (!requestParams.IsClientCredentialRequest &&
// Do not save RT in ADAL cache for client credentials flow or B2C
if (ServiceBundle.Config.LegacyCacheCompatibilityEnabled &&
!requestParams.IsClientCredentialRequest &&
requestParams.AuthorityInfo.AuthorityType != AuthorityType.B2C)
{
var authorityWithPreferredCache = Authority.CreateAuthorityWithEnvironment(
Expand Down Expand Up @@ -560,9 +561,10 @@ private MsalAccessTokenCacheItem FilterByKeyId(MsalAccessTokenCacheItem item, Au
requestParams.RequestContext.Logger.Info("Checking ADAL cache for matching RT. ");

// ADAL legacy cache does not store FRTs
if (requestParams.Account != null && string.IsNullOrEmpty(familyId))
if (ServiceBundle.Config.LegacyCacheCompatibilityEnabled &&
requestParams.Account != null &&
string.IsNullOrEmpty(familyId))
{

return CacheFallbackOperations.GetRefreshToken(
Logger,
LegacyCachePersistence,
Expand Down Expand Up @@ -631,17 +633,22 @@ async Task<IEnumerable<IAccount>> ITokenCacheInternal.GetAccountsAsync(Authentic
if (logger.IsLoggingEnabled(LogLevel.Verbose))
logger.Verbose($"GetAccounts found {rtCacheItems.Count()} RTs and {accountCacheItems.Count()} accounts in MSAL cache. ");

AdalUsersForMsal adalUsersResult = CacheFallbackOperations.GetAllAdalUsersForMsal(
Logger,
LegacyCachePersistence,
ClientId);

// Multi-cloud support - must filter by env.
ISet<string> allEnvironmentsInCache = new HashSet<string>(
accountCacheItems.Select(aci => aci.Environment),
StringComparer.OrdinalIgnoreCase);
allEnvironmentsInCache.UnionWith(rtCacheItems.Select(rt => rt.Environment));
allEnvironmentsInCache.UnionWith(adalUsersResult.GetAdalUserEnviroments());

AdalUsersForMsal adalUsersResult = null;

if (ServiceBundle.Config.LegacyCacheCompatibilityEnabled)
{
adalUsersResult = CacheFallbackOperations.GetAllAdalUsersForMsal(
Logger,
LegacyCachePersistence,
ClientId);
allEnvironmentsInCache.UnionWith(adalUsersResult.GetAdalUserEnviroments());
}

var instanceMetadata = await ServiceBundle.InstanceDiscoveryManager.GetMetadataEntryTryAvoidNetworkAsync(
requestParameters.AuthorityInfo.CanonicalAuthority,
Expand Down Expand Up @@ -671,11 +678,14 @@ async Task<IEnumerable<IAccount>> ITokenCacheInternal.GetAccountsAsync(Authentic
}
}

UpdateMapWithAdalAccountsWithClientInfo(
environment,
instanceMetadata.Aliases,
adalUsersResult,
clientInfoToAccountMap);
if (ServiceBundle.Config.LegacyCacheCompatibilityEnabled)
{
UpdateMapWithAdalAccountsWithClientInfo(
environment,
instanceMetadata.Aliases,
adalUsersResult,
clientInfoToAccountMap);
}

// Add WAM accounts stored in MSAL's cache - for which we do not have an RT
if (requestParameters.IsBrokerConfigured && ServiceBundle.PlatformProxy.BrokerSupportsWamAccounts)
Expand Down Expand Up @@ -792,7 +802,10 @@ async Task ITokenCacheInternal.RemoveAccountAsync(IAccount account, RequestConte
}

tokenCacheInternal.RemoveMsalAccountWithNoLocks(account, requestContext);
RemoveAdalUser(account);
if (ServiceBundle.Config.LegacyCacheCompatibilityEnabled)
{
RemoveAdalUser(account);
}
}
finally
{
Expand Down
19 changes: 11 additions & 8 deletions src/client/Microsoft.Identity.Client/TokenCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public sealed partial class TokenCache : ITokenCacheInternal
private ICoreLogger Logger => ServiceBundle.DefaultLogger;

internal IServiceBundle ServiceBundle { get; }
internal ILegacyCachePersistence LegacyCachePersistence { get; }
internal ILegacyCachePersistence LegacyCachePersistence { get; set; }
internal string ClientId => ServiceBundle.Config.ClientId;

ITokenCacheAccessor ITokenCacheInternal.Accessor => _accessor;
Expand Down Expand Up @@ -206,23 +206,26 @@ private bool RtMatchesAccount(MsalRefreshTokenCacheItem rtItem, MsalAccountCache
}
}

private static List<IAccount> UpdateWithAdalAccountsWithoutClientInfo(
private List<IAccount> UpdateWithAdalAccountsWithoutClientInfo(
string envFromRequest,
IEnumerable<string> envAliases,
AdalUsersForMsal adalUsers,
IDictionary<string, Account> clientInfoToAccountMap)
{
var accounts = new List<IAccount>();

accounts.AddRange(clientInfoToAccountMap.Values);
var uniqueUserNames = clientInfoToAccountMap.Values.Select(o => o.Username).Distinct().ToList();

foreach (AdalUserInfo user in adalUsers.GetUsersWithoutClientInfo(envAliases))
if (ServiceBundle.Config.LegacyCacheCompatibilityEnabled)
{
if (!string.IsNullOrEmpty(user.DisplayableId) && !uniqueUserNames.Contains(user.DisplayableId))
var uniqueUserNames = clientInfoToAccountMap.Values.Select(o => o.Username).Distinct().ToList();

foreach (AdalUserInfo user in adalUsers.GetUsersWithoutClientInfo(envAliases))
{
accounts.Add(new Account(null, user.DisplayableId, envFromRequest));
uniqueUserNames.Add(user.DisplayableId);
if (!string.IsNullOrEmpty(user.DisplayableId) && !uniqueUserNames.Contains(user.DisplayableId))
{
accounts.Add(new Account(null, user.DisplayableId, envFromRequest));
uniqueUserNames.Add(user.DisplayableId);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Threading.Tasks;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Engines;
using Microsoft.Identity.Client;
using Microsoft.Identity.Client.Instance;
using Microsoft.Identity.Client.Instance.Discovery;
using Microsoft.Identity.Client.Internal;
using Microsoft.Identity.Client.Internal.Requests;
using Microsoft.Identity.Client.OAuth2;
using Microsoft.Identity.Test.Common;
using Microsoft.Identity.Test.Common.Core.Mocks;
using Microsoft.Identity.Test.Unit;

namespace Microsoft.Identity.Test.Performance
{
[GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByMethod)]
public class LegacyCacheOperationsTests
{
private ITokenCacheInternal _cache;
private MsalTokenResponse _response;
private AuthenticationRequestParameters _requestParams;
private RequestContext _requestContext;
private readonly Consumer _consumer = new Consumer();

[Params(1, 100, 1000)]
public int TokenCacheSize { get; set; }

[ParamsAllValues]
public bool EnableLegacyCache { get; set; }

[GlobalSetup]
public void GlobalSetup()
{
var serviceBundle = TestCommon.CreateServiceBundleWithCustomHttpManager(null, isLegacyCacheEnabled: EnableLegacyCache);

_requestContext = new RequestContext(serviceBundle, Guid.NewGuid());
_cache = new TokenCache(serviceBundle, false);
_response = TestConstants.CreateMsalTokenResponse();

_requestParams = TestCommon.CreateAuthenticationRequestParameters(serviceBundle);
_requestParams.TenantUpdatedCanonicalAuthority = Authority.CreateAuthorityWithTenant(
_requestParams.AuthorityInfo,
TestConstants.Utid);
_requestParams.Account = new Account(TestConstants.s_userIdentifier, $"1{TestConstants.DisplayableId}", TestConstants.ProductionPrefNetworkEnvironment);

AddHostToInstanceCache(serviceBundle, TestConstants.ProductionPrefCacheEnvironment);

LegacyTokenCacheHelper.PopulateLegacyCache(serviceBundle.DefaultLogger, _cache.LegacyPersistence, TokenCacheSize);
TokenCacheHelper.AddRefreshTokensToCache(_cache.Accessor, TokenCacheSize);
}

[Benchmark(Description = "SaveToken")]
public async Task<string> SaveTokenResponseTestAsync()
{
var result = await _cache.SaveTokenResponseAsync(_requestParams, _response).ConfigureAwait(true);
return result.Item1.ClientId;
}

[Benchmark(Description = "FindToken")]
public async Task<string> FindRefreshTokenTestAsync()
{
var result = await _cache.FindRefreshTokenAsync(_requestParams).ConfigureAwait(true);
return result?.ClientId;
}

[Benchmark(Description = "GetAllUsers")]
public async Task GetAllAdalUsersTestAsync()
{
var result = await _cache.GetAccountsAsync(_requestParams).ConfigureAwait(true);
result.Consume(_consumer);
}

[Benchmark(Description = "RemoveUser")]
public async Task RemoveAdalUserTestAsync()
{
await _cache.RemoveAccountAsync(_requestParams.Account, _requestContext).ConfigureAwait(true);
}

private void AddHostToInstanceCache(IServiceBundle serviceBundle, string host)
{
(serviceBundle.InstanceDiscoveryManager as InstanceDiscoveryManager)
.AddTestValueToStaticProvider(
host,
new InstanceDiscoveryMetadataEntry
{
PreferredNetwork = host,
PreferredCache = host,
Aliases = new string[]
{
host
}
});
}
}
}
2 changes: 1 addition & 1 deletion tests/Microsoft.Identity.Client.Performance/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Program
{
static void Main(string[] args)
{
BenchmarkRunner.Run<AcquireTokenForClientLargeCacheTests>(
BenchmarkRunner.Run<LegacyCacheOperationsTests>(
DefaultConfig.Instance
.WithOptions(ConfigOptions.DontOverwriteResults)
.AddJob(
Expand Down
Loading

0 comments on commit 6b1cff8

Please sign in to comment.