Cherry-Pick Managed Identity Metadata fix for patch release 1.8.1 (#3…

…3478) * Fix instance metadata validation error (#32520) * Fix instance metadata validation error * update changelog * feedback and fixes Co-authored-by: Christopher Scott <chriss@microsoft.com>
Azure · Jan 13, 2023 · 3a2a7d4 · 3a2a7d4
1 parent 6676777
commit 3a2a7d4
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 10 deletions.
diff --git a/sdk/identity/Azure.Identity/CHANGELOG.md b/sdk/identity/Azure.Identity/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Release History
 
+## 1.8.1 (2023-01-13)
+
+### Bugs Fixed
+- Fixed an issue when using `ManagedIdentityCredential` in combination with authorities other than Azure public cloud that resulted in a incorrect instance metadata validation error. [#32498](https://github.com/Azure/azure-sdk-for-net/issues/32498)
+
 ## 1.8.0 (2022-11-08)
 
 ### Bugs Fixed

diff --git a/sdk/identity/Azure.Identity/src/Azure.Identity.csproj b/sdk/identity/Azure.Identity/src/Azure.Identity.csproj
@@ -2,9 +2,9 @@
   <PropertyGroup>
     <Description>This is the implementation of the Azure SDK Client Library for Azure Identity</Description>
     <AssemblyTitle>Microsoft Azure.Identity Component</AssemblyTitle>
-    <Version>1.8.0</Version>
+    <Version>1.8.1</Version>
     <!--The ApiCompatVersion is managed automatically and should not generally be modified manually.-->
-    <ApiCompatVersion>1.7.0</ApiCompatVersion>
+    <ApiCompatVersion>1.8.0</ApiCompatVersion>
     <PackageTags>Microsoft Azure Identity;$(PackageCommonTags)</PackageTags>
     <TargetFrameworks>$(RequiredTargetFrameworks)</TargetFrameworks>
     <NoWarn>$(NoWarn);3021;AZC0011</NoWarn>

diff --git a/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs b/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Runtime.CompilerServices;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading;
 using System.Threading.Tasks;
@@ -20,6 +19,7 @@ internal class MsalConfidentialClient : MsalClientBase<IConfidentialClientApplic
         private readonly Func<string> _assertionCallback;
         private readonly Func<CancellationToken, Task<string>> _asyncAssertionCallback;
         private readonly Func<AppTokenProviderParameters, Task<AppTokenProviderResult>> _appTokenProviderCallback;
+        private readonly Uri _authority;
 
         internal string RedirectUrl { get; }
 
@@ -59,6 +59,7 @@ public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, stri
             : base(pipeline, tenantId, clientId, options)
         {
             _appTokenProviderCallback = appTokenProviderCallback;
+            _authority = options?.AuthorityHost ?? AzureAuthorityHosts.AzurePublicCloud;
         }
 
         internal string RegionalAuthority { get; } = EnvironmentVariables.AzureRegionalAuthorityName;
@@ -69,11 +70,12 @@ protected override async ValueTask<IConfidentialClientApplication> CreateClientA
                 .WithHttpClientFactory(new HttpPipelineClientFactory(Pipeline.HttpPipeline))
                 .WithLogging(LogMsal, enablePiiLogging: IsPiiLoggingEnabled);
 
-            //special case for using appTokenProviderCallback, authority validation and instance metadata discovery should be disabled since we're not calling the STS
+            // Special case for using appTokenProviderCallback, authority validation and instance metadata discovery should be disabled since we're not calling the STS
+            // The authority matches the one configured in the CredentialOptions.
             if (_appTokenProviderCallback != null)
             {
                 confClientBuilder.WithAppTokenProvider(_appTokenProviderCallback)
-                    .WithAuthority(Pipeline.AuthorityHost.AbsoluteUri, TenantId, false)
+                    .WithAuthority(_authority.AbsoluteUri, TenantId, false)
                     .WithInstanceDiscoveryMetadata(s_instanceMetadata);
             }
             else
@@ -102,7 +104,7 @@ protected override async ValueTask<IConfidentialClientApplication> CreateClientA
                 confClientBuilder.WithCertificate(clientCertificate);
             }
 
-            if (!string.IsNullOrEmpty(RegionalAuthority))
+            if (_appTokenProviderCallback == null && !string.IsNullOrEmpty(RegionalAuthority))
             {
                 confClientBuilder.WithAzureRegion(RegionalAuthority);
             }

diff --git a/sdk/identity/Azure.Identity/tests/ManagedIdentityCredentialImdsLiveTests.cs b/sdk/identity/Azure.Identity/tests/ManagedIdentityCredentialImdsLiveTests.cs
@@ -3,11 +3,7 @@
 
 using System;
 using System.Collections.Generic;
-using System.Reflection;
-using System.Text;
 using System.Threading.Tasks;
-using Azure.Core.TestFramework;
-using Azure.Identity.Tests.Mock;
 using Azure.Security.KeyVault.Secrets;
 using NUnit.Framework;
 

diff --git a/sdk/identity/Azure.Identity/tests/ManagedIdentityCredentialTests.cs b/sdk/identity/Azure.Identity/tests/ManagedIdentityCredentialTests.cs
@@ -11,6 +11,7 @@
 using System.Web;
 using Azure.Core;
 using Azure.Core.Diagnostics;
+using Azure.Core.Pipeline;
 using Azure.Core.TestFramework;
 using Azure.Identity.Tests.Mock;
 using Microsoft.AspNetCore.Http;
@@ -100,6 +101,73 @@ public async Task VerifyImdsRequestWithClientIdMockAsync()
             Assert.AreEqual("true", metadataValue);
         }
 
+        [NonParallelizable]
+        [Test]
+        [TestCase(null)]
+        [TestCase("Auto-Detect")]
+        [TestCase("eastus")]
+        [TestCase("westus")]
+        public async Task VerifyImdsRequestWithClientIdAndRegionalAuthorityNameMockAsync(string regionName)
+        {
+            using var environment = new TestEnvVar(new() { {"AZURE_REGIONAL_AUTHORITY_NAME", regionName}, {"MSI_ENDPOINT", null }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", null }, { "IDENTITY_HEADER", null }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });
+
+            var response = CreateMockResponse(200, ExpectedToken);
+            var mockTransport = new MockTransport(response);
+            var options = new TokenCredentialOptions() { Transport = mockTransport };
+            var pipeline = CredentialPipeline.GetInstance(options);
+
+            ManagedIdentityCredential credential = InstrumentClient(new ManagedIdentityCredential("mock-client-id", pipeline));
+
+            AccessToken actualToken = await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default));
+
+            Assert.AreEqual(ExpectedToken, actualToken.Token);
+
+            MockRequest request = mockTransport.Requests[0];
+
+            string query = request.Uri.Query;
+
+            Assert.AreEqual(request.Uri.Host, "169.254.169.254");
+            Assert.AreEqual(request.Uri.Path, "/metadata/identity/oauth2/token");
+            Assert.IsTrue(query.Contains("api-version=2018-02-01"));
+            Assert.IsTrue(query.Contains($"resource={Uri.EscapeDataString(ScopeUtilities.ScopesToResource(MockScopes.Default))}"));
+            Assert.IsTrue(request.Headers.TryGetValue("Metadata", out string metadataValue));
+            Assert.IsTrue(query.Contains($"{Constants.ManagedIdentityClientId}=mock-client-id"));
+            Assert.AreEqual("true", metadataValue);
+        }
+
+        [NonParallelizable]
+        [Test]
+        [TestCaseSource(nameof(AuthorityHostValues))]
+        public async Task VerifyImdsRequestWithClientIdAndNonPubCloudMockAsync(Uri authority)
+        {
+            using var environment = new TestEnvVar(new() { { "MSI_ENDPOINT", null }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", null }, { "IDENTITY_HEADER", null }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });
+
+            var response = CreateMockResponse(200, ExpectedToken);
+            var mockTransport = new MockTransport(response);
+            var options = new TokenCredentialOptions() { Transport = mockTransport, AuthorityHost = authority };
+            //var pipeline = CredentialPipeline.GetInstance(options);
+            var _pipeline = new HttpPipeline(mockTransport);
+            var pipeline =  new CredentialPipeline(authority, _pipeline, new ClientDiagnostics(options));
+
+            ManagedIdentityCredential credential = InstrumentClient(new ManagedIdentityCredential(new ManagedIdentityClient( pipeline, "mock-client-id")));
+
+            AccessToken actualToken = await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default));
+
+            Assert.AreEqual(ExpectedToken, actualToken.Token);
+
+            MockRequest request = mockTransport.Requests[0];
+
+            string query = request.Uri.Query;
+
+            Assert.AreEqual(request.Uri.Host, "169.254.169.254");
+            Assert.AreEqual(request.Uri.Path, "/metadata/identity/oauth2/token");
+            Assert.IsTrue(query.Contains("api-version=2018-02-01"));
+            Assert.IsTrue(query.Contains($"resource={Uri.EscapeDataString(ScopeUtilities.ScopesToResource(MockScopes.Default))}"));
+            Assert.IsTrue(request.Headers.TryGetValue("Metadata", out string metadataValue));
+            Assert.IsTrue(query.Contains($"{Constants.ManagedIdentityClientId}=mock-client-id"));
+            Assert.AreEqual("true", metadataValue);
+        }
+
         [NonParallelizable]
         [Test]
         public async Task VerifyImdsRequestWithResourceIdMockAsync()
@@ -781,6 +849,17 @@ private static IEnumerable<TestCaseData> ExceptionalEnvironmentConfigs()
             yield return new TestCaseData(new Dictionary<string, string>() { { "MSI_ENDPOINT", null }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", null }, { "IDENTITY_HEADER", null }, { "IDENTITY_SERVER_THUMBPRINT", "null" }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", "http::@/bogusuri" } });
         }
 
+        public static IEnumerable<object[]> AuthorityHostValues()
+        {
+            // params
+            // az thrown Exception message, expected message, expected  exception
+            yield return new object[] { AzureAuthorityHosts.AzureChina };
+            yield return new object[] { AzureAuthorityHosts.AzureGermany };
+            yield return new object[] { AzureAuthorityHosts.AzureGovernment };
+            yield return new object[] { AzureAuthorityHosts.AzurePublicCloud };
+            yield return new object[] { new Uri("https://foo.bar") };
+        }
+
         private MockResponse CreateMockResponse(int responseCode, string token)
         {
             var response = new MockResponse(responseCode);