From 6e6082ef400ad2278ff8ca1af9c7b23f5b52ddca Mon Sep 17 00:00:00 2001 From: trwalke Date: Wed, 24 May 2023 01:11:29 -0700 Subject: [PATCH 01/10] Adding support for MSAL telemetry cache data. --- Directory.Build.props | 2 +- .../MsalDistributedTokenCacheAdapter.cs | 26 ++++++++++ .../MsalAbstractTokenCacheProvider.cs | 14 ++++++ .../TestMsalDistributedTokenCacheAdapter.cs | 6 +++ .../L1L2CacheTests.cs | 48 +++++++++++++++++-- 5 files changed, 92 insertions(+), 4 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 821031b72..88c87bdb5 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -64,7 +64,7 @@ 6.30.0 - 4.54.0 + 4.54.1 3.3.0 4.7.2 4.1.0 diff --git a/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs b/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs index b0ff7ab01..3c7f8e5d5 100644 --- a/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs +++ b/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs @@ -9,6 +9,8 @@ using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; +using Microsoft.IdentityModel.Abstractions; namespace Microsoft.Identity.Web.TokenCacheProviders.Distributed { @@ -144,6 +146,20 @@ await L2OperationWithRetryOnFailureAsync( /// Read blob representing a token cache for the cache key /// (account or app). protected override async Task ReadCacheBytesAsync(string cacheKey, CacheSerializerHints cacheSerializerHints) + { + return await ReadCacheBytesAsync(cacheKey, new CacheSerializerHints(), null).ConfigureAwait(false); + } + + /// + /// Read a specific token cache, described by its cache key, from the + /// distributed cache. + /// + /// Key of the cache item to retrieve. + /// Hints for the cache serialization implementation optimization. + /// Stores details to log to the + /// Read blob representing a token cache for the cache key + /// (account or app). + protected override async Task ReadCacheBytesAsync(string cacheKey, CacheSerializerHints cacheSerializerHints, TelemetryData? telemetryData) { const string read = "Read"; byte[]? result = null; @@ -185,6 +201,11 @@ await L2OperationWithRetryOnFailureAsync( Logger.BackPropagateL2toL1(_logger, memoryCacheEntryOptions.Size ?? 0); _memoryCache.Set(cacheKey, result, memoryCacheEntryOptions); Logger.MemoryCacheCount(_logger, _memoryCacheType, read, _memoryCache.Count); + + if (telemetryData != null) + { + telemetryData.CacheLevel = Client.Cache.CacheLevel.L2Cache; + } } } } @@ -195,6 +216,11 @@ await L2OperationWithRetryOnFailureAsync( (cacheKey) => _distributedCache.RefreshAsync(cacheKey, cacheSerializerHints.CancellationToken), cacheKey, result!).ConfigureAwait(false); + + if (telemetryData != null) + { + telemetryData.CacheLevel = Client.Cache.CacheLevel.L1Cache; + } } #pragma warning disable CS8603 // Possible null reference return. diff --git a/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs b/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs index 57fc0e96e..c78275578 100644 --- a/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs +++ b/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs @@ -7,6 +7,8 @@ using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.Logging; using Microsoft.Identity.Client; +using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; +using Microsoft.IdentityModel.Abstractions; namespace Microsoft.Identity.Web.TokenCacheProviders { @@ -219,6 +221,18 @@ protected virtual Task WriteCacheBytesAsync(string cacheKey, byte[] bytes, Cache return ReadCacheBytesAsync(cacheKey); // default implementation avoids a breaking change. } + /// + /// Method to be overridden by concrete cache serializers to Read the cache bytes. + /// + /// Cache key. + /// Hints for the cache serialization implementation optimization. + /// Stores details to log to the + /// Read bytes. + protected virtual Task ReadCacheBytesAsync(string cacheKey, CacheSerializerHints cacheSerializerHints, TelemetryData? telemetryData) + { + return ReadCacheBytesAsync(cacheKey); // default implementation avoids a breaking change. + } + /// /// Method to be implemented by concrete cache serializers to remove an entry from the cache. /// diff --git a/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/TestMsalDistributedTokenCacheAdapter.cs b/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/TestMsalDistributedTokenCacheAdapter.cs index 18ae3b83e..4659b8a98 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/TestMsalDistributedTokenCacheAdapter.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/TestMsalDistributedTokenCacheAdapter.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; using Microsoft.Identity.Web.TokenCacheProviders; using Microsoft.Identity.Web.TokenCacheProviders.Distributed; @@ -42,5 +43,10 @@ public async Task TestWriteCacheBytesAsync(string cacheKey, byte[] bytes, CacheS { return await ReadCacheBytesAsync(cacheKey).ConfigureAwait(false); } + + public async Task TestReadCacheBytesAsync(string cacheKey, TelemetryData telemetryData) + { + return await ReadCacheBytesAsync(cacheKey, new CacheSerializerHints(), telemetryData).ConfigureAwait(false); + } } } diff --git a/tests/Microsoft.Identity.Web.Test/L1L2CacheTests.cs b/tests/Microsoft.Identity.Web.Test/L1L2CacheTests.cs index df3ca637d..8857f3d38 100644 --- a/tests/Microsoft.Identity.Web.Test/L1L2CacheTests.cs +++ b/tests/Microsoft.Identity.Web.Test/L1L2CacheTests.cs @@ -7,6 +7,8 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Identity.Client.Cache; +using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; using Microsoft.Identity.Web.Test.Common.TestHelpers; using Microsoft.Identity.Web.TokenCacheProviders; using Microsoft.Identity.Web.TokenCacheProviders.Distributed; @@ -181,6 +183,7 @@ public async Task SetL1Cache_ReadL1_TestAsync() // Arrange byte[] cache = new byte[3]; cache[0] = 4; + TelemetryData telemetryData = new TelemetryData(); AssertCacheValues(_testCacheAdapter); Assert.NotNull(_testCacheAdapter._memoryCache); Assert.Equal(0, _testCacheAdapter._memoryCache.Count); @@ -189,11 +192,12 @@ public async Task SetL1Cache_ReadL1_TestAsync() Assert.Empty(L2Cache._dict); // Act - byte[]? result = await _testCacheAdapter.TestReadCacheBytesAsync(DefaultCacheKey).ConfigureAwait(false); + byte[]? result = await _testCacheAdapter.TestReadCacheBytesAsync(DefaultCacheKey, telemetryData).ConfigureAwait(false); // Assert Assert.NotNull(result); Assert.Equal(4, result[0]); + Assert.Equal(CacheLevel.L1Cache, telemetryData.CacheLevel); } [Fact] @@ -202,6 +206,7 @@ public async Task EmptyL1Cache_ReadL2AndSetL1_TestAsync() // Arrange byte[] cache = new byte[3]; cache[0] = 4; + TelemetryData telemetryData = new TelemetryData(); AssertCacheValues(_testCacheAdapter); _testCacheAdapter._distributedCache.Set(DefaultCacheKey, cache); Assert.Single(L2Cache._dict); @@ -209,13 +214,48 @@ public async Task EmptyL1Cache_ReadL2AndSetL1_TestAsync() Assert.Equal(0, _testCacheAdapter._memoryCache.Count); // Act - byte[]? result = await _testCacheAdapter.TestReadCacheBytesAsync(DefaultCacheKey).ConfigureAwait(false); + byte[]? result = await _testCacheAdapter.TestReadCacheBytesAsync(DefaultCacheKey, telemetryData).ConfigureAwait(false); // Assert Assert.NotNull(result); Assert.Equal(4, result[0]); Assert.Equal(1, _testCacheAdapter._memoryCache.Count); Assert.Single(L2Cache._dict); + Assert.Equal(CacheLevel.L2Cache, telemetryData.CacheLevel); + } + + [Fact] + public async Task EmptyL1Cache_ReadL2AndSetL1_ForTelemetryTestAsync() + { + // Arrange + byte[] cache = new byte[3]; + cache[0] = 4; + AssertCacheValues(_testCacheAdapter); + _testCacheAdapter._distributedCache.Set(DefaultCacheKey, cache); + TelemetryData telemetryData = new TelemetryData(); + Assert.Single(L2Cache._dict); + Assert.NotNull(_testCacheAdapter._memoryCache); + Assert.Equal(0, _testCacheAdapter._memoryCache.Count); + + // Act + byte[]? result = await _testCacheAdapter.TestReadCacheBytesAsync(DefaultCacheKey, telemetryData).ConfigureAwait(false); + + // Assert + Assert.NotNull(result); + Assert.Equal(4, result[0]); + Assert.Equal(1, _testCacheAdapter._memoryCache.Count); + Assert.Single(L2Cache._dict); + Assert.Equal(CacheLevel.L2Cache, telemetryData.CacheLevel); + + // Act + result = await _testCacheAdapter.TestReadCacheBytesAsync(DefaultCacheKey, telemetryData).ConfigureAwait(false); + + // Assert + Assert.NotNull(result); + Assert.Equal(4, result[0]); + Assert.Equal(1, _testCacheAdapter._memoryCache.Count); + Assert.Single(L2Cache._dict); + Assert.Equal(CacheLevel.L1Cache, telemetryData.CacheLevel); } [Fact] @@ -224,17 +264,19 @@ public async Task EmptyL1L2Cache_ReturnNullCacheResult_TestAsync() // Arrange byte[] cache = new byte[3]; cache[0] = 4; + TelemetryData telemetryData = new TelemetryData(); AssertCacheValues(_testCacheAdapter); Assert.NotNull(_testCacheAdapter._memoryCache); Assert.Equal(0, _testCacheAdapter._memoryCache.Count); // Act - byte[]? result = await _testCacheAdapter.TestReadCacheBytesAsync(DefaultCacheKey).ConfigureAwait(false); + byte[]? result = await _testCacheAdapter.TestReadCacheBytesAsync(DefaultCacheKey, telemetryData).ConfigureAwait(false); // Assert Assert.Null(result); Assert.Equal(0, _testCacheAdapter._memoryCache.Count); Assert.Empty(L2Cache._dict); + Assert.Equal(CacheLevel.None, telemetryData.CacheLevel); } [Fact] From d61d430c02d83e6f34083a6de44cef0f38b2b155 Mon Sep 17 00:00:00 2001 From: trwalke Date: Thu, 1 Jun 2023 09:05:21 -0700 Subject: [PATCH 02/10] Refactoring telemetryData to remove it from public api --- .../CacheSerializerHints.cs | 7 +++++++ .../MsalDistributedTokenCacheAdapter.cs | 15 +-------------- .../MsalAbstractTokenCacheProvider.cs | 14 +------------- .../Properties/InternalsVisibleTo.cs | 1 + .../TestMsalDistributedTokenCacheAdapter.cs | 2 +- 5 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.Identity.Web.TokenCache/CacheSerializerHints.cs b/src/Microsoft.Identity.Web.TokenCache/CacheSerializerHints.cs index 2ea239e18..7734c73c1 100644 --- a/src/Microsoft.Identity.Web.TokenCache/CacheSerializerHints.cs +++ b/src/Microsoft.Identity.Web.TokenCache/CacheSerializerHints.cs @@ -3,6 +3,8 @@ using System; using System.Threading; +using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; +using Microsoft.IdentityModel.Abstractions; namespace Microsoft.Identity.Web.TokenCacheProviders { @@ -21,5 +23,10 @@ public class CacheSerializerHints /// with the app token cache. /// public DateTimeOffset? SuggestedCacheExpiry { get; set; } + + /// + /// Stores details to log to the + /// + internal TelemetryData? TelemetryData { get; set; } } } diff --git a/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs b/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs index 3c7f8e5d5..0e32cb40e 100644 --- a/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs +++ b/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs @@ -146,23 +146,10 @@ await L2OperationWithRetryOnFailureAsync( /// Read blob representing a token cache for the cache key /// (account or app). protected override async Task ReadCacheBytesAsync(string cacheKey, CacheSerializerHints cacheSerializerHints) - { - return await ReadCacheBytesAsync(cacheKey, new CacheSerializerHints(), null).ConfigureAwait(false); - } - - /// - /// Read a specific token cache, described by its cache key, from the - /// distributed cache. - /// - /// Key of the cache item to retrieve. - /// Hints for the cache serialization implementation optimization. - /// Stores details to log to the - /// Read blob representing a token cache for the cache key - /// (account or app). - protected override async Task ReadCacheBytesAsync(string cacheKey, CacheSerializerHints cacheSerializerHints, TelemetryData? telemetryData) { const string read = "Read"; byte[]? result = null; + var telemetryData = cacheSerializerHints.TelemetryData; if (_memoryCache != null) { diff --git a/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs b/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs index c78275578..5085d73f3 100644 --- a/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs +++ b/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs @@ -104,7 +104,7 @@ private byte[] ProtectBytes(byte[] msalBytes) } private static CacheSerializerHints CreateHintsFromArgs(TokenCacheNotificationArgs args) => new CacheSerializerHints - { CancellationToken = args.CancellationToken, SuggestedCacheExpiry = args.SuggestedCacheExpiry }; + { CancellationToken = args.CancellationToken, SuggestedCacheExpiry = args.SuggestedCacheExpiry, TelemetryData = args.TelemetryData }; private async Task OnBeforeAccessAsync(TokenCacheNotificationArgs args) { @@ -221,18 +221,6 @@ protected virtual Task WriteCacheBytesAsync(string cacheKey, byte[] bytes, Cache return ReadCacheBytesAsync(cacheKey); // default implementation avoids a breaking change. } - /// - /// Method to be overridden by concrete cache serializers to Read the cache bytes. - /// - /// Cache key. - /// Hints for the cache serialization implementation optimization. - /// Stores details to log to the - /// Read bytes. - protected virtual Task ReadCacheBytesAsync(string cacheKey, CacheSerializerHints cacheSerializerHints, TelemetryData? telemetryData) - { - return ReadCacheBytesAsync(cacheKey); // default implementation avoids a breaking change. - } - /// /// Method to be implemented by concrete cache serializers to remove an entry from the cache. /// diff --git a/src/Microsoft.Identity.Web.TokenCache/Properties/InternalsVisibleTo.cs b/src/Microsoft.Identity.Web.TokenCache/Properties/InternalsVisibleTo.cs index bdaa961fd..41022c2b1 100644 --- a/src/Microsoft.Identity.Web.TokenCache/Properties/InternalsVisibleTo.cs +++ b/src/Microsoft.Identity.Web.TokenCache/Properties/InternalsVisibleTo.cs @@ -5,3 +5,4 @@ // Allow this assembly to be serviced when run on desktop CLR [assembly: InternalsVisibleTo("Microsoft.Identity.Web.Test, PublicKey=00240000048000009400000006020000002400005253413100040000010001002D96616729B54F6D013D71559A017F50AA4861487226C523959D1579B93F3FDF71C08B980FD3130062B03D3DE115C4B84E7AC46AEF5E192A40E7457D5F3A08F66CEAB71143807F2C3CB0DA5E23B38F0559769978406F6E5D30CEADD7985FC73A5A609A8B74A1DF0A29399074A003A226C943D480FEC96DBEC7106A87896539AD")] +[assembly: InternalsVisibleTo("Microsoft.Identity.Web.Test.Common, PublicKey=00240000048000009400000006020000002400005253413100040000010001002D96616729B54F6D013D71559A017F50AA4861487226C523959D1579B93F3FDF71C08B980FD3130062B03D3DE115C4B84E7AC46AEF5E192A40E7457D5F3A08F66CEAB71143807F2C3CB0DA5E23B38F0559769978406F6E5D30CEADD7985FC73A5A609A8B74A1DF0A29399074A003A226C943D480FEC96DBEC7106A87896539AD")] diff --git a/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/TestMsalDistributedTokenCacheAdapter.cs b/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/TestMsalDistributedTokenCacheAdapter.cs index 4659b8a98..7b51eeafa 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/TestMsalDistributedTokenCacheAdapter.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/TestMsalDistributedTokenCacheAdapter.cs @@ -46,7 +46,7 @@ public async Task TestWriteCacheBytesAsync(string cacheKey, byte[] bytes, CacheS public async Task TestReadCacheBytesAsync(string cacheKey, TelemetryData telemetryData) { - return await ReadCacheBytesAsync(cacheKey, new CacheSerializerHints(), telemetryData).ConfigureAwait(false); + return await ReadCacheBytesAsync(cacheKey, new CacheSerializerHints() { TelemetryData = telemetryData }).ConfigureAwait(false); } } } From 9649acf2b29c177a1e3ff28e605dec3cb49e203b Mon Sep 17 00:00:00 2001 From: trwalke Date: Tue, 6 Jun 2023 13:11:45 -0700 Subject: [PATCH 03/10] Adding more tests adding implementation for in memory cache --- .../MsalDistributedTokenCacheAdapter.cs | 10 ++-- .../InMemory/MsalMemoryTokenCacheProvider.cs | 18 ++++++ .../CacheExtensionsTests.cs | 44 +++++++++++++- .../Microsoft.Identity.Web.Test.csproj | 1 + .../TestTelemetryClient.cs | 59 +++++++++++++++++++ 5 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs diff --git a/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs b/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs index 0e32cb40e..67ddda0c3 100644 --- a/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs +++ b/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs @@ -172,6 +172,11 @@ await L2OperationWithRetryOnFailureAsync( }, cacheSerializerHints.CancellationToken).Measure().ConfigureAwait(false); #pragma warning restore CA1062 // Validate arguments of public methods + if (result != null && telemetryData != null) + { + telemetryData.CacheLevel = Client.Cache.CacheLevel.L2Cache; + } + Logger.DistributedCacheReadTime(_logger, _distributedCacheType, read, measure.MilliSeconds); if (_memoryCache != null) @@ -188,11 +193,6 @@ await L2OperationWithRetryOnFailureAsync( Logger.BackPropagateL2toL1(_logger, memoryCacheEntryOptions.Size ?? 0); _memoryCache.Set(cacheKey, result, memoryCacheEntryOptions); Logger.MemoryCacheCount(_logger, _memoryCacheType, read, _memoryCache.Count); - - if (telemetryData != null) - { - telemetryData.CacheLevel = Client.Cache.CacheLevel.L2Cache; - } } } } diff --git a/src/Microsoft.Identity.Web.TokenCache/InMemory/MsalMemoryTokenCacheProvider.cs b/src/Microsoft.Identity.Web.TokenCache/InMemory/MsalMemoryTokenCacheProvider.cs index 1d29b8d5e..e3351f01d 100644 --- a/src/Microsoft.Identity.Web.TokenCache/InMemory/MsalMemoryTokenCacheProvider.cs +++ b/src/Microsoft.Identity.Web.TokenCache/InMemory/MsalMemoryTokenCacheProvider.cs @@ -62,6 +62,24 @@ protected override Task RemoveKeyAsync(string cacheKey) return Task.FromResult(tokenCacheBytes); } + /// + /// Method to be overridden by concrete cache serializers to Read the cache bytes. + /// + /// Cache key. + /// Hints for the cache serialization implementation optimization. + /// Read bytes. + protected override Task ReadCacheBytesAsync(string cacheKey, CacheSerializerHints cacheSerializerHints) + { + byte[]? tokenCacheBytes = (byte[]?)_memoryCache.Get(cacheKey); + + if (tokenCacheBytes != null && cacheSerializerHints.TelemetryData != null) + { + cacheSerializerHints.TelemetryData.CacheLevel = Client.Cache.CacheLevel.L1Cache; + } + + return Task.FromResult(tokenCacheBytes); + } + /// /// Writes a token cache blob to the serialization cache (identified by its key). /// diff --git a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs index 01780a97e..30852eda2 100644 --- a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs @@ -2,12 +2,16 @@ // Licensed under the MIT License. using System; +using System.Globalization; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Identity.Client; +using Microsoft.Identity.Client.Cache; using Microsoft.Identity.Web.Test.Common; using Microsoft.Identity.Web.Test.Common.Mocks; +using Microsoft.Identity.Web.TokenCacheProviders.Distributed; +using Microsoft.IdentityModel.Abstractions; using Xunit; namespace Microsoft.Identity.Web.Test @@ -18,6 +22,7 @@ public class CacheExtensionsTests private IConfidentialClientApplication _confidentialApp; // Non nullable needed for the Argument null exception tests #pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. + private static TestTelemetryClient _testTelemetryClient; [Fact] public void InMemoryCacheExtensionsTests() @@ -36,16 +41,34 @@ public async Task CacheExtensions_CcaAlreadyExists_TestsAsync() // new InMemory serializer and new cca result = await CreateAppAndGetTokenAsync(CacheType.InMemory, addInstanceMock: true).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); + AssertCacheTelemetry(CacheLevel.None); result = await CreateAppAndGetTokenAsync(CacheType.InMemory, addTokenMock: false).ConfigureAwait(false); Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); + AssertCacheTelemetry(CacheLevel.L1Cache); // new DistributedInMemory and same cca result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); + AssertCacheTelemetry(CacheLevel.None); result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, addTokenMock: false).ConfigureAwait(false); Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); + AssertCacheTelemetry(CacheLevel.L1Cache); + } + + [Fact] + public async Task CacheExtensions_CcaAlreadyExistsL2_TestsAsync() + { + AuthenticationResult result; + // new DistributedInMemory serializer with L1 cache disabled + result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, addInstanceMock: true, disableL1Cache: true).ConfigureAwait(false); + Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); + AssertCacheTelemetry(CacheLevel.None); + + result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, addTokenMock: false, disableL1Cache: true).ConfigureAwait(false); + Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); + AssertCacheTelemetry(CacheLevel.L2Cache); } [Fact] @@ -131,11 +154,15 @@ private enum CacheType private static async Task CreateAppAndGetTokenAsync( CacheType cacheType, bool addTokenMock = true, - bool addInstanceMock = false) + bool addInstanceMock = false, + bool disableL1Cache = false) { using MockHttpClientFactory mockHttp = new MockHttpClientFactory(); using var discoveryHandler = MockHttpCreator.CreateInstanceDiscoveryMockHandler(); using var tokenHandler = MockHttpCreator.CreateClientCredentialTokenHandler(); + + _testTelemetryClient = new TestTelemetryClient(TestConstants.ClientId); + if (addInstanceMock) { mockHttp.AddMockHandler(discoveryHandler); @@ -152,6 +179,7 @@ private static async Task CreateAppAndGetTokenAsync( .WithAuthority(TestConstants.AuthorityCommonTenant) .WithHttpClientFactory(mockHttp) .WithClientSecret(TestConstants.ClientSecret) + .WithTelemetryClient(_testTelemetryClient) .Build(); switch (cacheType) @@ -166,6 +194,14 @@ private static async Task CreateAppAndGetTokenAsync( services.AddDistributedMemoryCache(); services.AddLogging(configure => configure.AddConsole()) .Configure(options => options.MinLevel = Microsoft.Extensions.Logging.LogLevel.Warning); + + if (disableL1Cache) + { + services.Configure(options => + { + options.DisableL1Cache = true; + }); + } }); break; } @@ -176,6 +212,12 @@ private static async Task CreateAppAndGetTokenAsync( return result; } + private void AssertCacheTelemetry(CacheLevel cacheLevel) + { + TelemetryEventDetails eventDetails = _testTelemetryClient.TestTelemetryEventDetails; + Assert.Equal(Convert.ToInt64(cacheLevel, new CultureInfo("en-US")), eventDetails.Properties["CacheLevel"]); + } + private void CreateCca() { if (_confidentialApp == null) diff --git a/tests/Microsoft.Identity.Web.Test/Microsoft.Identity.Web.Test.csproj b/tests/Microsoft.Identity.Web.Test/Microsoft.Identity.Web.Test.csproj index c366de812..de966aa56 100644 --- a/tests/Microsoft.Identity.Web.Test/Microsoft.Identity.Web.Test.csproj +++ b/tests/Microsoft.Identity.Web.Test/Microsoft.Identity.Web.Test.csproj @@ -43,6 +43,7 @@ + diff --git a/tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs b/tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs new file mode 100644 index 000000000..aa7dae3be --- /dev/null +++ b/tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs @@ -0,0 +1,59 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.IdentityModel.Abstractions; + +namespace Microsoft.Identity.Web.Test +{ + internal class TestTelemetryClient : ITelemetryClient + { + private const string _eventName = "acquire_token"; + public TelemetryEventDetails TestTelemetryEventDetails { get; set; } + + public TestTelemetryClient(string clientId) + { + ClientId = clientId; + TestTelemetryEventDetails = new MsalTelemetryEventDetails(_eventName); + } + + public string ClientId { get; set; } + + public void Initialize() + { + + } + + public bool IsEnabled() + { + return true; + } + + public bool IsEnabled(string eventName) + { + return _eventName.Equals(eventName, StringComparison.Ordinal); + } + + public void TrackEvent(TelemetryEventDetails eventDetails) + { + TestTelemetryEventDetails = eventDetails; + } + + public void TrackEvent(string eventName, IDictionary stringProperties = null, IDictionary longProperties = null, IDictionary boolProperties = null, IDictionary dateTimeProperties = null, IDictionary doubleProperties = null, IDictionary guidProperties = null) + { + throw new NotImplementedException(); + } + } + + internal class MsalTelemetryEventDetails : TelemetryEventDetails + { + public MsalTelemetryEventDetails(string eventName) + { + Name = eventName; + } + } +} From 1b2492a7a1db13eb7a278d83daa85c84c01b8b72 Mon Sep 17 00:00:00 2001 From: trwalke Date: Tue, 6 Jun 2023 15:21:41 -0700 Subject: [PATCH 04/10] Test update --- .../CacheExtensionsTests.cs | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs index 30852eda2..20be8e1ce 100644 --- a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs @@ -22,7 +22,6 @@ public class CacheExtensionsTests private IConfidentialClientApplication _confidentialApp; // Non nullable needed for the Argument null exception tests #pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. - private static TestTelemetryClient _testTelemetryClient; [Fact] public void InMemoryCacheExtensionsTests() @@ -38,37 +37,39 @@ public void InMemoryCacheExtensionsTests() public async Task CacheExtensions_CcaAlreadyExists_TestsAsync() { AuthenticationResult result; + TestTelemetryClient testTelemetryClient = new TestTelemetryClient(TestConstants.ClientId); // new InMemory serializer and new cca - result = await CreateAppAndGetTokenAsync(CacheType.InMemory, addInstanceMock: true).ConfigureAwait(false); + result = await CreateAppAndGetTokenAsync(CacheType.InMemory, testTelemetryClient, addInstanceMock: true).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); - AssertCacheTelemetry(CacheLevel.None); + AssertCacheTelemetry(testTelemetryClient, CacheLevel.None); - result = await CreateAppAndGetTokenAsync(CacheType.InMemory, addTokenMock: false).ConfigureAwait(false); + result = await CreateAppAndGetTokenAsync(CacheType.InMemory, testTelemetryClient, addTokenMock: false).ConfigureAwait(false); Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); - AssertCacheTelemetry(CacheLevel.L1Cache); + AssertCacheTelemetry(testTelemetryClient, CacheLevel.L1Cache); // new DistributedInMemory and same cca - result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory).ConfigureAwait(false); + result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); - AssertCacheTelemetry(CacheLevel.None); + AssertCacheTelemetry(testTelemetryClient, CacheLevel.None); - result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, addTokenMock: false).ConfigureAwait(false); + result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient, addTokenMock: false).ConfigureAwait(false); Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); - AssertCacheTelemetry(CacheLevel.L1Cache); + AssertCacheTelemetry(testTelemetryClient, CacheLevel.L1Cache); } [Fact] public async Task CacheExtensions_CcaAlreadyExistsL2_TestsAsync() { AuthenticationResult result; + TestTelemetryClient testTelemetryClient = new TestTelemetryClient(TestConstants.ClientId); // new DistributedInMemory serializer with L1 cache disabled - result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, addInstanceMock: true, disableL1Cache: true).ConfigureAwait(false); + result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient, addInstanceMock: true, disableL1Cache: true).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); - AssertCacheTelemetry(CacheLevel.None); + AssertCacheTelemetry(testTelemetryClient, CacheLevel.None); - result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, addTokenMock: false, disableL1Cache: true).ConfigureAwait(false); + result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient, addTokenMock: false, disableL1Cache: true).ConfigureAwait(false); Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); - AssertCacheTelemetry(CacheLevel.L2Cache); + AssertCacheTelemetry(testTelemetryClient, CacheLevel.L2Cache); } [Fact] @@ -153,6 +154,7 @@ private enum CacheType private static async Task CreateAppAndGetTokenAsync( CacheType cacheType, + ITelemetryClient telemetryClient, bool addTokenMock = true, bool addInstanceMock = false, bool disableL1Cache = false) @@ -161,8 +163,6 @@ private static async Task CreateAppAndGetTokenAsync( using var discoveryHandler = MockHttpCreator.CreateInstanceDiscoveryMockHandler(); using var tokenHandler = MockHttpCreator.CreateClientCredentialTokenHandler(); - _testTelemetryClient = new TestTelemetryClient(TestConstants.ClientId); - if (addInstanceMock) { mockHttp.AddMockHandler(discoveryHandler); @@ -179,7 +179,7 @@ private static async Task CreateAppAndGetTokenAsync( .WithAuthority(TestConstants.AuthorityCommonTenant) .WithHttpClientFactory(mockHttp) .WithClientSecret(TestConstants.ClientSecret) - .WithTelemetryClient(_testTelemetryClient) + .WithTelemetryClient(telemetryClient) .Build(); switch (cacheType) @@ -212,9 +212,9 @@ private static async Task CreateAppAndGetTokenAsync( return result; } - private void AssertCacheTelemetry(CacheLevel cacheLevel) + private void AssertCacheTelemetry(TestTelemetryClient testTelemetryClient, CacheLevel cacheLevel) { - TelemetryEventDetails eventDetails = _testTelemetryClient.TestTelemetryEventDetails; + TelemetryEventDetails eventDetails = testTelemetryClient.TestTelemetryEventDetails; Assert.Equal(Convert.ToInt64(cacheLevel, new CultureInfo("en-US")), eventDetails.Properties["CacheLevel"]); } From 0920665a8b799d23a60194bbaea0483944c11dad Mon Sep 17 00:00:00 2001 From: trwalke Date: Wed, 16 Aug 2023 01:12:20 -0700 Subject: [PATCH 05/10] Resolving test static cache issues --- .../CacheSerializerHints.cs | 3 +-- .../TokenCacheExtensions.cs | 10 ++++++++++ .../Mocks/MockHttpClientFactory.cs | 5 +++++ .../Mocks/MockHttpMessageHandler.cs | 19 ++++++++++++++++++ .../ReplaceMockHttpMessageHandlerEventArgs.cs | 16 +++++++++++++++ .../CacheExtensionsTests.cs | 20 +++++++++++-------- 6 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 tests/Microsoft.Identity.Web.Test.Common/Mocks/ReplaceMockHttpMessageHandlerEventArgs.cs diff --git a/src/Microsoft.Identity.Web.TokenCache/CacheSerializerHints.cs b/src/Microsoft.Identity.Web.TokenCache/CacheSerializerHints.cs index 7734c73c1..ea62cbde9 100644 --- a/src/Microsoft.Identity.Web.TokenCache/CacheSerializerHints.cs +++ b/src/Microsoft.Identity.Web.TokenCache/CacheSerializerHints.cs @@ -4,7 +4,6 @@ using System; using System.Threading; using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; -using Microsoft.IdentityModel.Abstractions; namespace Microsoft.Identity.Web.TokenCacheProviders { @@ -25,7 +24,7 @@ public class CacheSerializerHints public DateTimeOffset? SuggestedCacheExpiry { get; set; } /// - /// Stores details to log to the + /// Stores details to log to MSAL's telemetry client /// internal TelemetryData? TelemetryData { get; set; } } diff --git a/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs b/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs index 0289b1661..ea37b0408 100644 --- a/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs +++ b/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs @@ -19,8 +19,13 @@ namespace Microsoft.Identity.Web /// public static class TokenCacheExtensions { +#if DEBUG + internal static ConcurrentDictionary s_serviceProviderFromAction + = new ConcurrentDictionary(); +#else private static readonly ConcurrentDictionary s_serviceProviderFromAction = new ConcurrentDictionary(); +#endif /// /// Use a token cache and choose the serialization part by adding it to @@ -204,5 +209,10 @@ public static IConfidentialClientApplication AddDistributedTokenCache( }); return confidentialClientApp; } + + //internal static /*For testing only */ void ResetStaticCache() + //{ + // s_serviceProviderFromAction = new ConcurrentDictionary(); + //} } } diff --git a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpClientFactory.cs b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpClientFactory.cs index 1d2bac9f4..f6df39d2f 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpClientFactory.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpClientFactory.cs @@ -33,6 +33,11 @@ public void Dispose() } } + public void OnReplaceMockHandler(object sender, ReplaceMockHttpMessageHandlerEventArgs e) + { + AddMockHandler(e.MockHttpMessageHandler); + } + public MockHttpMessageHandler AddMockHandler(MockHttpMessageHandler handler) { _httpMessageHandlerQueue.Enqueue(handler); diff --git a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs index b9d7d292e..925cc08cc 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Net; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -11,6 +12,8 @@ namespace Microsoft.Identity.Web.Test.Common.Mocks { public class MockHttpMessageHandler : HttpMessageHandler { + public event EventHandler ReplaceMockHttpMessageHandler; + #pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. public MockHttpMessageHandler() #pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. @@ -41,6 +44,22 @@ protected override Task SendAsync(HttpRequestMessage reques var uri = request.RequestUri; Assert.NotNull(uri); + + //Intercept instance discovery requests and reque the current mock handler. + if (uri.AbsoluteUri.Contains("/discovery/instance")) + { + var args = new ReplaceMockHttpMessageHandlerEventArgs(); + args.MockHttpMessageHandler = this; + ReplaceMockHttpMessageHandler(this, args); + + var responseMessage = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(TestConstants.DiscoveryJsonResponse), + }; + + return new TaskFactory().StartNew(() => responseMessage, cancellationToken); + } + if (!string.IsNullOrEmpty(ExpectedUrl)) { Assert.Equal( diff --git a/tests/Microsoft.Identity.Web.Test.Common/Mocks/ReplaceMockHttpMessageHandlerEventArgs.cs b/tests/Microsoft.Identity.Web.Test.Common/Mocks/ReplaceMockHttpMessageHandlerEventArgs.cs new file mode 100644 index 000000000..6bb013386 --- /dev/null +++ b/tests/Microsoft.Identity.Web.Test.Common/Mocks/ReplaceMockHttpMessageHandlerEventArgs.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.Identity.Web.Test.Common.Mocks +{ + public class ReplaceMockHttpMessageHandlerEventArgs : EventArgs + { + public MockHttpMessageHandler MockHttpMessageHandler { get; set; } + } +} diff --git a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs index d006622cb..618eb8770 100644 --- a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs @@ -2,7 +2,9 @@ // Licensed under the MIT License. using System; +using System.Collections.Concurrent; using System.Globalization; +using System.Reflection; using System.Threading.Tasks; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection; @@ -35,7 +37,7 @@ public async Task CacheExtensions_CcaAlreadyExists_TestsAsync() AuthenticationResult result; TestTelemetryClient testTelemetryClient = new TestTelemetryClient(TestConstants.ClientId); // new InMemory serializer and new cca - result = await CreateAppAndGetTokenAsync(CacheType.InMemory, testTelemetryClient, addInstanceMock: true).ConfigureAwait(false); + result = await CreateAppAndGetTokenAsync(CacheType.InMemory, testTelemetryClient).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); AssertCacheTelemetry(testTelemetryClient, CacheLevel.None); @@ -43,6 +45,9 @@ public async Task CacheExtensions_CcaAlreadyExists_TestsAsync() Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); AssertCacheTelemetry(testTelemetryClient, CacheLevel.L1Cache); + //Resetting token caches due to potential collision with other tests + TokenCacheExtensions.s_serviceProviderFromAction = new ConcurrentDictionary(); + // new DistributedInMemory and same cca result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); @@ -59,7 +64,7 @@ public async Task CacheExtensions_CcaAlreadyExistsL2_TestsAsync() AuthenticationResult result; TestTelemetryClient testTelemetryClient = new TestTelemetryClient(TestConstants.ClientId); // new DistributedInMemory serializer with L1 cache disabled - result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient, addInstanceMock: true, disableL1Cache: true).ConfigureAwait(false); + result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient, disableL1Cache: true).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); AssertCacheTelemetry(testTelemetryClient, CacheLevel.None); @@ -153,6 +158,9 @@ public async Task SingletonMsal_ResultsInCorrectCacheEntries_Test() var cacheKey1 = $"{TestConstants.ClientId}_{tenantId1}_AppTokenCache"; var cacheKey2 = $"{TestConstants.ClientId}_{tenantId2}_AppTokenCache"; + //Resetting token caches due to potential collision with other tests + TokenCacheExtensions.s_serviceProviderFromAction = new ConcurrentDictionary(); + using MockHttpClientFactory mockHttpClient = new MockHttpClientFactory(); using (mockHttpClient.AddMockHandler(MockHttpCreator.CreateClientCredentialTokenHandler())) using (mockHttpClient.AddMockHandler(MockHttpCreator.CreateClientCredentialTokenHandler())) @@ -196,22 +204,17 @@ private static async Task CreateAppAndGetTokenAsync( CacheType cacheType, ITelemetryClient telemetryClient, bool addTokenMock = true, - bool addInstanceMock = false, bool disableL1Cache = false) { using MockHttpClientFactory mockHttp = new MockHttpClientFactory(); using var discoveryHandler = MockHttpCreator.CreateInstanceDiscoveryMockHandler(); using var tokenHandler = MockHttpCreator.CreateClientCredentialTokenHandler(); - if (addInstanceMock) - { - mockHttp.AddMockHandler(discoveryHandler); - } - // for when the token is requested from ESTS if (addTokenMock) { mockHttp.AddMockHandler(tokenHandler); + tokenHandler.ReplaceMockHttpMessageHandler += mockHttp.OnReplaceMockHandler; } var confidentialApp = ConfidentialClientApplicationBuilder @@ -249,6 +252,7 @@ private static async Task CreateAppAndGetTokenAsync( var result = await confidentialApp.AcquireTokenForClient(new[] { TestConstants.s_scopeForApp }) .ExecuteAsync().ConfigureAwait(false); + tokenHandler.ReplaceMockHttpMessageHandler -= mockHttp.OnReplaceMockHandler; return result; } From 99e6f094de520a3b18edb155ecdb57388500cafa Mon Sep 17 00:00:00 2001 From: trwalke Date: Wed, 16 Aug 2023 01:21:36 -0700 Subject: [PATCH 06/10] Resolving release build issue --- tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs index 618eb8770..88d85da35 100644 --- a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs @@ -44,10 +44,10 @@ public async Task CacheExtensions_CcaAlreadyExists_TestsAsync() result = await CreateAppAndGetTokenAsync(CacheType.InMemory, testTelemetryClient, addTokenMock: false).ConfigureAwait(false); Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); AssertCacheTelemetry(testTelemetryClient, CacheLevel.L1Cache); - +#if DEBUG //Resetting token caches due to potential collision with other tests TokenCacheExtensions.s_serviceProviderFromAction = new ConcurrentDictionary(); - +#endif // new DistributedInMemory and same cca result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); @@ -157,10 +157,10 @@ public async Task SingletonMsal_ResultsInCorrectCacheEntries_Test() var tenantId2 = "tenant2"; var cacheKey1 = $"{TestConstants.ClientId}_{tenantId1}_AppTokenCache"; var cacheKey2 = $"{TestConstants.ClientId}_{tenantId2}_AppTokenCache"; - +#if DEBUG //Resetting token caches due to potential collision with other tests TokenCacheExtensions.s_serviceProviderFromAction = new ConcurrentDictionary(); - +#endif using MockHttpClientFactory mockHttpClient = new MockHttpClientFactory(); using (mockHttpClient.AddMockHandler(MockHttpCreator.CreateClientCredentialTokenHandler())) using (mockHttpClient.AddMockHandler(MockHttpCreator.CreateClientCredentialTokenHandler())) From 61aeeef061a8939598ca581921794b397487a915 Mon Sep 17 00:00:00 2001 From: trwalke Date: Wed, 16 Aug 2023 01:37:59 -0700 Subject: [PATCH 07/10] Resolving test issues --- .../TokenCacheExtensions.cs | 7 +------ .../CacheExtensionsTests.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs b/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs index ea37b0408..abde4ce07 100644 --- a/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs +++ b/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs @@ -19,13 +19,8 @@ namespace Microsoft.Identity.Web /// public static class TokenCacheExtensions { -#if DEBUG - internal static ConcurrentDictionary s_serviceProviderFromAction + internal static readonly ConcurrentDictionary s_serviceProviderFromAction = new ConcurrentDictionary(); -#else - private static readonly ConcurrentDictionary s_serviceProviderFromAction - = new ConcurrentDictionary(); -#endif /// /// Use a token cache and choose the serialization part by adding it to diff --git a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs index 88d85da35..3fd9f639f 100644 --- a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs @@ -44,10 +44,10 @@ public async Task CacheExtensions_CcaAlreadyExists_TestsAsync() result = await CreateAppAndGetTokenAsync(CacheType.InMemory, testTelemetryClient, addTokenMock: false).ConfigureAwait(false); Assert.Equal(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource); AssertCacheTelemetry(testTelemetryClient, CacheLevel.L1Cache); -#if DEBUG + //Resetting token caches due to potential collision with other tests - TokenCacheExtensions.s_serviceProviderFromAction = new ConcurrentDictionary(); -#endif + TokenCacheExtensions.s_serviceProviderFromAction.Clear(); + // new DistributedInMemory and same cca result = await CreateAppAndGetTokenAsync(CacheType.DistributedInMemory, testTelemetryClient).ConfigureAwait(false); Assert.Equal(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource); @@ -157,10 +157,10 @@ public async Task SingletonMsal_ResultsInCorrectCacheEntries_Test() var tenantId2 = "tenant2"; var cacheKey1 = $"{TestConstants.ClientId}_{tenantId1}_AppTokenCache"; var cacheKey2 = $"{TestConstants.ClientId}_{tenantId2}_AppTokenCache"; -#if DEBUG + //Resetting token caches due to potential collision with other tests - TokenCacheExtensions.s_serviceProviderFromAction = new ConcurrentDictionary(); -#endif + TokenCacheExtensions.s_serviceProviderFromAction.Clear(); + using MockHttpClientFactory mockHttpClient = new MockHttpClientFactory(); using (mockHttpClient.AddMockHandler(MockHttpCreator.CreateClientCredentialTokenHandler())) using (mockHttpClient.AddMockHandler(MockHttpCreator.CreateClientCredentialTokenHandler())) From c8070316333b73ae56dc6d6369db2aae841e40fd Mon Sep 17 00:00:00 2001 From: trwalke Date: Wed, 16 Aug 2023 18:24:40 -0700 Subject: [PATCH 08/10] Updating instance discovery intercept logic --- .../Mocks/MockHttpClientFactory.cs | 5 ----- .../Mocks/MockHttpMessageHandler.cs | 9 ++++----- .../ReplaceMockHttpMessageHandlerEventArgs.cs | 16 ---------------- .../CacheExtensionsTests.cs | 7 ++++--- 4 files changed, 8 insertions(+), 29 deletions(-) delete mode 100644 tests/Microsoft.Identity.Web.Test.Common/Mocks/ReplaceMockHttpMessageHandlerEventArgs.cs diff --git a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpClientFactory.cs b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpClientFactory.cs index f6df39d2f..1d2bac9f4 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpClientFactory.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpClientFactory.cs @@ -33,11 +33,6 @@ public void Dispose() } } - public void OnReplaceMockHandler(object sender, ReplaceMockHttpMessageHandlerEventArgs e) - { - AddMockHandler(e.MockHttpMessageHandler); - } - public MockHttpMessageHandler AddMockHandler(MockHttpMessageHandler handler) { _httpMessageHandlerQueue.Enqueue(handler); diff --git a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs index 925cc08cc..728b33793 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs @@ -12,7 +12,7 @@ namespace Microsoft.Identity.Web.Test.Common.Mocks { public class MockHttpMessageHandler : HttpMessageHandler { - public event EventHandler ReplaceMockHttpMessageHandler; + public Func ReplaceMockHttpMessageHandler; #pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. public MockHttpMessageHandler() @@ -45,12 +45,11 @@ protected override Task SendAsync(HttpRequestMessage reques var uri = request.RequestUri; Assert.NotNull(uri); - //Intercept instance discovery requests and reque the current mock handler. + //Intercept instance discovery requests and serve a response. + //Also, requeue the current mock handler for MSAL's next request. if (uri.AbsoluteUri.Contains("/discovery/instance")) { - var args = new ReplaceMockHttpMessageHandlerEventArgs(); - args.MockHttpMessageHandler = this; - ReplaceMockHttpMessageHandler(this, args); + ReplaceMockHttpMessageHandler(this); var responseMessage = new HttpResponseMessage(HttpStatusCode.OK) { diff --git a/tests/Microsoft.Identity.Web.Test.Common/Mocks/ReplaceMockHttpMessageHandlerEventArgs.cs b/tests/Microsoft.Identity.Web.Test.Common/Mocks/ReplaceMockHttpMessageHandlerEventArgs.cs deleted file mode 100644 index 6bb013386..000000000 --- a/tests/Microsoft.Identity.Web.Test.Common/Mocks/ReplaceMockHttpMessageHandlerEventArgs.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace Microsoft.Identity.Web.Test.Common.Mocks -{ - public class ReplaceMockHttpMessageHandlerEventArgs : EventArgs - { - public MockHttpMessageHandler MockHttpMessageHandler { get; set; } - } -} diff --git a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs index 3fd9f639f..344cc6433 100644 --- a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs @@ -207,14 +207,15 @@ private static async Task CreateAppAndGetTokenAsync( bool disableL1Cache = false) { using MockHttpClientFactory mockHttp = new MockHttpClientFactory(); - using var discoveryHandler = MockHttpCreator.CreateInstanceDiscoveryMockHandler(); using var tokenHandler = MockHttpCreator.CreateClientCredentialTokenHandler(); // for when the token is requested from ESTS if (addTokenMock) { mockHttp.AddMockHandler(tokenHandler); - tokenHandler.ReplaceMockHttpMessageHandler += mockHttp.OnReplaceMockHandler; + + //Enables the mock handler to requeue requests that have been intercepted for instance discovery for example + tokenHandler.ReplaceMockHttpMessageHandler = mockHttp.AddMockHandler; } var confidentialApp = ConfidentialClientApplicationBuilder @@ -252,7 +253,7 @@ private static async Task CreateAppAndGetTokenAsync( var result = await confidentialApp.AcquireTokenForClient(new[] { TestConstants.s_scopeForApp }) .ExecuteAsync().ConfigureAwait(false); - tokenHandler.ReplaceMockHttpMessageHandler -= mockHttp.OnReplaceMockHandler; + tokenHandler.ReplaceMockHttpMessageHandler = null; return result; } From bd5dc6f314e134d5931414dfc5c7e00536aa5d99 Mon Sep 17 00:00:00 2001 From: trwalke Date: Wed, 16 Aug 2023 22:26:57 -0700 Subject: [PATCH 09/10] Clean up --- .../TokenCacheExtensions.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs b/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs index abde4ce07..1b210c38e 100644 --- a/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs +++ b/src/Microsoft.Identity.Web.TokenCache/TokenCacheExtensions.cs @@ -19,6 +19,7 @@ namespace Microsoft.Identity.Web /// public static class TokenCacheExtensions { + //internal for testing only internal static readonly ConcurrentDictionary s_serviceProviderFromAction = new ConcurrentDictionary(); @@ -204,10 +205,5 @@ public static IConfidentialClientApplication AddDistributedTokenCache( }); return confidentialClientApp; } - - //internal static /*For testing only */ void ResetStaticCache() - //{ - // s_serviceProviderFromAction = new ConcurrentDictionary(); - //} } } From 73ff201fc937af0a6ab75a19762fee9ed5a96a63 Mon Sep 17 00:00:00 2001 From: trwalke Date: Thu, 17 Aug 2023 13:35:50 -0700 Subject: [PATCH 10/10] Removing unneeded use statements resolving nullable warnings --- .../Distributed/MsalDistributedTokenCacheAdapter.cs | 2 -- .../MsalAbstractTokenCacheProvider.cs | 2 -- .../Mocks/MockHttpMessageHandler.cs | 8 ++++++-- tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs | 4 +--- tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs | 2 +- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs b/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs index 8b184d87b..708eb3f4d 100644 --- a/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs +++ b/src/Microsoft.Identity.Web.TokenCache/Distributed/MsalDistributedTokenCacheAdapter.cs @@ -9,8 +9,6 @@ using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; -using Microsoft.IdentityModel.Abstractions; namespace Microsoft.Identity.Web.TokenCacheProviders.Distributed { diff --git a/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs b/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs index ed660e671..1fcd74ff2 100644 --- a/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs +++ b/src/Microsoft.Identity.Web.TokenCache/MsalAbstractTokenCacheProvider.cs @@ -6,8 +6,6 @@ using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.Logging; using Microsoft.Identity.Client; -using Microsoft.Identity.Client.TelemetryCore.TelemetryClient; -using Microsoft.IdentityModel.Abstractions; namespace Microsoft.Identity.Web.TokenCacheProviders { diff --git a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs index 728b33793..ff1984057 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpMessageHandler.cs @@ -47,7 +47,11 @@ protected override Task SendAsync(HttpRequestMessage reques //Intercept instance discovery requests and serve a response. //Also, requeue the current mock handler for MSAL's next request. +#if NET6_0_OR_GREATER + if (uri.AbsoluteUri.Contains("/discovery/instance", StringComparison.OrdinalIgnoreCase)) +#else if (uri.AbsoluteUri.Contains("/discovery/instance")) +#endif { ReplaceMockHttpMessageHandler(this); @@ -56,7 +60,7 @@ protected override Task SendAsync(HttpRequestMessage reques Content = new StringContent(TestConstants.DiscoveryJsonResponse), }; - return new TaskFactory().StartNew(() => responseMessage, cancellationToken); + return Task.FromResult(responseMessage); } if (!string.IsNullOrEmpty(ExpectedUrl)) @@ -77,7 +81,7 @@ protected override Task SendAsync(HttpRequestMessage reques string postData = request.Content.ReadAsStringAsync().Result; } - return new TaskFactory().StartNew(() => ResponseMessage, cancellationToken); + return Task.FromResult(ResponseMessage); } } } diff --git a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs index 344cc6433..78986cf5f 100644 --- a/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CacheExtensionsTests.cs @@ -2,9 +2,7 @@ // Licensed under the MIT License. using System; -using System.Collections.Concurrent; using System.Globalization; -using System.Reflection; using System.Threading.Tasks; using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection; @@ -253,7 +251,7 @@ private static async Task CreateAppAndGetTokenAsync( var result = await confidentialApp.AcquireTokenForClient(new[] { TestConstants.s_scopeForApp }) .ExecuteAsync().ConfigureAwait(false); - tokenHandler.ReplaceMockHttpMessageHandler = null; + tokenHandler.ReplaceMockHttpMessageHandler = null!; return result; } diff --git a/tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs b/tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs index aa7dae3be..37fa1e687 100644 --- a/tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs +++ b/tests/Microsoft.Identity.Web.Test/TestTelemetryClient.cs @@ -43,7 +43,7 @@ public void TrackEvent(TelemetryEventDetails eventDetails) TestTelemetryEventDetails = eventDetails; } - public void TrackEvent(string eventName, IDictionary stringProperties = null, IDictionary longProperties = null, IDictionary boolProperties = null, IDictionary dateTimeProperties = null, IDictionary doubleProperties = null, IDictionary guidProperties = null) + public void TrackEvent(string eventName, IDictionary stringProperties, IDictionary longProperties, IDictionary boolProperties, IDictionary dateTimeProperties, IDictionary doubleProperties, IDictionary guidProperties) { throw new NotImplementedException(); }