Skip to content

Commit

Permalink
[Identity] DefaultAzureCredential should properly handle the managedl…
Browse files Browse the repository at this point in the history
…dentityClientId optional parameter (#13973)

While checking if the `DefaultAzureCredential` had issues while it's used on the Azure Functions, I realized we weren't handling the `managedIdentityClientId` optional parameter properly as well as we we're currently handling the `process.env.AZURE_CLIENT_ID`.

This pull request fixes this.

Related to: #13872
  • Loading branch information
sadasant authored Feb 26, 2021
1 parent 1cacc2b commit b789438
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
3 changes: 2 additions & 1 deletion sdk/identity/identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@

## 1.2.4-beta.2 (Unreleased)

- Replaced the use of the 'express' module with a Node-native http server, shrinking the resulting identity module considerably
- Bug fix: Now if the `managedIdentityClientId` optional parameter is provided to `DefaultAzureCredential`, it will be properly passed through to the underlying `ManagedIdentityCredential`. Related to customer issue: [13973](https://github.com/Azure/azure-sdk-for-js/pull/13973).
- `DefaultAzureCredential`'s implementation for browsers was simplified to throw a simple error instead of trying credentials that were already not supported for the browser.
- Breaking Change: `InteractiveBrowserCredential` for the browser now requires the client ID to be provided.
- Documentation was added to elaborate on how to configure an AAD application to support `InteractiveBrowserCredential`.
- Replaced the use of the 'express' module with a Node-native http server, shrinking the resulting identity module considerably
- Bug fix: `ManagedIdentityCredential` now also properly handles `EHOSTUNREACH` errors. Fixes issue [13894](https://github.com/Azure/azure-sdk-for-js/issues/13894).

## 1.2.4-beta.1 (2021-02-12)
Expand Down
17 changes: 11 additions & 6 deletions sdk/identity/identity/src/credentials/defaultAzureCredential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,20 @@ export class DefaultAzureCredential extends ChainedTokenCredential {
constructor(tokenCredentialOptions?: DefaultAzureCredentialOptions) {
const credentials = [];
credentials.push(new EnvironmentCredential(tokenCredentialOptions));
credentials.push(new ManagedIdentityCredential(tokenCredentialOptions));
if (process.env.AZURE_CLIENT_ID) {

// In case a user assigned ID has been provided.
const managedIdentityClientId =
tokenCredentialOptions?.managedIdentityClientId || process.env.AZURE_CLIENT_ID;

if (managedIdentityClientId) {
credentials.push(
new ManagedIdentityCredential(
tokenCredentialOptions?.managedIdentityClientId || process.env.AZURE_CLIENT_ID,
tokenCredentialOptions
)
new ManagedIdentityCredential(managedIdentityClientId, tokenCredentialOptions)
);
} else {
// If the user didn't provide an ID, we'll try with a system assigned ID.
credentials.push(new ManagedIdentityCredential(tokenCredentialOptions));
}

credentials.push(new AzureCliCredential());
credentials.push(new VisualStudioCodeCredential(tokenCredentialOptions));

Expand Down

0 comments on commit b789438

Please sign in to comment.