Set internal _syncAfter using only AutomaticRefreshInterval. (#2865)

* Set next automatic refresh time only considering AutomaticRefreshInterval. * Increase timeout as test is failing --------- Co-authored-by: id4s <user@contoso.com>
AzureAD · Oct 3, 2024 · b0b4f5b · b0b4f5b
1 parent 50af59d
commit b0b4f5b
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 1 deletion.
diff --git a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs
@@ -313,7 +313,7 @@ private void UpdateCurrentConfiguration()
             }
             finally
             {
-                _syncAfter = DateTimeUtil.Add(DateTime.UtcNow, AutomaticRefreshInterval < RefreshInterval ? AutomaticRefreshInterval : RefreshInterval);
+                _syncAfter = DateTimeUtil.Add(DateTime.UtcNow, AutomaticRefreshInterval);
                 Interlocked.Exchange(ref _configurationRetrieverState, ConfigurationRetrieverIdle);
             }
 #pragma warning restore CA1031 // Do not catch general exception types

diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs
@@ -510,6 +510,48 @@ public static TheoryData<ConfigurationManagerTheoryData<OpenIdConnectConfigurati
             }
         }
 
+        [Fact]
+        public async Task CheckSyncAfter()
+        {
+            // This test checks that the _syncAfter field is set correctly after a refresh.
+            var context = new CompareContext($"{this}.CheckSyncAfter");
+
+            var docRetriever = new FileDocumentRetriever();
+            var configManager = new ConfigurationManager<OpenIdConnectConfiguration>("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever);
+
+            // 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;
+
+            // 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", DateTimeOffset.UtcNow - TimeSpan.FromHours(1));
+            configuration = await configManager.GetConfigurationAsync(CancellationToken.None);
+            // wait 1000ms here because update of config is run as a new task.
+            Thread.Sleep(1000);
+
+            // check that _syncAfter is greater than DateTimeOffset.UtcNow + AutomaticRefreshInterval
+            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", DateTimeOffset.UtcNow - TimeSpan.FromHours(1));
+            configManager.RequestRefresh();
+            // wait 1000ms here because update of config is run as a new task.
+            Thread.Sleep(1000);
+
+            // check that _syncAfter is greater than DateTimeOffset.UtcNow + AutomaticRefreshInterval
+            syncAfter = (DateTimeOffset)TestUtilities.GetField(configManager, "_syncAfter");
+            if (syncAfter < minimumRefreshInterval)
+                context.Diffs.Add($"(RequestRefresh) syncAfter '{syncAfter}' < DateTimeOffset.UtcNow + configManager.AutomaticRefreshInterval: '{minimumRefreshInterval}'.");
+
+            TestUtilities.AssertFailIfErrors(context);
+        }
+
         [Fact]
         public async Task GetConfigurationAsync()
         {
@@ -520,6 +562,7 @@ public async Task GetConfigurationAsync()
             // Unable to obtain a new configuration, but _currentConfiguration is not null so it should be returned.
             configManager = new ConfigurationManager<OpenIdConnectConfiguration>("OpenIdConnectMetadata.json", new OpenIdConnectConfigurationRetriever(), docRetriever);
             var configuration = await configManager.GetConfigurationAsync(CancellationToken.None);
+
             TestUtilities.SetField(configManager, "_lastRequestRefresh", DateTimeOffset.UtcNow - TimeSpan.FromHours(1));
             configManager.RequestRefresh();
             configManager.MetadataAddress = "http://127.0.0.1";