Merge identity/1.10.2-rel back to main (Azure#39331)

* Bug fixes for development time credentials * update project version * Increment package version after release of Azure.Identity (Azure#39170) * tests --------- Co-authored-by: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com>
matthohn-msft · Oct 27, 2023 · 5cfd3f6 · 5cfd3f6
1 parent 8c5dfc5
commit 5cfd3f6
Show file tree

Hide file tree

Showing 11 changed files with 189 additions and 7 deletions.
diff --git a/sdk/identity/Azure.Identity/CHANGELOG.md b/sdk/identity/Azure.Identity/CHANGELOG.md
@@ -12,6 +12,13 @@
 ### Other Changes
 - Updated Microsoft.Identity.Client dependency to version 4.56.0
 
+## 1.10.2 (2023-10-10)
+
+### Bugs Fixed
+
+- Bug fixes for development time credentials.
+
+
 ## 1.10.1 (2023-09-12)
 
 ### Bugs Fixed

diff --git a/sdk/identity/Azure.Identity/src/Azure.Identity.csproj b/sdk/identity/Azure.Identity/src/Azure.Identity.csproj
@@ -4,7 +4,7 @@
     <AssemblyTitle>Microsoft Azure.Identity Component</AssemblyTitle>
     <Version>1.11.0-beta.1</Version>
     <!--The ApiCompatVersion is managed automatically and should not generally be modified manually.-->
-    <ApiCompatVersion>1.10.1</ApiCompatVersion>
+    <ApiCompatVersion>1.10.2</ApiCompatVersion>
     <PackageTags>Microsoft Azure Identity;$(PackageCommonTags)</PackageTags>
     <TargetFrameworks>$(RequiredTargetFrameworks)</TargetFrameworks>
     <NoWarn>$(NoWarn);3021;AZC0011</NoWarn>

diff --git a/sdk/identity/Azure.Identity/src/Credentials/AzureCliCredential.cs b/sdk/identity/Azure.Identity/src/Credentials/AzureCliCredential.cs
@@ -73,7 +73,7 @@ internal AzureCliCredential(CredentialPipeline pipeline, IProcessService process
             _pipeline = pipeline;
             _path = !string.IsNullOrEmpty(EnvironmentVariables.Path) ? EnvironmentVariables.Path : DefaultPath;
             _processService = processService ?? ProcessService.Default;
-            TenantId = options?.TenantId;
+            TenantId = Validations.ValidateTenantId(options?.TenantId, $"{nameof(options)}.{nameof(options.TenantId)}", true);
             AdditionallyAllowedTenantIds = TenantIdResolver.ResolveAddionallyAllowedTenantIds((options as ISupportsAdditionallyAllowedTenants)?.AdditionallyAllowedTenants);
             ProcessTimeout = options?.ProcessTimeout ?? TimeSpan.FromSeconds(13);
             _isChainedCredential = options?.IsChainedCredential ?? false;
@@ -121,6 +121,7 @@ private async ValueTask<AccessToken> RequestCliAccessTokenAsync(bool async, Toke
             string resource = ScopeUtilities.ScopesToResource(context.Scopes);
             string tenantId = TenantIdResolver.Resolve(TenantId, context, AdditionallyAllowedTenantIds);
 
+            Validations.ValidateTenantId(tenantId, nameof(context.TenantId), true);
             ScopeUtilities.ValidateScope(resource);
 
             GetFileNameAndArguments(resource, tenantId, out string fileName, out string argument);

diff --git a/sdk/identity/Azure.Identity/src/Credentials/AzureDeveloperCliCredential.cs b/sdk/identity/Azure.Identity/src/Credentials/AzureDeveloperCliCredential.cs
@@ -65,7 +65,7 @@ internal AzureDeveloperCliCredential(CredentialPipeline pipeline, IProcessServic
             _logAccountDetails = options?.Diagnostics?.IsAccountIdentifierLoggingEnabled ?? false;
             _pipeline = pipeline;
             _processService = processService ?? ProcessService.Default;
-            TenantId = options?.TenantId;
+            TenantId = Validations.ValidateTenantId(options?.TenantId, $"{nameof(options)}.{nameof(options.TenantId)}", true);
             AdditionallyAllowedTenantIds = TenantIdResolver.ResolveAddionallyAllowedTenantIds((options as ISupportsAdditionallyAllowedTenants)?.AdditionallyAllowedTenants);
             ProcessTimeout = options?.ProcessTimeout ?? TimeSpan.FromSeconds(13);
             _isChainedCredential = options?.IsChainedCredential ?? false;
@@ -112,6 +112,13 @@ private async ValueTask<AccessToken> RequestCliAccessTokenAsync(bool async, Toke
         {
             string tenantId = TenantIdResolver.Resolve(TenantId, context, AdditionallyAllowedTenantIds);
 
+            Validations.ValidateTenantId(tenantId, nameof(context.TenantId), true);
+
+            foreach (var scope in context.Scopes)
+            {
+                ScopeUtilities.ValidateScope(scope);
+            }
+
             GetFileNameAndArguments(context.Scopes, tenantId, out string fileName, out string argument);
             ProcessStartInfo processStartInfo = GetAzureDeveloperCliProcessStartInfo(fileName, argument);
             using var processRunner = new ProcessRunner(_processService.Create(processStartInfo), ProcessTimeout, _logPII, cancellationToken);

diff --git a/sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs b/sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs
@@ -63,7 +63,7 @@ internal AzurePowerShellCredential(AzurePowerShellCredentialOptions options, Cre
             UseLegacyPowerShell = false;
             _logPII = options?.IsUnsafeSupportLoggingEnabled ?? false;
             _logAccountDetails = options?.Diagnostics?.IsAccountIdentifierLoggingEnabled ?? false;
-            TenantId = options?.TenantId;
+            TenantId = Validations.ValidateTenantId(options?.TenantId, $"{nameof(options)}.{nameof(options.TenantId)}", true);
             _pipeline = pipeline ?? CredentialPipeline.GetInstance(options);
             _processService = processService ?? ProcessService.Default;
             AdditionallyAllowedTenantIds = TenantIdResolver.ResolveAddionallyAllowedTenantIds((options as ISupportsAdditionallyAllowedTenants)?.AdditionallyAllowedTenants);
@@ -139,9 +139,12 @@ private async ValueTask<AccessToken> RequestAzurePowerShellAccessTokenAsync(bool
         {
             string resource = ScopeUtilities.ScopesToResource(context.Scopes);
 
-            ScopeUtilities.ValidateScope(resource);
             var tenantId = TenantIdResolver.Resolve(TenantId, context, AdditionallyAllowedTenantIds);
 
+            Validations.ValidateTenantId(tenantId, nameof(context.TenantId), true);
+
+            ScopeUtilities.ValidateScope(resource);
+
             GetFileNameAndArguments(resource, tenantId, out string fileName, out string argument);
             ProcessStartInfo processStartInfo = GetAzurePowerShellProcessStartInfo(fileName, argument);
             using var processRunner = new ProcessRunner(

diff --git a/sdk/identity/Azure.Identity/src/ScopeUtilities.cs b/sdk/identity/Azure.Identity/src/ScopeUtilities.cs
@@ -15,7 +15,7 @@ internal static class ScopeUtilities
         private const string DefaultSuffix = "/.default";
         private const string ScopePattern = "^[0-9a-zA-Z-_.:/]+$";
 
-        private const string InvalidScopeMessage = "The specified scope is not in expected format. Only alphanumeric characters, '.', '-', ':', '_', and '/' are allowed";
+        internal const string InvalidScopeMessage = "The specified scope is not in expected format. Only alphanumeric characters, '.', '-', ':', '_', and '/' are allowed";
         private static readonly Regex scopeRegex = new Regex(ScopePattern);
 
         public static string ScopesToResource(string[] scopes)

diff --git a/sdk/identity/Azure.Identity/src/Validations.cs b/sdk/identity/Azure.Identity/src/Validations.cs
@@ -8,7 +8,7 @@ namespace Azure.Identity
 {
     internal static class Validations
     {
-        private const string InvalidTenantIdErrorMessage = "Invalid tenant id provided. You can locate your tenant id by following the instructions listed here: https://docs.microsoft.com/partner-center/find-ids-and-domain-names";
+        internal const string InvalidTenantIdErrorMessage = "Invalid tenant id provided. You can locate your tenant id by following the instructions listed here: https://docs.microsoft.com/partner-center/find-ids-and-domain-names";
 
         private const string NullTenantIdErrorMessage = "Tenant id cannot be null. You can locate your tenant id by following the instructions listed here: https://docs.microsoft.com/partner-center/find-ids-and-domain-names";
 

diff --git a/sdk/identity/Azure.Identity/tests/AzureCliCredentialTests.cs b/sdk/identity/Azure.Identity/tests/AzureCliCredentialTests.cs
@@ -3,6 +3,8 @@
 
 using System;
 using System.Collections.Generic;
+using System.Runtime.CompilerServices;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Azure.Core;
@@ -148,5 +150,58 @@ public void ConfigureCliProcessTimeout_ProcessTimeout([Values(true, false)] bool
             }
             Assert.AreEqual(AzureCliCredential.AzureCliTimeoutError, ex.Message);
         }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyCtorTenantIdValidation(char testChar)
+        {
+            string tenantId = Guid.NewGuid().ToString();
+
+            for (int i = 0; i < tenantId.Length; i++)
+            {
+                StringBuilder tenantIdBuilder = new StringBuilder(tenantId);
+
+                tenantIdBuilder.Insert(i, testChar);
+
+                Assert.Throws<ArgumentException>(() => new AzureCliCredential(new AzureCliCredentialOptions { TenantId = tenantIdBuilder.ToString() }), Validations.InvalidTenantIdErrorMessage);
+            }
+        }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyGetTokenTenantIdValidation(char testChar)
+        {
+            AzureCliCredential credential = InstrumentClient(new AzureCliCredential());
+
+            string tenantId = Guid.NewGuid().ToString();
+
+            for (int i = 0; i < tenantId.Length; i++)
+            {
+                StringBuilder tenantIdBuilder = new StringBuilder(tenantId);
+
+                tenantIdBuilder.Insert(i, testChar);
+
+                var tokenRequestContext = new TokenRequestContext(MockScopes.Default, tenantId: tenantIdBuilder.ToString());
+
+                Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(tokenRequestContext), Validations.InvalidTenantIdErrorMessage);
+            }
+        }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyGetTokenScopeValidation(char testChar)
+        {
+            AzureCliCredential credential = InstrumentClient(new AzureCliCredential());
+
+            string scope = MockScopes.Default.ToString();
+
+            for (int i = 0; i < scope.Length; i++)
+            {
+                StringBuilder scopeBuilder = new StringBuilder(scope);
+
+                scopeBuilder.Insert(i, testChar);
+
+                var tokenRequestContext = new TokenRequestContext(new string[] { scopeBuilder.ToString() });
+
+                Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(tokenRequestContext), ScopeUtilities.InvalidScopeMessage);
+            }
+        }
     }
 }
diff --git a/sdk/identity/Azure.Identity/tests/AzureDeveloperCliCredentialTests.cs b/sdk/identity/Azure.Identity/tests/AzureDeveloperCliCredentialTests.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Azure.Core;
@@ -134,5 +135,58 @@ public void ConfigureCliProcessTimeout_ProcessTimeout([Values(true, false)] bool
             }
             Assert.AreEqual(AzureDeveloperCliCredential.AzdCliTimeoutError, ex.Message);
         }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyCtorTenantIdValidation(char testChar)
+        {
+            string tenantId = Guid.NewGuid().ToString();
+
+            for (int i = 0; i < tenantId.Length; i++)
+            {
+                StringBuilder tenantIdBuilder = new StringBuilder(tenantId);
+
+                tenantIdBuilder.Insert(i, testChar);
+
+                Assert.Throws<ArgumentException>(() => new AzureDeveloperCliCredential(new AzureDeveloperCliCredentialOptions { TenantId = tenantIdBuilder.ToString() }), Validations.InvalidTenantIdErrorMessage);
+            }
+        }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyGetTokenTenantIdValidation(char testChar)
+        {
+            AzureDeveloperCliCredential credential = InstrumentClient(new AzureDeveloperCliCredential());
+
+            string tenantId = Guid.NewGuid().ToString();
+
+            for (int i = 0; i < tenantId.Length; i++)
+            {
+                StringBuilder tenantIdBuilder = new StringBuilder(tenantId);
+
+                tenantIdBuilder.Insert(i, testChar);
+
+                var tokenRequestContext = new TokenRequestContext(MockScopes.Default, tenantId: tenantIdBuilder.ToString());
+
+                Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(tokenRequestContext), Validations.InvalidTenantIdErrorMessage);
+            }
+        }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyGetTokenScopeValidation(char testChar)
+        {
+            AzureDeveloperCliCredential credential = InstrumentClient(new AzureDeveloperCliCredential());
+
+            string scope = MockScopes.Default.ToString();
+
+            for (int i = 0; i < scope.Length; i++)
+            {
+                StringBuilder scopeBuilder = new StringBuilder(scope);
+
+                scopeBuilder.Insert(i, testChar);
+
+                var tokenRequestContext = new TokenRequestContext(new string[] { scopeBuilder.ToString() });
+
+                Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(tokenRequestContext), ScopeUtilities.InvalidScopeMessage);
+            }
+        }
     }
 }
diff --git a/sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs b/sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs
@@ -243,5 +243,58 @@ public void AuthenticateWithAzurePowerShellCredential_CanceledByUser()
                 new AzurePowerShellCredential(new AzurePowerShellCredentialOptions(), CredentialPipeline.GetInstance(null), new TestProcessService(testProcess)));
             Assert.CatchAsync<OperationCanceledException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default), cts.Token));
         }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyCtorTenantIdValidation(char testChar)
+        {
+            string tenantId = Guid.NewGuid().ToString();
+
+            for (int i = 0; i < tenantId.Length; i++)
+            {
+                StringBuilder tenantIdBuilder = new StringBuilder(tenantId);
+
+                tenantIdBuilder.Insert(i, testChar);
+
+                Assert.Throws<ArgumentException>(() => new AzurePowerShellCredential(new AzurePowerShellCredentialOptions { TenantId = tenantIdBuilder.ToString() }), Validations.InvalidTenantIdErrorMessage);
+            }
+        }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyGetTokenTenantIdValidation(char testChar)
+        {
+            AzurePowerShellCredential credential = InstrumentClient(new AzurePowerShellCredential());
+
+            string tenantId = Guid.NewGuid().ToString();
+
+            for (int i = 0; i < tenantId.Length; i++)
+            {
+                StringBuilder tenantIdBuilder = new StringBuilder(tenantId);
+
+                tenantIdBuilder.Insert(i, testChar);
+
+                var tokenRequestContext = new TokenRequestContext(MockScopes.Default, tenantId: tenantIdBuilder.ToString());
+
+                Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(tokenRequestContext), Validations.InvalidTenantIdErrorMessage);
+            }
+        }
+
+        [TestCaseSource(nameof(NegativeTestCharacters))]
+        public void VerifyGetTokenScopeValidation(char testChar)
+        {
+            AzurePowerShellCredential credential = InstrumentClient(new AzurePowerShellCredential());
+
+            string scope = MockScopes.Default.ToString();
+
+            for (int i = 0; i < scope.Length; i++)
+            {
+                StringBuilder scopeBuilder = new StringBuilder(scope);
+
+                scopeBuilder.Insert(i, testChar);
+
+                var tokenRequestContext = new TokenRequestContext(new string[] { scopeBuilder.ToString() });
+
+                Assert.ThrowsAsync<AuthenticationFailedException>(async () => await credential.GetTokenAsync(tokenRequestContext), ScopeUtilities.InvalidScopeMessage);
+            }
+        }
     }
 }
diff --git a/sdk/identity/Azure.Identity/tests/CredentialTestBase.cs b/sdk/identity/Azure.Identity/tests/CredentialTestBase.cs
@@ -368,6 +368,8 @@ public override string ToString()
             }
         }
 
+        public static readonly char[] NegativeTestCharacters = new char[] { '|', '&', ';', '\'', '"', '!', '@', '$', '%', '^', '*', '(', ')', '=', '[', ']', '{', '}', '<', '>', '?' };
+
         public static IEnumerable<AllowedTenantsTestParameters> GetAllowedTenantsTestCases()
         {
             string tenant = Guid.NewGuid().ToString();