Warn on unsupported managed identity configuration (Azure#27966)

christothes · web-flow · commit 605cbc825dea · 2022-04-04T22:38:11.000Z
diff --git a/sdk/identity/Azure.Identity/src/AzureArcManagedIdentitySource.cs b/sdk/identity/Azure.Identity/src/AzureArcManagedIdentitySource.cs
@@ -37,13 +37,17 @@ public static ManagedIdentitySource TryCreate(ManagedIdentityClientOptions optio
                 throw new AuthenticationFailedException(IdentityEndpointInvalidUriError);
             }
 
-            return new AzureArcManagedIdentitySource(options.Pipeline, endpointUri, options.ClientId);
+            return new AzureArcManagedIdentitySource(endpointUri, options);
         }
 
-        private AzureArcManagedIdentitySource(CredentialPipeline pipeline, Uri endpoint, string clientId) : base(pipeline)
+        private AzureArcManagedIdentitySource(Uri endpoint, ManagedIdentityClientOptions options) : base(options.Pipeline)
         {
             _endpoint = endpoint;
-            _clientId = clientId;
+            _clientId = options.ClientId;
+            if (!string.IsNullOrEmpty(_clientId) || null != options.ResourceIdentifier)
+            {
+                AzureIdentityEventSource.Singleton.UserAssignedManagedIdentityNotSupported("Azure Arc");
+            }
         }
 
         protected override Request CreateRequest(string[] scopes)
diff --git a/sdk/identity/Azure.Identity/src/AzureIdentityEventSource.cs b/sdk/identity/Azure.Identity/src/AzureIdentityEventSource.cs
@@ -35,11 +35,15 @@ internal sealed class AzureIdentityEventSource : AzureEventSource
         private const int TenantIdDiscoveredAndUsedEvent = 18;
         internal const int AuthenticatedAccountDetailsEvent = 19;
         internal const int UnableToParseAccountDetailsFromTokenEvent = 20;
+        private const int UserAssignedManagedIdentityNotSupportedEvent = 21;
+        private const int ServiceFabricManagedIdentityRuntimeConfigurationNotSupportedEvent = 22;
         internal const string TenantIdDiscoveredAndNotUsedEventMessage = "A token was request for a different tenant than was configured on the credential, but the configured value was used since multi tenant authentication has been disabled. Configured TenantId: {0}, Requested TenantId {1}";
         internal const string TenantIdDiscoveredAndUsedEventMessage = "A token was requested for a different tenant than was configured on the credential, and the requested tenant id was used to authenticate. Configured TenantId: {0}, Requested TenantId {1}";
         internal const string AuthenticatedAccountDetailsMessage = "Client ID: {0}. Tenant ID: {1}. User Principal Name: {2} Object ID: {3}";
         internal const string Unavailable = "<not available>";
         internal const string UnableToParseAccountDetailsFromTokenMessage = "Unable to parse account details from the Access Token";
+        internal const string UserAssignedManagedIdentityNotSupportedMessage = "User assigned managed identities are not supported in the {0} environment.";
+        internal const string ServiceFabricManagedIdentityRuntimeConfigurationNotSupportedMessage = "Service Fabric user assigned managed identity ClientId or ResourceId is not configurable at runtime.";
 
         private AzureIdentityEventSource() : base(EventSourceName) { }
 
@@ -321,5 +325,23 @@ internal void UnableToParseAccountDetailsFromToken()
                 WriteEvent(UnableToParseAccountDetailsFromTokenEvent);
             }
         }
+
+        [Event(UserAssignedManagedIdentityNotSupportedEvent, Level = EventLevel.Warning, Message = UserAssignedManagedIdentityNotSupportedMessage)]
+        public void UserAssignedManagedIdentityNotSupported(string environment)
+        {
+            if (IsEnabled(EventLevel.Warning, EventKeywords.All))
+            {
+                WriteEvent(UserAssignedManagedIdentityNotSupportedEvent, environment);
+            }
+        }
+
+        [Event(ServiceFabricManagedIdentityRuntimeConfigurationNotSupportedEvent, Level = EventLevel.Warning, Message =ServiceFabricManagedIdentityRuntimeConfigurationNotSupportedMessage)]
+        public void ServiceFabricManagedIdentityRuntimeConfigurationNotSupported()
+        {
+            if (IsEnabled(EventLevel.Warning, EventKeywords.All))
+            {
+                WriteEvent(ServiceFabricManagedIdentityRuntimeConfigurationNotSupportedEvent);
+            }
+        }
     }
 }
diff --git a/sdk/identity/Azure.Identity/src/CloudShellManagedIdentitySource.cs b/sdk/identity/Azure.Identity/src/CloudShellManagedIdentitySource.cs
@@ -13,7 +13,6 @@ namespace Azure.Identity
     internal class CloudShellManagedIdentitySource : ManagedIdentitySource
     {
         private readonly Uri _endpoint;
-        private readonly string _clientId;
         private const string MsiEndpointInvalidUriError = "The environment variable MSI_ENDPOINT contains an invalid Uri.";
 
         public static ManagedIdentitySource TryCreate(ManagedIdentityClientOptions options)
@@ -36,38 +35,31 @@ public static ManagedIdentitySource TryCreate(ManagedIdentityClientOptions optio
                 throw new AuthenticationFailedException(MsiEndpointInvalidUriError, ex);
             }
 
-            return new CloudShellManagedIdentitySource(options.Pipeline, endpointUri, options.ClientId);
+            return new CloudShellManagedIdentitySource(endpointUri, options);
         }
 
-        private CloudShellManagedIdentitySource(CredentialPipeline pipeline, Uri endpoint, string clientId) : base(pipeline)
+        private CloudShellManagedIdentitySource(Uri endpoint, ManagedIdentityClientOptions options) : base(options.Pipeline)
         {
             _endpoint = endpoint;
-            _clientId = clientId;
+            if (!string.IsNullOrEmpty(options.ClientId) || null == options.ResourceIdentifier)
+            {
+                AzureIdentityEventSource.Singleton.UserAssignedManagedIdentityNotSupported("Cloud Shell");
+            }
         }
 
         protected override Request CreateRequest(string[] scopes)
         {
             // covert the scopes to a resource string
             string resource = ScopeUtilities.ScopesToResource(scopes);
-
             Request request = Pipeline.HttpPipeline.CreateRequest();
-
             request.Method = RequestMethod.Post;
-
             request.Headers.Add(HttpHeader.Common.FormUrlEncodedContentType);
-
             request.Uri.Reset(_endpoint);
-
             request.Headers.Add("Metadata", "true");
 
             var bodyStr = $"resource={Uri.EscapeDataString(resource)}";
-
-            if (!string.IsNullOrEmpty(_clientId))
-            {
-                bodyStr += $"&{Constants.ManagedIdentityClientId}={Uri.EscapeDataString(_clientId)}";
-            }
-
             ReadOnlyMemory<byte> content = Encoding.UTF8.GetBytes(bodyStr).AsMemory();
+
             request.Content = RequestContent.Create(content);
             return request;
         }
diff --git a/sdk/identity/Azure.Identity/src/Credentials/ManagedIdentityCredential.cs b/sdk/identity/Azure.Identity/src/Credentials/ManagedIdentityCredential.cs
@@ -67,8 +67,8 @@ public ManagedIdentityCredential(ResourceIdentifier resourceId, TokenCredentialO
             _clientId = resourceId.ToString();
         }
 
-        internal ManagedIdentityCredential(string clientId, CredentialPipeline pipeline)
-            : this(new ManagedIdentityClient(pipeline, clientId))
+        internal ManagedIdentityCredential(string clientId, CredentialPipeline pipeline, bool preserveTransport = false)
+            : this(new ManagedIdentityClient(new ManagedIdentityClientOptions { Pipeline = pipeline, ClientId = clientId, PreserveTransport = preserveTransport }))
         {
             _clientId = clientId;
         }
diff --git a/sdk/identity/Azure.Identity/src/ServiceFabricManagedIdentitySource.cs b/sdk/identity/Azure.Identity/src/ServiceFabricManagedIdentitySource.cs
@@ -63,6 +63,10 @@ private ServiceFabricManagedIdentitySource(CredentialPipeline pipeline, Uri endp
             _identityHeaderValue = identityHeaderValue;
             _clientId = options.ClientId;
             _resourceId = options.ResourceIdentifier?.ToString();
+            if (!string.IsNullOrEmpty(options.ClientId) || null != options.ResourceIdentifier)
+            {
+                AzureIdentityEventSource.Singleton.ServiceFabricManagedIdentityRuntimeConfigurationNotSupported();
+            }
         }
 
         protected override Request CreateRequest(string[] scopes)
diff --git a/sdk/identity/Azure.Identity/tests/ManagedIdentityCredentialTests.cs b/sdk/identity/Azure.Identity/tests/ManagedIdentityCredentialTests.cs
@@ -3,15 +3,18 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.Tracing;
 using System.IO;
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using System.Web;
 using Azure.Core;
+using Azure.Core.Diagnostics;
 using Azure.Core.TestFramework;
 using Azure.Identity.Tests.Mock;
 using Microsoft.AspNetCore.Http;
+using Microsoft.Diagnostics.Runtime.Interop;
 using NUnit.Framework;
 
 namespace Azure.Identity.Tests
@@ -82,13 +85,14 @@ public async Task VerifyImdsRequestWithResourceIdMockAsync()
             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.That(Uri.UnescapeDataString(query),  Does.Contain($"{Constants.ManagedIdentityResourceId}={_expectedResourceId}"));
+            Assert.That(Uri.UnescapeDataString(query), Does.Contain($"{Constants.ManagedIdentityResourceId}={_expectedResourceId}"));
             Assert.AreEqual("true", metadataValue);
         }
 
         [NonParallelizable]
         [Test]
-        public async Task VerifyServiceFabricRequestWithResourceIdMockAsync()
+        [TestCaseSource(nameof(ResourceAndClientIds))]
+        public async Task VerifyServiceFabricRequestWithResourceIdMockAsync(string clientId, bool includeResourceIdentifier)
         {
             using var environment = new TestEnvVar(
                 new()
@@ -101,12 +105,22 @@ public async Task VerifyServiceFabricRequestWithResourceIdMockAsync()
                     { "IDENTITY_SERVER_THUMBPRINT", "thumbprint" }
                 });
 
+            List<string> messages = new();
+            using AzureEventSourceListener listener = new AzureEventSourceListener(
+                (_, message) => messages.Add(message),
+                EventLevel.Warning);
+
             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(new ResourceIdentifier(_expectedResourceId), pipeline, true));
+            ManagedIdentityCredential credential = (clientId, includeResourceIdentifier) switch
+            {
+                (Item1: null, Item2: true) => InstrumentClient(new ManagedIdentityCredential(new ResourceIdentifier(_expectedResourceId), pipeline, true)),
+                (Item1: not null, Item2: false) => InstrumentClient(new ManagedIdentityCredential(clientId, pipeline, true)),
+                _ => InstrumentClient(new ManagedIdentityCredential(clientId: null, pipeline, true))
+            };
 
             AccessToken actualToken = await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default));
 
@@ -119,7 +133,14 @@ public async Task VerifyServiceFabricRequestWithResourceIdMockAsync()
             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.That(query,  Does.Contain($"{Constants.ManagedIdentityResourceId}={Uri.EscapeDataString(_expectedResourceId)}"));
+            if (includeResourceIdentifier)
+            {
+                Assert.That(query, Does.Contain($"{Constants.ManagedIdentityResourceId}={Uri.EscapeDataString(_expectedResourceId)}"));
+            }
+            if (clientId != null || includeResourceIdentifier)
+            {
+                Assert.That(messages, Does.Contain(AzureIdentityEventSource.ServiceFabricManagedIdentityRuntimeConfigurationNotSupportedMessage));
+            }
         }
 
         [NonParallelizable]
@@ -240,7 +261,7 @@ public async Task VerifyAppService2017RequestWithClientIdAndMockAsync([Values(nu
 
             var response = CreateMockResponse(200, ExpectedToken);
             var mockTransport = new MockTransport(response);
-            var options = new TokenCredentialOptions() {Transport = mockTransport};
+            var options = new TokenCredentialOptions() { Transport = mockTransport };
 
             ManagedIdentityCredential credential = InstrumentClient(new ManagedIdentityCredential(clientId, options));
 
@@ -328,7 +349,7 @@ public async Task VerifyAppService2019RequestMockAsync()
         [Test]
         public async Task AllAppServiceEnvVarsSetSelects2019Api()
         {
-            using var environment = new TestEnvVar(new() { { "MSI_ENDPOINT", "https://mock.msi.endpoint/"  }, { "MSI_SECRET", "mock-msi-secret" }, { "IDENTITY_ENDPOINT", "https://identity.endpoint/" }, { "IDENTITY_HEADER", "mock-identity-header" }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });
+            using var environment = new TestEnvVar(new() { { "MSI_ENDPOINT", "https://mock.msi.endpoint/" }, { "MSI_SECRET", "mock-msi-secret" }, { "IDENTITY_ENDPOINT", "https://identity.endpoint/" }, { "IDENTITY_HEADER", "mock-identity-header" }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });
 
             var response = CreateMockResponse(200, ExpectedToken);
             var mockTransport = new MockTransport(response);
@@ -386,7 +407,7 @@ public async Task VerifyAppService2019RequestWithResourceIdMockAsync()
 
             var response = CreateMockResponse(200, ExpectedToken);
             var mockTransport = new MockTransport(response);
-            var options = new TokenCredentialOptions() {Transport = mockTransport};
+            var options = new TokenCredentialOptions() { Transport = mockTransport };
             ManagedIdentityCredential credential =
                 InstrumentClient(new ManagedIdentityCredential(new ResourceIdentifier(resourceId), options));
 
@@ -444,6 +465,11 @@ public async Task VerifyCloudShellMsiRequestWithClientIdMockAsync(string clientI
         {
             using var environment = new TestEnvVar(new() { { "MSI_ENDPOINT", "https://mock.msi.endpoint/" }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", null }, { "IDENTITY_HEADER", null }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });
 
+            List<string> messages = new();
+            using AzureEventSourceListener listener = new AzureEventSourceListener(
+                (_, message) => messages.Add(message),
+                EventLevel.Warning);
+
             var response = CreateMockResponse(200, ExpectedToken);
             var mockTransport = new MockTransport(response);
             var options = new TokenCredentialOptions() { Transport = mockTransport };
@@ -467,7 +493,7 @@ public async Task VerifyCloudShellMsiRequestWithClientIdMockAsync(string clientI
             Assert.IsTrue(body.Contains($"resource={Uri.EscapeDataString(ScopeUtilities.ScopesToResource(MockScopes.Default))}"));
             if (clientId != null)
             {
-                Assert.IsTrue(body.Contains($"{Constants.ManagedIdentityClientId}=mock-client-id"));
+                Assert.That(messages, Does.Contain(string.Format(AzureIdentityEventSource.UserAssignedManagedIdentityNotSupportedMessage, "Cloud Shell")));
             }
             Assert.IsTrue(request.Headers.TryGetValue("Metadata", out string actMetadata));
             Assert.AreEqual("true", actMetadata);
@@ -510,7 +536,7 @@ public async Task VerifyMsiUnavailableOnIMDSRequestFailedExcpetion()
 
         [NonParallelizable]
         [Test]
-        public async Task VerifyMsiUnavailableOnIMDSGatewayErrorResponse([Values(502, 504)]int statusCode)
+        public async Task VerifyMsiUnavailableOnIMDSGatewayErrorResponse([Values(502, 504)] int statusCode)
         {
             using var server = new TestServer(context =>
             {
@@ -658,7 +684,7 @@ public async Task VerifyClientAuthenticateReturnsErrorResponse()
                 });
             var errorMessage = "Some error happened";
             var mockTransport = new MockTransport(request => CreateErrorMockResponse(404, errorMessage));
-            var options = new TokenCredentialOptions { Transport = mockTransport};
+            var options = new TokenCredentialOptions { Transport = mockTransport };
             options.Retry.MaxDelay = TimeSpan.Zero;
             var pipeline = CredentialPipeline.GetInstance(options);
 
@@ -684,6 +710,13 @@ public async Task VerifyAuthenticationFailedExceptionsAreDeferredToGetToken(Dict
             await Task.CompletedTask;
         }
 
+        private static IEnumerable<TestCaseData> ResourceAndClientIds()
+        {
+            yield return new TestCaseData(new object[] { null, false });
+            yield return new TestCaseData(new object[] { "mock-client-id", false });
+            yield return new TestCaseData(new object[] { null, true });
+        }
+
         private static IEnumerable<TestCaseData> ExceptionalEnvironmentConfigs()
         {
             // AppServiceV2017ManagedIdentitySource should throw
@@ -696,7 +729,7 @@ private static IEnumerable<TestCaseData> ExceptionalEnvironmentConfigs()
             yield return new TestCaseData(new Dictionary<string, string>() { { "MSI_ENDPOINT", null }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", "http::@/bogusuri" }, { "IMDS_ENDPOINT", "mockvalue" }, { "IDENTITY_HEADER", null }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });
 
             // ServiceFabricManagedIdentitySource should throw
-            yield return new TestCaseData(new Dictionary<string, string>() { { "MSI_ENDPOINT", null }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", "http::@/bogusuri" }, { "IDENTITY_HEADER", "mockvalue" }, { "IDENTITY_SERVER_THUMBPRINT", "mockvalue"}, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });
+            yield return new TestCaseData(new Dictionary<string, string>() { { "MSI_ENDPOINT", null }, { "MSI_SECRET", null }, { "IDENTITY_ENDPOINT", "http::@/bogusuri" }, { "IDENTITY_HEADER", "mockvalue" }, { "IDENTITY_SERVER_THUMBPRINT", "mockvalue" }, { "AZURE_POD_IDENTITY_AUTHORITY_HOST", null } });
 
             // ImdsManagedIdentitySource should throw
             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" } });

Original file line number	Diff line number	Diff line change
`@@ -37,13 +37,17 @@ public static ManagedIdentitySource TryCreate(ManagedIdentityClientOptions optio`
`37`	`37`	`throw new AuthenticationFailedException(IdentityEndpointInvalidUriError);`
`38`	`38`	`}`
`39`	`39`
`40`		`- return new AzureArcManagedIdentitySource(options.Pipeline, endpointUri, options.ClientId);`
	`40`	`+ return new AzureArcManagedIdentitySource(endpointUri, options);`
`41`	`41`	`}`
`42`	`42`
`43`		`- private AzureArcManagedIdentitySource(CredentialPipeline pipeline, Uri endpoint, string clientId) : base(pipeline)`
	`43`	`+ private AzureArcManagedIdentitySource(Uri endpoint, ManagedIdentityClientOptions options) : base(options.Pipeline)`
`44`	`44`	`{`
`45`	`45`	`_endpoint = endpoint;`
`46`		`- _clientId = clientId;`
	`46`	`+ _clientId = options.ClientId;`
	`47`	`+ if (!string.IsNullOrEmpty(_clientId) \|\| null != options.ResourceIdentifier)`
	`48`	`+ {`
	`49`	`+ AzureIdentityEventSource.Singleton.UserAssignedManagedIdentityNotSupported("Azure Arc");`
	`50`	`+ }`
`47`	`51`	`}`
`48`	`52`
`49`	`53`	`protected override Request CreateRequest(string[] scopes)`
Original file line number	Diff line number	Diff line change
`@@ -67,8 +67,8 @@ public ManagedIdentityCredential(ResourceIdentifier resourceId, TokenCredentialO`
`67`	`67`	`_clientId = resourceId.ToString();`
`68`	`68`	`}`
`69`	`69`
`70`		`- internal ManagedIdentityCredential(string clientId, CredentialPipeline pipeline)`
`71`		`- : this(new ManagedIdentityClient(pipeline, clientId))`
	`70`	`+ internal ManagedIdentityCredential(string clientId, CredentialPipeline pipeline, bool preserveTransport = false)`
	`71`	`+ : this(new ManagedIdentityClient(new ManagedIdentityClientOptions { Pipeline = pipeline, ClientId = clientId, PreserveTransport = preserveTransport }))`
`72`	`72`	`{`
`73`	`73`	`_clientId = clientId;`
`74`	`74`	`}`