diff --git a/build/commonTest.props b/build/commonTest.props index 74189e5955..eaf97cf0f5 100644 --- a/build/commonTest.props +++ b/build/commonTest.props @@ -51,7 +51,12 @@ - + + true + + + + true diff --git a/build/dependenciesTest.props b/build/dependenciesTest.props index 6bcbbd9361..2fc73e2b67 100644 --- a/build/dependenciesTest.props +++ b/build/dependenciesTest.props @@ -20,7 +20,6 @@ 3.0.0-pre.49 - 9.0.0 @@ -28,5 +27,4 @@ 8.10.0 - diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.HeaderClaimSet.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.HeaderClaimSet.cs index 4694399aa7..2a166298ab 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.HeaderClaimSet.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.HeaderClaimSet.cs @@ -82,7 +82,7 @@ internal JsonClaimSet CreateHeaderClaimSet(ReadOnlySpan byteSpan) break; else if (!reader.Read()) break; - }; + } return new JsonClaimSet(claims); } diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs index dac70e195b..844594ccc5 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs @@ -47,7 +47,7 @@ internal JsonClaimSet CreatePayloadClaimSet(ReadOnlySpan byteSpan) break; else if (!reader.Read()) break; - }; + } return new JsonClaimSet(claims); } diff --git a/src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs b/src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs index 36cac13e31..d106b7286d 100644 --- a/src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs +++ b/src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs @@ -210,7 +210,7 @@ public virtual void SetParameters(NameValueCollection nameValueCollection) foreach (string key in nameValueCollection.AllKeys) { SetParameter(key, nameValueCollection[key]); - }; + } } /// diff --git a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs index 200288572e..4891ade703 100644 --- a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs +++ b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs @@ -3,7 +3,6 @@ using System; using System.Net.Http; -using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.IdentityModel.Logging; @@ -18,28 +17,12 @@ namespace Microsoft.IdentityModel.Protocols /// /// The type of . [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1001:TypesThatOwnDisposableFieldsShouldBeDisposable")] - public partial class ConfigurationManager : BaseConfigurationManager, IConfigurationManager where T : class + public class ConfigurationManager : BaseConfigurationManager, IConfigurationManager where T : class { -#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 - { - get => _syncAfter; - set => AtomicUpdateDateTime(ref _syncAfter, ref value); - } - - private DateTime _lastRequestRefresh = DateTime.MinValue; - private DateTime LastRequestRefresh - { - get => _lastRequestRefresh; - set => AtomicUpdateDateTime(ref _lastRequestRefresh, ref value); - } + private DateTimeOffset _syncAfter = DateTimeOffset.MinValue; + private DateTimeOffset _lastRequestRefresh = DateTimeOffset.MinValue; private bool _isFirstRefreshRequest = true; private readonly SemaphoreSlim _configurationNullLock = new SemaphoreSlim(1); @@ -55,41 +38,9 @@ private DateTime LastRequestRefresh private const int ConfigurationRetrieverRunning = 1; private int _configurationRetrieverState = ConfigurationRetrieverIdle; - private readonly TimeProvider _timeProvider = TimeProvider.System; + internal TimeProvider TimeProvider = TimeProvider.System; internal ITelemetryClient TelemetryClient = new TelemetryClient(); - // If a refresh is requested, then do the refresh as a blocking operation - // not on a background thread. RequestRefresh signals that the app is explicitly - // 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. - 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. /// @@ -151,10 +102,6 @@ public ConfigurationManager(string metadataAddress, IConfigurationRetriever c MetadataAddress = metadataAddress; _docRetriever = docRetriever; _configRetriever = configRetriever; - _backgroundRefreshTaskCancellationTokenSource = new CancellationTokenSource(); - - if (!AppContextSwitches.UpdateConfigAsBlocking) - EnsureBackgroundRefreshTaskIsRunning(); } /// @@ -191,14 +138,8 @@ public ConfigurationManager(string metadataAddress, IConfigurationRetriever c /// /// Obtains an updated version of Configuration. /// - /// 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 . + /// Configuration of type T. + /// If the time since the last call is less than then is not called and the current Configuration is returned. public async Task GetConfigurationAsync() { return await GetConfigurationAsync(CancellationToken.None).ConfigureAwait(false); @@ -208,27 +149,13 @@ public async Task GetConfigurationAsync() /// Obtains an updated version of Configuration. /// /// CancellationToken - /// 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 . + /// Configuration of type T. + /// If the time since the last call is less than then is not called and the current Configuration is returned. public virtual async Task GetConfigurationAsync(CancellationToken cancel) { - if (_currentConfiguration != null && SyncAfter > _timeProvider.GetUtcNow()) + 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 @@ -304,15 +231,11 @@ private async Task GetConfigurationWithBackgroundTaskUpdatesAsync(Cancellatio { if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverRunning, ConfigurationRetrieverIdle) == ConfigurationRetrieverIdle) { - if (SyncAfter <= _timeProvider.GetUtcNow()) - { - EnsureBackgroundRefreshTaskIsRunning(); - _updateMetadataEvent.Set(); - } - else - { - Interlocked.Exchange(ref _configurationRetrieverState, ConfigurationRetrieverIdle); - } + TelemetryClient.IncrementConfigurationRefreshRequestCounter( + MetadataAddress, + TelemetryConstants.Protocols.Automatic); + + _ = Task.Run(UpdateCurrentConfiguration, CancellationToken.None); } } @@ -325,58 +248,11 @@ private async Task GetConfigurationWithBackgroundTaskUpdatesAsync(Cancellatio LogHelper.FormatInvariant( LogMessages.IDX20803, LogHelper.MarkAsNonPII(MetadataAddress ?? "null"), - LogHelper.MarkAsNonPII(SyncAfter), + LogHelper.MarkAsNonPII(_syncAfter), LogHelper.MarkAsNonPII(fetchMetadataFailure)), 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: @@ -384,9 +260,8 @@ private void UpdateCurrentConfigurationUsingSignals() /// private void UpdateCurrentConfiguration() { - TelemetryForUpdate(); #pragma warning disable CA1031 // Do not catch general exception types - long startTimestamp = _timeProvider.GetTimestamp(); + long startTimestamp = TimeProvider.GetTimestamp(); try { @@ -395,7 +270,7 @@ private void UpdateCurrentConfiguration() _docRetriever, CancellationToken.None).ConfigureAwait(false).GetAwaiter().GetResult(); - var elapsedTime = _timeProvider.GetElapsedTime(startTimestamp); + var elapsedTime = TimeProvider.GetElapsedTime(startTimestamp); TelemetryClient.LogConfigurationRetrievalDuration( MetadataAddress, elapsedTime); @@ -420,7 +295,7 @@ private void UpdateCurrentConfiguration() } catch (Exception ex) { - var elapsedTime = _timeProvider.GetElapsedTime(startTimestamp); + var elapsedTime = TimeProvider.GetElapsedTime(startTimestamp); TelemetryClient.LogConfigurationRetrievalDuration( MetadataAddress, elapsedTime, @@ -438,25 +313,16 @@ private void UpdateCurrentConfiguration() { Interlocked.Exchange(ref _configurationRetrieverState, ConfigurationRetrieverIdle); } + + _onBackgroundTaskFinish?.Invoke(); #pragma warning restore CA1031 // Do not catch general exception types } private void UpdateConfiguration(T configuration) { _currentConfiguration = configuration; - var newSyncTime = DateTimeUtil.Add(_timeProvider.GetUtcNow().UtcDateTime, AutomaticRefreshInterval + + _syncAfter = DateTimeUtil.Add(TimeProvider.GetUtcNow().DateTime, AutomaticRefreshInterval + TimeSpan.FromSeconds(new Random().Next((int)AutomaticRefreshInterval.TotalSeconds / 20))); - SyncAfter = newSyncTime; - } - - 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. - // time will always be updated to a UTC time. - // See the implementation of ToBinary on DateTime. - Interlocked.Exchange( - ref Unsafe.As(ref field), - Unsafe.As(ref value)); } /// @@ -479,30 +345,19 @@ public override async Task GetBaseConfigurationAsync(Cancella /// public override void RequestRefresh() { - if (AppContextSwitches.UpdateConfigAsBlocking) - { - RequestRefreshBlocking(); - } - else - { - RequestRefreshBackgroundThread(); - } - } - - private void RequestRefreshBackgroundThread() - { - DateTime now = _timeProvider.GetUtcNow().UtcDateTime; + DateTimeOffset now = TimeProvider.GetUtcNow(); - if (now >= DateTimeUtil.Add(LastRequestRefresh, RefreshInterval) || _isFirstRefreshRequest) + if (now >= DateTimeUtil.Add(_lastRequestRefresh.UtcDateTime, RefreshInterval) || _isFirstRefreshRequest) { - _isFirstRefreshRequest = false; + TelemetryClient.IncrementConfigurationRefreshRequestCounter( + MetadataAddress, + TelemetryConstants.Protocols.Manual); + _isFirstRefreshRequest = false; if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverRunning, ConfigurationRetrieverIdle) == ConfigurationRetrieverIdle) { - _refreshRequested = true; - LastRequestRefresh = now; - EnsureBackgroundRefreshTaskIsRunning(); - _updateMetadataEvent.Set(); + _ = Task.Run(UpdateCurrentConfiguration, CancellationToken.None); + _lastRequestRefresh = now; } } } diff --git a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager_Blocking.cs b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager_Blocking.cs deleted file mode 100644 index 8ecf701233..0000000000 --- a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager_Blocking.cs +++ /dev/null @@ -1,161 +0,0 @@ -// 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 9efe0a190c..901dc16ac8 100644 --- a/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs +++ b/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs @@ -12,8 +12,3 @@ #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 06f99d657b..2d03b12600 100644 --- a/src/Microsoft.IdentityModel.Protocols/InternalAPI.Unshipped.txt +++ b/src/Microsoft.IdentityModel.Protocols/InternalAPI.Unshipped.txt @@ -1,2 +1,3 @@ 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 29c5255246..5c617a3121 100644 --- a/src/Microsoft.IdentityModel.Protocols/PublicAPI.Unshipped.txt +++ b/src/Microsoft.IdentityModel.Protocols/PublicAPI.Unshipped.txt @@ -1,3 +1,2 @@ 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 eddf948ce5..b5e9f18c26 100644 --- a/src/Microsoft.IdentityModel.Tokens/AppContextSwitches.cs +++ b/src/Microsoft.IdentityModel.Tokens/AppContextSwitches.cs @@ -70,15 +70,14 @@ 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; + /// + /// Unused, part of a previous release. This is a friend, so we cannot remove. + /// internal static bool UpdateConfigAsBlocking => _updateConfigAsBlockingCall ??= (AppContext.TryGetSwitch(UpdateConfigAsBlockingSwitch, out bool blockingCall) && blockingCall); /// @@ -97,9 +96,6 @@ internal static void ResetAllSwitches() _useRfcDefinitionOfEpkAndKid = null; AppContext.SetSwitch(UseRfcDefinitionOfEpkAndKidSwitch, false); - - _updateConfigAsBlockingCall = null; - AppContext.SetSwitch(UpdateConfigAsBlockingSwitch, false); } } } diff --git a/src/Microsoft.IdentityModel.Tokens/EventBasedLRUCache.cs b/src/Microsoft.IdentityModel.Tokens/EventBasedLRUCache.cs index 69f80ea80e..5414c89064 100644 --- a/src/Microsoft.IdentityModel.Tokens/EventBasedLRUCache.cs +++ b/src/Microsoft.IdentityModel.Tokens/EventBasedLRUCache.cs @@ -663,7 +663,6 @@ internal void WaitForProcessing() while (!_eventQueue.IsEmpty) { } - ; } #endregion diff --git a/src/Microsoft.IdentityModel.Tokens/InternalAPI.Unshipped.txt b/src/Microsoft.IdentityModel.Tokens/InternalAPI.Unshipped.txt index 8ce71ead77..c8a8b0ba43 100644 --- a/src/Microsoft.IdentityModel.Tokens/InternalAPI.Unshipped.txt +++ b/src/Microsoft.IdentityModel.Tokens/InternalAPI.Unshipped.txt @@ -142,9 +142,6 @@ static Microsoft.IdentityModel.Tokens.Utility.SerializeAsSingleCommaDelimitedStr static Microsoft.IdentityModel.Tokens.ValidationError.GetCurrentStackFrame(string filePath = "", int lineNumber = 0, int skipFrames = 1) -> System.Diagnostics.StackFrame static readonly Microsoft.IdentityModel.Telemetry.TelemetryDataRecorder.ConfigurationManagerCounter -> System.Diagnostics.Metrics.Counter static readonly Microsoft.IdentityModel.Telemetry.TelemetryDataRecorder.TotalDurationHistogram -> System.Diagnostics.Metrics.Histogram -static readonly Microsoft.IdentityModel.Tokens.IssuerValidationSource.IssuerMatchedConfiguration -> Microsoft.IdentityModel.Tokens.IssuerValidationSource -static readonly Microsoft.IdentityModel.Tokens.IssuerValidationSource.IssuerMatchedValidationParameters -> Microsoft.IdentityModel.Tokens.IssuerValidationSource -static readonly Microsoft.IdentityModel.Tokens.IssuerValidationSource.NotValidated -> Microsoft.IdentityModel.Tokens.IssuerValidationSource static readonly Microsoft.IdentityModel.Tokens.LoggingEventId.TokenValidationFailed -> Microsoft.Extensions.Logging.EventId static readonly Microsoft.IdentityModel.Tokens.LoggingEventId.TokenValidationSucceeded -> Microsoft.Extensions.Logging.EventId static readonly Microsoft.IdentityModel.Telemetry.TelemetryDataRecorder.ConfigurationManagerCounter -> System.Diagnostics.Metrics.Counter diff --git a/src/Microsoft.IdentityModel.Tokens/Telemetry/ITelemetryClient.cs b/src/Microsoft.IdentityModel.Tokens/Telemetry/ITelemetryClient.cs index 0ab0c083ff..2640ec7357 100644 --- a/src/Microsoft.IdentityModel.Tokens/Telemetry/ITelemetryClient.cs +++ b/src/Microsoft.IdentityModel.Tokens/Telemetry/ITelemetryClient.cs @@ -25,6 +25,8 @@ internal void IncrementConfigurationRefreshRequestCounter( string operationStatus, Exception exception); + // Unused, this was part of a previous release, since it is a friend, + // it cannot be removed. 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 15d1404486..9df1e3e0a9 100644 --- a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryClient.cs +++ b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryClient.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System; -using System.Collections.Generic; using System.Diagnostics; using Microsoft.IdentityModel.Logging; using Microsoft.IdentityModel.Tokens; @@ -16,19 +15,13 @@ 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 }, - _blockingTagValue + { TelemetryConstants.OperationStatusTag, operationStatus } }; TelemetryDataRecorder.IncrementConfigurationRefreshRequestCounter(tagList); @@ -41,8 +34,7 @@ public void IncrementConfigurationRefreshRequestCounter(string metadataAddress, { TelemetryConstants.IdentityModelVersionTag, ClientVer }, { TelemetryConstants.MetadataAddressTag, metadataAddress }, { TelemetryConstants.OperationStatusTag, operationStatus }, - { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() }, - _blockingTagValue + { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() } }; TelemetryDataRecorder.IncrementConfigurationRefreshRequestCounter(tagList); @@ -54,7 +46,6 @@ public void LogConfigurationRetrievalDuration(string metadataAddress, TimeSpan o { { TelemetryConstants.IdentityModelVersionTag, ClientVer }, { TelemetryConstants.MetadataAddressTag, metadataAddress }, - _blockingTagValue }; long durationInMilliseconds = (long)operationDuration.TotalMilliseconds; @@ -67,8 +58,7 @@ public void LogConfigurationRetrievalDuration(string metadataAddress, TimeSpan o { { TelemetryConstants.IdentityModelVersionTag, ClientVer }, { TelemetryConstants.MetadataAddressTag, metadataAddress }, - { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() }, - _blockingTagValue + { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() } }; long durationInMilliseconds = (long)operationDuration.TotalMilliseconds; @@ -84,7 +74,7 @@ public void LogBackgroundConfigurationRefreshFailure( { TelemetryConstants.IdentityModelVersionTag, ClientVer }, { TelemetryConstants.MetadataAddressTag, metadataAddress }, { TelemetryConstants.ExceptionTypeTag, exception.GetType().ToString() }, - _blockingTagValue + { TelemetryConstants.BlockingTypeTag, AppContextSwitches.UpdateConfigAsBlocking.ToString() } }; TelemetryDataRecorder.IncrementBackgroundConfigurationRefreshFailureCounter(tagList); diff --git a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryDataRecorder.cs b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryDataRecorder.cs index cafcbe9b33..604ec0bda0 100644 --- a/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryDataRecorder.cs +++ b/src/Microsoft.IdentityModel.Tokens/Telemetry/TelemetryDataRecorder.cs @@ -26,9 +26,9 @@ 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. diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTelemetryTests.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTelemetryTests.cs index e54911535c..696d7089c8 100644 --- a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTelemetryTests.cs +++ b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTelemetryTests.cs @@ -13,29 +13,54 @@ 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() + public async Task RequestRefresh_IntervalHasNotPassed_ExpectedCount() { - await RequestRefresh_ExpectedTagsBody(); - } + // arrange + var testTelemetryClient = new MockTelemetryClient(); + var configurationManager = new ConfigurationManager( + OpenIdConfigData.AccountsGoogle, + new OpenIdConnectConfigurationRetriever(), + new HttpDocumentRetriever(), + new OpenIdConnectConfigurationValidator()) + { + TelemetryClient = testTelemetryClient + }; + var cancel = new CancellationToken(); - [Fact] - public async Task RequestRefresh_ExpectedTagsExist_Blocking() - { - AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); - await RequestRefresh_ExpectedTagsBody(true); + AutoResetEvent resetEvent = ConfigurationManagerTests.SetupResetEvent(configurationManager); + + var timeProvider = new FakeTimeProvider(); + configurationManager.TimeProvider = timeProvider; + + // act + // Retrieve the configuration for the first time + await configurationManager.GetConfigurationAsync(cancel); + testTelemetryClient.ClearExportedItems(); + + // Manually request a config refresh + configurationManager.RequestRefresh(); + await configurationManager.GetConfigurationAsync(cancel); + + ConfigurationManagerTests.WaitOrFail(resetEvent); + + // Request a second refresh, but don't wait for the interval to pass + configurationManager.RequestRefresh(); + await configurationManager.GetConfigurationAsync(cancel); + + // assert: There should be two calls here, first from the call to GetConfigurationAsync + // the second from RequestRefresh, first request refresh always goes through + Assert.Equal(2, testTelemetryClient.RequestRefreshCounter); } - private static async Task RequestRefresh_ExpectedTagsBody(bool blocking = false) + [Fact] + public async Task RequestRefresh_ExpectedTagsExist() { // arrange var testTelemetryClient = new MockTelemetryClient(); @@ -47,20 +72,23 @@ private static async Task RequestRefresh_ExpectedTagsBody(bool blocking = false) { TelemetryClient = testTelemetryClient }; + var cancel = new CancellationToken(); - AutoResetEvent resetEvent = ConfigurationManagerTests.SetupResetEvent(configurationManager, blocking); + AutoResetEvent resetEvent = ConfigurationManagerTests.SetupResetEvent(configurationManager); + + var timeProvider = new FakeTimeProvider(); + configurationManager.TimeProvider = timeProvider; // act // Retrieve the configuration for the first time - await configurationManager.GetConfigurationAsync(); + await configurationManager.GetConfigurationAsync(cancel); testTelemetryClient.ClearExportedItems(); // Manually request a config refresh configurationManager.RequestRefresh(); - await configurationManager.GetConfigurationAsync(); + await configurationManager.GetConfigurationAsync(cancel); - if (!blocking) - ConfigurationManagerTests.WaitOrFail(resetEvent); + ConfigurationManagerTests.WaitOrFail(resetEvent); // assert var expectedCounterTagList = new Dictionary @@ -76,34 +104,12 @@ private static async Task RequestRefresh_ExpectedTagsBody(bool blocking = false) { 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(); @@ -116,42 +122,27 @@ private static async Task GetConfigurationAsync_ExpectedTagList_Body( TelemetryClient = testTelemetryClient }; - AutoResetEvent resetEvent = ConfigurationManagerTests.SetupResetEvent(configurationManager, blocking); + AutoResetEvent resetEvent = ConfigurationManagerTests.SetupResetEvent(configurationManager); var timeProvider = new FakeTimeProvider(); - TestUtilities.SetField(configurationManager, "_timeProvider", timeProvider); - - OpenIdConnectConfiguration firstConfig = null; - OpenIdConnectConfiguration secondConfig = null; + configurationManager.TimeProvider = timeProvider; try { - firstConfig = await configurationManager.GetConfigurationAsync(); - if (theoryData.AdjustTime.HasValue) + await configurationManager.GetConfigurationAsync(); + if (theoryData.SyncAfter != null) { testTelemetryClient.ClearExportedItems(); - timeProvider.Advance(theoryData.AdjustTime.Value); - secondConfig = await configurationManager.GetConfigurationAsync(); + timeProvider.Advance((theoryData.SyncAfter - DateTimeOffset.UtcNow).Value); + await configurationManager.GetConfigurationAsync(); - if (!blocking) - ConfigurationManagerTests.WaitOrFail(resetEvent); + 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); } @@ -201,7 +192,7 @@ public static TheoryData { { TelemetryConstants.MetadataAddressTag, OpenIdConfigData.AADCommonUrl }, @@ -223,7 +214,7 @@ public ConfigurationManagerTelemetryTheoryData(string testId) : base(testId) { } public IConfigurationValidator ConfigurationValidator { get; set; } - public TimeSpan? AdjustTime { get; set; } + public DateTimeOffset? SyncAfter { get; set; } = null; 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 61ce51ea94..92e3afe66a 100644 --- a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs +++ b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -// Ignore Spelling: Metadata Validator +// Ignore Spelling: Metadata Validator Retreiver using System; using System.Collections.Generic; @@ -20,8 +20,9 @@ namespace Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests { - [ResetAppContextSwitches] - [Collection(nameof(AppContextSwitches.UpdateConfigAsBlocking))] + /// + /// + /// public class ConfigurationManagerTests { /// @@ -34,17 +35,15 @@ 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.ConfigurationRetreiver, + theoryData.DocumentRetriever, + theoryData.ConfigurationValidator); + var configuration = await configurationManager.GetConfigurationAsync(CancellationToken.None); Assert.NotNull(configuration); @@ -54,10 +53,6 @@ public async Task GetPublicMetadata(ConfigurationManagerTheoryData("AccountsGoogleCom") { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = new OpenIdConnectConfigurationValidator(), DocumentRetriever = new HttpDocumentRetriever(), MetadataAddress = OpenIdConfigData.AccountsGoogle @@ -76,7 +71,7 @@ public static TheoryData("AADCommonUrl") { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = new OpenIdConnectConfigurationValidator(), DocumentRetriever = new HttpDocumentRetriever(), MetadataAddress = OpenIdConfigData.AADCommonUrl @@ -84,7 +79,7 @@ public static TheoryData("AADCommonUrlV1") { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = new OpenIdConnectConfigurationValidator(), DocumentRetriever = new HttpDocumentRetriever(), MetadataAddress = OpenIdConfigData.AADCommonUrlV1 @@ -92,7 +87,7 @@ public static TheoryData("AADCommonUrlV2") { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = new OpenIdConnectConfigurationValidator(), DocumentRetriever = new HttpDocumentRetriever(), MetadataAddress = OpenIdConfigData.AADCommonUrlV2 @@ -107,7 +102,7 @@ public void OpenIdConnectConstructor(ConfigurationManagerTheoryData(theoryData.MetadataAddress, theoryData.ConfigurationRetriever, theoryData.DocumentRetriever, theoryData.ConfigurationValidator); + var configurationManager = new ConfigurationManager(theoryData.MetadataAddress, theoryData.ConfigurationRetreiver, theoryData.DocumentRetriever, theoryData.ConfigurationValidator); theoryData.ExpectedException.ProcessNoException(); } catch (Exception ex) @@ -126,7 +121,7 @@ public static TheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = new OpenIdConnectConfigurationValidator(), DocumentRetriever = new HttpDocumentRetriever(), ExpectedException = ExpectedException.ArgumentNullException("IDX10000:"), @@ -137,17 +132,17 @@ public static TheoryData { - ConfigurationRetriever = null, + ConfigurationRetreiver = null, ConfigurationValidator = new OpenIdConnectConfigurationValidator(), DocumentRetriever = new HttpDocumentRetriever(), ExpectedException = ExpectedException.ArgumentNullException("IDX10000:"), MetadataAddress = "OpenIdConnectMetadata.json", - TestId = "ConfigurationRetriever: NULL" + TestId = "ConfigurationRetreiver: NULL" }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = new OpenIdConnectConfigurationValidator(), DocumentRetriever = null, ExpectedException = ExpectedException.ArgumentNullException("IDX10000:"), @@ -157,7 +152,7 @@ public static TheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = null, DocumentRetriever = new HttpDocumentRetriever(), ExpectedException = ExpectedException.ArgumentNullException("IDX10000:"), @@ -182,24 +177,9 @@ 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); @@ -226,14 +206,52 @@ private async Task FetchMetadataFailureTestBody() IdentityComparer.AreEqual(firstFetchMetadataFailure, secondFetchMetadataFailure, context); } } - finally - { - configManager.ShutdownBackgroundTask(); - } TestUtilities.AssertFailIfErrors(context); } + [Fact] + public async Task VerifyInterlockGuardForRequestRefresh() + { + ManualResetEvent waitEvent = new ManualResetEvent(false); + ManualResetEvent signalEvent = new ManualResetEvent(false); + InMemoryDocumentRetriever inMemoryDocumentRetriever = InMemoryDocumentRetrieverWithEvents(waitEvent, signalEvent); + + var configurationManager = new ConfigurationManager( + "AADCommonV1Json", + new OpenIdConnectConfigurationRetriever(), + inMemoryDocumentRetriever); + + // populate the configurationManager with AADCommonV1Config + TestUtilities.SetField(configurationManager, "_currentConfiguration", OpenIdConfigData.AADCommonV1Config); + + // InMemoryDocumentRetrieverWithEvents will block until waitEvent.Set() is called. + // The first RequestRefresh will not have finished before the next RequestRefresh() is called. + // The guard '_lastRequestRefresh' will not block as we set it to DateTimeOffset.MinValue. + // Interlocked guard will block. + // Configuration should be AADCommonV1Config + signalEvent.Reset(); + configurationManager.RequestRefresh(); + + // InMemoryDocumentRetrieverWithEvents will signal when it is OK to change the MetadataAddress + // otherwise, it may be the case that the MetadataAddress is changed before the previous Task has finished. + signalEvent.WaitOne(); + + // AADCommonV1Json would have been passed to the the previous retriever, which is blocked on an event. + configurationManager.MetadataAddress = "AADCommonV2Json"; + TestUtilities.SetField(configurationManager, "_lastRequestRefresh", DateTimeOffset.MinValue); + configurationManager.RequestRefresh(); + + // Set the event to release the lock and let the previous retriever finish. + waitEvent.Set(); + + // Configuration should be AADCommonV1Config + var configuration = await configurationManager.GetConfigurationAsync(); + Assert.True(configuration.Issuer.Equals(OpenIdConfigData.AADCommonV1Config.Issuer), + $"configuration.Issuer from configurationManager was not as expected," + + $"configuration.Issuer: '{configuration.Issuer}' != expected '{OpenIdConfigData.AADCommonV1Config.Issuer}'."); + } + [Fact] public async Task VerifyInterlockGuardForGetConfigurationAsync() { @@ -243,14 +261,11 @@ 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. @@ -262,7 +277,7 @@ public async Task VerifyInterlockGuardForGetConfigurationAsync() waitEvent.Reset(); signalEvent.Reset(); - TestUtilities.SetField(configurationManager, "_syncAfter", DateTime.MinValue); + TestUtilities.SetField(configurationManager, "_syncAfter", DateTimeOffset.MinValue); await configurationManager.GetConfigurationAsync(CancellationToken.None); // InMemoryDocumentRetrieverWithEvents will signal when it is OK to change the MetadataAddress @@ -278,9 +293,6 @@ 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}'."); @@ -294,15 +306,12 @@ 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 @@ -313,8 +322,8 @@ public async Task BootstrapRefreshIntervalTest() { // _syncAfter should not have been changed, because the fetch failed. var syncAfter = TestUtilities.GetField(configManager, "_syncAfter"); - if ((DateTime)syncAfter != DateTime.MinValue) - context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal '{DateTime.MinValue}'."); + if ((DateTimeOffset)syncAfter != DateTimeOffset.MinValue) + context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal '{DateTimeOffset.MinValue}'."); if (firstFetchMetadataFailure.InnerException == null) context.AddDiff($"Expected exception to contain inner exception for fetch metadata failure."); @@ -332,61 +341,8 @@ public async Task BootstrapRefreshIntervalTest() // _syncAfter should not have been changed, because the fetch failed. syncAfter = TestUtilities.GetField(configManager, "_syncAfter"); - if ((DateTime)syncAfter != DateTime.MinValue) - context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal '{DateTime.MinValue}'."); - - 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."); + if ((DateTimeOffset)syncAfter != DateTimeOffset.MinValue) + context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal '{DateTimeOffset.MinValue}'."); IdentityComparer.AreEqual(firstFetchMetadataFailure, secondFetchMetadataFailure, context); } @@ -398,8 +354,6 @@ public async Task BootstrapRefreshIntervalTest_Blocking() [Fact] public void GetSets() { - AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); - TestUtilities.WriteHeader($"{this}.GetSets", "GetSets", true); int ExpectedPropertyCount = 7; @@ -433,21 +387,10 @@ 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); + + AutoResetEvent resetEvent = SetupResetEvent(theoryData.ConfigurationManager); try { @@ -455,14 +398,13 @@ private async Task AutomaticRefreshIntervalBody(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), @@ -521,9 +442,7 @@ public static TheoryData( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever) - { - }, + InMemoryDocumentRetriever), ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV1Config, SyncAfter = DateTime.UtcNow + TimeSpan.FromDays(2), @@ -536,13 +455,12 @@ public static TheoryData( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever) - { - }, + InMemoryDocumentRetriever), ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV2Config, SyncAfter = DateTime.UtcNow, - UpdatedMetadataAddress = "AADCommonV2Json" + UpdatedMetadataAddress = "AADCommonV2Json", + WaitForEvent = true }); return theoryData; @@ -552,34 +470,23 @@ public static TheoryData theoryData) { - await RequestRefreshBody(theoryData); - } + var context = new CompareContext($"{this}.RequestRefresh"); + AutoResetEvent resetEvent = SetupResetEvent(theoryData.ConfigurationManager); - [Theory, MemberData(nameof(RequestRefreshTestCases), DisableDiscoveryEnumeration = true)] - public async Task RequestRefresh_Blocking(ConfigurationManagerTheoryData theoryData) - { - AppContext.SetSwitch(AppContextSwitches.UpdateConfigAsBlockingSwitch, true); - await RequestRefreshBody(theoryData, true); - } + var timeProvider = new FakeTimeProvider(); + theoryData.ConfigurationManager.TimeProvider = timeProvider; - 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) + + if (theoryData.WaitForEvent) WaitOrFail(resetEvent); configuration = await theoryData.ConfigurationManager.GetConfigurationAsync(CancellationToken.None); @@ -592,13 +499,11 @@ private async Task RequestRefreshBody(ConfigurationManagerTheoryData>(); - var cts = new CancellationTokenSource(); + // RefreshInterval set to 1 sec should return new config. 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), RequestRefresh = true, SleepTimeInMs = 1000, - UpdatedMetadataAddress = "AADCommonV2Json" + UpdatedMetadataAddress = "AADCommonV2Json", + WaitForEvent = true, }); - cts = new CancellationTokenSource(); + // RefreshInterval set to TimeSpan.MaxValue should return same config. theoryData.Add(new ConfigurationManagerTheoryData("RequestRefresh_TimeSpan_MaxValue") { ConfigurationManager = new ConfigurationManager( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever) - { - }, + InMemoryDocumentRetriever), ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV1Config, RefreshInterval = TimeSpan.MaxValue, @@ -644,19 +546,18 @@ public static TheoryData("RequestRefresh_FirstRefresh") { ConfigurationManager = new ConfigurationManager( "AADCommonV1Json", new OpenIdConnectConfigurationRetriever(), - InMemoryDocumentRetriever) - { - }, + InMemoryDocumentRetriever), ExpectedConfiguration = OpenIdConfigData.AADCommonV1Config, ExpectedUpdatedConfiguration = OpenIdConfigData.AADCommonV2Config, - SleepTimeInMs = 1000, - UpdatedMetadataAddress = "AADCommonV2Json" + SleepTimeInMs = 100, + UpdatedMetadataAddress = "AADCommonV2Json", + WaitForEvent = true }); return theoryData; @@ -671,18 +572,12 @@ public async Task HttpFailures(ConfigurationManagerTheoryData( - "OpenIdConnectMetadata.json", - new OpenIdConnectConfigurationRetriever(), - docRetriever); + var configManager = new ConfigurationManager("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever); + AutoResetEvent resetEvent = SetupResetEvent(configManager); - AutoResetEvent resetEvent = SetupResetEvent(configManager, blocking); + var timeProvider = new FakeTimeProvider(); + configManager.TimeProvider = timeProvider; // This is the minimum time that should pass before an automatic refresh occurs // stored in advance to avoid any time drift issues. - DateTimeOffset minimumRefreshInterval = DateTimeOffset.UtcNow + configManager.AutomaticRefreshInterval; + DateTimeOffset minimumRefreshInterval = timeProvider.GetUtcNow() + configManager.AutomaticRefreshInterval; // get the first configuration, internal _syncAfter should be set to a time greater than UtcNow + AutomaticRefreshInterval. var configuration = await configManager.GetConfigurationAsync(CancellationToken.None); // force a refresh by setting internal field - TestUtilities.SetField(configManager, "_syncAfter", DateTime.UtcNow.Subtract(TimeSpan.FromHours(1))); + timeProvider.Advance(TimeSpan.FromDays(1)); configuration = await configManager.GetConfigurationAsync(CancellationToken.None); - if (!blocking) - WaitOrFail(resetEvent); + // wait 1000ms here because update of config is run as a new task. + WaitOrFail(resetEvent); // check that _syncAfter is greater than DateTimeOffset.UtcNow + AutomaticRefreshInterval - DateTime syncAfter = (DateTime)TestUtilities.GetField(configManager, "_syncAfter"); + DateTimeOffset syncAfter = (DateTimeOffset)TestUtilities.GetField(configManager, "_syncAfter"); if (syncAfter < minimumRefreshInterval) context.Diffs.Add($"(AutomaticRefreshInterval) syncAfter '{syncAfter}' < DateTimeOffset.UtcNow + configManager.AutomaticRefreshInterval: '{minimumRefreshInterval}'."); // make same check for RequestRefresh // force a refresh by setting internal field - TestUtilities.SetField(configManager, "_lastRequestRefresh", DateTime.UtcNow.Subtract(TimeSpan.FromHours(1))); - + TestUtilities.SetField(configManager, "_lastRequestRefresh", DateTimeOffset.UtcNow - TimeSpan.FromHours(1)); configManager.RequestRefresh(); - 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(); - - 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); + // wait 1000ms here because update of config is run as a new task. + WaitOrFail(resetEvent); // check that _syncAfter is greater than DateTimeOffset.UtcNow + AutomaticRefreshInterval - syncAfter = (DateTime)TestUtilities.GetField(configManager, "_syncAfter"); + syncAfter = (DateTimeOffset)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() { - 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); - + var configManager = new ConfigurationManager("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever); + var context = new CompareContext($"{this}.GetConfiguration"); + // 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"; + TestUtilities.SetField(configManager, "_lastRequestRefresh", DateTimeOffset.UtcNow - TimeSpan.FromHours(1)); configManager.RequestRefresh(); - - // Unable to obtain a new configuration, but _currentConfiguration is not null so it should be returned. + configManager.MetadataAddress = "http://127.0.0.1"; 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 @@ -855,18 +697,6 @@ private async Task GetConfigurationBody() // 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(); @@ -943,222 +773,28 @@ public void TestConfigurationComparer() TestUtilities.AssertFailIfErrors(context); } - [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); - - 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. - if (object.ReferenceEquals(configuration, configAfterFirstRefresh)) - context.Diffs.Add("object.ReferenceEquals(configuration, configAfterFirstRefresh)"); - - configManager.RequestRefresh(); - - var configAfterNoTimePassed = await configManager.GetConfigurationAsync(CancellationToken.None); - - // Second RequestRefresh should not trigger a refresh because the refresh interval has not passed. - if (!object.ReferenceEquals(configAfterFirstRefresh, configAfterNoTimePassed)) - context.Diffs.Add("!object.ReferenceEquals(configAfterFirstRefresh, configAfterNoTimePassed)"); - - // Advance time to trigger a refresh. - timeProvider.Advance(configManager.RefreshInterval); - - 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. - if (object.ReferenceEquals(configAfterNoTimePassed, configAfterRefreshInterval)) - context.Diffs.Add("object.ReferenceEquals(configAfterNoTimePassed, configAfterRefreshInterval)"); - - // 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. - if (!object.ReferenceEquals(configAfterRefreshInterval, configAfterLessThanRefreshInterval)) - context.Diffs.Add("object.ReferenceEquals(configAfterRefreshInterval, configAfterLessThanRefreshInterval)"); - - // 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)) - 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 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); - - var configNoAdvanceInTime = await configManager.GetConfigurationAsync(CancellationToken.None); - - // First GetConfigurationAsync should not trigger a refresh because the refresh interval has not passed. - if (!object.ReferenceEquals(configuration, configNoAdvanceInTime)) - context.Diffs.Add("!object.ReferenceEquals(configuration, configNoAdvanceInTime)"); - - // Advance time to trigger a refresh. - timeProvider.Advance(advanceInterval); - - var configAfterTimeIsAdvanced = await configManager.GetConfigurationAsync(CancellationToken.None); - - 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. - WaitOrFail(resetEvent); - - 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)"); - } - 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 resetEvent = SetupResetEvent(configurationManager, blocking); + var configurationManager = new ConfigurationManager(theoryData.MetadataAddress, theoryData.ConfigurationRetreiver, theoryData.DocumentRetriever, theoryData.ConfigurationValidator); if (theoryData.PresetCurrentConfiguration) TestUtilities.SetField(configurationManager, "_currentConfiguration", new OpenIdConnectConfiguration() { Issuer = Default.Issuer }); + AutoResetEvent resetEvent = SetupResetEvent(configurationManager); + try { //create a listener and enable it for logs - using var listener = TestUtils.SampleListener.CreateLoggerListener(EventLevel.Warning); - + var listener = TestUtils.SampleListener.CreateLoggerListener(EventLevel.Warning); configuration = await configurationManager.GetConfigurationAsync(); - if (!blocking && theoryData.ExpectedException is null && string.IsNullOrEmpty(theoryData.ExpectedErrorMessage)) + if (theoryData.WaitForEvent) 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}'"); @@ -1171,36 +807,10 @@ private async Task ValidateOIDCConfigurationBody(ConfigurationManagerTheoryData< 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 @@ -1211,96 +821,104 @@ public static TheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator, DocumentRetriever = new FileDocumentRetriever(), First = true, MetadataAddress = "OpenIdConnectMetadata.json", - TestId = "ValidConfiguration", + TestId = "ValidConfiguration" }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator2, DocumentRetriever = new FileDocumentRetriever(), ExpectedException = new ExpectedException(typeof(InvalidOperationException), "IDX21818:", typeof(InvalidConfigurationException)), MetadataAddress = "OpenIdConnectMetadata.json", TestId = "ValidConfiguration_NotEnoughKey", + WaitForEvent = true }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator2, DocumentRetriever = new FileDocumentRetriever(), PresetCurrentConfiguration = true, ExpectedErrorMessage = "IDX21818: ", MetadataAddress = "OpenIdConnectMetadata.json", TestId = "ValidConfiguration_NotEnoughKey_PresetCurrentConfiguration", + WaitForEvent = true }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator2, DocumentRetriever = new FileDocumentRetriever(), ExpectedException = new ExpectedException(typeof(InvalidOperationException), "IDX10810:", typeof(InvalidConfigurationException)), MetadataAddress = "OpenIdConnectMetadataUnrecognizedKty.json", TestId = "InvalidConfiguration_UnrecognizedKty", + WaitForEvent = true }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator2, DocumentRetriever = new FileDocumentRetriever(), PresetCurrentConfiguration = true, ExpectedErrorMessage = "IDX10810: ", MetadataAddress = "OpenIdConnectMetadataUnrecognizedKty.json", TestId = "InvalidConfiguration_UnrecognizedKty_PresetCurrentConfiguration", + WaitForEvent = true }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator2, DocumentRetriever = new FileDocumentRetriever(), ExpectedException = new ExpectedException(typeof(InvalidOperationException), "IDX21817:", typeof(InvalidConfigurationException)), MetadataAddress = "JsonWebKeySetUnrecognizedKty.json", TestId = "InvalidConfiguration_EmptyJsonWenKeySet", + WaitForEvent = true }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator2, DocumentRetriever = new FileDocumentRetriever(), PresetCurrentConfiguration = true, ExpectedErrorMessage = "IDX21817: ", MetadataAddress = "JsonWebKeySetUnrecognizedKty.json", TestId = "InvalidConfiguration_EmptyJsonWenKeySet_PresetCurrentConfiguration", + WaitForEvent = true }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator2, DocumentRetriever = new FileDocumentRetriever(), ExpectedException = new ExpectedException(typeof(InvalidOperationException), "IDX10814:", typeof(InvalidConfigurationException)), MetadataAddress = "OpenIdConnectMetadataBadRsaDataMissingComponent.json", TestId = "InvalidConfiguration_RsaKeyMissingComponent", + WaitForEvent = true }); theoryData.Add(new ConfigurationManagerTheoryData { - ConfigurationRetriever = new OpenIdConnectConfigurationRetriever(), + ConfigurationRetreiver = new OpenIdConnectConfigurationRetriever(), ConfigurationValidator = openIdConnectConfigurationValidator2, DocumentRetriever = new FileDocumentRetriever(), PresetCurrentConfiguration = true, ExpectedErrorMessage = "IDX10814: ", MetadataAddress = "OpenIdConnectMetadataBadRsaDataMissingComponent.json", TestId = "InvalidConfiguration_RsaKeyMissingComponent_PresetCurrentConfiguration", + WaitForEvent = true }); return theoryData; @@ -1330,6 +948,16 @@ private static InMemoryDocumentRetriever InMemoryDocumentRetrieverWithEvents(Man signalEvent); } + internal static AutoResetEvent SetupResetEvent(ConfigurationManager configurationManager) + { + var resetEvent = new AutoResetEvent(false); + + Action _waitAction = () => resetEvent.Set(); + configurationManager._onBackgroundTaskFinish = _waitAction; + + return resetEvent; + } + internal static void WaitOrFail(AutoResetEvent are) { if (!are.WaitOne(30000)) @@ -1346,7 +974,7 @@ public ConfigurationManagerTheoryData(string testId) : base(testId) { } public TimeSpan AutomaticRefreshInterval { get; set; } - public IConfigurationRetriever ConfigurationRetriever { get; set; } + public IConfigurationRetriever ConfigurationRetreiver { get; set; } public IConfigurationValidator ConfigurationValidator { get; set; } @@ -1374,6 +1002,8 @@ public ConfigurationManagerTheoryData(string testId) : base(testId) { } public DateTimeOffset SyncAfter { get; set; } = DateTime.UtcNow; + public bool WaitForEvent { get; set; } + public override string ToString() { return $"{TestId}, {MetadataAddress}, {ExpectedException}"; diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests.csproj b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests.csproj index aa78636b2d..74c2cf9312 100644 --- a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests.csproj +++ b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests.csproj @@ -11,12 +11,6 @@ true - - - true - - PreserveNewest @@ -34,7 +28,6 @@ - diff --git a/test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs b/test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs index 93216034c2..c37f5d04ce 100644 --- a/test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs +++ b/test/Microsoft.IdentityModel.Protocols.Tests/ExtensibilityTests.cs @@ -77,14 +77,11 @@ 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))); - + TestUtilities.SetField(configManager, "_lastRequestRefresh", DateTimeOffset.UtcNow - TimeSpan.FromHours(1)); configManager.MetadataAddress = "IssuerMetadata2.json"; // Wait for the refresh to complete. @@ -104,8 +101,6 @@ 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/Microsoft.IdentityModel.TestUtils.csproj b/test/Microsoft.IdentityModel.TestUtils/Microsoft.IdentityModel.TestUtils.csproj index 8ac6f21ecc..ebcc1d0e00 100644 --- a/test/Microsoft.IdentityModel.TestUtils/Microsoft.IdentityModel.TestUtils.csproj +++ b/test/Microsoft.IdentityModel.TestUtils/Microsoft.IdentityModel.TestUtils.csproj @@ -1,4 +1,4 @@ - + @@ -16,6 +16,7 @@ + diff --git a/test/Microsoft.IdentityModel.TestUtils/ResetAppContextSwitchesAttribute.cs b/test/Microsoft.IdentityModel.TestUtils/ResetAppContextSwitchesAttribute.cs deleted file mode 100644 index 57e1d92231..0000000000 --- a/test/Microsoft.IdentityModel.TestUtils/ResetAppContextSwitchesAttribute.cs +++ /dev/null @@ -1,23 +0,0 @@ -// 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 fa3d7a4134..87c1fd67f5 100644 --- a/test/Microsoft.IdentityModel.Tokens.Tests/Telemetry/MockTelemetryClient.cs +++ b/test/Microsoft.IdentityModel.Tokens.Tests/Telemetry/MockTelemetryClient.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Threading; using Microsoft.IdentityModel.Logging; namespace Microsoft.IdentityModel.Telemetry.Tests @@ -12,6 +13,10 @@ public class MockTelemetryClient : ITelemetryClient public Dictionary ExportedItems = new Dictionary(); public Dictionary ExportedHistogramItems = new Dictionary(); + internal int _requestRefreshCounter; + + public int RequestRefreshCounter => _requestRefreshCounter; + public void ClearExportedItems() { ExportedItems.Clear(); @@ -20,6 +25,7 @@ public void ClearExportedItems() public void IncrementConfigurationRefreshRequestCounter(string metadataAddress, string operationStatus) { + Interlocked.Increment(ref _requestRefreshCounter); ExportedItems.Add(TelemetryConstants.IdentityModelVersionTag, IdentityModelTelemetryUtil.ClientVer); ExportedItems.Add(TelemetryConstants.MetadataAddressTag, metadataAddress); ExportedItems.Add(TelemetryConstants.OperationStatusTag, operationStatus); @@ -27,6 +33,7 @@ public void IncrementConfigurationRefreshRequestCounter(string metadataAddress, public void IncrementConfigurationRefreshRequestCounter(string metadataAddress, string operationStatus, Exception exception) { + Interlocked.Increment(ref _requestRefreshCounter); ExportedItems.Add(TelemetryConstants.IdentityModelVersionTag, IdentityModelTelemetryUtil.ClientVer); ExportedItems.Add(TelemetryConstants.MetadataAddressTag, metadataAddress); ExportedItems.Add(TelemetryConstants.OperationStatusTag, operationStatus);