feat: Replace DefaultAzureCredential with ManagedIdentityCredential for production#199
Open
ArLucaID wants to merge 3 commits intomicrosoft:mainfrom
Open
feat: Replace DefaultAzureCredential with ManagedIdentityCredential for production#199ArLucaID wants to merge 3 commits intomicrosoft:mainfrom
ArLucaID wants to merge 3 commits intomicrosoft:mainfrom
Conversation
…ranting Addresses 6 gaps in the entra-agent-id skill: 1. Runtime token exchange — Added fmi_path two-step exchange pattern (parent token via client_credentials + fmi_path, then client_assertion exchange for Graph-scoped Agent Identity token). Covers both autonomous (app-only) and OBO (delegated) modes with complete code samples. 2. fmi_path parameter — Documented the fmi_path parameter on the standard /oauth2/v2.0/token endpoint that targets a specific Agent Identity. Clarified this is NOT RFC 8693 token exchange (which returns AADSTS82001). 3. OBO/delegated flow — Added Blueprint API configuration (identifierUris, oauth2PermissionScopes, preAuthorizedApplications, optionalClaims), user token audience requirements, and complete OBO exchange code. 4. Cross-tenant guidance — Documented that fmi_path works cross-tenant when the Blueprint is multi-tenant. Critical rule: step 1 must target the Agent Identity's home tenant, not the Blueprint's (AADSTS700211 otherwise). 5. Permission granting — Added appRoleAssignments for autonomous mode and oauth2PermissionGrants for OBO mode, with per-agent scoping examples. 6. oauth2-token-flow.md completion — Option B now shows the full two-step exchange (was incomplete — stopped at Blueprint token). Added Option C for OBO flow. Added cross-tenant exchange section. Updated acceptance-criteria.md with sections 9-12 covering runtime exchange, OBO, cross-tenant, and permission grant patterns (correct/incorrect examples). Added Troubleshooting table with 8 common errors and fixes. Tested patterns verified against live Entra endpoints (same-tenant autonomous, cross-tenant fmi_path exchange, OBO with delegated permissions).
Core skill for eliminating secrets from Azure apps using managed identities, workload identity federation, and Azure Identity SDK. - SKILL.md (155 lines) with principles, credential type table, RBAC roles, WIF patterns - references/migration-patterns.md: before/after code for 7 services, 4 languages - references/acceptance-criteria.md: correct/incorrect patterns - 6 test scenarios, 100% pass rate - Symlinks in python/dotnet/typescript/java/rust entra categories
…or production Switch credential-free-dev skill to recommend ManagedIdentityCredential for production workloads, keeping DefaultAzureCredential for local dev only. Per Azure Identity best practices guidance: DefaultAzureCredential's credential chain probing can cause subtle issues or silent failures in production apps. Replace with a specific TokenCredential implementation such as ManagedIdentityCredential. Updated across all 3 skill files: - SKILL.md: new production auth section, updated core principles, code samples, migration steps, and added pitfall about DAC in production - migration-patterns.md: all AFTER examples now use ManagedIdentityCredential (Python, .NET, TypeScript, Java across 7 Azure services) - acceptance-criteria.md: ManagedIdentityCredential as correct production pattern, DefaultAzureCredential in production marked as incorrect
Contributor
Author
|
@thegovind / @spboyer : Could I get some help in merging this one? I don't have permissions to request reviewers. |
Member
|
@scottaddie can you review this PR please? |
scottaddie
requested changes
Mar 20, 2026
| > — [Authentication best practices with the Azure Identity library](https://learn.microsoft.com/dotnet/azure/sdk/authentication/best-practices) | ||
|
|
||
| Use `ManagedIdentityCredential` directly for Azure-hosted workloads. For user-assigned | ||
| managed identities, pass the `client_id` explicitly. |
Member
There was a problem hiding this comment.
Object ID and resource ID are supported alternatives to client ID. We've encountered a few customers in constrained environments that can't use client ID.
| ```python | ||
| # Python — local dev only | ||
| from azure.identity import DefaultAzureCredential | ||
| credential = DefaultAzureCredential() |
Member
There was a problem hiding this comment.
For the local dev scenario, the best practice is to set env var AZURE_TOKEN_CREDENTIALS to dev. See https://learn.microsoft.com/azure/developer/python/sdk/authentication/credential-chains?tabs=dac#exclude-a-credential-type-category. Doing so disables the production-grade credentials at the front of the credential chain.
| ## DefaultAzureCredential (Local Development Only) | ||
|
|
||
| `DefaultAzureCredential` is convenient for local development because it automatically | ||
| falls through to developer credentials (Azure CLI, VS, etc.). **Do not use in production.** |
Member
There was a problem hiding this comment.
It's best not to list specific dev tool creds, since they can differ by language.
Suggested change
| falls through to developer credentials (Azure CLI, VS, etc.). **Do not use in production.** | |
| falls through to developer tool credentials. **Do not use in production.** |
| | Language | Package | Install | | ||
| |----------|---------|---------| | ||
| | Python | `azure-identity` | `pip install azure-identity` | | ||
| | .NET | `Azure.Identity` | `dotnet add package Azure.Identity` | |
Member
There was a problem hiding this comment.
Use .NET 10's version of the command instead (see https://learn.microsoft.com/dotnet/core/whats-new/dotnet-10/sdk#more-consistent-command-order)
Suggested change
| | .NET | `Azure.Identity` | `dotnet add package Azure.Identity` | | |
| | .NET | `Azure.Identity` | `dotnet package add Azure.Identity` | |
|
|
||
| 1. **Forgetting RBAC roles.** Enabling MI is step 1. Assigning the right role on the target resource is step 2. Most "MI doesn't work" issues are missing role assignments. | ||
| 2. **Overly broad roles.** `Contributor` on a resource group when you need `Storage Blob Data Reader` on one account. | ||
| 3. **Not testing locally.** `DefaultAzureCredential` falls through to Azure CLI. Make sure `az login` is done with the right subscription. |
Member
There was a problem hiding this comment.
Suggested change
| 3. **Not testing locally.** `DefaultAzureCredential` falls through to Azure CLI. Make sure `az login` is done with the right subscription. | |
| 3. **Not testing locally.** `DefaultAzureCredential` falls through to Azure CLI. Make sure `az login` is used with the right subscription. |
| | .NET | `Azure.Identity` | `dotnet add package Azure.Identity` | | ||
| | Java | `azure-identity` | Maven: `com.azure:azure-identity` | | ||
| | TypeScript | `@azure/identity` | `npm install @azure/identity` | | ||
| | Go | `azidentity` | `go get github.com/Azure/azure-sdk-for-go/sdk/azidentity` | |
Member
There was a problem hiding this comment.
Add C++ to the table:
Suggested change
| | Go | `azidentity` | `go get github.com/Azure/azure-sdk-for-go/sdk/azidentity` | | |
| | Go | `azidentity` | `go get github.com/Azure/azure-sdk-for-go/sdk/azidentity` | | |
| | C++ | `azure-identity-cpp` | `vcpkg add port azure-identity-cpp` | |
| ```csharp | ||
| // C# — production | ||
| var credential = new ManagedIdentityCredential(); // system-assigned | ||
| // var credential = new ManagedIdentityCredential("<user-assigned-mi-client-id>"); // user-assigned |
Member
There was a problem hiding this comment.
This is an older pattern that we no longer recommend. Do this instead:
Suggested change
| // var credential = new ManagedIdentityCredential("<user-assigned-mi-client-id>"); // user-assigned | |
| // var credential = new ManagedIdentityCredential(ManagedIdentityId.FromUserAssignedClientId("<user-assigned-mi-client-id>")); // user-assigned |
Comment on lines
+99
to
+100
| TokenCredential credential = new ManagedIdentityCredentialBuilder().build(); // system-assigned | ||
| // TokenCredential credential = new ManagedIdentityCredentialBuilder() |
Member
There was a problem hiding this comment.
Suggested change
| TokenCredential credential = new ManagedIdentityCredentialBuilder().build(); // system-assigned | |
| // TokenCredential credential = new ManagedIdentityCredentialBuilder() | |
| ManagedIdentityCredential credential = new ManagedIdentityCredentialBuilder().build(); // system-assigned | |
| // ManagedIdentityCredential credential = new ManagedIdentityCredentialBuilder() |
| // TypeScript — production | ||
| import { ManagedIdentityCredential } from "@azure/identity"; | ||
| const credential = new ManagedIdentityCredential(); // system-assigned | ||
| // const credential = new ManagedIdentityCredential("<user-assigned-mi-client-id>"); // user-assigned |
Member
There was a problem hiding this comment.
Suggested change
| // const credential = new ManagedIdentityCredential("<user-assigned-mi-client-id>"); // user-assigned | |
| // const credential = new ManagedIdentityCredential({ clientId: "<user-assigned-mi-client-id>" }); // user-assigned |
|
|
||
| | File | Contents | | ||
| |------|----------| | ||
| | [references/migration-patterns.md](references/migration-patterns.md) | Detailed before/after code for SQL, Storage, Cosmos DB, Service Bus, Event Hubs, Key Vault | |
Member
There was a problem hiding this comment.
The acceptance criteria file should also be referenced in this table
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Switch credential-free-dev skill to recommend ManagedIdentityCredential for production workloads, keeping DefaultAzureCredential for local dev only. Per Azure Identity authentication best practices: DefaultAzureCredential's credential chain probing can cause latency, silent fallback to unintended credentials, and hard-to-diagnose production failures. Updated all 3 skill files: SKILL.md (new production auth section, updated principles and code samples), migration-patterns.md (all AFTER examples now use ManagedIdentityCredential across 7 services and 4 languages), acceptance-criteria.md (ManagedIdentityCredential as correct production pattern, DefaultAzureCredential in production as incorrect).