Skip to content

Commit

Permalink
Revert "Remove SlimLock when updating metadata. (#2751)" (#2762)
Browse files Browse the repository at this point in the history 
This reverts commit bbc09a4.

Co-authored-by: Keegan Caruso <keegancaruso@microsoft.com>
  • Loading branch information
@keegan-caruso
keegan-caruso and Keegan Caruso authored Aug 5, 2024
1 parent bbc09a4 commit 23a0b12
Show file tree
Hide file tree
Showing 9 changed files with 225 additions and 684 deletions.
13 changes: 2 additions & 11 deletions ...dentityModel.Protocols.OpenIdConnect/Configuration/OpenIdConnectConfigurationValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,11 @@ public ConfigurationValidationResult Validate(OpenIdConnectConfiguration openIdC
Succeeded = false
};
}

int numberOfValidKeys = 0;
for (int i = 0; i < openIdConnectConfiguration.JsonWebKeySet.Keys.Count; i++)
if (openIdConnectConfiguration.JsonWebKeySet.Keys[i].ConvertedSecurityKey != null)
numberOfValidKeys++;
var numberOfValidKeys = openIdConnectConfiguration.JsonWebKeySet.Keys.Where(key => key.ConvertedSecurityKey != null).Count();

if (numberOfValidKeys < MinimumNumberOfKeys)
{
string convertKeyInfos = string.Join(
"\n",
openIdConnectConfiguration.JsonWebKeySet.Keys.Where(
key => !string.IsNullOrEmpty(key.ConvertKeyInfo))
.Select(key => key.Kid.ToString() + ": " + key.ConvertKeyInfo));

var convertKeyInfos = string.Join("\n", openIdConnectConfiguration.JsonWebKeySet.Keys.Where(key => !string.IsNullOrEmpty(key.ConvertKeyInfo)).Select(key => key.Kid.ToString() + ": " + key.ConvertKeyInfo));
return new ConfigurationValidationResult
{
ErrorMessage = LogHelper.FormatInvariant(
Expand Down
212 changes: 72 additions & 140 deletions src/Microsoft.IdentityModel.Protocols/Configuration/ConfigurationManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System;
using System.Diagnostics.Contracts;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -19,22 +20,17 @@ namespace Microsoft.IdentityModel.Protocols
public class ConfigurationManager<T> : BaseConfigurationManager, IConfigurationManager<T> where T : class
{
private DateTimeOffset _syncAfter = DateTimeOffset.MinValue;
private DateTimeOffset _lastRequestRefresh = DateTimeOffset.MinValue;
private DateTimeOffset _lastRefresh = DateTimeOffset.MinValue;
private bool _isFirstRefreshRequest = true;

private readonly SemaphoreSlim _refreshLock;
private readonly IDocumentRetriever _docRetriever;
private readonly IConfigurationRetriever<T> _configRetriever;
private readonly IConfigurationValidator<T> _configValidator;
private T _currentConfiguration;
private Exception _fetchMetadataFailure;
private TimeSpan _bootstrapRefreshInterval = TimeSpan.FromSeconds(1);

// task states are used to ensure the call to 'update config' (UpdateCurrentConfiguration) is a singleton. Uses Interlocked.CompareExchange.
// metadata is not being obtained
private const int ConfigurationRetrieverIdle = 0;
// metadata is being retrieved
private const int ConfigurationRetrieverRunning = 1;
private int _configurationRetrieverState = ConfigurationRetrieverIdle;

/// <summary>
/// Instantiates a new <see cref="ConfigurationManager{T}"/> that manages automatic and controls refreshing on configuration data.
/// </summary>
Expand Down Expand Up @@ -96,6 +92,7 @@ public ConfigurationManager(string metadataAddress, IConfigurationRetriever<T> c
MetadataAddress = metadataAddress;
_docRetriever = docRetriever;
_configRetriever = configRetriever;
_refreshLock = new SemaphoreSlim(1);
}

/// <summary>
Expand Down Expand Up @@ -148,149 +145,83 @@ public async Task<T> GetConfigurationAsync()
public virtual async Task<T> GetConfigurationAsync(CancellationToken cancel)
{
if (_currentConfiguration != null && _syncAfter > DateTimeOffset.UtcNow)
{
return _currentConfiguration;
}

Exception fetchMetadataFailure = null;

// LOGIC
// if configuration != null => configuration has been retrieved before
// reach out to the metadata endpoint
// else
// if task is running, return the current configuration
// else kick off task to update current configuration
if (_currentConfiguration == null)
await _refreshLock.WaitAsync(cancel).ConfigureAwait(false);
try
{
try
if (_syncAfter <= DateTimeOffset.UtcNow)
{
// 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);
if (_configValidator != null)
try
{
ConfigurationValidationResult result = _configValidator.Validate(configuration);
// in this case we have never had a valid configuration, so we will throw an exception if the validation fails
if (!result.Succeeded)
throw LogHelper.LogExceptionMessage(new InvalidConfigurationException(LogHelper.FormatInvariant(LogMessages.IDX20810, result.ErrorMessage)));
}

// Add a random amount between 0 and 5% of AutomaticRefreshInterval jitter to avoid spike traffic to IdentityProvider.
_syncAfter = DateTimeUtil.Add(DateTime.UtcNow, AutomaticRefreshInterval +
TimeSpan.FromSeconds(new Random().Next((int)AutomaticRefreshInterval.TotalSeconds / 20)));

_currentConfiguration = configuration;
}
catch (Exception ex)
{
fetchMetadataFailure = ex;
// 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);
if (_configValidator != null)
{
ConfigurationValidationResult result = _configValidator.Validate(configuration);
if (!result.Succeeded)
throw LogHelper.LogExceptionMessage(new InvalidConfigurationException(LogHelper.FormatInvariant(LogMessages.IDX20810, result.ErrorMessage)));
}

// In this case configuration was never obtained.
if (_currentConfiguration == null)
_lastRefresh = DateTimeOffset.UtcNow;
// Add a random amount between 0 and 5% of AutomaticRefreshInterval jitter to avoid spike traffic to IdentityProvider.
_syncAfter = DateTimeUtil.Add(DateTime.UtcNow, AutomaticRefreshInterval +
TimeSpan.FromSeconds(new Random().Next((int)AutomaticRefreshInterval.TotalSeconds / 20)));
_currentConfiguration = configuration;
}
catch (Exception ex)
{
if (_bootstrapRefreshInterval < RefreshInterval)
_fetchMetadataFailure = ex;

if (_currentConfiguration == null) // Throw an exception if there's no configuration to return.
{
// 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);
}
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(DateTime.UtcNow, AutomaticRefreshInterval < RefreshInterval ? AutomaticRefreshInterval : RefreshInterval);
}

throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(LogMessages.IDX20803, LogHelper.MarkAsNonPII(MetadataAddress ?? "null"), LogHelper.MarkAsNonPII(_syncAfter), LogHelper.MarkAsNonPII(ex)), ex));
}
else
{
_syncAfter = DateTimeUtil.Add(DateTime.UtcNow, AutomaticRefreshInterval < RefreshInterval ? AutomaticRefreshInterval : RefreshInterval);
}

throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
LogMessages.IDX20803,
LogHelper.MarkAsNonPII(MetadataAddress ?? "null"),
LogHelper.MarkAsNonPII(_syncAfter),
LogHelper.MarkAsNonPII(ex)),
ex));
}
else
{
_syncAfter = DateTimeUtil.Add(DateTime.UtcNow, AutomaticRefreshInterval < RefreshInterval ? AutomaticRefreshInterval : RefreshInterval);

LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
LogMessages.IDX20806,
LogHelper.MarkAsNonPII(MetadataAddress ?? "null"),
LogHelper.MarkAsNonPII(ex)),
ex));
LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(LogMessages.IDX20806, LogHelper.MarkAsNonPII(MetadataAddress ?? "null"), LogHelper.MarkAsNonPII(ex)), ex));
}
}
}
}
else
{
if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverIdle, ConfigurationRetrieverRunning) != ConfigurationRetrieverRunning)
{
_ = Task.Run(UpdateCurrentConfiguration, CancellationToken.None);
}
}

// If metadata exists return it.
if (_currentConfiguration != null)
return _currentConfiguration;

throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
LogMessages.IDX20803,
LogHelper.MarkAsNonPII(MetadataAddress ?? "null"),
LogHelper.MarkAsNonPII(_syncAfter),
LogHelper.MarkAsNonPII(fetchMetadataFailure)),
fetchMetadataFailure));
}

/// <summary>
/// This should be called when the configuration needs to be updated either from RequestRefresh or AutomaticRefresh, first checking the state checking state using:
/// if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverIdle, ConfigurationRetrieverRunning) != ConfigurationRetrieverRunning).
/// </summary>
private void UpdateCurrentConfiguration()
{
#pragma warning disable CA1031 // Do not catch general exception types
try
{
T configuration = _configRetriever.GetConfigurationAsync(
MetadataAddress,
_docRetriever,
CancellationToken.None).ConfigureAwait(false).GetAwaiter().GetResult();

if (_configValidator == null)
{
_currentConfiguration = configuration;
}
// Stale metadata is better than no metadata
if (_currentConfiguration != null)
return _currentConfiguration;
else
{
ConfigurationValidationResult result = _configValidator.Validate(configuration);

if (!result.Succeeded)
LogHelper.LogExceptionMessage(
new InvalidConfigurationException(
LogHelper.FormatInvariant(
LogMessages.IDX20810,
result.ErrorMessage)));
else
_currentConfiguration = configuration;
}
}
catch (Exception ex)
{
LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
LogMessages.IDX20806,
LogHelper.MarkAsNonPII(MetadataAddress ?? "null"),
ex),
ex));
throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
LogMessages.IDX20803,
LogHelper.MarkAsNonPII(MetadataAddress ?? "null"),
LogHelper.MarkAsNonPII(_syncAfter),
LogHelper.MarkAsNonPII(_fetchMetadataFailure)),
_fetchMetadataFailure));
}
finally
{
_syncAfter = DateTimeUtil.Add(DateTime.UtcNow, AutomaticRefreshInterval < RefreshInterval ? AutomaticRefreshInterval : RefreshInterval);
Interlocked.Exchange(ref _configurationRetrieverState, ConfigurationRetrieverIdle);
_refreshLock.Release();
}
#pragma warning restore CA1031 // Do not catch general exception types
}

/// <summary>
Expand All @@ -301,8 +232,10 @@ private void UpdateCurrentConfiguration()
/// <remarks>If the time since the last call is less than <see cref="BaseConfigurationManager.AutomaticRefreshInterval"/> then <see cref="IConfigurationRetriever{T}.GetConfigurationAsync"/> is not called and the current Configuration is returned.</remarks>
public override async Task<BaseConfiguration> GetBaseConfigurationAsync(CancellationToken cancel)
{
T obj = await GetConfigurationAsync(cancel).ConfigureAwait(false);
return obj as BaseConfiguration;
var obj = await GetConfigurationAsync(cancel).ConfigureAwait(false);
if (obj is BaseConfiguration)
return obj as BaseConfiguration;
return null;
}

/// <summary>
Expand All @@ -313,15 +246,14 @@ public override async Task<BaseConfiguration> GetBaseConfigurationAsync(Cancella
public override void RequestRefresh()
{
DateTimeOffset now = DateTimeOffset.UtcNow;

if (now >= DateTimeUtil.Add(_lastRequestRefresh.UtcDateTime, RefreshInterval) || _isFirstRefreshRequest )
if (_isFirstRefreshRequest)
{
_syncAfter = now;
_isFirstRefreshRequest = false;
_lastRequestRefresh = now;
if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverIdle, ConfigurationRetrieverRunning) != ConfigurationRetrieverRunning)
{
_ = Task.Run(UpdateCurrentConfiguration, CancellationToken.None);
}
}
else if (now >= DateTimeUtil.Add(_lastRefresh.UtcDateTime, RefreshInterval))
{
_syncAfter = now;
}
}

Expand Down
26 changes: 5 additions & 21 deletions src/Microsoft.IdentityModel.Protocols/Configuration/HttpDocumentRetriever.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,17 @@ public HttpDocumentRetriever(HttpClient httpClient)
public async Task<string> GetDocumentAsync(string address, CancellationToken cancel)
{
if (string.IsNullOrWhiteSpace(address))
throw LogHelper.LogArgumentNullException(nameof(address));
throw LogHelper.LogArgumentNullException("address");

if (!Utility.IsHttps(address) && RequireHttps)
throw LogHelper.LogExceptionMessage(
new ArgumentException(
LogHelper.FormatInvariant(
LogMessages.IDX20108,
LogHelper.MarkAsNonPII(address)),
nameof(address)));
throw LogHelper.LogExceptionMessage(new ArgumentException(LogHelper.FormatInvariant(LogMessages.IDX20108, address), nameof(address)));

Exception unsuccessfulHttpResponseException;
HttpResponseMessage response;
try
{
if (LogHelper.IsEnabled(EventLogLevel.Verbose))
LogHelper.LogVerbose(LogMessages.IDX20805, LogHelper.MarkAsNonPII(address));
LogHelper.LogVerbose(LogMessages.IDX20805, address);

var httpClient = _httpClient ?? _defaultHttpClient;
var uri = new Uri(address, UriKind.RelativeOrAbsolute);
Expand All @@ -109,24 +104,13 @@ public async Task<string> GetDocumentAsync(string address, CancellationToken can
if (response.IsSuccessStatusCode)
return responseContent;

unsuccessfulHttpResponseException = new IOException(
LogHelper.FormatInvariant(
LogMessages.IDX20807,
LogHelper.MarkAsNonPII(address),
response,
responseContent));

unsuccessfulHttpResponseException = new IOException(LogHelper.FormatInvariant(LogMessages.IDX20807, address, response, responseContent));
unsuccessfulHttpResponseException.Data.Add(StatusCode, response.StatusCode);
unsuccessfulHttpResponseException.Data.Add(ResponseContent, responseContent);
}
catch (Exception ex)
{
throw LogHelper.LogExceptionMessage(
new IOException(
LogHelper.FormatInvariant(
LogMessages.IDX20804,
LogHelper.MarkAsNonPII(address)),
ex));
throw LogHelper.LogExceptionMessage(new IOException(LogHelper.FormatInvariant(LogMessages.IDX20804, address), ex));
}

throw LogHelper.LogExceptionMessage(unsuccessfulHttpResponseException);
Expand Down
Loading

0 comments on commit 23a0b12

Please sign in to comment.