Skip to content

Commit d678c90

Browse files
Jonathan Turnerzzhxiaofeng
Jonathan Turner
and
zzhxiaofeng
authored
[Identity] Default credential error msg (Azure#9374)
* 123 * update * Simplify azure-identity error messaging * Simplify azure-identity error messaging * Simplify azure-identity error messaging * Simplify azure-identity error messaging * Simplify azure-identity error messaging * Simplify azure-identity error messaging * Simplify azure-identity error messaging * Simplify azure-identity error messaging * Azure.Identity simplify exception messaging * Azure.Identity simplify exception messaging * Azure.Identity simplify exception messaging * Simple change to try to re-run CI * Get building and test passing * Simplify error message and properly use CredentialUnavailable * Fix test Co-authored-by: Ziheng Zhou <v-zihz@microsoft.com> Co-authored-by: zzhxiaofeng <874256244@qq.com>
1 parent 8f7bcf8 commit d678c90

File tree

9 files changed

+117
-70
lines changed

9 files changed

+117
-70
lines changed

‎sdk/identity/identity/review/identity.api.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export { AccessToken }
1313

1414
// @public
1515
export class AggregateAuthenticationError extends Error {
16-
constructor(errors: any[]);
16+
constructor(errors: any[], errMsg?: string);
1717
errors: any[];
1818
}
1919

@@ -51,7 +51,8 @@ export type BrowserLoginStyle = "redirect" | "popup";
5151
export class ChainedTokenCredential implements TokenCredential {
5252
constructor(...sources: TokenCredential[]);
5353
getToken(scopes: string | string[], options?: GetTokenOptions): Promise<AccessToken | null>;
54-
}
54+
protected UnavailableMessage: string;
55+
}
5556

5657
// @public
5758
export class ClientCertificateCredential implements TokenCredential {

‎sdk/identity/identity/src/client/errors.ts‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,7 @@ export class AuthenticationError extends Error {
126126
}
127127

128128
super(
129-
`An error was returned while authenticating to Azure Active Directory (status code ${statusCode}).\n\nMore details:\n\n${JSON.stringify(
130-
errorResponse,
131-
null,
132-
" "
133-
)}`
129+
`${errorResponse.error}(status code ${statusCode}).\nMore details:\n${errorResponse.errorDescription}`
134130
);
135131
this.statusCode = statusCode;
136132
this.errorResponse = errorResponse;
@@ -156,10 +152,11 @@ export class AggregateAuthenticationError extends Error {
156152
*/
157153
public errors: any[];
158154

159-
constructor(errors: any[]) {
160-
super(
161-
`Authentication failed to complete due to the following errors:\n\n${errors.join("\n\n")}`
162-
);
155+
constructor(errors: any[], errMsg?: string) {
156+
let errorDetail =
157+
errors
158+
.join("\n");
159+
super(`${errMsg}\n\n${errorDetail}`);
163160
this.errors = errors;
164161

165162
// Ensure that this type reports the correct name

‎sdk/identity/identity/src/credentials/chainedTokenCredential.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ import { CanonicalCode } from "@opentelemetry/api";
1111
* until one of the getToken methods returns an access token.
1212
*/
1313
export class ChainedTokenCredential implements TokenCredential {
14+
/**
15+
* The message to use when the chained token fails to get a token
16+
*/
17+
protected UnavailableMessage =
18+
"ChainedTokenCredential failed to retrieve a token from the included credentials";
1419
private _sources: TokenCredential[] = [];
1520

1621
/**

‎sdk/identity/identity/src/credentials/defaultAzureCredential.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ export class DefaultAzureCredential extends ChainedTokenCredential {
3131
credentials.push(new EnvironmentCredential(tokenCredentialOptions));
3232
credentials.push(new ManagedIdentityCredential(tokenCredentialOptions));
3333
if (process.env.AZURE_CLIENT_ID) {
34-
credentials.push(new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, tokenCredentialOptions));
34+
credentials.push(
35+
new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, tokenCredentialOptions)
36+
);
3537
}
3638
credentials.push(new AzureCliCredential());
3739
credentials.push(new VSCodeCredential(tokenCredentialOptions));
@@ -48,5 +50,7 @@ export class DefaultAzureCredential extends ChainedTokenCredential {
4850
super(
4951
...credentials
5052
);
53+
this.UnavailableMessage =
54+
"DefaultAzureCredential failed to retrieve a token from the included credentials";
5155
}
5256
}

‎sdk/identity/identity/src/credentials/environmentCredential.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,13 @@ export class EnvironmentCredential implements TokenCredential {
125125
code,
126126
message: err.message
127127
});
128-
throw err;
128+
throw new AuthenticationError(400, {
129+
error: "EnvironmentCredential authentication failed.",
130+
error_description: err.message
131+
.toString()
132+
.split("More details:")
133+
.join("")
134+
});
129135
} finally {
130136
span.end();
131137
}
@@ -135,11 +141,6 @@ export class EnvironmentCredential implements TokenCredential {
135141
// the user knows the credential was not configured appropriately
136142
span.setStatus({ code: CanonicalCode.UNAUTHENTICATED });
137143
span.end();
138-
throw new CredentialUnavailable(`EnvironmentCredential cannot return a token because one or more of the following environment variables is missing:
139-
140-
${this._environmentVarsMissing.join("\n")}
141-
142-
To authenticate with a service principal AZURE_TENANT_ID, AZURE_CLIENT_ID, and either AZURE_CLIENT_SECRET or AZURE_CLIENT_CERTIFICATE_PATH must be set. To authenticate with a user account AZURE_TENANT_ID, AZURE_USERNAME, and AZURE_PASSWORD must be set.
143-
`);
144+
throw new CredentialUnavailable("EnvironmentCredential is unavailable. Environment variables are not fully configured.");
144145
}
145146
}

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
} from "@azure/core-http";
1212
import { IdentityClient, TokenCredentialOptions } from "../client/identityClient";
1313
import { createSpan } from "../util/tracing";
14-
import { AuthenticationErrorName, CredentialUnavailable } from "../client/errors";
14+
import { AuthenticationErrorName, AuthenticationError, CredentialUnavailable } from "../client/errors";
1515
import { CanonicalCode } from "@opentelemetry/api";
1616
import { logger } from "../util/logging";
1717

@@ -337,15 +337,24 @@ export class ManagedIdentityCredential implements TokenCredential {
337337
} else {
338338
throw new CredentialUnavailable("The managed identity endpoint is not currently available");
339339
}
340-
341340
return result;
342341
} catch (err) {
343342
span.setStatus({
344343
code: CanonicalCode.UNKNOWN,
345344
message: err.message
346345
});
347-
throw err;
346+
347+
if (err.code == "ENETUNREACH") {
348+
throw new CredentialUnavailable("ManagedIdentityCredential is unavailable. No managed identity endpoint found.");
349+
}
350+
throw new AuthenticationError(400, {
351+
error: "ManagedIdentityCredential authentication failed.",
352+
error_description: err.message
353+
});
348354
} finally {
355+
if (this.isEndpointUnavailable) {
356+
throw new CredentialUnavailable("ManagedIdentityCredential is unavailable. No managed identity endpoint found.");
357+
}
349358
span.end();
350359
}
351360
}

‎sdk/identity/identity/test/errors.spec.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@ describe("AggregateAuthenticationError", function() {
1111
new Error("Boom again.")
1212
]);
1313

14-
assert.strictEqual(
15-
aggregateError.message,
16-
"Authentication failed to complete due to the following errors:\n\nError: Boom.\n\nError: Boom again."
17-
);
14+
assert(aggregateError.message.includes("Error: Boom.\nError: Boom again."));
1815
});
1916
});

‎sdk/identity/identity/test/node/environmentCredential.spec.ts‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
assertRejects
1212
} from "../authTestUtils";
1313
import { TestTracer, setTracer, SpanGraph } from "@azure/core-tracing";
14-
import { AllSupportedEnvironmentVariables } from "../../src/credentials/environmentCredential";
14+
import { OAuthErrorResponse } from "../../src/client/errors";
1515

1616
describe("EnvironmentCredential", function() {
1717
it("finds and uses client credential environment variables", async () => {
@@ -143,8 +143,9 @@ describe("EnvironmentCredential", function() {
143143
await assertRejects(
144144
credential.getToken("scope"),
145145
(error: CredentialUnavailable) =>
146-
error.message.indexOf(AllSupportedEnvironmentVariables.join("\n")) >
147-
-1
146+
error.message.indexOf(
147+
"EnvironmentCredential is unavailable. Environment variables are not fully configured."
148+
) > -1
148149
);
149150

150151
process.env.AZURE_TENANT_ID = "It me";
@@ -153,9 +154,33 @@ describe("EnvironmentCredential", function() {
153154
await assertRejects(
154155
credentialDeux.getToken("scope"),
155156
(error: CredentialUnavailable) =>
156-
error.message.match(/^AZURE_TENANT_ID/gm) === null
157+
error.message.indexOf(
158+
"EnvironmentCredential is unavailable. Environment variables are not fully configured."
159+
) > -1
157160
);
158161

159162
delete process.env.AZURE_TENANT_ID;
160163
});
164+
165+
it("throws an AuthenticationError when getToken is called and EnvironmentCredential authentication failed", async () => {
166+
process.env.AZURE_TENANT_ID = "tenant";
167+
process.env.AZURE_CLIENT_ID = "errclient";
168+
process.env.AZURE_CLIENT_SECRET = "secret";
169+
170+
const errResponse: OAuthErrorResponse = {
171+
error: "EnvironmentCredential authentication failed.",
172+
error_description: ""
173+
};
174+
175+
const mockHttpClient = new MockAuthHttpClient({
176+
authResponse: [{ status: 400, parsedBody: errResponse }]
177+
});
178+
179+
const credential = new EnvironmentCredential(mockHttpClient.tokenCredentialOptions);
180+
await assertRejects(
181+
credential.getToken("scope"),
182+
(error: AuthenticationError) =>
183+
error.errorResponse.error.indexOf("EnvironmentCredential authentication failed.") > -1
184+
);
185+
});
161186
});

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

Lines changed: 48 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33

44
import qs from "qs";
55
import assert from "assert";
6-
import { ManagedIdentityCredential, CredentialUnavailable } from "../../src";
6+
import { ManagedIdentityCredential, AuthenticationError, CredentialUnavailable } from "../../src";
77
import {
88
ImdsEndpoint,
99
ImdsApiVersion,
1010
AppServiceMsiApiVersion
1111
} from "../../src/credentials/managedIdentityCredential";
1212
import { MockAuthHttpClient, MockAuthHttpClientOptions, assertRejects } from "../authTestUtils";
1313
import { WebResource, AccessToken } from "@azure/core-http";
14+
import { OAuthErrorResponse } from "../../src/client/errors";
1415

1516
interface AuthRequestDetails {
1617
requests: WebResource[];
@@ -76,54 +77,61 @@ describe("ManagedIdentityCredential", function() {
7677
}
7778
});
7879

79-
it("returns null when IMDS endpoint can't be detected", async function() {
80-
// Mock a timeout so that the endpoint ping fails
81-
const authDetails = await getMsiTokenAuthRequest(["https://service/.default"], "client", {
82-
mockTimeout: true
83-
});
80+
it("returns error when ManagedIdentityCredential authentication failed", async function() {
81+
process.env.AZURE_CLIENT_ID = "errclient";
8482

85-
assert.strictEqual(authDetails.requests[0].timeout, 500);
86-
assert.strictEqual(authDetails.token, null);
87-
});
83+
const errResponse: OAuthErrorResponse = {
84+
error: "ManagedIdentityCredential authentication failed.",
85+
error_description: ""
86+
};
8887

89-
it("can extend timeout for IMDS endpoint", async function() {
90-
// Mock a timeout so that the endpoint ping fails
91-
const authDetails = await getMsiTokenAuthRequest(
92-
["https://service/.default"],
93-
"client",
94-
{ mockTimeout: true },
95-
5000
96-
); // Set the timeout higher
97-
98-
assert.strictEqual(authDetails.requests[0].timeout, 5000);
99-
assert.strictEqual(authDetails.token, null);
100-
});
88+
const mockHttpClient = new MockAuthHttpClient({
89+
authResponse: [{ status: 400, parsedBody: errResponse }]
90+
});
10191

102-
it("doesn't try IMDS endpoint again once it can't be detected", async function() {
103-
const mockHttpClient = new MockAuthHttpClient({ mockTimeout: true });
104-
const credential = new ManagedIdentityCredential("client", {
92+
const credential = new ManagedIdentityCredential(process.env.AZURE_CLIENT_ID, {
10593
...mockHttpClient.tokenCredentialOptions
10694
});
107-
108-
// Run getToken twice and verify that an auth request is only
109-
// attempted the first time. It should be skipped the second
110-
// time after no IMDS endpoint was found.
111-
await assertRejects(
112-
credential.getToken("scope"),
113-
(error: CredentialUnavailable) =>
114-
error.message.indexOf("The managed identity endpoint is not currently available") == 0
115-
);
116-
117-
11895
await assertRejects(
119-
credential.getToken("scope"),
120-
(error: CredentialUnavailable) =>
121-
error.message.indexOf("The managed identity endpoint is not currently available") == 0
96+
credential.getToken("scopes"),
97+
(error: AuthenticationError) =>
98+
error.errorResponse.error.indexOf("ManagedIdentityCredential authentication failed.") > -1
12299
);
123-
124-
assert.strictEqual(mockHttpClient.requests.length, 1);
125100
});
126101

102+
// Unavailable exception throws while IMDS endpoint is unavailable. This test not valid.
103+
// it("can extend timeout for IMDS endpoint", async function() {
104+
// // Mock a timeout so that the endpoint ping fails
105+
// const authDetails = await getMsiTokenAuthRequest(
106+
// ["https://service/.default"],
107+
// "client",
108+
// { mockTimeout: true },
109+
// 5000
110+
// ); // Set the timeout higher
111+
112+
// assert.strictEqual(authDetails.requests[0].timeout, 5000);
113+
// assert.strictEqual(authDetails.token, null);
114+
// });
115+
116+
// unavailable exception throws while IMDS endpoint is unavailable. This test not valid.
117+
// it("doesn't try IMDS endpoint again once it can't be detected", async function() {
118+
// const mockHttpClient = new MockAuthHttpClient({ mockTimeout: true });
119+
// const credential = new ManagedIdentityCredential("client", {
120+
// ...mockHttpClient.tokenCredentialOptions
121+
// });
122+
123+
// // Run getToken twice and verify that an auth request is only
124+
// // attempted the first time. It should be skipped the second
125+
// // time after no IMDS endpoint was found.
126+
127+
// const firstGetToken = await credential.getToken("scopes");
128+
// const secondGetToken = await credential.getToken("scopes");
129+
130+
// assert.strictEqual(firstGetToken, null);
131+
// assert.strictEqual(secondGetToken, null);
132+
// assert.strictEqual(mockHttpClient.requests.length, 1);
133+
// });
134+
127135
it("sends an authorization request correctly in an App Service environment", async () => {
128136
// Trigger App Service behavior by setting environment variables
129137
process.env.MSI_ENDPOINT = "https://endpoint";

0 commit comments

Comments
 (0)