diff --git a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs index 6d1fb7a5a2..200288572e 100644 --- a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs +++ b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs @@ -18,17 +18,27 @@ namespace Microsoft.IdentityModel.Protocols /// /// The type of . [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1001:TypesThatOwnDisposableFieldsShouldBeDisposable")] - public class ConfigurationManager : BaseConfigurationManager, IConfigurationManager where T : class + public partial class ConfigurationManager : BaseConfigurationManager, IConfigurationManager where T : class { - // To prevent tearing, this needs to be only updated through AtomicUpdateSyncAfter. - // Reads should be done through the property SyncAfter. +#pragma warning disable IDE0044 // Add readonly modifier +#pragma warning disable CS0649 // Unused, it gets used in tests. + internal Action _onBackgroundTaskFinish; +#pragma warning restore CS0649 // Unused +#pragma warning restore IDE0044 // Add readonly modifier + private DateTime _syncAfter = DateTime.MinValue; - private DateTime SyncAfter => _syncAfter; + private DateTime SyncAfter + { + get => _syncAfter; + set => AtomicUpdateDateTime(ref _syncAfter, ref value); + } - // See comment above, this should only be updated through AtomicUpdateLastRequestRefresh, - // read through LastRequestRefresh. private DateTime _lastRequestRefresh = DateTime.MinValue; - private DateTime LastRequestRefresh => _lastRequestRefresh; + private DateTime LastRequestRefresh + { + get => _lastRequestRefresh; + set => AtomicUpdateDateTime(ref _lastRequestRefresh, ref value); + } private bool _isFirstRefreshRequest = true; private readonly SemaphoreSlim _configurationNullLock = new SemaphoreSlim(1); @@ -53,8 +63,32 @@ public class ConfigurationManager : BaseConfigurationManager, IConfigurationM // requesting a refresh, so it should be done immediately so the next // call to GetConfiguration will return new configuration if the minimum // refresh interval has passed. - bool _refreshRequested; + private bool _refreshRequested; + // Wait handle used to signal a background task to update the configuration. + // Handle starts unset, and AutoResetEvent.Set() sets it, this indicates that + // the background refresh task should immediately run. + private readonly AutoResetEvent _updateMetadataEvent = new(false); + + // Background task that updates the configuration. Signaled with _updateMetadataEvent. + // Task should be started with EnsureBackgroundRefreshTaskIsRunning. + private Task _updateMetadataTask; + + private readonly CancellationTokenSource _backgroundRefreshTaskCancellationTokenSource; + + /// + /// Requests that background tasks be shutdown. + /// This only applies if 'Switch.Microsoft.IdentityModel.UpdateConfigAsBlocking' is not set or set to false. + /// Note that this does not influence . + /// If the background task stops, the next time the task would be signaled, the task will be + /// restarted unless is called. + /// If using a background task, the cannot + /// be used after calling this method. + /// + public void ShutdownBackgroundTask() + { + _backgroundRefreshTaskCancellationTokenSource.Cancel(); + } /// /// Instantiates a new that manages automatic and controls refreshing on configuration data. @@ -117,6 +151,10 @@ public ConfigurationManager(string metadataAddress, IConfigurationRetriever c MetadataAddress = metadataAddress; _docRetriever = docRetriever; _configRetriever = configRetriever; + _backgroundRefreshTaskCancellationTokenSource = new CancellationTokenSource(); + + if (!AppContextSwitches.UpdateConfigAsBlocking) + EnsureBackgroundRefreshTaskIsRunning(); } /// @@ -153,8 +191,14 @@ public ConfigurationManager(string metadataAddress, IConfigurationRetriever c /// /// Obtains an updated version of Configuration. /// - /// Configuration of type T. - /// If the time since the last call is less than then is not called and the current Configuration is returned. + /// Configuration of type . + /// + /// If the time since the last call is less than + /// then is not called and the current Configuration is returned. + /// If the configuration is not able to be updated, but a previous configuration was previously retrieved, the previous configuration is returned. + /// + /// Throw if the configuration is unable to be retrieved. + /// Throw if the configuration fails to be validated by the . public async Task GetConfigurationAsync() { return await GetConfigurationAsync(CancellationToken.None).ConfigureAwait(false); @@ -164,13 +208,27 @@ public async Task GetConfigurationAsync() /// Obtains an updated version of Configuration. /// /// CancellationToken - /// Configuration of type T. - /// If the time since the last call is less than then is not called and the current Configuration is returned. + /// Configuration of type . + /// + /// If the time since the last call is less than + /// then is not called and the current Configuration is returned. + /// If the configuration is not able to be updated, but a previous configuration was previously retrieved, the previous configuration is returned. + /// + /// Throw if the configuration is unable to be retrieved. + /// Throw if the configuration fails to be validated by the . public virtual async Task GetConfigurationAsync(CancellationToken cancel) { if (_currentConfiguration != null && SyncAfter > _timeProvider.GetUtcNow()) return _currentConfiguration; + if (AppContextSwitches.UpdateConfigAsBlocking) + return await GetConfigurationWithBlockingAsync(cancel).ConfigureAwait(false); + else + return await GetConfigurationWithBackgroundTaskUpdatesAsync(cancel).ConfigureAwait(false); + } + + private async Task GetConfigurationWithBackgroundTaskUpdatesAsync(CancellationToken cancel) + { Exception fetchMetadataFailure = null; // LOGIC @@ -246,38 +304,10 @@ public virtual async Task GetConfigurationAsync(CancellationToken cancel) { if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverRunning, ConfigurationRetrieverIdle) == ConfigurationRetrieverIdle) { - if (_refreshRequested) + if (SyncAfter <= _timeProvider.GetUtcNow()) { - _refreshRequested = false; - - try - { - // Log as manual because RequestRefresh was called - TelemetryClient.IncrementConfigurationRefreshRequestCounter( - MetadataAddress, - TelemetryConstants.Protocols.Manual); - } -#pragma warning disable CA1031 // Do not catch general exception types - catch - { } -#pragma warning restore CA1031 // Do not catch general exception types - - UpdateCurrentConfiguration(); - } - else if (SyncAfter <= _timeProvider.GetUtcNow()) - { - try - { - TelemetryClient.IncrementConfigurationRefreshRequestCounter( - MetadataAddress, - TelemetryConstants.Protocols.Automatic); - } -#pragma warning disable CA1031 // Do not catch general exception types - catch - { } -#pragma warning restore CA1031 // Do not catch general exception types - - _ = Task.Run(UpdateCurrentConfiguration, CancellationToken.None); + EnsureBackgroundRefreshTaskIsRunning(); + _updateMetadataEvent.Set(); } else { @@ -300,6 +330,53 @@ public virtual async Task GetConfigurationAsync(CancellationToken cancel) fetchMetadataFailure)); } + private void EnsureBackgroundRefreshTaskIsRunning() + { + if (_backgroundRefreshTaskCancellationTokenSource.IsCancellationRequested) + return; + + if (_updateMetadataTask == null || _updateMetadataTask.Status != TaskStatus.Running) + _updateMetadataTask = Task.Run(UpdateCurrentConfigurationUsingSignals); + } + + private void TelemetryForUpdate() + { + var updateMode = _refreshRequested ? TelemetryConstants.Protocols.Manual : TelemetryConstants.Protocols.Automatic; + + if (_refreshRequested) + _refreshRequested = false; + + try + { + TelemetryClient.IncrementConfigurationRefreshRequestCounter( + MetadataAddress, + updateMode); + } +#pragma warning disable CA1031 // Do not catch general exception types + catch + { } +#pragma warning restore CA1031 // Do not catch general exception types + } + + private void UpdateCurrentConfigurationUsingSignals() + { + try + { + while (!_backgroundRefreshTaskCancellationTokenSource.IsCancellationRequested) + { + if (_updateMetadataEvent.WaitOne(500)) + { + UpdateCurrentConfiguration(); + _onBackgroundTaskFinish?.Invoke(); + } + } + } + catch (Exception ex) + { + TelemetryClient.LogBackgroundConfigurationRefreshFailure(MetadataAddress, ex); + } + } + /// /// This should be called when the configuration needs to be updated either from RequestRefresh or AutomaticRefresh /// The Caller should first check the state checking state using: @@ -307,6 +384,7 @@ public virtual async Task GetConfigurationAsync(CancellationToken cancel) /// private void UpdateCurrentConfiguration() { + TelemetryForUpdate(); #pragma warning disable CA1031 // Do not catch general exception types long startTimestamp = _timeProvider.GetTimestamp(); @@ -368,25 +446,17 @@ private void UpdateConfiguration(T configuration) _currentConfiguration = configuration; var newSyncTime = DateTimeUtil.Add(_timeProvider.GetUtcNow().UtcDateTime, AutomaticRefreshInterval + TimeSpan.FromSeconds(new Random().Next((int)AutomaticRefreshInterval.TotalSeconds / 20))); - AtomicUpdateSyncAfter(newSyncTime); + SyncAfter = newSyncTime; } - private void AtomicUpdateSyncAfter(DateTime syncAfter) + private static void AtomicUpdateDateTime(ref DateTime field, ref DateTime value) { // DateTime's backing data is safe to treat as a long if the Kind is not local. - // _syncAfter will always be updated to a UTC time. + // time will always be updated to a UTC time. // See the implementation of ToBinary on DateTime. Interlocked.Exchange( - ref Unsafe.As(ref _syncAfter), - Unsafe.As(ref syncAfter)); - } - - private void AtomicUpdateLastRequestRefresh(DateTime lastRequestRefresh) - { - // See the comment in AtomicUpdateSyncAfter. - Interlocked.Exchange( - ref Unsafe.As(ref _lastRequestRefresh), - Unsafe.As(ref lastRequestRefresh)); + ref Unsafe.As(ref field), + Unsafe.As(ref value)); } /// @@ -408,15 +478,32 @@ public override async Task GetBaseConfigurationAsync(Cancella /// If == then this method does nothing. /// public override void RequestRefresh() + { + if (AppContextSwitches.UpdateConfigAsBlocking) + { + RequestRefreshBlocking(); + } + else + { + RequestRefreshBackgroundThread(); + } + } + + private void RequestRefreshBackgroundThread() { DateTime now = _timeProvider.GetUtcNow().UtcDateTime; if (now >= DateTimeUtil.Add(LastRequestRefresh, RefreshInterval) || _isFirstRefreshRequest) { _isFirstRefreshRequest = false; - AtomicUpdateSyncAfter(now); - AtomicUpdateLastRequestRefresh(now); - _refreshRequested = true; + + if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverRunning, ConfigurationRetrieverIdle) == ConfigurationRetrieverIdle) + { + _refreshRequested = true; + LastRequestRefresh = now; + EnsureBackgroundRefreshTaskIsRunning(); + _updateMetadataEvent.Set(); + } } } diff --git a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager_Blocking.cs b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager_Blocking.cs new file mode 100644 index 0000000000..8ecf701233 --- /dev/null +++ b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager_Blocking.cs @@ -0,0 +1,161 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Threading.Tasks; +using System.Threading; +using System; +using Microsoft.IdentityModel.Logging; +using Microsoft.IdentityModel.Protocols.Configuration; +using Microsoft.IdentityModel.Tokens; +using Microsoft.IdentityModel.Telemetry; + +namespace Microsoft.IdentityModel.Protocols +{ + partial class ConfigurationManager where T : class + { + private readonly SemaphoreSlim _refreshLock = new(1); + + private TimeSpan _bootstrapRefreshInterval = TimeSpan.FromSeconds(1); + + private async Task GetConfigurationWithBlockingAsync(CancellationToken cancel) + { + Exception _fetchMetadataFailure = null; + await _refreshLock.WaitAsync(cancel).ConfigureAwait(false); + + long startTimestamp = _timeProvider.GetTimestamp(); + + try + { + if (SyncAfter <= _timeProvider.GetUtcNow()) + { + try + { + // Don't use the individual CT here, this is a shared operation that shouldn't be affected by an individual's cancellation. + // The transport should have it's own timeouts, etc.. + var configuration = await _configRetriever.GetConfigurationAsync(MetadataAddress, _docRetriever, CancellationToken.None).ConfigureAwait(false); + + var elapsedTime = _timeProvider.GetElapsedTime(startTimestamp); + TelemetryClient.LogConfigurationRetrievalDuration( + MetadataAddress, + elapsedTime); + + if (_configValidator != null) + { + ConfigurationValidationResult result = _configValidator.Validate(configuration); + if (!result.Succeeded) + throw LogHelper.LogExceptionMessage(new InvalidConfigurationException(LogHelper.FormatInvariant(LogMessages.IDX20810, result.ErrorMessage))); + } + + LastRequestRefresh = _timeProvider.GetUtcNow().UtcDateTime; + TelemetryForUpdateBlocking(); + UpdateConfiguration(configuration); + } + catch (Exception ex) + { + _fetchMetadataFailure = ex; + + if (_currentConfiguration == null) // Throw an exception if there's no configuration to return. + { + if (_bootstrapRefreshInterval < RefreshInterval) + { + // Adopt exponential backoff for bootstrap refresh interval with a decorrelated jitter if it is not longer than the refresh interval. + TimeSpan _bootstrapRefreshIntervalWithJitter = TimeSpan.FromSeconds(new Random().Next((int)_bootstrapRefreshInterval.TotalSeconds)); + _bootstrapRefreshInterval += _bootstrapRefreshInterval; + _syncAfter = DateTimeUtil.Add(DateTime.UtcNow, _bootstrapRefreshIntervalWithJitter); + } + else + { + _syncAfter = DateTimeUtil.Add( + _timeProvider.GetUtcNow().UtcDateTime, + AutomaticRefreshInterval < RefreshInterval ? AutomaticRefreshInterval : RefreshInterval); + } + + TelemetryClient.IncrementConfigurationRefreshRequestCounter( + MetadataAddress, + TelemetryConstants.Protocols.FirstRefresh, + ex); + + throw LogHelper.LogExceptionMessage( + new InvalidOperationException( + LogHelper.FormatInvariant(LogMessages.IDX20803, LogHelper.MarkAsNonPII(MetadataAddress ?? "null"), LogHelper.MarkAsNonPII(_syncAfter), LogHelper.MarkAsNonPII(ex)), ex)); + } + else + { + _syncAfter = DateTimeUtil.Add( + _timeProvider.GetUtcNow().UtcDateTime, + AutomaticRefreshInterval < RefreshInterval ? AutomaticRefreshInterval : RefreshInterval); + + var elapsedTime = _timeProvider.GetElapsedTime(startTimestamp); + + TelemetryClient.LogConfigurationRetrievalDuration( + MetadataAddress, + elapsedTime, + ex); + + LogHelper.LogExceptionMessage( + new InvalidOperationException( + LogHelper.FormatInvariant(LogMessages.IDX20806, LogHelper.MarkAsNonPII(MetadataAddress ?? "null"), LogHelper.MarkAsNonPII(ex)), ex)); + } + } + } + + // Stale metadata is better than no metadata + if (_currentConfiguration != null) + return _currentConfiguration; + else + throw LogHelper.LogExceptionMessage( + new InvalidOperationException( + LogHelper.FormatInvariant( + LogMessages.IDX20803, + LogHelper.MarkAsNonPII(MetadataAddress ?? "null"), + LogHelper.MarkAsNonPII(_syncAfter), + LogHelper.MarkAsNonPII(_fetchMetadataFailure)), + _fetchMetadataFailure)); + } + finally + { + _refreshLock.Release(); + } + } + + private void RequestRefreshBlocking() + { + DateTime now = _timeProvider.GetUtcNow().UtcDateTime; + + if (now >= DateTimeUtil.Add(LastRequestRefresh, RefreshInterval) || _isFirstRefreshRequest) + { + _refreshRequested = true; + _syncAfter = now; + _isFirstRefreshRequest = false; + } + } + + private void TelemetryForUpdateBlocking() + { + string updateMode; + + if (_currentConfiguration is null) + { + updateMode = TelemetryConstants.Protocols.FirstRefresh; + } + else + { + updateMode = _refreshRequested ? TelemetryConstants.Protocols.Manual : TelemetryConstants.Protocols.Automatic; + + if (_refreshRequested) + _refreshRequested = false; + } + + try + { + TelemetryClient.IncrementConfigurationRefreshRequestCounter( + MetadataAddress, + updateMode); + } +#pragma warning disable CA1031 // Do not catch general exception types + catch + { } +#pragma warning restore CA1031 // Do not catch general exception types + } + } +} diff --git a/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs b/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs index 901dc16ac8..9efe0a190c 100644 --- a/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs +++ b/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs @@ -12,3 +12,8 @@ #if NET6_0_OR_GREATER [assembly: SuppressMessage("Globalization", "CA1307:Specify StringComparison", Justification = "Adding StringComparison.Ordinal adds a performance penalty.", Scope = "member", Target = "~M:Microsoft.IdentityModel.Protocols.AuthenticationProtocolMessage.BuildRedirectUrl~System.String")] #endif + +[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", + Justification = "Background thread needs to never throw an unhandled exception.", + Scope = "member", + Target = "~M:Microsoft.IdentityModel.Protocols.ConfigurationManager`1.UpdateCurrentConfigurationUsingSignals")] diff --git a/src/Microsoft.IdentityModel.Protocols/InternalAPI.Unshipped.txt b/src/Microsoft.IdentityModel.Protocols/InternalAPI.Unshipped.txt index 98597bed68..06f99d657b 100644 --- a/src/Microsoft.IdentityModel.Protocols/InternalAPI.Unshipped.txt +++ b/src/Microsoft.IdentityModel.Protocols/InternalAPI.Unshipped.txt @@ -1,2 +1,2 @@ Microsoft.IdentityModel.Protocols.ConfigurationManager.TelemetryClient -> Microsoft.IdentityModel.Telemetry.ITelemetryClient -Microsoft.IdentityModel.Protocols.ConfigurationManager.TimeProvider -> System.TimeProvider +Microsoft.IdentityModel.Protocols.ConfigurationManager._onBackgroundTaskFinish -> System.Action diff --git a/src/Microsoft.IdentityModel.Protocols/PublicAPI.Unshipped.txt b/src/Microsoft.IdentityModel.Protocols/PublicAPI.Unshipped.txt index 5c617a3121..29c5255246 100644 --- a/src/Microsoft.IdentityModel.Protocols/PublicAPI.Unshipped.txt +++ b/src/Microsoft.IdentityModel.Protocols/PublicAPI.Unshipped.txt @@ -1,2 +1,3 @@ Microsoft.IdentityModel.Protocols.HttpDocumentRetriever.HttpVersion.get -> System.Version Microsoft.IdentityModel.Protocols.HttpDocumentRetriever.HttpVersion.set -> void +Microsoft.IdentityModel.Protocols.ConfigurationManager.ShutdownBackgroundTask() -> void diff --git a/src/Microsoft.IdentityModel.Tokens/AppContextSwitches.cs b/src/Microsoft.IdentityModel.Tokens/AppContextSwitches.cs index 0105963f08..eddf948ce5 100644 --- a/src/Microsoft.IdentityModel.Tokens/AppContextSwitches.cs +++ b/src/Microsoft.IdentityModel.Tokens/AppContextSwitches.cs @@ -70,6 +70,17 @@ internal static class AppContextSwitches internal static bool UseRfcDefinitionOfEpkAndKid => _useRfcDefinitionOfEpkAndKid ??= (AppContext.TryGetSwitch(UseRfcDefinitionOfEpkAndKidSwitch, out bool isEnabled) && isEnabled); + /// + /// Enabling this switch will cause the configuration manager to block other requests to GetConfigurationAsync if a request is already in progress. + /// The default configuration refresh behavior is if a request is already in progress, the current configuration will be returned until the ongoing request is completed on + /// a background thread. + /// + internal const string UpdateConfigAsBlockingSwitch = "Switch.Microsoft.IdentityModel.UpdateConfigAsBlocking"; + + private static bool? _updateConfigAsBlockingCall; + + internal static bool UpdateConfigAsBlocking => _updateConfigAsBlockingCall ??= (AppContext.TryGetSwitch(UpdateConfigAsBlockingSwitch, out bool blockingCall) && blockingCall); + /// /// Used for testing to reset all switches to its default value. /// @@ -86,6 +97,9 @@ internal static void ResetAllSwitches() _useRfcDefinitionOfEpkAndKid = null; AppContext.SetSwitch(UseRfcDefinitionOfEpkAndKidSwitch, false); + + _updateConfigAsBlockingCall = null; + AppContext.SetSwitch(UpdateConfigAsBlockingSwitch, false); } } } diff --git a/src/Microsoft.IdentityModel.Tokens/Telemetry/ITelemetryClient.cs b/src/Microsoft.IdentityModel.Tokens/Telemetry/ITelemetryClient.cs index 656fb6bdf5..0ab0c083ff 100644 --- a/src/Microsoft.IdentityModel.Tokens/Telemetry/ITelemetryClient.cs +++ b/src/Microsoft.IdentityModel.Tokens/Telemetry/ITelemetryClient.cs @@ -24,5 +24,9 @@ internal void IncrementConfigurationRefreshRequestCounter( string metadataAddress, string operationStatus, Exception exception); + + internal void LogBackgroundConfigurationRefreshFailure( + string metadataAddress, + Exception exception); } } diff --git a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryClient.cs b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryClient.cs index 20cbfcf15b..15d1404486 100644 --- a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryClient.cs +++ b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryClient.cs @@ -2,8 +2,10 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Diagnostics; using Microsoft.IdentityModel.Logging; +using Microsoft.IdentityModel.Tokens; namespace Microsoft.IdentityModel.Telemetry { @@ -14,13 +16,19 @@ internal class TelemetryClient : ITelemetryClient { public string ClientVer = IdentityModelTelemetryUtil.ClientVer; + private KeyValuePair _blockingTagValue = new( + TelemetryConstants.BlockingTypeTag, + AppContextSwitches.UpdateConfigAsBlocking.ToString() + ); + public void IncrementConfigurationRefreshRequestCounter(string metadataAddress, string operationStatus) { var tagList = new TagList() { { TelemetryConstants.IdentityModelVersionTag, ClientVer }, { TelemetryConstants.MetadataAddressTag, metadataAddress }, - { TelemetryConstants.OperationStatusTag, operationStatus } + { TelemetryConstants.OperationStatusTag, operationStatus }, + _blockingTagValue }; TelemetryDataRecorder.IncrementConfigurationRefreshRequestCounter(tagList); @@ -33,7 +41,8 @@ public void IncrementConfigurationRefreshRequestCounter(string metadataAddress, { TelemetryConstants.IdentityModelVersionTag, ClientVer }, { TelemetryConstants.MetadataAddressTag, metadataAddress }, { TelemetryConstants.OperationStatusTag, operationStatus }, - { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() } + { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() }, + _blockingTagValue }; TelemetryDataRecorder.IncrementConfigurationRefreshRequestCounter(tagList); @@ -45,6 +54,7 @@ public void LogConfigurationRetrievalDuration(string metadataAddress, TimeSpan o { { TelemetryConstants.IdentityModelVersionTag, ClientVer }, { TelemetryConstants.MetadataAddressTag, metadataAddress }, + _blockingTagValue }; long durationInMilliseconds = (long)operationDuration.TotalMilliseconds; @@ -57,11 +67,27 @@ public void LogConfigurationRetrievalDuration(string metadataAddress, TimeSpan o { { TelemetryConstants.IdentityModelVersionTag, ClientVer }, { TelemetryConstants.MetadataAddressTag, metadataAddress }, - { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() } + { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() }, + _blockingTagValue }; long durationInMilliseconds = (long)operationDuration.TotalMilliseconds; TelemetryDataRecorder.RecordConfigurationRetrievalDurationHistogram(durationInMilliseconds, tagList); } + + public void LogBackgroundConfigurationRefreshFailure( + string metadataAddress, + Exception exception) + { + var tagList = new TagList() + { + { TelemetryConstants.IdentityModelVersionTag, ClientVer }, + { TelemetryConstants.MetadataAddressTag, metadataAddress }, + { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() }, + _blockingTagValue + }; + + TelemetryDataRecorder.IncrementBackgroundConfigurationRefreshFailureCounter(tagList); + } } } diff --git a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryConstants.cs b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryConstants.cs index a4d9449e75..643b603305 100644 --- a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryConstants.cs +++ b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryConstants.cs @@ -27,6 +27,11 @@ internal static class TelemetryConstants /// public const string ExceptionTypeTag = "ExceptionType"; + /// + /// Telemetry tag indicating if the update was blocking. + /// + public const string BlockingTypeTag = "Blocking"; + public static class Protocols { // Configuration manager refresh statuses diff --git a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryDataRecorder.cs b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryDataRecorder.cs index 5023606081..cafcbe9b33 100644 --- a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryDataRecorder.cs +++ b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryDataRecorder.cs @@ -26,9 +26,16 @@ internal class TelemetryDataRecorder /// /// Counter to capture configuration refresh requests to ConfigurationManager. /// + internal static readonly Counter ConfigurationManagerCounter = IdentityModelMeter.CreateCounter(IdentityModelConfigurationManagerCounterName, description: IdentityModelConfigurationManagerCounterDescription); internal const string IdentityModelConfigurationManagerCounterName = "IdentityModelConfigurationManager"; internal const string IdentityModelConfigurationManagerCounterDescription = "Counter capturing configuration manager operations."; - internal static readonly Counter ConfigurationManagerCounter = IdentityModelMeter.CreateCounter(IdentityModelConfigurationManagerCounterName, description: IdentityModelConfigurationManagerCounterDescription); + + /// + /// Counter to capture background refresh failures in the ConfigurationManager. + /// + internal static readonly Counter BackgroundConfigurationRefreshFailureCounter = IdentityModelMeter.CreateCounter(BackgroundConfigurationRefreshFailureCounterName, description: BackgroundConfigurationRefreshFailureCounterDescription); + internal const string BackgroundConfigurationRefreshFailureCounterName = "IdentityModelConfigurationManagerBackgroundRefreshFailure"; + internal const string BackgroundConfigurationRefreshFailureCounterDescription = "Counter capturing configuration manager background refresh failures."; /// /// Histogram to capture total duration of configuration retrieval by ConfigurationManager in milliseconds. @@ -47,5 +54,10 @@ internal static void IncrementConfigurationRefreshRequestCounter(in TagList tagL { ConfigurationManagerCounter.Add(1, tagList); } + + internal static void IncrementBackgroundConfigurationRefreshFailureCounter(in TagList tagList) + { + BackgroundConfigurationRefreshFailureCounter.Add(1, tagList); + } } } diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTelemetryTests.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTelemetryTests.cs index 583781a2d8..e54911535c 100644 --- a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTelemetryTests.cs +++ b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTelemetryTests.cs @@ -6,20 +6,36 @@ using System.IO; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Time.Testing; using Microsoft.IdentityModel.Logging; using Microsoft.IdentityModel.Protocols.Configuration; using Microsoft.IdentityModel.Protocols.OpenIdConnect.Configuration; -using Microsoft.IdentityModel.TestUtils; using Microsoft.IdentityModel.Telemetry; using Microsoft.IdentityModel.Telemetry.Tests; +using Microsoft.IdentityModel.TestUtils; +using Microsoft.IdentityModel.Tokens; using Xunit; namespace Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests { + [ResetAppContextSwitches] + [Collection(nameof(AppContextSwitches.UpdateConfigAsBlocking))] public class ConfigurationManagerTelemetryTests { [Fact] public async Task RequestRefresh_ExpectedTagsExist() + { + await RequestRefresh_ExpectedTagsBody(); + } + + [Fact] + public async Task RequestRefresh_ExpectedTagsExist_Blocking() + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + await RequestRefresh_ExpectedTagsBody(true); + } + + private static async Task RequestRefresh_ExpectedTagsBody(bool blocking = false) { // arrange var testTelemetryClient = new MockTelemetryClient(); @@ -31,16 +47,20 @@ public async Task RequestRefresh_ExpectedTagsExist() { TelemetryClient = testTelemetryClient }; - var cancel = new CancellationToken(); + + AutoResetEvent resetEvent = ConfigurationManagerTests.SetupResetEvent(configurationManager, blocking); // act // Retrieve the configuration for the first time - await configurationManager.GetConfigurationAsync(cancel); + await configurationManager.GetConfigurationAsync(); testTelemetryClient.ClearExportedItems(); // Manually request a config refresh configurationManager.RequestRefresh(); - await configurationManager.GetConfigurationAsync(cancel); + await configurationManager.GetConfigurationAsync(); + + if (!blocking) + ConfigurationManagerTests.WaitOrFail(resetEvent); // assert var expectedCounterTagList = new Dictionary @@ -56,12 +76,34 @@ public async Task RequestRefresh_ExpectedTagsExist() { TelemetryConstants.IdentityModelVersionTag, IdentityModelTelemetryUtil.ClientVer } }; + configurationManager.ShutdownBackgroundTask(); + + await ConfigurationManagerTests.PollForConditionAsync( + () => expectedCounterTagList.Count == testTelemetryClient.ExportedItems.Count && + expectedHistogramTagList.Count == testTelemetryClient.ExportedHistogramItems.Count, + TimeSpan.FromMilliseconds(250), + TimeSpan.FromSeconds(20)); + Assert.Equal(expectedCounterTagList, testTelemetryClient.ExportedItems); Assert.Equal(expectedHistogramTagList, testTelemetryClient.ExportedHistogramItems); } [Theory, MemberData(nameof(GetConfiguration_ExpectedTagList_TheoryData), DisableDiscoveryEnumeration = true)] public async Task GetConfigurationAsync_ExpectedTagsExist(ConfigurationManagerTelemetryTheoryData theoryData) + { + await GetConfigurationAsync_ExpectedTagList_Body(theoryData); + } + + [Theory, MemberData(nameof(GetConfiguration_ExpectedTagList_TheoryData), DisableDiscoveryEnumeration = true)] + public async Task GetConfigurationAsync_ExpectedTagsExist_Blocking(ConfigurationManagerTelemetryTheoryData theoryData) + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + await GetConfigurationAsync_ExpectedTagList_Body(theoryData, true); + } + + private static async Task GetConfigurationAsync_ExpectedTagList_Body( + ConfigurationManagerTelemetryTheoryData theoryData, + bool blocking = false) { var testTelemetryClient = new MockTelemetryClient(); @@ -74,21 +116,42 @@ public async Task GetConfigurationAsync_ExpectedTagsExist(ConfigurationManagerTe TelemetryClient = testTelemetryClient }; + AutoResetEvent resetEvent = ConfigurationManagerTests.SetupResetEvent(configurationManager, blocking); + + var timeProvider = new FakeTimeProvider(); + TestUtilities.SetField(configurationManager, "_timeProvider", timeProvider); + + OpenIdConnectConfiguration firstConfig = null; + OpenIdConnectConfiguration secondConfig = null; + try { - await configurationManager.GetConfigurationAsync(); - if (theoryData.SyncAfter != null) + firstConfig = await configurationManager.GetConfigurationAsync(); + if (theoryData.AdjustTime.HasValue) { testTelemetryClient.ClearExportedItems(); - TestUtilities.SetField(configurationManager, "_syncAfter", theoryData.SyncAfter); - await configurationManager.GetConfigurationAsync(); - } + timeProvider.Advance(theoryData.AdjustTime.Value); + secondConfig = await configurationManager.GetConfigurationAsync(); + if (!blocking) + ConfigurationManagerTests.WaitOrFail(resetEvent); + } } catch (Exception) { // Ignore exceptions } + finally + { + configurationManager.ShutdownBackgroundTask(); + } + + await ConfigurationManagerTests.PollForConditionAsync( + () => theoryData.ExpectedTagList.Count == testTelemetryClient.ExportedItems.Count, + TimeSpan.FromMilliseconds(250), + TimeSpan.FromSeconds(20)); + + DateTime syncAfter = (DateTime)TestUtilities.GetField(configurationManager, "_syncAfter"); Assert.Equal(theoryData.ExpectedTagList, testTelemetryClient.ExportedItems); } @@ -138,7 +201,7 @@ public static TheoryData { { TelemetryConstants.MetadataAddressTag, OpenIdConfigData.AADCommonUrl }, @@ -160,7 +223,7 @@ public ConfigurationManagerTelemetryTheoryData(string testId) : base(testId) { } public IConfigurationValidator ConfigurationValidator { get; set; } - public DateTime? SyncAfter { get; set; } = null; + public TimeSpan? AdjustTime { get; set; } public Dictionary ExpectedTagList { get; set; } } diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs index 1ee7645570..61ce51ea94 100644 --- a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs +++ b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs @@ -20,6 +20,8 @@ namespace Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests { + [ResetAppContextSwitches] + [Collection(nameof(AppContextSwitches.UpdateConfigAsBlocking))] public class ConfigurationManagerTests { /// @@ -32,15 +34,17 @@ public class ConfigurationManagerTests [Theory, MemberData(nameof(GetPublicMetadataTheoryData), DisableDiscoveryEnumeration = true)] public async Task GetPublicMetadata(ConfigurationManagerTheoryData theoryData) { + var cts = new CancellationTokenSource(); CompareContext context = TestUtilities.WriteHeader($"{this}.GetPublicMetadata", theoryData); + + var configurationManager = new ConfigurationManager( + theoryData.MetadataAddress, + theoryData.ConfigurationRetriever, + theoryData.DocumentRetriever, + theoryData.ConfigurationValidator); + try { - var configurationManager = new ConfigurationManager( - theoryData.MetadataAddress, - theoryData.ConfigurationRetriever, - theoryData.DocumentRetriever, - theoryData.ConfigurationValidator); - var configuration = await configurationManager.GetConfigurationAsync(CancellationToken.None); Assert.NotNull(configuration); @@ -50,6 +54,10 @@ public async Task GetPublicMetadata(ConfigurationManagerTheoryData @@ -174,9 +182,24 @@ public void Defaults() [Fact] public async Task FetchMetadataFailureTest() + { + await FetchMetadataFailureTestBody(); + } + + [Fact] + public async Task FetchMetadataFailureTest_Blocking() + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + + await FetchMetadataFailureTestBody(); + } + + private async Task FetchMetadataFailureTestBody() { var context = new CompareContext($"{this}.FetchMetadataFailureTest"); + var cts = new CancellationTokenSource(); + var documentRetriever = new HttpDocumentRetriever(HttpResponseMessageUtils.SetupHttpClientThatReturns("OpenIdConnectMetadata.json", HttpStatusCode.NotFound)); var configManager = new ConfigurationManager("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), documentRetriever); @@ -203,6 +226,10 @@ public async Task FetchMetadataFailureTest() IdentityComparer.AreEqual(firstFetchMetadataFailure, secondFetchMetadataFailure, context); } } + finally + { + configManager.ShutdownBackgroundTask(); + } TestUtilities.AssertFailIfErrors(context); } @@ -216,11 +243,14 @@ public async Task VerifyInterlockGuardForGetConfigurationAsync() InMemoryDocumentRetriever inMemoryDocumentRetriever = InMemoryDocumentRetrieverWithEvents(waitEvent, signalEvent); waitEvent.Set(); + var cts = new CancellationTokenSource(); + var configurationManager = new ConfigurationManager( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), inMemoryDocumentRetriever); + OpenIdConnectConfiguration configuration = await configurationManager.GetConfigurationAsync(); // InMemoryDocumentRetrieverWithEvents will block until waitEvent.Set() is called. @@ -248,6 +278,9 @@ public async Task VerifyInterlockGuardForGetConfigurationAsync() // Configuration should be AADCommonV1Config configuration = await configurationManager.GetConfigurationAsync(); + + configurationManager.ShutdownBackgroundTask(); + Assert.True(configuration.Issuer.Equals(OpenIdConfigData.AADCommonV1Config.Issuer), $"configuration.Issuer from configurationManager was not as expected," + $" configuration.Issuer: '{configuration.Issuer}' != expected: '{OpenIdConfigData.AADCommonV1Config.Issuer}'."); @@ -261,12 +294,15 @@ public async Task BootstrapRefreshIntervalTest() var documentRetriever = new HttpDocumentRetriever( HttpResponseMessageUtils.SetupHttpClientThatReturns("OpenIdConnectMetadata.json", HttpStatusCode.NotFound)); + var cts = new CancellationTokenSource(); + var configManager = new ConfigurationManager( "OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), documentRetriever) { RefreshInterval = TimeSpan.FromSeconds(2) }; + // ConfigurationManager._syncAfter is set to DateTimeOffset.MinValue on startup // If obtaining the metadata fails due to error, the value should not change try @@ -276,15 +312,13 @@ public async Task BootstrapRefreshIntervalTest() catch (Exception firstFetchMetadataFailure) { // _syncAfter should not have been changed, because the fetch failed. - DateTime syncAfter = (DateTime)TestUtilities.GetField(configManager, "_syncAfter"); - if (syncAfter != DateTime.MinValue) - context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal '{DateTimeOffset.MinValue}'."); + var syncAfter = TestUtilities.GetField(configManager, "_syncAfter"); + if ((DateTime)syncAfter != DateTime.MinValue) + context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal '{DateTime.MinValue}'."); if (firstFetchMetadataFailure.InnerException == null) context.AddDiff($"Expected exception to contain inner exception for fetch metadata failure."); - DateTime requestTime = DateTime.UtcNow; - // Fetch metadata again during refresh interval, the exception should be same from above. try { @@ -297,10 +331,62 @@ public async Task BootstrapRefreshIntervalTest() context.AddDiff($"Expected exception to contain inner exception for fetch metadata failure."); // _syncAfter should not have been changed, because the fetch failed. - syncAfter = (DateTime)TestUtilities.GetField(configManager, "_syncAfter"); + syncAfter = TestUtilities.GetField(configManager, "_syncAfter"); + if ((DateTime)syncAfter != DateTime.MinValue) + context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal '{DateTime.MinValue}'."); - if (!IdentityComparer.AreDatesEqualWithEpsilon(requestTime, syncAfter, 1)) - context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal be within 1 second of '{requestTime}'."); + IdentityComparer.AreEqual(firstFetchMetadataFailure, secondFetchMetadataFailure, context); + } + } + finally + { + configManager.ShutdownBackgroundTask(); + } + + TestUtilities.AssertFailIfErrors(context); + } + + [Fact] + public async Task BootstrapRefreshIntervalTest_Blocking() + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + + var context = new CompareContext($"{this}.BootstrapRefreshIntervalTest_Blocking"); + + var documentRetriever = new HttpDocumentRetriever(HttpResponseMessageUtils.SetupHttpClientThatReturns("OpenIdConnectMetadata.json", HttpStatusCode.NotFound)); + var configManager = new ConfigurationManager("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), documentRetriever) { RefreshInterval = TimeSpan.FromSeconds(2) }; + + // First time to fetch metadata. + try + { + var configuration = await configManager.GetConfigurationAsync(); + } + catch (Exception firstFetchMetadataFailure) + { + // Refresh interval is BootstrapRefreshInterval + var syncAfter = configManager.GetType().GetField("_syncAfter", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(configManager); + if ((DateTime)syncAfter > DateTime.UtcNow + TimeSpan.FromSeconds(2)) + context.AddDiff($"Expected the refresh interval is longer than 2 seconds."); + + if (firstFetchMetadataFailure.InnerException == null) + context.AddDiff($"Expected exception to contain inner exception for fetch metadata failure."); + + // Fetch metadata again during refresh interval, the exception should be same from above. + try + { + configManager.RequestRefresh(); + var configuration = await configManager.GetConfigurationAsync(); + } + catch (Exception secondFetchMetadataFailure) + { + if (secondFetchMetadataFailure.InnerException == null) + context.AddDiff($"Expected exception to contain inner exception for fetch metadata failure."); + + syncAfter = configManager.GetType().GetField("_syncAfter", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(configManager); + + // Refresh interval is RefreshInterval + if ((DateTime)syncAfter > DateTime.UtcNow + configManager.RefreshInterval) + context.AddDiff($"Expected the refresh interval is longer than 2 seconds."); IdentityComparer.AreEqual(firstFetchMetadataFailure, secondFetchMetadataFailure, context); } @@ -312,6 +398,8 @@ public async Task BootstrapRefreshIntervalTest() [Fact] public void GetSets() { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + TestUtilities.WriteHeader($"{this}.GetSets", "GetSets", true); int ExpectedPropertyCount = 7; @@ -345,21 +433,36 @@ public void GetSets() [Theory, MemberData(nameof(AutomaticIntervalTestCases), DisableDiscoveryEnumeration = true)] public async Task AutomaticRefreshInterval(ConfigurationManagerTheoryData theoryData) + { + await AutomaticRefreshIntervalBody(theoryData); + } + + [Theory, MemberData(nameof(AutomaticIntervalTestCases), DisableDiscoveryEnumeration = true)] + public async Task AutomaticRefreshInterval_Blocking(ConfigurationManagerTheoryData theoryData) + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + await AutomaticRefreshIntervalBody(theoryData, true); + } + + private async Task AutomaticRefreshIntervalBody(ConfigurationManagerTheoryData theoryData, bool blocking = false) { var context = new CompareContext($"{this}.AutomaticRefreshInterval"); + AutoResetEvent resetEvent = SetupResetEvent(theoryData.ConfigurationManager, blocking); try { - var configuration = await theoryData.ConfigurationManager.GetConfigurationAsync(CancellationToken.None); IdentityComparer.AreEqual(configuration, theoryData.ExpectedConfiguration, context); theoryData.ConfigurationManager.MetadataAddress = theoryData.UpdatedMetadataAddress; TestUtilities.SetField(theoryData.ConfigurationManager, "_syncAfter", theoryData.SyncAfter.UtcDateTime); var updatedConfiguration = await theoryData.ConfigurationManager.GetConfigurationAsync(CancellationToken.None); - // we wait 100 ms here to make the task is finished. - Thread.Sleep(100); + + if (!blocking && theoryData.SyncAfter < DateTimeOffset.UtcNow.Add(TimeSpan.FromMinutes(1))) + WaitOrFail(resetEvent); + updatedConfiguration = await theoryData.ConfigurationManager.GetConfigurationAsync(CancellationToken.None); + IdentityComparer.AreEqual(updatedConfiguration, theoryData.ExpectedUpdatedConfiguration, context); theoryData.ExpectedException.ProcessNoException(context); @@ -368,23 +471,44 @@ public async Task AutomaticRefreshInterval(ConfigurationManagerTheoryData configurationManager, bool blocking) + { + var resetEvent = new AutoResetEvent(false); + + if (!blocking) + { + Action _waitAction = () => resetEvent.Set(); + TestUtilities.SetField(configurationManager, "_onBackgroundTaskFinish", _waitAction); + } + + return resetEvent; + } + public static TheoryData> AutomaticIntervalTestCases { get { var theoryData = new TheoryData>(); + var cts = new CancellationTokenSource(); + // Failing to get metadata returns existing. theoryData.Add(new ConfigurationManagerTheoryData("HttpFault_ReturnExisting") { ConfigurationManager = new ConfigurationManager( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever), + InMemoryDocumentRetriever) + { + }, ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV1Config, SyncAfter = DateTime.UtcNow - TimeSpan.FromDays(2), @@ -397,7 +521,9 @@ public static TheoryData( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever), + InMemoryDocumentRetriever) + { + }, ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV1Config, SyncAfter = DateTime.UtcNow + TimeSpan.FromDays(2), @@ -410,7 +536,9 @@ public static TheoryData( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever), + InMemoryDocumentRetriever) + { + }, ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV2Config, SyncAfter = DateTime.UtcNow, @@ -424,32 +552,53 @@ public static TheoryData theoryData) { - var context = new CompareContext($"{this}.RequestRefresh"); + await RequestRefreshBody(theoryData); + } + + + [Theory, MemberData(nameof(RequestRefreshTestCases), DisableDiscoveryEnumeration = true)] + public async Task RequestRefresh_Blocking(ConfigurationManagerTheoryData theoryData) + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + await RequestRefreshBody(theoryData, true); + } + private async Task RequestRefreshBody(ConfigurationManagerTheoryData theoryData, bool blocking = false) + { + var context = new CompareContext($"{this}.RequestRefresh"); var configuration = await theoryData.ConfigurationManager.GetConfigurationAsync(CancellationToken.None); IdentityComparer.AreEqual(configuration, theoryData.ExpectedConfiguration, context); + AutoResetEvent resetEvent = SetupResetEvent(theoryData.ConfigurationManager, blocking); + + var timeProvider = new FakeTimeProvider(); + TestUtilities.SetField(theoryData.ConfigurationManager, "_timeProvider", timeProvider); + // the first call to RequestRefresh will trigger a refresh with ConfigurationManager.RefreshInterval being ignored. // Testing RefreshInterval requires a two calls, the second call will trigger a refresh with ConfigurationManager.RefreshInterval being used. if (theoryData.RequestRefresh) { theoryData.ConfigurationManager.RequestRefresh(); + if (!blocking) + WaitOrFail(resetEvent); + configuration = await theoryData.ConfigurationManager.GetConfigurationAsync(CancellationToken.None); } - if (theoryData.SleepTimeInMs > 0) - Thread.Sleep(theoryData.SleepTimeInMs); - theoryData.ConfigurationManager.RefreshInterval = theoryData.RefreshInterval; theoryData.ConfigurationManager.MetadataAddress = theoryData.UpdatedMetadataAddress; + timeProvider.Advance(TimeSpan.FromMilliseconds(theoryData.SleepTimeInMs)); + theoryData.ConfigurationManager.RequestRefresh(); - if (theoryData.SleepTimeInMs > 0) - Thread.Sleep(theoryData.SleepTimeInMs); + if (!blocking && theoryData.RefreshInterval != TimeSpan.MaxValue) + WaitOrFail(resetEvent); var updatedConfiguration = await theoryData.ConfigurationManager.GetConfigurationAsync(CancellationToken.None); + theoryData.ConfigurationManager.ShutdownBackgroundTask(); + IdentityComparer.AreEqual(updatedConfiguration, theoryData.ExpectedUpdatedConfiguration, context); TestUtilities.AssertFailIfErrors(context); @@ -461,13 +610,15 @@ public static TheoryData>(); - // RefreshInterval set to 1 sec should return new config. + var cts = new CancellationTokenSource(); theoryData.Add(new ConfigurationManagerTheoryData("RequestRefresh_TimeSpan_1000ms") { ConfigurationManager = new ConfigurationManager( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever), + InMemoryDocumentRetriever) + { + }, ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV2Config, RefreshInterval = TimeSpan.FromSeconds(1), @@ -476,13 +627,15 @@ public static TheoryData("RequestRefresh_TimeSpan_MaxValue") { ConfigurationManager = new ConfigurationManager( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever), + InMemoryDocumentRetriever) + { + }, ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV1Config, RefreshInterval = TimeSpan.MaxValue, @@ -491,16 +644,18 @@ public static TheoryData("RequestRefresh_FirstRefresh") { ConfigurationManager = new ConfigurationManager( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever), + InMemoryDocumentRetriever) + { + }, ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV2Config, - SleepTimeInMs = 100, + SleepTimeInMs = 1000, UpdatedMetadataAddress = "AADCommonV2Json" }); @@ -516,12 +671,18 @@ public async Task HttpFailures(ConfigurationManagerTheoryData("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever); + var configManager = new ConfigurationManager( + "OpenIdConnectMetadata.json", + new OpenIdConnectConfigurationRetriever(), + docRetriever); + + + AutoResetEvent resetEvent = SetupResetEvent(configManager, blocking); // This is the minimum time that should pass before an automatic refresh occurs // stored in advance to avoid any time drift issues. @@ -582,8 +763,9 @@ public async Task CheckSyncAfterAndRefreshRequested() // force a refresh by setting internal field TestUtilities.SetField(configManager, "_syncAfter", DateTime.UtcNow.Subtract(TimeSpan.FromHours(1))); configuration = await configManager.GetConfigurationAsync(CancellationToken.None); - // wait 1000ms here because update of config is run as a new task. - Thread.Sleep(1000); + + if (!blocking) + WaitOrFail(resetEvent); // check that _syncAfter is greater than DateTimeOffset.UtcNow + AutomaticRefreshInterval DateTime syncAfter = (DateTime)TestUtilities.GetField(configManager, "_syncAfter"); @@ -596,43 +778,74 @@ public async Task CheckSyncAfterAndRefreshRequested() configManager.RequestRefresh(); - bool refreshRequested = (bool)TestUtilities.GetField(configManager, "_refreshRequested"); - if (!refreshRequested) - context.Diffs.Add("Refresh is expected to be requested after RequestRefresh is called"); + if (blocking) + { + bool refreshRequested = (bool)TestUtilities.GetField(configManager, "_refreshRequested"); + if (!refreshRequested) + context.Diffs.Add("Refresh is expected to be requested after RequestRefresh is called"); + } await configManager.GetConfigurationAsync(); - refreshRequested = (bool)TestUtilities.GetField(configManager, "_refreshRequested"); - if (refreshRequested) - context.Diffs.Add("Refresh is not expected to be requested after GetConfigurationAsync is called"); + if (blocking) + { + bool refreshRequested = (bool)TestUtilities.GetField(configManager, "_refreshRequested"); + if (refreshRequested) + context.Diffs.Add("Refresh is expected to be requested after RequestRefresh is called"); + } + + if (!blocking) + WaitOrFail(resetEvent); // check that _syncAfter is greater than DateTimeOffset.UtcNow + AutomaticRefreshInterval syncAfter = (DateTime)TestUtilities.GetField(configManager, "_syncAfter"); if (syncAfter < minimumRefreshInterval) context.Diffs.Add($"(RequestRefresh) syncAfter '{syncAfter}' < DateTimeOffset.UtcNow + configManager.AutomaticRefreshInterval: '{minimumRefreshInterval}'."); + configManager.ShutdownBackgroundTask(); + TestUtilities.AssertFailIfErrors(context); } [Fact] public async Task GetConfigurationAsync() { - var docRetriever = new FileDocumentRetriever(); - var configManager = new ConfigurationManager("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever); + await GetConfigurationBody(); + } + + [Fact] + public async Task GetConfigurationAsync_Blocking() + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + await GetConfigurationBody(); + } + + private async Task GetConfigurationBody() + { var context = new CompareContext($"{this}.GetConfiguration"); + var cts = new CancellationTokenSource(); + + var docRetriever = new FileDocumentRetriever(); + var configManager = new ConfigurationManager( + "OpenIdConnectMetadata.json", + new OpenIdConnectConfigurationRetriever(), + docRetriever); + - // Unable to obtain a new configuration, but _currentConfiguration is not null so it should be returned. - configManager = new ConfigurationManager("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever); var configuration = await configManager.GetConfigurationAsync(CancellationToken.None); TestUtilities.SetField(configManager, "_lastRequestRefresh", DateTime.UtcNow.Subtract(TimeSpan.FromHours(1))); configManager.MetadataAddress = "http://127.0.0.1"; configManager.RequestRefresh(); + + // Unable to obtain a new configuration, but _currentConfiguration is not null so it should be returned. var configuration2 = await configManager.GetConfigurationAsync(CancellationToken.None); IdentityComparer.AreEqual(configuration, configuration2, context); if (!object.ReferenceEquals(configuration, configuration2)) context.Diffs.Add("!object.ReferenceEquals(configuration, configuration2)"); + configManager.ShutdownBackgroundTask(); + // get configuration from http address, should throw // get configuration with unsuccessful HTTP response status code TestUtilities.AssertFailIfErrors(context); @@ -642,6 +855,18 @@ public async Task GetConfigurationAsync() // a new LKG is set. [Fact] public void ResetLastKnownGoodLifetime() + { + ResetLastKnownGoodLifetimeBody(); + } + + [Fact] + public void ResetLastKnownGoodLifetime_Blocking() + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + ResetLastKnownGoodLifetimeBody(); + } + + private void ResetLastKnownGoodLifetimeBody() { TestUtilities.WriteHeader($"{this}.ResetLastKnownGoodLifetime"); var context = new CompareContext(); @@ -720,21 +945,43 @@ public void TestConfigurationComparer() [Fact] public async Task RequestRefresh_RespectsRefreshInterval() + { + await RequestRefresh_RespectsRefreshInterval_Body(); + } + + [Fact] + public async Task RequestRefresh_RespectsRefreshInterval_Blocking() + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + await RequestRefresh_RespectsRefreshInterval_Body(true); + } + + private async Task RequestRefresh_RespectsRefreshInterval_Body(bool blocking = false) { // This test checks that the _syncAfter field is set correctly after a refresh. var context = new CompareContext($"{this}.RequestRefresh_RespectsRefreshInterval"); + var cts = new CancellationTokenSource(); var timeProvider = new FakeTimeProvider(); var docRetriever = new FileDocumentRetriever(); - var configManager = new ConfigurationManager("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever); + var configManager = new ConfigurationManager( + "OpenIdConnectMetadata.json", + new OpenIdConnectConfigurationRetriever(), + docRetriever); + TestUtilities.SetField(configManager, "_timeProvider", timeProvider); + var resetEvent = SetupResetEvent(configManager, blocking); + // Get the first configuration. var configuration = await configManager.GetConfigurationAsync(CancellationToken.None); configManager.RequestRefresh(); + if (!blocking) + WaitOrFail(resetEvent); + var configAfterFirstRefresh = await configManager.GetConfigurationAsync(CancellationToken.None); // First RequestRefresh triggers a refresh. @@ -754,6 +1001,9 @@ public async Task RequestRefresh_RespectsRefreshInterval() configManager.RequestRefresh(); + if (!blocking) + WaitOrFail(resetEvent); + var configAfterRefreshInterval = await configManager.GetConfigurationAsync(CancellationToken.None); // Third RequestRefresh should trigger a refresh because the refresh interval has passed. @@ -763,6 +1013,8 @@ public async Task RequestRefresh_RespectsRefreshInterval() // Advance time just prior to a refresh. timeProvider.Advance(configManager.RefreshInterval.Subtract(TimeSpan.FromSeconds(1))); + configManager.RequestRefresh(); + var configAfterLessThanRefreshInterval = await configManager.GetConfigurationAsync(CancellationToken.None); // Fourth RequestRefresh should not trigger a refresh because the refresh interval has not passed. @@ -772,30 +1024,52 @@ public async Task RequestRefresh_RespectsRefreshInterval() // Advance time 365 days. timeProvider.Advance(TimeSpan.FromDays(365)); + configManager.RequestRefresh(); + + if (!blocking) + WaitOrFail(resetEvent); + var configAfterOneYear = await configManager.GetConfigurationAsync(CancellationToken.None); // Fifth RequestRefresh should trigger a refresh because the refresh interval has passed. - if (!object.ReferenceEquals(configAfterLessThanRefreshInterval, configAfterOneYear)) + if (object.ReferenceEquals(configAfterLessThanRefreshInterval, configAfterOneYear)) context.Diffs.Add("object.ReferenceEquals(configAfterLessThanRefreshInterval, configAfterOneYear)"); + configManager.ShutdownBackgroundTask(); + TestUtilities.AssertFailIfErrors(context); } [Fact] public async Task GetConfigurationAsync_RespectsRefreshInterval() + { + await GetConfigurationAsync_RespectsRefreshIntervalBody(); + } + + [Fact] + public async Task GetConfigurationAsync_RespectsRefreshInterval_Blocking() + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + await GetConfigurationAsync_RespectsRefreshIntervalBody(true); + } + + private async Task GetConfigurationAsync_RespectsRefreshIntervalBody(bool blocking = false) { var context = new CompareContext($"{this}.GetConfigurationAsync_RespectsRefreshInterval"); var timeProvider = new FakeTimeProvider(); - var docRetriever = new FileDocumentRetriever(); - var configManager = new ConfigurationManager("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever); - TestUtilities.SetField(configManager, "_timeProvider", timeProvider); - TimeSpan advanceInterval = BaseConfigurationManager.DefaultAutomaticRefreshInterval.Add(TimeSpan.FromSeconds(configManager.AutomaticRefreshInterval.TotalSeconds / 20)); + var cts = new CancellationTokenSource(); + var configManager = new ConfigurationManager( + "OpenIdConnectMetadata.json", + new OpenIdConnectConfigurationRetriever(), + docRetriever); TestUtilities.SetField(configManager, "_timeProvider", timeProvider); + TimeSpan advanceInterval = BaseConfigurationManager.DefaultAutomaticRefreshInterval.Add(TimeSpan.FromSeconds(configManager.AutomaticRefreshInterval.TotalSeconds)); + // Get the first configuration. var configuration = await configManager.GetConfigurationAsync(CancellationToken.None); @@ -810,29 +1084,58 @@ public async Task GetConfigurationAsync_RespectsRefreshInterval() var configAfterTimeIsAdvanced = await configManager.GetConfigurationAsync(CancellationToken.None); - // Same config, but a task is queued to update the configuration. - if (!object.ReferenceEquals(configNoAdvanceInTime, configAfterTimeIsAdvanced)) - context.Diffs.Add("!object.ReferenceEquals(configuration, configAfterTimeIsAdvanced)"); + if (!blocking) + { + var resetEvent = SetupResetEvent(configManager, blocking); + // Same config, but a task is queued to update the configuration. + if (!object.ReferenceEquals(configNoAdvanceInTime, configAfterTimeIsAdvanced)) + context.Diffs.Add("!object.ReferenceEquals(configuration, configAfterTimeIsAdvanced)"); - // Need to wait for background task to finish. - Thread.Sleep(250); + // Need to wait for background task to finish. + WaitOrFail(resetEvent); - var configAfterBackgroundTask = await configManager.GetConfigurationAsync(CancellationToken.None); + var configAfterBackgroundTask = await configManager.GetConfigurationAsync(CancellationToken.None); - // Configuration should be updated after the background task finishes. - if (object.ReferenceEquals(configAfterTimeIsAdvanced, configAfterBackgroundTask)) - context.Diffs.Add("object.ReferenceEquals(configuration, configAfterBackgroundTask)"); + // Configuration should be updated after the background task finishes. + if (object.ReferenceEquals(configAfterTimeIsAdvanced, configAfterBackgroundTask)) + context.Diffs.Add("object.ReferenceEquals(configuration, configAfterBackgroundTask)"); + } + else + { + if (object.ReferenceEquals(configAfterTimeIsAdvanced, configuration)) + context.Diffs.Add("object.ReferenceEquals(configAfterTimeIsAdvanced, configuration)"); + } + + configManager.ShutdownBackgroundTask(); TestUtilities.AssertFailIfErrors(context); } + [Theory, MemberData(nameof(ValidateOpenIdConnectConfigurationTestCases), DisableDiscoveryEnumeration = true)] + public async Task ValidateOpenIdConnectConfigurationTests_Blocking(ConfigurationManagerTheoryData theoryData) + { + AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); + await ValidateOIDCConfigurationBody(theoryData, true); + } + [Theory, MemberData(nameof(ValidateOpenIdConnectConfigurationTestCases), DisableDiscoveryEnumeration = true)] public async Task ValidateOpenIdConnectConfigurationTests(ConfigurationManagerTheoryData theoryData) + { + await ValidateOIDCConfigurationBody(theoryData); + } + + private async Task ValidateOIDCConfigurationBody(ConfigurationManagerTheoryData theoryData, bool blocking = false) { TestUtilities.WriteHeader($"{this}.ValidateOpenIdConnectConfigurationTests"); var context = new CompareContext(); OpenIdConnectConfiguration configuration; - var configurationManager = new ConfigurationManager(theoryData.MetadataAddress, theoryData.ConfigurationRetriever, theoryData.DocumentRetriever, theoryData.ConfigurationValidator); + var configurationManager = new ConfigurationManager( + theoryData.MetadataAddress, + theoryData.ConfigurationRetriever, + theoryData.DocumentRetriever, + theoryData.ConfigurationValidator); + + var resetEvent = SetupResetEvent(configurationManager, blocking); if (theoryData.PresetCurrentConfiguration) TestUtilities.SetField(configurationManager, "_currentConfiguration", new OpenIdConnectConfiguration() { Issuer = Default.Issuer }); @@ -840,11 +1143,21 @@ public async Task ValidateOpenIdConnectConfigurationTests(ConfigurationManagerTh try { //create a listener and enable it for logs - var listener = TestUtils.SampleListener.CreateLoggerListener(EventLevel.Warning); + using var listener = TestUtils.SampleListener.CreateLoggerListener(EventLevel.Warning); + configuration = await configurationManager.GetConfigurationAsync(); - // we need to sleep here to make sure the task that updates configuration has finished. - Thread.Sleep(250); + if (!blocking && theoryData.ExpectedException is null && string.IsNullOrEmpty(theoryData.ExpectedErrorMessage)) + WaitOrFail(resetEvent); + + // Need to wait for the events on the listener to be processed. + if (!string.IsNullOrEmpty(theoryData.ExpectedErrorMessage)) + { + var success = await PollForConditionAsync( + () => listener.TraceBuffer.Contains(theoryData.ExpectedErrorMessage), + TimeSpan.FromMilliseconds(100), + TimeSpan.FromSeconds(10)); + } if (!string.IsNullOrEmpty(theoryData.ExpectedErrorMessage) && !listener.TraceBuffer.Contains(theoryData.ExpectedErrorMessage)) context.AddDiff($"Expected exception to contain: '{theoryData.ExpectedErrorMessage}'.{Environment.NewLine}Log is:{Environment.NewLine}'{listener.TraceBuffer}'"); @@ -858,10 +1171,36 @@ public async Task ValidateOpenIdConnectConfigurationTests(ConfigurationManagerTh theoryData.ExpectedException.ProcessException(ex, context); } + finally + { + configurationManager.ShutdownBackgroundTask(); + } TestUtilities.AssertFailIfErrors(context); } + internal static async Task PollForConditionAsync(Func condition, TimeSpan interval, TimeSpan timeout) + { + var startTime = DateTime.UtcNow; + + while (DateTime.UtcNow - startTime < timeout) + { + if (condition()) + return true; + + try + { + await Task.Delay(interval); + } + catch (TaskCanceledException) + { + return false; + } + } + + return false; + } + public static TheoryData> ValidateOpenIdConnectConfigurationTestCases { get @@ -877,7 +1216,7 @@ public static TheoryData @@ -887,7 +1226,7 @@ public static TheoryData @@ -898,7 +1237,7 @@ public static TheoryData @@ -908,7 +1247,7 @@ public static TheoryData @@ -919,7 +1258,7 @@ public static TheoryData @@ -929,7 +1268,7 @@ public static TheoryData @@ -940,7 +1279,7 @@ public static TheoryData @@ -950,7 +1289,7 @@ public static TheoryData @@ -961,7 +1300,7 @@ public static TheoryData : TheoryDataBase where T : class { public ConfigurationManager ConfigurationManager { get; set; } diff --git a/test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs b/test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs index 71cf3e66d8..93216034c2 100644 --- a/test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs +++ b/test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs @@ -77,11 +77,14 @@ public async Task ConfigurationManagerUsingCustomClass() if (!IdentityComparer.AreEqual(configuration.Issuer, configuration2.Issuer, context)) context.Diffs.Add("!IdentityComparer.AreEqual(configuration, configuration2)"); + configManager.ShutdownBackgroundTask(); + // AutomaticRefreshInterval should pick up new bits. configManager = new ConfigurationManager("IssuerMetadata.json", new IssuerConfigurationRetriever(), docRetriever); configManager.RequestRefresh(); configuration = await configManager.GetConfigurationAsync(); TestUtilities.SetField(configManager, "_lastRequestRefresh", DateTime.UtcNow.Subtract(TimeSpan.FromHours(1))); + configManager.MetadataAddress = "IssuerMetadata2.json"; // Wait for the refresh to complete. @@ -101,6 +104,8 @@ public async Task ConfigurationManagerUsingCustomClass() if (IdentityComparer.AreEqual(configuration.Issuer, configuration2.Issuer)) context.Diffs.Add($"Expected: {configuration.Issuer}, to be different from: {configuration2.Issuer}"); + configManager.ShutdownBackgroundTask(); + TestUtilities.AssertFailIfErrors(context); } diff --git a/test/Microsoft.IdentityModel.TestUtils/ResetAppContextSwitchesAttribute.cs b/test/Microsoft.IdentityModel.TestUtils/ResetAppContextSwitchesAttribute.cs new file mode 100644 index 0000000000..57e1d92231 --- /dev/null +++ b/test/Microsoft.IdentityModel.TestUtils/ResetAppContextSwitchesAttribute.cs @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Reflection; +using Microsoft.IdentityModel.Tokens; +using Xunit.Sdk; + +namespace Microsoft.IdentityModel.TestUtils +{ + /// + public class ResetAppContextSwitchesAttribute : BeforeAfterTestAttribute + { + public override void Before(MethodInfo methodUnderTest) + { + AppContextSwitches.ResetAllSwitches(); + } + + public override void After(MethodInfo methodUnderTest) + { + AppContextSwitches.ResetAllSwitches(); + } + } +} diff --git a/test/Microsoft.IdentityModel.Tokens.Tests/Telemetry/MockTelemetryClient.cs b/test/Microsoft.IdentityModel.Tokens.Tests/Telemetry/MockTelemetryClient.cs index ba76c4c0fe..fa3d7a4134 100644 --- a/test/Microsoft.IdentityModel.Tokens.Tests/Telemetry/MockTelemetryClient.cs +++ b/test/Microsoft.IdentityModel.Tokens.Tests/Telemetry/MockTelemetryClient.cs @@ -15,6 +15,7 @@ public class MockTelemetryClient : ITelemetryClient public void ClearExportedItems() { ExportedItems.Clear(); + ExportedHistogramItems.Clear(); } public void IncrementConfigurationRefreshRequestCounter(string metadataAddress, string operationStatus) @@ -44,5 +45,12 @@ public void LogConfigurationRetrievalDuration(string metadataAddress, TimeSpan o ExportedHistogramItems.Add(TelemetryConstants.MetadataAddressTag, metadataAddress); ExportedHistogramItems.Add(TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString()); } + + void ITelemetryClient.LogBackgroundConfigurationRefreshFailure(string metadataAddress, Exception exception) + { + ExportedItems.Add(TelemetryConstants.IdentityModelVersionTag, IdentityModelTelemetryUtil.ClientVer); + ExportedItems.Add(TelemetryConstants.MetadataAddressTag, metadataAddress); + ExportedItems.Add(TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString()); + } } }