Skip to content

Commit 152af23

Browse files
jennyf19bgavrilMS
jennyf19
and
bgavrilMS
authored
Add suggested cache key for confidential client scenarios (#1908)
* initial commit for adding a suggested cache key to the token args * Update for cleaner use * add a few more tests for cache key * fix tests * More tests * Test Co-authored-by: Bogdan Gavril <[email protected]>
1 parent 0965832 commit 152af23

File tree

11 files changed

+199
-55
lines changed

11 files changed

+199
-55
lines changed

‎src/client/Microsoft.Identity.Client/AccountId.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public override bool Equals(object obj)
8383
return false;
8484
}
8585

86-
return string.Compare(Identifier, otherMsalAccountId.Identifier, StringComparison.OrdinalIgnoreCase) == 0;
86+
return string.Equals(Identifier, otherMsalAccountId.Identifier, StringComparison.OrdinalIgnoreCase);
8787
}
8888

8989
/// <summary>

‎src/client/Microsoft.Identity.Client/Cache/CacheSessionManager.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,15 @@ private async Task RefreshCacheForReadOperationsAsync(CacheEvent.TokenTypes cach
101101
{
102102
if (!_cacheRefreshedForRead) // double check locking
103103
{
104-
105104
using (_requestParams.RequestContext.CreateTelemetryHelper(cacheEvent))
106105
{
107106
TokenCacheNotificationArgs args = new TokenCacheNotificationArgs(
108107
TokenCacheInternal,
109108
_requestParams.ClientId,
110109
_requestParams.Account,
111110
hasStateChanged: false,
112-
TokenCacheInternal.IsApplicationCache);
111+
TokenCacheInternal.IsApplicationCache,
112+
_requestParams.SuggestedWebAppCacheKey);
113113

114114
try
115115
{

‎src/client/Microsoft.Identity.Client/ClientApplicationBase.cs‎

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) Microsoft Corporation. All rights reserved.
1+
// Copyright (c) Microsoft Corporation. All rights reserved.77
22
// Licensed under the MIT License.
33

44
using System;
@@ -75,50 +75,70 @@ internal ClientApplicationBase(ApplicationConfiguration config)
7575
/// Returns all the available <see cref="IAccount">accounts</see> in the user token cache for the application.
7676
/// </summary>
7777
public async Task<IEnumerable<IAccount>> GetAccountsAsync()
78+
{
79+
return await GetAccountsAndSetCacheKeyAsync(null).ConfigureAwait(false);
80+
}
81+
82+
/// <summary>
83+
/// Returns all the available <see cref="IAccount">accounts</see> in the user token cache for the application.
84+
/// Also sets the cache key based on a given home account id, which is the account id of the home account for the user.
85+
/// This uniquely identifies the user across AAD tenants.
86+
/// </summary>
87+
/// <param name="homeAccountId">The identifier is the home account id of the account being targetted in the cache./>
88+
/// </param>
89+
private async Task<IEnumerable<IAccount>> GetAccountsAndSetCacheKeyAsync(string homeAccountId)
7890
{
7991
RequestContext requestContext = CreateRequestContext(Guid.NewGuid());
80-
IEnumerable<IAccount> localAccounts = Enumerable.Empty<IAccount>();
81-
IEnumerable<IAccount> brokerAccounts = Enumerable.Empty<IAccount>();
8292

83-
if (UserTokenCache == null)
84-
{
85-
if (!AppConfig.IsBrokerEnabled)
86-
{
87-
requestContext.Logger.Info("Token cache is null or empty. Returning empty list of accounts.");
88-
}
89-
}
90-
else
91-
{
92-
// a simple session consisting of a single call
93-
CacheSessionManager cacheSessionManager = new CacheSessionManager(
94-
UserTokenCacheInternal,
95-
new AuthenticationRequestParameters(
96-
ServiceBundle,
97-
UserTokenCacheInternal,
98-
new AcquireTokenCommonParameters(),
99-
requestContext));
93+
var accountsFromCache = await GetAccountsFromCacheAsync(homeAccountId, requestContext).ConfigureAwait(false);
94+
IEnumerable<IAccount> cacheAndBrokerAccounts =
95+
await MergeWithBrokerAccountsAsync(accountsFromCache).ConfigureAwait(false);
96+
10097

101-
localAccounts = await cacheSessionManager.GetAccountsAsync(Authority).ConfigureAwait(false);
102-
}
98+
return cacheAndBrokerAccounts;
99+
}
103100

101+
private async Task<IEnumerable<IAccount>> MergeWithBrokerAccountsAsync(IEnumerable<IAccount> accountsFromCache)
102+
{
104103
if (AppConfig.IsBrokerEnabled && ServiceBundle.PlatformProxy.CanBrokerSupportSilentAuth())
105104
{
105+
List<IAccount> allAccounts = new List<IAccount>(accountsFromCache);
106106
//Broker is available so accounts will be merged using home account ID with local accounts taking priority
107107
var broker = ServiceBundle.PlatformProxy.CreateBroker(null);
108-
brokerAccounts = await broker.GetAccountsAsync(AppConfig.ClientId, AppConfig.RedirectUri).ConfigureAwait(false);
108+
var brokerAccounts = await broker.GetAccountsAsync(AppConfig.ClientId, AppConfig.RedirectUri).ConfigureAwait(false);
109109

110-
foreach(IAccount account in brokerAccounts)
110+
foreach (IAccount account in brokerAccounts)
111111
{
112-
if (!localAccounts.Any(x => x.HomeAccountId.Equals(account.HomeAccountId)))
112+
if (!accountsFromCache.Any(x => x.HomeAccountId.Equals(account.HomeAccountId)))
113113
{
114-
(localAccounts as List<IAccount>).Add(account);
114+
allAccounts.Add(account);
115115
}
116116
}
117117

118-
return localAccounts;
118+
return allAccounts;
119119
}
120120

121-
return localAccounts;
121+
return accountsFromCache;
122+
}
123+
124+
private async Task<IEnumerable<IAccount>> GetAccountsFromCacheAsync(
125+
string homeAccountId,
126+
RequestContext requestContext)
127+
{
128+
var authParameters = new AuthenticationRequestParameters(
129+
ServiceBundle,
130+
UserTokenCacheInternal,
131+
new AcquireTokenCommonParameters(),
132+
requestContext);
133+
134+
authParameters.SuggestedWebAppCacheKey = homeAccountId;
135+
136+
// a simple session consisting of a single call
137+
CacheSessionManager cacheSessionManager = new CacheSessionManager(
138+
UserTokenCacheInternal,
139+
authParameters);
140+
141+
return await cacheSessionManager.GetAccountsAsync(Authority).ConfigureAwait(false);
122142
}
123143

124144
/// <summary>
@@ -136,7 +156,7 @@ public async Task<IEnumerable<IAccount>> GetAccountsAsync(string userFlow)
136156

137157
var accounts = await GetAccountsAsync().ConfigureAwait(false);
138158

139-
return accounts.Where(acc =>
159+
return accounts.Where(acc =>
140160
acc.HomeAccountId.ObjectId.Split('.')[0].EndsWith(
141161
userFlow, StringComparison.OrdinalIgnoreCase));
142162
}
@@ -150,7 +170,7 @@ public async Task<IEnumerable<IAccount>> GetAccountsAsync(string userFlow)
150170
/// </param>
151171
public async Task<IAccount> GetAccountAsync(string accountId)
152172
{
153-
var accounts = await GetAccountsAsync().ConfigureAwait(false);
173+
var accounts = await GetAccountsAndSetCacheKeyAsync(accountId).ConfigureAwait(false);
154174
return accounts.FirstOrDefault(account => account.HomeAccountId.Identifier.Equals(accountId, StringComparison.OrdinalIgnoreCase));
155175
}
156176

‎src/client/Microsoft.Identity.Client/Internal/Requests/AuthenticationRequestParameters.cs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ public IEnumerable<KeyValuePair<string, string>> GetApiTelemetryFeatures()
7575
public bool HasScopes => Scope != null && Scope.Any();
7676

7777
public string ClientId { get; }
78+
7879
public Uri RedirectUri { get; set; }
7980

8081
/// <summary>
@@ -86,6 +87,11 @@ public IEnumerable<KeyValuePair<string, string>> GetApiTelemetryFeatures()
8687

8788
public string ClaimsAndClientCapabilities { get; private set; }
8889

90+
/// <summary>
91+
/// Used by Identity.Web (and others) to store token caches given the 1 cache per user pattern.
92+
/// </summary>
93+
public string SuggestedWebAppCacheKey { get; set; }
94+
8995
/// <summary>
9096
/// Indicates if the user configured claims via .WithClaims. Not affected by Client Capabilities
9197
/// </summary>
@@ -100,7 +106,7 @@ public string Claims
100106

101107
public AuthorityInfo AuthorityOverride => _commonParameters.AuthorityOverride;
102108

103-
internal bool IsBrokerConfigured { get; set; }
109+
internal bool IsBrokerConfigured { get; set; /* set only for test */ }
104110

105111
public IAuthenticationScheme AuthenticationScheme => _commonParameters.AuthenticationScheme;
106112

@@ -158,6 +164,13 @@ public void LogParameters()
158164
builder.AppendLine("Redirect Uri - " + RedirectUri?.OriginalString);
159165
builder.AppendLine("Extra Query Params Keys (space separated) - " + ExtraQueryParameters.Keys.AsSingleString());
160166
builder.AppendLine("ClaimsAndClientCapabilities - " + ClaimsAndClientCapabilities);
167+
builder.AppendLine("Authority - " + AuthorityInfo?.CanonicalAuthority);
168+
builder.AppendLine("ApiId - " + ApiId);
169+
builder.AppendLine("SuggestedCacheKey - " + SuggestedWebAppCacheKey);
170+
builder.AppendLine("IsConfidentialClient - " + IsConfidentialClient);
171+
builder.AppendLine("SendX5C - " + SendX5C);
172+
builder.AppendLine("LoginHint - " + LoginHint);
173+
builder.AppendLine("IsBrokerConfigured - " + IsBrokerConfigured);
161174

162175
string messageWithPii = builder.ToString();
163176

@@ -168,6 +181,13 @@ public void LogParameters()
168181
Environment.NewLine);
169182
builder.AppendLine("Scopes - " + Scope?.AsSingleString());
170183
builder.AppendLine("Extra Query Params Keys (space separated) - " + ExtraQueryParameters.Keys.AsSingleString());
184+
builder.AppendLine("ApiId - " + ApiId);
185+
builder.AppendLine("SuggestedCacheKey - " + SuggestedWebAppCacheKey);
186+
builder.AppendLine("IsConfidentialClient - " + IsConfidentialClient);
187+
builder.AppendLine("SendX5C - " + SendX5C);
188+
builder.AppendLine("LoginHint ? " + !string.IsNullOrEmpty(LoginHint));
189+
builder.AppendLine("IsBrokerConfigured - " + IsBrokerConfigured);
190+
171191
logger.InfoPii(messageWithPii, builder.ToString());
172192
}
173193

‎src/client/Microsoft.Identity.Client/Internal/Requests/ClientCredentialRequest.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
4141
if (!_clientParameters.ForceRefresh &&
4242
string.IsNullOrEmpty(AuthenticationRequestParameters.Claims))
4343
{
44+
AuthenticationRequestParameters.SuggestedWebAppCacheKey = AuthenticationRequestParameters.ClientId + "_AppTokenCache";
4445
cachedAccessTokenItem = await CacheManager.FindAccessTokenAsync().ConfigureAwait(false);
4546

4647
if (cachedAccessTokenItem != null && !cachedAccessTokenItem.NeedsRefresh())

‎src/client/Microsoft.Identity.Client/Internal/Requests/OnBehalfOfRequest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
4141
// or new assertion has been passed. We should not use Refresh Token
4242
// for the user because the new incoming token may have updated claims
4343
// like mfa etc.
44-
44+
AuthenticationRequestParameters.SuggestedWebAppCacheKey = AuthenticationRequestParameters.UserAssertion.AssertionHash;
4545
MsalAccessTokenCacheItem msalAccessTokenItem = await CacheManager.FindAccessTokenAsync().ConfigureAwait(false);
4646
if (msalAccessTokenItem != null)
4747
{

‎src/client/Microsoft.Identity.Client/Internal/Requests/Silent/SilentClientAuthStrategy.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,12 @@ public async Task PreRunAsync()
3939
{
4040
IAccount account = await GetAccountFromParamsOrLoginHintAsync(_silentParameters).ConfigureAwait(false);
4141
AuthenticationRequestParameters.Account = account;
42+
AuthenticationRequestParameters.SuggestedWebAppCacheKey = account.HomeAccountId?.Identifier;
4243

4344
AuthenticationRequestParameters.Authority = Authority.CreateAuthorityForRequest(
4445
ServiceBundle.Config.AuthorityInfo,
4546
AuthenticationRequestParameters.AuthorityOverride,
46-
account?.HomeAccountId?.TenantId);
47+
account.HomeAccountId?.TenantId);
4748
}
4849

4950
public async Task<AuthenticationResult> ExecuteAsync(CancellationToken cancellationToken)

‎src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,13 @@ async Task<Tuple<MsalAccessTokenCacheItem, MsalIdTokenCacheItem>> ITokenCacheInt
111111
await _semaphoreSlim.WaitAsync().ConfigureAwait(false);
112112
try
113113
{
114-
115114
var args = new TokenCacheNotificationArgs(
116115
this,
117116
ClientId,
118117
account,
119118
hasStateChanged: true,
120-
(this as ITokenCacheInternal).IsApplicationCache);
119+
(this as ITokenCacheInternal).IsApplicationCache,
120+
requestParams.SuggestedWebAppCacheKey);
121121

122122
#pragma warning disable CS0618 // Type or member is obsolete
123123
HasStateChanged = true;
@@ -163,7 +163,7 @@ async Task<Tuple<MsalAccessTokenCacheItem, MsalIdTokenCacheItem>> ITokenCacheInt
163163
if (!requestParams.IsClientCredentialRequest &&
164164
requestParams.AuthorityInfo.AuthorityType != AuthorityType.B2C)
165165
{
166-
var authorityWithPrefferedCache = Authority.CreateAuthorityWithEnvironment(
166+
var authorityWithPreferredCache = Authority.CreateAuthorityWithEnvironment(
167167
requestParams.TenantUpdatedCanonicalAuthority.AuthorityInfo,
168168
instanceDiscoveryMetadata.PreferredCache);
169169

@@ -172,7 +172,7 @@ async Task<Tuple<MsalAccessTokenCacheItem, MsalIdTokenCacheItem>> ITokenCacheInt
172172
LegacyCachePersistence,
173173
msalRefreshTokenCacheItem,
174174
msalIdTokenCacheItem,
175-
authorityWithPrefferedCache.AuthorityInfo.CanonicalAuthority,
175+
authorityWithPreferredCache.AuthorityInfo.CanonicalAuthority,
176176
msalIdTokenCacheItem.IdToken.ObjectId,
177177
response.Scope);
178178
}
@@ -661,7 +661,13 @@ async Task ITokenCacheInternal.RemoveAccountAsync(IAccount account, RequestConte
661661

662662
try
663663
{
664-
var args = new TokenCacheNotificationArgs(this, ClientId, account, true, (this as ITokenCacheInternal).IsApplicationCache);
664+
var args = new TokenCacheNotificationArgs(
665+
this,
666+
ClientId,
667+
account,
668+
true,
669+
(this as ITokenCacheInternal).IsApplicationCache,
670+
account.HomeAccountId.Identifier);
665671

666672
try
667673
{
@@ -748,7 +754,13 @@ async Task ITokenCacheInternal.ClearAsync()
748754
await _semaphoreSlim.WaitAsync().ConfigureAwait(false);
749755
try
750756
{
751-
TokenCacheNotificationArgs args = new TokenCacheNotificationArgs(this, ClientId, null, true, (this as ITokenCacheInternal).IsApplicationCache);
757+
TokenCacheNotificationArgs args = new TokenCacheNotificationArgs(
758+
this,
759+
ClientId,
760+
null,
761+
true,
762+
(this as ITokenCacheInternal).IsApplicationCache,
763+
null);
752764

753765
try
754766
{

‎src/client/Microsoft.Identity.Client/TokenCacheNotificationArgs.cs‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ internal TokenCacheNotificationArgs(
1616
string clientId,
1717
IAccount account,
1818
bool hasStateChanged,
19-
bool isAppCache)
19+
bool isAppCache,
20+
string suggestedCacheKey = null)
2021
{
2122
TokenCache = tokenCacheSerializer;
2223
ClientId = clientId;
2324
Account = account;
2425
HasStateChanged = hasStateChanged;
2526
IsApplicationCache = isAppCache;
27+
SuggestedCacheKey = suggestedCacheKey;
2628
}
2729

2830
/// <summary>
@@ -55,5 +57,21 @@ internal TokenCacheNotificationArgs(
5557
/// See https://aka.ms/msal-net-app-cache-serialization for details.
5658
/// </remarks>
5759
public bool IsApplicationCache { get; }
60+
61+
/// <summary>
62+
/// A suggested token cache key, which can be used with general purpose storage mechanisms that allow
63+
/// storing key-value pairs and key based retrieval. Useful in applications that store 1 token cache per user,
64+
/// the recommended pattern for web apps.
65+
///
66+
/// The value is:
67+
///
68+
/// <list type="bullet">
69+
/// <item>the homeAccountId for AcquireTokenSilent and GetAccount(homeAccountId)</item>
70+
/// <item>clientID + "_AppTokenCache" for AcquireTokenForClient</item>
71+
/// <item>the hash of the original token for AcquireTokenOnBehalfOf</item>
72+
/// <item>null for all other calls, such as PubliClientApplication calls, which should persist the token cache in a single location</item>
73+
/// </list>
74+
/// </summary>
75+
public string SuggestedCacheKey { get; }
5876
}
5977
}

0 commit comments

Comments
 (0)