From b0b4f5b88c0ed90ba9511416aa8a9aecc014e8f1 Mon Sep 17 00:00:00 2001
From: BrentSchmaltz <brentschmaltz@hotmail.com>
Date: Thu, 3 Oct 2024 11:19:34 -0700
Subject: [PATCH] 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>
---
 .../Configuration/ConfigurationManager.cs     |  2 +-
 .../ConfigurationManagerTests.cs              | 43 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs
index 3ab4c16aef..f747a37840 100644
--- 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
index 7534937345..d64a13649c 100644
--- 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";