diff --git a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs index 721034ef4c..ab5f2eb040 100644 --- a/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs +++ b/src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs @@ -22,6 +22,7 @@ public class ConfigurationManager : BaseConfigurationManager, IConfigurationM private DateTimeOffset _lastRefresh = DateTimeOffset.MinValue; private bool _isFirstRefreshRequest = true; private bool _skipDistributedConfigurationManager = false; + private bool _distributedCacheUpdateInProgress = false; private readonly SemaphoreSlim _refreshLock; private readonly IDocumentRetriever _docRetriever; @@ -166,26 +167,33 @@ public async Task GetConfigurationAsync(CancellationToken cancel) T configuration = null; if (DistributedConfigurationManager != null && !_skipDistributedConfigurationManager) { - // TODO handle try/catch - configuration = await DistributedConfigurationManager.GetConfigurationAsync(MetadataAddress, s_distributedConfigurationOptions, CancellationToken.None).ConfigureAwait(false); - if (_configValidator != null) + try { - // TODO don't throw but log another exception - ConfigurationValidationResult result = _configValidator.Validate(configuration); - if (!result.Succeeded) + configuration = await DistributedConfigurationManager.GetConfigurationAsync(MetadataAddress, s_distributedConfigurationOptions, CancellationToken.None).ConfigureAwait(false); + if (_configValidator != null) { - configuration = null; - LogHelper.LogExceptionMessage(new InvalidConfigurationException(LogHelper.FormatInvariant(LogMessages.IDX20810, result.ErrorMessage))); + ConfigurationValidationResult result = _configValidator.Validate(configuration); + if (!result.Succeeded) + { + configuration = null; + LogHelper.LogExceptionMessage(new InvalidConfigurationException(LogHelper.FormatInvariant(LogMessages.IDX20812, result.ErrorMessage))); + } } } + catch (Exception ex) + { + LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(LogMessages.IDX20811), ex)); + } } if (configuration == null) { + LogHelper.LogInformation(LogMessages.IDX20811); + // 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.. configuration = await _configRetriever.GetConfigurationAsync(MetadataAddress, _docRetriever, CancellationToken.None).ConfigureAwait(false); - if (configuration is IConfigurationRetrievalTime configRetrievalTime) + if (configuration is IConfigurationTimeRetriever configRetrievalTime) configRetrievalTime.RetrievalTime = DateTimeOffset.UtcNow; if (_configValidator != null) @@ -198,11 +206,16 @@ public async Task GetConfigurationAsync(CancellationToken cancel) if (_skipDistributedConfigurationManager) _skipDistributedConfigurationManager = false; - if (DistributedConfigurationManager != null) - // TODO fire and forget (or not if refresh happens on a background thread) - await DistributedConfigurationManager.SetConfigurationAsync(MetadataAddress, configuration, s_distributedConfigurationOptions, CancellationToken.None).ConfigureAwait(false); + if (DistributedConfigurationManager != null && !_distributedCacheUpdateInProgress) + { + _distributedCacheUpdateInProgress = true; + _ = DistributedConfigurationManager.SetConfigurationAsync(MetadataAddress, configuration, s_distributedConfigurationOptions, CancellationToken.None).ContinueWith(t => + { + _distributedCacheUpdateInProgress = false; + }, TaskScheduler.Default); + } - if (configuration is IConfigurationRetrievalTime configurationRetrievalTime) + if (configuration is IConfigurationTimeRetriever configurationRetrievalTime) _lastRefresh = configurationRetrievalTime.RetrievalTime; else _lastRefresh = DateTimeOffset.UtcNow; diff --git a/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs b/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs index 901dc16ac8..2d1d69b451 100644 --- a/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs +++ b/src/Microsoft.IdentityModel.Protocols/GlobalSuppressions.cs @@ -9,6 +9,7 @@ [assembly: SuppressMessage("Performance", "CA1819:Properties should not return arrays", Justification = "Previously released as returning an array", Scope = "member", Target = "~P:Microsoft.IdentityModel.Protocols.HttpRequestData.Body")] [assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "Previously released read/write", Scope = "member", Target = "~P:Microsoft.IdentityModel.Protocols.HttpRequestData.Headers")] [assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "Previously released read/write", Scope = "member", Target = "~P:Microsoft.IdentityModel.Protocols.HttpRequestData.PropertyBag")] +[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Any exception type can be thrown by custom distributed configuration manager.", Scope = "member", Target = "~M:Microsoft.IdentityModel.Protocols.ConfigurationManager`1.GetConfigurationAsync(System.Threading.CancellationToken)~System.Threading.Tasks.Task{`0}")] #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 diff --git a/src/Microsoft.IdentityModel.Protocols/LogMessages.cs b/src/Microsoft.IdentityModel.Protocols/LogMessages.cs index aef3aaa132..0eda8bfb42 100644 --- a/src/Microsoft.IdentityModel.Protocols/LogMessages.cs +++ b/src/Microsoft.IdentityModel.Protocols/LogMessages.cs @@ -29,6 +29,8 @@ internal static class LogMessages internal const string IDX20808 = "IDX20808: Network error occurred. Status code: '{0}'. \nResponse content: '{1}'. \nAttempting to retrieve document again from: '{2}'."; internal const string IDX20809 = "IDX20809: Unable to retrieve document from: '{0}'. Status code: '{1}'. \nResponse content: '{2}'."; internal const string IDX20810 = "IDX20810: Configuration validation failed, see inner exception for more details. Exception: '{0}'."; + internal const string IDX20811 = "IDX20811: Unable to obtain configuration from distributed cache."; + internal const string IDX20812 = "IDX20812: Configuration retrieved from distributed cache validation failed, see inner exception for more details. Exception: '{0}'."; #pragma warning restore 1591 } diff --git a/src/Microsoft.IdentityModel.Tokens/BaseConfiguration.cs b/src/Microsoft.IdentityModel.Tokens/BaseConfiguration.cs index 91dce52b34..7450daac27 100644 --- a/src/Microsoft.IdentityModel.Tokens/BaseConfiguration.cs +++ b/src/Microsoft.IdentityModel.Tokens/BaseConfiguration.cs @@ -11,7 +11,7 @@ namespace Microsoft.IdentityModel.Tokens /// /// Represents a generic metadata configuration which is applicable for both XML and JSON based configurations. /// - public abstract class BaseConfiguration /*: IConfigurationRetrievalTime*/ // L2 TODO: internal until L2 cache is implemented S2S. + public abstract class BaseConfiguration /*: IConfigurationTimeRetriever*/ // L2 TODO: internal until L2 cache is implemented S2S. { /// /// Gets the issuer specified via the metadata endpoint. diff --git a/src/Microsoft.IdentityModel.Tokens/IConfigurationRetrievalTime.cs b/src/Microsoft.IdentityModel.Tokens/IConfigurationTimeRetriever.cs similarity index 88% rename from src/Microsoft.IdentityModel.Tokens/IConfigurationRetrievalTime.cs rename to src/Microsoft.IdentityModel.Tokens/IConfigurationTimeRetriever.cs index ec160015b8..2fbff5db45 100644 --- a/src/Microsoft.IdentityModel.Tokens/IConfigurationRetrievalTime.cs +++ b/src/Microsoft.IdentityModel.Tokens/IConfigurationTimeRetriever.cs @@ -9,7 +9,7 @@ namespace Microsoft.IdentityModel.Tokens /// /// // L2 TODO: internal until L2 cache is implemented S2S. - internal interface IConfigurationRetrievalTime + internal interface IConfigurationTimeRetriever { // L2 TODO: internal until L2 cache is implemented S2S. internal DateTimeOffset RetrievalTime { get; set; }