Refine error message about MFA #28124
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
There was a problem hiding this comment.
Pull Request Overview
This PR refines the error message shown when a request fails due to an MFA (claims) challenge, providing clearer guidance and embedding the claims challenge for easier replay.
- Changed
MatchClaimsChallengePatternto return the extracted claims challenge. - Introduced
FormatClaimsChallengeErrorMessageto build a more actionable, base64-encoded CLI command. - Updated handlers to use the new formatter and updated XML docs and changelog.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Accounts/Authenticators/MsalAccessToken.cs | Added XML docs for claims-challenge handler and corrected signature. |
| src/Accounts/Authentication/Utilities/ClaimsChallengeUtilities.cs | Made GetClaimsChallenge private, enhanced pattern matcher, and added FormatClaimsChallengeErrorMessage. |
| src/Accounts/Authentication/ClaimsChallengeHandler.cs | Switched to the new formatter in exception handling. |
| src/Accounts/Accounts/CommonModule/ContextAdapter.cs | Updated catch to throw formatted error message. |
| src/Accounts/Authentication/Authentication/IClaimsChallengeProcessor.cs | Improved <returns> documentation wording. |
| src/Accounts/Accounts/ChangeLog.md | Noted the refined MFA error message under Upcoming Release. |
Comments suppressed due to low confidence (2)
src/Accounts/Authenticators/MsalAccessToken.cs:136
- The method name 'OnClaimsChallenageAsync' contains a typo; consider renaming it to 'OnClaimsChallengeAsync' for clarity and consistency.
public async ValueTask<bool> OnClaimsChallenageAsync(HttpRequestMessage request, string claimsChallenge, CancellationToken cancellationToken)
src/Accounts/Authenticators/MsalAccessToken.cs:132
- Provide a description for the
<param name="request">tag to clarify the purpose of this parameter.
/// <param name="request"></param>
| /// <param name="request"></param> | ||
| /// <param name="claimsChallenge"></param> | ||
| /// <param name="cancellationToken"></param> | ||
| /// <returns>A boolean indicated whether the request should be retried. Throws if the reauth fails.</returns> |
There was a problem hiding this comment.
Correct the grammar in the <returns> tag: change 'indicated' to 'indicating'.
Suggested change
| /// <returns>A boolean indicated whether the request should be retried. Throws if the reauth fails.</returns> | |
| /// <returns>A boolean indicating whether the request should be retried. Throws if the reauth fails.</returns> |
Copilot uses AI. Check for mistakes.
| // Convert claimsChallenge to base64 | ||
| var claimsChallengeBase64 = Convert.ToBase64String(Encoding.UTF8.GetBytes(claimsChallenge ?? string.Empty)); | ||
| // todo: use resource string | ||
| return $@"[This message needs review] Interactive authentication is required. Please run the following cmdlet and add additional parameters as needed: |
There was a problem hiding this comment.
Avoid hard-coding user-facing strings; consider moving this template message into a resource file to support localization and maintainability.
Copilot uses AI. Check for mistakes.
|
Sample error message: PS > new-azkeyvault -ResourceGroupName *** -VaultName ***-Location eastus2
New-AzKeyVault: SharedTokenCacheCredential authentication unavailable. Token acquisition failed for user ***@***. Ensure that you have authenticated with a developer tool that supports Azure single sign on.
[This message needs review] Interactive authentication is required. Please run the following cmdlet and add additional parameters as needed:
Connect-AzAccount -ClaimsChallenge "eyJhY2Nlc3NfdG9rZW4iOnsiYWNycyI6eyJlc3NlbnRpYWwiOnRydWUsInZhbHVlcyI6WyJwMSJdfX19"
Error details:
Resource '***' was disallowed by policy. Policy identifiers: '[{"policyAssignment":{"name":"[Preview]: Users must authenticate with multi-factor authentication to create or update resources","id":"/subscriptions/***/resourceGroups/yemingtemp/providers/Microsoft.Authorization/policyAssignments/***"},"policyDefinition":{"name":"Users must authenticate with multi-factor authentication to create or update resources","id":"/providers/Microsoft.Authorization/policyDefinitions/***","version":"1.0.0-preview"}}]'.The error message of autorest-based cmdlets are the same, but less readable => anything we can do to improve? Maybe show less message? PS > new-azdatabricksworkspace -ResourceGroupName *** -Name *** -Location eastus2
New-AzDatabricksWorkspace_CreateExpanded: C:\Users\***\Documents\PowerShell\Modules\Az.Databricks\1.10.0\Databricks.Autorest\custom\New-AzDatabricksWorkspace.ps1:465
Line |
465 | Az.Databricks.internal\New-AzDatabricksWorkspace @PSBound …
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| SharedTokenCacheCredential authentication unavailable. Token acquisition failed for user
| ***@***. Ensure that you have authenticated with a developer tool that supports
| Azure single sign on. [This message needs review] Interactive authentication is required. Please run the
| following cmdlet and add additional parameters as needed: Connect-AzAccount -ClaimsChallenge
| "eyJhY2Nlc3NfdG9rZW4iOnsiYWNycyI6eyJlc3NlbnRpYWwiOnRydWUsInZhbHVlcyI6WyJwMSJdfX19" Error details: Resource
| '***' was disallowed by policy. Policy identifiers: '[{"policyAssignment":{"name":"[Preview]: Users
| must authenticate with multi-factor authentication to create or update
| resources","id":"/subscriptions/***/resourceGroups/***/providers/Microsoft.Authorization/policyAssignments/***"},"policyDefinition":{"name":"Users must authenticate with multi-factor authentication to create or update resources","id":"/providers/Microsoft.Authorization/policyDefinitions/***","version":"1.0.0-preview"}}]'. |
isra-fel
commented
Jul 3, 2025
src/Accounts/Authentication/Utilities/ClaimsChallengeUtilities.cs
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request introduces several improvements to the handling of claims challenges in the authentication flow. Key changes include refining error messages for better guidance, enhancing the processing of claims challenges, and improving code readability and maintainability. These updates aim to provide clearer feedback to users and streamline the handling of authentication challenges.
Error Message Improvements:
src/Accounts/Accounts/ChangeLog.md)FormatClaimsChallengeErrorMessageto generate detailed error messages for claims challenges, including policy violation details and recommended actions. (src/Accounts/Authentication/Utilities/ClaimsChallengeUtilities.cs)Claims Challenge Handling Enhancements:
MatchClaimsChallengePatternto return the claims challenge string via anoutparameter, simplifying its usage. (src/Accounts/Authentication/Utilities/ClaimsChallengeUtilities.cs)OnChallengeAsyncand related methods to accept the claims challenge string directly, removing redundant calls to extract the challenge. (src/Accounts/Authentication/ClaimsChallengeHandler.cs) [1] [2]OnClaimsChallenageAsyncmethod inMsalAccessTokento process claims challenges and renew access tokens more effectively. (src/Accounts/Authenticators/MsalAccessToken.cs)Codebase Simplification:
GetClaimsChallengemethod to private scope to limit its usage and improve encapsulation. (src/Accounts/Authentication/Utilities/ClaimsChallengeUtilities.cs)src/Accounts/Authentication/Authentication/IClaimsChallengeProcessor.cs)These updates collectively enhance the user experience and maintainability of the authentication module.
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.