fix(Accounts): handle management.azure.com as ARM endpoint in AccessTokenAuthenticator#29390
fix(Accounts): handle management.azure.com as ARM endpoint in AccessTokenAuthenticator#29390khang-11 wants to merge 1 commit intoAzure:mainfrom
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
…ticator When users sign in via Connect-AzAccount -AccessToken, calling Get-AzAccessToken -ResourceUrl 'https://management.azure.com/' would fail with '[AccessTokenAuthenticator] failed to retrieve access token for resource'. This is because the authenticator only matched the legacy management.core.windows.net URL as the ARM token audience, not the ResourceManager endpoint URL (management.azure.com). Both URLs are valid audiences for the same ARM access token. Add a check for environment.GetEndpoint(AzureEnvironment.Endpoint.ResourceManager) in the ARM token branch so that management.azure.com is correctly resolved to the stored access token. Fixes Azure#28028
eb7ce55 to
9fdf5f5
Compare
There was a problem hiding this comment.
Pull request overview
Fixes an Az.Accounts access-token-login edge case where Get-AzAccessToken -ResourceUrl https://management.azure.com/ failed because AccessTokenAuthenticator didn’t recognize the ARM ResourceManager endpoint URL as a valid audience for the stored ARM token.
Changes:
- Extend
AccessTokenAuthenticatorARM token routing to also match the environment’sResourceManagerendpoint URL. - Add new
AccessTokenAuthenticatorTestsunit tests covering ARM/Graph/KeyVault routing, unsupported resource behavior, andCanAuthenticate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Accounts/Authenticators/AccessTokenAuthenticator.cs |
Adds ARM token matching for environment.GetEndpoint(AzureEnvironment.Endpoint.ResourceManager) to support management.azure.com audience. |
src/Accounts/Authentication.Test/AuthenticatorsTest/AccessTokenAuthenticatorTests.cs |
Introduces unit tests validating token routing behavior and regression coverage for management.azure.com. |
|
|
||
| private readonly IAzureEnvironment _azureCloud = AzureEnvironment.PublicEnvironments["AzureCloud"]; | ||
|
|
||
| public AccessTokenAuthenticatorTests(ITestOutputHelper output) | ||
| { |
There was a problem hiding this comment.
The test class constructor takes an ITestOutputHelper parameter but doesn't use it. Other authenticator test classes in this project store the output helper (e.g., ManagedServiceIdentityAuthenticatorTests) to avoid analyzer warnings about unused parameters. Consider either assigning it to a field/property or removing the parameter (and corresponding using) if it's not needed.
| private readonly IAzureEnvironment _azureCloud = AzureEnvironment.PublicEnvironments["AzureCloud"]; | |
| public AccessTokenAuthenticatorTests(ITestOutputHelper output) | |
| { | |
| private readonly ITestOutputHelper _output; | |
| private readonly IAzureEnvironment _azureCloud = AzureEnvironment.PublicEnvironments["AzureCloud"]; | |
| public AccessTokenAuthenticatorTests(ITestOutputHelper output) | |
| { | |
| _output = output; |
| /// <summary> | ||
| /// Verifies that CanAuthenticate returns true only for AccessTokenParameters. | ||
| /// </summary> | ||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] | ||
| public void CanAuthenticate_ReturnsTrueForAccessTokenParameters() | ||
| { | ||
| var parameters = CreateParameters("https://management.azure.com/"); | ||
| var authenticator = new AccessTokenAuthenticator(); | ||
|
|
||
| Assert.True(authenticator.CanAuthenticate(parameters)); | ||
| } |
There was a problem hiding this comment.
The XML summary says CanAuthenticate "returns true only for AccessTokenParameters", but the test currently only asserts the true case. Either add a negative assertion (e.g., with a different AuthenticationParameters type) or adjust the summary to avoid implying behavior that isn't being validated.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@khang-11 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Description
Fixes #28028
Get-AzAccessToken -ResourceUrl "https://management.azure.com/"fails when signed in viaConnect-AzAccount -AccessToken, even though the stored ARM access token is valid for that audience.Root Cause
AccessTokenAuthenticatormaps the incomingresourceIdto one of the pre-stored tokens. The ARM token branch only matched resource IDs equivalent toActiveDirectoryServiceEndpointResourceId(i.e.https://management.core.windows.net/). When the user passeshttps://management.azure.com/(the ResourceManager endpoint URL), none of the conditions matched and the authenticator threw:Both
https://management.core.windows.net/andhttps://management.azure.com/are valid audiences for the same ARM access token, so the fix is to also recognise the ResourceManager endpoint URL in the ARM token branch.Fix
In
AccessTokenAuthenticator.cs, one extra condition is added to the ARM token branch:Tests
A new test class
AccessTokenAuthenticatorTestsis added with 6 unit tests:ReturnsArmToken_WhenResourceIsManagementAzureComReturnsArmToken_WhenResourceIsManagementCoreWindowsNetReturnsKeyVaultToken_WhenResourceIsKeyVaultEndpointReturnsGraphToken_WhenResourceIsGraphEndpointThrowsInvalidOperationException_WhenResourceIsUnsupportedCanAuthenticate_ReturnsTrueForAccessTokenParametersChecklist