Skip to content

Commit

Permalink
Update for cleaner use
Browse files Browse the repository at this point in the history
  • Loading branch information
bgavrilMS committed Jun 29, 2020
1 parent 6c8d8b7 commit 22cd934
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 42 deletions.
2 changes: 1 addition & 1 deletion src/client/Microsoft.Identity.Client/AccountId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public override bool Equals(object obj)
return false;
}

return string.Compare(Identifier, otherMsalAccountId.Identifier, StringComparison.OrdinalIgnoreCase) == 0;
return string.Equals(Identifier, otherMsalAccountId.Identifier, StringComparison.OrdinalIgnoreCase);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ private async Task RefreshCacheForReadOperationsAsync(CacheEvent.TokenTypes cach
_requestParams.Account,
hasStateChanged: false,
TokenCacheInternal.IsApplicationCache,
_requestParams.SuggestedCacheKey ?? _requestParams.Account?.HomeAccountId?.Identifier);
_requestParams.SuggestedWebAppCacheKey);

try
{
Expand Down
69 changes: 37 additions & 32 deletions src/client/Microsoft.Identity.Client/ClientApplicationBase.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.77

This comment has been minimized.

Copy link
@jmprieur

jmprieur Jun 29, 2020

Contributor

what is 77, @bgavrilMS

This comment has been minimized.

Copy link
@bgavrilMS

bgavrilMS Jun 29, 2020

Author Member

The answer to life, the universe and everything. Oh no, wait, that's 42.

// Licensed under the MIT License.

using System;
Expand Down Expand Up @@ -89,51 +89,56 @@ public async Task<IEnumerable<IAccount>> GetAccountsAsync()
private async Task<IEnumerable<IAccount>> GetAccountsAndSetCacheKeyAsync(string homeAccountId)
{
RequestContext requestContext = CreateRequestContext(Guid.NewGuid());
IEnumerable<IAccount> localAccounts = Enumerable.Empty<IAccount>();
IEnumerable<IAccount> brokerAccounts = Enumerable.Empty<IAccount>();

if (UserTokenCache == null)
{
if (!AppConfig.IsBrokerEnabled)
{
requestContext.Logger.Info("Token cache is null or empty. Returning empty list of accounts.");
}
}
else
{
var authParameters = new AuthenticationRequestParameters(
ServiceBundle,
UserTokenCacheInternal,
new AcquireTokenCommonParameters(),
requestContext);

authParameters.Account = new Account(homeAccountId, null, Authority);
// a simple session consisting of a single call
CacheSessionManager cacheSessionManager = new CacheSessionManager(
UserTokenCacheInternal,
authParameters);
var accountsFromCache = await GetAccountsFromCacheAsync(homeAccountId, requestContext).ConfigureAwait(false);
IEnumerable<IAccount> cacheAndBrokerAccounts =
await MergeWithBrokerAccountsAsync(accountsFromCache).ConfigureAwait(false);


localAccounts = await cacheSessionManager.GetAccountsAsync(Authority).ConfigureAwait(false);
}
return cacheAndBrokerAccounts;
}

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

foreach(IAccount account in brokerAccounts)
foreach (IAccount account in brokerAccounts)
{
if (!localAccounts.Any(x => x.HomeAccountId.Equals(account.HomeAccountId)))
if (!accountsFromCache.Any(x => x.HomeAccountId.Equals(account.HomeAccountId)))
{
(localAccounts as List<IAccount>).Add(account);
allAccounts.Add(account);
}
}

return localAccounts;
return allAccounts;
}

return localAccounts;
return accountsFromCache;
}

private async Task<IEnumerable<IAccount>> GetAccountsFromCacheAsync(
string homeAccountId,
RequestContext requestContext)
{
var authParameters = new AuthenticationRequestParameters(
ServiceBundle,
UserTokenCacheInternal,
new AcquireTokenCommonParameters(),
requestContext);

authParameters.SuggestedWebAppCacheKey = homeAccountId;

// a simple session consisting of a single call
CacheSessionManager cacheSessionManager = new CacheSessionManager(
UserTokenCacheInternal,
authParameters);

return await cacheSessionManager.GetAccountsAsync(Authority).ConfigureAwait(false);
}

/// <summary>
Expand All @@ -151,7 +156,7 @@ public async Task<IEnumerable<IAccount>> GetAccountsAsync(string userFlow)

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

return accounts.Where(acc =>
return accounts.Where(acc =>
acc.HomeAccountId.ObjectId.Split('.')[0].EndsWith(
userFlow, StringComparison.OrdinalIgnoreCase));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public IEnumerable<KeyValuePair<string, string>> GetApiTelemetryFeatures()
public bool HasScopes => Scope != null && Scope.Any();

public string ClientId { get; }

public Uri RedirectUri { get; set; }

/// <summary>
Expand All @@ -86,7 +87,10 @@ public IEnumerable<KeyValuePair<string, string>> GetApiTelemetryFeatures()

public string ClaimsAndClientCapabilities { get; private set; }

public string SuggestedCacheKey { get; set; }
/// <summary>
/// Used by Identity.Web (and others) to store token caches given the 1 cache per user pattern.
/// </summary>
public string SuggestedWebAppCacheKey { get; set; }

/// <summary>
/// Indicates if the user configured claims via .WithClaims. Not affected by Client Capabilities
Expand All @@ -102,7 +106,7 @@ public string Claims

public AuthorityInfo AuthorityOverride => _commonParameters.AuthorityOverride;

internal bool IsBrokerConfigured { get; set; }
internal bool IsBrokerConfigured { get; set; /* set only for test */ }

public IAuthenticationScheme AuthenticationScheme => _commonParameters.AuthenticationScheme;

Expand Down Expand Up @@ -160,6 +164,13 @@ public void LogParameters()
builder.AppendLine("Redirect Uri - " + RedirectUri?.OriginalString);
builder.AppendLine("Extra Query Params Keys (space separated) - " + ExtraQueryParameters.Keys.AsSingleString());
builder.AppendLine("ClaimsAndClientCapabilities - " + ClaimsAndClientCapabilities);
builder.AppendLine("Authority - " + AuthorityInfo?.CanonicalAuthority);
builder.AppendLine("ApiId - " + ApiId);
builder.AppendLine("SuggestedCacheKey - " + SuggestedWebAppCacheKey);
builder.AppendLine("IsConfidentialClient - " + IsConfidentialClient);
builder.AppendLine("SendX5C - " + SendX5C);
builder.AppendLine("LoginHint - " + LoginHint);
builder.AppendLine("IsBrokerConfigured - " + IsBrokerConfigured);

string messageWithPii = builder.ToString();

Expand All @@ -170,6 +181,13 @@ public void LogParameters()
Environment.NewLine);
builder.AppendLine("Scopes - " + Scope?.AsSingleString());
builder.AppendLine("Extra Query Params Keys (space separated) - " + ExtraQueryParameters.Keys.AsSingleString());
builder.AppendLine("ApiId - " + ApiId);
builder.AppendLine("SuggestedCacheKey - " + SuggestedWebAppCacheKey);
builder.AppendLine("IsConfidentialClient - " + IsConfidentialClient);
builder.AppendLine("SendX5C - " + SendX5C);
builder.AppendLine("LoginHint ? " + !string.IsNullOrEmpty(LoginHint));
builder.AppendLine("IsBrokerConfigured - " + IsBrokerConfigured);

logger.InfoPii(messageWithPii, builder.ToString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
if (!_clientParameters.ForceRefresh &&
string.IsNullOrEmpty(AuthenticationRequestParameters.Claims))
{
AuthenticationRequestParameters.SuggestedCacheKey = AuthenticationRequestParameters.ClientId + "_AppTokenCache";
AuthenticationRequestParameters.SuggestedWebAppCacheKey = AuthenticationRequestParameters.ClientId + "_AppTokenCache";
cachedAccessTokenItem = await CacheManager.FindAccessTokenAsync().ConfigureAwait(false);

if (cachedAccessTokenItem != null && !cachedAccessTokenItem.NeedsRefresh())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
// or new assertion has been passed. We should not use Refresh Token
// for the user because the new incoming token may have updated claims
// like mfa etc.
AuthenticationRequestParameters.SuggestedCacheKey = AuthenticationRequestParameters.UserAssertion.AssertionHash;
AuthenticationRequestParameters.SuggestedWebAppCacheKey = AuthenticationRequestParameters.UserAssertion.AssertionHash;
MsalAccessTokenCacheItem msalAccessTokenItem = await CacheManager.FindAccessTokenAsync().ConfigureAwait(false);
if (msalAccessTokenItem != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ public async Task PreRunAsync()
{
IAccount account = await GetAccountFromParamsOrLoginHintAsync(_silentParameters).ConfigureAwait(false);
AuthenticationRequestParameters.Account = account;
AuthenticationRequestParameters.SuggestedWebAppCacheKey = account.HomeAccountId?.Identifier;

AuthenticationRequestParameters.Authority = Authority.CreateAuthorityForRequest(
ServiceBundle.Config.AuthorityInfo,
AuthenticationRequestParameters.AuthorityOverride,
account?.HomeAccountId?.TenantId);
account.HomeAccountId?.TenantId);
}

public async Task<AuthenticationResult> ExecuteAsync(CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
{
try
{
AuthenticationRequestParameters.SuggestedCacheKey = _silentParameters.Account?.HomeAccountId?.Identifier;
_logger.Info("Attempting to acquire token using using local cache...");
return await _clientStrategy.ExecuteAsync(cancellationToken).ConfigureAwait(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ async Task<Tuple<MsalAccessTokenCacheItem, MsalIdTokenCacheItem>> ITokenCacheInt
account,
hasStateChanged: true,
(this as ITokenCacheInternal).IsApplicationCache,
requestParams.SuggestedCacheKey ?? homeAccountId);
requestParams.SuggestedWebAppCacheKey);

#pragma warning disable CS0618 // Type or member is obsolete
HasStateChanged = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,18 @@ internal TokenCacheNotificationArgs(
public bool IsApplicationCache { get; }

/// <summary>
///
/// A suggested token cache key, which can be used with general purpose storage mechanisms that allow
/// storing key-value pairs and key based retrieval. Useful in applications that store 1 token cache per user,
/// the recommended pattern for web apps.
///
/// The value is:
///
/// <list type="bullet">
/// <item>the homeAccountId for AcquireTokenSilent and GetAccount(homeAccountId)</item>
/// <item>clientID + "_AppTokenCache" for AcquireTokenForClient</item>
/// <item>the hash of the original token for AcquireTokenOnBehalfOf</item>
/// <item>null for all other calls, such as PubliClientApplication calls, which should persist the token cache in a single location</item>
/// </list>
/// </summary>
public string SuggestedCacheKey { get; }
}
Expand Down

0 comments on commit 22cd934

Please sign in to comment.