Improve sanitized secrets definition (#14284)

sdk/appconfiguration/Azure.Data.AppConfiguration/tests/AppConfigurationTestEnvironment.cs

@@ -11,7 +11,7 @@ public AppConfigurationTestEnvironment() : base("appconfiguration")
         {
         }
 
-        public string ConnectionString => GetRecordedVariable("APPCONFIGURATION_CONNECTION_STRING");
+        public string ConnectionString => GetRecordedVariable("APPCONFIGURATION_CONNECTION_STRING", options => options.HasSecretConnectionStringParameter("secret"));
         public string Endpoint => GetRecordedVariable("APPCONFIGURATION_ENDPOINT_STRING");
     }
 }

sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationLiveTests.cs

@@ -18,7 +18,6 @@ public class ConfigurationLiveTests : RecordedTestBase<AppConfigurationTestEnvir
 
         public ConfigurationLiveTests(bool isAsync) : base(isAsync)
         {
-            Sanitizer = new ConfigurationRecordedTestSanitizer();
             Matcher = new ConfigurationRecordMatcher();
         }
 

sdk/appconfiguration/CONTRIBUTING.md

@@ -75,5 +75,4 @@ For additional insight and context, the development, release, and issue history
 [sdk_design_guidelines]: https://azure.github.io/azure-sdk/general_introduction.html
 [sdk_design_guidelines_dotnet]: https://azure.github.io/azure-sdk/dotnet_introduction.html
 [sdk_dotnet_code_readme]: ../../sdk/core/Azure.Core/README.md
-[tests_sanitized]: ./Azure.Data.AppConfiguration/tests/ConfigurationRecordedTestSanitizer.cs
 [email_opencode]: mailto:opencode@microsoft.com

sdk/core/Azure.Core.TestFramework/README.md

@@ -69,7 +69,7 @@ public class AppConfigurationTestEnvironment : TestEnvironment
 
     // Variables retrieved using GetRecordedVariable will be recorded in recorded tests
     // Argument is the output name in the test-resources.json
-    public string ConnectionString => GetRecordedVariable("APPCONFIGURATION_CONNECTION_STRING");
+    public string Endpoint => GetRecordedVariable("APPCONFIGURATION_ENDPOINT");
     // Variables retrieved using GetVariable will not be recorded but the method will throw if the variable is not set
     public string SystemAssignedVault => GetVariable("IDENTITYTEST_IMDSTEST_SYSTEMASSIGNEDVAULT");
     // Variables retrieved using GetOptionalVariable will not be recorded and the method will return null if variable is not set
@@ -79,6 +79,24 @@ public class AppConfigurationTestEnvironment : TestEnvironment
 
 **NOTE:** Make sure that variables containing secret values are not recorded or are sanitized.
 
+To sanitize variables use the `options` parameter of `GetRecordedVariable`:
+
+``` C#
+    // HasSecretConnectionStringParameter would ensure the right connection string parameter is sanitized before storing the record
+    public string ConnectionString => GetRecordedVariable("APPCONFIGURATION_CONNECTION_STRING", options => options.HasSecretConnectionStringParameter("secret"));
+    // IsSecret would ensure the entire value is sanitized before storage
+    public string Key => GetRecordedVariable("APPCONFIGURATION_KEY", options => options.IsSecret());
+```
+
+If the client expects a Base64 secret value use the `SanitizedValue` parameter to use a Base64 compatible replacement value:
+
+``` C#
+    // Connection string parameter would be replaced with Kg==
+    public string ConnectionString => GetRecordedVariable("APPCONFIGURATION_CONNECTION_STRING", options => options.HasSecretConnectionStringParameter("secret", SanitizedValue.Base64));
+    // Secret value would be replaced with Kg==
+    public string Key => GetRecordedVariable("APPCONFIGURATION_KEY", options => options.IsSecret(SanitizedValue.Base64));
+```
+
 You can now retrieve these values in tests:
 
 ``` C#
@@ -192,59 +210,30 @@ For example:
 ``` C#
     public class ConfigurationRecordedTestSanitizer : RecordedTestSanitizer
     {
-        public override string SanitizeVariable(string variableName, string environmentVariableValue) =>
-            variableName switch
-            {
-                "APPCONFIGURATION_CONNECTION_STRING" => SanitizeConnectionString(environmentVariableValue),
-                _ => base.SanitizeVariable(variableName, environmentVariableValue)
-            };
-
-        private static string SanitizeConnectionString(string connectionString)
+        public ConfigurationRecordedTestSanitizer()
         {
-            const string secretKey = "secret";
-            var parsed = ConnectionString.Parse(connectionString, allowEmptyValues: true);
+            // Add headers that might contain secrets
+            SanitizedHeaders.Add("My-Custom-Auth-Header");
+        }
 
-            // Configuration client expects secret to be base64 encoded so we can't use the placeholder
-            parsed.Replace(secretKey, string.Empty);
-            return parsed.ToString();
+        public override string SanitizeUri(string uri)
+        {
+            // sanitize url
         }
     }
 ```
-The above example also illustrates a common pattern where you need to sanitize the connection string, but you must ensure that the account key is base64 encoded or else the test will fail your connection string validation when run in Playback mode. In such cases, you need to override the default placeholder that is used to ensure the value is base64.
 
 Another sanitizer property that is available for sanitizing Json payloads is the `JsonPathSanitizers`. This property contains a list of [Json Path](https://www.newtonsoft.com/json/help/html/QueryJsonSelectToken.htm) format strings that will be validated against the body. If a match exists, the value will be sanitized.
 
 ```c#
     public class FormRecognizerRecordedTestSanitizer : RecordedTestSanitizer
     {
-        private const string SanitizedSasUri = "https://sanitized.blob.core.windows.net";
-
         public FormRecognizerRecordedTestSanitizer()
             : base()
         {
             JsonPathSanitizers.Add("$..accessToken");
             JsonPathSanitizers.Add("$..source");
         }
-
-        public override void SanitizeHeaders(IDictionary<string, string[]> headers)
-        {
-            if (headers.ContainsKey(Constants.AuthorizationHeader))
-            {
-                headers[Constants.AuthorizationHeader] = new[] { SanitizeValue };
-            }
-
-            base.SanitizeHeaders(headers);
-        }
-
-        public override string SanitizeVariable(string variableName, string environmentVariableValue)
-        {
-            return variableName switch
-            {
-                FormRecognizerTestEnvironment.ApiKeyEnvironmentVariableName => SanitizeValue,
-                FormRecognizerTestEnvironment.BlobContainerSasUrlEnvironmentVariableName => SanitizedSasUri,
-                _ => base.SanitizeVariable(variableName, environmentVariableValue)
-            };
-        }
     }
 ```
 

sdk/core/Azure.Core.TestFramework/src/Azure.Core.TestFramework.csproj

@@ -16,5 +16,6 @@
   <ItemGroup>
     <Compile Include="$(AzureCoreSharedSources)EventSourceEventFormatting.cs" Link="Shared\%(RecursiveDir)\%(Filename)%(Extension)" />
     <Compile Include="$(AzureCoreSharedSources)ContentTypeUtilities.cs" Link="Shared\%(RecursiveDir)\%(Filename)%(Extension)" />
+    <Compile Include="$(AzureCoreSharedSources)ConnectionString.cs" Link="Shared\%(RecursiveDir)\%(Filename)%(Extension)" />
   </ItemGroup>
 </Project>

sdk/core/Azure.Core.TestFramework/src/RecordedVariableOptions.cs

@@ -0,0 +1,69 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Collections.Generic;
+using System.Linq;
+
+namespace Azure.Core.TestFramework
+{
+    public class RecordedVariableOptions
+    {
+        private readonly Dictionary<string, string> _secretConnectionStringParameters = new Dictionary<string, string>();
+        private string _sanitizedValue;
+
+        public RecordedVariableOptions HasSecretConnectionStringParameter(string name, SanitizedValue sanitizedValue = default)
+        {
+            _secretConnectionStringParameters[name] = GetStringValue(sanitizedValue);
+            return this;
+        }
+
+        public RecordedVariableOptions HasSecretConnectionStringParameter(string name, string sanitizedValue)
+        {
+            _secretConnectionStringParameters[name] = sanitizedValue;
+            return this;
+        }
+
+        public RecordedVariableOptions IsSecret(SanitizedValue sanitizedValue = default)
+        {
+            _sanitizedValue = GetStringValue(sanitizedValue);
+            return this;
+        }
+
+        public RecordedVariableOptions IsSecret(string sanitizedValue)
+        {
+            _sanitizedValue = sanitizedValue;
+            return this;
+        }
+
+        private string GetStringValue(SanitizedValue sanitizedValue)
+        {
+            return sanitizedValue switch
+            {
+                SanitizedValue.Base64 => "Kg==",
+                _ => RecordedTestSanitizer.SanitizeValue,
+            };
+        }
+
+        public string Apply(string value)
+        {
+            if (_secretConnectionStringParameters.Any())
+            {
+                var parsed = ConnectionString.Parse(value, allowEmptyValues: true);
+
+                foreach (var connectionStringParameter in _secretConnectionStringParameters)
+                {
+                    parsed.Replace(connectionStringParameter.Key, connectionStringParameter.Value);
+                }
+
+                value = parsed.ToString();
+            }
+
+            if (_sanitizedValue != null)
+            {
+                value = _sanitizedValue;
+            }
+
+            return value;
+        }
+    }
+}

sdk/core/Azure.Core.TestFramework/src/SanitizedValue.cs

@@ -0,0 +1,11 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+namespace Azure.Core.TestFramework
+{
+    public enum SanitizedValue
+    {
+        Default,
+        Base64
+    }
+}

sdk/core/Azure.Core.TestFramework/src/TestEnvironment.cs

@@ -74,7 +74,7 @@ static TestEnvironment()
             RepositoryRoot = directoryInfo?.Parent?.FullName;
         }
 
-        internal RecordedTestMode? Mode { get; set; }
+        public RecordedTestMode? Mode { get; set; }
 
         /// <summary>
         ///   The name of the Azure subscription containing the resource group to be used for Live tests. Recorded.
@@ -161,6 +161,14 @@ public TokenCredential Credential
         /// Returns and records an environment variable value when running live or recorded value during playback.
         /// </summary>
         protected string GetRecordedOptionalVariable(string name)
+        {
+            return GetRecordedOptionalVariable(name, _ => { });
+        }
+
+        /// <summary>
+        /// Returns and records an environment variable value when running live or recorded value during playback.
+        /// </summary>
+        protected string GetRecordedOptionalVariable(string name, Action<RecordedVariableOptions> options)
         {
             if (Mode == RecordedTestMode.Playback)
             {
@@ -169,8 +177,21 @@ protected string GetRecordedOptionalVariable(string name)
 
             string value = GetOptionalVariable(name);
 
-            SetRecordedValue(name, value);
+            var optionsInstance = new RecordedVariableOptions();
+            options?.Invoke(optionsInstance);
+            var sanitizedValue = optionsInstance.Apply(value);
 
+            if (!Mode.HasValue)
+            {
+                return value;
+            }
+
+            if (_recording == null)
+            {
+                throw new InvalidOperationException("Recorded value should not be set outside the test method invocation");
+            }
+
+            _recording?.SetVariable(name, sanitizedValue);
             return value;
         }
 
@@ -180,7 +201,16 @@ protected string GetRecordedOptionalVariable(string name)
         /// </summary>
         protected string GetRecordedVariable(string name)
         {
-            var value = GetRecordedOptionalVariable(name);
+            return GetRecordedVariable(name, null);
+        }
+
+        /// <summary>
+        /// Returns and records an environment variable value when running live or recorded value during playback.
+        /// Throws when variable is not found.
+        /// </summary>
+        protected string GetRecordedVariable(string name, Action<RecordedVariableOptions> options)
+        {
+            var value = GetRecordedOptionalVariable(name, options);
             EnsureValue(name, value);
             return value;
         }
@@ -247,21 +277,6 @@ private string GetRecordedValue(string name)
             return _recording.GetVariable(name, null);
         }
 
-        private void SetRecordedValue(string name, string value)
-        {
-            if (!Mode.HasValue)
-            {
-                return;
-            }
-
-            if (_recording == null)
-            {
-                throw new InvalidOperationException("Recorded value should not be set outside the test method invocation");
-            }
-
-            _recording?.SetVariable(name, value);
-        }
-
         private class TestCredential : TokenCredential
         {
             public override ValueTask<AccessToken> GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken)

sdk/core/Azure.Core/tests/TestEnvironmentTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.IO;
 using Azure.Core.TestFramework;
 using NUnit.Framework;
 
@@ -14,6 +15,11 @@ public void SetUp()
         {
             Environment.SetEnvironmentVariable("CORE_RECORDED", "1");
             Environment.SetEnvironmentVariable("CORE_NOTRECORDED", "2");
+
+            Environment.SetEnvironmentVariable("CORE_Base64Secret", "1");
+            Environment.SetEnvironmentVariable("CORE_CustomSecret", "1");
+            Environment.SetEnvironmentVariable("CORE_DefaultSecret", "1");
+            Environment.SetEnvironmentVariable("CORE_ConnectionStringWithSecret", "endpoint=1;key=2");
         }
 
         [Theory]
@@ -49,6 +55,30 @@ public void ReadingNonRecordedValueOutsideRecordedTestWorks()
             Assert.AreEqual("2", MockTestEnvironment.Instance.NotRecordedValue);
         }
 
+        [Test]
+        public void RecordedVariableSanitized()
+        {
+            var tempFile = Path.GetTempFileName();
+            var env = new MockTestEnvironment();
+            var testRecording = new TestRecording(RecordedTestMode.Record, tempFile, new RecordedTestSanitizer(), new RecordMatcher());
+            env.Mode = RecordedTestMode.Record;
+            env.SetRecording(testRecording);
+
+            Assert.AreEqual("1", env.Base64Secret);
+            Assert.AreEqual("1", env.CustomSecret);
+            Assert.AreEqual("1", env.DefaultSecret);
+            Assert.AreEqual("endpoint=1;key=2", env.ConnectionStringWithSecret);
+
+            testRecording.Dispose();
+
+            testRecording = new TestRecording(RecordedTestMode.Playback, tempFile, new RecordedTestSanitizer(), new RecordMatcher());
+
+            Assert.AreEqual("Kg==", testRecording.GetVariable("Base64Secret", ""));
+            Assert.AreEqual("Custom", testRecording.GetVariable("CustomSecret", ""));
+            Assert.AreEqual("Sanitized", testRecording.GetVariable("DefaultSecret", ""));
+            Assert.AreEqual("endpoint=1;key=Sanitized", testRecording.GetVariable("ConnectionStringWithSecret", ""));
+        }
+
         private class RecordedVariableMisuse : RecordedTestBase<MockTestEnvironment>
         {
             // To make NUnit happy
@@ -81,6 +111,11 @@ public MockTestEnvironment(): base("core")
             public string RecordedValue => GetRecordedVariable("RECORDED");
             public string NotRecordedValue => GetVariable("NOTRECORDED");
 
+            public string Base64Secret => GetRecordedVariable("Base64Secret", option => option.IsSecret(SanitizedValue.Base64));
+            public string DefaultSecret => GetRecordedVariable("DefaultSecret", option => option.IsSecret(SanitizedValue.Default));
+            public string CustomSecret => GetRecordedVariable("CustomSecret", option => option.IsSecret("Custom"));
+            public string ConnectionStringWithSecret => GetRecordedVariable("ConnectionStringWithSecret", option => option.HasSecretConnectionStringParameter("key"));
+
         }
     }
 }