From 627b4d9325ea74186dd1e0afa59abe18d9b2f22d Mon Sep 17 00:00:00 2001 From: pmaytak <34331512+pmaytak@users.noreply.github.com> Date: Thu, 11 Nov 2021 22:55:07 -0800 Subject: [PATCH] Add exception comment to the public API methods. Remove exception if Initiate is called with already existing key. Update tests. --- .../ILongRunningWebApi.cs | 2 ++ .../Microsoft.Identity.Client/MsalError.cs | 11 +--------- .../MsalErrorMessage.cs | 2 -- .../TokenCache.ITokenCacheInternal.cs | 20 +++---------------- .../RequestsTests/OboRequestTests.cs | 20 +++++++++++-------- 5 files changed, 18 insertions(+), 37 deletions(-) diff --git a/src/client/Microsoft.Identity.Client/ILongRunningWebApi.cs b/src/client/Microsoft.Identity.Client/ILongRunningWebApi.cs index 7ca3be85f3..80f8373b7c 100644 --- a/src/client/Microsoft.Identity.Client/ILongRunningWebApi.cs +++ b/src/client/Microsoft.Identity.Client/ILongRunningWebApi.cs @@ -36,6 +36,8 @@ public interface ILongRunningWebApi /// Scopes requested to access a protected API /// Key by which to look up the token in the cache /// A builder enabling you to add optional parameters before executing the token request + /// is thrown if the token cache does not contain a token + /// with an OBO cache key that matches the . AcquireTokenOnBehalfOfParameterBuilder AcquireTokenInLongRunningProcess(IEnumerable scopes, string longRunningProcessSessionKey); } } diff --git a/src/client/Microsoft.Identity.Client/MsalError.cs b/src/client/Microsoft.Identity.Client/MsalError.cs index 72160265fa..f3f067bb3f 100644 --- a/src/client/Microsoft.Identity.Client/MsalError.cs +++ b/src/client/Microsoft.Identity.Client/MsalError.cs @@ -1032,15 +1032,6 @@ public static class MsalError /// public const string RegionalAndAuthorityOverride = "authority_override_regional"; - /// - /// What happens?The token cache already contains a token with an OBO cache key that - /// matches the longRunningProcessSessionKey passed into . - /// MitigationCall with a new longRunningProcessSessionKey - /// that does not exist in the token cache or call with an already used - /// longRunningProcessSessionKey. - /// - public const string OboCacheKeyAlreadyInCacheError = "obo_cache_key_already_in_cache_error"; - /// /// What happens?The token cache does not contain a token with an OBO cache key that /// matches the longRunningProcessSessionKey passed into . @@ -1050,4 +1041,4 @@ public static class MsalError /// public const string OboCacheKeyNotInCacheError = "obo_cache_key_not_in_cache_error"; } -} +} diff --git a/src/client/Microsoft.Identity.Client/MsalErrorMessage.cs b/src/client/Microsoft.Identity.Client/MsalErrorMessage.cs index 321872a698..ecee2fc578 100644 --- a/src/client/Microsoft.Identity.Client/MsalErrorMessage.cs +++ b/src/client/Microsoft.Identity.Client/MsalErrorMessage.cs @@ -406,8 +406,6 @@ public static string InitializeProcessSecurityError(string errorCode) => public const string RegionalAndAuthorityOverride = "You configured WithAuthority at the request level, and also WithAzureRegion. This is not supported when the environment changes from application to request. Use WithTenantId at the request level instead."; - public const string OboCacheKeyAlreadyInCache = "The token cache already contains a token with an OBO cache key that matches the longRunningProcessSessionKey passed into IConfidentialClientApplication.AcquireTokenInLongRunningProcess method. Call IConfidentialClientApplication.InitiateLongRunningProcessInWebApi method with a new longRunningProcessSessionKey that does not exist in the token cache or call IConfidentialClientApplication.AcquireTokenInLongRunningProcess method with an already used longRunningProcessSessionKey."; - public const string OboCacheKeyNotInCache = "The token cache does not contain a token with an OBO cache key that matches the longRunningProcessSessionKey passed into IConfidentialClientApplication.AcquireTokenInLongRunningProcess method. Call IConfidentialClientApplication.InitiateLongRunningProcessInWebApi method with a new longRunningProcessSessionKey that does not exist in the token cache or call IConfidentialClientApplication.AcquireTokenInLongRunningProcess method with an already used longRunningProcessSessionKey."; } } diff --git a/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs b/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs index 1bff7cfc78..40345e135c 100644 --- a/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs +++ b/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs @@ -434,12 +434,6 @@ async Task ITokenCacheInternal.FindAccessTokenAsync( throw new MsalClientException(MsalError.OboCacheKeyNotInCacheError, MsalErrorMessage.OboCacheKeyNotInCache); } - if (InitiateLongRunningOboWasCalled(requestParams) && tokenCacheItems.Count > 0) - { - logger.Error("[FindAccessTokenAsync] InitiateLongRunningProcessInWebApi was called and OBO token was already found in the cache."); - throw new MsalClientException(MsalError.OboCacheKeyAlreadyInCacheError, MsalErrorMessage.OboCacheKeyAlreadyInCache); - } - // no match if (tokenCacheItems.Count == 0) { @@ -717,7 +711,7 @@ internal async Task ExpireAllAccessTokensForTestAsync() { accessor.SaveAccessToken(atItem.WithExpiresOn(DateTimeOffset.UtcNow)); } - + if (tokenCacheInternal.IsAppSubscribedToSerializationEvents()) { var args = new TokenCacheNotificationArgs( @@ -729,8 +723,8 @@ internal async Task ExpireAllAccessTokensForTestAsync() tokenCacheInternal.HasTokensNoLocks(), default, suggestedCacheKey: null, - suggestedCacheExpiry: null); - + suggestedCacheExpiry: null); + await tokenCacheInternal.OnAfterAccessAsync(args).ConfigureAwait(false); } } @@ -1208,14 +1202,6 @@ bool ITokenCacheInternal.HasTokensNoLocks() .ForEach(accItem => _accessor.DeleteAccount(accItem)); } - // Returns whether InitiateLongRunningProcessInWebAPI was called (user assertion is specified in this case) - private bool InitiateLongRunningOboWasCalled(AuthenticationRequestParameters requestParameters) - { - return requestParameters.ApiId == ApiEvent.ApiIds.AcquireTokenOnBehalfOf && - requestParameters.UserAssertion != null && - !string.IsNullOrEmpty(requestParameters.LongRunningOboCacheKey); - } - // Returns whether AcquireTokenInLongRunningProcess was called (user assertion is null in this case) private bool AcquireTokenInLongRunningOboWasCalled(AuthenticationRequestParameters requestParameters) { diff --git a/tests/Microsoft.Identity.Test.Unit/RequestsTests/OboRequestTests.cs b/tests/Microsoft.Identity.Test.Unit/RequestsTests/OboRequestTests.cs index 007129fab9..72ccf8dcdb 100644 --- a/tests/Microsoft.Identity.Test.Unit/RequestsTests/OboRequestTests.cs +++ b/tests/Microsoft.Identity.Test.Unit/RequestsTests/OboRequestTests.cs @@ -248,7 +248,7 @@ public async Task AcquireTokenByObo_InitiateLongRunningProcessInWebApi_CacheKeyA // Cache is empty or token with the same scopes, OBO cache key, etc. not in cache -> AT and RT are retrieved from IdP and saved Assert.IsNotNull(result); Assert.AreEqual(TestConstants.ATSecret, result.AccessToken); - Assert.AreEqual(result.AuthenticationResultMetadata.TokenSource, TokenSource.IdentityProvider); + Assert.AreEqual(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); MsalAccessTokenCacheItem cachedAccessToken = cca.UserTokenCacheInternal.Accessor.GetAllAccessTokens().Single(); MsalRefreshTokenCacheItem cachedRefreshToken = cca.UserTokenCacheInternal.Accessor.GetAllRefreshTokens().Single(); Assert.AreEqual(oboCacheKey, cachedAccessToken.OboCacheKey); @@ -256,13 +256,17 @@ public async Task AcquireTokenByObo_InitiateLongRunningProcessInWebApi_CacheKeyA Assert.AreEqual(TestConstants.RTSecret, cachedRefreshToken.Secret); userCacheAccess.AssertAccessCounts(1, 1); - // Token with the same scopes, OBO cache key, etc. exists in the cache -> throw error - var exception = await AssertException.TaskThrowsAsync( - () => cca.InitiateLongRunningProcessInWebApi(TestConstants.s_scope, TestConstants.DefaultAccessToken, ref oboCacheKey) - .ExecuteAsync()) - .ConfigureAwait(false); - - Assert.AreEqual(MsalError.OboCacheKeyAlreadyInCacheError, exception.ErrorCode); + // Token with the same scopes, OBO cache key, etc. exists in the cache -> AT is retrieved from the cache + result = await cca.InitiateLongRunningProcessInWebApi(TestConstants.s_scope, TestConstants.DefaultAccessToken, ref oboCacheKey) + .ExecuteAsync().ConfigureAwait(false); + Assert.IsNotNull(result); + Assert.AreEqual(TestConstants.ATSecret, result.AccessToken); + Assert.AreEqual(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); + cachedAccessToken = cca.UserTokenCacheInternal.Accessor.GetAllAccessTokens().Single(); + cachedRefreshToken = cca.UserTokenCacheInternal.Accessor.GetAllRefreshTokens().Single(); + Assert.AreEqual(oboCacheKey, cachedAccessToken.OboCacheKey); + Assert.AreEqual(oboCacheKey, cachedRefreshToken.OboCacheKey); + Assert.AreEqual(TestConstants.RTSecret, cachedRefreshToken.Secret); userCacheAccess.AssertAccessCounts(2, 1); AddMockHandlerAadSuccess(httpManager,