From af74365fb74e1476502d9d961eba524bd723937f Mon Sep 17 00:00:00 2001 From: Robert Ginsburg Date: Fri, 14 Jul 2023 16:12:30 -0400 Subject: [PATCH] Implemented GitHub Feedback --- lib/msal-common/src/index.ts | 1 + .../src/response/AuthenticationResult.ts | 2 +- .../src/response/ResponseHandler.ts | 20 ++++++++++--------- lib/msal-common/src/utils/Constants.ts | 15 ++++++++------ .../test/response/ResponseHandler.spec.ts | 10 ---------- .../test/test_kit/StringConstants.ts | 1 - lib/msal-node/src/network/HttpClient.ts | 2 +- lib/msal-node/src/network/LoopbackClient.ts | 2 +- lib/msal-node/src/utils/Constants.ts | 11 ---------- lib/msal-node/test/utils/TestConstants.ts | 1 - 10 files changed, 24 insertions(+), 41 deletions(-) diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index ae9dd84c5c..3a08195e05 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -171,6 +171,7 @@ export { ONE_DAY_IN_MS, GrantType, AADAuthorityConstants, + HttpStatus, } from "./utils/Constants"; export { StringUtils } from "./utils/StringUtils"; export { StringDict } from "./utils/MsalTypes"; diff --git a/lib/msal-common/src/response/AuthenticationResult.ts b/lib/msal-common/src/response/AuthenticationResult.ts index 5693128ef7..fdac504232 100644 --- a/lib/msal-common/src/response/AuthenticationResult.ts +++ b/lib/msal-common/src/response/AuthenticationResult.ts @@ -34,7 +34,7 @@ export type AuthenticationResult = { fromCache: boolean; expiresOn: Date | null; extExpiresOn?: Date; - refreshOn: Date | null; + refreshOn?: Date; tokenType: string; correlationId: string; requestId?: string; diff --git a/lib/msal-common/src/response/ResponseHandler.ts b/lib/msal-common/src/response/ResponseHandler.ts index e6bd86fe2c..c54987e324 100644 --- a/lib/msal-common/src/response/ResponseHandler.ts +++ b/lib/msal-common/src/response/ResponseHandler.ts @@ -28,7 +28,7 @@ import { AuthenticationScheme, Constants, THE_FAMILY_ID, - ErrorCodeBounds, + HttpStatus, } from "../utils/Constants"; import { PopTokenGenerator } from "../crypto/PopTokenGenerator"; import { AppMetadataEntity } from "../cache/entities/AppMetadataEntity"; @@ -169,8 +169,8 @@ export class ResponseHandler { if ( refreshAccessToken && serverResponse.status && - (serverResponse.status >= ErrorCodeBounds.stsErrorLowerLimit) && - (serverResponse.status <= ErrorCodeBounds.stsErrorUpperLimit) + (serverResponse.status >= HttpStatus.SERVER_ERROR_RANGE_START) && + (serverResponse.status <= HttpStatus.SERVER_ERROR_RANGE_END) ) { this.logger.warning( "executeTokenRequest:validateTokenResponse - AAD is currently unavailable and the access token is unable to be refreshed." @@ -182,8 +182,8 @@ export class ResponseHandler { } else if ( refreshAccessToken && serverResponse.status && - (serverResponse.status >= ErrorCodeBounds.clientErrorLowerLimit) && - (serverResponse.status <= ErrorCodeBounds.clientErrorUpperLimit) + (serverResponse.status >= HttpStatus.CLIENT_ERROR_RANGE_START) && + (serverResponse.status <= HttpStatus.CLIENT_ERROR_RANGE_END) ) { this.logger.warning( "executeTokenRequest:validateTokenResponse - AAD is currently available but is unable to refresh the access token." @@ -583,7 +583,7 @@ export class ResponseHandler { let responseScopes: Array = []; let expiresOn: Date | null = null; let extExpiresOn: Date | undefined; - let refreshOn: Date | null = null; + let refreshOn: Date | undefined; let familyId: string = Constants.EMPTY_STRING; if (cacheRecord.accessToken) { @@ -615,9 +615,11 @@ export class ResponseHandler { extExpiresOn = new Date( Number(cacheRecord.accessToken.extendedExpiresOn) * 1000 ); - refreshOn = new Date( - Number(cacheRecord.accessToken.refreshOn) * 1000 - ); + if (cacheRecord.accessToken.refreshOn) { + refreshOn = new Date( + Number(cacheRecord.accessToken.refreshOn) * 1000 + ); + } } if (cacheRecord.appMetadata) { diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index b494ff85fd..568ad208ba 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -64,13 +64,16 @@ export const Constants = { INVALID_INSTANCE: "invalid_instance", }; -export const ErrorCodeBounds = { - clientErrorLowerLimit: 400, - clientErrorUpperLimit: 499, - stsErrorLowerLimit: 500, - stsErrorUpperLimit: 599, +export const HttpStatus = { + SUCCESS_RANGE_START: 200, + SUCCESS_RANGE_END: 299, + REDIRECT: 302, + CLIENT_ERROR_RANGE_START: 400, + CLIENT_ERROR_RANGE_END: 499, + SERVER_ERROR_RANGE_START: 500, + SERVER_ERROR_RANGE_END: 599, } as const; -export type ErrorCodeBounds = typeof ErrorCodeBounds[keyof typeof ErrorCodeBounds]; +export type HttpStatus = typeof HttpStatus[keyof typeof HttpStatus]; export const OIDC_DEFAULT_SCOPES = [ Constants.OPENID_SCOPE, diff --git a/lib/msal-common/test/response/ResponseHandler.spec.ts b/lib/msal-common/test/response/ResponseHandler.spec.ts index b0523cd39c..caed8cd4cc 100644 --- a/lib/msal-common/test/response/ResponseHandler.spec.ts +++ b/lib/msal-common/test/response/ResponseHandler.spec.ts @@ -110,7 +110,6 @@ const testServerTokenResponse = { scope: TEST_CONFIG.DEFAULT_SCOPES.join(" "), expires_in: TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN, ext_expires_in: TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN, - refresh_in: TEST_TOKEN_LIFETIMES.DEFAULT_REFESH_IN, access_token: TEST_TOKENS.ACCESS_TOKEN, refresh_token: TEST_TOKENS.REFRESH_TOKEN, id_token: TEST_TOKENS.IDTOKEN_V2, @@ -266,9 +265,6 @@ describe("ResponseHandler.ts", () => { expiresOn: new Date( Date.now() + testServerTokenResponse.body.expires_in * 1000 ), - refreshOn: new Date( - Date.now() + testServerTokenResponse.body.refresh_in * 1000 - ), // one day in the future account: testAccount, tokenType: AuthenticationScheme.BEARER, }; @@ -335,9 +331,6 @@ describe("ResponseHandler.ts", () => { expiresOn: new Date( Date.now() + testServerTokenResponse.body.expires_in * 1000 ), - refreshOn: new Date( - Date.now() + testServerTokenResponse.body.refresh_in * 1000 - ), // one day in the future account: testAccount, tokenType: AuthenticationScheme.BEARER, }; @@ -403,9 +396,6 @@ describe("ResponseHandler.ts", () => { expiresOn: new Date( Date.now() + testServerTokenResponse.body.expires_in * 1000 ), - refreshOn: new Date( - Date.now() + testServerTokenResponse.body.refresh_in * 1000 - ), // one day in the future account: testAccount, tokenType: AuthenticationScheme.BEARER, }; diff --git a/lib/msal-common/test/test_kit/StringConstants.ts b/lib/msal-common/test/test_kit/StringConstants.ts index 953c7eeadf..49fd79b42e 100644 --- a/lib/msal-common/test/test_kit/StringConstants.ts +++ b/lib/msal-common/test/test_kit/StringConstants.ts @@ -73,7 +73,6 @@ export const ID_TOKEN_CLAIMS = { // Test Expiration Vals export const TEST_TOKEN_LIFETIMES = { DEFAULT_EXPIRES_IN: 3599, - DEFAULT_REFESH_IN: 86400, TEST_ID_TOKEN_EXP: 1536361411, TEST_ACCESS_TOKEN_EXP: 1537234948, }; diff --git a/lib/msal-node/src/network/HttpClient.ts b/lib/msal-node/src/network/HttpClient.ts index 69f94d059b..aed1dd79ae 100644 --- a/lib/msal-node/src/network/HttpClient.ts +++ b/lib/msal-node/src/network/HttpClient.ts @@ -7,11 +7,11 @@ import { INetworkModule, NetworkRequestOptions, NetworkResponse, + HttpStatus, } from "@azure/msal-common"; import { HttpMethod, Constants, - HttpStatus, ProxyStatus, } from "../utils/Constants"; import { NetworkUtils } from "../utils/NetworkUtils"; diff --git a/lib/msal-node/src/network/LoopbackClient.ts b/lib/msal-node/src/network/LoopbackClient.ts index f876e9daaa..302dcf9065 100644 --- a/lib/msal-node/src/network/LoopbackClient.ts +++ b/lib/msal-node/src/network/LoopbackClient.ts @@ -7,12 +7,12 @@ import { Constants as CommonConstants, ServerAuthorizationCodeResponse, UrlString, + HttpStatus, } from "@azure/msal-common"; import { createServer, IncomingMessage, Server, ServerResponse } from "http"; import { NodeAuthError } from "../error/NodeAuthError"; import { Constants, - HttpStatus, LOOPBACK_SERVER_CONSTANTS, } from "../utils/Constants"; import { ILoopbackClient } from "./ILoopbackClient"; diff --git a/lib/msal-node/src/utils/Constants.ts b/lib/msal-node/src/utils/Constants.ts index 40d4b3878f..c588c5cdda 100644 --- a/lib/msal-node/src/utils/Constants.ts +++ b/lib/msal-node/src/utils/Constants.ts @@ -12,17 +12,6 @@ export const HttpMethod = { } as const; export type HttpMethod = typeof HttpMethod[keyof typeof HttpMethod]; -export const HttpStatus = { - SUCCESS_RANGE_START: 200, - SUCCESS_RANGE_END: 299, - REDIRECT: 302, - CLIENT_ERROR_RANGE_START: 400, - CLIENT_ERROR_RANGE_END: 499, - SERVER_ERROR_RANGE_START: 500, - SERVER_ERROR_RANGE_END: 599, -} as const; -export type HttpStatus = typeof HttpStatus[keyof typeof HttpStatus]; - export const ProxyStatus = { SUCCESS_RANGE_START: 200, SUCCESS_RANGE_END: 299, diff --git a/lib/msal-node/test/utils/TestConstants.ts b/lib/msal-node/test/utils/TestConstants.ts index c45e01b5e8..d578836b88 100644 --- a/lib/msal-node/test/utils/TestConstants.ts +++ b/lib/msal-node/test/utils/TestConstants.ts @@ -269,7 +269,6 @@ export const mockAuthenticationResult: AuthenticationResult = { accessToken: TEST_CONSTANTS.ACCESS_TOKEN, fromCache: false, expiresOn: new Date(), - refreshOn: new Date(Date.now() + 86400000), // one day in the future tokenType: "BEARER", correlationId: "test-correlationId", };