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.

Sign up for GitHub

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

Jump to bottom

Fix regional endpoint validation error #33544

Merged
merged 8 commits into from
Jan 24, 2023
Merged

Fix regional endpoint validation error #33544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7fef033
Fix regional endpoint validation error
christothes Jan 17, 2023
602b0ed
adjust flaky test
christothes Jan 17, 2023
9b206db
fb
christothes Jan 20, 2023
f04343a
upgrade Microsoft.Identity.Client to get WithInstanceDiscovery(bool)
christothes Jan 23, 2023
cf48e38
fix conflict
christothes Jan 23, 2023
84ed1c5
improve tests
christothes Jan 23, 2023
7f2b948
update Microsoft.Identity.Client.Broker
christothes Jan 24, 2023
e303a7e
update Microsoft.Identity.Client.Extensions.Msal
christothes Jan 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions eng/Packages.Data.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@
<!-- Other approved packages -->
<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.46.0" />
<PackageReference Update="Microsoft.Identity.Client.Extensions.Msal" Version="2.23.0" />
<PackageReference Update="Microsoft.Identity.Client" Version="4.49.1" />
christothes marked this conversation as resolved.
Show resolved Hide resolved
<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" />
Expand Down
4 changes: 2 additions & 2 deletions sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -76,7 +75,7 @@ protected override async ValueTask<IConfidentialClientApplication> CreateClientA
{
confClientBuilder.WithAppTokenProvider(_appTokenProviderCallback)
.WithAuthority(_authority.AbsoluteUri, TenantId, false)
.WithInstanceDiscoveryMetadata(s_instanceMetadata);
.WithInstanceDiscovery(false);
}
else
{
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 9 additions & 19 deletions sdk/identity/Azure.Identity/tests/ManagedIdentityCredentialTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,9 @@ public async Task VerifyImdsRequestWithClientIdMockAsync()
[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 } });
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 mockTransport = new MockTransport(req => CreateMockResponse(200, ExpectedToken));
var options = new TokenCredentialOptions() { Transport = mockTransport };
var pipeline = CredentialPipeline.GetInstance(options);

Expand All @@ -121,18 +120,6 @@ public async Task VerifyImdsRequestWithClientIdAndRegionalAuthorityNameMockAsync
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]
Expand All @@ -147,9 +134,12 @@ public async Task VerifyImdsRequestWithClientIdAndNonPubCloudMockAsync(Uri autho
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));
var pipeline = new CredentialPipeline(authority, _pipeline, new ClientDiagnostics(options));

ManagedIdentityCredential credential = InstrumentClient(new ManagedIdentityCredential(new ManagedIdentityClient( pipeline, "mock-client-id")));
ManagedIdentityCredential credential = InstrumentClient(
new ManagedIdentityCredential(
new ManagedIdentityClient(
new ManagedIdentityClientOptions { Pipeline = pipeline, ClientId = "mock-client-id", Options = options })));

AccessToken actualToken = await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default));

Expand Down Expand Up @@ -696,10 +686,11 @@ public async Task VerifyInitialImdsConnectionTimeoutHonored()
var startTime = DateTimeOffset.UtcNow;

var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));
var endTime = DateTimeOffset.UtcNow;

Assert.That(ex.Message, Does.Contain(ImdsManagedIdentitySource.AggregateError));

Assert.Less(DateTimeOffset.UtcNow - startTime, TimeSpan.FromSeconds(2));
Assert.Less(endTime - startTime, TimeSpan.FromSeconds(2));

await Task.CompletedTask;
}
Expand Down Expand Up @@ -857,7 +848,6 @@ public static IEnumerable<object[]> AuthorityHostValues()
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)
Expand Down