Skip to content

Commit e8cac41

Browse files
bmahallbmahall
authored
Merge branch 'dev' into tokenSizesRTModMerged
2 parents 2002ee9 + 8ddf7c0 commit e8cac41

File tree

8 files changed

+42
-22
lines changed

8 files changed

+42
-22
lines changed

‎change/@azure-msal-common-ce351b78-515d-41e1-a614-9593bd8b2a3f.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "none",
3+
"comment": "Undo changes in error classes to avoid custom description of server errors. #5175",
4+
"packageName": "@azure/msal-common",
5+
"email": "[email protected]",
6+
"dependentChangeType": "none"
7+
}

‎change/@azure-msal-node-b1f6afd4-c1f6-408a-82a9-6d9e8a063035.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "This PR modifies HttpClient to return apprpropriate underlying error that is handled upstream. #5175",
4+
"packageName": "@azure/msal-node",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎lib/msal-common/src/error/ClientAuthError.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ export class ClientAuthError extends AuthError {
249249
* Creates an error thrown when the id token string is null or empty.
250250
* @param invalidRawTokenString
251251
*/
252-
static createTokenNullOrEmptyError(invalidRawTokenString: string) : ClientAuthError {
252+
static createTokenNullOrEmptyError(invalidRawTokenString: string): ClientAuthError {
253253
return new ClientAuthError(ClientAuthErrorMessage.nullOrEmptyToken.code,
254254
`${ClientAuthErrorMessage.nullOrEmptyToken.desc} Raw Token Value: ${invalidRawTokenString}`);
255255
}

‎lib/msal-common/src/error/InteractionRequiredAuthError.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export class InteractionRequiredAuthError extends AuthError {
5454
* @param errorString
5555
* @param subError
5656
*/
57-
static isInteractionRequiredError(errorCode?: string, errorString?: string, subError?: string) : boolean {
57+
static isInteractionRequiredError(errorCode?: string, errorString?: string, subError?: string): boolean {
5858
const isInteractionRequiredErrorCode = !!errorCode && InteractionRequiredServerErrorMessage.indexOf(errorCode) > -1;
5959
const isInteractionRequiredSubError = !!subError && InteractionRequiredAuthSubErrorMessage.indexOf(subError) > -1;
6060
const isInteractionRequiredErrorDesc = !!errorString && InteractionRequiredServerErrorMessage.some((irErrorCode) => {

‎lib/msal-common/src/network/NetworkManager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import { INetworkModule, NetworkRequestOptions } from "./INetworkModule";
77
import { RequestThumbprint } from "./RequestThumbprint";
88
import { ThrottlingUtils } from "./ThrottlingUtils";
99
import { CacheManager } from "../cache/CacheManager";
10-
import { ClientAuthError } from "../error/ClientAuthError";
1110
import { AuthError } from "../error/AuthError";
11+
import { ClientAuthError } from "../error/ClientAuthError";
1212

1313
export type NetworkResponse<T> = {
1414
headers: Record<string, string>;

‎lib/msal-node/src/network/HttpClient.ts‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import {
77
INetworkModule,
88
NetworkRequestOptions,
9-
NetworkResponse,
9+
NetworkResponse
1010
} from "@azure/msal-common";
1111
import { HttpMethod, Constants } from "../utils/Constants";
1212
import http from "http";
@@ -107,9 +107,8 @@ const networkRequestViaProxy = <T>(
107107
if (statusCode < 200 || statusCode > 299) {
108108
request.destroy();
109109
socket.destroy();
110-
reject(new Error(`HTTP status code ${statusCode}`));
110+
reject(new Error(` Error connecting to proxy: ${response.statusCode}, ${response?.statusMessage}`));
111111
}
112-
113112
if (tunnelRequestOptions.timeout) {
114113
socket.setTimeout(tunnelRequestOptions.timeout);
115114
socket.on("timeout", () => {
@@ -175,15 +174,11 @@ const networkRequestViaProxy = <T>(
175174
body: JSON.parse(body) as T,
176175
status: statusCode as number,
177176
};
178-
179177
if ((statusCode < 200 || statusCode > 299) &&
180178
// do not destroy the request for the device code flow
181179
networkResponse.body["error"] !== Constants.AUTHORIZATION_PENDING) {
182180
request.destroy();
183-
socket.destroy();
184-
reject(new Error(`HTTP status code ${statusCode}`));
185181
}
186-
187182
resolve(networkResponse);
188183
});
189184

@@ -267,9 +262,7 @@ const networkRequestViaHttps = <T>(
267262
// do not destroy the request for the device code flow
268263
networkResponse.body["error"] !== Constants.AUTHORIZATION_PENDING) {
269264
request.destroy();
270-
reject(new Error(`HTTP status code ${statusCode}`));
271265
}
272-
273266
resolve(networkResponse);
274267
});
275268
});
@@ -280,3 +273,4 @@ const networkRequestViaHttps = <T>(
280273
});
281274
});
282275
};
276+

‎lib/msal-node/test/network/HttpClient.spec.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { HttpClient } from '../../src/network/HttpClient';
22
import { NetworkResponse, NetworkRequestOptions } from '../../../msal-common';
3+
import { MockedMetadataResponse } from '../utils/TestConstants';
34

45
import http from "http";
56
jest.mock("http", () => ({
@@ -199,44 +200,48 @@ describe("HttpClient", () => {
199200
});
200201
});
201202

203+
202204
describe("Bad Status Code Error", () => {
203205
const statusCodeError: number = 500;
204-
const error: Error = new Error(`HTTP status code ${statusCodeError}`);
206+
const error: Error = new Error("Error connecting to proxy");
207+
const networkResponse: NetworkResponse<MockedMetadataResponse> = getNetworkResponse<MockedMetadataResponse>(mockGetResponseBody, statusCodeError);
205208

206209
describe("Get Request", () => {
207210
test("Via Https", async () => {
208211
(https.request as jest.Mock).mockImplementationOnce(mockHttpsRequest(mockGetResponseBodyBuffer, statusCodeError));
209-
await expect(httpClient.sendGetRequestAsync(url)).rejects.toEqual(error);
212+
await expect(httpClient.sendGetRequestAsync(url)).resolves.toEqual(networkResponse);
210213
});
211214

212215
describe("Via Proxy", () => {
213216
test("Http Status Code", async () => {
214217
(http.request as jest.Mock).mockImplementationOnce(mockHttpRequest(mockGetResponseBody, statusCodeError, statusCodeError));
215-
await expect(httpClient.sendGetRequestAsync(url, getNetworkRequestOptionsWithProxyUrl)).rejects.toEqual(error);
218+
await expect(httpClient.sendGetRequestAsync(url, getNetworkRequestOptionsWithProxyUrl)).rejects.toEqual(expect.objectContaining(error));
216219
});
217220

218221
test("Socket Status Code", async () => {
219222
(http.request as jest.Mock).mockImplementationOnce(mockHttpRequest(mockGetResponseBody, statusCodeOk, statusCodeError));
220-
await expect(httpClient.sendGetRequestAsync(url, getNetworkRequestOptionsWithProxyUrl)).rejects.toEqual(error);
223+
await expect(httpClient.sendGetRequestAsync(url, getNetworkRequestOptionsWithProxyUrl)).rejects.toEqual(expect.objectContaining(error));
221224
});
222225
});
223226
});
224227

225-
describe("Post Request", () => {
228+
describe("Post Request", <T>() => {
229+
const networkResponse: NetworkResponse<T> = getNetworkResponse(mockPostResponseBody, statusCodeError);
230+
const error: Error = new Error("Error in connection to proxy");
226231
test("Via Https", async () => {
227232
(https.request as jest.Mock).mockImplementationOnce(mockHttpsRequest(mockPostResponseBodyBuffer, statusCodeError));
228-
await expect(httpClient.sendPostRequestAsync(url, postNetworkRequestOptionsWithoutProxyUrl)).rejects.toEqual(error);
233+
await expect(httpClient.sendPostRequestAsync(url, postNetworkRequestOptionsWithoutProxyUrl)).resolves.toEqual(networkResponse);
229234
});
230235

231236
describe("Via Proxy", () => {
232237
test("Http Status Code", async () => {
233238
(http.request as jest.Mock).mockImplementationOnce(mockHttpRequest(mockPostResponseBody, statusCodeError, statusCodeError));
234-
await expect(httpClient.sendPostRequestAsync(url, postNetworkRequestOptionsWithProxyUrl)).rejects.toEqual(error);
239+
await expect(httpClient.sendPostRequestAsync(url, postNetworkRequestOptionsWithProxyUrl)).rejects.toEqual(expect.objectContaining(error));
235240
});
236241

237242
test("Socket Status Code", async () => {
238243
(http.request as jest.Mock).mockImplementationOnce(mockHttpRequest(mockPostResponseBody, statusCodeOk, statusCodeError));
239-
await expect(httpClient.sendPostRequestAsync(url, postNetworkRequestOptionsWithProxyUrl)).rejects.toEqual(error);
244+
await expect(httpClient.sendPostRequestAsync(url, postNetworkRequestOptionsWithProxyUrl)).rejects.toEqual(expect.objectContaining(error));
240245
});
241246
});
242247
});

‎lib/msal-node/test/utils/TestConstants.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export const TEST_CONSTANTS = {
3131
THUMBPRINT: "6182de7d4b84517655fe0bfa97076890d66bf37a",
3232
PRIVATE_KEY: "PRIVATE_KEY",
3333
PUBLIC_CERTIFICATE:
34-
`-----BEGIN CERTIFICATE-----
34+
`-----BEGIN CERTIFICATE-----
3535
line1
3636
line2
3737
-----END CERTIFICATE-----
@@ -43,7 +43,7 @@ line4
4343
`,
4444
CLAIMS: 'claim1 claim2',
4545
SNI_CERTIFICATE:
46-
`-----BEGIN PRIVATE KEY-----\r
46+
`-----BEGIN PRIVATE KEY-----\r
4747
line1\r
4848
line2\r
4949
line3\r
@@ -216,4 +216,11 @@ export const mockAuthenticationResult: AuthenticationResult = {
216216
expiresOn: new Date(),
217217
tokenType: "BEARER",
218218
correlationId: "test-correlationId"
219+
}
220+
221+
export type MockedMetadataResponse = {
222+
tenant_discovery_endpoint: string,
223+
token_endpoint: string,
224+
authorization_endpoint: string,
225+
device_authorization_endpoint: string
219226
}

0 commit comments

Comments
 (0)