Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix instance metadata validation error #32520

Merged
merged 10 commits into from
Dec 2, 2022
1 change: 1 addition & 0 deletions sdk/identity/Azure.Identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
### Bugs Fixed
- Fixed error message parsing in `AzureCliCredential` which would misinterpret AAD errors with the need to login with `az login`.
- `ManagedIdentityCredential` will no longer fail when a response received from the endpoint is invalid JSON. It now treats this scenario as if the credential is unavailable.
- Fixed an issue when using ManagedIdentityCredential in combindation with authorities other than Azure public cloud that resulted in a bogus instance metadata validation error [#32498](https://github.com/Azure/azure-sdk-for-net/issues/32498).
christothes marked this conversation as resolved.
Show resolved Hide resolved

### Other Changes

Expand Down
10 changes: 7 additions & 3 deletions sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
Expand All @@ -13,13 +14,14 @@ 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\"]}]}";
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\"]},{\"preferred_network\":\"login.partner.microsoftonline.cn\",\"preferred_cache\":\"login.partner.microsoftonline.cn\",\"aliases\":[\"login.partner.microsoftonline.cn\",\"login.chinacloudapi.cn\"]},{\"preferred_network\":\"login.microsoftonline.de\",\"preferred_cache\":\"login.microsoftonline.de\",\"aliases\":[\"login.microsoftonline.de\"]},{\"preferred_network\":\"login.microsoftonline.us\",\"preferred_cache\":\"login.microsoftonline.us\",\"aliases\":[\"login.microsoftonline.us\",\"login.usgovcloudapi.net\"]},{\"preferred_network\":\"login-us.microsoftonline.com\",\"preferred_cache\":\"login-us.microsoftonline.com\",\"aliases\":[\"login-us.microsoftonline.com\"]}]}";
internal readonly string _clientSecret;
internal readonly bool _includeX5CClaimHeader;
internal readonly IX509Certificate2Provider _certificateProvider;
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; }

Expand Down Expand Up @@ -59,6 +61,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;
Expand All @@ -69,11 +72,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 is hard coded to public cloud in order to match the host found in s_instanceMetadata, but is not actually used in the request.
if (_appTokenProviderCallback != null)
{
confClientBuilder.WithAppTokenProvider(_appTokenProviderCallback)
.WithAuthority(Pipeline.AuthorityHost.AbsoluteUri, TenantId, false)
.WithAuthority(_authority.AbsoluteUri, TenantId, false)
.WithInstanceDiscoveryMetadata(s_instanceMetadata);
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -22,7 +18,8 @@ public ManagedIdentityCredentialImdsLiveTests(bool isAsync) : base(isAsync)

[NonParallelizable]
[Test]
public async Task ValidateImdsSystemAssignedIdentity()
[TestCaseSource(nameof(AuthorityHostValues))]
public async Task ValidateImdsSystemAssignedIdentity(Uri authority)
{
if (string.IsNullOrEmpty(TestEnvironment.IMDSEnable))
{
Expand All @@ -33,7 +30,7 @@ public async Task ValidateImdsSystemAssignedIdentity()
{
var vaultUri = new Uri(TestEnvironment.SystemAssignedVault);

var cred = CreateManagedIdentityCredential();
var cred = CreateManagedIdentityCredential(authority: authority);

// Hard code service version or recorded tests will fail: https://github.com/Azure/azure-sdk-for-net/issues/10432
var kvoptions = InstrumentClientOptions(new SecretClientOptions(SecretClientOptions.ServiceVersion.V7_0));
Expand All @@ -48,7 +45,8 @@ public async Task ValidateImdsSystemAssignedIdentity()

[NonParallelizable]
[Test]
public async Task ValidateImdsUserAssignedIdentity()
[TestCaseSource(nameof(AuthorityHostValues))]
public async Task ValidateImdsUserAssignedIdentity(Uri authority)
{
if (string.IsNullOrEmpty(TestEnvironment.IMDSEnable))
{
Expand All @@ -61,7 +59,7 @@ public async Task ValidateImdsUserAssignedIdentity()

var clientId = TestEnvironment.IMDSClientId;

var cred = CreateManagedIdentityCredential(clientId);
var cred = CreateManagedIdentityCredential(clientId, authority: authority);

// Hard code service version or recorded tests will fail: https://github.com/Azure/azure-sdk-for-net/issues/10432
var kvoptions = InstrumentClientOptions(new SecretClientOptions(SecretClientOptions.ServiceVersion.V7_0));
Expand All @@ -74,9 +72,21 @@ public async Task ValidateImdsUserAssignedIdentity()
}
}

private ManagedIdentityCredential CreateManagedIdentityCredential(string clientId = null, TokenCredentialOptions options = null)
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 ManagedIdentityCredential CreateManagedIdentityCredential(string clientId = null, TokenCredentialOptions options = null, Uri authority = null)
{
options = InstrumentClientOptions(options ?? new TokenCredentialOptions());
options.AuthorityHost = authority;

var pipeline = CredentialPipeline.GetInstance(options);

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading