From a4c8a0bb2b268d954760e9124fef7fb5c57ba269 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Mon, 22 Jun 2020 11:59:11 -0700 Subject: [PATCH 01/34] Setting up NetworkManager skeleton --- lib/msal-common/src/network/INetworkModule.ts | 2 +- lib/msal-common/src/network/NetworkManager.ts | 85 ++++++++++++++++++- 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/lib/msal-common/src/network/INetworkModule.ts b/lib/msal-common/src/network/INetworkModule.ts index 567c7ad8d20..788498bb650 100644 --- a/lib/msal-common/src/network/INetworkModule.ts +++ b/lib/msal-common/src/network/INetworkModule.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import {NetworkResponse} from "./NetworkManager"; +import { NetworkResponse } from "./NetworkManager"; /** * Options allowed by network request APIs. diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index d560a64b450..88f07aeb4d7 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -3,11 +3,94 @@ * Licensed under the MIT License. */ +import { ICrypto } from '../crypto/ICrypto'; +import { ICacheStorage } from '../cache/interface/ICacheStorage'; +import { INetworkModule, NetworkRequestOptions } from './INetworkModule'; +import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; + export type NetworkResponse = { headers: Record; body: T; status: number; }; +export class NetworkThumbprint { + clientId: string; + authority: string; + scopes: Array; + homeAccountIdentifier: string; + + constructor(clientId: string, authority: string, scopes: Array, homeAccountIdentifier: string) { + this.clientId = clientId; + this.authority = authority; + this.scopes = scopes; + this.homeAccountIdentifier = homeAccountIdentifier; + } + + // base64Encode function + + // base64Decode function + + // generateCacheKey +} + // TODO placeholder: this will be filled in by the throttling PR -export class NetworkManager {} +export class NetworkManager { + cryptoObj: ICrypto; + cacheStorage: ICacheStorage + networkClient: INetworkModule + + constructor(cryptoObj:ICrypto, cacheStorage: ICacheStorage, networkClient: INetworkModule) { + this.cryptoObj = cryptoObj; + this.cacheStorage = cacheStorage; + this.networkClient = networkClient; + } + + private async sendPostRequest(tokenEndpoint: string, options: NetworkRequestOptions, isInteractive: boolean): Promise> { + const thumbprint = new NetworkThumbprint(); + + this.preProcess(thumbprint, isInteractive); + const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); + this.postProcess(thumbprint, response as ServerAuthorizationTokenResponse); + + return response; + } + + private preProcess(thumbprint: NetworkThumbprint, isInteractive: boolean): void { + const key = generateCacheKey(thumbprint); + const cacheValue = this.cacheStorage.getItem(key); + const date = new Date(); + + if (cacheValue) { + if (cacheValue.throttleTime >= date.getTime()) { + // remove throttle here + return; + } + if (isInteractive) { + return; + } + // throw error back to user here + } + } + // create thumbprint + // check against cache + STATE + // if we find nothing, make request + // if we find something: + // check if throttled + // check expiration of throttle + // remove throttle if expired? + + + private postProcess(thumbprint: NetworkThumbprint, response: ServerAuthorizationTokenResponse): void { + + } + // private postProcess() + // create thumbprint + // check the response, put things in cache if necessary + + // cache storage? + +} + + +// InteractionType state? \ No newline at end of file From cfccf770b393f5e0b20544bf9971d510291fc17e Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Mon, 22 Jun 2020 12:34:01 -0700 Subject: [PATCH 02/34] Changes to NetworkManager --- lib/msal-common/src/network/NetworkManager.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index 88f07aeb4d7..0875306738e 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -59,10 +59,9 @@ export class NetworkManager { private preProcess(thumbprint: NetworkThumbprint, isInteractive: boolean): void { const key = generateCacheKey(thumbprint); const cacheValue = this.cacheStorage.getItem(key); - const date = new Date(); if (cacheValue) { - if (cacheValue.throttleTime >= date.getTime()) { + if (cacheValue.throttleTime >= Date.now()) { // remove throttle here return; } From 1ff039033004b48fd2f6820a119a6e54978acbbf Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 23 Jun 2020 14:34:59 -0700 Subject: [PATCH 03/34] Add new header and throttling constants --- lib/msal-common/src/utils/Constants.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index d370a21e6f5..c264158aea5 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -34,7 +34,10 @@ export const Constants = { S256_CODE_CHALLENGE_METHOD: "S256", URL_FORM_CONTENT_TYPE: "application/x-www-form-urlencoded;charset=utf-8", AUTHORIZATION_PENDING: "authorization_pending", - NOT_DEFINED: "not_defined" + NOT_DEFINED: "not_defined", + // Default time to throttle RequestThumbprint in milliseconds + DEFAULT_THROTTLE_TIME_MS: 120000, + DEFAULT_MAX_THROTTLE_TIME_MS: 3600000 }; /** From 6bec96c1bbb1a45cff01227b4614e45c6fcaafc6 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 23 Jun 2020 14:39:15 -0700 Subject: [PATCH 04/34] Add postProcess logic + helpers --- .../cache/entities/RequestThumbprintEntity.ts | 19 ++++++ lib/msal-common/src/client/BaseClient.ts | 4 +- .../src/client/RefreshTokenClient.ts | 18 ++++- lib/msal-common/src/network/NetworkManager.ts | 65 +++++++------------ lib/msal-common/src/utils/Constants.ts | 5 +- 5 files changed, 67 insertions(+), 44 deletions(-) create mode 100644 lib/msal-common/src/cache/entities/RequestThumbprintEntity.ts diff --git a/lib/msal-common/src/cache/entities/RequestThumbprintEntity.ts b/lib/msal-common/src/cache/entities/RequestThumbprintEntity.ts new file mode 100644 index 00000000000..a87dc89762f --- /dev/null +++ b/lib/msal-common/src/cache/entities/RequestThumbprintEntity.ts @@ -0,0 +1,19 @@ +export class RequestThumbprint { + clientId: string; + authority: string; + scopes: Array; + homeAccountIdentifier?: string; + + constructor(clientId: string, authority: string, scopes: Array, homeAccountIdentifier?: string) { + this.clientId = clientId; + this.authority = authority; + this.scopes = scopes; + this.homeAccountIdentifier = homeAccountIdentifier; + } + + // base64Encode function + + // base64Decode function + + // generateCacheKey +} \ No newline at end of file diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 89c779fcc75..81a4b1e6fae 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -5,10 +5,11 @@ import { ClientConfiguration, buildClientConfiguration } from "../config/ClientConfiguration"; import { INetworkModule } from "../network/INetworkModule"; +import { NetworkManager } from "../network/NetworkManager"; import { ICrypto } from "../crypto/ICrypto"; import { Authority } from "../authority/Authority"; import { Logger } from "../logger/Logger"; -import { AADServerParamKeys, Constants, HeaderNames } from "../utils/Constants"; +import { AADServerParamKeys, Constants, HeaderNames, HeaderValues } from "../utils/Constants"; import { NetworkResponse } from "../network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizationTokenResponse"; import { TrustedAuthority } from "../authority/TrustedAuthority"; @@ -70,6 +71,7 @@ export abstract class BaseClient { protected createDefaultTokenRequestHeaders(): Record { const headers = this.createDefaultLibraryHeaders(); headers[HeaderNames.CONTENT_TYPE] = Constants.URL_FORM_CONTENT_TYPE; + headers[HeaderNames.X_MS_LIB_CAPABILITY] = HeaderNames.X_MS_LIB_CAPABILITY_VALUE; if (this.serverTelemetryManager) { headers[HeaderNames.X_CLIENT_CURR_TELEM] = this.serverTelemetryManager.generateCurrentRequestHeaderValue(); diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 1ebc98fc41c..83b6f0574ce 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -13,7 +13,12 @@ import { ScopeSet } from "../request/ScopeSet"; import { GrantType } from "../utils/Constants"; import { ResponseHandler } from "../response/ResponseHandler"; import { AuthenticationResult } from "../response/AuthenticationResult"; +<<<<<<< HEAD import { StringUtils } from "../utils/StringUtils"; +======= +import { NetworkManager } from '../network/NetworkManager'; +import { RequestThumbprint } from '../cache/entities/RequestThumbprintEntity'; +>>>>>>> 74b7d0a4... Add postProcess logic + helpers /** * OAuth2.0 refresh token client @@ -49,7 +54,18 @@ export class RefreshTokenClient extends BaseClient { const requestBody = this.createTokenRequestBody(request); const headers: Record = this.createDefaultTokenRequestHeaders(); - return this.executePostToTokenEndpoint(authority.tokenEndpoint, requestBody, headers); + const thumbprint = new RequestThumbprint(/* TODO: fill out */); + const queryParams = { + body: this.createTokenRequestBody(request), + headers: this.createDefaultTokenRequestHeaders() + }; + + return this.networkManager.sendPostRequest( + thumbprint, + authority.tokenEndpoint, + queryParams, + false + ); } private createTokenRequestBody(request: RefreshTokenRequest): string { diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index 0875306738e..48a09582d84 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -7,6 +7,7 @@ import { ICrypto } from '../crypto/ICrypto'; import { ICacheStorage } from '../cache/interface/ICacheStorage'; import { INetworkModule, NetworkRequestOptions } from './INetworkModule'; import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; +import { Constants, HeaderNames } from "../utils/Constants"; export type NetworkResponse = { headers: Record; @@ -14,27 +15,6 @@ export type NetworkResponse = { status: number; }; -export class NetworkThumbprint { - clientId: string; - authority: string; - scopes: Array; - homeAccountIdentifier: string; - - constructor(clientId: string, authority: string, scopes: Array, homeAccountIdentifier: string) { - this.clientId = clientId; - this.authority = authority; - this.scopes = scopes; - this.homeAccountIdentifier = homeAccountIdentifier; - } - - // base64Encode function - - // base64Decode function - - // generateCacheKey -} - -// TODO placeholder: this will be filled in by the throttling PR export class NetworkManager { cryptoObj: ICrypto; cacheStorage: ICacheStorage @@ -46,12 +26,11 @@ export class NetworkManager { this.networkClient = networkClient; } - private async sendPostRequest(tokenEndpoint: string, options: NetworkRequestOptions, isInteractive: boolean): Promise> { - const thumbprint = new NetworkThumbprint(); + public async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions, isInteractive: boolean): Promise> { this.preProcess(thumbprint, isInteractive); const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); - this.postProcess(thumbprint, response as ServerAuthorizationTokenResponse); + this.postProcess(thumbprint, response); return response; } @@ -62,34 +41,38 @@ export class NetworkManager { if (cacheValue) { if (cacheValue.throttleTime >= Date.now()) { - // remove throttle here + // TODO: remove throttle from cache return; } if (isInteractive) { return; } - // throw error back to user here + // TODO: throw error back to user here } } - // create thumbprint - // check against cache + STATE - // if we find nothing, make request - // if we find something: - // check if throttled - // check expiration of throttle - // remove throttle if expired? - - private postProcess(thumbprint: NetworkThumbprint, response: ServerAuthorizationTokenResponse): void { + + private postProcess(thumbprint: RequestThumbprint, response: NetworkResponse): void { + if (this.checkResponseForThrottle(response)) { + const throttleTime = this.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))); + // TODO: setItem to cache + } } - // private postProcess() - // create thumbprint - // check the response, put things in cache if necessary - // cache storage? + private checkResponseForThrottle(response: NetworkResponse): boolean { + if (response.status == 429 || response.status >= 500 && response.status < 600) { + return true; + } -} + if (response.headers.has(HeaderNames.RETRY_AFTER) && response.status < 200 && response.status >= 300) { + return true; + } + return false; + } -// InteractionType state? \ No newline at end of file + private calculateThrottleTime(throttleTime: number): number { + return Math.min(throttleTime || Date.now() + Constants.DEFAULT_THROTTLE_TIME_MS, Constants.DEFAULT_MAX_THROTTLE_TIME_MS); + } +} diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index c264158aea5..cd2adf519f7 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -46,7 +46,10 @@ export const Constants = { export enum HeaderNames { CONTENT_TYPE = "Content-Type", X_CLIENT_CURR_TELEM = "x-client-current-telemetry", - X_CLIENT_LAST_TELEM = "x-client-last-telemetry" + X_CLIENT_LAST_TELEM = "x-client-last-telemetry", + RETRY_AFTER = "Retry-After", + X_MS_LIB_CAPABILITY = "x-ms-lib-capability", + X_MS_LIB_CAPABILITY_VALUE = "retry-after, h429" } /** From cc2c61c939e085a8037b681b85de53c3ef5d0a2b Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 25 Jun 2020 14:41:26 -0700 Subject: [PATCH 05/34] Rebase and resolve merge conflicts --- lib/msal-common/src/client/BaseClient.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 81a4b1e6fae..fa747b68fe9 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -38,6 +38,9 @@ export abstract class BaseClient { // Server Telemetry Manager protected serverTelemetryManager: ServerTelemetryManager; + // Network Manager + protected networkManager: NetworkManager; + // Default authority object protected authority: Authority; @@ -61,6 +64,14 @@ export abstract class BaseClient { this.serverTelemetryManager = this.config.serverTelemetryManager; TrustedAuthority.setTrustedAuthoritiesFromConfig(this.config.authOptions.knownAuthorities, this.config.authOptions.cloudDiscoveryMetadata); + // Set the NetworkManager + this.networkManager = new NetworkManager( + this.config.cryptoInterface, + this.config.storageInterface, + this.config.networkInterface + ); + + B2cAuthority.setKnownAuthorities(this.config.authOptions.knownAuthorities); this.authority = this.config.authOptions.authority; } From 75723c50a763bc0b7e3ba881edf8ae2ec8b771e5 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 7 Jul 2020 10:47:08 -0700 Subject: [PATCH 06/34] Resolve rebase conflicts --- .../src/client/AuthorizationCodeClient.ts | 9 +- lib/msal-common/src/client/BaseClient.ts | 4 +- .../src/client/DeviceCodeClient.ts | 23 ++++- .../src/client/RefreshTokenClient.ts | 27 +++--- lib/msal-common/src/index.ts | 6 +- lib/msal-common/src/network/INetworkModule.ts | 2 +- .../RequestThumbprint.ts} | 15 ++- .../src/network/RequestThumbprintValue.ts | 28 ++++++ .../src/network/ThrottlingManager.ts | 95 +++++++++++++++++++ lib/msal-common/src/utils/Constants.ts | 2 + 10 files changed, 181 insertions(+), 30 deletions(-) rename lib/msal-common/src/{cache/entities/RequestThumbprintEntity.ts => network/RequestThumbprint.ts} (60%) create mode 100644 lib/msal-common/src/network/RequestThumbprintValue.ts create mode 100644 lib/msal-common/src/network/ThrottlingManager.ts diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 1e05e2490de..9456b2b128f 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -22,6 +22,7 @@ import { ServerAuthorizationCodeResponse } from "../response/ServerAuthorization import { AccountEntity } from "../cache/entities/AccountEntity"; import { EndSessionRequest } from "../request/EndSessionRequest"; import { ClientConfigurationError } from "../error/ClientConfigurationError"; +import { RequestThumbprint } from '../network/RequestThumbprint'; /** * Oauth2.0 Authorization Code client @@ -130,10 +131,16 @@ export class AuthorizationCodeClient extends BaseClient { * @param request */ private async executeTokenRequest(authority: Authority, request: AuthorizationCodeRequest): Promise> { + const thumbprint = new RequestThumbprint( + this.config.authOptions.clientId, + request.authority, + request.scopes + ); + const requestBody = this.createTokenRequestBody(request); const headers: Record = this.createDefaultTokenRequestHeaders(); - return this.executePostToTokenEndpoint(authority.tokenEndpoint, requestBody, headers); + return this.executePostToTokenEndpoint(authority.tokenEndpoint, requestBody, headers, thumbprint); } /** diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index fa747b68fe9..da91374d849 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -5,7 +5,7 @@ import { ClientConfiguration, buildClientConfiguration } from "../config/ClientConfiguration"; import { INetworkModule } from "../network/INetworkModule"; -import { NetworkManager } from "../network/NetworkManager"; +import { ThrottlingManager, NetworkResponse } from "../network/ThrottlingManager"; import { ICrypto } from "../crypto/ICrypto"; import { Authority } from "../authority/Authority"; import { Logger } from "../logger/Logger"; @@ -15,6 +15,7 @@ import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizatio import { TrustedAuthority } from "../authority/TrustedAuthority"; import { CacheManager } from "../cache/CacheManager"; import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManager"; +import { RequestThumbprint } from "../network/RequestThumbprint"; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. @@ -112,6 +113,7 @@ export abstract class BaseClient { * @param tokenEndpoint * @param queryString * @param headers + * @param thumbprint */ protected async executePostToTokenEndpoint(tokenEndpoint: string, queryString: string, headers: Record): Promise> { const response = await this.networkClient.sendPostRequestAsync< diff --git a/lib/msal-common/src/client/DeviceCodeClient.ts b/lib/msal-common/src/client/DeviceCodeClient.ts index e209d7480fe..55e994e9805 100644 --- a/lib/msal-common/src/client/DeviceCodeClient.ts +++ b/lib/msal-common/src/client/DeviceCodeClient.ts @@ -16,6 +16,7 @@ import { ScopeSet } from "../request/ScopeSet"; import { ResponseHandler } from "../response/ResponseHandler"; import { AuthenticationResult } from "../response/AuthenticationResult"; import { StringUtils } from "../utils/StringUtils"; +import { RequestThumbprint } from '../network/RequestThumbprint'; /** * OAuth2.0 Device code client @@ -62,10 +63,16 @@ export class DeviceCodeClient extends BaseClient { */ private async getDeviceCode(request: DeviceCodeRequest): Promise { + const thumbprint = new RequestThumbprint( + this.config.authOptions.clientId, + request.authority, + request.scopes + ); + const queryString = this.createQueryString(request); const headers = this.createDefaultLibraryHeaders(); - return this.executePostRequestToDeviceCodeEndpoint(this.authority.deviceCodeEndpoint, queryString, headers); + return this.executePostRequestToDeviceCodeEndpoint(this.authority.deviceCodeEndpoint, queryString, headers, thumbprint); } /** @@ -77,7 +84,8 @@ export class DeviceCodeClient extends BaseClient { private async executePostRequestToDeviceCodeEndpoint( deviceCodeEndpoint: string, queryString: string, - headers: Record): Promise { + headers: Record, + thumbprint: RequestThumbprint): Promise { const { body: { @@ -88,7 +96,8 @@ export class DeviceCodeClient extends BaseClient { interval, message } - } = await this.networkClient.sendPostRequestAsync( + } = await this.throttlingManager.sendPostRequest( + thumbprint, deviceCodeEndpoint, { body: queryString, @@ -157,10 +166,16 @@ export class DeviceCodeClient extends BaseClient { reject(ClientAuthError.createDeviceCodeExpiredError()); } else { + const thumbprint = new RequestThumbprint( + this.config.authOptions.clientId, + request.authority, + request.scopes + ); const response = await this.executePostToTokenEndpoint( this.authority.tokenEndpoint, requestBody, - headers); + headers, + thumbprint); if (response.body && response.body.error == Constants.AUTHORIZATION_PENDING) { // user authorization is pending. Sleep for polling interval and try again diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 83b6f0574ce..9a02c3a50fc 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -13,12 +13,12 @@ import { ScopeSet } from "../request/ScopeSet"; import { GrantType } from "../utils/Constants"; import { ResponseHandler } from "../response/ResponseHandler"; import { AuthenticationResult } from "../response/AuthenticationResult"; -<<<<<<< HEAD import { StringUtils } from "../utils/StringUtils"; -======= import { NetworkManager } from '../network/NetworkManager'; -import { RequestThumbprint } from '../cache/entities/RequestThumbprintEntity'; ->>>>>>> 74b7d0a4... Add postProcess logic + helpers +import { NetworkResponse } from "../network/ThrottlingManager"; +import { RequestThumbprint } from "../network/RequestThumbprint" +import { RequestThumbprintValue } from "../network/RequestThumbprintValue"; + /** * OAuth2.0 refresh token client @@ -50,22 +50,17 @@ export class RefreshTokenClient extends BaseClient { private async executeTokenRequest(request: RefreshTokenRequest, authority: Authority) : Promise> { + + const thumbprint = new RequestThumbprint( + this.config.authOptions.clientId, + request.authority, + request.scopes + ); const requestBody = this.createTokenRequestBody(request); const headers: Record = this.createDefaultTokenRequestHeaders(); - const thumbprint = new RequestThumbprint(/* TODO: fill out */); - const queryParams = { - body: this.createTokenRequestBody(request), - headers: this.createDefaultTokenRequestHeaders() - }; - - return this.networkManager.sendPostRequest( - thumbprint, - authority.tokenEndpoint, - queryParams, - false - ); + return this.networkManager.sendPostRequest(authority.tokenEndpoint, requestBody, headers, thumbprint); } private createTokenRequestBody(request: RefreshTokenRequest): string { diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index e5d1426ade7..30dabd1eccb 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -26,8 +26,10 @@ export { IdTokenEntity } from "./cache/entities/IdTokenEntity"; export { AccessTokenEntity } from "./cache/entities/AccessTokenEntity"; export { RefreshTokenEntity } from "./cache/entities/RefreshTokenEntity"; // Network Interface -export { INetworkModule, NetworkRequestOptions } from "./network/INetworkModule"; -export { NetworkResponse } from "./network/NetworkManager"; +export { INetworkModule } from "./network/INetworkModule"; +export { ThrottlingManager } from "./network/ThrottlingManager"; +export { RequestThumbprint } from "./network/RequestThumbprint"; +export { RequestThumbprintValue } from "./network/RequestThumbprintValue"; export { IUri } from "./url/IUri"; export { UrlString } from "./url/UrlString"; // Crypto Interface diff --git a/lib/msal-common/src/network/INetworkModule.ts b/lib/msal-common/src/network/INetworkModule.ts index 788498bb650..695211bdb47 100644 --- a/lib/msal-common/src/network/INetworkModule.ts +++ b/lib/msal-common/src/network/INetworkModule.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { NetworkResponse } from "./NetworkManager"; +import { NetworkResponse } from "./ThrottlingManager"; /** * Options allowed by network request APIs. diff --git a/lib/msal-common/src/cache/entities/RequestThumbprintEntity.ts b/lib/msal-common/src/network/RequestThumbprint.ts similarity index 60% rename from lib/msal-common/src/cache/entities/RequestThumbprintEntity.ts rename to lib/msal-common/src/network/RequestThumbprint.ts index a87dc89762f..83693c0ac8f 100644 --- a/lib/msal-common/src/cache/entities/RequestThumbprintEntity.ts +++ b/lib/msal-common/src/network/RequestThumbprint.ts @@ -1,3 +1,10 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { Constants } from "../utils/Constants"; + export class RequestThumbprint { clientId: string; authority: string; @@ -11,9 +18,7 @@ export class RequestThumbprint { this.homeAccountIdentifier = homeAccountIdentifier; } - // base64Encode function - - // base64Decode function - - // generateCacheKey + public generateStorageKey(): string { + return `${Constants.THROTTLE_PREFIX}.${JSON.stringify(this)}` + } } \ No newline at end of file diff --git a/lib/msal-common/src/network/RequestThumbprintValue.ts b/lib/msal-common/src/network/RequestThumbprintValue.ts new file mode 100644 index 00000000000..605299914d7 --- /dev/null +++ b/lib/msal-common/src/network/RequestThumbprintValue.ts @@ -0,0 +1,28 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { Constants } from "../utils/Constants"; + +export class RequestThumbprintValue { + // Unix-time value representing the expiration of the throttle + throttleTime: number; + // Information provided by the server + error?: string; + errorCodes?: Array; + errorMessage?: string; + subError?: string; + + constructor(throttleTime: number, error?: string, errorCodes?: Array, errorMessage?: string, subError?: string) { + this.throttleTime = throttleTime; + this.error = error; + this.errorCodes = errorCodes; + this.errorMessage = errorMessage; + this.subError = subError; + } + + public generateStorageValue(): string { + return `${Constants.THROTTLE_PREFIX}.${JSON.stringify(this)}` + } +} \ No newline at end of file diff --git a/lib/msal-common/src/network/ThrottlingManager.ts b/lib/msal-common/src/network/ThrottlingManager.ts new file mode 100644 index 00000000000..2b3768ed665 --- /dev/null +++ b/lib/msal-common/src/network/ThrottlingManager.ts @@ -0,0 +1,95 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { INetworkModule, NetworkRequestOptions } from "./INetworkModule"; +import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; +import { Constants, HeaderNames } from "../utils/Constants"; +import { RequestThumbprint } from "./RequestThumbprint"; +import { RequestThumbprintValue } from "./RequestThumbprintValue"; +import { AuthError } from "../error/AuthError"; + +export type NetworkResponse = { + headers: Map; + body: T; + status: number; +}; + +export abstract class ThrottlingManager { + protected networkClient: INetworkModule; + abstract getThrottlingItem(thumbprint: RequestThumbprint): RequestThumbprintValue | null; + abstract setThrottlingItem(thumbprint: RequestThumbprint, thumbprintValue: RequestThumbprintValue): void; + abstract removeThrottlingItem(thumbprint: RequestThumbprint): boolean; + + constructor(networkClient: INetworkModule) { + this.networkClient = networkClient; + } + + public async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise> { + + this.preProcess(thumbprint); + const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); + this.postProcess(thumbprint, response); + + return response; + } + + public preProcess(thumbprint: RequestThumbprint): void { + const storageValue = this.getThrottlingItem(thumbprint); + + if (storageValue) { + if (storageValue.throttleTime >= Date.now()) { + this.removeThrottlingItem(thumbprint); + return; + } + // TODO: implement this error + // ThrottleError extends ServerError and adds a message about how long the request is throttled for + // throw new ThrottleError(storageValue.throttleTime, storageValue.error, storageValue.errorDescription, storageValue.subError); + } + } + + public postProcess(thumbprint: RequestThumbprint, response: NetworkResponse): void { + if (this.checkResponseForThrottle(response)) { + const thumbprintValue = new RequestThumbprintValue( + this.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))), + response.body.error, + response.body.error_codes, + response.body.error_description, + response.body.suberror + ); + this.setThrottlingItem(thumbprint, thumbprintValue); + } + } + + private checkResponseForThrottle(response: NetworkResponse): boolean { + if (response.status == 429 || response.status >= 500 && response.status < 600) { + return true; + } + + if (response.headers.has(HeaderNames.RETRY_AFTER) && response.status < 200 && response.status >= 300) { + return true; + } + + return false; + } + + private calculateThrottleTime(throttleTime: number): number { + return Math.min(throttleTime || Date.now() + Constants.DEFAULT_THROTTLE_TIME_MS, Constants.DEFAULT_MAX_THROTTLE_TIME_MS); + } +} + +export class DefaultThrottlingManager extends ThrottlingManager { + getThrottlingItem(): RequestThumbprintValue | null { + const notImplErr = "Throttling abstract class - getThrottlingItem() has not been implemented"; + throw AuthError.createUnexpectedError(notImplErr); + } + setThrottlingItem(): void { + const notImplErr = "Throttling abstract class - setThrottlingItem() has not been implemented"; + throw AuthError.createUnexpectedError(notImplErr); + } + removeThrottlingItem(): boolean { + const notImplErr = "Throttling abstract class - removeThrottlingItem() has not been implemented"; + throw AuthError.createUnexpectedError(notImplErr); + } +} diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index cd2adf519f7..bf0cbf66caf 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -7,6 +7,8 @@ export const Constants = { SKU: "msal.js.common", // Prefix for all library cache entries CACHE_PREFIX: "msal", + // Prefix for storing throttling entries + THROTTLE_PREFIX: "throttle", // default authority DEFAULT_AUTHORITY: "https://login.microsoftonline.com/common/", DEFAULT_AUTHORITY_HOST: "login.microsoftonline.com", From 31e5c26a7a9d7c415b51d21f606686437a4c523b Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 7 Jul 2020 15:14:17 -0700 Subject: [PATCH 07/34] Rework abstract class into utils file --- .../src/client/AuthorizationCodeClient.ts | 12 +-- lib/msal-common/src/client/BaseClient.ts | 17 ++-- .../src/client/DeviceCodeClient.ts | 22 ++--- .../src/client/RefreshTokenClient.ts | 14 +-- lib/msal-common/src/index.ts | 5 +- lib/msal-common/src/network/INetworkModule.ts | 2 +- lib/msal-common/src/network/NetworkManager.ts | 69 +++----------- .../src/network/RequestThumbprint.ts | 24 ----- .../src/network/RequestThumbprintValue.ts | 28 ------ .../src/network/ThrottlingManager.ts | 95 ------------------- .../src/network/ThrottlingUtils.ts | 83 ++++++++++++++++ lib/msal-common/src/utils/StringUtils.ts | 6 ++ 12 files changed, 138 insertions(+), 239 deletions(-) delete mode 100644 lib/msal-common/src/network/RequestThumbprint.ts delete mode 100644 lib/msal-common/src/network/RequestThumbprintValue.ts delete mode 100644 lib/msal-common/src/network/ThrottlingManager.ts create mode 100644 lib/msal-common/src/network/ThrottlingUtils.ts diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 9456b2b128f..6d614640344 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -22,7 +22,7 @@ import { ServerAuthorizationCodeResponse } from "../response/ServerAuthorization import { AccountEntity } from "../cache/entities/AccountEntity"; import { EndSessionRequest } from "../request/EndSessionRequest"; import { ClientConfigurationError } from "../error/ClientConfigurationError"; -import { RequestThumbprint } from '../network/RequestThumbprint'; +import { RequestThumbprint } from '../network/ThrottlingUtils'; /** * Oauth2.0 Authorization Code client @@ -131,11 +131,11 @@ export class AuthorizationCodeClient extends BaseClient { * @param request */ private async executeTokenRequest(authority: Authority, request: AuthorizationCodeRequest): Promise> { - const thumbprint = new RequestThumbprint( - this.config.authOptions.clientId, - request.authority, - request.scopes - ); + const thumbprint: RequestThumbprint = { + clientId: this.config.authOptions.clientId, + authority: authority.canonicalAuthority, + scopes: request.scopes + }; const requestBody = this.createTokenRequestBody(request); const headers: Record = this.createDefaultTokenRequestHeaders(); diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index da91374d849..a03211bec93 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -5,7 +5,7 @@ import { ClientConfiguration, buildClientConfiguration } from "../config/ClientConfiguration"; import { INetworkModule } from "../network/INetworkModule"; -import { ThrottlingManager, NetworkResponse } from "../network/ThrottlingManager"; +import { NetworkManager, NetworkResponse } from "../network/NetworkManager"; import { ICrypto } from "../crypto/ICrypto"; import { Authority } from "../authority/Authority"; import { Logger } from "../logger/Logger"; @@ -16,6 +16,7 @@ import { TrustedAuthority } from "../authority/TrustedAuthority"; import { CacheManager } from "../cache/CacheManager"; import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManager"; import { RequestThumbprint } from "../network/RequestThumbprint"; +import { RequestThumbprint } from '../network/ThrottlingUtils'; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. @@ -63,6 +64,7 @@ export abstract class BaseClient { // Set TelemetryManager this.serverTelemetryManager = this.config.serverTelemetryManager; + this.networkManager = new NetworkManager(this.networkClient, this.cacheManager); TrustedAuthority.setTrustedAuthoritiesFromConfig(this.config.authOptions.knownAuthorities, this.config.authOptions.cloudDiscoveryMetadata); // Set the NetworkManager @@ -115,13 +117,12 @@ export abstract class BaseClient { * @param headers * @param thumbprint */ - protected async executePostToTokenEndpoint(tokenEndpoint: string, queryString: string, headers: Record): Promise> { - const response = await this.networkClient.sendPostRequestAsync< - ServerAuthorizationTokenResponse - >(tokenEndpoint, { - body: queryString, - headers: headers, - }); + protected async executePostToTokenEndpoint(tokenEndpoint: string, queryString: string, headers: Record, thumbprint: RequestThumbprint): Promise> { + const response = await this.networkManager.sendPostRequest( + thumbprint, + tokenEndpoint, + { body: queryString, headers: headers } + ); if (this.config.serverTelemetryManager && response.status < 500 && response.status !== 429) { // Telemetry data successfully logged by server, clear Telemetry cache diff --git a/lib/msal-common/src/client/DeviceCodeClient.ts b/lib/msal-common/src/client/DeviceCodeClient.ts index 55e994e9805..da7e21eaa50 100644 --- a/lib/msal-common/src/client/DeviceCodeClient.ts +++ b/lib/msal-common/src/client/DeviceCodeClient.ts @@ -63,11 +63,11 @@ export class DeviceCodeClient extends BaseClient { */ private async getDeviceCode(request: DeviceCodeRequest): Promise { - const thumbprint = new RequestThumbprint( - this.config.authOptions.clientId, - request.authority, - request.scopes - ); + const thumbprint: RequestThumbprint = { + clientId: this.config.authOptions.clientId, + authority: request.authority, + scopes: request.scopes + }; const queryString = this.createQueryString(request); const headers = this.createDefaultLibraryHeaders(); @@ -96,7 +96,7 @@ export class DeviceCodeClient extends BaseClient { interval, message } - } = await this.throttlingManager.sendPostRequest( + } = await this.networkManager.sendPostRequest( thumbprint, deviceCodeEndpoint, { @@ -166,11 +166,11 @@ export class DeviceCodeClient extends BaseClient { reject(ClientAuthError.createDeviceCodeExpiredError()); } else { - const thumbprint = new RequestThumbprint( - this.config.authOptions.clientId, - request.authority, - request.scopes - ); + const thumbprint: RequestThumbprint = { + clientId: this.config.authOptions.clientId, + authority: request.authority, + scopes: request.scopes + }; const response = await this.executePostToTokenEndpoint( this.authority.tokenEndpoint, requestBody, diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 9a02c3a50fc..99082fe969e 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -18,6 +18,8 @@ import { NetworkManager } from '../network/NetworkManager'; import { NetworkResponse } from "../network/ThrottlingManager"; import { RequestThumbprint } from "../network/RequestThumbprint" import { RequestThumbprintValue } from "../network/RequestThumbprintValue"; +import { NetworkResponse } from "../network/NetworkManager"; +import { RequestThumbprint} from "../network/ThrottlingUtils"; /** @@ -50,12 +52,12 @@ export class RefreshTokenClient extends BaseClient { private async executeTokenRequest(request: RefreshTokenRequest, authority: Authority) : Promise> { - - const thumbprint = new RequestThumbprint( - this.config.authOptions.clientId, - request.authority, - request.scopes - ); + + const thumbprint: RequestThumbprint = { + clientId: this.config.authOptions.clientId, + authority: authority.canonicalAuthority, + scopes: request.scopes + }; const requestBody = this.createTokenRequestBody(request); const headers: Record = this.createDefaultTokenRequestHeaders(); diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index 30dabd1eccb..854b7e792e7 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -27,9 +27,8 @@ export { AccessTokenEntity } from "./cache/entities/AccessTokenEntity"; export { RefreshTokenEntity } from "./cache/entities/RefreshTokenEntity"; // Network Interface export { INetworkModule } from "./network/INetworkModule"; -export { ThrottlingManager } from "./network/ThrottlingManager"; -export { RequestThumbprint } from "./network/RequestThumbprint"; -export { RequestThumbprintValue } from "./network/RequestThumbprintValue"; +export { NetworkManager } from "./network/NetworkManager"; +export { ThrottlingUtils, RequestThumbprint, RequestThumbprintValue } from "./network/ThrottlingUtils"; export { IUri } from "./url/IUri"; export { UrlString } from "./url/UrlString"; // Crypto Interface diff --git a/lib/msal-common/src/network/INetworkModule.ts b/lib/msal-common/src/network/INetworkModule.ts index 695211bdb47..788498bb650 100644 --- a/lib/msal-common/src/network/INetworkModule.ts +++ b/lib/msal-common/src/network/INetworkModule.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { NetworkResponse } from "./ThrottlingManager"; +import { NetworkResponse } from "./NetworkManager"; /** * Options allowed by network request APIs. diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index 48a09582d84..9a389a018dd 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -3,11 +3,10 @@ * Licensed under the MIT License. */ -import { ICrypto } from '../crypto/ICrypto'; -import { ICacheStorage } from '../cache/interface/ICacheStorage'; -import { INetworkModule, NetworkRequestOptions } from './INetworkModule'; -import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; -import { Constants, HeaderNames } from "../utils/Constants"; +import { INetworkModule, NetworkRequestOptions } from "./INetworkModule"; +import { RequestThumbprint } from "./ThrottlingUtils"; +import { ThrottlingUtils } from "./ThrottlingUtils"; +import { CacheManager } from '../cache/CacheManager'; export type NetworkResponse = { headers: Record; @@ -16,63 +15,19 @@ export type NetworkResponse = { }; export class NetworkManager { - cryptoObj: ICrypto; - cacheStorage: ICacheStorage - networkClient: INetworkModule + private networkClient: INetworkModule; + private cacheManager: CacheManager; - constructor(cryptoObj:ICrypto, cacheStorage: ICacheStorage, networkClient: INetworkModule) { - this.cryptoObj = cryptoObj; - this.cacheStorage = cacheStorage; + constructor(networkClient: INetworkModule, cacheManager: CacheManager) { this.networkClient = networkClient; + this.cacheManager = cacheManager; } - public async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions, isInteractive: boolean): Promise> { - - this.preProcess(thumbprint, isInteractive); - const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); - this.postProcess(thumbprint, response); + public async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise> { + ThrottlingUtils.preProcess(this.cacheManager, thumbprint); + const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); + ThrottlingUtils.postProcess(this.cacheManager, thumbprint, response); return response; } - - private preProcess(thumbprint: NetworkThumbprint, isInteractive: boolean): void { - const key = generateCacheKey(thumbprint); - const cacheValue = this.cacheStorage.getItem(key); - - if (cacheValue) { - if (cacheValue.throttleTime >= Date.now()) { - // TODO: remove throttle from cache - return; - } - if (isInteractive) { - return; - } - // TODO: throw error back to user here - } - } - - - - private postProcess(thumbprint: RequestThumbprint, response: NetworkResponse): void { - if (this.checkResponseForThrottle(response)) { - const throttleTime = this.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))); - // TODO: setItem to cache - } - } - - private checkResponseForThrottle(response: NetworkResponse): boolean { - if (response.status == 429 || response.status >= 500 && response.status < 600) { - return true; - } - - if (response.headers.has(HeaderNames.RETRY_AFTER) && response.status < 200 && response.status >= 300) { - return true; - } - - return false; - } - - private calculateThrottleTime(throttleTime: number): number { - return Math.min(throttleTime || Date.now() + Constants.DEFAULT_THROTTLE_TIME_MS, Constants.DEFAULT_MAX_THROTTLE_TIME_MS); - } } diff --git a/lib/msal-common/src/network/RequestThumbprint.ts b/lib/msal-common/src/network/RequestThumbprint.ts deleted file mode 100644 index 83693c0ac8f..00000000000 --- a/lib/msal-common/src/network/RequestThumbprint.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -import { Constants } from "../utils/Constants"; - -export class RequestThumbprint { - clientId: string; - authority: string; - scopes: Array; - homeAccountIdentifier?: string; - - constructor(clientId: string, authority: string, scopes: Array, homeAccountIdentifier?: string) { - this.clientId = clientId; - this.authority = authority; - this.scopes = scopes; - this.homeAccountIdentifier = homeAccountIdentifier; - } - - public generateStorageKey(): string { - return `${Constants.THROTTLE_PREFIX}.${JSON.stringify(this)}` - } -} \ No newline at end of file diff --git a/lib/msal-common/src/network/RequestThumbprintValue.ts b/lib/msal-common/src/network/RequestThumbprintValue.ts deleted file mode 100644 index 605299914d7..00000000000 --- a/lib/msal-common/src/network/RequestThumbprintValue.ts +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -import { Constants } from "../utils/Constants"; - -export class RequestThumbprintValue { - // Unix-time value representing the expiration of the throttle - throttleTime: number; - // Information provided by the server - error?: string; - errorCodes?: Array; - errorMessage?: string; - subError?: string; - - constructor(throttleTime: number, error?: string, errorCodes?: Array, errorMessage?: string, subError?: string) { - this.throttleTime = throttleTime; - this.error = error; - this.errorCodes = errorCodes; - this.errorMessage = errorMessage; - this.subError = subError; - } - - public generateStorageValue(): string { - return `${Constants.THROTTLE_PREFIX}.${JSON.stringify(this)}` - } -} \ No newline at end of file diff --git a/lib/msal-common/src/network/ThrottlingManager.ts b/lib/msal-common/src/network/ThrottlingManager.ts deleted file mode 100644 index 2b3768ed665..00000000000 --- a/lib/msal-common/src/network/ThrottlingManager.ts +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -import { INetworkModule, NetworkRequestOptions } from "./INetworkModule"; -import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; -import { Constants, HeaderNames } from "../utils/Constants"; -import { RequestThumbprint } from "./RequestThumbprint"; -import { RequestThumbprintValue } from "./RequestThumbprintValue"; -import { AuthError } from "../error/AuthError"; - -export type NetworkResponse = { - headers: Map; - body: T; - status: number; -}; - -export abstract class ThrottlingManager { - protected networkClient: INetworkModule; - abstract getThrottlingItem(thumbprint: RequestThumbprint): RequestThumbprintValue | null; - abstract setThrottlingItem(thumbprint: RequestThumbprint, thumbprintValue: RequestThumbprintValue): void; - abstract removeThrottlingItem(thumbprint: RequestThumbprint): boolean; - - constructor(networkClient: INetworkModule) { - this.networkClient = networkClient; - } - - public async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise> { - - this.preProcess(thumbprint); - const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); - this.postProcess(thumbprint, response); - - return response; - } - - public preProcess(thumbprint: RequestThumbprint): void { - const storageValue = this.getThrottlingItem(thumbprint); - - if (storageValue) { - if (storageValue.throttleTime >= Date.now()) { - this.removeThrottlingItem(thumbprint); - return; - } - // TODO: implement this error - // ThrottleError extends ServerError and adds a message about how long the request is throttled for - // throw new ThrottleError(storageValue.throttleTime, storageValue.error, storageValue.errorDescription, storageValue.subError); - } - } - - public postProcess(thumbprint: RequestThumbprint, response: NetworkResponse): void { - if (this.checkResponseForThrottle(response)) { - const thumbprintValue = new RequestThumbprintValue( - this.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))), - response.body.error, - response.body.error_codes, - response.body.error_description, - response.body.suberror - ); - this.setThrottlingItem(thumbprint, thumbprintValue); - } - } - - private checkResponseForThrottle(response: NetworkResponse): boolean { - if (response.status == 429 || response.status >= 500 && response.status < 600) { - return true; - } - - if (response.headers.has(HeaderNames.RETRY_AFTER) && response.status < 200 && response.status >= 300) { - return true; - } - - return false; - } - - private calculateThrottleTime(throttleTime: number): number { - return Math.min(throttleTime || Date.now() + Constants.DEFAULT_THROTTLE_TIME_MS, Constants.DEFAULT_MAX_THROTTLE_TIME_MS); - } -} - -export class DefaultThrottlingManager extends ThrottlingManager { - getThrottlingItem(): RequestThumbprintValue | null { - const notImplErr = "Throttling abstract class - getThrottlingItem() has not been implemented"; - throw AuthError.createUnexpectedError(notImplErr); - } - setThrottlingItem(): void { - const notImplErr = "Throttling abstract class - setThrottlingItem() has not been implemented"; - throw AuthError.createUnexpectedError(notImplErr); - } - removeThrottlingItem(): boolean { - const notImplErr = "Throttling abstract class - removeThrottlingItem() has not been implemented"; - throw AuthError.createUnexpectedError(notImplErr); - } -} diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts new file mode 100644 index 00000000000..4249400844d --- /dev/null +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -0,0 +1,83 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { NetworkResponse } from "./NetworkManager"; +import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; +import { Constants, HeaderNames } from "../utils/Constants"; +import { CacheManager } from "../cache/CacheManager"; +import { StringUtils } from "../utils/StringUtils"; + +export type RequestThumbprint = { + clientId: string; + authority: string; + scopes: Array; + homeAccountIdentifier?: string; +} + +export type RequestThumbprintValue = { + // Unix-time value representing the expiration of the throttle + throttleTime: number; + // Information provided by the server + error?: string; + errorCodes?: Array; + errorMessage?: string; + subError?: string; +} + +export class ThrottlingUtils { + + static generateThrottlingStorageKey(thumbprint: RequestThumbprint | RequestThumbprintValue): string { + return `${Constants.THROTTLE_PREFIX}.${JSON.stringify(thumbprint)}`; + } + + static preProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint): void { + const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); + const storageValue = cacheManager.getItem(key) as string; + + if (storageValue) { + const parsedValue = StringUtils.jsonParseHelper(storageValue); + + if (parsedValue.throttleTime >= Date.now()) { + cacheManager.removeItem(key); + return; + } + // TODO: implement this error + // ThrottleError extends ServerError and adds a message about how long the request is throttled for + // throw new ThrottleError(storageValue.throttleTime, storageValue.error, storageValue.errorDescription, storageValue.subError); + } + } + + static postProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint, response: NetworkResponse): void { + if (ThrottlingUtils.checkResponseForThrottle(response)) { + const thumbprintValue: RequestThumbprintValue = { + throttleTime: ThrottlingUtils.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))), + error: response.body.error, + errorCodes: response.body.error_codes, + errorMessage: response.body.error_description, + subError: response.body.suberror + }; + cacheManager.setItem( + ThrottlingUtils.generateThrottlingStorageKey(thumbprint), + ThrottlingUtils.generateThrottlingStorageKey(thumbprintValue) + ); + } + } + + private static checkResponseForThrottle(response: NetworkResponse): boolean { + if (response.status == 429 || response.status >= 500 && response.status < 600) { + return true; + } + + if (response.headers.has(HeaderNames.RETRY_AFTER) && response.status < 200 && response.status >= 300) { + return true; + } + + return false; + } + + private static calculateThrottleTime(throttleTime: number): number { + return Math.min(throttleTime || Date.now() + Constants.DEFAULT_THROTTLE_TIME_MS, Constants.DEFAULT_MAX_THROTTLE_TIME_MS); + } +} \ No newline at end of file diff --git a/lib/msal-common/src/utils/StringUtils.ts b/lib/msal-common/src/utils/StringUtils.ts index b424b11534f..b9a18d5e26a 100644 --- a/lib/msal-common/src/utils/StringUtils.ts +++ b/lib/msal-common/src/utils/StringUtils.ts @@ -86,4 +86,10 @@ export class StringUtils { return !StringUtils.isEmpty(entry); }); } + + static jsonParseHelper(json: string) { + try { + return JSON.parse(json); + } catch (e) { } + } } From a67296c02886dc10d68412dc33452c984eeca2f3 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Wed, 8 Jul 2020 12:31:11 -0700 Subject: [PATCH 08/34] Move http status check into NetworkManager for telemetry --- lib/msal-common/src/network/NetworkManager.ts | 11 +++++- .../src/network/ThrottlingUtils.ts | 38 ++++++++----------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index 9a389a018dd..38bf49b3640 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -26,8 +26,15 @@ export class NetworkManager { public async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise> { ThrottlingUtils.preProcess(this.cacheManager, thumbprint); const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); - ThrottlingUtils.postProcess(this.cacheManager, thumbprint, response); - + + if (ThrottlingUtils.checkResponseStatus(response)) { + // Placeholder for Telemetry hook + ThrottlingUtils.postProcess(this.cacheManager, thumbprint, response); + } + else if (ThrottlingUtils.checkResponseForRetryAfter(response)) { + ThrottlingUtils.postProcess(this.cacheManager, thumbprint, response); + } + return response; } } diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 4249400844d..bd8e7012496 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -50,31 +50,25 @@ export class ThrottlingUtils { } static postProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint, response: NetworkResponse): void { - if (ThrottlingUtils.checkResponseForThrottle(response)) { - const thumbprintValue: RequestThumbprintValue = { - throttleTime: ThrottlingUtils.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))), - error: response.body.error, - errorCodes: response.body.error_codes, - errorMessage: response.body.error_description, - subError: response.body.suberror - }; - cacheManager.setItem( - ThrottlingUtils.generateThrottlingStorageKey(thumbprint), - ThrottlingUtils.generateThrottlingStorageKey(thumbprintValue) - ); - } + const thumbprintValue: RequestThumbprintValue = { + throttleTime: ThrottlingUtils.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))), + error: response.body.error, + errorCodes: response.body.error_codes, + errorMessage: response.body.error_description, + subError: response.body.suberror + }; + cacheManager.setItem( + ThrottlingUtils.generateThrottlingStorageKey(thumbprint), + ThrottlingUtils.generateThrottlingStorageKey(thumbprintValue) + ); } - private static checkResponseForThrottle(response: NetworkResponse): boolean { - if (response.status == 429 || response.status >= 500 && response.status < 600) { - return true; - } - - if (response.headers.has(HeaderNames.RETRY_AFTER) && response.status < 200 && response.status >= 300) { - return true; - } + public static checkResponseStatus(response: NetworkResponse): boolean { + return response.status == 429 || response.status >= 500 && response.status < 600; + } - return false; + public static checkResponseForRetryAfter(response: NetworkResponse): boolean { + return response.headers.has(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300) } private static calculateThrottleTime(throttleTime: number): number { From 582c18ef6e02515a8b27dc783f44f7abb661dc75 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Wed, 8 Jul 2020 12:53:22 -0700 Subject: [PATCH 09/34] Fix JSON.parse and add ServerError --- lib/msal-common/src/client/BaseClient.ts | 10 ++++++++++ lib/msal-common/src/client/RefreshTokenClient.ts | 2 +- lib/msal-common/src/network/NetworkManager.ts | 2 +- lib/msal-common/src/network/ThrottlingUtils.ts | 16 ++++++++-------- lib/msal-common/src/utils/StringUtils.ts | 12 +++++++++--- 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index a03211bec93..af8a1e805a4 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -9,9 +9,14 @@ import { NetworkManager, NetworkResponse } from "../network/NetworkManager"; import { ICrypto } from "../crypto/ICrypto"; import { Authority } from "../authority/Authority"; import { Logger } from "../logger/Logger"; +<<<<<<< HEAD import { AADServerParamKeys, Constants, HeaderNames, HeaderValues } from "../utils/Constants"; import { NetworkResponse } from "../network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizationTokenResponse"; +======= +import { AADServerParamKeys, Constants, HeaderNames } from "../utils/Constants"; +import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; +>>>>>>> fb76454f... Fix JSON.parse and add ServerError import { TrustedAuthority } from "../authority/TrustedAuthority"; import { CacheManager } from "../cache/CacheManager"; import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManager"; @@ -84,8 +89,13 @@ export abstract class BaseClient { */ protected createDefaultTokenRequestHeaders(): Record { const headers = this.createDefaultLibraryHeaders(); +<<<<<<< HEAD headers[HeaderNames.CONTENT_TYPE] = Constants.URL_FORM_CONTENT_TYPE; headers[HeaderNames.X_MS_LIB_CAPABILITY] = HeaderNames.X_MS_LIB_CAPABILITY_VALUE; +======= + headers.set(HeaderNames.CONTENT_TYPE, Constants.URL_FORM_CONTENT_TYPE); + headers.set(HeaderNames.X_MS_LIB_CAPABILITY, HeaderNames.X_MS_LIB_CAPABILITY_VALUE); +>>>>>>> b12c9108... Fix JSON.parse and add ServerError if (this.serverTelemetryManager) { headers[HeaderNames.X_CLIENT_CURR_TELEM] = this.serverTelemetryManager.generateCurrentRequestHeaderValue(); diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 99082fe969e..a18e06ce54c 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -6,7 +6,7 @@ import { ClientConfiguration } from "../config/ClientConfiguration"; import { BaseClient } from "./BaseClient"; import { RefreshTokenRequest } from "../request/RefreshTokenRequest"; -import { Authority, NetworkResponse } from ".."; +import { Authority } from ".."; import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizationTokenResponse"; import { RequestParameterBuilder } from "../request/RequestParameterBuilder"; import { ScopeSet } from "../request/ScopeSet"; diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index 38bf49b3640..d56be29755f 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -23,7 +23,7 @@ export class NetworkManager { this.cacheManager = cacheManager; } - public async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise> { + async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise> { ThrottlingUtils.preProcess(this.cacheManager, thumbprint); const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index bd8e7012496..e2303241e1d 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -8,6 +8,7 @@ import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationT import { Constants, HeaderNames } from "../utils/Constants"; import { CacheManager } from "../cache/CacheManager"; import { StringUtils } from "../utils/StringUtils"; +import { ServerError } from "../error/ServerError"; export type RequestThumbprint = { clientId: string; @@ -28,7 +29,7 @@ export type RequestThumbprintValue = { export class ThrottlingUtils { - static generateThrottlingStorageKey(thumbprint: RequestThumbprint | RequestThumbprintValue): string { + static generateThrottlingStorageKey(thumbprint: RequestThumbprint): string { return `${Constants.THROTTLE_PREFIX}.${JSON.stringify(thumbprint)}`; } @@ -39,13 +40,12 @@ export class ThrottlingUtils { if (storageValue) { const parsedValue = StringUtils.jsonParseHelper(storageValue); - if (parsedValue.throttleTime >= Date.now()) { + if (parsedValue && parsedValue.throttleTime >= Date.now()) { cacheManager.removeItem(key); return; } - // TODO: implement this error - // ThrottleError extends ServerError and adds a message about how long the request is throttled for - // throw new ThrottleError(storageValue.throttleTime, storageValue.error, storageValue.errorDescription, storageValue.subError); + + throw new ServerError(parsedValue.errorCodes, parsedValue.errorDescription, parsedValue.subError); } } @@ -59,15 +59,15 @@ export class ThrottlingUtils { }; cacheManager.setItem( ThrottlingUtils.generateThrottlingStorageKey(thumbprint), - ThrottlingUtils.generateThrottlingStorageKey(thumbprintValue) + JSON.stringify(thumbprintValue) ); } - public static checkResponseStatus(response: NetworkResponse): boolean { + static checkResponseStatus(response: NetworkResponse): boolean { return response.status == 429 || response.status >= 500 && response.status < 600; } - public static checkResponseForRetryAfter(response: NetworkResponse): boolean { + static checkResponseForRetryAfter(response: NetworkResponse): boolean { return response.headers.has(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300) } diff --git a/lib/msal-common/src/utils/StringUtils.ts b/lib/msal-common/src/utils/StringUtils.ts index b9a18d5e26a..408e811719a 100644 --- a/lib/msal-common/src/utils/StringUtils.ts +++ b/lib/msal-common/src/utils/StringUtils.ts @@ -87,9 +87,15 @@ export class StringUtils { }); } - static jsonParseHelper(json: string) { + /** + * Attempts to parse a string into JSON + * @param str + */ + static jsonParseHelper(str: string) { try { - return JSON.parse(json); - } catch (e) { } + return JSON.parse(str); + } catch (e) { + return null; + } } } From 527d4148ada93e7a641d861522654e4459ad4df3 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Wed, 8 Jul 2020 13:09:56 -0700 Subject: [PATCH 10/34] Move http status checks out of NetworkManager --- lib/msal-common/src/network/NetworkManager.ts | 13 ++++------ .../src/network/ThrottlingUtils.ts | 24 ++++++++++--------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index d56be29755f..a589e1f8de3 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -26,15 +26,10 @@ export class NetworkManager { async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise> { ThrottlingUtils.preProcess(this.cacheManager, thumbprint); const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); - - if (ThrottlingUtils.checkResponseStatus(response)) { - // Placeholder for Telemetry hook - ThrottlingUtils.postProcess(this.cacheManager, thumbprint, response); - } - else if (ThrottlingUtils.checkResponseForRetryAfter(response)) { - ThrottlingUtils.postProcess(this.cacheManager, thumbprint, response); - } - + ThrottlingUtils.postProcess(this.cacheManager, thumbprint, response); + + // Placeholder for Telemetry hook + return response; } } diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index e2303241e1d..0894c14969b 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -50,17 +50,19 @@ export class ThrottlingUtils { } static postProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint, response: NetworkResponse): void { - const thumbprintValue: RequestThumbprintValue = { - throttleTime: ThrottlingUtils.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))), - error: response.body.error, - errorCodes: response.body.error_codes, - errorMessage: response.body.error_description, - subError: response.body.suberror - }; - cacheManager.setItem( - ThrottlingUtils.generateThrottlingStorageKey(thumbprint), - JSON.stringify(thumbprintValue) - ); + if (ThrottlingUtils.checkResponseStatus(response) || ThrottlingUtils.checkResponseForRetryAfter(response)) { + const thumbprintValue: RequestThumbprintValue = { + throttleTime: ThrottlingUtils.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))), + error: response.body.error, + errorCodes: response.body.error_codes, + errorMessage: response.body.error_description, + subError: response.body.suberror + }; + cacheManager.setItem( + ThrottlingUtils.generateThrottlingStorageKey(thumbprint), + JSON.stringify(thumbprintValue) + ); + } } static checkResponseStatus(response: NetworkResponse): boolean { From 620fa1455ff589ea7f07ea458c2d9ed4f62fcdbb Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Wed, 8 Jul 2020 13:30:29 -0700 Subject: [PATCH 11/34] Fix calculateThrottleTime --- lib/msal-common/src/network/ThrottlingUtils.ts | 6 +++++- lib/msal-common/src/utils/Constants.ts | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 0894c14969b..be908faef41 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -74,6 +74,10 @@ export class ThrottlingUtils { } private static calculateThrottleTime(throttleTime: number): number { - return Math.min(throttleTime || Date.now() + Constants.DEFAULT_THROTTLE_TIME_MS, Constants.DEFAULT_MAX_THROTTLE_TIME_MS); + const currentSeconds = Date.now() * 1000; + return Math.min( + currentSeconds + (throttleTime || Constants.DEFAULT_THROTTLE_TIME_SECONDS), + currentSeconds + Constants.DEFAULT_MAX_THROTTLE_TIME_SECONDS + ); } } \ No newline at end of file diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index bf0cbf66caf..5aafd2d8743 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -38,8 +38,8 @@ export const Constants = { AUTHORIZATION_PENDING: "authorization_pending", NOT_DEFINED: "not_defined", // Default time to throttle RequestThumbprint in milliseconds - DEFAULT_THROTTLE_TIME_MS: 120000, - DEFAULT_MAX_THROTTLE_TIME_MS: 3600000 + DEFAULT_THROTTLE_TIME_SECONDS: 60, + DEFAULT_MAX_THROTTLE_TIME_SECONDS: 3600 }; /** From 02a3ed965afb29d87c646b1fa158d1cfe5b70b7f Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Wed, 8 Jul 2020 15:13:12 -0700 Subject: [PATCH 12/34] Add function comments --- lib/msal-common/src/network/NetworkManager.ts | 6 ++++ .../src/network/ThrottlingUtils.ts | 35 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index a589e1f8de3..d9043fc6062 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -23,6 +23,12 @@ export class NetworkManager { this.cacheManager = cacheManager; } + /** + * Wraps sendPostRequestAsync with necessary preflight and postflight logic + * @param thumbprint + * @param tokenEndpoint + * @param options + */ async sendPostRequest(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise> { ThrottlingUtils.preProcess(this.cacheManager, thumbprint); const response = await this.networkClient.sendPostRequestAsync(tokenEndpoint, options); diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index be908faef41..8bee1c6ee13 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -10,6 +10,9 @@ import { CacheManager } from "../cache/CacheManager"; import { StringUtils } from "../utils/StringUtils"; import { ServerError } from "../error/ServerError"; +/** + * Type representing a unique request thumbprint. + */ export type RequestThumbprint = { clientId: string; authority: string; @@ -17,6 +20,9 @@ export type RequestThumbprint = { homeAccountIdentifier?: string; } +/** + * Type representing the values associated with a RequestThumbprint. + */ export type RequestThumbprintValue = { // Unix-time value representing the expiration of the throttle throttleTime: number; @@ -29,10 +35,19 @@ export type RequestThumbprintValue = { export class ThrottlingUtils { + /** + * Prepares a RequestThumbprint to be stored as a key. + * @param thumbprint + */ static generateThrottlingStorageKey(thumbprint: RequestThumbprint): string { return `${Constants.THROTTLE_PREFIX}.${JSON.stringify(thumbprint)}`; } + /** + * Performs necessary throttling checks before a network request. + * @param cacheManager + * @param thumbprint + */ static preProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint): void { const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); const storageValue = cacheManager.getItem(key) as string; @@ -47,8 +62,14 @@ export class ThrottlingUtils { throw new ServerError(parsedValue.errorCodes, parsedValue.errorDescription, parsedValue.subError); } - } + } + /** + * Performs necessary throttling checks after a network request. + * @param cacheManager + * @param thumbprint + * @param response + */ static postProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint, response: NetworkResponse): void { if (ThrottlingUtils.checkResponseStatus(response) || ThrottlingUtils.checkResponseForRetryAfter(response)) { const thumbprintValue: RequestThumbprintValue = { @@ -65,14 +86,26 @@ export class ThrottlingUtils { } } + /** + * Checks a NetworkResponse object's status codes against 429 or 5xx + * @param response + */ static checkResponseStatus(response: NetworkResponse): boolean { return response.status == 429 || response.status >= 500 && response.status < 600; } + /** + * Checks a NetworkResponse object's RetryAfter header + * @param response + */ static checkResponseForRetryAfter(response: NetworkResponse): boolean { return response.headers.has(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300) } + /** + * Calculates the Unix-time value for a throttle to expire given throttleTime in seconds. + * @param throttleTime + */ private static calculateThrottleTime(throttleTime: number): number { const currentSeconds = Date.now() * 1000; return Math.min( From d8bf30c22738c810e0e470f7d82def36c2de903e Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Wed, 8 Jul 2020 15:19:09 -0700 Subject: [PATCH 13/34] Fix calculateThrottleTime decimals --- lib/msal-common/src/network/ThrottlingUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 8bee1c6ee13..9b5f0f0abfa 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -107,10 +107,10 @@ export class ThrottlingUtils { * @param throttleTime */ private static calculateThrottleTime(throttleTime: number): number { - const currentSeconds = Date.now() * 1000; - return Math.min( + const currentSeconds = Date.now() / 1000; + return Math.floor(Math.min( currentSeconds + (throttleTime || Constants.DEFAULT_THROTTLE_TIME_SECONDS), currentSeconds + Constants.DEFAULT_MAX_THROTTLE_TIME_SECONDS - ); + ) * 1000); } } \ No newline at end of file From d8e469005a443699ec5bf36740f559edd24a8319 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 14 Jul 2020 13:50:19 -0700 Subject: [PATCH 14/34] Add a few unit tests --- .../src/network/ThrottlingUtils.ts | 5 +- .../test/network/NetworkManager.spec.ts | 0 .../test/network/ThrottlingUtils.spec.ts | 88 +++++++++++++++++++ .../test/utils/StringUtils.spec.ts | 20 +++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 lib/msal-common/test/network/NetworkManager.spec.ts create mode 100644 lib/msal-common/test/network/ThrottlingUtils.spec.ts diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 9b5f0f0abfa..b78639a7879 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -106,7 +106,10 @@ export class ThrottlingUtils { * Calculates the Unix-time value for a throttle to expire given throttleTime in seconds. * @param throttleTime */ - private static calculateThrottleTime(throttleTime: number): number { + static calculateThrottleTime(throttleTime: number): number { + if(throttleTime <= 0) { + throttleTime = null; + } const currentSeconds = Date.now() / 1000; return Math.floor(Math.min( currentSeconds + (throttleTime || Constants.DEFAULT_THROTTLE_TIME_SECONDS), diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts new file mode 100644 index 00000000000..6d07d8df7dd --- /dev/null +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -0,0 +1,88 @@ +import { expect } from "chai"; +import sinon from "sinon"; +import { ThrottlingUtils } from "../../src/network/ThrottlingUtils"; +import { NetworkResponse } from "../../src/network/NetworkManager"; +import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; + +describe.only("ThrottlingUtils", () => { + describe("checkResponseForRetryAfter", () => { + it("returns true when Retry-After header exists and when status <= 200", () => { + const headers = new Map(); + headers.set("Retry-After", "test"); + const res: NetworkResponse = { + headers, + body: { }, + status: 199 + }; + + const bool = ThrottlingUtils.checkResponseForRetryAfter(res); + expect(bool).to.be.true; + }); + + it("returns true when Retry-After header exists and when status > 300", () => { + const headers = new Map(); + headers.set("Retry-After", "test"); + const res: NetworkResponse = { + headers, + body: { }, + status: 300 + }; + + const bool = ThrottlingUtils.checkResponseForRetryAfter(res); + expect(bool).to.be.true; + }); + + it("returns false when there is no RetryAfter header", () => { + const headers = new Map(); + const res: NetworkResponse = { + headers, + body: { }, + status: 301 + }; + + const bool = ThrottlingUtils.checkResponseForRetryAfter(res); + expect(bool).to.be.false; + }); + + it("returns false when 200 <= status < 300", () => { + const headers = new Map(); + const res: NetworkResponse = { + headers, + body: { }, + status: 200 + }; + + const bool = ThrottlingUtils.checkResponseForRetryAfter(res); + expect(bool).to.be.false; + }); + }); + + describe("calculateThrottleTime", () => { + before(() => { + sinon.stub(Date, "now").callsFake(() => 5000) + }); + + after(() => { + sinon.restore(); + }); + + it("returns calculated time to throttle", () => { + const time = ThrottlingUtils.calculateThrottleTime(10); + expect(time).to.be.deep.eq(15000); + }); + + it("calculates with the default time given a bad number", () => { + const time1 = ThrottlingUtils.calculateThrottleTime(-1); + const time2 = ThrottlingUtils.calculateThrottleTime(0); + const time3 = ThrottlingUtils.calculateThrottleTime(null); + expect(time1).to.be.deep.eq(65000); + expect(time2).to.be.deep.eq(65000); + expect(time3).to.be.deep.eq(65000); + }); + + it("calculates with the default MAX if given too large of a number", () => { + const time = ThrottlingUtils.calculateThrottleTime(1000000000); + expect(time).to.be.deep.eq(3605000); + }); + }); +}); \ No newline at end of file diff --git a/lib/msal-common/test/utils/StringUtils.spec.ts b/lib/msal-common/test/utils/StringUtils.spec.ts index ea6232ceab9..a5b41e88cf8 100644 --- a/lib/msal-common/test/utils/StringUtils.spec.ts +++ b/lib/msal-common/test/utils/StringUtils.spec.ts @@ -3,7 +3,11 @@ import { StringUtils } from "../../src/utils/StringUtils"; import { TEST_TOKENS } from "./StringConstants"; import { ClientAuthError, ClientAuthErrorMessage } from "../../src/error/ClientAuthError"; import { AuthError } from "../../src/error/AuthError"; +<<<<<<< HEAD import { IdToken } from "../../src"; +======= +import mocha from "mocha"; +>>>>>>> e78e3542... Add a few unit tests describe("StringUtils.ts Class Unit Tests", () => { @@ -131,4 +135,20 @@ describe("StringUtils.ts Class Unit Tests", () => { it("removeEmptyStringsFromArray() removes empty strings from an array", () => { }); + + describe("jsonParseHelper", () => { + it("parses json", () => { + const test = { test: "json" }; + const jsonString = JSON.stringify(test); + const parsedVal = StringUtils.jsonParseHelper(jsonString); + expect(parsedVal).to.be.deep.eq(test); + }); + + it("returns null on error", () => { + const parsedValNull = StringUtils.jsonParseHelper(null); + const parsedValEmptyString = StringUtils.jsonParseHelper(""); + expect(parsedValNull).to.be.null; + expect(parsedValEmptyString).to.be.null; + }) + }); }); From b65f62516aed21d60dde4911669150f84bd267dd Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 14 Jul 2020 15:55:47 -0700 Subject: [PATCH 15/34] Add the rest of ThrottlingUtils tests --- .../src/network/ThrottlingUtils.ts | 11 +- .../test/network/ThrottlingUtils.spec.ts | 170 +++++++++++++++++- 2 files changed, 175 insertions(+), 6 deletions(-) diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index b78639a7879..94f7d08e189 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -55,12 +55,13 @@ export class ThrottlingUtils { if (storageValue) { const parsedValue = StringUtils.jsonParseHelper(storageValue); - if (parsedValue && parsedValue.throttleTime >= Date.now()) { - cacheManager.removeItem(key); - return; + if (parsedValue) { + if (parsedValue.throttleTime >= Date.now()) { + cacheManager.removeItem(key); + return; + } + throw new ServerError(parsedValue.errorCodes, parsedValue.errorDescription, parsedValue.subError); } - - throw new ServerError(parsedValue.errorCodes, parsedValue.errorDescription, parsedValue.subError); } } diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index 6d07d8df7dd..835d9c84000 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -1,10 +1,174 @@ import { expect } from "chai"; import sinon from "sinon"; -import { ThrottlingUtils } from "../../src/network/ThrottlingUtils"; +import { ThrottlingUtils, RequestThumbprint } from "../../src/network/ThrottlingUtils"; import { NetworkResponse } from "../../src/network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; +import { MockStorageClass } from "../client/ClientTestUtils"; +import { ServerError } from "../../src"; describe.only("ThrottlingUtils", () => { + describe("generateThrottlingStorageKey", () => { + it("returns a throttling key", () => { + const thumbprint: RequestThumbprint = { + clientId: "", + authority: "", + scopes: new Array() + }; + const jsonString = JSON.stringify(thumbprint); + const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); + + expect(key).to.deep.eq(`throttle.${jsonString}`); + }); + }); + + describe("preProcess", () => { + afterEach(() => { + sinon.restore(); + }) + + it("checks the cache and throws an error", () => { + const thumbprint: RequestThumbprint = { + clientId: "", + authority: "", + scopes: new Array() + }; + + const str = JSON.stringify({ + throttleTime: 5, + errorCodes: "", + errorDescription: "", + subError: "" + }); + + + const cache = new MockStorageClass(); + const removeItemStub = sinon.stub(cache, "removeItem"); + sinon.stub(cache, "getItem").callsFake(() => str); + + try { + ThrottlingUtils.preProcess(cache, thumbprint); + } catch { } + sinon.assert.callCount(removeItemStub, 0) + + expect(() => ThrottlingUtils.preProcess(cache, thumbprint)).to.throw(ServerError); + }); + + it("checks the cache and removes an item", () => { + const thumbprint: RequestThumbprint = { + clientId: "", + authority: "", + scopes: new Array() + }; + + const str = JSON.stringify({ throttleTime: 5 }); + const cache = new MockStorageClass(); + const removeItemStub = sinon.stub(cache, "removeItem"); + sinon.stub(cache, "getItem").callsFake(() => str); + sinon.stub(Date, "now").callsFake(() => 1) + + ThrottlingUtils.preProcess(cache, thumbprint); + sinon.assert.callCount(removeItemStub, 1) + + expect(() => ThrottlingUtils.preProcess(cache, thumbprint)).to.not.throw; + }); + + it("checks the cache and does nothing with no match", () => { + const thumbprint: RequestThumbprint = { + clientId: "", + authority: "", + scopes: new Array() + }; + + const cache = new MockStorageClass(); + const removeItemStub = sinon.stub(cache, "removeItem"); + sinon.stub(cache, "getItem").callsFake(() => ""); + + ThrottlingUtils.preProcess(cache, thumbprint); + sinon.assert.callCount(removeItemStub, 0) + + expect(() => ThrottlingUtils.preProcess(cache, thumbprint)).to.not.throw; + }); + }); + + describe("postProcess", () => { + afterEach(() => { + sinon.restore(); + }); + + it("sets an item in the cache", () => { + const thumbprint: RequestThumbprint = { + clientId: "", + authority: "", + scopes: new Array() + }; + + const res: NetworkResponse = { + headers: new Map(), + body: { }, + status: 429 + }; + const cache = new MockStorageClass(); + const setItemStub = sinon.stub(cache, "setItem"); + + ThrottlingUtils.postProcess(cache, thumbprint, res); + sinon.assert.callCount(setItemStub, 1); + }); + + it("does not set an item in the cache", () => { + const thumbprint: RequestThumbprint = { + clientId: "", + authority: "", + scopes: new Array() + }; + + const res: NetworkResponse = { + headers: new Map(), + body: { }, + status: 430 + }; + const cache = new MockStorageClass(); + const setItemStub = sinon.stub(cache, "setItem"); + + ThrottlingUtils.postProcess(cache, thumbprint, res); + sinon.assert.callCount(setItemStub, 0); + }); + }) + + describe("checkResponseStatus", () => { + it("returns true if status == 429", () => { + const res: NetworkResponse = { + headers: new Map(), + body: { }, + status: 429 + }; + + const bool = ThrottlingUtils.checkResponseStatus(res); + expect(bool).to.be.true; + }); + + it("returns true if 500 <= status < 600", () => { + const res: NetworkResponse = { + headers: new Map(), + body: { }, + status: 500 + }; + + const bool = ThrottlingUtils.checkResponseStatus(res); + expect(bool).to.be.true; + }); + + it("returns false if status is not 429 or between 500 and 600", () => { + const res: NetworkResponse = { + headers: new Map(), + body: { }, + status: 430 + }; + + const bool = ThrottlingUtils.checkResponseStatus(res); + expect(bool).to.be.false; + }); + }); + describe("checkResponseForRetryAfter", () => { it("returns true when Retry-After header exists and when status <= 200", () => { const headers = new Map(); @@ -75,6 +239,8 @@ describe.only("ThrottlingUtils", () => { const time1 = ThrottlingUtils.calculateThrottleTime(-1); const time2 = ThrottlingUtils.calculateThrottleTime(0); const time3 = ThrottlingUtils.calculateThrottleTime(null); + + // Based on Constants.DEFAULT_THROTTLE_TIME_SECONDS expect(time1).to.be.deep.eq(65000); expect(time2).to.be.deep.eq(65000); expect(time3).to.be.deep.eq(65000); @@ -82,6 +248,8 @@ describe.only("ThrottlingUtils", () => { it("calculates with the default MAX if given too large of a number", () => { const time = ThrottlingUtils.calculateThrottleTime(1000000000); + + // Based on Constants.DEFAULT_MAX_THROTTLE_TIME_SECONDS expect(time).to.be.deep.eq(3605000); }); }); From a819ece20ad1a54260cb86bb482a33ffb76b8844 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 14 Jul 2020 16:46:31 -0700 Subject: [PATCH 16/34] Add NetworkManager tests --- .../test/network/NetworkManager.spec.ts | 53 +++++++++++++++++++ .../test/network/ThrottlingUtils.spec.ts | 7 ++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts index e69de29bb2d..3de1f43d315 100644 --- a/lib/msal-common/test/network/NetworkManager.spec.ts +++ b/lib/msal-common/test/network/NetworkManager.spec.ts @@ -0,0 +1,53 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { expect } from "chai"; +import sinon from "sinon"; +import { ThrottlingUtils, RequestThumbprint } from "../../src/network/ThrottlingUtils"; +import { NetworkManager } from "../../src/network/NetworkManager"; +import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; +import { MockStorageClass } from "../client/ClientTestUtils"; +import { NetworkRequestOptions } from "../../src/network/INetworkModule"; + +describe("NetworkManager", () => { + describe("sendPostRequest", () => { + afterEach(() => { + sinon.restore(); + }); + + it("returns a response", async () => { + const networkInterface = { + sendGetRequestAsync: async (url: string, options?: NetworkRequestOptions): Promise => { + return { test: "test" }; + }, + sendPostRequestAsync: async (url: string, options?: NetworkRequestOptions): Promise => { + return { test: "test" }; + } + } + const cache = new MockStorageClass(); + const networkManager = new NetworkManager(networkInterface, cache); + const thumbprint: RequestThumbprint = { + clientId: "", + authority: "", + scopes: new Array() + }; + const options: NetworkRequestOptions = { + headers: new Map(), + body: "" + } + const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync").returns(Promise.resolve("test")); + const preProcessStub = sinon.stub(ThrottlingUtils, "preProcess"); + const postProcessStub = sinon.stub(ThrottlingUtils, "postProcess"); + + const res = await networkManager.sendPostRequest(thumbprint, "tokenEndpoint", options); + console.log("RES ", res); + + sinon.assert.callCount(networkStub, 1); + sinon.assert.callCount(preProcessStub, 1); + sinon.assert.callCount(postProcessStub, 1); + expect(res).to.deep.eq("test"); + }); + }); +}); \ No newline at end of file diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index 835d9c84000..76608afb32e 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -1,3 +1,8 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + import { expect } from "chai"; import sinon from "sinon"; import { ThrottlingUtils, RequestThumbprint } from "../../src/network/ThrottlingUtils"; @@ -6,7 +11,7 @@ import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthori import { MockStorageClass } from "../client/ClientTestUtils"; import { ServerError } from "../../src"; -describe.only("ThrottlingUtils", () => { +describe("ThrottlingUtils", () => { describe("generateThrottlingStorageKey", () => { it("returns a throttling key", () => { const thumbprint: RequestThumbprint = { From f37e5351d14e49595692740774bf6dffd14befb5 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 16 Jul 2020 10:44:34 -0700 Subject: [PATCH 17/34] Fix white space --- lib/msal-common/test/network/NetworkManager.spec.ts | 3 +-- lib/msal-common/test/network/ThrottlingUtils.spec.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts index 3de1f43d315..b189cc2bf72 100644 --- a/lib/msal-common/test/network/NetworkManager.spec.ts +++ b/lib/msal-common/test/network/NetworkManager.spec.ts @@ -16,7 +16,7 @@ describe("NetworkManager", () => { afterEach(() => { sinon.restore(); }); - + it("returns a response", async () => { const networkInterface = { sendGetRequestAsync: async (url: string, options?: NetworkRequestOptions): Promise => { @@ -42,7 +42,6 @@ describe("NetworkManager", () => { const postProcessStub = sinon.stub(ThrottlingUtils, "postProcess"); const res = await networkManager.sendPostRequest(thumbprint, "tokenEndpoint", options); - console.log("RES ", res); sinon.assert.callCount(networkStub, 1); sinon.assert.callCount(preProcessStub, 1); diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index 76608afb32e..5569c9b0045 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -45,7 +45,6 @@ describe("ThrottlingUtils", () => { subError: "" }); - const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); sinon.stub(cache, "getItem").callsFake(() => str); @@ -129,7 +128,7 @@ describe("ThrottlingUtils", () => { const res: NetworkResponse = { headers: new Map(), body: { }, - status: 430 + status: 200 }; const cache = new MockStorageClass(); const setItemStub = sinon.stub(cache, "setItem"); From 29fd838695865efebad9012fdb617555bf6f874a Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 16 Jul 2020 12:11:17 -0700 Subject: [PATCH 18/34] Add tests constants --- .../src/network/ThrottlingUtils.ts | 2 +- .../test/network/NetworkManager.spec.ts | 117 ++++++++++++++---- .../test/network/ThrottlingUtils.spec.ts | 61 +++------ lib/msal-common/test/utils/StringConstants.ts | 30 +++++ 4 files changed, 140 insertions(+), 70 deletions(-) diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 94f7d08e189..f98cd153258 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -56,7 +56,7 @@ export class ThrottlingUtils { const parsedValue = StringUtils.jsonParseHelper(storageValue); if (parsedValue) { - if (parsedValue.throttleTime >= Date.now()) { + if (parsedValue.throttleTime < Date.now()) { cacheManager.removeItem(key); return; } diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts index b189cc2bf72..ceb2eb49c94 100644 --- a/lib/msal-common/test/network/NetworkManager.spec.ts +++ b/lib/msal-common/test/network/NetworkManager.spec.ts @@ -6,10 +6,12 @@ import { expect } from "chai"; import sinon from "sinon"; import { ThrottlingUtils, RequestThumbprint } from "../../src/network/ThrottlingUtils"; -import { NetworkManager } from "../../src/network/NetworkManager"; +import { NetworkManager, NetworkResponse } from "../../src/network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; import { NetworkRequestOptions } from "../../src/network/INetworkModule"; +import { ServerError } from "../../src/error/ServerError"; +import { AUTHENTICATION_RESULT, NETWORK_REQUEST_OPTIONS, THUMBPRINT, THUMBPRINT_VALUE, DEFAULT_NETWORK_IMPLEMENTATION } from "../utils/StringConstants"; describe("NetworkManager", () => { describe("sendPostRequest", () => { @@ -18,35 +20,106 @@ describe("NetworkManager", () => { }); it("returns a response", async () => { - const networkInterface = { - sendGetRequestAsync: async (url: string, options?: NetworkRequestOptions): Promise => { - return { test: "test" }; - }, - sendPostRequestAsync: async (url: string, options?: NetworkRequestOptions): Promise => { - return { test: "test" }; - } + const networkInterface = DEFAULT_NETWORK_IMPLEMENTATION; + const cache = new MockStorageClass(); + const networkManager = new NetworkManager(networkInterface, cache); + const thumbprint: RequestThumbprint = THUMBPRINT; + const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; + const mockRes: NetworkResponse = { + headers: new Map(), + body: AUTHENTICATION_RESULT.body, + status: 200 } + const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync").returns(Promise.resolve(mockRes)); + const getItemStub = sinon.stub(cache, "getItem"); + const setItemStub = sinon.stub(cache, "setItem"); + const removeItemStub = sinon.stub(cache, "removeItem"); + sinon.stub(Date, "now").callsFake(() => 1) + + const res = await networkManager.sendPostRequest>(thumbprint, "tokenEndpoint", options); + + sinon.assert.callCount(networkStub, 1); + sinon.assert.callCount(getItemStub, 1); + sinon.assert.callCount(setItemStub, 0); + sinon.assert.callCount(removeItemStub, 0); + expect(res).to.deep.eq(mockRes); + }); + + it("blocks the request if item is found in the cache", async () => { + const networkInterface = DEFAULT_NETWORK_IMPLEMENTATION; + const cache = new MockStorageClass(); + const networkManager = new NetworkManager(networkInterface, cache); + const thumbprint: RequestThumbprint = THUMBPRINT; + const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; + const mockThumbprintValue = THUMBPRINT_VALUE; + const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync"); + const getItemStub = sinon.stub(cache, "getItem").returns(JSON.stringify(mockThumbprintValue)); + const setItemStub = sinon.stub(cache, "setItem"); + const removeItemStub = sinon.stub(cache, "removeItem"); + sinon.stub(Date, "now").callsFake(() => 1) + + try { + await networkManager.sendPostRequest>(thumbprint, "tokenEndpoint", options); + } catch { } + + sinon.assert.callCount(networkStub, 0); + sinon.assert.callCount(getItemStub, 1); + sinon.assert.callCount(setItemStub, 0); + sinon.assert.callCount(removeItemStub, 0); + expect(() => ThrottlingUtils.preProcess(cache, thumbprint)).to.throw(ServerError); + }); + + it("passes request through if expired item in cache", async () => { + const networkInterface = DEFAULT_NETWORK_IMPLEMENTATION; + const cache = new MockStorageClass(); + const networkManager = new NetworkManager(networkInterface, cache); + const thumbprint: RequestThumbprint = THUMBPRINT; + const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; + const mockRes: NetworkResponse = { + headers: new Map(), + body: AUTHENTICATION_RESULT.body, + status: 200 + } + const mockThumbprintValue = THUMBPRINT_VALUE; + const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync").returns(Promise.resolve(mockRes)); + const getItemStub = sinon.stub(cache, "getItem").returns(JSON.stringify(mockThumbprintValue)); + const setItemStub = sinon.stub(cache, "setItem"); + const removeItemStub = sinon.stub(cache, "removeItem"); + sinon.stub(Date, "now").callsFake(() => 10) + + const res = await networkManager.sendPostRequest>(thumbprint, "tokenEndpoint", options); + + sinon.assert.callCount(networkStub, 1); + sinon.assert.callCount(getItemStub, 1); + sinon.assert.callCount(setItemStub, 0); + sinon.assert.callCount(removeItemStub, 1); + expect(res).to.deep.eq(mockRes); + }); + + it("creates cache entry on error", async () => { + const networkInterface = DEFAULT_NETWORK_IMPLEMENTATION; const cache = new MockStorageClass(); const networkManager = new NetworkManager(networkInterface, cache); - const thumbprint: RequestThumbprint = { - clientId: "", - authority: "", - scopes: new Array() - }; - const options: NetworkRequestOptions = { + const thumbprint: RequestThumbprint = THUMBPRINT; + const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; + const mockRes: NetworkResponse = { headers: new Map(), - body: "" + body: AUTHENTICATION_RESULT.body, + status: 500 } - const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync").returns(Promise.resolve("test")); - const preProcessStub = sinon.stub(ThrottlingUtils, "preProcess"); - const postProcessStub = sinon.stub(ThrottlingUtils, "postProcess"); + const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync").returns(Promise.resolve(mockRes)); + const getItemStub = sinon.stub(cache, "getItem"); + const setItemStub = sinon.stub(cache, "setItem"); + const removeItemStub = sinon.stub(cache, "removeItem"); + sinon.stub(Date, "now").callsFake(() => 1) - const res = await networkManager.sendPostRequest(thumbprint, "tokenEndpoint", options); + const res = await networkManager.sendPostRequest>(thumbprint, "tokenEndpoint", options); sinon.assert.callCount(networkStub, 1); - sinon.assert.callCount(preProcessStub, 1); - sinon.assert.callCount(postProcessStub, 1); - expect(res).to.deep.eq("test"); + sinon.assert.callCount(getItemStub, 1); + sinon.assert.callCount(setItemStub, 1); + sinon.assert.callCount(removeItemStub, 0); + expect(res).to.deep.eq(mockRes); }); }); }); \ No newline at end of file diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index 5569c9b0045..1e457d68806 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -5,20 +5,17 @@ import { expect } from "chai"; import sinon from "sinon"; -import { ThrottlingUtils, RequestThumbprint } from "../../src/network/ThrottlingUtils"; +import { ThrottlingUtils, RequestThumbprint, RequestThumbprintValue } from "../../src/network/ThrottlingUtils"; import { NetworkResponse } from "../../src/network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; import { ServerError } from "../../src"; +import { THUMBPRINT, THUMBPRINT_VALUE } from "../utils/StringConstants"; describe("ThrottlingUtils", () => { describe("generateThrottlingStorageKey", () => { it("returns a throttling key", () => { - const thumbprint: RequestThumbprint = { - clientId: "", - authority: "", - scopes: new Array() - }; + const thumbprint: RequestThumbprint = THUMBPRINT; const jsonString = JSON.stringify(thumbprint); const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); @@ -32,22 +29,12 @@ describe("ThrottlingUtils", () => { }) it("checks the cache and throws an error", () => { - const thumbprint: RequestThumbprint = { - clientId: "", - authority: "", - scopes: new Array() - }; - - const str = JSON.stringify({ - throttleTime: 5, - errorCodes: "", - errorDescription: "", - subError: "" - }); - + const thumbprint: RequestThumbprint = THUMBPRINT; + const thumbprintValue: RequestThumbprintValue = THUMBPRINT_VALUE; const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); - sinon.stub(cache, "getItem").callsFake(() => str); + sinon.stub(cache, "getItem").callsFake(() => JSON.stringify(thumbprintValue)); + sinon.stub(Date, "now").callsFake(() => 1) try { ThrottlingUtils.preProcess(cache, thumbprint); @@ -58,17 +45,12 @@ describe("ThrottlingUtils", () => { }); it("checks the cache and removes an item", () => { - const thumbprint: RequestThumbprint = { - clientId: "", - authority: "", - scopes: new Array() - }; - - const str = JSON.stringify({ throttleTime: 5 }); + const thumbprint: RequestThumbprint = THUMBPRINT; + const thumbprintValue: RequestThumbprintValue = THUMBPRINT_VALUE; const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); - sinon.stub(cache, "getItem").callsFake(() => str); - sinon.stub(Date, "now").callsFake(() => 1) + sinon.stub(cache, "getItem").callsFake(() => JSON.stringify(thumbprintValue)); + sinon.stub(Date, "now").callsFake(() => 10) ThrottlingUtils.preProcess(cache, thumbprint); sinon.assert.callCount(removeItemStub, 1) @@ -77,12 +59,7 @@ describe("ThrottlingUtils", () => { }); it("checks the cache and does nothing with no match", () => { - const thumbprint: RequestThumbprint = { - clientId: "", - authority: "", - scopes: new Array() - }; - + const thumbprint: RequestThumbprint = THUMBPRINT; const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); sinon.stub(cache, "getItem").callsFake(() => ""); @@ -100,12 +77,7 @@ describe("ThrottlingUtils", () => { }); it("sets an item in the cache", () => { - const thumbprint: RequestThumbprint = { - clientId: "", - authority: "", - scopes: new Array() - }; - + const thumbprint: RequestThumbprint = THUMBPRINT; const res: NetworkResponse = { headers: new Map(), body: { }, @@ -119,12 +91,7 @@ describe("ThrottlingUtils", () => { }); it("does not set an item in the cache", () => { - const thumbprint: RequestThumbprint = { - clientId: "", - authority: "", - scopes: new Array() - }; - + const thumbprint: RequestThumbprint = THUMBPRINT; const res: NetworkResponse = { headers: new Map(), body: { }, diff --git a/lib/msal-common/test/utils/StringConstants.ts b/lib/msal-common/test/utils/StringConstants.ts index 6d16b07a5a7..c403ef80dfc 100644 --- a/lib/msal-common/test/utils/StringConstants.ts +++ b/lib/msal-common/test/utils/StringConstants.ts @@ -3,6 +3,8 @@ */ import { Constants } from "../../src/utils/Constants"; +import { RequestThumbprint, RequestThumbprintValue } from "../../src"; +import { NetworkRequestOptions } from "../../src/network/INetworkModule"; // Test Tokens export const TEST_TOKENS = { @@ -276,3 +278,31 @@ export const AUTHORIZATION_PENDING_RESPONSE = { error_uri: 'https://login.microsoftonline.com/error?code=70016' } }; + +export const DEFAULT_NETWORK_IMPLEMENTATION = { + sendGetRequestAsync: async (url: string, options?: NetworkRequestOptions): Promise => { + return { test: "test" }; + }, + sendPostRequestAsync: async (url: string, options?: NetworkRequestOptions): Promise => { + return { test: "test" }; + } +} + +export const THUMBPRINT: RequestThumbprint = { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + authority: TEST_CONFIG.validAuthority, + scopes: TEST_CONFIG.DEFAULT_SCOPES +}; + +export const THUMBPRINT_VALUE: RequestThumbprintValue = { + throttleTime: 5, + error: "This is a error", + errorCodes: ["ErrorCode"], + errorMessage:"This is an errorMessage", + subError: "This is a subError" +}; + +export const NETWORK_REQUEST_OPTIONS: NetworkRequestOptions = { + headers: new Map(), + body: "" +}; From b4cb6f90f2d28c534fdc77e7631a4aedabf89e82 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 16 Jul 2020 12:39:56 -0700 Subject: [PATCH 19/34] Fix lint errors and move types into separate files --- .../src/client/AuthorizationCodeClient.ts | 2 +- lib/msal-common/src/client/BaseClient.ts | 4 +-- .../src/client/RefreshTokenClient.ts | 2 +- lib/msal-common/src/index.ts | 4 ++- lib/msal-common/src/network/NetworkManager.ts | 4 +-- .../src/network/RequestThumbprint.ts | 14 ++++++++ .../src/network/RequestThumbprintValue.ts | 17 ++++++++++ .../src/network/ThrottlingUtils.ts | 33 ++++--------------- lib/msal-common/src/utils/StringUtils.ts | 4 +-- .../test/network/NetworkManager.spec.ts | 3 +- .../test/network/ThrottlingUtils.spec.ts | 4 ++- .../test/utils/StringUtils.spec.ts | 4 --- 12 files changed, 52 insertions(+), 43 deletions(-) create mode 100644 lib/msal-common/src/network/RequestThumbprint.ts create mode 100644 lib/msal-common/src/network/RequestThumbprintValue.ts diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 6d614640344..ab20fd31da8 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -22,7 +22,7 @@ import { ServerAuthorizationCodeResponse } from "../response/ServerAuthorization import { AccountEntity } from "../cache/entities/AccountEntity"; import { EndSessionRequest } from "../request/EndSessionRequest"; import { ClientConfigurationError } from "../error/ClientConfigurationError"; -import { RequestThumbprint } from '../network/ThrottlingUtils'; +import { RequestThumbprint } from "../network/RequestThumbprint"; /** * Oauth2.0 Authorization Code client diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index af8a1e805a4..04de41ac8fd 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -9,19 +9,17 @@ import { NetworkManager, NetworkResponse } from "../network/NetworkManager"; import { ICrypto } from "../crypto/ICrypto"; import { Authority } from "../authority/Authority"; import { Logger } from "../logger/Logger"; -<<<<<<< HEAD import { AADServerParamKeys, Constants, HeaderNames, HeaderValues } from "../utils/Constants"; import { NetworkResponse } from "../network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizationTokenResponse"; -======= import { AADServerParamKeys, Constants, HeaderNames } from "../utils/Constants"; import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; ->>>>>>> fb76454f... Fix JSON.parse and add ServerError import { TrustedAuthority } from "../authority/TrustedAuthority"; import { CacheManager } from "../cache/CacheManager"; import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManager"; import { RequestThumbprint } from "../network/RequestThumbprint"; import { RequestThumbprint } from '../network/ThrottlingUtils'; +import { RequestThumbprint } from "../network/RequestThumbprint"; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index a18e06ce54c..d7a1da55c4a 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -19,7 +19,7 @@ import { NetworkResponse } from "../network/ThrottlingManager"; import { RequestThumbprint } from "../network/RequestThumbprint" import { RequestThumbprintValue } from "../network/RequestThumbprintValue"; import { NetworkResponse } from "../network/NetworkManager"; -import { RequestThumbprint} from "../network/ThrottlingUtils"; +import { RequestThumbprint} from "../network/RequestThumbprint"; /** diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index 854b7e792e7..e7cceb05e43 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -28,7 +28,9 @@ export { RefreshTokenEntity } from "./cache/entities/RefreshTokenEntity"; // Network Interface export { INetworkModule } from "./network/INetworkModule"; export { NetworkManager } from "./network/NetworkManager"; -export { ThrottlingUtils, RequestThumbprint, RequestThumbprintValue } from "./network/ThrottlingUtils"; +export { ThrottlingUtils } from "./network/ThrottlingUtils"; +export { RequestThumbprint } from "./network/RequestThumbprint"; +export { RequestThumbprintValue } from "./network/RequestThumbprintValue"; export { IUri } from "./url/IUri"; export { UrlString } from "./url/UrlString"; // Crypto Interface diff --git a/lib/msal-common/src/network/NetworkManager.ts b/lib/msal-common/src/network/NetworkManager.ts index d9043fc6062..68c44e01d48 100644 --- a/lib/msal-common/src/network/NetworkManager.ts +++ b/lib/msal-common/src/network/NetworkManager.ts @@ -4,9 +4,9 @@ */ import { INetworkModule, NetworkRequestOptions } from "./INetworkModule"; -import { RequestThumbprint } from "./ThrottlingUtils"; +import { RequestThumbprint } from "./RequestThumbprint"; import { ThrottlingUtils } from "./ThrottlingUtils"; -import { CacheManager } from '../cache/CacheManager'; +import { CacheManager } from "../cache/CacheManager"; export type NetworkResponse = { headers: Record; diff --git a/lib/msal-common/src/network/RequestThumbprint.ts b/lib/msal-common/src/network/RequestThumbprint.ts new file mode 100644 index 00000000000..d792acca141 --- /dev/null +++ b/lib/msal-common/src/network/RequestThumbprint.ts @@ -0,0 +1,14 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Type representing a unique request thumbprint. + */ +export type RequestThumbprint = { + clientId: string; + authority: string; + scopes: Array; + homeAccountIdentifier?: string; +}; diff --git a/lib/msal-common/src/network/RequestThumbprintValue.ts b/lib/msal-common/src/network/RequestThumbprintValue.ts new file mode 100644 index 00000000000..eceee5a12ec --- /dev/null +++ b/lib/msal-common/src/network/RequestThumbprintValue.ts @@ -0,0 +1,17 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Type representing the values associated with a RequestThumbprint. + */ +export type RequestThumbprintValue = { + // Unix-time value representing the expiration of the throttle + throttleTime: number; + // Information provided by the server + error?: string; + errorCodes?: Array; + errorMessage?: string; + subError?: string; +}; diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index f98cd153258..793175d0899 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -9,29 +9,8 @@ import { Constants, HeaderNames } from "../utils/Constants"; import { CacheManager } from "../cache/CacheManager"; import { StringUtils } from "../utils/StringUtils"; import { ServerError } from "../error/ServerError"; - -/** - * Type representing a unique request thumbprint. - */ -export type RequestThumbprint = { - clientId: string; - authority: string; - scopes: Array; - homeAccountIdentifier?: string; -} - -/** - * Type representing the values associated with a RequestThumbprint. - */ -export type RequestThumbprintValue = { - // Unix-time value representing the expiration of the throttle - throttleTime: number; - // Information provided by the server - error?: string; - errorCodes?: Array; - errorMessage?: string; - subError?: string; -} +import { RequestThumbprint } from "./RequestThumbprint"; +import { RequestThumbprintValue } from "./RequestThumbprintValue"; export class ThrottlingUtils { @@ -53,14 +32,14 @@ export class ThrottlingUtils { const storageValue = cacheManager.getItem(key) as string; if (storageValue) { - const parsedValue = StringUtils.jsonParseHelper(storageValue); + const parsedValue = StringUtils.jsonParseHelper(storageValue); if (parsedValue) { if (parsedValue.throttleTime < Date.now()) { cacheManager.removeItem(key); return; } - throw new ServerError(parsedValue.errorCodes, parsedValue.errorDescription, parsedValue.subError); + throw new ServerError(parsedValue.errorCodes.join(" "), parsedValue.errorMessage, parsedValue.subError); } } } @@ -100,7 +79,7 @@ export class ThrottlingUtils { * @param response */ static checkResponseForRetryAfter(response: NetworkResponse): boolean { - return response.headers.has(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300) + return response.headers.has(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300); } /** @@ -117,4 +96,4 @@ export class ThrottlingUtils { currentSeconds + Constants.DEFAULT_MAX_THROTTLE_TIME_SECONDS ) * 1000); } -} \ No newline at end of file +} diff --git a/lib/msal-common/src/utils/StringUtils.ts b/lib/msal-common/src/utils/StringUtils.ts index 408e811719a..51059d9e02e 100644 --- a/lib/msal-common/src/utils/StringUtils.ts +++ b/lib/msal-common/src/utils/StringUtils.ts @@ -91,9 +91,9 @@ export class StringUtils { * Attempts to parse a string into JSON * @param str */ - static jsonParseHelper(str: string) { + static jsonParseHelper(str: string): T { try { - return JSON.parse(str); + return JSON.parse(str) as T; } catch (e) { return null; } diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts index ceb2eb49c94..36a9f41b7eb 100644 --- a/lib/msal-common/test/network/NetworkManager.spec.ts +++ b/lib/msal-common/test/network/NetworkManager.spec.ts @@ -5,7 +5,8 @@ import { expect } from "chai"; import sinon from "sinon"; -import { ThrottlingUtils, RequestThumbprint } from "../../src/network/ThrottlingUtils"; +import { ThrottlingUtils } from "../../src/network/ThrottlingUtils"; +import { RequestThumbprint } from "../../src/network/RequestThumbprint"; import { NetworkManager, NetworkResponse } from "../../src/network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index 1e457d68806..86bc23ca20f 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -5,7 +5,9 @@ import { expect } from "chai"; import sinon from "sinon"; -import { ThrottlingUtils, RequestThumbprint, RequestThumbprintValue } from "../../src/network/ThrottlingUtils"; +import { ThrottlingUtils } from "../../src/network/ThrottlingUtils"; +import { RequestThumbprint } from "../../src/network/RequestThumbprint"; +import { RequestThumbprintValue } from "../../src/network/RequestThumbprintValue"; import { NetworkResponse } from "../../src/network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; diff --git a/lib/msal-common/test/utils/StringUtils.spec.ts b/lib/msal-common/test/utils/StringUtils.spec.ts index a5b41e88cf8..895587c682f 100644 --- a/lib/msal-common/test/utils/StringUtils.spec.ts +++ b/lib/msal-common/test/utils/StringUtils.spec.ts @@ -3,11 +3,7 @@ import { StringUtils } from "../../src/utils/StringUtils"; import { TEST_TOKENS } from "./StringConstants"; import { ClientAuthError, ClientAuthErrorMessage } from "../../src/error/ClientAuthError"; import { AuthError } from "../../src/error/AuthError"; -<<<<<<< HEAD import { IdToken } from "../../src"; -======= -import mocha from "mocha"; ->>>>>>> e78e3542... Add a few unit tests describe("StringUtils.ts Class Unit Tests", () => { From 2541aed66a2a347231aa1a6a5c8454a2f01d10a6 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 16 Jul 2020 14:37:40 -0700 Subject: [PATCH 20/34] Add removeThrottle --- lib/msal-common/src/index.ts | 2 +- .../src/network/ThrottlingUtils.ts | 12 ++++++ .../test/network/ThrottlingUtils.spec.ts | 37 ++++++++++++++++++- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index e7cceb05e43..eb23dc8d57d 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -26,7 +26,7 @@ export { IdTokenEntity } from "./cache/entities/IdTokenEntity"; export { AccessTokenEntity } from "./cache/entities/AccessTokenEntity"; export { RefreshTokenEntity } from "./cache/entities/RefreshTokenEntity"; // Network Interface -export { INetworkModule } from "./network/INetworkModule"; +export { INetworkModule, NetworkRequestOptions } from "./network/INetworkModule"; export { NetworkManager } from "./network/NetworkManager"; export { ThrottlingUtils } from "./network/ThrottlingUtils"; export { RequestThumbprint } from "./network/RequestThumbprint"; diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 793175d0899..5fece53b6bb 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -96,4 +96,16 @@ export class ThrottlingUtils { currentSeconds + Constants.DEFAULT_MAX_THROTTLE_TIME_SECONDS ) * 1000); } + + static removeThrottle(cacheManager: CacheManager, clientId: string, authority: string, scopes: Array, homeAccountIdentifier?: string): boolean { + const thumbprint: RequestThumbprint = { + clientId, + authority, + scopes, + homeAccountIdentifier + }; + + const key = this.generateThrottlingStorageKey(thumbprint); + return cacheManager.removeItem(key); + } } diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index 86bc23ca20f..491e80d86f8 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -3,8 +3,9 @@ * Licensed under the MIT License. */ -import { expect } from "chai"; +import { expect, assert } from "chai"; import sinon from "sinon"; +import mocha from "mocha"; import { ThrottlingUtils } from "../../src/network/ThrottlingUtils"; import { RequestThumbprint } from "../../src/network/RequestThumbprint"; import { RequestThumbprintValue } from "../../src/network/RequestThumbprintValue"; @@ -12,7 +13,7 @@ import { NetworkResponse } from "../../src/network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; import { ServerError } from "../../src"; -import { THUMBPRINT, THUMBPRINT_VALUE } from "../utils/StringConstants"; +import { THUMBPRINT, THUMBPRINT_VALUE, TEST_CONFIG } from "../utils/StringConstants"; describe("ThrottlingUtils", () => { describe("generateThrottlingStorageKey", () => { @@ -226,4 +227,36 @@ describe("ThrottlingUtils", () => { expect(time).to.be.deep.eq(3605000); }); }); + + describe("removeThrottle", () => { + after(() => { + sinon.restore(); + }); + + it("removes the entry from storage and returns true", () => { + const cache = new MockStorageClass(); + const removeItemStub = sinon.stub(cache, "removeItem").returns(true); + const clientId = TEST_CONFIG.MSAL_CLIENT_ID; + const authority = TEST_CONFIG.validAuthority; + const scopes = TEST_CONFIG.DEFAULT_SCOPES; + + const res = ThrottlingUtils.removeThrottle(cache, clientId, authority, scopes); + + sinon.assert.callCount(removeItemStub, 1); + expect(res).to.be.true; + }) + + it("doesn't find an entry and returns false", () => { + const cache = new MockStorageClass(); + const removeItemStub = sinon.stub(cache, "removeItem").returns(false); + const clientId = TEST_CONFIG.MSAL_CLIENT_ID; + const authority = TEST_CONFIG.validAuthority; + const scopes = TEST_CONFIG.DEFAULT_SCOPES; + + const res = ThrottlingUtils.removeThrottle(cache, clientId, authority, scopes); + + sinon.assert.callCount(removeItemStub, 1); + expect(res).to.be.false; + }); + }); }); \ No newline at end of file From 032336458a9a79f391fb879b85d753500d441b71 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 16 Jul 2020 15:48:49 -0700 Subject: [PATCH 21/34] Add changes to msal-browser --- lib/msal-browser/src/app/ClientApplication.ts | 7 +++++-- lib/msal-browser/src/cache/BrowserStorage.ts | 2 +- .../src/interaction_handler/RedirectHandler.ts | 5 ++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/msal-browser/src/app/ClientApplication.ts b/lib/msal-browser/src/app/ClientApplication.ts index 75933255166..64832e7f547 100644 --- a/lib/msal-browser/src/app/ClientApplication.ts +++ b/lib/msal-browser/src/app/ClientApplication.ts @@ -5,7 +5,7 @@ import { CryptoOps } from "../crypto/CryptoOps"; import { BrowserStorage } from "../cache/BrowserStorage"; -import { Authority, TrustedAuthority, StringUtils, CacheSchemaType, UrlString, ServerAuthorizationCodeResponse, AuthorizationCodeRequest, AuthorizationUrlRequest, AuthorizationCodeClient, PromptValue, SilentFlowRequest, ServerError, InteractionRequiredAuthError, EndSessionRequest, AccountInfo, AuthorityFactory, ServerTelemetryManager, SilentFlowClient, ClientConfiguration, BaseAuthRequest, ServerTelemetryRequest, PersistentCacheKeys, IdToken, ProtocolUtils, ResponseMode, Constants, INetworkModule, AuthenticationResult, Logger } from "@azure/msal-common"; +import { Authority, TrustedAuthority, StringUtils, CacheSchemaType, UrlString, ServerAuthorizationCodeResponse, AuthorizationCodeRequest, AuthorizationUrlRequest, AuthorizationCodeClient, PromptValue, SilentFlowRequest, ServerError, InteractionRequiredAuthError, EndSessionRequest, AccountInfo, AuthorityFactory, ServerTelemetryManager, SilentFlowClient, ClientConfiguration, BaseAuthRequest, ServerTelemetryRequest, PersistentCacheKeys, IdToken, ProtocolUtils, ResponseMode, Constants, INetworkModule, AuthenticationResult, Logger, ThrottlingUtils } from "@azure/msal-common"; import { buildConfiguration, Configuration } from "../config/Configuration"; import { TemporaryCacheKeys, InteractionType, ApiId, BrowserConstants, DEFAULT_REQUEST } from "../utils/BrowserConstants"; import { BrowserUtils } from "../utils/BrowserUtils"; @@ -194,7 +194,7 @@ export abstract class ClientApplication { const currentAuthority = this.browserStorage.getCachedAuthority(); const authClient = await this.createAuthCodeClient(serverTelemetryManager, currentAuthority); const interactionHandler = new RedirectHandler(authClient, this.browserStorage); - return await interactionHandler.handleCodeResponse(responseHash, this.browserCrypto); + return await interactionHandler.handleCodeResponse(responseHash, this.browserCrypto, this.config.auth.clientId); } catch (e) { serverTelemetryManager.cacheFailedRequest(e); this.browserStorage.cleanRequest(); @@ -290,6 +290,9 @@ export abstract class ClientApplication { // Monitor the window for the hash. Return the string value and close the popup when the hash is received. Default timeout is 60 seconds. const hash = await interactionHandler.monitorPopupForHash(popupWindow, this.config.system.windowHashTimeout); + // Remove throttle if it exists + ThrottlingUtils.removeThrottle(this.browserStorage, this.config.auth.clientId, authCodeRequest.authority, authCodeRequest.scopes); + // Handle response from hash string. return await interactionHandler.handleCodeResponse(hash); } catch (e) { diff --git a/lib/msal-browser/src/cache/BrowserStorage.ts b/lib/msal-browser/src/cache/BrowserStorage.ts index e0116fe4921..9044db537f6 100644 --- a/lib/msal-browser/src/cache/BrowserStorage.ts +++ b/lib/msal-browser/src/cache/BrowserStorage.ts @@ -23,7 +23,7 @@ export class BrowserStorage extends CacheManager { // Window storage object (either local or sessionStorage) private windowStorage: Storage; // Client id of application. Used in cache keys to partition cache correctly in the case of multiple instances of MSAL. - private clientId: string; + public clientId: string; constructor(clientId: string, cacheConfig: CacheOptions) { super(); diff --git a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts index 5cfa23e2d47..491c110b24c 100644 --- a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts +++ b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts @@ -2,7 +2,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ -import { StringUtils, AuthorizationCodeRequest, ICrypto, CacheSchemaType, AuthenticationResult } from "@azure/msal-common"; +import { StringUtils, AuthorizationCodeRequest, ICrypto, CacheSchemaType, AuthenticationResult, ThrottlingUtils } from "@azure/msal-common"; import { InteractionHandler } from "./InteractionHandler"; import { BrowserAuthError } from "../error/BrowserAuthError"; import { BrowserConstants, TemporaryCacheKeys } from "../utils/BrowserConstants"; @@ -65,6 +65,9 @@ export class RedirectHandler extends InteractionHandler { this.authCodeRequest = this.browserStorage.getCachedRequest(requestState, browserCrypto); this.authCodeRequest.code = authCode; + // Remove throttle if it exists + ThrottlingUtils.removeThrottle(this.browserStorage, this.browserStorage.clientId, this.authCodeRequest.authority, this.authCodeRequest.scopes); + // Acquire token with retrieved code. const tokenResponse = await this.authModule.acquireToken(this.authCodeRequest, cachedNonce, requestState); From 2b19b2fa67ee2f09cd5929a4adbffbd4c4820f00 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 16 Jul 2020 16:00:21 -0700 Subject: [PATCH 22/34] Fix export --- lib/msal-common/src/index.ts | 2 +- lib/msal-common/src/network/ThrottlingUtils.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index eb23dc8d57d..bffa67c6087 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -27,7 +27,7 @@ export { AccessTokenEntity } from "./cache/entities/AccessTokenEntity"; export { RefreshTokenEntity } from "./cache/entities/RefreshTokenEntity"; // Network Interface export { INetworkModule, NetworkRequestOptions } from "./network/INetworkModule"; -export { NetworkManager } from "./network/NetworkManager"; +export { NetworkManager, NetworkResponse } from "./network/NetworkManager"; export { ThrottlingUtils } from "./network/ThrottlingUtils"; export { RequestThumbprint } from "./network/RequestThumbprint"; export { RequestThumbprintValue } from "./network/RequestThumbprintValue"; diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 5fece53b6bb..f57b7828f86 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -79,7 +79,10 @@ export class ThrottlingUtils { * @param response */ static checkResponseForRetryAfter(response: NetworkResponse): boolean { - return response.headers.has(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300); + if (response.headers) { + return response.headers.has(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300); + } + return false; } /** From e92e45b5d67ed33eebe2f713b0175c8fa6d28954 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Fri, 17 Jul 2020 11:20:32 -0700 Subject: [PATCH 23/34] Move constants and add CacheSchemaType to calls --- lib/msal-browser/src/cache/BrowserStorage.ts | 9 ++++++++- .../src/interaction_handler/RedirectHandler.ts | 2 +- .../test/cache/BrowserStorage.spec.ts | 8 +++++++- lib/msal-common/src/network/ThrottlingUtils.ts | 17 +++++++++-------- lib/msal-common/src/utils/Constants.ts | 17 ++++++++++++----- 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/lib/msal-browser/src/cache/BrowserStorage.ts b/lib/msal-browser/src/cache/BrowserStorage.ts index 9044db537f6..6c9d8fac30a 100644 --- a/lib/msal-browser/src/cache/BrowserStorage.ts +++ b/lib/msal-browser/src/cache/BrowserStorage.ts @@ -23,7 +23,7 @@ export class BrowserStorage extends CacheManager { // Window storage object (either local or sessionStorage) private windowStorage: Storage; // Client id of application. Used in cache keys to partition cache correctly in the case of multiple instances of MSAL. - public clientId: string; + private clientId: string; constructor(clientId: string, cacheConfig: CacheOptions) { super(); @@ -427,4 +427,11 @@ export class BrowserStorage extends CacheManager { throw BrowserAuthError.createTokenRequestCacheError(err); } } + + /** + * Gets the clientId + */ + getClientId(): string { + return this.clientId; + } } diff --git a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts index 491c110b24c..44a8e291914 100644 --- a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts +++ b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts @@ -66,7 +66,7 @@ export class RedirectHandler extends InteractionHandler { this.authCodeRequest.code = authCode; // Remove throttle if it exists - ThrottlingUtils.removeThrottle(this.browserStorage, this.browserStorage.clientId, this.authCodeRequest.authority, this.authCodeRequest.scopes); + ThrottlingUtils.removeThrottle(this.browserStorage, this.browserStorage.getClientId(), this.authCodeRequest.authority, this.authCodeRequest.scopes); // Acquire token with retrieved code. const tokenResponse = await this.authModule.acquireToken(this.authCodeRequest, cachedNonce, requestState); diff --git a/lib/msal-browser/test/cache/BrowserStorage.spec.ts b/lib/msal-browser/test/cache/BrowserStorage.spec.ts index 5c42bea57fa..ea74dcf575e 100644 --- a/lib/msal-browser/test/cache/BrowserStorage.spec.ts +++ b/lib/msal-browser/test/cache/BrowserStorage.spec.ts @@ -495,6 +495,12 @@ describe("BrowserStorage() tests", () => { // Perform test const tokenRequest = browserStorage.getCachedRequest(RANDOM_TEST_GUID, browserCrypto); expect(tokenRequest.authority).to.be.eq(alternateAuthority); - }); + }); + + it("getClientId returns a clientId", () => { + const browserStorage = new BrowserStorage(TEST_CONFIG.MSAL_CLIENT_ID, cacheConfig); + const clientId = browserStorage.getClientId(); + expect(clientId).to.deep.eq(TEST_CONFIG.MSAL_CLIENT_ID); + }); }); }); diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index f57b7828f86..d05987e593a 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -5,7 +5,7 @@ import { NetworkResponse } from "./NetworkManager"; import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; -import { Constants, HeaderNames } from "../utils/Constants"; +import { HeaderNames, CacheSchemaType, ThrottleConstants } from "../utils/Constants"; import { CacheManager } from "../cache/CacheManager"; import { StringUtils } from "../utils/StringUtils"; import { ServerError } from "../error/ServerError"; @@ -19,7 +19,7 @@ export class ThrottlingUtils { * @param thumbprint */ static generateThrottlingStorageKey(thumbprint: RequestThumbprint): string { - return `${Constants.THROTTLE_PREFIX}.${JSON.stringify(thumbprint)}`; + return `${ThrottleConstants.THROTTLE_PREFIX}.${JSON.stringify(thumbprint)}`; } /** @@ -29,14 +29,14 @@ export class ThrottlingUtils { */ static preProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint): void { const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); - const storageValue = cacheManager.getItem(key) as string; + const storageValue = cacheManager.getItem(key, CacheSchemaType.TEMPORARY) as string; if (storageValue) { const parsedValue = StringUtils.jsonParseHelper(storageValue); if (parsedValue) { if (parsedValue.throttleTime < Date.now()) { - cacheManager.removeItem(key); + cacheManager.removeItem(key, CacheSchemaType.TEMPORARY); return; } throw new ServerError(parsedValue.errorCodes.join(" "), parsedValue.errorMessage, parsedValue.subError); @@ -61,7 +61,8 @@ export class ThrottlingUtils { }; cacheManager.setItem( ThrottlingUtils.generateThrottlingStorageKey(thumbprint), - JSON.stringify(thumbprintValue) + JSON.stringify(thumbprintValue), + CacheSchemaType.TEMPORARY ); } } @@ -95,8 +96,8 @@ export class ThrottlingUtils { } const currentSeconds = Date.now() / 1000; return Math.floor(Math.min( - currentSeconds + (throttleTime || Constants.DEFAULT_THROTTLE_TIME_SECONDS), - currentSeconds + Constants.DEFAULT_MAX_THROTTLE_TIME_SECONDS + currentSeconds + (throttleTime || ThrottleConstants.DEFAULT_THROTTLE_TIME_SECONDS), + currentSeconds + ThrottleConstants.DEFAULT_MAX_THROTTLE_TIME_SECONDS ) * 1000); } @@ -109,6 +110,6 @@ export class ThrottlingUtils { }; const key = this.generateThrottlingStorageKey(thumbprint); - return cacheManager.removeItem(key); + return cacheManager.removeItem(key, CacheSchemaType.TEMPORARY); } } diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index 5aafd2d8743..5b23d71d216 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -7,8 +7,6 @@ export const Constants = { SKU: "msal.js.common", // Prefix for all library cache entries CACHE_PREFIX: "msal", - // Prefix for storing throttling entries - THROTTLE_PREFIX: "throttle", // default authority DEFAULT_AUTHORITY: "https://login.microsoftonline.com/common/", DEFAULT_AUTHORITY_HOST: "login.microsoftonline.com", @@ -37,9 +35,6 @@ export const Constants = { URL_FORM_CONTENT_TYPE: "application/x-www-form-urlencoded;charset=utf-8", AUTHORIZATION_PENDING: "authorization_pending", NOT_DEFINED: "not_defined", - // Default time to throttle RequestThumbprint in milliseconds - DEFAULT_THROTTLE_TIME_SECONDS: 60, - DEFAULT_MAX_THROTTLE_TIME_SECONDS: 3600 }; /** @@ -279,3 +274,15 @@ export const SERVER_TELEM_CONSTANTS = { CATEGORY_SEPARATOR: "|", VALUE_SEPARATOR: "," }; + +/** + * Constants related to throttling + */ +export const ThrottleConstants = { + // Default time to throttle RequestThumbprint in milliseconds + DEFAULT_THROTTLE_TIME_SECONDS: 60, + // Default maximum time to throttle in milliseconds, overrides what the server sends back + DEFAULT_MAX_THROTTLE_TIME_SECONDS: 3600, + // Prefix for storing throttling entries + THROTTLE_PREFIX: "throttle" +} From 1a769e17e5396da1465f45489e5d27f58f3dc29e Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Mon, 20 Jul 2020 12:13:26 -0700 Subject: [PATCH 24/34] Fix comments --- lib/msal-common/src/utils/Constants.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index 5b23d71d216..7f750ef0cad 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -279,10 +279,10 @@ export const SERVER_TELEM_CONSTANTS = { * Constants related to throttling */ export const ThrottleConstants = { - // Default time to throttle RequestThumbprint in milliseconds + // Default time to throttle RequestThumbprint in seconds DEFAULT_THROTTLE_TIME_SECONDS: 60, - // Default maximum time to throttle in milliseconds, overrides what the server sends back + // Default maximum time to throttle in seconds, overrides what the server sends back DEFAULT_MAX_THROTTLE_TIME_SECONDS: 3600, // Prefix for storing throttling entries THROTTLE_PREFIX: "throttle" -} +}; From b9ef0ea17927f13346dcaf348e3e13dd53a9b81a Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 21 Jul 2020 13:14:26 -0700 Subject: [PATCH 25/34] Remove headers and pass in clientId to InteractionHandler.handleCodeResponse --- lib/msal-browser/src/cache/BrowserStorage.ts | 7 ------- .../src/interaction_handler/RedirectHandler.ts | 8 +++++--- lib/msal-browser/test/cache/BrowserStorage.spec.ts | 6 ------ 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/lib/msal-browser/src/cache/BrowserStorage.ts b/lib/msal-browser/src/cache/BrowserStorage.ts index 6c9d8fac30a..e0116fe4921 100644 --- a/lib/msal-browser/src/cache/BrowserStorage.ts +++ b/lib/msal-browser/src/cache/BrowserStorage.ts @@ -427,11 +427,4 @@ export class BrowserStorage extends CacheManager { throw BrowserAuthError.createTokenRequestCacheError(err); } } - - /** - * Gets the clientId - */ - getClientId(): string { - return this.clientId; - } } diff --git a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts index 44a8e291914..8fcaffdf41d 100644 --- a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts +++ b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts @@ -46,7 +46,7 @@ export class RedirectHandler extends InteractionHandler { * Handle authorization code response in the window. * @param hash */ - async handleCodeResponse(locationHash: string, browserCrypto?: ICrypto): Promise { + async handleCodeResponse(locationHash: string, browserCrypto?: ICrypto, clientId?: string): Promise { // Check that location hash isn't empty. if (StringUtils.isEmpty(locationHash)) { throw BrowserAuthError.createEmptyHashError(locationHash); @@ -66,8 +66,10 @@ export class RedirectHandler extends InteractionHandler { this.authCodeRequest.code = authCode; // Remove throttle if it exists - ThrottlingUtils.removeThrottle(this.browserStorage, this.browserStorage.getClientId(), this.authCodeRequest.authority, this.authCodeRequest.scopes); - + if (clientId) { + ThrottlingUtils.removeThrottle(this.browserStorage, clientId, this.authCodeRequest.authority, this.authCodeRequest.scopes); + } + // Acquire token with retrieved code. const tokenResponse = await this.authModule.acquireToken(this.authCodeRequest, cachedNonce, requestState); diff --git a/lib/msal-browser/test/cache/BrowserStorage.spec.ts b/lib/msal-browser/test/cache/BrowserStorage.spec.ts index ea74dcf575e..aecabdfd578 100644 --- a/lib/msal-browser/test/cache/BrowserStorage.spec.ts +++ b/lib/msal-browser/test/cache/BrowserStorage.spec.ts @@ -496,11 +496,5 @@ describe("BrowserStorage() tests", () => { const tokenRequest = browserStorage.getCachedRequest(RANDOM_TEST_GUID, browserCrypto); expect(tokenRequest.authority).to.be.eq(alternateAuthority); }); - - it("getClientId returns a clientId", () => { - const browserStorage = new BrowserStorage(TEST_CONFIG.MSAL_CLIENT_ID, cacheConfig); - const clientId = browserStorage.getClientId(); - expect(clientId).to.deep.eq(TEST_CONFIG.MSAL_CLIENT_ID); - }); }); }); From e6c4be9dbe3a769b526223d97995baa63c053fd7 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 30 Jul 2020 13:26:41 -0700 Subject: [PATCH 26/34] Add CacheSchemaType.THROTTLE --- lib/msal-browser/src/cache/BrowserStorage.ts | 8 ++++++-- lib/msal-common/src/network/ThrottlingUtils.ts | 8 ++++---- lib/msal-common/src/utils/Constants.ts | 7 ++++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/msal-browser/src/cache/BrowserStorage.ts b/lib/msal-browser/src/cache/BrowserStorage.ts index e0116fe4921..adad009ecff 100644 --- a/lib/msal-browser/src/cache/BrowserStorage.ts +++ b/lib/msal-browser/src/cache/BrowserStorage.ts @@ -2,7 +2,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ -import { Constants, PersistentCacheKeys, StringUtils, AuthorizationCodeRequest, ICrypto, CacheSchemaType, AccountEntity, IdTokenEntity, CredentialType, AccessTokenEntity, RefreshTokenEntity, AppMetadataEntity, CacheManager, CredentialEntity, ServerTelemetryCacheValue } from "@azure/msal-common"; +import { Constants, PersistentCacheKeys, StringUtils, AuthorizationCodeRequest, ICrypto, CacheSchemaType, AccountEntity, IdTokenEntity, CredentialType, AccessTokenEntity, RefreshTokenEntity, AppMetadataEntity, CacheManager, CredentialEntity, ServerTelemetryCacheValue, RequestThumbprintValue } from "@azure/msal-common"; import { CacheOptions } from "../config/Configuration"; import { BrowserAuthError } from "../error/BrowserAuthError"; import { BrowserConfigurationAuthError } from "../error/BrowserConfigurationAuthError"; @@ -113,7 +113,8 @@ export class BrowserStorage extends CacheManager { switch (type) { case CacheSchemaType.ACCOUNT: case CacheSchemaType.CREDENTIAL: - case CacheSchemaType.APP_METADATA: + case CacheSchemaType.APP_META_DATA: + case CacheSchemaType.THROTTLE: this.windowStorage.setItem(key, JSON.stringify(value)); break; case CacheSchemaType.TEMPORARY: { @@ -169,6 +170,9 @@ export class BrowserStorage extends CacheManager { case CacheSchemaType.APP_METADATA: { return (JSON.parse(value) as AppMetadataEntity); } + case CacheSchemaType.THROTTLE: { + return value; + } case CacheSchemaType.TEMPORARY: { const itemCookie = this.getItemCookie(key); if (this.cacheConfig.storeAuthStateInCookie) { diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index d05987e593a..8422cd4d02c 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -29,14 +29,14 @@ export class ThrottlingUtils { */ static preProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint): void { const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); - const storageValue = cacheManager.getItem(key, CacheSchemaType.TEMPORARY) as string; + const storageValue = cacheManager.getItem(key, CacheSchemaType.THROTTLE) as string; if (storageValue) { const parsedValue = StringUtils.jsonParseHelper(storageValue); if (parsedValue) { if (parsedValue.throttleTime < Date.now()) { - cacheManager.removeItem(key, CacheSchemaType.TEMPORARY); + cacheManager.removeItem(key, CacheSchemaType.THROTTLE); return; } throw new ServerError(parsedValue.errorCodes.join(" "), parsedValue.errorMessage, parsedValue.subError); @@ -62,7 +62,7 @@ export class ThrottlingUtils { cacheManager.setItem( ThrottlingUtils.generateThrottlingStorageKey(thumbprint), JSON.stringify(thumbprintValue), - CacheSchemaType.TEMPORARY + CacheSchemaType.THROTTLE ); } } @@ -110,6 +110,6 @@ export class ThrottlingUtils { }; const key = this.generateThrottlingStorageKey(thumbprint); - return cacheManager.removeItem(key, CacheSchemaType.TEMPORARY); + return cacheManager.removeItem(key, CacheSchemaType.THROTTLE); } } diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index 7f750ef0cad..3c3835fbf01 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -223,7 +223,7 @@ export enum Separators { } /** - * Credentail Type stored in the cache + * Credential Type stored in the cache */ export enum CredentialType { ID_TOKEN = "IdToken", @@ -232,7 +232,7 @@ export enum CredentialType { } /** - * Credentail Type stored in the cache + * Credential Type stored in the cache */ export enum CacheSchemaType { ACCOUNT = "Account", @@ -243,7 +243,8 @@ export enum CacheSchemaType { APP_METADATA = "AppMetadata", TEMPORARY = "TempCache", TELEMETRY = "Telemetry", - UNDEFINED = "Undefined" + UNDEFINED = "Undefined", + THROTTLE = "Throttle" } /** From 06bb893cd264535eb74472ca5829a180f454b5b9 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 30 Jul 2020 13:33:23 -0700 Subject: [PATCH 27/34] Remove extra stringify --- lib/msal-common/src/network/ThrottlingUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 8422cd4d02c..7a49e53c0bb 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -61,7 +61,7 @@ export class ThrottlingUtils { }; cacheManager.setItem( ThrottlingUtils.generateThrottlingStorageKey(thumbprint), - JSON.stringify(thumbprintValue), + thumbprintValue, CacheSchemaType.THROTTLE ); } From b5b72ff919016f4805a69a30c67f1c2f61150efb Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 30 Jul 2020 14:03:15 -0700 Subject: [PATCH 28/34] Fix tests --- lib/msal-browser/src/cache/BrowserStorage.ts | 2 +- lib/msal-common/src/network/ThrottlingUtils.ts | 17 +++++++---------- .../test/network/NetworkManager.spec.ts | 4 ++-- .../test/network/ThrottlingUtils.spec.ts | 6 +++--- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/lib/msal-browser/src/cache/BrowserStorage.ts b/lib/msal-browser/src/cache/BrowserStorage.ts index adad009ecff..dd274a72056 100644 --- a/lib/msal-browser/src/cache/BrowserStorage.ts +++ b/lib/msal-browser/src/cache/BrowserStorage.ts @@ -171,7 +171,7 @@ export class BrowserStorage extends CacheManager { return (JSON.parse(value) as AppMetadataEntity); } case CacheSchemaType.THROTTLE: { - return value; + return (JSON.parse(value) as RequestThumbprintValue); } case CacheSchemaType.TEMPORARY: { const itemCookie = this.getItemCookie(key); diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index 7a49e53c0bb..a28fcd28a96 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -29,18 +29,15 @@ export class ThrottlingUtils { */ static preProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint): void { const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); - const storageValue = cacheManager.getItem(key, CacheSchemaType.THROTTLE) as string; + const value = cacheManager.getItem(key, CacheSchemaType.THROTTLE) as RequestThumbprintValue; + // console.log("VALUE:", value); - if (storageValue) { - const parsedValue = StringUtils.jsonParseHelper(storageValue); - - if (parsedValue) { - if (parsedValue.throttleTime < Date.now()) { - cacheManager.removeItem(key, CacheSchemaType.THROTTLE); - return; - } - throw new ServerError(parsedValue.errorCodes.join(" "), parsedValue.errorMessage, parsedValue.subError); + if (value) { + if (value.throttleTime < Date.now()) { + cacheManager.removeItem(key, CacheSchemaType.THROTTLE); + return; } + throw new ServerError(value.errorCodes.join(" "), value.errorMessage, value.subError); } } diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts index 36a9f41b7eb..c27cad5e3b5 100644 --- a/lib/msal-common/test/network/NetworkManager.spec.ts +++ b/lib/msal-common/test/network/NetworkManager.spec.ts @@ -54,7 +54,7 @@ describe("NetworkManager", () => { const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; const mockThumbprintValue = THUMBPRINT_VALUE; const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync"); - const getItemStub = sinon.stub(cache, "getItem").returns(JSON.stringify(mockThumbprintValue)); + const getItemStub = sinon.stub(cache, "getItem").returns(mockThumbprintValue); const setItemStub = sinon.stub(cache, "setItem"); const removeItemStub = sinon.stub(cache, "removeItem"); sinon.stub(Date, "now").callsFake(() => 1) @@ -83,7 +83,7 @@ describe("NetworkManager", () => { } const mockThumbprintValue = THUMBPRINT_VALUE; const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync").returns(Promise.resolve(mockRes)); - const getItemStub = sinon.stub(cache, "getItem").returns(JSON.stringify(mockThumbprintValue)); + const getItemStub = sinon.stub(cache, "getItem").returns(mockThumbprintValue); const setItemStub = sinon.stub(cache, "setItem"); const removeItemStub = sinon.stub(cache, "removeItem"); sinon.stub(Date, "now").callsFake(() => 10) diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index 491e80d86f8..814180e2d72 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -36,7 +36,7 @@ describe("ThrottlingUtils", () => { const thumbprintValue: RequestThumbprintValue = THUMBPRINT_VALUE; const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); - sinon.stub(cache, "getItem").callsFake(() => JSON.stringify(thumbprintValue)); + sinon.stub(cache, "getItem").callsFake(() => thumbprintValue); sinon.stub(Date, "now").callsFake(() => 1) try { @@ -52,7 +52,7 @@ describe("ThrottlingUtils", () => { const thumbprintValue: RequestThumbprintValue = THUMBPRINT_VALUE; const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); - sinon.stub(cache, "getItem").callsFake(() => JSON.stringify(thumbprintValue)); + sinon.stub(cache, "getItem").callsFake(() => thumbprintValue); sinon.stub(Date, "now").callsFake(() => 10) ThrottlingUtils.preProcess(cache, thumbprint); @@ -65,7 +65,7 @@ describe("ThrottlingUtils", () => { const thumbprint: RequestThumbprint = THUMBPRINT; const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); - sinon.stub(cache, "getItem").callsFake(() => ""); + sinon.stub(cache, "getItem").callsFake(() => null); ThrottlingUtils.preProcess(cache, thumbprint); sinon.assert.callCount(removeItemStub, 0) From 4c3b1061d5848be866ba8abb367d766ca529edb0 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 11 Aug 2020 12:02:55 -0700 Subject: [PATCH 29/34] Resolve rebase MCs --- lib/msal-common/src/client/BaseClient.ts | 19 ++++--------------- .../src/network/ThrottlingUtils.ts | 3 +-- .../test/network/NetworkManager.spec.ts | 2 +- .../test/network/ThrottlingUtils.spec.ts | 5 ++--- 4 files changed, 8 insertions(+), 21 deletions(-) diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 04de41ac8fd..fa8b0c8f5db 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -9,17 +9,12 @@ import { NetworkManager, NetworkResponse } from "../network/NetworkManager"; import { ICrypto } from "../crypto/ICrypto"; import { Authority } from "../authority/Authority"; import { Logger } from "../logger/Logger"; -import { AADServerParamKeys, Constants, HeaderNames, HeaderValues } from "../utils/Constants"; -import { NetworkResponse } from "../network/NetworkManager"; -import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizationTokenResponse"; import { AADServerParamKeys, Constants, HeaderNames } from "../utils/Constants"; -import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; +import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizationTokenResponse"; import { TrustedAuthority } from "../authority/TrustedAuthority"; import { CacheManager } from "../cache/CacheManager"; import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManager"; import { RequestThumbprint } from "../network/RequestThumbprint"; -import { RequestThumbprint } from '../network/ThrottlingUtils'; -import { RequestThumbprint } from "../network/RequestThumbprint"; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. @@ -65,19 +60,13 @@ export abstract class BaseClient { // Set the network interface this.networkClient = this.config.networkInterface; + // Set the NetworkManager + this.networkManager = new NetworkManager(this.networkClient, this.cacheManager); + // Set TelemetryManager this.serverTelemetryManager = this.config.serverTelemetryManager; - this.networkManager = new NetworkManager(this.networkClient, this.cacheManager); TrustedAuthority.setTrustedAuthoritiesFromConfig(this.config.authOptions.knownAuthorities, this.config.authOptions.cloudDiscoveryMetadata); - // Set the NetworkManager - this.networkManager = new NetworkManager( - this.config.cryptoInterface, - this.config.storageInterface, - this.config.networkInterface - ); - - B2cAuthority.setKnownAuthorities(this.config.authOptions.knownAuthorities); this.authority = this.config.authOptions.authority; } diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index a28fcd28a96..af85ce9a619 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -4,10 +4,9 @@ */ import { NetworkResponse } from "./NetworkManager"; -import { ServerAuthorizationTokenResponse } from "../server/ServerAuthorizationTokenResponse"; +import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizationTokenResponse"; import { HeaderNames, CacheSchemaType, ThrottleConstants } from "../utils/Constants"; import { CacheManager } from "../cache/CacheManager"; -import { StringUtils } from "../utils/StringUtils"; import { ServerError } from "../error/ServerError"; import { RequestThumbprint } from "./RequestThumbprint"; import { RequestThumbprintValue } from "./RequestThumbprintValue"; diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts index c27cad5e3b5..b6b4a0d714e 100644 --- a/lib/msal-common/test/network/NetworkManager.spec.ts +++ b/lib/msal-common/test/network/NetworkManager.spec.ts @@ -8,7 +8,7 @@ import sinon from "sinon"; import { ThrottlingUtils } from "../../src/network/ThrottlingUtils"; import { RequestThumbprint } from "../../src/network/RequestThumbprint"; import { NetworkManager, NetworkResponse } from "../../src/network/NetworkManager"; -import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; +import { ServerAuthorizationTokenResponse } from "../../src/response/ServerAuthorizationTokenResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; import { NetworkRequestOptions } from "../../src/network/INetworkModule"; import { ServerError } from "../../src/error/ServerError"; diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index 814180e2d72..ee68e8545e4 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -3,14 +3,13 @@ * Licensed under the MIT License. */ -import { expect, assert } from "chai"; +import { expect } from "chai"; import sinon from "sinon"; -import mocha from "mocha"; import { ThrottlingUtils } from "../../src/network/ThrottlingUtils"; import { RequestThumbprint } from "../../src/network/RequestThumbprint"; import { RequestThumbprintValue } from "../../src/network/RequestThumbprintValue"; import { NetworkResponse } from "../../src/network/NetworkManager"; -import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse"; +import { ServerAuthorizationTokenResponse } from "../../src/response/ServerAuthorizationTokenResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; import { ServerError } from "../../src"; import { THUMBPRINT, THUMBPRINT_VALUE, TEST_CONFIG } from "../utils/StringConstants"; From 91a1dba29dbfb8f6fc329c0381767d6b6f661281 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 11 Aug 2020 12:05:23 -0700 Subject: [PATCH 30/34] Change files --- ...-msal-browser-2020-08-11-12-05-23-msal-throttling.json | 8 ++++++++ ...e-msal-common-2020-08-11-12-05-23-msal-throttling.json | 8 ++++++++ 2 files changed, 16 insertions(+) create mode 100644 change/@azure-msal-browser-2020-08-11-12-05-23-msal-throttling.json create mode 100644 change/@azure-msal-common-2020-08-11-12-05-23-msal-throttling.json diff --git a/change/@azure-msal-browser-2020-08-11-12-05-23-msal-throttling.json b/change/@azure-msal-browser-2020-08-11-12-05-23-msal-throttling.json new file mode 100644 index 00000000000..894a096d93d --- /dev/null +++ b/change/@azure-msal-browser-2020-08-11-12-05-23-msal-throttling.json @@ -0,0 +1,8 @@ +{ + "type": "minor", + "comment": "Added client-side throttling to enhance server stability (#1907)", + "packageName": "@azure/msal-browser", + "email": "jamckenn@microsoft.com", + "dependentChangeType": "patch", + "date": "2020-08-11T19:04:58.604Z" +} diff --git a/change/@azure-msal-common-2020-08-11-12-05-23-msal-throttling.json b/change/@azure-msal-common-2020-08-11-12-05-23-msal-throttling.json new file mode 100644 index 00000000000..4ee85349ec5 --- /dev/null +++ b/change/@azure-msal-common-2020-08-11-12-05-23-msal-throttling.json @@ -0,0 +1,8 @@ +{ + "type": "minor", + "comment": "Added client-side throttling to enhance server stability (#1907)", + "packageName": "@azure/msal-common", + "email": "jamckenn@microsoft.com", + "dependentChangeType": "patch", + "date": "2020-08-11T19:05:23.196Z" +} From 3179acf0d010051cd5d5dabc6948d516e158e465 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 11 Aug 2020 12:54:57 -0700 Subject: [PATCH 31/34] Remove comma and add EOL to test files --- lib/msal-common/src/utils/Constants.ts | 2 +- lib/msal-common/test/network/NetworkManager.spec.ts | 2 +- lib/msal-common/test/network/ThrottlingUtils.spec.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index 3c3835fbf01..caae68bd311 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -34,7 +34,7 @@ export const Constants = { S256_CODE_CHALLENGE_METHOD: "S256", URL_FORM_CONTENT_TYPE: "application/x-www-form-urlencoded;charset=utf-8", AUTHORIZATION_PENDING: "authorization_pending", - NOT_DEFINED: "not_defined", + NOT_DEFINED: "not_defined" }; /** diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts index b6b4a0d714e..a6ad7dd8ee4 100644 --- a/lib/msal-common/test/network/NetworkManager.spec.ts +++ b/lib/msal-common/test/network/NetworkManager.spec.ts @@ -123,4 +123,4 @@ describe("NetworkManager", () => { expect(res).to.deep.eq(mockRes); }); }); -}); \ No newline at end of file +}); diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index ee68e8545e4..abab3e37e03 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -258,4 +258,4 @@ describe("ThrottlingUtils", () => { expect(res).to.be.false; }); }); -}); \ No newline at end of file +}); From 87e1ed2b4eed44114de3ae0c865d27dac7161bfd Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Mon, 31 Aug 2020 13:47:44 -0700 Subject: [PATCH 32/34] Fix MCs --- lib/msal-common/src/client/BaseClient.ts | 5 ----- lib/msal-common/src/client/RefreshTokenClient.ts | 6 +----- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index fa8b0c8f5db..3a79d85d570 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -76,13 +76,8 @@ export abstract class BaseClient { */ protected createDefaultTokenRequestHeaders(): Record { const headers = this.createDefaultLibraryHeaders(); -<<<<<<< HEAD headers[HeaderNames.CONTENT_TYPE] = Constants.URL_FORM_CONTENT_TYPE; headers[HeaderNames.X_MS_LIB_CAPABILITY] = HeaderNames.X_MS_LIB_CAPABILITY_VALUE; -======= - headers.set(HeaderNames.CONTENT_TYPE, Constants.URL_FORM_CONTENT_TYPE); - headers.set(HeaderNames.X_MS_LIB_CAPABILITY, HeaderNames.X_MS_LIB_CAPABILITY_VALUE); ->>>>>>> b12c9108... Fix JSON.parse and add ServerError if (this.serverTelemetryManager) { headers[HeaderNames.X_CLIENT_CURR_TELEM] = this.serverTelemetryManager.generateCurrentRequestHeaderValue(); diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index d7a1da55c4a..ec87281fe82 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -14,12 +14,8 @@ import { GrantType } from "../utils/Constants"; import { ResponseHandler } from "../response/ResponseHandler"; import { AuthenticationResult } from "../response/AuthenticationResult"; import { StringUtils } from "../utils/StringUtils"; -import { NetworkManager } from '../network/NetworkManager'; -import { NetworkResponse } from "../network/ThrottlingManager"; import { RequestThumbprint } from "../network/RequestThumbprint" -import { RequestThumbprintValue } from "../network/RequestThumbprintValue"; import { NetworkResponse } from "../network/NetworkManager"; -import { RequestThumbprint} from "../network/RequestThumbprint"; /** @@ -62,7 +58,7 @@ export class RefreshTokenClient extends BaseClient { const requestBody = this.createTokenRequestBody(request); const headers: Record = this.createDefaultTokenRequestHeaders(); - return this.networkManager.sendPostRequest(authority.tokenEndpoint, requestBody, headers, thumbprint); + return this.executePostToTokenEndpoint(authority.tokenEndpoint, requestBody, headers, thumbprint); } private createTokenRequestBody(request: RefreshTokenRequest): string { From dd09f365c682d9a562d4b7bf639e8d147201b1be Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Tue, 1 Sep 2020 10:37:46 -0700 Subject: [PATCH 33/34] Add ThrottlingEntity, delete RequestThumbprintValue --- lib/msal-browser/src/cache/BrowserStorage.ts | 10 ++--- .../src/cache/entities/ThrottlingEntity.ts | 35 +++++++++++++++ .../src/client/ClientCredentialClient.ts | 8 +++- .../src/client/DeviceCodeClient.ts | 8 ++-- .../src/client/RefreshTokenClient.ts | 9 ++-- lib/msal-common/src/index.ts | 2 +- .../src/network/RequestThumbprintValue.ts | 17 ------- .../src/network/ThrottlingUtils.ts | 25 +++++------ lib/msal-common/src/utils/Constants.ts | 6 +-- .../cache/entities/ThrottlingEntity.spec.ts | 45 +++++++++++++++++++ .../test/network/NetworkManager.spec.ts | 16 +++---- .../test/network/ThrottlingUtils.spec.ts | 32 ++++++------- lib/msal-common/test/utils/StringConstants.ts | 6 +-- 13 files changed, 141 insertions(+), 78 deletions(-) create mode 100644 lib/msal-common/src/cache/entities/ThrottlingEntity.ts delete mode 100644 lib/msal-common/src/network/RequestThumbprintValue.ts create mode 100644 lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts diff --git a/lib/msal-browser/src/cache/BrowserStorage.ts b/lib/msal-browser/src/cache/BrowserStorage.ts index dd274a72056..5a4db8ebca9 100644 --- a/lib/msal-browser/src/cache/BrowserStorage.ts +++ b/lib/msal-browser/src/cache/BrowserStorage.ts @@ -2,7 +2,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ -import { Constants, PersistentCacheKeys, StringUtils, AuthorizationCodeRequest, ICrypto, CacheSchemaType, AccountEntity, IdTokenEntity, CredentialType, AccessTokenEntity, RefreshTokenEntity, AppMetadataEntity, CacheManager, CredentialEntity, ServerTelemetryCacheValue, RequestThumbprintValue } from "@azure/msal-common"; +import { Constants, PersistentCacheKeys, StringUtils, AuthorizationCodeRequest, ICrypto, CacheSchemaType, AccountEntity, IdTokenEntity, CredentialType, AccessTokenEntity, RefreshTokenEntity, AppMetadataEntity, CacheManager, CredentialEntity, ServerTelemetryCacheValue, ThrottlingEntity } from "@azure/msal-common"; import { CacheOptions } from "../config/Configuration"; import { BrowserAuthError } from "../error/BrowserAuthError"; import { BrowserConfigurationAuthError } from "../error/BrowserConfigurationAuthError"; @@ -113,8 +113,8 @@ export class BrowserStorage extends CacheManager { switch (type) { case CacheSchemaType.ACCOUNT: case CacheSchemaType.CREDENTIAL: - case CacheSchemaType.APP_META_DATA: - case CacheSchemaType.THROTTLE: + case CacheSchemaType.APP_METADATA: + case CacheSchemaType.THROTTLING: this.windowStorage.setItem(key, JSON.stringify(value)); break; case CacheSchemaType.TEMPORARY: { @@ -170,8 +170,8 @@ export class BrowserStorage extends CacheManager { case CacheSchemaType.APP_METADATA: { return (JSON.parse(value) as AppMetadataEntity); } - case CacheSchemaType.THROTTLE: { - return (JSON.parse(value) as RequestThumbprintValue); + case CacheSchemaType.THROTTLING: { + return (JSON.parse(value) as ThrottlingEntity); } case CacheSchemaType.TEMPORARY: { const itemCookie = this.getItemCookie(key); diff --git a/lib/msal-common/src/cache/entities/ThrottlingEntity.ts b/lib/msal-common/src/cache/entities/ThrottlingEntity.ts new file mode 100644 index 00000000000..f5f9d741f7e --- /dev/null +++ b/lib/msal-common/src/cache/entities/ThrottlingEntity.ts @@ -0,0 +1,35 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ +import { ThrottlingConstants } from "../../utils/Constants"; + +export class ThrottlingEntity { + // Unix-time value representing the expiration of the throttle + throttleTime: number; + // Information provided by the server + error?: string; + errorCodes?: Array; + errorMessage?: string; + subError?: string; + + /** + * validates if a given cache entry is "Throttling", parses + * @param key + * @param entity + */ + static isThrottlingEntity(key: string, entity?: object): boolean { + + let validateKey: boolean = false; + if (key) { + validateKey = key.indexOf(ThrottlingConstants.THROTTLING_PREFIX) === 0; + } + + let validateEntity: boolean = true; + if (entity) { + validateEntity = entity.hasOwnProperty("throttleTime"); + } + + return validateKey && validateEntity; + } +} diff --git a/lib/msal-common/src/client/ClientCredentialClient.ts b/lib/msal-common/src/client/ClientCredentialClient.ts index 60b2607de30..fed669f126b 100644 --- a/lib/msal-common/src/client/ClientCredentialClient.ts +++ b/lib/msal-common/src/client/ClientCredentialClient.ts @@ -17,6 +17,7 @@ import { CredentialType } from "../utils/Constants"; import { AccessTokenEntity } from "../cache/entities/AccessTokenEntity"; import { TimeUtils } from "../utils/TimeUtils"; import { StringUtils } from "../utils/StringUtils"; +import { RequestThumbprint } from "../network/RequestThumbprint"; /** * OAuth2.0 client credential grant @@ -81,8 +82,13 @@ export class ClientCredentialClient extends BaseClient { const requestBody = this.createTokenRequestBody(request); const headers: Record = this.createDefaultTokenRequestHeaders(); + const thumbprint: RequestThumbprint = { + clientId: this.config.authOptions.clientId, + authority: request.authority, + scopes: request.scopes + }; - const response = await this.executePostToTokenEndpoint(authority.tokenEndpoint, requestBody, headers); + const response = await this.executePostToTokenEndpoint(authority.tokenEndpoint, requestBody, headers, thumbprint); const responseHandler = new ResponseHandler( this.config.authOptions.clientId, diff --git a/lib/msal-common/src/client/DeviceCodeClient.ts b/lib/msal-common/src/client/DeviceCodeClient.ts index da7e21eaa50..35e49b0c3dc 100644 --- a/lib/msal-common/src/client/DeviceCodeClient.ts +++ b/lib/msal-common/src/client/DeviceCodeClient.ts @@ -16,7 +16,7 @@ import { ScopeSet } from "../request/ScopeSet"; import { ResponseHandler } from "../response/ResponseHandler"; import { AuthenticationResult } from "../response/AuthenticationResult"; import { StringUtils } from "../utils/StringUtils"; -import { RequestThumbprint } from '../network/RequestThumbprint'; +import { RequestThumbprint } from "../network/RequestThumbprint"; /** * OAuth2.0 Device code client @@ -62,16 +62,14 @@ export class DeviceCodeClient extends BaseClient { * @param request */ private async getDeviceCode(request: DeviceCodeRequest): Promise { - + const queryString = this.createQueryString(request); + const headers = this.createDefaultLibraryHeaders(); const thumbprint: RequestThumbprint = { clientId: this.config.authOptions.clientId, authority: request.authority, scopes: request.scopes }; - const queryString = this.createQueryString(request); - const headers = this.createDefaultLibraryHeaders(); - return this.executePostRequestToDeviceCodeEndpoint(this.authority.deviceCodeEndpoint, queryString, headers, thumbprint); } diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index ec87281fe82..018ae73c551 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -14,10 +14,9 @@ import { GrantType } from "../utils/Constants"; import { ResponseHandler } from "../response/ResponseHandler"; import { AuthenticationResult } from "../response/AuthenticationResult"; import { StringUtils } from "../utils/StringUtils"; -import { RequestThumbprint } from "../network/RequestThumbprint" +import { RequestThumbprint } from "../network/RequestThumbprint"; import { NetworkResponse } from "../network/NetworkManager"; - /** * OAuth2.0 refresh token client */ @@ -48,16 +47,14 @@ export class RefreshTokenClient extends BaseClient { private async executeTokenRequest(request: RefreshTokenRequest, authority: Authority) : Promise> { - + const requestBody = this.createTokenRequestBody(request); + const headers: Record = this.createDefaultTokenRequestHeaders(); const thumbprint: RequestThumbprint = { clientId: this.config.authOptions.clientId, authority: authority.canonicalAuthority, scopes: request.scopes }; - const requestBody = this.createTokenRequestBody(request); - const headers: Record = this.createDefaultTokenRequestHeaders(); - return this.executePostToTokenEndpoint(authority.tokenEndpoint, requestBody, headers, thumbprint); } diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index bffa67c6087..9422fb1c898 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -25,12 +25,12 @@ export { AccountEntity } from "./cache/entities/AccountEntity"; export { IdTokenEntity } from "./cache/entities/IdTokenEntity"; export { AccessTokenEntity } from "./cache/entities/AccessTokenEntity"; export { RefreshTokenEntity } from "./cache/entities/RefreshTokenEntity"; +export { ThrottlingEntity } from "./cache/entities/ThrottlingEntity"; // Network Interface export { INetworkModule, NetworkRequestOptions } from "./network/INetworkModule"; export { NetworkManager, NetworkResponse } from "./network/NetworkManager"; export { ThrottlingUtils } from "./network/ThrottlingUtils"; export { RequestThumbprint } from "./network/RequestThumbprint"; -export { RequestThumbprintValue } from "./network/RequestThumbprintValue"; export { IUri } from "./url/IUri"; export { UrlString } from "./url/UrlString"; // Crypto Interface diff --git a/lib/msal-common/src/network/RequestThumbprintValue.ts b/lib/msal-common/src/network/RequestThumbprintValue.ts deleted file mode 100644 index eceee5a12ec..00000000000 --- a/lib/msal-common/src/network/RequestThumbprintValue.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -/** - * Type representing the values associated with a RequestThumbprint. - */ -export type RequestThumbprintValue = { - // Unix-time value representing the expiration of the throttle - throttleTime: number; - // Information provided by the server - error?: string; - errorCodes?: Array; - errorMessage?: string; - subError?: string; -}; diff --git a/lib/msal-common/src/network/ThrottlingUtils.ts b/lib/msal-common/src/network/ThrottlingUtils.ts index af85ce9a619..b3a36d6da8e 100644 --- a/lib/msal-common/src/network/ThrottlingUtils.ts +++ b/lib/msal-common/src/network/ThrottlingUtils.ts @@ -5,11 +5,11 @@ import { NetworkResponse } from "./NetworkManager"; import { ServerAuthorizationTokenResponse } from "../response/ServerAuthorizationTokenResponse"; -import { HeaderNames, CacheSchemaType, ThrottleConstants } from "../utils/Constants"; +import { HeaderNames, CacheSchemaType, ThrottlingConstants } from "../utils/Constants"; import { CacheManager } from "../cache/CacheManager"; import { ServerError } from "../error/ServerError"; import { RequestThumbprint } from "./RequestThumbprint"; -import { RequestThumbprintValue } from "./RequestThumbprintValue"; +import { ThrottlingEntity } from "../cache/entities/ThrottlingEntity"; export class ThrottlingUtils { @@ -18,7 +18,7 @@ export class ThrottlingUtils { * @param thumbprint */ static generateThrottlingStorageKey(thumbprint: RequestThumbprint): string { - return `${ThrottleConstants.THROTTLE_PREFIX}.${JSON.stringify(thumbprint)}`; + return `${ThrottlingConstants.THROTTLING_PREFIX}.${JSON.stringify(thumbprint)}`; } /** @@ -28,12 +28,11 @@ export class ThrottlingUtils { */ static preProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint): void { const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); - const value = cacheManager.getItem(key, CacheSchemaType.THROTTLE) as RequestThumbprintValue; - // console.log("VALUE:", value); + const value = cacheManager.getItem(key, CacheSchemaType.THROTTLING) as ThrottlingEntity; if (value) { if (value.throttleTime < Date.now()) { - cacheManager.removeItem(key, CacheSchemaType.THROTTLE); + cacheManager.removeItem(key, CacheSchemaType.THROTTLING); return; } throw new ServerError(value.errorCodes.join(" "), value.errorMessage, value.subError); @@ -48,8 +47,8 @@ export class ThrottlingUtils { */ static postProcess(cacheManager: CacheManager, thumbprint: RequestThumbprint, response: NetworkResponse): void { if (ThrottlingUtils.checkResponseStatus(response) || ThrottlingUtils.checkResponseForRetryAfter(response)) { - const thumbprintValue: RequestThumbprintValue = { - throttleTime: ThrottlingUtils.calculateThrottleTime(parseInt(response.headers.get(HeaderNames.RETRY_AFTER))), + const thumbprintValue: ThrottlingEntity = { + throttleTime: ThrottlingUtils.calculateThrottleTime(parseInt(response.headers[HeaderNames.RETRY_AFTER])), error: response.body.error, errorCodes: response.body.error_codes, errorMessage: response.body.error_description, @@ -58,7 +57,7 @@ export class ThrottlingUtils { cacheManager.setItem( ThrottlingUtils.generateThrottlingStorageKey(thumbprint), thumbprintValue, - CacheSchemaType.THROTTLE + CacheSchemaType.THROTTLING ); } } @@ -77,7 +76,7 @@ export class ThrottlingUtils { */ static checkResponseForRetryAfter(response: NetworkResponse): boolean { if (response.headers) { - return response.headers.has(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300); + return response.headers.hasOwnProperty(HeaderNames.RETRY_AFTER) && (response.status < 200 || response.status >= 300); } return false; } @@ -92,8 +91,8 @@ export class ThrottlingUtils { } const currentSeconds = Date.now() / 1000; return Math.floor(Math.min( - currentSeconds + (throttleTime || ThrottleConstants.DEFAULT_THROTTLE_TIME_SECONDS), - currentSeconds + ThrottleConstants.DEFAULT_MAX_THROTTLE_TIME_SECONDS + currentSeconds + (throttleTime || ThrottlingConstants.DEFAULT_THROTTLE_TIME_SECONDS), + currentSeconds + ThrottlingConstants.DEFAULT_MAX_THROTTLE_TIME_SECONDS ) * 1000); } @@ -106,6 +105,6 @@ export class ThrottlingUtils { }; const key = this.generateThrottlingStorageKey(thumbprint); - return cacheManager.removeItem(key, CacheSchemaType.THROTTLE); + return cacheManager.removeItem(key, CacheSchemaType.THROTTLING); } } diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index caae68bd311..40dd3695fcb 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -244,7 +244,7 @@ export enum CacheSchemaType { TEMPORARY = "TempCache", TELEMETRY = "Telemetry", UNDEFINED = "Undefined", - THROTTLE = "Throttle" + THROTTLING = "Throttling" } /** @@ -279,11 +279,11 @@ export const SERVER_TELEM_CONSTANTS = { /** * Constants related to throttling */ -export const ThrottleConstants = { +export const ThrottlingConstants = { // Default time to throttle RequestThumbprint in seconds DEFAULT_THROTTLE_TIME_SECONDS: 60, // Default maximum time to throttle in seconds, overrides what the server sends back DEFAULT_MAX_THROTTLE_TIME_SECONDS: 3600, // Prefix for storing throttling entries - THROTTLE_PREFIX: "throttle" + THROTTLING_PREFIX: "throttling" }; diff --git a/lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts b/lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts new file mode 100644 index 00000000000..1432342f1a0 --- /dev/null +++ b/lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts @@ -0,0 +1,45 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import "mocha" +import { expect } from "chai"; +import { ThrottlingEntity } from "../../../src/cache/entities/ThrottlingEntity"; +import { ThrottlingConstants, Separators } from "../../../src/utils/Constants"; +import { TEST_CONFIG } from "../../utils/StringConstants"; + +describe("ThrottlingEntity", () => { + describe("isThrottlingEntity", () => { + + const key = ThrottlingConstants.THROTTLING_PREFIX + Separators.CACHE_KEY_SEPARATOR + TEST_CONFIG.MSAL_CLIENT_ID; + it("Verifies if an object is a ThrottlingEntity", () => { + const throttlingObject = { + throttleTime: 0 + } + expect(ThrottlingEntity.isThrottlingEntity(key, throttlingObject)).to.be.true; + + }); + + it("Verifies if an object is a ThrottlingEntity when no object is given", () => { + expect(ThrottlingEntity.isThrottlingEntity(key)).to.be.true; + expect(ThrottlingEntity.isThrottlingEntity(key, null)).to.be.true; + }); + + it("Verifies if an object is not a ThrottlingEntity based on field", () => { + const throttlingObject = { + test: 0 + } + expect(ThrottlingEntity.isThrottlingEntity(key, throttlingObject)).to.be.false; + }); + + it("Verifies if an object is not a ThrottlingEntity based on key", () => { + const throttlingObject = { + throttleTime: 0 + } + expect(ThrottlingEntity.isThrottlingEntity("asd", throttlingObject)).to.be.false; + expect(ThrottlingEntity.isThrottlingEntity("", throttlingObject)).to.be.false; + expect(ThrottlingEntity.isThrottlingEntity(null, throttlingObject)).to.be.false; + }) + }); +}); diff --git a/lib/msal-common/test/network/NetworkManager.spec.ts b/lib/msal-common/test/network/NetworkManager.spec.ts index a6ad7dd8ee4..f471bc55b9a 100644 --- a/lib/msal-common/test/network/NetworkManager.spec.ts +++ b/lib/msal-common/test/network/NetworkManager.spec.ts @@ -12,7 +12,7 @@ import { ServerAuthorizationTokenResponse } from "../../src/response/ServerAutho import { MockStorageClass } from "../client/ClientTestUtils"; import { NetworkRequestOptions } from "../../src/network/INetworkModule"; import { ServerError } from "../../src/error/ServerError"; -import { AUTHENTICATION_RESULT, NETWORK_REQUEST_OPTIONS, THUMBPRINT, THUMBPRINT_VALUE, DEFAULT_NETWORK_IMPLEMENTATION } from "../utils/StringConstants"; +import { AUTHENTICATION_RESULT, NETWORK_REQUEST_OPTIONS, THUMBPRINT, THROTTLING_ENTITY, DEFAULT_NETWORK_IMPLEMENTATION } from "../utils/StringConstants"; describe("NetworkManager", () => { describe("sendPostRequest", () => { @@ -27,7 +27,7 @@ describe("NetworkManager", () => { const thumbprint: RequestThumbprint = THUMBPRINT; const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; const mockRes: NetworkResponse = { - headers: new Map(), + headers: { }, body: AUTHENTICATION_RESULT.body, status: 200 } @@ -52,9 +52,9 @@ describe("NetworkManager", () => { const networkManager = new NetworkManager(networkInterface, cache); const thumbprint: RequestThumbprint = THUMBPRINT; const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; - const mockThumbprintValue = THUMBPRINT_VALUE; + const mockThrottlingEntity = THROTTLING_ENTITY; const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync"); - const getItemStub = sinon.stub(cache, "getItem").returns(mockThumbprintValue); + const getItemStub = sinon.stub(cache, "getItem").returns(mockThrottlingEntity); const setItemStub = sinon.stub(cache, "setItem"); const removeItemStub = sinon.stub(cache, "removeItem"); sinon.stub(Date, "now").callsFake(() => 1) @@ -77,13 +77,13 @@ describe("NetworkManager", () => { const thumbprint: RequestThumbprint = THUMBPRINT; const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; const mockRes: NetworkResponse = { - headers: new Map(), + headers: { }, body: AUTHENTICATION_RESULT.body, status: 200 } - const mockThumbprintValue = THUMBPRINT_VALUE; + const mockThrottlingEntity = THROTTLING_ENTITY; const networkStub = sinon.stub(networkInterface, "sendPostRequestAsync").returns(Promise.resolve(mockRes)); - const getItemStub = sinon.stub(cache, "getItem").returns(mockThumbprintValue); + const getItemStub = sinon.stub(cache, "getItem").returns(mockThrottlingEntity); const setItemStub = sinon.stub(cache, "setItem"); const removeItemStub = sinon.stub(cache, "removeItem"); sinon.stub(Date, "now").callsFake(() => 10) @@ -104,7 +104,7 @@ describe("NetworkManager", () => { const thumbprint: RequestThumbprint = THUMBPRINT; const options: NetworkRequestOptions = NETWORK_REQUEST_OPTIONS; const mockRes: NetworkResponse = { - headers: new Map(), + headers: { }, body: AUTHENTICATION_RESULT.body, status: 500 } diff --git a/lib/msal-common/test/network/ThrottlingUtils.spec.ts b/lib/msal-common/test/network/ThrottlingUtils.spec.ts index abab3e37e03..09c51692c77 100644 --- a/lib/msal-common/test/network/ThrottlingUtils.spec.ts +++ b/lib/msal-common/test/network/ThrottlingUtils.spec.ts @@ -7,12 +7,12 @@ import { expect } from "chai"; import sinon from "sinon"; import { ThrottlingUtils } from "../../src/network/ThrottlingUtils"; import { RequestThumbprint } from "../../src/network/RequestThumbprint"; -import { RequestThumbprintValue } from "../../src/network/RequestThumbprintValue"; +import { ThrottlingEntity } from "../../src/cache/entities/ThrottlingEntity"; import { NetworkResponse } from "../../src/network/NetworkManager"; import { ServerAuthorizationTokenResponse } from "../../src/response/ServerAuthorizationTokenResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; import { ServerError } from "../../src"; -import { THUMBPRINT, THUMBPRINT_VALUE, TEST_CONFIG } from "../utils/StringConstants"; +import { THUMBPRINT, THROTTLING_ENTITY, TEST_CONFIG } from "../utils/StringConstants"; describe("ThrottlingUtils", () => { describe("generateThrottlingStorageKey", () => { @@ -21,7 +21,7 @@ describe("ThrottlingUtils", () => { const jsonString = JSON.stringify(thumbprint); const key = ThrottlingUtils.generateThrottlingStorageKey(thumbprint); - expect(key).to.deep.eq(`throttle.${jsonString}`); + expect(key).to.deep.eq(`throttling.${jsonString}`); }); }); @@ -32,7 +32,7 @@ describe("ThrottlingUtils", () => { it("checks the cache and throws an error", () => { const thumbprint: RequestThumbprint = THUMBPRINT; - const thumbprintValue: RequestThumbprintValue = THUMBPRINT_VALUE; + const thumbprintValue: ThrottlingEntity = THROTTLING_ENTITY; const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); sinon.stub(cache, "getItem").callsFake(() => thumbprintValue); @@ -48,7 +48,7 @@ describe("ThrottlingUtils", () => { it("checks the cache and removes an item", () => { const thumbprint: RequestThumbprint = THUMBPRINT; - const thumbprintValue: RequestThumbprintValue = THUMBPRINT_VALUE; + const thumbprintValue: ThrottlingEntity = THROTTLING_ENTITY; const cache = new MockStorageClass(); const removeItemStub = sinon.stub(cache, "removeItem"); sinon.stub(cache, "getItem").callsFake(() => thumbprintValue); @@ -81,7 +81,7 @@ describe("ThrottlingUtils", () => { it("sets an item in the cache", () => { const thumbprint: RequestThumbprint = THUMBPRINT; const res: NetworkResponse = { - headers: new Map(), + headers: { }, body: { }, status: 429 }; @@ -95,7 +95,7 @@ describe("ThrottlingUtils", () => { it("does not set an item in the cache", () => { const thumbprint: RequestThumbprint = THUMBPRINT; const res: NetworkResponse = { - headers: new Map(), + headers: { }, body: { }, status: 200 }; @@ -110,7 +110,7 @@ describe("ThrottlingUtils", () => { describe("checkResponseStatus", () => { it("returns true if status == 429", () => { const res: NetworkResponse = { - headers: new Map(), + headers: { }, body: { }, status: 429 }; @@ -121,7 +121,7 @@ describe("ThrottlingUtils", () => { it("returns true if 500 <= status < 600", () => { const res: NetworkResponse = { - headers: new Map(), + headers: { }, body: { }, status: 500 }; @@ -132,7 +132,7 @@ describe("ThrottlingUtils", () => { it("returns false if status is not 429 or between 500 and 600", () => { const res: NetworkResponse = { - headers: new Map(), + headers: { }, body: { }, status: 430 }; @@ -144,8 +144,8 @@ describe("ThrottlingUtils", () => { describe("checkResponseForRetryAfter", () => { it("returns true when Retry-After header exists and when status <= 200", () => { - const headers = new Map(); - headers.set("Retry-After", "test"); + const headers: Record = { }; + headers["Retry-After"] = "test"; const res: NetworkResponse = { headers, body: { }, @@ -157,8 +157,8 @@ describe("ThrottlingUtils", () => { }); it("returns true when Retry-After header exists and when status > 300", () => { - const headers = new Map(); - headers.set("Retry-After", "test"); + const headers: Record = { }; + headers["Retry-After"] = "test"; const res: NetworkResponse = { headers, body: { }, @@ -170,7 +170,7 @@ describe("ThrottlingUtils", () => { }); it("returns false when there is no RetryAfter header", () => { - const headers = new Map(); + const headers: Record = { }; const res: NetworkResponse = { headers, body: { }, @@ -182,7 +182,7 @@ describe("ThrottlingUtils", () => { }); it("returns false when 200 <= status < 300", () => { - const headers = new Map(); + const headers: Record = { }; const res: NetworkResponse = { headers, body: { }, diff --git a/lib/msal-common/test/utils/StringConstants.ts b/lib/msal-common/test/utils/StringConstants.ts index c403ef80dfc..676c91c0c46 100644 --- a/lib/msal-common/test/utils/StringConstants.ts +++ b/lib/msal-common/test/utils/StringConstants.ts @@ -3,7 +3,7 @@ */ import { Constants } from "../../src/utils/Constants"; -import { RequestThumbprint, RequestThumbprintValue } from "../../src"; +import { RequestThumbprint, ThrottlingEntity } from "../../src"; import { NetworkRequestOptions } from "../../src/network/INetworkModule"; // Test Tokens @@ -294,7 +294,7 @@ export const THUMBPRINT: RequestThumbprint = { scopes: TEST_CONFIG.DEFAULT_SCOPES }; -export const THUMBPRINT_VALUE: RequestThumbprintValue = { +export const THROTTLING_ENTITY: ThrottlingEntity = { throttleTime: 5, error: "This is a error", errorCodes: ["ErrorCode"], @@ -303,6 +303,6 @@ export const THUMBPRINT_VALUE: RequestThumbprintValue = { }; export const NETWORK_REQUEST_OPTIONS: NetworkRequestOptions = { - headers: new Map(), + headers: { }, body: "" }; From a1f48845aad8e9e4c01e3aa6eb7f993c021bb2b9 Mon Sep 17 00:00:00 2001 From: Jake McKennon Date: Thu, 3 Sep 2020 15:52:03 -0700 Subject: [PATCH 34/34] Add CacheValueType for ThrottlingEntity --- lib/msal-common/src/cache/utils/CacheTypes.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/msal-common/src/cache/utils/CacheTypes.ts b/lib/msal-common/src/cache/utils/CacheTypes.ts index e2bb6de336a..bba10b9ef73 100644 --- a/lib/msal-common/src/cache/utils/CacheTypes.ts +++ b/lib/msal-common/src/cache/utils/CacheTypes.ts @@ -9,6 +9,7 @@ import { AccessTokenEntity } from "../entities/AccessTokenEntity"; import { RefreshTokenEntity } from "../entities/RefreshTokenEntity"; import { AppMetadataEntity } from "../entities/AppMetadataEntity"; import { ServerTelemetryEntity } from "../entities/ServerTelemetryEntity"; +import { ThrottlingEntity } from "../entities/ThrottlingEntity"; export type AccountCache = Record; export type IdTokenCache = Record; @@ -21,7 +22,7 @@ export type CredentialCache = { refreshTokens: RefreshTokenCache; }; -export type ValidCacheType = AccountEntity | IdTokenEntity | AccessTokenEntity | RefreshTokenEntity | AppMetadataEntity | ServerTelemetryEntity | string; +export type ValidCacheType = AccountEntity | IdTokenEntity | AccessTokenEntity | RefreshTokenEntity | AppMetadataEntity | ServerTelemetryEntity | ThrottlingEntity | string; /** * Account: --