Cherry-Pick fixes for 1.8.2 patch release (#33984)

* cherry-pick Do not handle AADSTS errors as PowerShell not installed (#32251) * Do not handle AADSTS errors as PowerShell not installed * cherry-pick Fix regional endpoint validation error (#33544) * Fix regional endpoint validation error and update MSAL dependencies * update changelog for 1.8.2 release * update project version * cherry-pick First iteration of a simple ManagedIdentity integration test (#33017) * First iteration of a simple ManagedIdentity integration test --------- Co-authored-by: Christopher Scott <chriss@microsoft.com>
Azure · Feb 8, 2023 · 9623f9a · 9623f9a
1 parent 3a2a7d4
commit 9623f9a
Show file tree

Hide file tree

Showing 25 changed files with 403 additions and 84 deletions.
diff --git a/eng/.docsettings.yml b/eng/.docsettings.yml
@@ -9,6 +9,9 @@ omitted_paths:
   - samples/*
   - sdk/*/*.Management.*/*
   - sdk/*/*/perf/*
+  - sdk/*/*/integration/*
+  - sdk/*/*/tests/Samples/*
+  - sdk/*/*/tests/samples/*
   - sdk/*/*/samples/*
   - sdk/*/samples/*
   - sdk/*/swagger/*

diff --git a/eng/Packages.Data.props b/eng/Packages.Data.props
@@ -102,15 +102,15 @@
     <PackageReference Update="Azure.ResourceManager" Version="1.3.1" />
 
     <!-- Other approved packages -->
-    <PackageReference Update="Microsoft.Azure.Amqp" Version="2.5.10" />
-    <PackageReference Update="Microsoft.Azure.WebPubSub.Common" Version="1.1.0" />
-    <PackageReference Update="Microsoft.Identity.Client" Version="4.46.0" />
-    <PackageReference Update="Microsoft.Identity.Client.Extensions.Msal" Version="2.23.0" />
+    <PackageReference Update="Microsoft.Azure.Amqp" Version="2.6.1" />
+    <PackageReference Update="Microsoft.Azure.WebPubSub.Common" Version="1.2.0" />
+    <PackageReference Update="Microsoft.Identity.Client" Version="4.49.1" />
+    <PackageReference Update="Microsoft.Identity.Client.Extensions.Msal" Version="2.25.3" />
     <!--
       TODO: This package needs to be released as GA and arch-board approved before taking a dependency in any stable SDK library.
       Currently, it is referencd by Azure.Identity.BrokeredAuthentication which is still in beta
     -->
-    <PackageReference Update="Microsoft.Identity.Client.Broker" Version="4.46.0-preview" />
+    <PackageReference Update="Microsoft.Identity.Client.Broker" Version="4.49.1-preview" />
 
     <!-- TODO: Make sure this package is arch-board approved -->
     <PackageReference Update="System.IdentityModel.Tokens.Jwt" Version="6.5.0" />

diff --git a/sdk/identity/Azure.Identity/CHANGELOG.md b/sdk/identity/Azure.Identity/CHANGELOG.md
@@ -1,4 +1,9 @@
 # Release History
+## 1.8.2 (2023-02-08)
+
+### Bugs Fixed
+- Fixed error message parsing in `AzurePowerShellCredential` which would misinterpret AAD errors with the need to install PowerShell. [#31998](https://github.com/Azure/azure-sdk-for-net/issues/31998)
+- Fix regional endpoint validation error when using `ManagedIdentityCredential`. [#32498])(https://github.com/Azure/azure-sdk-for-net/issues/32498)
 
 ## 1.8.1 (2023-01-13)
 

diff --git a/sdk/identity/Azure.Identity/integration/WebApp/Controllers/TestController.cs b/sdk/identity/Azure.Identity/integration/WebApp/Controllers/TestController.cs
@@ -0,0 +1,39 @@
+using System;
+using System.Linq;
+using Azure.Core;
+using Azure.Identity;
+using Azure.Storage.Blobs;
+using Microsoft.AspNetCore.Mvc;
+
+namespace WebApp.Controllers
+{
+
+    [ApiController]
+    [Route("[controller]")]
+    public class TestController : ControllerBase
+    {
+
+        [HttpGet(Name = "GetTest")]
+        public IActionResult Get()
+        {
+            string resourceId = Environment.GetEnvironmentVariable("IDENTITY_WEBAPP_USER_DEFINED_IDENTITY")!;
+            string account1 = Environment.GetEnvironmentVariable("IDENTITY_STORAGE_NAME_1")!;
+            string account2 = Environment.GetEnvironmentVariable("IDENTITY_STORAGE_NAME_2")!;
+
+            var credential1 = new ManagedIdentityCredential();
+            var credential2 = new ManagedIdentityCredential(new ResourceIdentifier(resourceId));
+            var client1 = new BlobServiceClient(new Uri($"https://{account1}.blob.core.windows.net/"), credential1);
+            var client2 = new BlobServiceClient(new Uri($"https://{account2}.blob.core.windows.net/"), credential2);
+            try
+            {
+                var results = client1.GetBlobContainers().ToList();
+                results = client2.GetBlobContainers().ToList();
+                return Ok("Successfully acquired a token from ManagedIdentityCredential");
+            }
+            catch (Exception ex)
+            {
+                return BadRequest(ex.ToString());
+            }
+        }
+    }
+}
diff --git a/sdk/identity/Azure.Identity/integration/WebApp/Integration.Identity.WebApp.csproj b/sdk/identity/Azure.Identity/integration/WebApp/Integration.Identity.WebApp.csproj
@@ -0,0 +1,16 @@
+<Project Sdk="Microsoft.NET.Sdk.Web">
+
+  <PropertyGroup>
+    <TargetFramework>net6.0</TargetFramework>
+    <Nullable>enable</Nullable>
+    <ImplicitUsings>enable</ImplicitUsings>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Azure.Storage.Blobs" />
+  </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\..\src\Azure.Identity.csproj" />
+  </ItemGroup>
+
+</Project>
diff --git a/sdk/identity/Azure.Identity/integration/WebApp/Program.cs b/sdk/identity/Azure.Identity/integration/WebApp/Program.cs
@@ -0,0 +1,21 @@
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.DependencyInjection;
+
+var builder = WebApplication.CreateBuilder(args);
+
+// Add services to the container.
+
+builder.Services.AddControllers();
+// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle
+builder.Services.AddEndpointsApiExplorer();
+
+var app = builder.Build();
+
+// Configure the HTTP request pipeline.
+app.UseHttpsRedirection();
+
+app.UseAuthorization();
+
+app.MapControllers();
+
+app.Run();
diff --git a/sdk/identity/Azure.Identity/integration/WebApp/appsettings.Development.json b/sdk/identity/Azure.Identity/integration/WebApp/appsettings.Development.json
@@ -0,0 +1,8 @@
+{
+  "Logging": {
+    "LogLevel": {
+      "Default": "Information",
+      "Microsoft.AspNetCore": "Warning"
+    }
+  }
+}
diff --git a/sdk/identity/Azure.Identity/integration/WebApp/appsettings.json b/sdk/identity/Azure.Identity/integration/WebApp/appsettings.json
@@ -0,0 +1,9 @@
+{
+  "Logging": {
+    "LogLevel": {
+      "Default": "Information",
+      "Microsoft.AspNetCore": "Warning"
+    }
+  },
+  "AllowedHosts": "*"
+}
diff --git a/sdk/identity/Azure.Identity/integration/nuget.config b/sdk/identity/Azure.Identity/integration/nuget.config
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8"?>
+<configuration>
+    <config>
+        <!--
+        <add key="repositoryPath" value="./NugetInstall" />
+        <add key="globalPackagesFolder" value="./Nuget" />
+        -->
+        <add key="repositoryPath" value="%AGENT_WORKFOLDER%/NugetInstall" />
+        <add key="globalPackagesFolder" value="%AGENT_WORKFOLDER%/Nuget" />
+    </config>
+    <packageSources>
+        <add key="NuGet official package source" value="https://api.nuget.org/v3/index.json" />
+    </packageSources>
+</configuration>
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.1</Version>
+    <Version>1.8.2</Version>
     <!--The ApiCompatVersion is managed automatically and should not generally be modified manually.-->
-    <ApiCompatVersion>1.8.0</ApiCompatVersion>
+    <ApiCompatVersion>1.8.1</ApiCompatVersion>
     <PackageTags>Microsoft Azure Identity;$(PackageCommonTags)</PackageTags>
     <TargetFrameworks>$(RequiredTargetFrameworks)</TargetFrameworks>
     <NoWarn>$(NoWarn);3021;AZC0011</NoWarn>

diff --git a/sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs b/sdk/identity/Azure.Identity/src/Credentials/AzurePowerShellCredential.cs
@@ -27,7 +27,7 @@ public class AzurePowerShellCredential : TokenCredential
         internal bool UseLegacyPowerShell { get; set; }
 
         private const string Troubleshooting = "See the troubleshooting guide for more information. https://aka.ms/azsdk/net/identity/powershellcredential/troubleshoot";
-        private const string AzurePowerShellFailedError = "Azure PowerShell authentication failed due to an unknown error. " + Troubleshooting;
+        internal const string AzurePowerShellFailedError = "Azure PowerShell authentication failed due to an unknown error. " + Troubleshooting;
         private const string RunConnectAzAccountToLogin = "Run Connect-AzAccount to login";
         private const string NoAccountsWereFoundInTheCache = "No accounts were found in the cache";
         private const string CannotRetrieveAccessToken = "cannot retrieve access token";
@@ -170,8 +170,10 @@ private async ValueTask<AccessToken> RequestAzurePowerShellAccessTokenAsync(bool
 
         private static void CheckForErrors(string output)
         {
-            bool noPowerShell = output.IndexOf("not found", StringComparison.OrdinalIgnoreCase) != -1 ||
-                                output.IndexOf("is not recognized", StringComparison.OrdinalIgnoreCase) != -1;
+            bool noPowerShell = (output.IndexOf("not found", StringComparison.OrdinalIgnoreCase) != -1 ||
+                                output.IndexOf("is not recognized", StringComparison.OrdinalIgnoreCase) != -1) &&
+                                // If the error contains AADSTS, this should be treated as a general error to be bubbled to the user
+                                output.IndexOf("AADSTS", StringComparison.OrdinalIgnoreCase) == -1;
             if (noPowerShell)
             {
                 throw new CredentialUnavailableException(PowerShellNotInstalledError);

diff --git a/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs b/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs
@@ -12,7 +12,6 @@ namespace Azure.Identity
 {
     internal class MsalConfidentialClient : MsalClientBase<IConfidentialClientApplication>
     {
-        private const string s_instanceMetadata = "{\"tenant_discovery_endpoint\":\"https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration\",\"api-version\":\"1.1\",\"metadata\":[{\"preferred_network\":\"login.microsoftonline.com\",\"preferred_cache\":\"login.windows.net\",\"aliases\":[\"login.microsoftonline.com\",\"login.windows.net\",\"login.microsoft.com\",\"sts.windows.net\"]}]}";
         internal readonly string _clientSecret;
         internal readonly bool _includeX5CClaimHeader;
         internal readonly IX509Certificate2Provider _certificateProvider;
@@ -76,7 +75,7 @@ protected override async ValueTask<IConfidentialClientApplication> CreateClientA
             {
                 confClientBuilder.WithAppTokenProvider(_appTokenProviderCallback)
                     .WithAuthority(_authority.AbsoluteUri, TenantId, false)
-                    .WithInstanceDiscoveryMetadata(s_instanceMetadata);
+                    .WithInstanceDiscovery(false);
             }
             else
             {
@@ -104,6 +103,7 @@ protected override async ValueTask<IConfidentialClientApplication> CreateClientA
                 confClientBuilder.WithCertificate(clientCertificate);
             }
 
+            // When the appTokenProviderCallback is set, meaning this is for managed identity, the regional authority is not relevant.
             if (_appTokenProviderCallback == null && !string.IsNullOrEmpty(RegionalAuthority))
             {
                 confClientBuilder.WithAzureRegion(RegionalAuthority);

diff --git a/sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs b/sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs
@@ -93,21 +93,25 @@ public override async Task VerifyAllowedTenantEnforcement(AllowedTenantsTestPara
 
         private static IEnumerable<object[]> ErrorScenarios()
         {
-            yield return new object[] { "Run Connect-AzAccount to login", AzurePowerShellCredential.AzurePowerShellNotLogInError };
-            yield return new object[] { "NoAzAccountModule", AzurePowerShellCredential.AzurePowerShellModuleNotInstalledError };
-            yield return new object[] { "Get-AzAccessToken: Run Connect-AzAccount to login.", AzurePowerShellCredential.AzurePowerShellNotLogInError };
-            yield return new object[] { "No accounts were found in the cache", AzurePowerShellCredential.AzurePowerShellNotLogInError };
-            yield return new object[] { "cannot retrieve access token", AzurePowerShellCredential.AzurePowerShellNotLogInError };
+            yield return new object[] { "Run Connect-AzAccount to login", AzurePowerShellCredential.AzurePowerShellNotLogInError, typeof(CredentialUnavailableException) };
+            yield return new object[] { "NoAzAccountModule", AzurePowerShellCredential.AzurePowerShellModuleNotInstalledError, typeof(CredentialUnavailableException) };
+            yield return new object[] { "Get-AzAccessToken: Run Connect-AzAccount to login.", AzurePowerShellCredential.AzurePowerShellNotLogInError, typeof(CredentialUnavailableException) };
+            yield return new object[] { "No accounts were found in the cache", AzurePowerShellCredential.AzurePowerShellNotLogInError, typeof(CredentialUnavailableException) };
+            yield return new object[] { "cannot retrieve access token", AzurePowerShellCredential.AzurePowerShellNotLogInError, typeof(CredentialUnavailableException) };
+            yield return new object[] {
+                "AADSTS500011: The resource principal named <RESOURCE> was not found in the tenant named",
+                AzurePowerShellCredential.AzurePowerShellFailedError +  " AADSTS500011: The resource principal named <RESOURCE> was not found in the tenant named",
+                typeof(AuthenticationFailedException) };
         }
 
         [Test]
         [TestCaseSource(nameof(ErrorScenarios))]
-        public void AuthenticateWithAzurePowerShellCredential_ErrorScenarios(string errorMessage, string expectedError)
+        public void AuthenticateWithAzurePowerShellCredential_ErrorScenarios(string errorMessage, string expectedError, Type expectedException)
         {
             var testProcess = new TestProcess { Error = errorMessage };
             AzurePowerShellCredential credential = InstrumentClient(
                 new AzurePowerShellCredential(new AzurePowerShellCredentialOptions(), CredentialPipeline.GetInstance(null), new TestProcessService(testProcess)));
-            var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
+            var ex = Assert.ThrowsAsync(expectedException, async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
             Assert.AreEqual(expectedError, ex.Message);
         }
 

diff --git a/sdk/identity/Azure.Identity/tests/ClientSecretCredentialLiveTests.cs b/sdk/identity/Azure.Identity/tests/ClientSecretCredentialLiveTests.cs
@@ -7,6 +7,7 @@
 using Azure.Core;
 using Azure.Core.TestFramework;
 using Microsoft.Identity.Client;
+using Moq;
 using NUnit.Framework;
 
 namespace Azure.Identity.Tests

diff --git a/sdk/identity/Azure.Identity/tests/CredentialTestHelpers.cs b/sdk/identity/Azure.Identity/tests/CredentialTestHelpers.cs
@@ -82,12 +82,12 @@ public static TestFileSystemService CreateFileSystemForVisualStudioCode(Identity
         {
             var sb = new StringBuilder("{");
 
-            if (testEnvironment.TestTenantId != default)
+            if (testEnvironment.IdentityTenantId != default)
             {
-                sb.AppendFormat("\"azure.tenant\": \"{0}\"", testEnvironment.TestTenantId);
+                sb.AppendFormat("\"azure.tenant\": \"{0}\"", testEnvironment.IdentityTenantId);
             }
 
-            if (testEnvironment.TestTenantId != default && cloudName != default)
+            if (testEnvironment.IdentityTenantId != default && cloudName != default)
             {
                 sb.Append(',');
             }
@@ -117,7 +117,7 @@ public static async ValueTask<AuthenticationRecord> GetAuthenticationRecordAsync
 
             var username = testEnvironment.Username;
             var password = testEnvironment.Password;
-            var tenantId = testEnvironment.TestTenantId;
+            var tenantId = testEnvironment.IdentityTenantId;
 
             var result = await PublicClientApplicationBuilder.Create(clientId)
                 .WithTenantId(tenantId)
@@ -138,7 +138,7 @@ public static async Task<string> GetRefreshTokenAsync(IdentityTestEnvironment te
             var clientId = "aebc6443-996d-45c2-90f0-388ff96faa56";
             var username = testEnvironment.Username;
             var password = testEnvironment.Password;
-            var authorityUri = new Uri(new Uri(testEnvironment.AuthorityHostUrl), testEnvironment.TestTenantId).ToString();
+            var authorityUri = new Uri(new Uri(testEnvironment.AuthorityHostUrl), testEnvironment.IdentityTenantId).ToString();
 
             var client = PublicClientApplicationBuilder.Create(clientId)
                 .WithAuthority(authorityUri)

diff --git a/sdk/identity/Azure.Identity/tests/DefaultAzureCredentialLiveTests.cs b/sdk/identity/Azure.Identity/tests/DefaultAzureCredentialLiveTests.cs
@@ -62,6 +62,7 @@ public async Task DefaultAzureCredential_UseVisualStudioCredential()
 
         [RecordedTest]
         [RunOnlyOnPlatforms(Windows = true, OSX = true, ContainerNames = new[] { "ubuntu_netcore_keyring" })]
+        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/27263")]
         public async Task DefaultAzureCredential_UseVisualStudioCodeCredential()
         {
             var options = InstrumentClientOptions(new DefaultAzureCredentialOptions
@@ -73,7 +74,7 @@ public async Task DefaultAzureCredential_UseVisualStudioCodeCredential()
                 ExcludeVisualStudioCredential = true,
                 ExcludeAzureCliCredential = true,
                 ExcludeVisualStudioCodeCredential = false,
-                VisualStudioCodeTenantId = TestEnvironment.TestTenantId
+                VisualStudioCodeTenantId = TestEnvironment.IdentityTenantId
             });
 
             var cloudName = Guid.NewGuid().ToString();
@@ -102,6 +103,7 @@ public async Task DefaultAzureCredential_UseVisualStudioCodeCredential()
 
         [RecordedTest]
         [RunOnlyOnPlatforms(Windows = true, OSX = true, ContainerNames = new[] { "ubuntu_netcore_keyring" })]
+        [Ignore("https://github.com/Azure/azure-sdk-for-net/issues/27263")]
         public async Task DefaultAzureCredential_UseVisualStudioCodeCredential_ParallelCalls()
         {
             var options = InstrumentClientOptions(new DefaultAzureCredentialOptions
@@ -112,7 +114,7 @@ public async Task DefaultAzureCredential_UseVisualStudioCodeCredential_ParallelC
                 ExcludeManagedIdentityCredential = true,
                 ExcludeAzureCliCredential = true,
                 ExcludeVisualStudioCodeCredential = false,
-                VisualStudioCodeTenantId = TestEnvironment.TestTenantId
+                VisualStudioCodeTenantId = TestEnvironment.IdentityTenantId
             });
 
             var cloudName = Guid.NewGuid().ToString();
@@ -149,7 +151,7 @@ public async Task DefaultAzureCredential_UseAzureCliCredential()
                 ExcludeSharedTokenCacheCredential = true,
                 ExcludeManagedIdentityCredential = true,
                 ExcludeVisualStudioCodeCredential = false,
-                VisualStudioCodeTenantId = TestEnvironment.TestTenantId
+                VisualStudioCodeTenantId = TestEnvironment.IdentityTenantId
             });
 
             var (expectedToken, expectedExpiresOn, processOutput) = CredentialTestHelpers.CreateTokenForAzureCli();
@@ -188,7 +190,7 @@ public async Task DefaultAzureCredential_UseAzureCliCredential_ParallelCalls()
                 ExcludeSharedTokenCacheCredential = true,
                 ExcludeManagedIdentityCredential = true,
                 ExcludeVisualStudioCodeCredential = false,
-                VisualStudioCodeTenantId = TestEnvironment.TestTenantId
+                VisualStudioCodeTenantId = TestEnvironment.IdentityTenantId
             });
 
             var (expectedToken, expectedExpiresOn, processOutput) = CredentialTestHelpers.CreateTokenForAzureCli();

diff --git a/sdk/identity/Azure.Identity/tests/IdentityTestEnvironment.cs b/sdk/identity/Azure.Identity/tests/IdentityTestEnvironment.cs
@@ -30,7 +30,6 @@ public class IdentityTestEnvironment : TestEnvironment
         public string UserAssignedVault => GetRecordedOptionalVariable("IDENTITYTEST_IMDSTEST_USERASSIGNEDVAULT");
 
         public string TestPassword => GetOptionalVariable("AZURE_IDENTITY_TEST_PASSWORD") ?? "SANITIZED";
-        public string TestTenantId => GetRecordedOptionalVariable("TENANT_ID") ?? GetRecordedVariable("AZURE_IDENTITY_TEST_TENANTID");
         public string KeyvaultScope => GetRecordedOptionalVariable("AZURE_KEYVAULT_SCOPE") ?? "https://vault.azure.net/.default";
 
         public string ServicePrincipalClientId => GetRecordedVariable("IDENTITY_SP_CLIENT_ID");
@@ -39,5 +38,6 @@ public class IdentityTestEnvironment : TestEnvironment
         public string ServicePrincipalCertificatePfxPath => GetOptionalVariable("IDENTITY_SP_CERT_PFX") ?? Path.Combine(TestContext.CurrentContext.TestDirectory, "Data", "cert.pfx");
         public string ServicePrincipalCertificatePemPath => GetOptionalVariable("IDENTITY_SP_CERT_PEM") ?? Path.Combine(TestContext.CurrentContext.TestDirectory, "Data", "cert.pem");
         public string ServicePrincipalSniCertificatePath => GetOptionalVariable("IDENTITY_SP_CERT_SNI") ?? Path.Combine(TestContext.CurrentContext.TestDirectory, "Data", "cert.pfx");
+        public string IdentityTestWebName =>  GetRecordedVariable("IDENTITY_WEBAPP_NAME");
     }
 }