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

Add public API for long-running OBO methods with custom cache key and do not use refresh token for normal OBO #2820

Merged
merged 23 commits into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cc0b587
Add OBO cache key that can be used to find a token instead of a user …
pmaytak Aug 12, 2021
0d27b48
Rename UserAssertionHash to OboCacheKey and logic to set it. Add meth…
pmaytak Aug 14, 2021
84185f1
Merge remote-tracking branch 'origin/master' into pmaytak/obo-key
pmaytak Aug 14, 2021
51a7fa3
Add tests. Minor fix in cache filtering.
pmaytak Aug 17, 2021
0c388a6
Merge remote-tracking branch 'origin/master' into pmaytak/obo-key
pmaytak Aug 17, 2021
dfb40f7
Update the public API, update related tests.
pmaytak Aug 27, 2021
9cea76f
Merge master
pmaytak Aug 27, 2021
fc93c0e
Fix.
pmaytak Aug 27, 2021
e667465
Update logic if token in cache or not with the OBO cache key provided…
pmaytak Aug 29, 2021
dc8af6c
Merge origin/master
pmaytak Nov 3, 2021
48a2f5a
Fix merge issues.
pmaytak Nov 4, 2021
474b810
Create new ILongRunningWebApi interface. Fix how cache key factory cr…
pmaytak Nov 6, 2021
8b8c781
Nits: add comments, refactor.
pmaytak Nov 9, 2021
ebcc5d9
Remove RT from token response for normal OBO flow.
pmaytak Nov 9, 2021
26da477
Add mock in-memory partitioned cache.
pmaytak Nov 9, 2021
61969ba
Fix tests.
pmaytak Nov 10, 2021
5db4195
PR feedback: Rename OboCacheKey to LongRunningOboCacheKey; add logging.
pmaytak Nov 10, 2021
b4e8fbd
Add tests.
pmaytak Nov 10, 2021
627b4d9
Add exception comment to the public API methods. Remove exception if …
pmaytak Nov 12, 2021
f89db0b
Add tests for combinations of long-running and normal OBO calls.
pmaytak Nov 12, 2021
37280d5
Merge remote-tracking branch 'origin/master' into pmaytak/obo-key
pmaytak Nov 12, 2021
52ff2e1
Nits. Add aka.ms link. Fix comments.
pmaytak Nov 16, 2021
8c2f8d4
Merge remote-tracking branch 'origin/master' into pmaytak/obo-key
pmaytak Nov 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,54 @@ internal AcquireTokenOnBehalfOfParameterBuilder(IConfidentialClientApplicationEx

internal static AcquireTokenOnBehalfOfParameterBuilder Create(
IConfidentialClientApplicationExecutor confidentialClientApplicationExecutor,
IEnumerable<string> scopes,
IEnumerable<string> scopes,
UserAssertion userAssertion)
{
return new AcquireTokenOnBehalfOfParameterBuilder(confidentialClientApplicationExecutor)
.WithScopes(scopes)
.WithUserAssertion(userAssertion);
}

internal static AcquireTokenOnBehalfOfParameterBuilder Create(
IConfidentialClientApplicationExecutor confidentialClientApplicationExecutor,
IEnumerable<string> scopes,
UserAssertion userAssertion,
string cacheKey)
{
return new AcquireTokenOnBehalfOfParameterBuilder(confidentialClientApplicationExecutor)
.WithScopes(scopes)
.WithUserAssertion(userAssertion)
.WithCacheKey(cacheKey);
}

internal static AcquireTokenOnBehalfOfParameterBuilder Create(
IConfidentialClientApplicationExecutor confidentialClientApplicationExecutor,
IEnumerable<string> scopes,
string cacheKey)
{
return new AcquireTokenOnBehalfOfParameterBuilder(confidentialClientApplicationExecutor)
.WithScopes(scopes)
.WithCacheKey(cacheKey);
}

private AcquireTokenOnBehalfOfParameterBuilder WithUserAssertion(UserAssertion userAssertion)
{
CommonParameters.AddApiTelemetryFeature(ApiTelemetryFeature.WithUserAssertion);
Parameters.UserAssertion = userAssertion;
return this;
}

/// <summary>
/// Specifies a key by which to look up the token in the cache instead of searching by an assertion.
/// </summary>
/// <param name="cacheKey">Key by which to look up the token in the cache</param>
/// <returns>A builder enabling you to add optional parameters before executing the token request</returns>
private AcquireTokenOnBehalfOfParameterBuilder WithCacheKey(string cacheKey)
{
Parameters.OboCacheKey = cacheKey ?? throw new ArgumentNullException(nameof(cacheKey));
return this;
}

/// <summary>
/// Specifies if the x5c claim (public key of the certificate) should be sent to the STS.
/// Sending the x5c enables application developers to achieve easy certificate roll-over in Azure AD:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Identity.Client.ApiConfig.Parameters;
using Microsoft.Identity.Client.Instance;
using Microsoft.Identity.Client.Internal;
using Microsoft.Identity.Client.Internal.Requests;

Expand Down Expand Up @@ -42,7 +41,7 @@ public async Task<AuthenticationResult> ExecuteAsync(
var handler = new ConfidentialAuthCodeRequest(
ServiceBundle,
requestParams,
authorizationCodeParameters);
authorizationCodeParameters);

return await handler.RunAsync(cancellationToken).ConfigureAwait(false);
}
Expand All @@ -58,9 +57,9 @@ public async Task<AuthenticationResult> ExecuteAsync(
commonParameters,
requestContext,
_confidentialClientApplication.AppTokenCacheInternal).ConfigureAwait(false);

requestParams.SendX5C = clientParameters.SendX5C;

var handler = new ClientCredentialRequest(
ServiceBundle,
requestParams,
Expand All @@ -83,6 +82,7 @@ public async Task<AuthenticationResult> ExecuteAsync(

requestParams.SendX5C = onBehalfOfParameters.SendX5C;
requestParams.UserAssertion = onBehalfOfParameters.UserAssertion;
requestParams.OboCacheKey = onBehalfOfParameters.OboCacheKey;

var handler = new OnBehalfOfRequest(
ServiceBundle,
Expand Down Expand Up @@ -113,7 +113,7 @@ public async Task<Uri> ExecuteAsync(
requestParameters.RedirectUri = new Uri(authorizationRequestUrlParameters.RedirectUri);
}

await requestParameters.AuthorityManager.RunInstanceDiscoveryAndValidationAsync().ConfigureAwait(false);
await requestParameters.AuthorityManager.RunInstanceDiscoveryAndValidationAsync().ConfigureAwait(false);
var handler = new AuthCodeRequestComponent(
requestParameters,
authorizationRequestUrlParameters.ToInteractiveParameters());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ namespace Microsoft.Identity.Client.ApiConfig.Parameters
internal class AcquireTokenOnBehalfOfParameters : IAcquireTokenParameters
{
public UserAssertion UserAssertion { get; set; }
public bool SendX5C { get; set;}
public string OboCacheKey { get; set; }
public bool SendX5C { get; set; }
public bool ForceRefresh { get; set; }

/// <inheritdoc />
Expand All @@ -19,6 +20,8 @@ public void LogParameters(ICoreLogger logger)
builder.AppendLine("=== OnBehalfOfParameters ===");
builder.AppendLine("SendX5C: " + SendX5C);
builder.AppendLine("ForceRefresh: " + ForceRefresh);
builder.AppendLine("UserAssertion set: " + (UserAssertion != null));
builder.AppendLine("OboCacheKey set: " + !string.IsNullOrWhiteSpace(OboCacheKey));
logger.Info(builder.ToString());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ private async Task RefreshCacheForReadOperationsAsync(CacheEvent.TokenTypes cach
TokenType = cacheEventType
};

_requestParams.RequestContext.Logger.Verbose($"[Cache Session Manager] Enterering the cache semaphore. { TokenCacheInternal.Semaphore.GetCurrentCountLogMessage()}");
_requestParams.RequestContext.Logger.Verbose($"[Cache Session Manager] Entering the cache semaphore. { TokenCacheInternal.Semaphore.GetCurrentCountLogMessage()}");
await TokenCacheInternal.Semaphore.WaitAsync(_requestParams.RequestContext.UserCancellationToken).ConfigureAwait(false);
_requestParams.RequestContext.Logger.Verbose("[Cache Session Manager] Entered cache semaphore");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ internal string TenantId
internal string CachedAt { get; set; }
internal string ExpiresOnUnixTimestamp { get; set; }
internal string ExtendedExpiresOnUnixTimestamp { get; set; }
internal string UserAssertionHash { get; set; }

/// <summary>
/// Used to find the token in the cache. Can be a token assertion hash or a user provided key.
/// </summary>
internal string OboCacheKey { get; set; }

/// <summary>
/// Used when the token is bound to a public / private key pair which is identified by a key id (kid).
Expand Down Expand Up @@ -173,12 +177,12 @@ internal static MsalAccessTokenCacheItem FromJObject(JObject j)

var item = new MsalAccessTokenCacheItem(JsonUtils.ExtractExistingOrEmptyString(j, StorageJsonKeys.Target))
{
TenantId = JsonUtils.ExtractExistingOrEmptyString(j, StorageJsonKeys.Realm),
TenantId = JsonUtils.ExtractExistingOrEmptyString(j, StorageJsonKeys.Realm),
CachedAt = cachedAt.ToString(CultureInfo.InvariantCulture),
ExpiresOnUnixTimestamp = expiresOn.ToString(CultureInfo.InvariantCulture),
ExtendedExpiresOnUnixTimestamp = extendedExpiresOn.ToString(CultureInfo.InvariantCulture),
RefreshOnUnixTimestamp = JsonUtils.ExtractExistingOrDefault<string>(j, StorageJsonKeys.RefreshOn),
UserAssertionHash = JsonUtils.ExtractExistingOrEmptyString(j, StorageJsonKeys.UserAssertionHash),
OboCacheKey = JsonUtils.ExtractExistingOrEmptyString(j, StorageJsonKeys.UserAssertionHash),
KeyId = JsonUtils.ExtractExistingOrDefault<string>(j, StorageJsonKeys.KeyId),
TokenType = JsonUtils.ExtractExistingOrDefault<string>(j, StorageJsonKeys.TokenType) ?? StorageJsonValues.TokenTypeBearer
};
Expand All @@ -194,16 +198,16 @@ internal override JObject ToJObject()

SetItemIfValueNotNull(json, StorageJsonKeys.Realm, TenantId);
SetItemIfValueNotNull(json, StorageJsonKeys.Target, _scopes);
SetItemIfValueNotNull(json, StorageJsonKeys.UserAssertionHash, UserAssertionHash);
SetItemIfValueNotNull(json, StorageJsonKeys.UserAssertionHash, OboCacheKey);
SetItemIfValueNotNull(json, StorageJsonKeys.CachedAt, CachedAt);
SetItemIfValueNotNull(json, StorageJsonKeys.ExpiresOn, ExpiresOnUnixTimestamp);
SetItemIfValueNotNull(json, StorageJsonKeys.ExtendedExpiresOn, ExtendedExpiresOnUnixTimestamp);
SetItemIfValueNotNull(json, StorageJsonKeys.KeyId, KeyId);
SetItemIfValueNotNullOrDefault(json, StorageJsonKeys.TokenType, TokenType, StorageJsonValues.TokenTypeBearer);
SetItemIfValueNotNull(json, StorageJsonKeys.RefreshOn, RefreshOnUnixTimestamp);

// previous versions of msal used "ext_expires_on" instead of the correct "extended_expires_on".
// this is here for back compat
// previous versions of MSAL used "ext_expires_on" instead of the correct "extended_expires_on".
// this is here for back compatibility
SetItemIfValueNotNull(json, StorageJsonKeys.ExtendedExpiresOn_MsalCompat, ExtendedExpiresOnUnixTimestamp);

return json;
Expand All @@ -221,7 +225,7 @@ internal MsalAccessTokenCacheKey GetKey()
TenantId,
HomeAccountId,
ClientId,
_scopes,
_scopes,
TokenType);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ internal MsalRefreshTokenCacheItem(
MsalTokenResponse response,
string homeAccountId)
: this(
preferredCacheEnv,
clientId,
response.RefreshToken,
response.ClientInfo,
response.FamilyId,
preferredCacheEnv,
clientId,
response.RefreshToken,
response.ClientInfo,
response.FamilyId,
homeAccountId)
{
}
Expand All @@ -52,7 +52,10 @@ internal MsalRefreshTokenCacheItem(
/// </summary>
public string FamilyId { get; set; }

internal string UserAssertionHash { get; set; }
/// <summary>
/// Used to find the token in the cache. Can be a token assertion hash or a user provided key.
/// </summary>
internal string OboCacheKey { get; set; }
pmaytak marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Family Refresh Tokens, can be used for all clients part of the family
Expand All @@ -78,7 +81,7 @@ internal static MsalRefreshTokenCacheItem FromJObject(JObject j)
{
var item = new MsalRefreshTokenCacheItem();
item.FamilyId = JsonUtils.ExtractExistingOrEmptyString(j, StorageJsonKeys.FamilyId);
item.UserAssertionHash = JsonUtils.ExtractExistingOrEmptyString(j, StorageJsonKeys.UserAssertionHash);
item.OboCacheKey = JsonUtils.ExtractExistingOrEmptyString(j, StorageJsonKeys.UserAssertionHash);

item.PopulateFieldsFromJObject(j);

Expand All @@ -89,7 +92,7 @@ internal override JObject ToJObject()
{
var json = base.ToJObject();
SetItemIfValueNotNull(json, StorageJsonKeys.FamilyId, FamilyId);
SetItemIfValueNotNull(json, StorageJsonKeys.UserAssertionHash, UserAssertionHash);
SetItemIfValueNotNull(json, StorageJsonKeys.UserAssertionHash, OboCacheKey);
return json;
}

Expand Down
4 changes: 2 additions & 2 deletions src/client/Microsoft.Identity.Client/Cache/StorageJsonKeys.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ internal static class StorageJsonKeys
// todo(cache): this needs to be added to the spec. needed for OBO flow on .NET.
public const string UserAssertionHash = "user_assertion_hash";

// previous versions of msal used "ext_expires_on" instead of the correct "extended_expires_on".
// this is here for back compat
// previous versions of MSAL used "ext_expires_on" instead of the correct "extended_expires_on".
// this is here for back compatibility
public const string ExtendedExpiresOn_MsalCompat = "ext_expires_on";
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using Microsoft.Identity.Client.Internal.Requests;

namespace Microsoft.Identity.Client.Cache
Expand Down Expand Up @@ -50,15 +49,15 @@ private static bool GetOboOrAppKey(AuthenticationRequestParameters requestParame
{
if (requestParameters.ApiId == TelemetryCore.Internal.Events.ApiEvent.ApiIds.AcquireTokenOnBehalfOf)
{
key = requestParameters.UserAssertion.AssertionHash;
key = !string.IsNullOrEmpty(requestParameters.OboCacheKey) ? requestParameters.OboCacheKey : requestParameters.UserAssertion.AssertionHash;
return true;
}

if (requestParameters.ApiId == TelemetryCore.Internal.Events.ApiEvent.ApiIds.AcquireTokenForClient)
{
string tenantId = requestParameters.Authority.TenantId ?? "";
key = GetClientCredentialKey(requestParameters.AppConfig.ClientId, tenantId);

return true;
}

Expand All @@ -67,7 +66,7 @@ private static bool GetOboOrAppKey(AuthenticationRequestParameters requestParame
}

public /* for test */ static string GetClientCredentialKey(string clientId, string tenantId)
{
{
return $"{clientId}_{tenantId}_AppTokenCache";
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace Microsoft.Identity.Client
/// and never directly exposed. For details see https://aka.ms/msal-net-client-applications
/// </remarks>
#if !SUPPORTS_CONFIDENTIAL_CLIENT
[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] // hide confidentail client on mobile
[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] // hide confidential client on mobile
#endif
public sealed partial class ConfidentialClientApplication
: ClientApplicationBase,
Expand Down Expand Up @@ -82,7 +82,7 @@ public AcquireTokenByAuthorizationCodeParameterBuilder AcquireTokenByAuthorizati
/// <param name="scopes">scopes requested to access a protected API. For this flow (client credentials), the scopes
/// should be of the form "{ResourceIdUri/.default}" for instance <c>https://management.azure.net/.default</c> or, for Microsoft
/// Graph, <c>https://graph.microsoft.com/.default</c> as the requested scopes are defined statically with the application registration
/// in the portal, and cannot be overriden in the application.</param>
/// in the portal, and cannot be overridden in the application.</param>
/// <returns>A builder enabling you to add optional parameters before executing the token request</returns>
/// <remarks>You can also chain the following optional parameters:
/// <see cref="AcquireTokenForClientParameterBuilder.WithForceRefresh(bool)"/>
Expand Down Expand Up @@ -126,6 +126,47 @@ public AcquireTokenOnBehalfOfParameterBuilder AcquireTokenOnBehalfOf(
userAssertion);
}

/// <inheritdoc />
pmaytak marked this conversation as resolved.
Show resolved Hide resolved
public AcquireTokenOnBehalfOfParameterBuilder InitiateLongRunningProcessInWebApi(
IEnumerable<string> scopes,
string userToken,
ref string longRunningProcessSessionKey)
{
if (userToken == null)
{
throw new ArgumentNullException(nameof(userToken));
}

UserAssertion userAssertion = new UserAssertion(userToken);

if (string.IsNullOrEmpty(longRunningProcessSessionKey))
{
longRunningProcessSessionKey = userAssertion.AssertionHash;
}

return AcquireTokenOnBehalfOfParameterBuilder.Create(
ClientExecutorFactory.CreateConfidentialClientExecutor(this),
scopes,
userAssertion,
longRunningProcessSessionKey);
}

/// <inheritdoc />
public AcquireTokenOnBehalfOfParameterBuilder AcquireTokenInLongRunningProcess(
IEnumerable<string> scopes,
string longRunningProcessSessionKey)
{
if (longRunningProcessSessionKey == null)
{
throw new ArgumentNullException(nameof(longRunningProcessSessionKey));
}

return AcquireTokenOnBehalfOfParameterBuilder.Create(
ClientExecutorFactory.CreateConfidentialClientExecutor(this),
scopes,
longRunningProcessSessionKey);
}

/// <summary>
/// Computes the URL of the authorization request letting the user sign-in and consent to the application accessing specific scopes in
/// the user's name. The URL targets the /authorize endpoint of the authority configured in the application.
Expand Down
Loading