Fix IsChainedCredential for single credential selection in DACFactory#56334
Fix IsChainedCredential for single credential selection in DACFactory#56334
Conversation
When CredentialSource selects a specific single credential, each Create* method now checks Options.CredentialSource to set IsChainedCredential to false. This ensures proper exception behavior - non-chained credentials throw AuthenticationFailedException for unknown errors instead of wrapping them as CredentialUnavailableException. Fixes #56324 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates DefaultAzureCredentialFactory and related tests to ensure credentials created as a single explicit selection are not marked as chained, aligning exception behavior with the actual authentication flow used by DefaultAzureCredential and ConfigurableCredential.
Changes:
- Update
DefaultAzureCredentialFactoryto setIsChainedCredentialbased on whether the credential is part of a chain vs a standalone selection. - Add
IsChainedCredentialSupportedto test bases and skip chained-only scenarios for configurable (single-selection) credentials. - Adjust configurable credential tests to reflect non-chained behavior when
CredentialSourceis used.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/identity/Azure.Identity/src/DefaultAzureCredentialFactory.cs | Changes how IsChainedCredential is set when constructing credentials via the factory. |
| sdk/identity/Azure.Identity/tests/VisualStudioCredentialTests.cs | Adds IsChainedCredentialSupported and skips chained-only scenarios when unsupported. |
| sdk/identity/Azure.Identity/tests/ConfigurableCredentials/VisualStudioCredentialTests.cs | Marks configurable VS credential tests as not supporting chained scenarios. |
| sdk/identity/Azure.Identity/tests/AzurePowerShellCredentialsTests.cs | Introduces IsChainedCredentialSupported and adds guards for chained-only test paths. |
| sdk/identity/Azure.Identity/tests/ConfigurableCredentials/AzurePowerShellCredentialTests.cs | Switches configurable PS tests to “non-chained only” via IsChainedCredentialSupported. |
| sdk/identity/Azure.Identity/tests/AzureDeveloperCliCredentialTests.cs | Adds chained-scenario skip guards for configurable single-credential runs. |
| sdk/identity/Azure.Identity/tests/ConfigurableCredentials/AzureDeveloperCliCredentialTests.cs | Marks configurable azd tests as not supporting chained scenarios. |
| sdk/identity/Azure.Identity/tests/AzureCliCredentialTests.cs | Adds chained-scenario skip guards for configurable single-credential runs. |
| sdk/identity/Azure.Identity/tests/ConfigurableCredentials/AzureCliCredentialTests.cs | Marks configurable az tests as not supporting chained scenarios. |
jsquire
left a comment
There was a problem hiding this comment.
This one is subtle. You'll want to get Chris or Jonathan to also sign-off before merging just in case I'm overlooking an issue in the behavior.
JonathanCrd
left a comment
There was a problem hiding this comment.
The changes look good overall, but there's a scenario involving the Dev and Prod chains that I'd like another perspective on, just to be sure we're not missing something :)
| (true, _) => CreateDevelopmentCredentialChain(), | ||
| (_, true) => CreateProductionCredentialChain(), |
There was a problem hiding this comment.
I'm wondering if there's an scenario where CredentialSource is set to "dev" or "prod", and Options.CredentialSource is set to something, then IsChainedCredential would be false on every credential of the selected sub-chain.
If that’s possible, then I think this is incorrect. Since those credentials are part of the Dev or Prod chain, I would expect IsChainedCredential to be true.
@christothes do you think this scenario can actually occur?
There was a problem hiding this comment.
Yes, that scenario seems possible - Perhaps we should change the logic to not check for 'CredentialSource' having a value, but that the value is either null or not 'dev' or 'prod'.
Description
When
CredentialSourceselects a specific single credential (not a chain), the credential'sIsChainedCredentialshould befalse. Previously, allCreate*methods inDefaultAzureCredentialFactoryhardcodedIsChainedCredential = true.Each
Create*method now checksOptions.CredentialSource == nullto determine whether the credential is part of a chain or a standalone selection. ForCreateManagedIdentityCredential, the existingisChainedparameter is also considered to handle the environment variable path.Changes
Create*method setsIsChainedCredential = Options.CredentialSource == nullinstead of hardcodingtrueIsChainedCredentialSupportedproperty and skip guards for chained-only test scenariosIsChainedCredentialSupported => falsesinceCredentialSourcealways creates non-chained credentialsFixes #56324