Add RefreshableSecretReader to allow background thread secret refresh (#304)

Progress on NuGet/NuGetGallery#7337

Author: joelverhagen

src/NuGet.Services.KeyVault/EmptySecretReader.cs

@@ -1,7 +1,6 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System;
 using System.Threading.Tasks;
 
 namespace NuGet.Services.KeyVault

src/NuGet.Services.KeyVault/IRefreshableSecretReaderFactory.cs

@@ -0,0 +1,22 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace NuGet.Services.KeyVault
+{
+    /// <summary>
+    /// An interface that allows caching and refreshing the secrets fetched by secret readers.
+    /// </summary>
+    public interface IRefreshableSecretReaderFactory : ISecretReaderFactory
+    {
+        /// <summary>
+        /// Refresh the values of the secrets that have already been read and cached. Since the cache is shared between
+        /// all <see cref="ISecretReader"/> instances creates, this refresh applies to all secret readers created by
+        /// this factory.
+        /// </summary>
+        /// <param name="token">A cancellation token.</param>
+        Task RefreshAsync(CancellationToken token);
+    }
+}

src/NuGet.Services.KeyVault/ISecret.cs

@@ -2,10 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. 
 
 using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 
 namespace NuGet.Services.KeyVault
 {

src/NuGet.Services.KeyVault/ISecretReader.cs

@@ -1,7 +1,6 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System;
 using System.Threading.Tasks;
 
 namespace NuGet.Services.KeyVault

src/NuGet.Services.KeyVault/KeyVaultSecret.cs

@@ -2,10 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. 
 
 using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 
 namespace NuGet.Services.KeyVault
 {

src/NuGet.Services.KeyVault/NuGet.Services.KeyVault.csproj

@@ -53,6 +53,7 @@
     <Compile Include="CachingSecretReader.cs" />
     <Compile Include="CachingSecretReaderFactory.cs" />
     <Compile Include="CertificateUtility.cs" />
+    <Compile Include="IRefreshableSecretReaderFactory.cs" />
     <Compile Include="ISecret.cs" />
     <Compile Include="ISecretReaderFactory.cs" />
     <Compile Include="EmptySecretReader.cs" />
@@ -61,6 +62,9 @@
     <Compile Include="KeyVaultConfiguration.cs" />
     <Compile Include="KeyVaultReader.cs" />
     <Compile Include="KeyVaultSecret.cs" />
+    <Compile Include="RefreshableSecretReader.cs" />
+    <Compile Include="RefreshableSecretReaderFactory.cs" />
+    <Compile Include="RefreshableSecretReaderSettings.cs" />
     <Compile Include="SecretInjector.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Properties\AssemblyInfo.*.cs" />

src/NuGet.Services.KeyVault/RefreshableSecretReader.cs

@@ -0,0 +1,96 @@
+// Copyright (c) .NET Foundation. All rights reserved. 
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. 
+
+using System;
+using System.Collections.Concurrent;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace NuGet.Services.KeyVault
+{
+    /// <summary>
+    /// A secret reader that separates the refreshing of secret values from the reading of them from an in-memory
+    /// cache. Although the <see cref="GetSecretAsync(string)"/> and <see cref="GetSecretObjectAsync(string)"/> methods
+    /// are asynchronous in definition they do not result an any asynchronous operations when a secret has already been
+    /// cached with a previous invocation. The <see cref="RefreshAsync"/> method is used to refresh the values of
+    /// secrets that have already been cached.
+    /// </summary>
+    public class RefreshableSecretReader : ISecretReader
+    {
+        private readonly ISecretReader _secretReader;
+        private readonly ConcurrentDictionary<string, ISecret> _cache;
+        private readonly RefreshableSecretReaderSettings _settings;
+
+        public RefreshableSecretReader(
+            ISecretReader secretReader,
+            ConcurrentDictionary<string, ISecret> cache,
+            RefreshableSecretReaderSettings settings)
+        {
+            _secretReader = secretReader ?? throw new ArgumentNullException(nameof(secretReader));
+            _cache = cache ?? throw new ArgumentNullException(nameof(cache));
+            _settings = settings ?? throw new ArgumentNullException(nameof(settings));
+        }
+
+        public async Task RefreshAsync(CancellationToken token)
+        {
+            foreach (var secretName in _cache.Keys)
+            {
+                if (token.IsCancellationRequested)
+                {
+                    return;
+                }
+
+                await UncachedGetSecretObjectAsync(secretName);
+            }
+        }
+
+        public Task<string> GetSecretAsync(string secretName)
+        {
+            if (TryGetCachedSecretObject(secretName, out var secret))
+            {
+                return Task.FromResult(secret.Value);
+            }
+
+            return UncachedGetSecretAsync(secretName);
+        }
+
+        public Task<ISecret> GetSecretObjectAsync(string secretName)
+        {
+            if (TryGetCachedSecretObject(secretName, out var secret))
+            {
+                return Task.FromResult(secret);
+            }
+
+            return UncachedGetSecretObjectAsync(secretName);
+        }
+
+        private async Task<string> UncachedGetSecretAsync(string secretName)
+        {
+            var secretObject = await UncachedGetSecretObjectAsync(secretName);
+            return secretObject.Value;
+        }
+
+        private async Task<ISecret> UncachedGetSecretObjectAsync(string secretName)
+        {
+            var secretObject = await _secretReader.GetSecretObjectAsync(secretName);
+            _cache.AddOrUpdate(secretName, secretObject, (_, __) => secretObject);
+            return secretObject;
+        }
+
+        private bool TryGetCachedSecretObject(string secretName, out ISecret secret)
+        {
+            if (_cache.TryGetValue(secretName, out secret))
+            {
+                return true;
+            }
+
+            if (_settings.BlockUncachedReads)
+            {
+                throw new InvalidOperationException($"The secret '{secretName}' is not cached.");
+            }
+
+            secret = null;
+            return false;
+        }
+    }
+}

src/NuGet.Services.KeyVault/RefreshableSecretReaderFactory.cs

@@ -0,0 +1,49 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Concurrent;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace NuGet.Services.KeyVault
+{
+    /// <summary>
+    /// Wraps existing secret reader factory to provide a caching layer where the cache refresh is controlled by
+    /// the <see cref="RefreshAsync"/> method on the factory created <see cref="ISecretReader"/> instances.
+    /// </summary>
+    public class RefreshableSecretReaderFactory : IRefreshableSecretReaderFactory
+    {
+        private readonly ISecretReaderFactory _underlyingFactory;
+        private readonly ConcurrentDictionary<string, ISecret> _cache;
+        private readonly RefreshableSecretReaderSettings _settings;
+
+        public RefreshableSecretReaderFactory(ISecretReaderFactory underlyingFactory, RefreshableSecretReaderSettings settings)
+        {
+            _underlyingFactory = underlyingFactory ?? throw new ArgumentNullException(nameof(underlyingFactory));
+            _cache = new ConcurrentDictionary<string, ISecret>();
+            _settings = settings ?? throw new ArgumentNullException(nameof(settings));
+        }
+
+        public async Task RefreshAsync(CancellationToken token)
+        {
+            await GetRefreshableSecretReader().RefreshAsync(token);
+        }
+
+        public ISecretInjector CreateSecretInjector(ISecretReader secretReader)
+        {
+            return _underlyingFactory.CreateSecretInjector(secretReader);
+        }
+
+        public ISecretReader CreateSecretReader()
+        {
+            return GetRefreshableSecretReader();
+        }
+
+        private RefreshableSecretReader GetRefreshableSecretReader()
+        {
+            var innerSecretReader = _underlyingFactory.CreateSecretReader();
+            return new RefreshableSecretReader(innerSecretReader, _cache, _settings);
+        }
+    }
+}

src/NuGet.Services.KeyVault/RefreshableSecretReaderSettings.cs

@@ -0,0 +1,26 @@
+// Copyright (c) .NET Foundation. All rights reserved. 
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. 
+
+using System;
+
+namespace NuGet.Services.KeyVault
+{
+    /// <summary>
+    /// The purpose of this class is to allow a <see cref="RefreshableSecretReaderFactory"/> to dynamically control the
+    /// settings of the <see cref="RefreshableSecretReader"/> instance that it creates. This does not follow the 
+    /// Microsoft.Extensions.Options (e.g. IOptionsSnapshot, IOptions) pattern because this is meant to initialized and
+    /// modified at runtime.
+    /// </summary>
+    public class RefreshableSecretReaderSettings
+    {
+        /// <summary>
+        /// Prevent <see cref="RefreshableSecretReader.GetSecretAsync(string)"/> or
+        /// <see cref="RefreshableSecretReader.GetSecretObjectAsync(string)"/> from getting secrets from the underlying
+        /// secret reader. If one of these methods is executed and the provided secret is not found, an
+        /// <see cref="InvalidOperationException"/> will be thrown. In a web application, this should be enabled during
+        /// startup so that requests encounter an exception instead of reading a secret from KeyVault in a context that
+        /// may cause a deadlock. It's better to throw an exception than deadlock.
+        /// </summary>
+        public bool BlockUncachedReads { get; set; }
+    }
+}

tests/NuGet.Services.KeyVault.Tests/NuGet.Services.KeyVault.Tests.csproj

@@ -41,6 +41,8 @@
     <Compile Include="CachingSecretReaderFacts.cs" />
     <Compile Include="KeyVaultReaderFormatterFacts.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
+    <Compile Include="RefreshableSecretReaderFactoryFacts.cs" />
+    <Compile Include="RefreshableSecretReaderFacts.cs" />
     <Compile Include="SecretReaderFacts.cs" />
   </ItemGroup>
   <ItemGroup>