Skip to content

Commit 39d7184

Browse files
authored
[Identity] ManagedIdentityCredential fix (Azure#13919)
This pull request makes the ManagedIdentityCredential: - Treat unreachable host as we were treating unreachable network. - This should have been done before, but we didn't catch it. - I couldn't find any other error that seemed relevant to me, using this as reference: [link](https://github.com/nodejs/node/blob/606df7c4e79324b9725bfcfe019a8b75bfa04c3f/deps/uv/src/win/error.c). - Treat errors with undefined status code as if the credential was unavailable, to avoid breaking the chained token credentials in that case. - Add tests (turns out that some where being skipped by mistake!) - Add more comments to improve reading. This PR: Fixes Azure#13894
1 parent 2930b04 commit 39d7184

File tree

5 files changed

+97
-11
lines changed

5 files changed

+97
-11
lines changed

sdk/identity/identity/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- `DefaultAzureCredential`'s implementation for browsers was simplified to throw a simple error instead of trying credentials that were already not supported for the browser.
88
- Breaking Change: `InteractiveBrowserCredential` for the browser now requires the client ID to be provided.
99
- Documentation was added to elaborate on how to configure an AAD application to support `InteractiveBrowserCredential`.
10+
- Bug fix: `ManagedIdentityCredential` now also properly handles `EHOSTUNREACH` errors. Fixes issue [13894](https://github.com/Azure/azure-sdk-for-js/issues/13894).
1011

1112
## 1.2.4-beta.1 (2021-02-12)
1213

sdk/identity/identity/rollup.base.config.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ export function nodeConfig(test = false) {
3838
baseConfig.input = [
3939
"dist-esm/test/public/*.spec.js",
4040
"dist-esm/test/public/node/*.spec.js",
41-
"dist-esm/test/internal/*.spec.js"
41+
"dist-esm/test/internal/*.spec.js",
42+
"dist-esm/test/internal/node/*.spec.js"
4243
];
4344
baseConfig.plugins.unshift(multiEntry({ exports: false }));
4445

@@ -97,7 +98,8 @@ export function browserConfig(test = false) {
9798
baseConfig.input = [
9899
"dist-esm/test/public/*.spec.js",
99100
"dist-esm/test/public/browser/*.spec.js",
100-
"dist-esm/test/internal/*.spec.js"
101+
"dist-esm/test/internal/*.spec.js",
102+
"dist-esm/test/internal/browser/*.spec.js"
101103
];
102104
baseConfig.plugins.unshift(multiEntry({ exports: false }));
103105
baseConfig.output.file = "test-browser/index.js";

sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,20 @@ export class ManagedIdentityCredential implements TokenCredential {
196196
message: err.message
197197
});
198198

199+
// If either the network is unreachable,
200+
// we can safely assume the credential is unavailable.
199201
if (err.code === "ENETUNREACH") {
202+
const error = new CredentialUnavailable(
203+
"ManagedIdentityCredential is unavailable. Network unreachable."
204+
);
205+
206+
logger.getToken.info(formatError(scopes, error));
207+
throw error;
208+
}
209+
210+
// If either the host was unreachable,
211+
// we can safely assume the credential is unavailable.
212+
if (err.code === "EHOSTUNREACH") {
200213
const error = new CredentialUnavailable(
201214
"ManagedIdentityCredential is unavailable. No managed identity endpoint found."
202215
);
@@ -213,6 +226,15 @@ export class ManagedIdentityCredential implements TokenCredential {
213226
);
214227
}
215228

229+
// If the error has no status code, we can assume there was no available identity.
230+
// This will throw silently during any ChainedTokenCredential.
231+
if (err.statusCode === undefined) {
232+
throw new CredentialUnavailable(
233+
`ManagedIdentityCredential authentication failed. Message ${err.message}`
234+
);
235+
}
236+
237+
// Any other error should break the chain.
216238
throw new AuthenticationError(err.statusCode, {
217239
error: "ManagedIdentityCredential authentication failed.",
218240
error_description: err.message

sdk/identity/identity/test/authTestUtils.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import {
1414
import * as coreHttp from "@azure/core-http";
1515

1616
export interface MockAuthResponse {
17-
status: number;
17+
status?: number;
18+
error?: RestError;
1819
headers?: HttpHeaders;
1920
parsedBody?: any;
2021
bodyAsText?: string;
@@ -27,7 +28,7 @@ export interface MockAuthHttpClientOptions {
2728

2829
export class MockAuthHttpClient implements HttpClient {
2930
private authResponses: MockAuthResponse[] = [];
30-
private currentResponse: number = 0;
31+
private currentResponseIndex: number = 0;
3132
private mockTimeout: boolean;
3233

3334
public tokenCredentialOptions: ClientCertificateCredentialOptions;
@@ -76,13 +77,22 @@ export class MockAuthHttpClient implements HttpClient {
7677
throw new Error("The number of requests has exceeded the number of authResponses");
7778
}
7879

80+
const authResponse = this.authResponses[this.currentResponseIndex];
81+
82+
if (authResponse.error) {
83+
this.currentResponseIndex++;
84+
throw authResponse.error;
85+
}
86+
7987
const response = {
8088
request: httpRequest,
81-
headers: this.authResponses[this.currentResponse].headers || new HttpHeaders(),
82-
...this.authResponses[this.currentResponse]
89+
headers: authResponse.headers || new HttpHeaders(),
90+
status: authResponse.status || 200,
91+
parsedBody: authResponse.parsedBody,
92+
bodyAsText: authResponse.bodyAsText
8393
};
8494

85-
this.currentResponse++;
95+
this.currentResponseIndex++;
8696
return response;
8797
}
8898
}

sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
imdsApiVersion
1010
} from "../../../src/credentials/managedIdentityCredential/constants";
1111
import { MockAuthHttpClient, MockAuthHttpClientOptions, assertRejects } from "../../authTestUtils";
12-
import { WebResource, AccessToken, HttpHeaders } from "@azure/core-http";
12+
import { WebResource, AccessToken, HttpHeaders, RestError } from "@azure/core-http";
1313
import { OAuthErrorResponse } from "../../../src/client/errors";
1414

1515
interface AuthRequestDetails {
@@ -83,7 +83,24 @@ describe("ManagedIdentityCredential", function() {
8383
}
8484
});
8585

86-
it("returns error when ManagedIdentityCredential authentication failed", async function() {
86+
it("returns error when no MSI is available", async function() {
87+
process.env.AZURE_CLIENT_ID = "errclient";
88+
89+
const imdsError: RestError = new RestError("Request Timeout", "REQUEST_SEND_ERROR", 408);
90+
const mockHttpClient = new MockAuthHttpClient({
91+
authResponse: [{ error: imdsError }]
92+
});
93+
94+
const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, {
95+
...mockHttpClient.tokenCredentialOptions
96+
});
97+
await assertRejects(
98+
credential.getToken("scopes"),
99+
(error: AuthenticationError) => error.message.indexOf("No MSI credential available") > -1
100+
);
101+
});
102+
103+
it("an unexpected error bubbles all the way up", async function() {
87104
process.env.AZURE_CLIENT_ID = "errclient";
88105

89106
const errResponse: OAuthErrorResponse = {
@@ -92,7 +109,41 @@ describe("ManagedIdentityCredential", function() {
92109
};
93110

94111
const mockHttpClient = new MockAuthHttpClient({
95-
authResponse: [{ status: 400, parsedBody: errResponse }]
112+
authResponse: [{ status: 200 }, { status: 500, parsedBody: errResponse }]
113+
});
114+
115+
const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, {
116+
...mockHttpClient.tokenCredentialOptions
117+
});
118+
await assertRejects(
119+
credential.getToken("scopes"),
120+
(error: AuthenticationError) => error.message.indexOf(errResponse.error) > -1
121+
);
122+
});
123+
124+
it("returns expected error when the network was unreachable", async function() {
125+
process.env.AZURE_CLIENT_ID = "errclient";
126+
127+
const netError: RestError = new RestError("Request Timeout", "ENETUNREACH", 408);
128+
const mockHttpClient = new MockAuthHttpClient({
129+
authResponse: [{ status: 200 }, { error: netError }]
130+
});
131+
132+
const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, {
133+
...mockHttpClient.tokenCredentialOptions
134+
});
135+
await assertRejects(
136+
credential.getToken("scopes"),
137+
(error: AuthenticationError) => error.message.indexOf("Network unreachable.") > -1
138+
);
139+
});
140+
141+
it("returns expected error when the host was unreachable", async function() {
142+
process.env.AZURE_CLIENT_ID = "errclient";
143+
144+
const hostError: RestError = new RestError("Request Timeout", "EHOSTUNREACH", 408);
145+
const mockHttpClient = new MockAuthHttpClient({
146+
authResponse: [{ status: 200 }, { error: hostError }]
96147
});
97148

98149
const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, {
@@ -101,7 +152,7 @@ describe("ManagedIdentityCredential", function() {
101152
await assertRejects(
102153
credential.getToken("scopes"),
103154
(error: AuthenticationError) =>
104-
error.errorResponse.error.indexOf("ManagedIdentityCredential authentication failed.") > -1
155+
error.message.indexOf("No managed identity endpoint found.") > -1
105156
);
106157
});
107158

0 commit comments

Comments
 (0)