diff --git a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs index ed3d4b7e67..8e37d60ac2 100644 --- a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs +++ b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs @@ -35,6 +35,10 @@ public class ConfigurationManager : BaseConfigurationManager, IConfigurationM private const int ConfigurationRetrieverRunning = 1; private int _configurationRetrieverState = ConfigurationRetrieverIdle; + // If a refresh is requested, then do the refresh as a blocking operation + // not on a background thread. + bool _refreshRequested; + /// /// Instantiates a new that manages automatic and controls refreshing on configuration data. /// @@ -214,7 +218,13 @@ public virtual async Task GetConfigurationAsync(CancellationToken cancel) { if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverRunning, ConfigurationRetrieverIdle) == ConfigurationRetrieverIdle) { - _ = Task.Run(UpdateCurrentConfiguration, CancellationToken.None); + if (_refreshRequested) + { + UpdateCurrentConfiguration(); + _refreshRequested = false; + } + else + _ = Task.Run(UpdateCurrentConfiguration, CancellationToken.None); } } @@ -310,15 +320,11 @@ public override async Task GetBaseConfigurationAsync(Cancella public override void RequestRefresh() { DateTimeOffset now = DateTimeOffset.UtcNow; - if (now >= DateTimeUtil.Add(_lastRequestRefresh.UtcDateTime, RefreshInterval) || _isFirstRefreshRequest) { _isFirstRefreshRequest = false; - if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverRunning, ConfigurationRetrieverIdle) == ConfigurationRetrieverIdle) - { - UpdateCurrentConfiguration(); - _lastRequestRefresh = now; - } + _syncAfter = now; + _refreshRequested = true; } } diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs index 2fba523106..f7bba705bc 100644 --- a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs +++ b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs @@ -209,48 +209,6 @@ public async Task FetchMetadataFailureTest() 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(); - _ = Task.Run(() => 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); - _ = Task.Run(() => 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() { @@ -320,13 +278,15 @@ public async Task BootstrapRefreshIntervalTest() catch (Exception firstFetchMetadataFailure) { // _syncAfter should not have been changed, because the fetch failed. - var syncAfter = TestUtilities.GetField(configManager, "_syncAfter"); - if ((DateTimeOffset)syncAfter != DateTimeOffset.MinValue) + DateTimeOffset syncAfter = (DateTimeOffset)TestUtilities.GetField(configManager, "_syncAfter"); + if (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."); + DateTime requestTime = DateTime.UtcNow; + // Fetch metadata again during refresh interval, the exception should be same from above. try { @@ -339,9 +299,10 @@ 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 = TestUtilities.GetField(configManager, "_syncAfter"); - if ((DateTimeOffset)syncAfter != DateTimeOffset.MinValue) - context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter}' should equal '{DateTimeOffset.MinValue}'."); + syncAfter = (DateTimeOffset)TestUtilities.GetField(configManager, "_syncAfter"); + + if (!IdentityComparer.AreDatesEqualWithEpsilon(requestTime, syncAfter.UtcDateTime, 1)) + context.AddDiff($"ConfigurationManager._syncAfter: '{syncAfter.UtcDateTime}' should equal be within 1 second of '{requestTime}'."); IdentityComparer.AreEqual(firstFetchMetadataFailure, secondFetchMetadataFailure, context); } @@ -605,7 +566,7 @@ public static TheoryData