diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index 44b27d1af370..f37b92839ad5 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -1,5 +1,10 @@ # Release History +## 1.2.4 (2021-03-08) + +- 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: [13872](https://github.com/Azure/azure-sdk-for-js/issues/13872). +- Bug fix: `ManagedIdentityCredential` now also properly handles `EHOSTUNREACH` errors. Fixes issue [13894](https://github.com/Azure/azure-sdk-for-js/issues/13894). + ## 1.2.3 (2021-02-09) - Fixed Azure Stack support for the NodeJS version of the `InteractiveBrowserCredential`. diff --git a/sdk/identity/identity/package.json b/sdk/identity/identity/package.json index 7c1a0dd70a9a..f82fe1a31f09 100644 --- a/sdk/identity/identity/package.json +++ b/sdk/identity/identity/package.json @@ -1,7 +1,7 @@ { "name": "@azure/identity", "sdk-type": "client", - "version": "1.2.3", + "version": "1.2.4", "description": "Provides credential implementations for Azure SDK libraries that can authenticate with Azure Active Directory", "main": "dist/index.js", "module": "dist-esm/src/index.js", diff --git a/sdk/identity/identity/rollup.base.config.js b/sdk/identity/identity/rollup.base.config.js index 763201886cff..444bea458f15 100644 --- a/sdk/identity/identity/rollup.base.config.js +++ b/sdk/identity/identity/rollup.base.config.js @@ -38,7 +38,8 @@ export function nodeConfig(test = false) { baseConfig.input = [ "dist-esm/test/public/*.spec.js", "dist-esm/test/public/node/*.spec.js", - "dist-esm/test/internal/*.spec.js" + "dist-esm/test/internal/*.spec.js", + "dist-esm/test/internal/node/*.spec.js" ]; baseConfig.plugins.unshift(multiEntry({ exports: false })); @@ -97,7 +98,8 @@ export function browserConfig(test = false) { baseConfig.input = [ "dist-esm/test/public/*.spec.js", "dist-esm/test/public/browser/*.spec.js", - "dist-esm/test/internal/*.spec.js" + "dist-esm/test/internal/*.spec.js", + "dist-esm/test/internal/browser/*.spec.js" ]; baseConfig.plugins.unshift(multiEntry({ exports: false })); baseConfig.output.file = "test-browser/index.js"; diff --git a/sdk/identity/identity/src/credentials/defaultAzureCredential.ts b/sdk/identity/identity/src/credentials/defaultAzureCredential.ts index 6641f4d1b23c..f5688bd9c6c6 100644 --- a/sdk/identity/identity/src/credentials/defaultAzureCredential.ts +++ b/sdk/identity/identity/src/credentials/defaultAzureCredential.ts @@ -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)); diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts index 58ecfef907a4..1337b188b51b 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts @@ -196,7 +196,20 @@ export class ManagedIdentityCredential implements TokenCredential { message: err.message }); + // If either the network is unreachable, + // we can safely assume the credential is unavailable. if (err.code === "ENETUNREACH") { + const error = new CredentialUnavailable( + "ManagedIdentityCredential is unavailable. Network unreachable." + ); + + logger.getToken.info(formatError(scopes, error)); + throw error; + } + + // If either the host was unreachable, + // we can safely assume the credential is unavailable. + if (err.code === "EHOSTUNREACH") { const error = new CredentialUnavailable( "ManagedIdentityCredential is unavailable. No managed identity endpoint found." ); @@ -213,6 +226,15 @@ export class ManagedIdentityCredential implements TokenCredential { ); } + // If the error has no status code, we can assume there was no available identity. + // This will throw silently during any ChainedTokenCredential. + if (err.statusCode === undefined) { + throw new CredentialUnavailable( + `ManagedIdentityCredential authentication failed. Message ${err.message}` + ); + } + + // Any other error should break the chain. throw new AuthenticationError(err.statusCode, { error: "ManagedIdentityCredential authentication failed.", error_description: err.message diff --git a/sdk/identity/identity/test/authTestUtils.ts b/sdk/identity/identity/test/authTestUtils.ts index 068a3d9102fd..d60dea1d123e 100644 --- a/sdk/identity/identity/test/authTestUtils.ts +++ b/sdk/identity/identity/test/authTestUtils.ts @@ -14,7 +14,8 @@ import { import * as coreHttp from "@azure/core-http"; export interface MockAuthResponse { - status: number; + status?: number; + error?: RestError; headers?: HttpHeaders; parsedBody?: any; bodyAsText?: string; @@ -27,7 +28,7 @@ export interface MockAuthHttpClientOptions { export class MockAuthHttpClient implements HttpClient { private authResponses: MockAuthResponse[] = []; - private currentResponse: number = 0; + private currentResponseIndex: number = 0; private mockTimeout: boolean; public tokenCredentialOptions: ClientCertificateCredentialOptions; @@ -76,13 +77,22 @@ export class MockAuthHttpClient implements HttpClient { throw new Error("The number of requests has exceeded the number of authResponses"); } + const authResponse = this.authResponses[this.currentResponseIndex]; + + if (authResponse.error) { + this.currentResponseIndex++; + throw authResponse.error; + } + const response = { request: httpRequest, - headers: this.authResponses[this.currentResponse].headers || new HttpHeaders(), - ...this.authResponses[this.currentResponse] + headers: authResponse.headers || new HttpHeaders(), + status: authResponse.status || 200, + parsedBody: authResponse.parsedBody, + bodyAsText: authResponse.bodyAsText }; - this.currentResponse++; + this.currentResponseIndex++; return response; } } diff --git a/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts b/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts index ca8d663fdc3a..01dd445f35de 100644 --- a/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts +++ b/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts @@ -9,7 +9,7 @@ import { imdsApiVersion } from "../../../src/credentials/managedIdentityCredential/constants"; import { MockAuthHttpClient, MockAuthHttpClientOptions, assertRejects } from "../../authTestUtils"; -import { WebResource, AccessToken, HttpHeaders } from "@azure/core-http"; +import { WebResource, AccessToken, HttpHeaders, RestError } from "@azure/core-http"; import { OAuthErrorResponse } from "../../../src/client/errors"; interface AuthRequestDetails { @@ -17,7 +17,7 @@ interface AuthRequestDetails { token: AccessToken | null; } -describe("ManagedIdentityCredential", function() { +describe("ManagedIdentityCredential", function () { afterEach(() => { delete process.env.IDENTITY_ENDPOINT; delete process.env.IDENTITY_HEADER; @@ -26,7 +26,7 @@ describe("ManagedIdentityCredential", function() { delete process.env.IDENTITY_SERVER_THUMBPRINT; }); - it("sends an authorization request with a modified resource name", async function() { + it("sends an authorization request with a modified resource name", async function () { const authDetails = await getMsiTokenAuthRequest(["https://service/.default"], "client", { authResponse: [ { status: 200 }, // Respond to IMDS isAvailable @@ -83,7 +83,24 @@ describe("ManagedIdentityCredential", function() { } }); - it("returns error when ManagedIdentityCredential authentication failed", async function() { + it("returns error when no MSI is available", async function () { + process.env.AZURE_CLIENT_ID = "errclient"; + + const imdsError: RestError = new RestError("Request Timeout", "REQUEST_SEND_ERROR", 408); + const mockHttpClient = new MockAuthHttpClient({ + authResponse: [{ error: imdsError }] + }); + + const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, { + ...mockHttpClient.tokenCredentialOptions + }); + await assertRejects( + credential.getToken("scopes"), + (error: AuthenticationError) => error.message.indexOf("No MSI credential available") > -1 + ); + }); + + it("an unexpected error bubbles all the way up", async function () { process.env.AZURE_CLIENT_ID = "errclient"; const errResponse: OAuthErrorResponse = { @@ -92,7 +109,41 @@ describe("ManagedIdentityCredential", function() { }; const mockHttpClient = new MockAuthHttpClient({ - authResponse: [{ status: 400, parsedBody: errResponse }] + authResponse: [{ status: 200 }, { status: 500, parsedBody: errResponse }] + }); + + const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, { + ...mockHttpClient.tokenCredentialOptions + }); + await assertRejects( + credential.getToken("scopes"), + (error: AuthenticationError) => error.message.indexOf(errResponse.error) > -1 + ); + }); + + it("returns expected error when the network was unreachable", async function () { + process.env.AZURE_CLIENT_ID = "errclient"; + + const netError: RestError = new RestError("Request Timeout", "ENETUNREACH", 408); + const mockHttpClient = new MockAuthHttpClient({ + authResponse: [{ status: 200 }, { error: netError }] + }); + + const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, { + ...mockHttpClient.tokenCredentialOptions + }); + await assertRejects( + credential.getToken("scopes"), + (error: AuthenticationError) => error.message.indexOf("Network unreachable.") > -1 + ); + }); + + it("returns expected error when the host was unreachable", async function () { + process.env.AZURE_CLIENT_ID = "errclient"; + + const hostError: RestError = new RestError("Request Timeout", "EHOSTUNREACH", 408); + const mockHttpClient = new MockAuthHttpClient({ + authResponse: [{ status: 200 }, { error: hostError }] }); const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, { @@ -101,7 +152,7 @@ describe("ManagedIdentityCredential", function() { await assertRejects( credential.getToken("scopes"), (error: AuthenticationError) => - error.errorResponse.error.indexOf("ManagedIdentityCredential authentication failed.") > -1 + error.message.indexOf("No managed identity endpoint found.") > -1 ); });