From d0dcf1da5b188083ae2045f8bf1fc051c6fc2334 Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Thu, 4 Feb 2021 14:00:24 -0800 Subject: [PATCH 01/12] Reload secrets from key vault --- .../AzureAppConfigurationKeyVaultOptions.cs | 12 + .../AzureAppConfigurationOptions.cs | 3 +- .../AzureAppConfigurationProvider.cs | 38 +- .../AzureKeyVaultKeyValueAdapter.cs | 19 +- .../AzureKeyVaultSecretProvider.cs | 208 ++++++++++- .../CachedKeyVaultSecret.cs | 51 +++ .../FeatureManagementKeyValueAdapter.cs | 10 + .../IKeyValueAdapter.cs | 4 + .../JsonKeyValueAdapter.cs | 10 + ...Configuration.AzureAppConfiguration.csproj | 1 + .../KeyVaultReferenceTests.cs | 325 ++++++++++++++++++ 11 files changed, 667 insertions(+), 14 deletions(-) create mode 100644 src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index c781063bd..34f2f7aa8 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. // using Azure.Core; +using Azure.Security.KeyVault.Certificates; using Azure.Security.KeyVault.Secrets; using System; using System.Collections.Generic; @@ -16,6 +17,7 @@ public class AzureAppConfigurationKeyVaultOptions { internal TokenCredential Credential; internal List SecretClients = new List(); + internal List CertificateClients = new List(); internal Func> SecretResolver; /// @@ -38,6 +40,16 @@ public AzureAppConfigurationKeyVaultOptions Register(SecretClient secretClient) return this; } + /// + /// Registers the specified instance to reload certificates from Key Vault based on their auto-renewal policy. + /// + /// Certificate client instance. + public AzureAppConfigurationKeyVaultOptions Register(CertificateClient certificateClient) + { + CertificateClients.Add(certificateClient); + return this; + } + /// /// Sets the callback used to resolve key vault references that have no registered . /// diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index 860900861..c0c88770a 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -26,6 +26,7 @@ public class AzureAppConfigurationOptions private List _changeWatchers = new List(); private List _multiKeyWatchers = new List(); + private List _adapters = new List() { new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider()), @@ -336,7 +337,7 @@ public AzureAppConfigurationOptions ConfigureKeyVault(Action a is AzureKeyVaultKeyValueAdapter); - _adapters.Add(new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider(keyVaultOptions.Credential, keyVaultOptions.SecretClients, keyVaultOptions.SecretResolver))); + _adapters.Add(new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider(keyVaultOptions.Credential, keyVaultOptions.SecretClients, keyVaultOptions.CertificateClients, keyVaultOptions.SecretResolver))); return this; } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index 319ab865e..494376c9a 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -167,6 +167,7 @@ public async Task RefreshAsync() await RefreshIndividualKeyValues().ConfigureAwait(false); await RefreshKeyValueCollections().ConfigureAwait(false); + await RefreshKeyValueCollections().ConfigureAwait(false); } finally { @@ -294,6 +295,12 @@ await CallWithRequestTracing(async () => if (data != null) { + // Invalidate all the cached KeyVault secrets + foreach (IKeyValueAdapter adapter in _options.Adapters) + { + adapter.InvalidateCache(); + } + await SetData(data, ignoreFailures).ConfigureAwait(false); // Set the cache expiration time for all refresh registered settings @@ -429,11 +436,17 @@ await TracingUtils.CallWithRequestTracing(_requestTracingEnabled, RequestType.Wa hasChanged = true; - // Add the key-value if it is not loaded, or update it if it was loaded with a different label - _applicationSettings[watchedKey] = watchedKv; - _watchedSettings[watchedKeyLabel] = watchedKv; + // Add the key-value if it is not loaded, or update it if it was loaded with a different label + _applicationSettings[watchedKey] = watchedKv; + _watchedSettings[watchedKeyLabel] = watchedKv; + + // Invalidate the cached Key Vault secret (if any) for this ConfigurationSetting + foreach (IKeyValueAdapter adapter in _options.Adapters) + { + adapter.InvalidateCache(watchedKv); + } + } } - } if (hasChanged) { @@ -448,6 +461,17 @@ await TracingUtils.CallWithRequestTracing(_requestTracingEnabled, RequestType.Wa } } + private async Task RefreshKeyVaultSecrets() + { + foreach (IKeyValueAdapter adapter in _options.Adapters) + { + if(adapter.NeedsRefresh()) + { + await SetData(_applicationSettings).ConfigureAwait(false); + } + } + } + private async Task RefreshKeyValueCollections() { foreach (KeyValueWatcher changeWatcher in _options.MultiKeyWatchers) @@ -581,6 +605,12 @@ private void ProcessChanges(IEnumerable changes) { _applicationSettings[change.Key] = change.Current; } + + // Invalidate the cached Key Vault secret (if any) for this ConfigurationSetting + foreach (IKeyValueAdapter adapter in _options.Adapters) + { + adapter.InvalidateCache(change.Current); + } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs index 77a27c2e0..70f3c9cd7 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs @@ -50,7 +50,7 @@ public async Task>> ProcessKeyValue(Con try { - secret = await _secretProvider.GetSecretValue(secretUri, cancellationToken).ConfigureAwait(false); + secret = await _secretProvider.GetSecretValue(secretUri, setting, cancellationToken).ConfigureAwait(false); } catch (Exception e) when (e is UnauthorizedAccessException || (e.Source?.Equals(AzureIdentityAssemblyName, StringComparison.OrdinalIgnoreCase) ?? false)) { @@ -84,5 +84,22 @@ public bool CanProcess(ConfigurationSetting setting) string contentType = setting?.ContentType?.Split(';')[0].Trim(); return string.Equals(contentType, KeyVaultConstants.ContentType); } + + public void InvalidateCache(ConfigurationSetting setting = null) + { + if (setting == null) + { + _secretProvider.RemoveAllSecretsFromCache(); + } + else + { + _secretProvider.RemoveExpiredSecretFromCache(setting.Key, setting.Label); + } + } + + public bool NeedsRefresh() + { + return _secretProvider.AnyExpiredSecrets(); + } } } \ No newline at end of file diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index a1a8d7bbe..bb4c475d2 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -1,7 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // +using Azure; using Azure.Core; +using Azure.Data.AppConfiguration; +using Azure.Security.KeyVault.Certificates; using Azure.Security.KeyVault.Secrets; using System; using System.Collections.Generic; @@ -13,14 +16,18 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault { internal class AzureKeyVaultSecretProvider { + private const string AzureIdentityAssemblyName = "Azure.Identity"; private readonly IDictionary _secretClients; + private readonly IDictionary _certificateClients; private readonly TokenCredential _credential; private readonly Func> _secretResolver; + private HashSet _cachedKeyVaultSecrets = new HashSet(); - public AzureKeyVaultSecretProvider(TokenCredential credential = null, IEnumerable secretClients = null, Func> secretResolver = null) + public AzureKeyVaultSecretProvider(TokenCredential credential = null, IEnumerable secretClients = null, IEnumerable certificateClients = null, Func> secretResolver = null) { _credential = credential; _secretClients = new Dictionary(StringComparer.OrdinalIgnoreCase); + _certificateClients = new Dictionary(StringComparer.OrdinalIgnoreCase); _secretResolver = secretResolver; if (secretClients != null) @@ -31,15 +38,19 @@ public AzureKeyVaultSecretProvider(TokenCredential credential = null, IEnumerabl _secretClients[keyVaultId] = client; } } - } - public async Task GetSecretValue(Uri secretUri, CancellationToken cancellationToken) - { - if (secretUri == null) + if (certificateClients != null) { - throw new ArgumentNullException(nameof(secretUri)); + foreach (CertificateClient client in certificateClients) + { + string keyVaultId = client.VaultUri.Host; + _certificateClients[keyVaultId] = client; + } } + } + public async Task GetSecretValue(Uri secretUri, ConfigurationSetting setting, CancellationToken cancellationToken) + { string secretName = secretUri?.Segments?.ElementAtOrDefault(2)?.TrimEnd('/'); string secretVersion = secretUri?.Segments?.ElementAtOrDefault(3)?.TrimEnd('/'); string secretValue; @@ -48,8 +59,42 @@ public async Task GetSecretValue(Uri secretUri, CancellationToken cancel if (client != null) { - KeyVaultSecret secret = await client.GetSecretAsync(secretName, secretVersion, cancellationToken).ConfigureAwait(false); - secretValue = secret?.Value; + // Try to load secret value from the cache first + secretValue = GetCachedSecretValue(setting.Key, setting.Label); + + if (secretValue == null) + { + KeyVaultSecret secret; + + try + { + secret = await client.GetSecretAsync(secretName, secretVersion, cancellationToken).ConfigureAwait(false); + } + catch (Exception e) when ( + e is UnauthorizedAccessException || + (e.Source?.Equals(AzureIdentityAssemblyName, StringComparison.OrdinalIgnoreCase) ?? false) || + e is RequestFailedException || + ((e as AggregateException)?.InnerExceptions?.All(e => e is RequestFailedException) ?? false)) + { + // Permission to get secrets may have been revoked. + // Delete any cached secret for this key and label and rethrow exception. + RemoveExpiredSecretFromCache(setting.Key, setting.Label); + throw; + } + + secretValue = secret?.Value; + + if (secret != null) + { + UpdateCachedKeyVaultSecrets(secret, setting.Key, setting.Label, cancellationToken); + } + else + { + // Secret may have been deleted from KeyVault. + // Delete the secret from cache too. + RemoveExpiredSecretFromCache(setting.Key, setting.Label); + } + } } else if (_secretResolver != null) { @@ -63,6 +108,38 @@ public async Task GetSecretValue(Uri secretUri, CancellationToken cancel return secretValue; } + internal bool AnyExpiredSecrets() + { + bool shouldRefreshKeyVaultSecrets = false; + List secretsToBeRemovedFromCache = new List(); + + foreach (var cachedSecret in _cachedKeyVaultSecrets) + { + // Skip the refresh for this key vault secret if it has no expiration time or if it hasn't expired yet + if (cachedSecret.ExpiresOn == null || DateTimeOffset.UtcNow < cachedSecret.ExpiresOn) + { + continue; + } + + // Remove the cached Key Vault secret for this key and label + secretsToBeRemovedFromCache.Add(new CachedKeyVaultSecret(cachedSecret.Key, cachedSecret.Label)); + shouldRefreshKeyVaultSecrets = true; + } + + secretsToBeRemovedFromCache.ForEach(secret => RemoveExpiredSecretFromCache(secret.Key, secret.Label)); + return shouldRefreshKeyVaultSecrets; + } + + internal void RemoveAllSecretsFromCache() + { + _cachedKeyVaultSecrets.Clear(); + } + + internal void RemoveExpiredSecretFromCache(string key, string label) + { + _cachedKeyVaultSecrets.Remove(new CachedKeyVaultSecret(key, label)); + } + private SecretClient GetSecretClient(Uri secretUri) { string keyVaultId = secretUri.Host; @@ -81,5 +158,120 @@ private SecretClient GetSecretClient(Uri secretUri) _secretClients.Add(keyVaultId, client); return client; } + + private CertificateClient GetCertificateClient(Uri secretUri) + { + string keyVaultId = secretUri.Host; + + if (_certificateClients.TryGetValue(keyVaultId, out CertificateClient client)) + { + return client; + } + + if (_credential == null) + { + return null; + } + + client = new CertificateClient(new Uri(secretUri.GetLeftPart(UriPartial.Authority)), _credential); + _certificateClients.Add(keyVaultId, client); + return client; + } + + private async Task GetSecretExpirationTime(KeyVaultSecret secret, CancellationToken cancellationToken) + { + DateTimeOffset? secretExpirationTime = secret.Properties.ExpiresOn; + + if (secret.Properties.Managed) + { + CertificateClient certClient = GetCertificateClient(secret.Id); + + if (certClient != null) + { + KeyVaultCertificateWithPolicy latestCertificate = null; + + try + { + latestCertificate = await certClient.GetCertificateAsync(secret.Name, cancellationToken).ConfigureAwait(false); + } + catch (Exception e) when ( + e is UnauthorizedAccessException || + (e.Source?.Equals(AzureIdentityAssemblyName, StringComparison.OrdinalIgnoreCase) ?? false) || + e is RequestFailedException || + ((e as AggregateException)?.InnerExceptions?.All(e => e is RequestFailedException) ?? false)) + { + // User may not have the right permissions to get certificates, but have the permission to get secrets. + // Use the expiry date of the secret, if available. Otherwise, treat this as a non-rotating secret. + } + + // Calculate the auto-renewal time only if this is the latest version of certificate. + // If the secret reference is for an older version of certificate, CertificatePolicy + // does not apply to this certificate because it will never be auto-rotated. + if (latestCertificate != null && latestCertificate.Properties != null && latestCertificate.Properties.Version == secret.Properties.Version) + { + secretExpirationTime = latestCertificate.Properties.ExpiresOn; + CertificatePolicy policy = latestCertificate?.Policy; + + if (policy?.LifetimeActions != null) + { + // Currently, only a single LifetimeAction is allowed. It will either be "AutoRenew" or "EmailContacts". + var autoRenewPolicy = policy.LifetimeActions.FirstOrDefault(act => act.Action == CertificatePolicyAction.AutoRenew); + + if (autoRenewPolicy != null) + { + // Either DaysBeforeExpiry or LifetimePercentage will be present + if (autoRenewPolicy.DaysBeforeExpiry.HasValue) + { + int daysBeforeExpiry = autoRenewPolicy.DaysBeforeExpiry.Value; + secretExpirationTime = (DateTimeOffset)(latestCertificate.Properties.ExpiresOn?.AddDays(-daysBeforeExpiry)); + } + else if (autoRenewPolicy.LifetimePercentage.HasValue) + { + int lifetimePercentage = autoRenewPolicy.LifetimePercentage.Value; + var startTime = (DateTimeOffset)latestCertificate.Properties.CreatedOn; + var endTime = (DateTimeOffset)latestCertificate.Properties.ExpiresOn; + var diff = (endTime - startTime).Ticks; + var certLifetimeTicks = diff * lifetimePercentage / 100; + secretExpirationTime = startTime.AddTicks(certLifetimeTicks); + } + } + } + } + } + } + + return secretExpirationTime; + } + + private async void UpdateCachedKeyVaultSecrets(KeyVaultSecret secret, string key, string label, CancellationToken cancellationToken) + { + DateTimeOffset? secretExpirationTime = await GetSecretExpirationTime(secret, cancellationToken).ConfigureAwait(false); + + var cachedSecret = new CachedKeyVaultSecret(key, label); + _cachedKeyVaultSecrets.Remove(cachedSecret); + + // Users may be referencing secrets that have already expired in Key Vault. Cache the secret only if it's not already expired. + // No need to cache expired secrets since they will be fetched from Key Vault with every RefreshAsync call. + if (secretExpirationTime == null || DateTime.UtcNow < secretExpirationTime) + { + cachedSecret.ExpiresOn = secretExpirationTime; + cachedSecret.SecretValue = secret.Value; + _cachedKeyVaultSecrets.Add(cachedSecret); + } + } + + private string GetCachedSecretValue(string key, string label) + { + CachedKeyVaultSecret cachedSecret = _cachedKeyVaultSecrets.FirstOrDefault(secret => secret.Key == key && secret.Label == label); + string cachedSecretValue = null; + + if(cachedSecret != null && (cachedSecret.ExpiresOn == null || DateTimeOffset.UtcNow < cachedSecret.ExpiresOn)) + { + // Return cached secret if it has no expiration time or if it hasn't expired yet + cachedSecretValue = cachedSecret.SecretValue; + } + + return cachedSecretValue; + } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs new file mode 100644 index 000000000..eb45afd2b --- /dev/null +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +using System; + +namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault +{ + internal class CachedKeyVaultSecret + { + /// + /// Key of the Key Vault reference in App Configuration. + /// + public string Key { get; set; } + + /// + /// Label of the Key Vault reference in App Configuration. + /// + public string Label { get; set; } + + ///// + ///// The value of the Key Vault secret. + ///// + public string SecretValue { get; set; } + + /// + /// The cache expiration time for the Key Vault secret. + /// + public DateTimeOffset? ExpiresOn { get; set; } + + public CachedKeyVaultSecret(string key, string label) + { + Key = key; + Label = label; + } + + public override bool Equals(object obj) + { + if (obj is CachedKeyVaultSecret keyLabel) + { + return Key == keyLabel.Key && Label == keyLabel.Label; + } + + return false; + } + + public override int GetHashCode() + { + return Label != null ? Key.GetHashCode() ^ Label.GetHashCode() : Key.GetHashCode(); + } + } +} diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs index e8aaf181b..61afbd986 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs @@ -89,5 +89,15 @@ public bool CanProcess(ConfigurationSetting setting) return string.Equals(contentType, FeatureManagementConstants.ContentType) || setting.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker); } + + public void InvalidateCache(ConfigurationSetting setting = null) + { + return; + } + + public bool NeedsRefresh() + { + return false; + } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/IKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/IKeyValueAdapter.cs index 31839fe3d..c72cc7857 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/IKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/IKeyValueAdapter.cs @@ -13,5 +13,9 @@ internal interface IKeyValueAdapter Task>> ProcessKeyValue(ConfigurationSetting setting, CancellationToken cancellationToken); bool CanProcess(ConfigurationSetting setting); + + void InvalidateCache(ConfigurationSetting setting = null); + + bool NeedsRefresh(); } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/JsonKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/JsonKeyValueAdapter.cs index 176e202b0..f245dee97 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/JsonKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/JsonKeyValueAdapter.cs @@ -89,5 +89,15 @@ public bool CanProcess(ConfigurationSetting setting) return false; } + + public void InvalidateCache(ConfigurationSetting setting = null) + { + return; + } + + public bool NeedsRefresh() + { + return false; + } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj index ada10707d..e6187b43b 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj @@ -15,6 +15,7 @@ + diff --git a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs index aaecd577b..cc1cf677d 100644 --- a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs +++ b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs @@ -5,6 +5,7 @@ using Azure.Core.Testing; using Azure.Data.AppConfiguration; using Azure.Identity; +using Azure.Security.KeyVault.Certificates; using Azure.Security.KeyVault.Secrets; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.AzureAppConfiguration; @@ -12,6 +13,7 @@ using Moq; using System; using System.Collections.Generic; +using System.Text; using System.Threading; using System.Threading.Tasks; using Xunit; @@ -23,6 +25,8 @@ public class KeyVaultReferenceTests string _secretValue = "SecretValue from KeyVault"; string _secretUri = "https://keyvault-theclassics.vault.azure.net/secrets/TheTrialSecret"; + ConfigurationSetting sentinelKv = new ConfigurationSetting("Sentinel", "Value1"); + ConfigurationSetting _kv = ConfigurationModelFactory.ConfigurationSetting( key: "TestKey1", value: @" @@ -32,6 +36,15 @@ public class KeyVaultReferenceTests eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1"), contentType: KeyVaultConstants.ContentType + "; charset=utf-8"); + ConfigurationSetting _kvCertRef = ConfigurationModelFactory.ConfigurationSetting( + key: "TestCertificateKey", + value: @" + { + ""uri"":""https://keyvault-theclassics.vault.azure.net/certificates/TestCertificate"" + }", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1"), + contentType: KeyVaultConstants.ContentType + "; charset=utf-8"); + ConfigurationSetting _kvNoUrl = ConfigurationModelFactory.ConfigurationSetting( key: "TestKey1", value: "Test", @@ -506,5 +519,317 @@ public void DontUseSecretResolverCallbackWhenMatchingSecretClientIsPresent() Assert.Equal(_secretValue, configuration[_kv.Key]); } + + [Fact] + public void SecretIsReturnedFromCacheIfSecretCacheHasNotExpired() + { + IConfigurationRefresher refresher = null; + TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); + + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + + Response GetTestKey(string key, string label, CancellationToken cancellationToken) + { + return Response.FromValue(TestHelpers.CloneSetting(sentinelKv), mockResponse.Object); + } + + Response GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken) + { + var unchanged = sentinelKv.Key == setting.Key && sentinelKv.Label == setting.Label && sentinelKv.Value == setting.Value; + var response = new MockResponse(unchanged ? 304 : 200); + return Response.FromValue(sentinelKv, response); + } + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List { _kv })); + + mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((Func>)GetTestKey); + + mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((Func>)GetIfChanged); + + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(new KeyVaultSecret(name, _secretValue)))); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object)); + + options.ConfigureRefresh(refreshOptions => + { + refreshOptions.Register("Sentinel") + .SetCacheExpiration(cacheExpirationTime); + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal("Value1", config["Sentinel"]); + Assert.Equal(_secretValue, config[_kv.Key]); + + // Update sentinel key-value to trigger refresh operation + sentinelKv.Value = "Value2"; + Thread.Sleep(cacheExpirationTime); + refresher.RefreshAsync().Wait(); + + Assert.Equal("Value2", config["Sentinel"]); + Assert.Equal(_secretValue, config[_kv.Key]); + + // Validate that only 1 call was made to fetch secrets from KeyVault + mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + } + + [Fact] + public void CachedSecretIsInvalidatedWhenRefreshAllIsTrue() + { + IConfigurationRefresher refresher = null; + TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); + + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + + Response GetTestKey(string key, string label, CancellationToken cancellationToken) + { + return Response.FromValue(TestHelpers.CloneSetting(sentinelKv), mockResponse.Object); + } + + Response GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken) + { + var unchanged = sentinelKv.Key == setting.Key && sentinelKv.Label == setting.Label && sentinelKv.Value == setting.Value; + var response = new MockResponse(unchanged ? 304 : 200); + return Response.FromValue(sentinelKv, response); + } + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List { _kv })); + + mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((Func>)GetTestKey); + + mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((Func>)GetIfChanged); + + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(new KeyVaultSecret(name, _secretValue)))); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object)); + + options.ConfigureRefresh(refreshOptions => + { + refreshOptions.Register("Sentinel", refreshAll: true) + .SetCacheExpiration(cacheExpirationTime); + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal("Value1", config["Sentinel"]); + Assert.Equal(_secretValue, config[_kv.Key]); + + // Update sentinel key-value to trigger refresh operation + sentinelKv.Value = "Value2"; + Thread.Sleep(cacheExpirationTime); + refresher.RefreshAsync().Wait(); + + Assert.Equal("Value2", config["Sentinel"]); + Assert.Equal(_secretValue, config[_kv.Key]); + + // Validate that 2 calls were made to fetch secrets from KeyVault + mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + } + + [Fact] + public void ExpiredSecretIsReloadedFromKeyVault() + { + IConfigurationRefresher refresher = null; + TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); + + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + + Response GetTestKey(string key, string label, CancellationToken cancellationToken) + { + return Response.FromValue(TestHelpers.CloneSetting(sentinelKv), mockResponse.Object); + } + + Response GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken) + { + var unchanged = sentinelKv.Key == setting.Key && sentinelKv.Label == setting.Label && sentinelKv.Value == setting.Value; + var response = new MockResponse(unchanged ? 304 : 200); + return Response.FromValue(sentinelKv, response); + } + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List { _kv })); + + mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((Func>)GetTestKey); + + mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((Func>)GetIfChanged); + + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); + KeyVaultSecret keyVaultSecret = new KeyVaultSecret("TheTrialSecret", _secretValue); + keyVaultSecret.Properties.ExpiresOn = DateTimeOffset.UtcNow.Add(cacheExpirationTime); + + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(keyVaultSecret))); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object)); + + options.ConfigureRefresh(refreshOptions => + { + refreshOptions.Register("Sentinel") + .SetCacheExpiration(cacheExpirationTime); + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal("Value1", config["Sentinel"]); + Assert.Equal(_secretValue, config[_kv.Key]); + + // Sleep to let the secret expire in KeyVault + Thread.Sleep(cacheExpirationTime); + refresher.RefreshAsync().Wait(); + + // Validate that 2 calls were made to fetch secrets from KeyVault because the secret had expired. + mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + + // We will fetch expired secret from KeyVault with every RefreshAsync call even if its value did not change in Key Vault. + Assert.Equal("Value1", config["Sentinel"]); + Assert.Equal(_secretValue, config[_kv.Key]); + } + + [Fact] + public void ExpiredCertIsReloadedFromKeyVault() + { + IConfigurationRefresher refresher = null; + TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); + TimeSpan certExpirationTime = TimeSpan.FromSeconds(5); + + string certName = "TestCertificate"; + string secretValue = "Dummy certificate thumbprint"; + byte[] certValue = Encoding.UTF8.GetBytes(secretValue); + DateTimeOffset certExpiresOn = DateTimeOffset.UtcNow.Add(certExpirationTime); + + // Create Secret Properties with no expiration time + Uri vaultUri = new Uri("https://keyvault-theclassics.vault.azure.net"); + Uri secretId = new Uri($"https://keyvault-theclassics.vault.azure.net/secrets/{certName}"); + Uri keyId = new Uri($"https://keyvault-theclassics.vault.azure.net/keys/{certName}"); + SecretProperties secretProp = SecretModelFactory.SecretProperties(secretId, vaultUri, certName, managed: true, keyId: keyId); + + // Create Secret + KeyVaultSecret keyVaultSecret = SecretModelFactory.KeyVaultSecret(secretProp, secretValue); + + // Create Certificate Properties with an expiration time + Uri certId = new Uri($"https://keyvault-theclassics.vault.azure.net/certificates/{certName}"); + CertificateProperties certProp = CertificateModelFactory.CertificateProperties(certId, certName, vaultUri, expiresOn: certExpiresOn); + + // Create Certificate Policy + string certSubject = $"CN={certName}"; + DateTimeOffset createdOn = DateTimeOffset.UtcNow; + CertificatePolicy certPolicy = CertificateModelFactory.CertificatePolicy(certSubject, createdOn: createdOn); + + // Create Certificate + KeyVaultCertificateWithPolicy certWithPolicy = CertificateModelFactory.KeyVaultCertificateWithPolicy(certProp, keyId, secretId, certValue, certPolicy); + + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + + Response GetTestKey(string key, string label, CancellationToken cancellationToken) + { + return Response.FromValue(TestHelpers.CloneSetting(sentinelKv), mockResponse.Object); + } + + Response GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken) + { + var unchanged = sentinelKv.Key == setting.Key && sentinelKv.Label == setting.Label && sentinelKv.Value == setting.Value; + var response = new MockResponse(unchanged ? 304 : 200); + return Response.FromValue(sentinelKv, response); + } + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List { _kvCertRef })); + + mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((Func>)GetTestKey); + + mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((Func>)GetIfChanged); + + // Setup SecretClient + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(vaultUri); + + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(keyVaultSecret))); + + // Setup CertificateClient + var mockCertClient = new Mock(MockBehavior.Strict); + mockCertClient.SetupGet(client => client.VaultUri).Returns(vaultUri); + + mockCertClient.Setup(client => client.GetCertificateAsync(It.IsAny(), It.IsAny())) + .Returns((string name, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(certWithPolicy))); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.ConfigureKeyVault(kv => + { + kv.Register(mockSecretClient.Object); + kv.Register(mockCertClient.Object); + }); + + options.ConfigureRefresh(refreshOptions => + { + refreshOptions.Register("Sentinel") + .SetCacheExpiration(cacheExpirationTime); + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal("Value1", config["Sentinel"]); + Assert.Equal(secretValue, config[_kvCertRef.Key]); + + // Sleep to let the cert expire in KeyVault + Thread.Sleep(certExpirationTime); + refresher.RefreshAsync().Wait(); + + // Validate that 2 calls were made to fetch secrets from KeyVault because the cert had expired. + mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + + // We will fetch expired secret from KeyVault with every RefreshAsync call even if its value did not change in Key Vault. + Assert.Equal("Value1", config["Sentinel"]); + Assert.Equal(secretValue, config[_kvCertRef.Key]); + } } } From 5ee03e9b1f065379e5bf10c86b50e42639e57fd8 Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Thu, 1 Apr 2021 14:40:12 -0700 Subject: [PATCH 02/12] Add InvalidateCache definition for a test that mocks IKeyValueAdapter --- tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs index cc1cf677d..7ca40dda6 100644 --- a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs +++ b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs @@ -371,6 +371,7 @@ public void DoesNotThrowKeyVaultExceptionWhenProviderIsOptional() .Returns(true); mockKeyValueAdapter.Setup(adapter => adapter.ProcessKeyValue(_kv, It.IsAny())) .Throws(new KeyVaultReferenceException("Key vault error", null)); + mockKeyValueAdapter.Setup(adapter => adapter.InvalidateCache(null)); new ConfigurationBuilder() .AddAzureAppConfiguration(options => From a437c7be46dee330674e00e4a1228626c07da01d Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Tue, 13 Apr 2021 14:38:34 -0700 Subject: [PATCH 03/12] Updated code for SetSecretRefreshInterval API --- .../AzureAppConfigurationKeyVaultOptions.cs | 40 ++-- .../AzureAppConfigurationOptions.cs | 2 +- .../AzureAppConfigurationProvider.cs | 7 +- .../AzureKeyVaultKeyValueAdapter.cs | 4 +- .../AzureKeyVaultSecretProvider.cs | 179 ++++------------- .../CachedKeyVaultSecret.cs | 14 +- .../Constants/ErrorMessages.cs | 1 + ...Configuration.AzureAppConfiguration.csproj | 1 - .../KeyVaultReferenceTests.cs | 180 +++++++----------- 9 files changed, 142 insertions(+), 286 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index 34f2f7aa8..a8bdf6a2e 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. // using Azure.Core; -using Azure.Security.KeyVault.Certificates; using Azure.Security.KeyVault.Secrets; using System; using System.Collections.Generic; @@ -15,10 +14,18 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration /// public class AzureAppConfigurationKeyVaultOptions { + private static readonly TimeSpan DefaultRefreshInterval = TimeSpan.FromHours(12); + private static readonly TimeSpan MinimumRefreshInterval = TimeSpan.FromHours(1); + internal TokenCredential Credential; internal List SecretClients = new List(); - internal List CertificateClients = new List(); internal Func> SecretResolver; + internal Dictionary SecretRefreshIntervals = new Dictionary(); + + /// + /// If true, certificates will be reloaded from Key Vault based on their auto-renewal policy. + /// + public bool? UseCertificateRotationPolicy { get; set; } = null; /// /// Sets the credentials used to authenticate to key vaults that have no registered . @@ -40,16 +47,6 @@ public AzureAppConfigurationKeyVaultOptions Register(SecretClient secretClient) return this; } - /// - /// Registers the specified instance to reload certificates from Key Vault based on their auto-renewal policy. - /// - /// Certificate client instance. - public AzureAppConfigurationKeyVaultOptions Register(CertificateClient certificateClient) - { - CertificateClients.Add(certificateClient); - return this; - } - /// /// Sets the callback used to resolve key vault references that have no registered . /// @@ -64,5 +61,24 @@ public AzureAppConfigurationKeyVaultOptions SetSecretResolver(Func + /// Sets the refresh interval for periodically reloading a secret from Key Vault. Refresh interval must be greater than 1 hour. Default refresh interval is 12 hours. + /// Any refresh operation triggered using will not update the value for a Key Vault secret until the cached value for that secret has expired. + /// + /// Key of the Key Vault reference in Azure App Configuration. + /// Minimum time that must elapse before the secret is reloaded from Key Vault. + public AzureAppConfigurationKeyVaultOptions SetSecretRefreshInterval(string key, TimeSpan? refreshInterval = null) + { + if (refreshInterval != null && refreshInterval < MinimumRefreshInterval) + { + throw new ArgumentOutOfRangeException(nameof(refreshInterval), refreshInterval?.TotalHours, + string.Format(ErrorMessages.SecretRefreshIntervalTooShort, MinimumRefreshInterval.TotalHours)); + } + + SecretRefreshIntervals[key] = refreshInterval ?? DefaultRefreshInterval; + + return this; + } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index c0c88770a..6bffbe8f0 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -337,7 +337,7 @@ public AzureAppConfigurationOptions ConfigureKeyVault(Action a is AzureKeyVaultKeyValueAdapter); - _adapters.Add(new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider(keyVaultOptions.Credential, keyVaultOptions.SecretClients, keyVaultOptions.CertificateClients, keyVaultOptions.SecretResolver))); + _adapters.Add(new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider(keyVaultOptions.Credential, keyVaultOptions.SecretClients, keyVaultOptions.SecretResolver, keyVaultOptions.SecretRefreshIntervals))); return this; } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index 494376c9a..a6c4c5317 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -207,6 +207,11 @@ public void SetDirty(TimeSpan? maxDelay) { changeWatcher.CacheExpires = cacheExpires; } + + foreach (IKeyValueAdapter adapter in _options.Adapters) + { + adapter.InvalidateCache(); + } } private async Task LoadAll(bool ignoreFailures) @@ -465,7 +470,7 @@ private async Task RefreshKeyVaultSecrets() { foreach (IKeyValueAdapter adapter in _options.Adapters) { - if(adapter.NeedsRefresh()) + if (adapter.NeedsRefresh()) { await SetData(_applicationSettings).ConfigureAwait(false); } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs index 70f3c9cd7..72305bdf2 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs @@ -50,7 +50,7 @@ public async Task>> ProcessKeyValue(Con try { - secret = await _secretProvider.GetSecretValue(secretUri, setting, cancellationToken).ConfigureAwait(false); + secret = await _secretProvider.GetSecretValue(secretUri, setting.Key, cancellationToken).ConfigureAwait(false); } catch (Exception e) when (e is UnauthorizedAccessException || (e.Source?.Equals(AzureIdentityAssemblyName, StringComparison.OrdinalIgnoreCase) ?? false)) { @@ -93,7 +93,7 @@ public void InvalidateCache(ConfigurationSetting setting = null) } else { - _secretProvider.RemoveExpiredSecretFromCache(setting.Key, setting.Label); + _secretProvider.RemoveExpiredSecretFromCache(setting.Key); } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index bb4c475d2..54c1a486e 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -1,10 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // -using Azure; using Azure.Core; -using Azure.Data.AppConfiguration; -using Azure.Security.KeyVault.Certificates; using Azure.Security.KeyVault.Secrets; using System; using System.Collections.Generic; @@ -16,19 +13,18 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault { internal class AzureKeyVaultSecretProvider { - private const string AzureIdentityAssemblyName = "Azure.Identity"; private readonly IDictionary _secretClients; - private readonly IDictionary _certificateClients; private readonly TokenCredential _credential; private readonly Func> _secretResolver; + private readonly Dictionary _secretRefreshIntervals = new Dictionary(); private HashSet _cachedKeyVaultSecrets = new HashSet(); - public AzureKeyVaultSecretProvider(TokenCredential credential = null, IEnumerable secretClients = null, IEnumerable certificateClients = null, Func> secretResolver = null) + public AzureKeyVaultSecretProvider(TokenCredential credential = null, IEnumerable secretClients = null, Func> secretResolver = null, Dictionary secretRefreshIntervals = null) { _credential = credential; _secretClients = new Dictionary(StringComparer.OrdinalIgnoreCase); - _certificateClients = new Dictionary(StringComparer.OrdinalIgnoreCase); _secretResolver = secretResolver; + _secretRefreshIntervals = secretRefreshIntervals; if (secretClients != null) { @@ -38,18 +34,9 @@ public AzureKeyVaultSecretProvider(TokenCredential credential = null, IEnumerabl _secretClients[keyVaultId] = client; } } - - if (certificateClients != null) - { - foreach (CertificateClient client in certificateClients) - { - string keyVaultId = client.VaultUri.Host; - _certificateClients[keyVaultId] = client; - } - } } - public async Task GetSecretValue(Uri secretUri, ConfigurationSetting setting, CancellationToken cancellationToken) + public async Task GetSecretValue(Uri secretUri, string key, CancellationToken cancellationToken) { string secretName = secretUri?.Segments?.ElementAtOrDefault(2)?.TrimEnd('/'); string secretVersion = secretUri?.Segments?.ElementAtOrDefault(3)?.TrimEnd('/'); @@ -59,40 +46,27 @@ public async Task GetSecretValue(Uri secretUri, ConfigurationSetting set if (client != null) { + KeyVaultSecret secret; + // Try to load secret value from the cache first - secretValue = GetCachedSecretValue(setting.Key, setting.Label); - + secretValue = GetCachedSecretValue(key); + if (secretValue == null) { - KeyVaultSecret secret; - - try - { - secret = await client.GetSecretAsync(secretName, secretVersion, cancellationToken).ConfigureAwait(false); - } - catch (Exception e) when ( - e is UnauthorizedAccessException || - (e.Source?.Equals(AzureIdentityAssemblyName, StringComparison.OrdinalIgnoreCase) ?? false) || - e is RequestFailedException || - ((e as AggregateException)?.InnerExceptions?.All(e => e is RequestFailedException) ?? false)) - { - // Permission to get secrets may have been revoked. - // Delete any cached secret for this key and label and rethrow exception. - RemoveExpiredSecretFromCache(setting.Key, setting.Label); - throw; - } - + // We dont have a cached secret value for this key vault reference. + // Get the secret from Key Vault and update the cache. + secret = await client.GetSecretAsync(secretName, secretVersion, cancellationToken).ConfigureAwait(false); secretValue = secret?.Value; if (secret != null) { - UpdateCachedKeyVaultSecrets(secret, setting.Key, setting.Label, cancellationToken); + UpdateCachedKeyVaultSecrets(key, secretValue); } else { // Secret may have been deleted from KeyVault. // Delete the secret from cache too. - RemoveExpiredSecretFromCache(setting.Key, setting.Label); + RemoveExpiredSecretFromCache(key); } } } @@ -121,12 +95,12 @@ internal bool AnyExpiredSecrets() continue; } - // Remove the cached Key Vault secret for this key and label - secretsToBeRemovedFromCache.Add(new CachedKeyVaultSecret(cachedSecret.Key, cachedSecret.Label)); + // Remove the cached Key Vault secret for this key + secretsToBeRemovedFromCache.Add(new CachedKeyVaultSecret(cachedSecret.Key)); shouldRefreshKeyVaultSecrets = true; } - secretsToBeRemovedFromCache.ForEach(secret => RemoveExpiredSecretFromCache(secret.Key, secret.Label)); + secretsToBeRemovedFromCache.ForEach(secret => RemoveExpiredSecretFromCache(secret.Key)); return shouldRefreshKeyVaultSecrets; } @@ -135,9 +109,9 @@ internal void RemoveAllSecretsFromCache() _cachedKeyVaultSecrets.Clear(); } - internal void RemoveExpiredSecretFromCache(string key, string label) + internal void RemoveExpiredSecretFromCache(string key) { - _cachedKeyVaultSecrets.Remove(new CachedKeyVaultSecret(key, label)); + _cachedKeyVaultSecrets.Remove(new CachedKeyVaultSecret(key)); } private SecretClient GetSecretClient(Uri secretUri) @@ -159,119 +133,30 @@ private SecretClient GetSecretClient(Uri secretUri) return client; } - private CertificateClient GetCertificateClient(Uri secretUri) - { - string keyVaultId = secretUri.Host; - - if (_certificateClients.TryGetValue(keyVaultId, out CertificateClient client)) - { - return client; - } - - if (_credential == null) - { - return null; - } - - client = new CertificateClient(new Uri(secretUri.GetLeftPart(UriPartial.Authority)), _credential); - _certificateClients.Add(keyVaultId, client); - return client; - } - - private async Task GetSecretExpirationTime(KeyVaultSecret secret, CancellationToken cancellationToken) + private void UpdateCachedKeyVaultSecrets(string key, string secretValue) { - DateTimeOffset? secretExpirationTime = secret.Properties.ExpiresOn; + DateTimeOffset? secretExpirationTime = null; - if (secret.Properties.Managed) + if(_secretRefreshIntervals != null && _secretRefreshIntervals.TryGetValue(key, out TimeSpan refreshInterval)) { - CertificateClient certClient = GetCertificateClient(secret.Id); - - if (certClient != null) - { - KeyVaultCertificateWithPolicy latestCertificate = null; - - try - { - latestCertificate = await certClient.GetCertificateAsync(secret.Name, cancellationToken).ConfigureAwait(false); - } - catch (Exception e) when ( - e is UnauthorizedAccessException || - (e.Source?.Equals(AzureIdentityAssemblyName, StringComparison.OrdinalIgnoreCase) ?? false) || - e is RequestFailedException || - ((e as AggregateException)?.InnerExceptions?.All(e => e is RequestFailedException) ?? false)) - { - // User may not have the right permissions to get certificates, but have the permission to get secrets. - // Use the expiry date of the secret, if available. Otherwise, treat this as a non-rotating secret. - } - - // Calculate the auto-renewal time only if this is the latest version of certificate. - // If the secret reference is for an older version of certificate, CertificatePolicy - // does not apply to this certificate because it will never be auto-rotated. - if (latestCertificate != null && latestCertificate.Properties != null && latestCertificate.Properties.Version == secret.Properties.Version) - { - secretExpirationTime = latestCertificate.Properties.ExpiresOn; - CertificatePolicy policy = latestCertificate?.Policy; - - if (policy?.LifetimeActions != null) - { - // Currently, only a single LifetimeAction is allowed. It will either be "AutoRenew" or "EmailContacts". - var autoRenewPolicy = policy.LifetimeActions.FirstOrDefault(act => act.Action == CertificatePolicyAction.AutoRenew); - - if (autoRenewPolicy != null) - { - // Either DaysBeforeExpiry or LifetimePercentage will be present - if (autoRenewPolicy.DaysBeforeExpiry.HasValue) - { - int daysBeforeExpiry = autoRenewPolicy.DaysBeforeExpiry.Value; - secretExpirationTime = (DateTimeOffset)(latestCertificate.Properties.ExpiresOn?.AddDays(-daysBeforeExpiry)); - } - else if (autoRenewPolicy.LifetimePercentage.HasValue) - { - int lifetimePercentage = autoRenewPolicy.LifetimePercentage.Value; - var startTime = (DateTimeOffset)latestCertificate.Properties.CreatedOn; - var endTime = (DateTimeOffset)latestCertificate.Properties.ExpiresOn; - var diff = (endTime - startTime).Ticks; - var certLifetimeTicks = diff * lifetimePercentage / 100; - secretExpirationTime = startTime.AddTicks(certLifetimeTicks); - } - } - } - } - } + // Set the cache expiration time using the refresh interval specified for this key + secretExpirationTime = DateTimeOffset.UtcNow.Add(refreshInterval); } - - return secretExpirationTime; - } - private async void UpdateCachedKeyVaultSecrets(KeyVaultSecret secret, string key, string label, CancellationToken cancellationToken) - { - DateTimeOffset? secretExpirationTime = await GetSecretExpirationTime(secret, cancellationToken).ConfigureAwait(false); - - var cachedSecret = new CachedKeyVaultSecret(key, label); + var cachedSecret = new CachedKeyVaultSecret(key); _cachedKeyVaultSecrets.Remove(cachedSecret); - // Users may be referencing secrets that have already expired in Key Vault. Cache the secret only if it's not already expired. - // No need to cache expired secrets since they will be fetched from Key Vault with every RefreshAsync call. - if (secretExpirationTime == null || DateTime.UtcNow < secretExpirationTime) - { - cachedSecret.ExpiresOn = secretExpirationTime; - cachedSecret.SecretValue = secret.Value; - _cachedKeyVaultSecrets.Add(cachedSecret); - } + // If there is no refresh interval for this key, cache expiration time will be null, + // i.e., this secret will not be refreshed automatically. + cachedSecret.ExpiresOn = secretExpirationTime; + cachedSecret.SecretValue = secretValue; + _cachedKeyVaultSecrets.Add(cachedSecret); } - private string GetCachedSecretValue(string key, string label) + private string GetCachedSecretValue(string key) { - CachedKeyVaultSecret cachedSecret = _cachedKeyVaultSecrets.FirstOrDefault(secret => secret.Key == key && secret.Label == label); - string cachedSecretValue = null; - - if(cachedSecret != null && (cachedSecret.ExpiresOn == null || DateTimeOffset.UtcNow < cachedSecret.ExpiresOn)) - { - // Return cached secret if it has no expiration time or if it hasn't expired yet - cachedSecretValue = cachedSecret.SecretValue; - } - - return cachedSecretValue; + CachedKeyVaultSecret cachedSecret = _cachedKeyVaultSecrets.FirstOrDefault(secret => secret.Key == key); + return cachedSecret?.SecretValue; } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs index eb45afd2b..65bf6c9c6 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs @@ -12,11 +12,6 @@ internal class CachedKeyVaultSecret /// public string Key { get; set; } - /// - /// Label of the Key Vault reference in App Configuration. - /// - public string Label { get; set; } - ///// ///// The value of the Key Vault secret. ///// @@ -27,17 +22,16 @@ internal class CachedKeyVaultSecret /// public DateTimeOffset? ExpiresOn { get; set; } - public CachedKeyVaultSecret(string key, string label) + public CachedKeyVaultSecret(string key) { Key = key; - Label = label; } public override bool Equals(object obj) { - if (obj is CachedKeyVaultSecret keyLabel) + if (obj is CachedKeyVaultSecret cachedSecret) { - return Key == keyLabel.Key && Label == keyLabel.Label; + return Key == cachedSecret.Key; } return false; @@ -45,7 +39,7 @@ public override bool Equals(object obj) public override int GetHashCode() { - return Label != null ? Key.GetHashCode() ^ Label.GetHashCode() : Key.GetHashCode(); + return Key.GetHashCode(); } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/ErrorMessages.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/ErrorMessages.cs index c53daa659..3e83b4902 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/ErrorMessages.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/ErrorMessages.cs @@ -6,5 +6,6 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration internal class ErrorMessages { public const string CacheExpirationTimeTooShort = "The cache expiration time cannot be less than {0} milliseconds."; + public const string SecretRefreshIntervalTooShort = "The secret refresh interval cannot be less than {0} hour(s)."; } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj index e6187b43b..ada10707d 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj @@ -15,7 +15,6 @@ - diff --git a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs index 7ca40dda6..94fdaa5c7 100644 --- a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs +++ b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs @@ -5,7 +5,6 @@ using Azure.Core.Testing; using Azure.Data.AppConfiguration; using Azure.Identity; -using Azure.Security.KeyVault.Certificates; using Azure.Security.KeyVault.Secrets; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.AzureAppConfiguration; @@ -135,6 +134,31 @@ public void UseSecret() Assert.Equal(_secretValue, configuration[_kv.Key]); } + [Fact] + public void UseNullSecretValueWhenSecretNotFound() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List { _kv })); + + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(null))); + + var configuration = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object)); + }) + .Build(); + + Assert.Null(configuration[_kv.Key]); + } + [Fact] public void DisabledSecretIdentifier() { @@ -561,7 +585,12 @@ Response GetIfChanged(ConfigurationSetting setting, bool o .AddAzureAppConfiguration(options => { options.Client = mockClient.Object; - options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object)); + + options.ConfigureKeyVault(kv => + { + kv.Register(mockSecretClient.Object); + kv.SetSecretRefreshInterval(_kv.Key, TimeSpan.FromDays(1)); + }); options.ConfigureRefresh(refreshOptions => { @@ -576,7 +605,7 @@ Response GetIfChanged(ConfigurationSetting setting, bool o Assert.Equal("Value1", config["Sentinel"]); Assert.Equal(_secretValue, config[_kv.Key]); - // Update sentinel key-value to trigger refresh operation + // Update sentinel key-value sentinelKv.Value = "Value2"; Thread.Sleep(cacheExpirationTime); refresher.RefreshAsync().Wait(); @@ -585,6 +614,7 @@ Response GetIfChanged(ConfigurationSetting setting, bool o Assert.Equal(_secretValue, config[_kv.Key]); // Validate that only 1 call was made to fetch secrets from KeyVault + // Since Key Vault refresh interval has not elapsed, the sentinel key change should fetch secret from Key Vault mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } @@ -628,7 +658,11 @@ Response GetIfChanged(ConfigurationSetting setting, bool o .AddAzureAppConfiguration(options => { options.Client = mockClient.Object; - options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object)); + options.ConfigureKeyVault(kv => + { + kv.Register(mockSecretClient.Object); + kv.SetSecretRefreshInterval(_kv.Key, TimeSpan.FromDays(1)); + }); options.ConfigureRefresh(refreshOptions => { @@ -652,14 +686,15 @@ Response GetIfChanged(ConfigurationSetting setting, bool o Assert.Equal(_secretValue, config[_kv.Key]); // Validate that 2 calls were made to fetch secrets from KeyVault + // Even though Key Vault refresh interval has not elapsed, refreshAll trigger should fetch secret from Key Vault again mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); } [Fact] - public void ExpiredSecretIsReloadedFromKeyVault() + public void SetDirtyForcesRefreshOfKeyVaultSecrets() { IConfigurationRefresher refresher = null; - TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); + TimeSpan longCacheExpirationTime = TimeSpan.FromDays(1); var mockResponse = new Mock(); var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); @@ -687,23 +722,24 @@ Response GetIfChanged(ConfigurationSetting setting, bool o var mockSecretClient = new Mock(MockBehavior.Strict); mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); - KeyVaultSecret keyVaultSecret = new KeyVaultSecret("TheTrialSecret", _secretValue); - keyVaultSecret.Properties.ExpiresOn = DateTimeOffset.UtcNow.Add(cacheExpirationTime); - mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) .Returns((string name, string version, CancellationToken cancellationToken) => - Task.FromResult((Response)new MockResponse(keyVaultSecret))); + Task.FromResult((Response)new MockResponse(new KeyVaultSecret(name, _secretValue)))); var config = new ConfigurationBuilder() .AddAzureAppConfiguration(options => { options.Client = mockClient.Object; - options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object)); + options.ConfigureKeyVault(kv => + { + kv.Register(mockSecretClient.Object); + kv.SetSecretRefreshInterval(_kv.Key, longCacheExpirationTime); + }); options.ConfigureRefresh(refreshOptions => { refreshOptions.Register("Sentinel") - .SetCacheExpiration(cacheExpirationTime); + .SetCacheExpiration(longCacheExpirationTime); }); refresher = options.GetRefresher(); @@ -713,124 +749,44 @@ Response GetIfChanged(ConfigurationSetting setting, bool o Assert.Equal("Value1", config["Sentinel"]); Assert.Equal(_secretValue, config[_kv.Key]); - // Sleep to let the secret expire in KeyVault - Thread.Sleep(cacheExpirationTime); + // Update sentinel key-value + sentinelKv.Value = "Value2"; + refresher.SetDirty(TimeSpan.FromSeconds(1)); + + // Wait for the cache to expire based on the randomized delay in SetDirty() + Thread.Sleep(1200); refresher.RefreshAsync().Wait(); - - // Validate that 2 calls were made to fetch secrets from KeyVault because the secret had expired. - mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); - // We will fetch expired secret from KeyVault with every RefreshAsync call even if its value did not change in Key Vault. - Assert.Equal("Value1", config["Sentinel"]); + Assert.Equal("Value2", config["Sentinel"]); Assert.Equal(_secretValue, config[_kv.Key]); + + // Validate that 2 calls were made to fetch secrets from KeyVault + // Even though Key Vault refresh interval has not elapsed, SetDirty should invalidate the cached secret and get secret from Key Vault again + mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + + } [Fact] - public void ExpiredCertIsReloadedFromKeyVault() + public void ThrowsWhenSecretRefreshIntervalIsLessThanMinimumInterval() { - IConfigurationRefresher refresher = null; - TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); - TimeSpan certExpirationTime = TimeSpan.FromSeconds(5); - - string certName = "TestCertificate"; - string secretValue = "Dummy certificate thumbprint"; - byte[] certValue = Encoding.UTF8.GetBytes(secretValue); - DateTimeOffset certExpiresOn = DateTimeOffset.UtcNow.Add(certExpirationTime); - - // Create Secret Properties with no expiration time - Uri vaultUri = new Uri("https://keyvault-theclassics.vault.azure.net"); - Uri secretId = new Uri($"https://keyvault-theclassics.vault.azure.net/secrets/{certName}"); - Uri keyId = new Uri($"https://keyvault-theclassics.vault.azure.net/keys/{certName}"); - SecretProperties secretProp = SecretModelFactory.SecretProperties(secretId, vaultUri, certName, managed: true, keyId: keyId); - - // Create Secret - KeyVaultSecret keyVaultSecret = SecretModelFactory.KeyVaultSecret(secretProp, secretValue); - - // Create Certificate Properties with an expiration time - Uri certId = new Uri($"https://keyvault-theclassics.vault.azure.net/certificates/{certName}"); - CertificateProperties certProp = CertificateModelFactory.CertificateProperties(certId, certName, vaultUri, expiresOn: certExpiresOn); - - // Create Certificate Policy - string certSubject = $"CN={certName}"; - DateTimeOffset createdOn = DateTimeOffset.UtcNow; - CertificatePolicy certPolicy = CertificateModelFactory.CertificatePolicy(certSubject, createdOn: createdOn); - - // Create Certificate - KeyVaultCertificateWithPolicy certWithPolicy = CertificateModelFactory.KeyVaultCertificateWithPolicy(certProp, keyId, secretId, certValue, certPolicy); - var mockResponse = new Mock(); var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); - - Response GetTestKey(string key, string label, CancellationToken cancellationToken) - { - return Response.FromValue(TestHelpers.CloneSetting(sentinelKv), mockResponse.Object); - } - - Response GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken) - { - var unchanged = sentinelKv.Key == setting.Key && sentinelKv.Label == setting.Label && sentinelKv.Value == setting.Value; - var response = new MockResponse(unchanged ? 304 : 200); - return Response.FromValue(sentinelKv, response); - } - mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) - .Returns(new MockAsyncPageable(new List { _kvCertRef })); - - mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync((Func>)GetTestKey); - - mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync((Func>)GetIfChanged); - - // Setup SecretClient - var mockSecretClient = new Mock(MockBehavior.Strict); - mockSecretClient.SetupGet(client => client.VaultUri).Returns(vaultUri); - - mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) - .Returns((string name, string version, CancellationToken cancellationToken) => - Task.FromResult((Response)new MockResponse(keyVaultSecret))); - - // Setup CertificateClient - var mockCertClient = new Mock(MockBehavior.Strict); - mockCertClient.SetupGet(client => client.VaultUri).Returns(vaultUri); - - mockCertClient.Setup(client => client.GetCertificateAsync(It.IsAny(), It.IsAny())) - .Returns((string name, CancellationToken cancellationToken) => - Task.FromResult((Response)new MockResponse(certWithPolicy))); + .Returns(new MockAsyncPageable(new List { _kv })); - var config = new ConfigurationBuilder() - .AddAzureAppConfiguration(options => + Assert.Throws(() => + { + new ConfigurationBuilder().AddAzureAppConfiguration(options => { options.Client = mockClient.Object; options.ConfigureKeyVault(kv => { - kv.Register(mockSecretClient.Object); - kv.Register(mockCertClient.Object); + kv.SetSecretRefreshInterval(_kv.Key, TimeSpan.FromMinutes(30)); }); - - options.ConfigureRefresh(refreshOptions => - { - refreshOptions.Register("Sentinel") - .SetCacheExpiration(cacheExpirationTime); - }); - - refresher = options.GetRefresher(); }) .Build(); - - Assert.Equal("Value1", config["Sentinel"]); - Assert.Equal(secretValue, config[_kvCertRef.Key]); - - // Sleep to let the cert expire in KeyVault - Thread.Sleep(certExpirationTime); - refresher.RefreshAsync().Wait(); - - // Validate that 2 calls were made to fetch secrets from KeyVault because the cert had expired. - mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); - - // We will fetch expired secret from KeyVault with every RefreshAsync call even if its value did not change in Key Vault. - Assert.Equal("Value1", config["Sentinel"]); - Assert.Equal(secretValue, config[_kvCertRef.Key]); + }); } } } From 4d6a211e5e5b29cd371fa77e950d12eb245f99dd Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Tue, 20 Apr 2021 14:33:54 -0700 Subject: [PATCH 04/12] cleanup --- .../AzureAppConfigurationKeyVaultOptions.cs | 5 ----- .../AzureAppConfigurationOptions.cs | 1 - 2 files changed, 6 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index a8bdf6a2e..c2cef7752 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -22,11 +22,6 @@ public class AzureAppConfigurationKeyVaultOptions internal Func> SecretResolver; internal Dictionary SecretRefreshIntervals = new Dictionary(); - /// - /// If true, certificates will be reloaded from Key Vault based on their auto-renewal policy. - /// - public bool? UseCertificateRotationPolicy { get; set; } = null; - /// /// Sets the credentials used to authenticate to key vaults that have no registered . /// diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index 6bffbe8f0..e5d2aa501 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -26,7 +26,6 @@ public class AzureAppConfigurationOptions private List _changeWatchers = new List(); private List _multiKeyWatchers = new List(); - private List _adapters = new List() { new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider()), From 31f167edb00199b1c647b94d3d18c47005428d50 Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Wed, 21 Apr 2021 12:37:01 -0700 Subject: [PATCH 05/12] Resolving comments --- .../AzureAppConfigurationKeyVaultOptions.cs | 28 ++-- .../AzureAppConfigurationOptions.cs | 2 +- .../AzureAppConfigurationProvider.cs | 3 +- .../AzureKeyVaultKeyValueAdapter.cs | 6 +- .../AzureKeyVaultSecretProvider.cs | 97 +++++------ .../CachedKeyVaultSecret.cs | 31 +--- .../KeyVaultReferenceTests.cs | 151 +++++++++++++++++- 7 files changed, 215 insertions(+), 103 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index c2cef7752..a4e27d34a 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -14,13 +14,11 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration /// public class AzureAppConfigurationKeyVaultOptions { - private static readonly TimeSpan DefaultRefreshInterval = TimeSpan.FromHours(12); - private static readonly TimeSpan MinimumRefreshInterval = TimeSpan.FromHours(1); - internal TokenCredential Credential; internal List SecretClients = new List(); internal Func> SecretResolver; internal Dictionary SecretRefreshIntervals = new Dictionary(); + internal TimeSpan? DefaultSecretRefreshInterval = null; /// /// Sets the credentials used to authenticate to key vaults that have no registered . @@ -58,21 +56,25 @@ public AzureAppConfigurationKeyVaultOptions SetSecretResolver(Func - /// Sets the refresh interval for periodically reloading a secret from Key Vault. Refresh interval must be greater than 1 hour. Default refresh interval is 12 hours. + /// Sets the refresh interval for periodically reloading a secret from Key Vault. /// Any refresh operation triggered using will not update the value for a Key Vault secret until the cached value for that secret has expired. /// - /// Key of the Key Vault reference in Azure App Configuration. + /// Key of the Key Vault reference in Azure App Configuration. /// Minimum time that must elapse before the secret is reloaded from Key Vault. - public AzureAppConfigurationKeyVaultOptions SetSecretRefreshInterval(string key, TimeSpan? refreshInterval = null) + public AzureAppConfigurationKeyVaultOptions SetSecretRefreshInterval(string secretReferenceKey, TimeSpan refreshInterval) { - if (refreshInterval != null && refreshInterval < MinimumRefreshInterval) - { - throw new ArgumentOutOfRangeException(nameof(refreshInterval), refreshInterval?.TotalHours, - string.Format(ErrorMessages.SecretRefreshIntervalTooShort, MinimumRefreshInterval.TotalHours)); - } - - SecretRefreshIntervals[key] = refreshInterval ?? DefaultRefreshInterval; + SecretRefreshIntervals[secretReferenceKey] = refreshInterval; + return this; + } + /// + /// Sets the refresh interval for periodically reloading all those secrets which do not have individual refresh intervals. + /// Any refresh operation triggered using will not update the value for a Key Vault secret until the cached value for that secret has expired. + /// + /// Minimum time that must elapse before the secrets are reloaded from Key Vault. + public AzureAppConfigurationKeyVaultOptions SetSecretRefreshInterval(TimeSpan refreshInterval) + { + DefaultSecretRefreshInterval = refreshInterval; return this; } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index e5d2aa501..43e52d1f7 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -336,7 +336,7 @@ public AzureAppConfigurationOptions ConfigureKeyVault(Action a is AzureKeyVaultKeyValueAdapter); - _adapters.Add(new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider(keyVaultOptions.Credential, keyVaultOptions.SecretClients, keyVaultOptions.SecretResolver, keyVaultOptions.SecretRefreshIntervals))); + _adapters.Add(new AzureKeyVaultKeyValueAdapter(new AzureKeyVaultSecretProvider(keyVaultOptions))); return this; } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index a6c4c5317..c9d2d3a10 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -466,13 +466,14 @@ await TracingUtils.CallWithRequestTracing(_requestTracingEnabled, RequestType.Wa } } - private async Task RefreshKeyVaultSecrets() + private async Task RefreshKeyValueAdapters() { foreach (IKeyValueAdapter adapter in _options.Adapters) { if (adapter.NeedsRefresh()) { await SetData(_applicationSettings).ConfigureAwait(false); + break; } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs index 72305bdf2..3fc84d833 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs @@ -89,17 +89,17 @@ public void InvalidateCache(ConfigurationSetting setting = null) { if (setting == null) { - _secretProvider.RemoveAllSecretsFromCache(); + _secretProvider.ClearCache(); } else { - _secretProvider.RemoveExpiredSecretFromCache(setting.Key); + _secretProvider.RemoveSecretFromCache(setting.Key); } } public bool NeedsRefresh() { - return _secretProvider.AnyExpiredSecrets(); + return _secretProvider.ShouldRefreshKeyVaultSecrets(); } } } \ No newline at end of file diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index 54c1a486e..21e85b1a8 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -1,9 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // -using Azure.Core; using Azure.Security.KeyVault.Secrets; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -14,21 +14,17 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault internal class AzureKeyVaultSecretProvider { private readonly IDictionary _secretClients; - private readonly TokenCredential _credential; - private readonly Func> _secretResolver; - private readonly Dictionary _secretRefreshIntervals = new Dictionary(); - private HashSet _cachedKeyVaultSecrets = new HashSet(); + private ConcurrentDictionary _cachedKeyVaultSecrets = new ConcurrentDictionary(); + private AzureAppConfigurationKeyVaultOptions _keyVaultOptions; - public AzureKeyVaultSecretProvider(TokenCredential credential = null, IEnumerable secretClients = null, Func> secretResolver = null, Dictionary secretRefreshIntervals = null) + public AzureKeyVaultSecretProvider(AzureAppConfigurationKeyVaultOptions keyVaultOptions = null) { - _credential = credential; + _keyVaultOptions = keyVaultOptions ?? new AzureAppConfigurationKeyVaultOptions(); _secretClients = new Dictionary(StringComparer.OrdinalIgnoreCase); - _secretResolver = secretResolver; - _secretRefreshIntervals = secretRefreshIntervals; - if (secretClients != null) + if (_keyVaultOptions.SecretClients != null) { - foreach (SecretClient client in secretClients) + foreach (SecretClient client in _keyVaultOptions.SecretClients) { string keyVaultId = client.VaultUri.Host; _secretClients[keyVaultId] = client; @@ -48,12 +44,12 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation { KeyVaultSecret secret; - // Try to load secret value from the cache first + // Try to load secret value from the cache first. secretValue = GetCachedSecretValue(key); if (secretValue == null) { - // We dont have a cached secret value for this key vault reference. + // We dont have a cached secret value or the cached value has expired. // Get the secret from Key Vault and update the cache. secret = await client.GetSecretAsync(secretName, secretVersion, cancellationToken).ConfigureAwait(false); secretValue = secret?.Value; @@ -66,13 +62,13 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation { // Secret may have been deleted from KeyVault. // Delete the secret from cache too. - RemoveExpiredSecretFromCache(key); + RemoveSecretFromCache(key); } } } - else if (_secretResolver != null) + else if (_keyVaultOptions.SecretResolver != null) { - secretValue = await _secretResolver(secretUri).ConfigureAwait(false); + secretValue = await _keyVaultOptions.SecretResolver(secretUri).ConfigureAwait(false); } else { @@ -82,36 +78,20 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation return secretValue; } - internal bool AnyExpiredSecrets() + public bool ShouldRefreshKeyVaultSecrets() { - bool shouldRefreshKeyVaultSecrets = false; - List secretsToBeRemovedFromCache = new List(); - - foreach (var cachedSecret in _cachedKeyVaultSecrets) - { - // Skip the refresh for this key vault secret if it has no expiration time or if it hasn't expired yet - if (cachedSecret.ExpiresOn == null || DateTimeOffset.UtcNow < cachedSecret.ExpiresOn) - { - continue; - } - - // Remove the cached Key Vault secret for this key - secretsToBeRemovedFromCache.Add(new CachedKeyVaultSecret(cachedSecret.Key)); - shouldRefreshKeyVaultSecrets = true; - } - - secretsToBeRemovedFromCache.ForEach(secret => RemoveExpiredSecretFromCache(secret.Key)); - return shouldRefreshKeyVaultSecrets; + // return true if the RefreshAt time of any cached secret has already elapsed. + return _cachedKeyVaultSecrets.Any(cachedSecret => cachedSecret.Value.RefreshAt.HasValue && cachedSecret.Value.RefreshAt.Value < DateTimeOffset.UtcNow); } - internal void RemoveAllSecretsFromCache() + public void ClearCache() { _cachedKeyVaultSecrets.Clear(); } - internal void RemoveExpiredSecretFromCache(string key) + public void RemoveSecretFromCache(string key) { - _cachedKeyVaultSecrets.Remove(new CachedKeyVaultSecret(key)); + _cachedKeyVaultSecrets.TryRemove(key, out CachedKeyVaultSecret _); } private SecretClient GetSecretClient(Uri secretUri) @@ -123,40 +103,49 @@ private SecretClient GetSecretClient(Uri secretUri) return client; } - if (_credential == null) + if (_keyVaultOptions.Credential == null) { return null; } - client = new SecretClient(new Uri(secretUri.GetLeftPart(UriPartial.Authority)), _credential); + client = new SecretClient(new Uri(secretUri.GetLeftPart(UriPartial.Authority)), _keyVaultOptions.Credential); _secretClients.Add(keyVaultId, client); return client; } private void UpdateCachedKeyVaultSecrets(string key, string secretValue) { - DateTimeOffset? secretExpirationTime = null; + // If refresh interval for this key or a default refresh interval for all keys has not been specified, + // cache expiration time will be null, i.e., this secret will not be refreshed automatically. + DateTimeOffset? refreshSecretAt = null; - if(_secretRefreshIntervals != null && _secretRefreshIntervals.TryGetValue(key, out TimeSpan refreshInterval)) + if (_keyVaultOptions.SecretRefreshIntervals.TryGetValue(key, out TimeSpan refreshInterval)) { - // Set the cache expiration time using the refresh interval specified for this key - secretExpirationTime = DateTimeOffset.UtcNow.Add(refreshInterval); + // Set the cache expiration time using the refresh interval specified for this key. + refreshSecretAt = DateTimeOffset.UtcNow.Add(refreshInterval); + } + else if (_keyVaultOptions.DefaultSecretRefreshInterval.HasValue) + { + // Set the cache expiration time using the default refresh interval specified for all keys. + refreshSecretAt = DateTimeOffset.UtcNow.Add(_keyVaultOptions.DefaultSecretRefreshInterval.Value); } - var cachedSecret = new CachedKeyVaultSecret(key); - _cachedKeyVaultSecrets.Remove(cachedSecret); - - // If there is no refresh interval for this key, cache expiration time will be null, - // i.e., this secret will not be refreshed automatically. - cachedSecret.ExpiresOn = secretExpirationTime; - cachedSecret.SecretValue = secretValue; - _cachedKeyVaultSecrets.Add(cachedSecret); + // Add or update the cache. + _cachedKeyVaultSecrets[key] = new CachedKeyVaultSecret(secretValue, refreshSecretAt); } private string GetCachedSecretValue(string key) { - CachedKeyVaultSecret cachedSecret = _cachedKeyVaultSecrets.FirstOrDefault(secret => secret.Key == key); - return cachedSecret?.SecretValue; + string cachedSecretValue = null; + + // Use the cached value of this key vault secret if RefreshAt time is null or in the future + if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && + (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) + { + cachedSecretValue = cachedSecret.SecretValue; + } + + return cachedSecretValue; } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs index 65bf6c9c6..08907769f 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs @@ -5,41 +5,22 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault { - internal class CachedKeyVaultSecret + internal struct CachedKeyVaultSecret { - /// - /// Key of the Key Vault reference in App Configuration. - /// - public string Key { get; set; } - ///// ///// The value of the Key Vault secret. ///// public string SecretValue { get; set; } /// - /// The cache expiration time for the Key Vault secret. + /// The time when this secret should be reloaded from Key Vault. /// - public DateTimeOffset? ExpiresOn { get; set; } - - public CachedKeyVaultSecret(string key) - { - Key = key; - } - - public override bool Equals(object obj) - { - if (obj is CachedKeyVaultSecret cachedSecret) - { - return Key == cachedSecret.Key; - } - - return false; - } + public DateTimeOffset? RefreshAt { get; set; } - public override int GetHashCode() + public CachedKeyVaultSecret(string secretValue, DateTimeOffset? refreshAt) { - return Key.GetHashCode(); + SecretValue = secretValue; + RefreshAt = refreshAt; } } } diff --git a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs index 94fdaa5c7..17679dfbb 100644 --- a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs +++ b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs @@ -22,6 +22,7 @@ namespace Tests.AzureAppConfiguration public class KeyVaultReferenceTests { string _secretValue = "SecretValue from KeyVault"; + string _certValue = "Certificate Value from KeyVault"; string _secretUri = "https://keyvault-theclassics.vault.azure.net/secrets/TheTrialSecret"; ConfigurationSetting sentinelKv = new ConfigurationSetting("Sentinel", "Value1"); @@ -134,6 +135,31 @@ public void UseSecret() Assert.Equal(_secretValue, configuration[_kv.Key]); } + [Fact] + public void UseCertificate() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(new List { _kvCertRef })); + + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(new KeyVaultSecret(name, _certValue)))); + + var configuration = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.ConfigureKeyVault(kv => kv.Register(mockSecretClient.Object)); + }) + .Build(); + + Assert.Equal(_certValue, configuration[_kvCertRef.Key]); + } + [Fact] public void UseNullSecretValueWhenSecretNotFound() { @@ -768,25 +794,138 @@ Response GetIfChanged(ConfigurationSetting setting, bool o } [Fact] - public void ThrowsWhenSecretRefreshIntervalIsLessThanMinimumInterval() + public void SecretIsReloadedFromKeyVaultWhenCacheExpires() { + IConfigurationRefresher refresher = null; + TimeSpan cacheExpirationTime = TimeSpan.FromSeconds(1); + var mockResponse = new Mock(); var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) .Returns(new MockAsyncPageable(new List { _kv })); - Assert.Throws(() => - { - new ConfigurationBuilder().AddAzureAppConfiguration(options => + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(new KeyVaultSecret(name, _secretValue)))); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => { options.Client = mockClient.Object; options.ConfigureKeyVault(kv => { - kv.SetSecretRefreshInterval(_kv.Key, TimeSpan.FromMinutes(30)); + kv.Register(mockSecretClient.Object); + kv.SetSecretRefreshInterval(_kv.Key, cacheExpirationTime); }); + + refresher = options.GetRefresher(); }) .Build(); - }); + + Assert.Equal(_secretValue, config[_kv.Key]); + + // Sleep to let the secret cache expire + Thread.Sleep(cacheExpirationTime); + refresher.RefreshAsync().Wait(); + + Assert.Equal(_secretValue, config[_kv.Key]); + + // Validate that 2 calls were made to fetch secrets from KeyVault because the secret cache had expired. + mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + } + + [Fact] + public void SecretsWithDefaultRefreshInterval() + { + IConfigurationRefresher refresher = null; + TimeSpan shortCacheExpirationTime = TimeSpan.FromSeconds(1); + + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(_kvCollectionPageOne)); + + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(new KeyVaultSecret(name, _secretValue)))); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.ConfigureKeyVault(kv => + { + kv.Register(mockSecretClient.Object); + kv.SetSecretRefreshInterval(shortCacheExpirationTime); + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal(_secretValue, config["TK1"]); + Assert.Equal(_secretValue, config["TK2"]); + + // Sleep to let the secret cache expire for both secrets + Thread.Sleep(shortCacheExpirationTime); + refresher.RefreshAsync().Wait(); + + Assert.Equal(_secretValue, config["TK1"]); + Assert.Equal(_secretValue, config["TK2"]); + + // Validate that 4 calls were made to fetch secrets from KeyVault because the secret cache had expired for both secrets. + mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(4)); + } + + [Fact] + public void SecretsWithDifferentRefreshIntervals() + { + IConfigurationRefresher refresher = null; + TimeSpan shortCacheExpirationTime = TimeSpan.FromSeconds(1); + TimeSpan longCacheExpirationTime = TimeSpan.FromDays(1); + + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(_kvCollectionPageOne)); + + var mockSecretClient = new Mock(MockBehavior.Strict); + mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); + mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, string version, CancellationToken cancellationToken) => + Task.FromResult((Response)new MockResponse(new KeyVaultSecret(name, _secretValue)))); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.ConfigureKeyVault(kv => + { + kv.Register(mockSecretClient.Object); + kv.SetSecretRefreshInterval("TK1", shortCacheExpirationTime); + kv.SetSecretRefreshInterval(longCacheExpirationTime); + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal(_secretValue, config["TK1"]); + Assert.Equal(_secretValue, config["TK2"]); + + // Sleep to let the secret cache expire for one secret + Thread.Sleep(shortCacheExpirationTime); + refresher.RefreshAsync().Wait(); + + Assert.Equal(_secretValue, config["TK1"]); + Assert.Equal(_secretValue, config["TK2"]); + + // Validate that 3 calls were made to fetch secrets from KeyVault because the secret cache had expired for only one secret. + mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(3)); } } } From bbf4a12338d58ac5d93268831319a466dc66bdb8 Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Mon, 26 Apr 2021 11:30:03 -0700 Subject: [PATCH 06/12] Track next refresh time in secret provider and add semaphore for adapter refresh --- .../AzureAppConfigurationKeyVaultOptions.cs | 5 ++ .../AzureAppConfigurationProvider.cs | 20 ++++- .../AzureKeyVaultSecretProvider.cs | 81 ++++++++++++------- .../CachedKeyVaultSecret.cs | 2 +- .../Constants/ErrorMessages.cs | 1 - 5 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs index a4e27d34a..c49395a51 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationKeyVaultOptions.cs @@ -63,6 +63,11 @@ public AzureAppConfigurationKeyVaultOptions SetSecretResolver(FuncMinimum time that must elapse before the secret is reloaded from Key Vault. public AzureAppConfigurationKeyVaultOptions SetSecretRefreshInterval(string secretReferenceKey, TimeSpan refreshInterval) { + if (string.IsNullOrEmpty(secretReferenceKey)) + { + throw new ArgumentNullException(nameof(secretReferenceKey)); + } + SecretRefreshIntervals[secretReferenceKey] = refreshInterval; return this; } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index c9d2d3a10..250e8190c 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -468,14 +468,26 @@ await TracingUtils.CallWithRequestTracing(_requestTracingEnabled, RequestType.Wa private async Task RefreshKeyValueAdapters() { - foreach (IKeyValueAdapter adapter in _options.Adapters) + if (!AdapterRefreshSemaphore.Wait(0)) + { + return; + } + + try { - if (adapter.NeedsRefresh()) + foreach (IKeyValueAdapter adapter in _options.Adapters) { - await SetData(_applicationSettings).ConfigureAwait(false); - break; + if (adapter.NeedsRefresh()) + { + await SetData(_applicationSettings).ConfigureAwait(false); + break; + } } } + finally + { + AdapterRefreshSemaphore.Release(); + } } private async Task RefreshKeyValueCollections() diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index 21e85b1a8..66a22123d 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -14,6 +14,8 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault internal class AzureKeyVaultSecretProvider { private readonly IDictionary _secretClients; + private readonly object _syncObject = new object(); + private DateTimeOffset? _nextRefreshTime = null; private ConcurrentDictionary _cachedKeyVaultSecrets = new ConcurrentDictionary(); private AzureAppConfigurationKeyVaultOptions _keyVaultOptions; @@ -44,26 +46,19 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation { KeyVaultSecret secret; - // Try to load secret value from the cache first. - secretValue = GetCachedSecretValue(key); - - if (secretValue == null) + // Use the cached value of this key vault secret if RefreshAt time is null or in the future + if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && + (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) + { + secretValue = cachedSecret.SecretValue; + } + else { // We dont have a cached secret value or the cached value has expired. // Get the secret from Key Vault and update the cache. secret = await client.GetSecretAsync(secretName, secretVersion, cancellationToken).ConfigureAwait(false); secretValue = secret?.Value; - - if (secret != null) - { - UpdateCachedKeyVaultSecrets(key, secretValue); - } - else - { - // Secret may have been deleted from KeyVault. - // Delete the secret from cache too. - RemoveSecretFromCache(key); - } + SetSecretInCache(key, secretValue); } } else if (_keyVaultOptions.SecretResolver != null) @@ -80,18 +75,46 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation public bool ShouldRefreshKeyVaultSecrets() { - // return true if the RefreshAt time of any cached secret has already elapsed. - return _cachedKeyVaultSecrets.Any(cachedSecret => cachedSecret.Value.RefreshAt.HasValue && cachedSecret.Value.RefreshAt.Value < DateTimeOffset.UtcNow); + lock (_syncObject) + { + // return true if the _nextRefreshTime has already elapsed. + return _nextRefreshTime.HasValue && _nextRefreshTime.Value < DateTimeOffset.UtcNow; + } } public void ClearCache() { _cachedKeyVaultSecrets.Clear(); + + lock (_syncObject) + { + _nextRefreshTime = null; + } } public void RemoveSecretFromCache(string key) { - _cachedKeyVaultSecrets.TryRemove(key, out CachedKeyVaultSecret _); + if (_cachedKeyVaultSecrets.TryRemove(key, out CachedKeyVaultSecret cachedSecret) && cachedSecret.RefreshAt.HasValue) + { + lock (_syncObject) + { + if (_nextRefreshTime.HasValue && _nextRefreshTime.Value == cachedSecret.RefreshAt.Value) + { + // The secret that may have controlled the next refresh time has been removed from cache. + // Find the next earliest refresh time from cached secrets ----> takes O(n) time. + DateTimeOffset? minRefreshTime = DateTimeOffset.MaxValue; + foreach (CachedKeyVaultSecret secret in _cachedKeyVaultSecrets.Values) + { + if (secret.RefreshAt.HasValue && secret.RefreshAt.Value < minRefreshTime) + { + minRefreshTime = secret.RefreshAt.Value; + } + } + + _nextRefreshTime = minRefreshTime != DateTimeOffset.MaxValue ? minRefreshTime : null; + } + } + } } private SecretClient GetSecretClient(Uri secretUri) @@ -113,7 +136,7 @@ private SecretClient GetSecretClient(Uri secretUri) return client; } - private void UpdateCachedKeyVaultSecrets(string key, string secretValue) + private void SetSecretInCache(string key, string secretValue) { // If refresh interval for this key or a default refresh interval for all keys has not been specified, // cache expiration time will be null, i.e., this secret will not be refreshed automatically. @@ -132,20 +155,18 @@ private void UpdateCachedKeyVaultSecrets(string key, string secretValue) // Add or update the cache. _cachedKeyVaultSecrets[key] = new CachedKeyVaultSecret(secretValue, refreshSecretAt); - } - private string GetCachedSecretValue(string key) - { - string cachedSecretValue = null; - - // Use the cached value of this key vault secret if RefreshAt time is null or in the future - if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && - (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) + // Update the next earliest refresh time to keep track of the next refresh operation. + if(refreshSecretAt.HasValue) { - cachedSecretValue = cachedSecret.SecretValue; + lock (_syncObject) + { + if (!_nextRefreshTime.HasValue || (_nextRefreshTime.HasValue && refreshSecretAt.Value < _nextRefreshTime.Value)) + { + _nextRefreshTime = refreshSecretAt; + } + } } - - return cachedSecretValue; } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs index 08907769f..e5737120b 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/CachedKeyVaultSecret.cs @@ -5,7 +5,7 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault { - internal struct CachedKeyVaultSecret + internal class CachedKeyVaultSecret { ///// ///// The value of the Key Vault secret. diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/ErrorMessages.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/ErrorMessages.cs index 3e83b4902..c53daa659 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/ErrorMessages.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/ErrorMessages.cs @@ -6,6 +6,5 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration internal class ErrorMessages { public const string CacheExpirationTimeTooShort = "The cache expiration time cannot be less than {0} milliseconds."; - public const string SecretRefreshIntervalTooShort = "The secret refresh interval cannot be less than {0} hour(s)."; } } From f40ff49a8d8b203eb6b01ba41caafe5038811eda Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Wed, 28 Apr 2021 11:10:07 -0700 Subject: [PATCH 07/12] Remove _nextRefreshTime and acquire semaphore only if refresh is needed Block concurrent network operations (#254) --- .../AzureAppConfigurationProvider.cs | 22 ++------ .../AzureKeyVaultSecretProvider.cs | 50 ++----------------- 2 files changed, 7 insertions(+), 65 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index 250e8190c..735376d43 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -167,7 +167,7 @@ public async Task RefreshAsync() await RefreshIndividualKeyValues().ConfigureAwait(false); await RefreshKeyValueCollections().ConfigureAwait(false); - await RefreshKeyValueCollections().ConfigureAwait(false); + await RefreshKeyValueAdapters().ConfigureAwait(false); } finally { @@ -468,25 +468,9 @@ await TracingUtils.CallWithRequestTracing(_requestTracingEnabled, RequestType.Wa private async Task RefreshKeyValueAdapters() { - if (!AdapterRefreshSemaphore.Wait(0)) - { - return; - } - - try - { - foreach (IKeyValueAdapter adapter in _options.Adapters) - { - if (adapter.NeedsRefresh()) - { - await SetData(_applicationSettings).ConfigureAwait(false); - break; - } - } - } - finally + if (_options.Adapters.Any(adapter => adapter.NeedsRefresh())) { - AdapterRefreshSemaphore.Release(); + SetData(_applicationSettings); } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index 66a22123d..b362ddc4e 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -14,8 +14,6 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault internal class AzureKeyVaultSecretProvider { private readonly IDictionary _secretClients; - private readonly object _syncObject = new object(); - private DateTimeOffset? _nextRefreshTime = null; private ConcurrentDictionary _cachedKeyVaultSecrets = new ConcurrentDictionary(); private AzureAppConfigurationKeyVaultOptions _keyVaultOptions; @@ -75,46 +73,18 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation public bool ShouldRefreshKeyVaultSecrets() { - lock (_syncObject) - { - // return true if the _nextRefreshTime has already elapsed. - return _nextRefreshTime.HasValue && _nextRefreshTime.Value < DateTimeOffset.UtcNow; - } + // return true if the _nextRefreshTime has already elapsed. + return _cachedKeyVaultSecrets.Any(cachedSecret => cachedSecret.Value.RefreshAt.HasValue && cachedSecret.Value.RefreshAt.Value < DateTimeOffset.UtcNow); } public void ClearCache() { _cachedKeyVaultSecrets.Clear(); - - lock (_syncObject) - { - _nextRefreshTime = null; - } } public void RemoveSecretFromCache(string key) { - if (_cachedKeyVaultSecrets.TryRemove(key, out CachedKeyVaultSecret cachedSecret) && cachedSecret.RefreshAt.HasValue) - { - lock (_syncObject) - { - if (_nextRefreshTime.HasValue && _nextRefreshTime.Value == cachedSecret.RefreshAt.Value) - { - // The secret that may have controlled the next refresh time has been removed from cache. - // Find the next earliest refresh time from cached secrets ----> takes O(n) time. - DateTimeOffset? minRefreshTime = DateTimeOffset.MaxValue; - foreach (CachedKeyVaultSecret secret in _cachedKeyVaultSecrets.Values) - { - if (secret.RefreshAt.HasValue && secret.RefreshAt.Value < minRefreshTime) - { - minRefreshTime = secret.RefreshAt.Value; - } - } - - _nextRefreshTime = minRefreshTime != DateTimeOffset.MaxValue ? minRefreshTime : null; - } - } - } + _cachedKeyVaultSecrets.TryRemove(key, out CachedKeyVaultSecret _); } private SecretClient GetSecretClient(Uri secretUri) @@ -153,20 +123,8 @@ private void SetSecretInCache(string key, string secretValue) refreshSecretAt = DateTimeOffset.UtcNow.Add(_keyVaultOptions.DefaultSecretRefreshInterval.Value); } - // Add or update the cache. + // Add or update the secret in cache. _cachedKeyVaultSecrets[key] = new CachedKeyVaultSecret(secretValue, refreshSecretAt); - - // Update the next earliest refresh time to keep track of the next refresh operation. - if(refreshSecretAt.HasValue) - { - lock (_syncObject) - { - if (!_nextRefreshTime.HasValue || (_nextRefreshTime.HasValue && refreshSecretAt.Value < _nextRefreshTime.Value)) - { - _nextRefreshTime = refreshSecretAt; - } - } - } } } } From e1589db863cf0f36c37d8a9fa117156f8ad03307 Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Fri, 30 Apr 2021 14:30:20 -0700 Subject: [PATCH 08/12] Updating secret provider after concurrency changes --- .../AzureKeyVaultSecretProvider.cs | 51 +++++++++++++++---- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index b362ddc4e..eef64756b 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -3,7 +3,6 @@ // using Azure.Security.KeyVault.Secrets; using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -14,7 +13,9 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault internal class AzureKeyVaultSecretProvider { private readonly IDictionary _secretClients; - private ConcurrentDictionary _cachedKeyVaultSecrets = new ConcurrentDictionary(); + private Dictionary _cachedKeyVaultSecrets = new Dictionary(); + private string _nextRefreshKey; + private DateTimeOffset? _nextRefreshTime; private AzureAppConfigurationKeyVaultOptions _keyVaultOptions; public AzureKeyVaultSecretProvider(AzureAppConfigurationKeyVaultOptions keyVaultOptions = null) @@ -44,7 +45,6 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation { KeyVaultSecret secret; - // Use the cached value of this key vault secret if RefreshAt time is null or in the future if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) { @@ -73,18 +73,24 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation public bool ShouldRefreshKeyVaultSecrets() { - // return true if the _nextRefreshTime has already elapsed. - return _cachedKeyVaultSecrets.Any(cachedSecret => cachedSecret.Value.RefreshAt.HasValue && cachedSecret.Value.RefreshAt.Value < DateTimeOffset.UtcNow); + return _nextRefreshTime.HasValue && _nextRefreshTime.Value < DateTimeOffset.UtcNow; } public void ClearCache() { _cachedKeyVaultSecrets.Clear(); + _nextRefreshKey = null; + _nextRefreshTime = null; } public void RemoveSecretFromCache(string key) { - _cachedKeyVaultSecrets.TryRemove(key, out CachedKeyVaultSecret _); + _cachedKeyVaultSecrets.Remove(key); + + if (key == _nextRefreshKey) + { + UpdateNextRefreshableSecretFromCache(); + } } private SecretClient GetSecretClient(Uri secretUri) @@ -108,23 +114,46 @@ private SecretClient GetSecretClient(Uri secretUri) private void SetSecretInCache(string key, string secretValue) { - // If refresh interval for this key or a default refresh interval for all keys has not been specified, - // cache expiration time will be null, i.e., this secret will not be refreshed automatically. DateTimeOffset? refreshSecretAt = null; if (_keyVaultOptions.SecretRefreshIntervals.TryGetValue(key, out TimeSpan refreshInterval)) { - // Set the cache expiration time using the refresh interval specified for this key. refreshSecretAt = DateTimeOffset.UtcNow.Add(refreshInterval); } else if (_keyVaultOptions.DefaultSecretRefreshInterval.HasValue) { - // Set the cache expiration time using the default refresh interval specified for all keys. refreshSecretAt = DateTimeOffset.UtcNow.Add(_keyVaultOptions.DefaultSecretRefreshInterval.Value); } - // Add or update the secret in cache. _cachedKeyVaultSecrets[key] = new CachedKeyVaultSecret(secretValue, refreshSecretAt); + + if (key == _nextRefreshKey) + { + UpdateNextRefreshableSecretFromCache(); + } + else if ((refreshSecretAt.HasValue && _nextRefreshTime.HasValue && refreshSecretAt.Value < _nextRefreshTime.Value) + || (refreshSecretAt.HasValue && !_nextRefreshTime.HasValue)) + { + _nextRefreshKey = key; + _nextRefreshTime = refreshSecretAt.Value; + } + } + + private void UpdateNextRefreshableSecretFromCache() + { + _nextRefreshKey = null; + _nextRefreshTime = DateTimeOffset.MaxValue; + + foreach (KeyValuePair secret in _cachedKeyVaultSecrets) + { + if (secret.Value.RefreshAt.HasValue && secret.Value.RefreshAt.Value < _nextRefreshTime) + { + _nextRefreshTime = secret.Value.RefreshAt; + _nextRefreshKey = secret.Key; + } + } + + _nextRefreshTime = _nextRefreshTime != DateTimeOffset.MaxValue ? _nextRefreshTime : null; } } } From c677d6ff96a42c7d666d318c0c1b7bfe3b612456 Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Mon, 3 May 2021 11:56:19 -0700 Subject: [PATCH 09/12] Cache secrets returned by SecretResolver --- .../AzureKeyVaultSecretProvider.cs | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index eef64756b..f1c55aecf 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -12,15 +12,16 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault { internal class AzureKeyVaultSecretProvider { + private readonly AzureAppConfigurationKeyVaultOptions _keyVaultOptions; private readonly IDictionary _secretClients; - private Dictionary _cachedKeyVaultSecrets = new Dictionary(); + private readonly Dictionary _cachedKeyVaultSecrets; private string _nextRefreshKey; private DateTimeOffset? _nextRefreshTime; - private AzureAppConfigurationKeyVaultOptions _keyVaultOptions; public AzureKeyVaultSecretProvider(AzureAppConfigurationKeyVaultOptions keyVaultOptions = null) { _keyVaultOptions = keyVaultOptions ?? new AzureAppConfigurationKeyVaultOptions(); + _cachedKeyVaultSecrets = new Dictionary(StringComparer.OrdinalIgnoreCase); _secretClients = new Dictionary(StringComparer.OrdinalIgnoreCase); if (_keyVaultOptions.SecretClients != null) @@ -61,7 +62,16 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation } else if (_keyVaultOptions.SecretResolver != null) { - secretValue = await _keyVaultOptions.SecretResolver(secretUri).ConfigureAwait(false); + if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && + (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) + { + secretValue = cachedSecret.SecretValue; + } + else + { + secretValue = await _keyVaultOptions.SecretResolver(secretUri).ConfigureAwait(false); + SetSecretInCache(key, secretValue); + } } else { @@ -153,7 +163,10 @@ private void UpdateNextRefreshableSecretFromCache() } } - _nextRefreshTime = _nextRefreshTime != DateTimeOffset.MaxValue ? _nextRefreshTime : null; + if (_nextRefreshTime == DateTimeOffset.MaxValue) + { + _nextRefreshTime = null; + } } } } From cd0ac91de320ba16245e7be1d6242af4ba37b9b0 Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Mon, 3 May 2021 12:12:59 -0700 Subject: [PATCH 10/12] Remove duplicate cache calculation --- .../AzureKeyVaultSecretProvider.cs | 36 ++++++------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs index f1c55aecf..4fbfc627d 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultSecretProvider.cs @@ -42,36 +42,22 @@ public async Task GetSecretValue(Uri secretUri, string key, Cancellation SecretClient client = GetSecretClient(secretUri); - if (client != null) + if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && + (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) + { + secretValue = cachedSecret.SecretValue; + } + else if (client != null) { KeyVaultSecret secret; - - if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && - (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) - { - secretValue = cachedSecret.SecretValue; - } - else - { - // We dont have a cached secret value or the cached value has expired. - // Get the secret from Key Vault and update the cache. - secret = await client.GetSecretAsync(secretName, secretVersion, cancellationToken).ConfigureAwait(false); - secretValue = secret?.Value; - SetSecretInCache(key, secretValue); - } + secret = await client.GetSecretAsync(secretName, secretVersion, cancellationToken).ConfigureAwait(false); + secretValue = secret?.Value; + SetSecretInCache(key, secretValue); } else if (_keyVaultOptions.SecretResolver != null) { - if (_cachedKeyVaultSecrets.TryGetValue(key, out CachedKeyVaultSecret cachedSecret) && - (!cachedSecret.RefreshAt.HasValue || DateTimeOffset.UtcNow < cachedSecret.RefreshAt.Value)) - { - secretValue = cachedSecret.SecretValue; - } - else - { - secretValue = await _keyVaultOptions.SecretResolver(secretUri).ConfigureAwait(false); - SetSecretInCache(key, secretValue); - } + secretValue = await _keyVaultOptions.SecretResolver(secretUri).ConfigureAwait(false); + SetSecretInCache(key, secretValue); } else { From c0a5c4cf1d71a4f9e560c62b37b1ba7b0855bc15 Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Mon, 3 May 2021 18:11:41 -0700 Subject: [PATCH 11/12] SetDirty should not clear key vault cache --- .../AzureAppConfigurationProvider.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index 735376d43..5bd9cb74f 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -207,11 +207,6 @@ public void SetDirty(TimeSpan? maxDelay) { changeWatcher.CacheExpires = cacheExpires; } - - foreach (IKeyValueAdapter adapter in _options.Adapters) - { - adapter.InvalidateCache(); - } } private async Task LoadAll(bool ignoreFailures) From fcab95e98582883ea9696a30943d123de626bcff Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Mon, 3 May 2021 19:03:59 -0700 Subject: [PATCH 12/12] Remove SetDirty unit test from Key Vault tests --- .../KeyVaultReferenceTests.cs | 77 ------------------- 1 file changed, 77 deletions(-) diff --git a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs index 17679dfbb..e41f7f85f 100644 --- a/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs +++ b/tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs @@ -716,83 +716,6 @@ Response GetIfChanged(ConfigurationSetting setting, bool o mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); } - [Fact] - public void SetDirtyForcesRefreshOfKeyVaultSecrets() - { - IConfigurationRefresher refresher = null; - TimeSpan longCacheExpirationTime = TimeSpan.FromDays(1); - - var mockResponse = new Mock(); - var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); - - Response GetTestKey(string key, string label, CancellationToken cancellationToken) - { - return Response.FromValue(TestHelpers.CloneSetting(sentinelKv), mockResponse.Object); - } - - Response GetIfChanged(ConfigurationSetting setting, bool onlyIfChanged, CancellationToken cancellationToken) - { - var unchanged = sentinelKv.Key == setting.Key && sentinelKv.Label == setting.Label && sentinelKv.Value == setting.Value; - var response = new MockResponse(unchanged ? 304 : 200); - return Response.FromValue(sentinelKv, response); - } - - mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) - .Returns(new MockAsyncPageable(new List { _kv })); - - mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync((Func>)GetTestKey); - - mockClient.Setup(c => c.GetConfigurationSettingAsync(It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync((Func>)GetIfChanged); - - var mockSecretClient = new Mock(MockBehavior.Strict); - mockSecretClient.SetupGet(client => client.VaultUri).Returns(new Uri("https://keyvault-theclassics.vault.azure.net")); - mockSecretClient.Setup(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny())) - .Returns((string name, string version, CancellationToken cancellationToken) => - Task.FromResult((Response)new MockResponse(new KeyVaultSecret(name, _secretValue)))); - - var config = new ConfigurationBuilder() - .AddAzureAppConfiguration(options => - { - options.Client = mockClient.Object; - options.ConfigureKeyVault(kv => - { - kv.Register(mockSecretClient.Object); - kv.SetSecretRefreshInterval(_kv.Key, longCacheExpirationTime); - }); - - options.ConfigureRefresh(refreshOptions => - { - refreshOptions.Register("Sentinel") - .SetCacheExpiration(longCacheExpirationTime); - }); - - refresher = options.GetRefresher(); - }) - .Build(); - - Assert.Equal("Value1", config["Sentinel"]); - Assert.Equal(_secretValue, config[_kv.Key]); - - // Update sentinel key-value - sentinelKv.Value = "Value2"; - refresher.SetDirty(TimeSpan.FromSeconds(1)); - - // Wait for the cache to expire based on the randomized delay in SetDirty() - Thread.Sleep(1200); - refresher.RefreshAsync().Wait(); - - Assert.Equal("Value2", config["Sentinel"]); - Assert.Equal(_secretValue, config[_kv.Key]); - - // Validate that 2 calls were made to fetch secrets from KeyVault - // Even though Key Vault refresh interval has not elapsed, SetDirty should invalidate the cached secret and get secret from Key Vault again - mockSecretClient.Verify(client => client.GetSecretAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); - - - } - [Fact] public void SecretIsReloadedFromKeyVaultWhenCacheExpires() {