From 781325ba46c8c7eb216b8fff8eebda253aadfea9 Mon Sep 17 00:00:00 2001 From: Thomas Norling Date: Wed, 27 Sep 2023 13:05:31 -0700 Subject: [PATCH] Instrument Crypto APIs (#6512) - Adds instrumentation for Crypto APIs, specifically around PKCE generation - Also refactors `PkceGenerator` and `BrowserCrypto` classes into pure functions for better minification and simpler usage --- ...-a42f4c95-e6b7-42fd-9f86-3e1eb633116c.json | 7 + ...-d4f5a186-023f-4206-8f33-b15b00183000.json | 7 + ...-9cd7fd0e-9f0d-4019-9263-a2dab97bf42f.json | 7 + .../nativeBroker/NativeMessageHandler.ts | 16 +- .../src/controllers/StandardController.ts | 6 +- lib/msal-browser/src/crypto/BrowserCrypto.ts | 221 ++++++++++-------- lib/msal-browser/src/crypto/CryptoOps.ts | 32 +-- lib/msal-browser/src/crypto/ISubtleCrypto.ts | 30 --- .../src/crypto/ModernBrowserCrypto.ts | 69 ------ lib/msal-browser/src/crypto/PkceGenerator.ts | 147 +++++++----- lib/msal-browser/src/event/EventHandler.ts | 3 +- .../BaseInteractionClient.ts | 4 +- .../StandardInteractionClient.ts | 16 +- .../test/app/PublicClientApplication.spec.ts | 104 +++++---- .../test/broker/NativeMessageHandler.spec.ts | 30 +-- .../test/crypto/CryptoOps.spec.ts | 57 ++--- .../test/crypto/PkceGenerator.spec.ts | 23 +- .../test/crypto/SignedHttpRequest.spec.ts | 5 +- .../NativeInteractionClient.spec.ts | 3 +- .../interaction_client/PopupClient.spec.ts | 49 ++-- .../interaction_client/RedirectClient.spec.ts | 53 +++-- .../SilentIframeClient.spec.ts | 59 +++-- .../SilentRefreshClient.spec.ts | 8 +- .../StandardInteractionClient.spec.ts | 5 +- .../RedirectHandler.spec.ts | 3 - .../interaction_handler/SilentHandler.spec.ts | 3 - lib/msal-common/src/crypto/ICrypto.ts | 7 - .../telemetry/performance/PerformanceEvent.ts | 9 + lib/msal-common/src/utils/FunctionWrappers.ts | 1 - .../test/account/AuthToken.spec.ts | 9 +- .../test/account/ClientInfo.spec.ts | 9 +- .../test/cache/entities/AccountEntity.spec.ts | 9 +- .../test/client/ClientTestUtils.ts | 7 - .../test/config/ClientConfiguration.spec.ts | 19 -- .../test/crypto/PopTokenGenerator.spec.ts | 8 +- .../test/response/ResponseHandler.spec.ts | 8 +- .../test/utils/ProtocolUtils.spec.ts | 9 +- lib/msal-node/test/utils/TestConstants.ts | 4 - 38 files changed, 480 insertions(+), 586 deletions(-) create mode 100644 change/@azure-msal-browser-a42f4c95-e6b7-42fd-9f86-3e1eb633116c.json create mode 100644 change/@azure-msal-common-d4f5a186-023f-4206-8f33-b15b00183000.json create mode 100644 change/@azure-msal-node-9cd7fd0e-9f0d-4019-9263-a2dab97bf42f.json delete mode 100644 lib/msal-browser/src/crypto/ISubtleCrypto.ts delete mode 100644 lib/msal-browser/src/crypto/ModernBrowserCrypto.ts diff --git a/change/@azure-msal-browser-a42f4c95-e6b7-42fd-9f86-3e1eb633116c.json b/change/@azure-msal-browser-a42f4c95-e6b7-42fd-9f86-3e1eb633116c.json new file mode 100644 index 0000000000..e1ca016ecf --- /dev/null +++ b/change/@azure-msal-browser-a42f4c95-e6b7-42fd-9f86-3e1eb633116c.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Instrument Crypto APIs #6512", + "packageName": "@azure/msal-browser", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-msal-common-d4f5a186-023f-4206-8f33-b15b00183000.json b/change/@azure-msal-common-d4f5a186-023f-4206-8f33-b15b00183000.json new file mode 100644 index 0000000000..c6d3710d93 --- /dev/null +++ b/change/@azure-msal-common-d4f5a186-023f-4206-8f33-b15b00183000.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Instrument Crypto APIs #6512", + "packageName": "@azure/msal-common", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-msal-node-9cd7fd0e-9f0d-4019-9263-a2dab97bf42f.json b/change/@azure-msal-node-9cd7fd0e-9f0d-4019-9263-a2dab97bf42f.json new file mode 100644 index 0000000000..eaff29fb72 --- /dev/null +++ b/change/@azure-msal-node-9cd7fd0e-9f0d-4019-9263-a2dab97bf42f.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "Update tests #6512", + "packageName": "@azure/msal-node", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "none" +} diff --git a/lib/msal-browser/src/broker/nativeBroker/NativeMessageHandler.ts b/lib/msal-browser/src/broker/nativeBroker/NativeMessageHandler.ts index 0c20646132..faffc7a429 100644 --- a/lib/msal-browser/src/broker/nativeBroker/NativeMessageHandler.ts +++ b/lib/msal-browser/src/broker/nativeBroker/NativeMessageHandler.ts @@ -16,7 +16,6 @@ import { InProgressPerformanceEvent, PerformanceEvents, IPerformanceClient, - ICrypto, } from "@azure/msal-common"; import { NativeExtensionRequest, @@ -28,6 +27,7 @@ import { BrowserAuthErrorCodes, } from "../../error/BrowserAuthError"; import { BrowserConfiguration } from "../../config/Configuration"; +import { createNewGuid } from "../../crypto/BrowserCrypto"; type ResponseResolvers = { resolve: (value: T | PromiseLike) => void; @@ -40,7 +40,6 @@ export class NativeMessageHandler { private extensionId: string | undefined; private extensionVersion: string | undefined; private logger: Logger; - private crypto: ICrypto; private readonly handshakeTimeoutMs: number; private timeoutId: number | undefined; private resolvers: Map>; @@ -54,7 +53,6 @@ export class NativeMessageHandler { logger: Logger, handshakeTimeoutMs: number, performanceClient: IPerformanceClient, - crypto: ICrypto, extensionId?: string ) { this.logger = logger; @@ -68,7 +66,6 @@ export class NativeMessageHandler { this.handshakeEvent = performanceClient.startMeasurement( PerformanceEvents.NativeMessageHandlerHandshake ); - this.crypto = crypto; } /** @@ -80,7 +77,7 @@ export class NativeMessageHandler { const req: NativeExtensionRequest = { channel: NativeConstants.CHANNEL_ID, extensionId: this.extensionId, - responseId: this.crypto.createNewGuid(), + responseId: createNewGuid(), body: body, }; @@ -109,8 +106,7 @@ export class NativeMessageHandler { static async createProvider( logger: Logger, handshakeTimeoutMs: number, - performanceClient: IPerformanceClient, - crypto: ICrypto + performanceClient: IPerformanceClient ): Promise { logger.trace("NativeMessageHandler - createProvider called."); try { @@ -118,7 +114,6 @@ export class NativeMessageHandler { logger, handshakeTimeoutMs, performanceClient, - crypto, NativeConstants.PREFERRED_EXTENSION_ID ); await preferredProvider.sendHandshakeRequest(); @@ -128,8 +123,7 @@ export class NativeMessageHandler { const backupProvider = new NativeMessageHandler( logger, handshakeTimeoutMs, - performanceClient, - crypto + performanceClient ); await backupProvider.sendHandshakeRequest(); return backupProvider; @@ -149,7 +143,7 @@ export class NativeMessageHandler { const req: NativeExtensionRequest = { channel: NativeConstants.CHANNEL_ID, extensionId: this.extensionId, - responseId: this.crypto.createNewGuid(), + responseId: createNewGuid(), body: { method: NativeExtensionMethod.HandshakeRequest, }, diff --git a/lib/msal-browser/src/controllers/StandardController.ts b/lib/msal-browser/src/controllers/StandardController.ts index e5fedf1cad..bd22b75150 100644 --- a/lib/msal-browser/src/controllers/StandardController.ts +++ b/lib/msal-browser/src/controllers/StandardController.ts @@ -85,6 +85,7 @@ import { BaseOperatingContext } from "../operatingcontext/BaseOperatingContext"; import { IController } from "./IController"; import { AuthenticationResult } from "../response/AuthenticationResult"; import { ClearCacheRequest } from "../request/ClearCacheRequest"; +import { createNewGuid } from "../crypto/BrowserCrypto"; export class StandardController implements IController { // OperatingContext @@ -289,8 +290,7 @@ export class StandardController implements IController { await NativeMessageHandler.createProvider( this.logger, this.config.system.nativeBrokerHandshakeTimeout, - this.performanceClient, - this.browserCrypto + this.performanceClient ); } catch (e) { this.logger.verbose(e as string); @@ -1826,7 +1826,7 @@ export class StandardController implements IController { } if (this.isBrowserEnvironment) { - return this.browserCrypto.createNewGuid(); + return createNewGuid(); } /* diff --git a/lib/msal-browser/src/crypto/BrowserCrypto.ts b/lib/msal-browser/src/crypto/BrowserCrypto.ts index 8cca03e9a1..0bbc08b63f 100644 --- a/lib/msal-browser/src/crypto/BrowserCrypto.ts +++ b/lib/msal-browser/src/crypto/BrowserCrypto.ts @@ -8,9 +8,18 @@ import { createBrowserAuthError, BrowserAuthErrorCodes, } from "../error/BrowserAuthError"; -import { ISubtleCrypto } from "./ISubtleCrypto"; -import { ModernBrowserCrypto } from "./ModernBrowserCrypto"; -import { Logger } from "@azure/msal-common"; +import { + IPerformanceClient, + Logger, + PerformanceEvents, +} from "@azure/msal-common"; +import { KEY_FORMAT_JWK } from "../utils/BrowserConstants"; + +/** + * This file defines functions used by the browser library to perform cryptography operations such as + * hashing and encoding. It also has helper functions to validate the availability of specific APIs. + */ + /** * See here for more info on RsaHashedKeyGenParams: https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedKeyGenParams */ @@ -23,113 +32,121 @@ const MODULUS_LENGTH = 2048; // Public Exponent const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]); +const keygenAlgorithmOptions: RsaHashedKeyGenParams = { + name: PKCS1_V15_KEYGEN_ALG, + hash: S256_HASH_ALG, + modulusLength: MODULUS_LENGTH, + publicExponent: PUBLIC_EXPONENT, +}; + /** - * This class implements functions used by the browser library to perform cryptography operations such as - * hashing and encoding. It also has helper functions to validate the availability of specific APIs. + * Check whether browser crypto is available. */ -export class BrowserCrypto { - private keygenAlgorithmOptions: RsaHashedKeyGenParams; - private subtleCrypto: ISubtleCrypto; - private logger: Logger; - - constructor(logger: Logger) { - this.logger = logger; - - if (this.hasBrowserCrypto()) { - // Use standard modern web crypto if available - this.logger.verbose( - "BrowserCrypto: modern crypto interface available" - ); - this.subtleCrypto = new ModernBrowserCrypto(); - } else { - this.logger.error("BrowserCrypto: crypto interface is unavailable"); - throw createBrowserAuthError( - BrowserAuthErrorCodes.cryptoNonExistent - ); - } - - this.keygenAlgorithmOptions = { - name: PKCS1_V15_KEYGEN_ALG, - hash: S256_HASH_ALG, - modulusLength: MODULUS_LENGTH, - publicExponent: PUBLIC_EXPONENT, - }; +export function validateCryptoAvailable(logger: Logger): void { + if ("crypto" in window) { + logger.verbose("BrowserCrypto: modern crypto interface available"); + } else { + logger.error("BrowserCrypto: crypto interface is unavailable"); + throw createBrowserAuthError(BrowserAuthErrorCodes.cryptoNonExistent); } +} - /** - * Check whether browser crypto is available. - */ - private hasBrowserCrypto(): boolean { - return "crypto" in window; - } +/** + * Returns a sha-256 hash of the given dataString as an ArrayBuffer. + * @param dataString + */ +export async function sha256Digest( + dataString: string, + performanceClient?: IPerformanceClient, + correlationId?: string +): Promise { + performanceClient?.addQueueMeasurement( + PerformanceEvents.Sha256Digest, + correlationId + ); + const data = BrowserStringUtils.stringToUtf8Arr(dataString); + // MSR Crypto wants object with name property, instead of string + return window.crypto.subtle.digest( + keygenAlgorithmOptions, + data + ) as Promise; +} - /** - * Returns a sha-256 hash of the given dataString as an ArrayBuffer. - * @param dataString - */ - async sha256Digest(dataString: string): Promise { - const data = BrowserStringUtils.stringToUtf8Arr(dataString); - // MSR Crypto wants object with name property, instead of string - return this.subtleCrypto.digest({ name: S256_HASH_ALG }, data); - } +/** + * Populates buffer with cryptographically random values. + * @param dataBuffer + */ +export function getRandomValues(dataBuffer: Uint8Array): Uint8Array { + return window.crypto.getRandomValues(dataBuffer); +} - /** - * Populates buffer with cryptographically random values. - * @param dataBuffer - */ - getRandomValues(dataBuffer: Uint8Array): Uint8Array { - return this.subtleCrypto.getRandomValues(dataBuffer); - } +/** + * Creates a new random GUID + * @returns + */ +export function createNewGuid(): string { + return window.crypto.randomUUID(); +} - /** - * Generates a keypair based on current keygen algorithm config. - * @param extractable - * @param usages - */ - async generateKeyPair( - extractable: boolean, - usages: Array - ): Promise { - return this.subtleCrypto.generateKey( - this.keygenAlgorithmOptions, - extractable, - usages - ); - } +/** + * Generates a keypair based on current keygen algorithm config. + * @param extractable + * @param usages + */ +export async function generateKeyPair( + extractable: boolean, + usages: Array +): Promise { + return window.crypto.subtle.generateKey( + keygenAlgorithmOptions, + extractable, + usages + ) as Promise; +} - /** - * Export key as Json Web Key (JWK) - * @param key - */ - async exportJwk(key: CryptoKey): Promise { - return this.subtleCrypto.exportKey(key); - } +/** + * Export key as Json Web Key (JWK) + * @param key + */ +export async function exportJwk(key: CryptoKey): Promise { + return window.crypto.subtle.exportKey( + KEY_FORMAT_JWK, + key + ) as Promise; +} - /** - * Imports key as Json Web Key (JWK), can set extractable and usages. - * @param key - * @param extractable - * @param usages - */ - async importJwk( - key: JsonWebKey, - extractable: boolean, - usages: Array - ): Promise { - return this.subtleCrypto.importKey( - key, - this.keygenAlgorithmOptions, - extractable, - usages - ); - } +/** + * Imports key as Json Web Key (JWK), can set extractable and usages. + * @param key + * @param extractable + * @param usages + */ +export async function importJwk( + key: JsonWebKey, + extractable: boolean, + usages: Array +): Promise { + return window.crypto.subtle.importKey( + KEY_FORMAT_JWK, + key, + keygenAlgorithmOptions, + extractable, + usages + ) as Promise; +} - /** - * Signs given data with given key - * @param key - * @param data - */ - async sign(key: CryptoKey, data: ArrayBuffer): Promise { - return this.subtleCrypto.sign(this.keygenAlgorithmOptions, key, data); - } +/** + * Signs given data with given key + * @param key + * @param data + */ +export async function sign( + key: CryptoKey, + data: ArrayBuffer +): Promise { + return window.crypto.subtle.sign( + keygenAlgorithmOptions, + key, + data + ) as Promise; } diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index f367a10904..82a1bf0866 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -9,14 +9,12 @@ import { JoseHeader, Logger, PerformanceEvents, - PkceCodes, SignedHttpRequest, SignedHttpRequestParameters, } from "@azure/msal-common"; import { base64Encode, urlEncode, urlEncodeArr } from "../encode/Base64Encode"; import { base64Decode } from "../encode/Base64Decode"; -import { PkceGenerator } from "./PkceGenerator"; -import { BrowserCrypto } from "./BrowserCrypto"; +import * as BrowserCrypto from "./BrowserCrypto"; import { BrowserStringUtils } from "../utils/BrowserStringUtils"; import { createBrowserAuthError, @@ -36,8 +34,6 @@ export type CachedKeyPair = { * implementing Proof Key for Code Exchange specs for the OAuth Authorization Code Flow using PKCE (rfc here: https://tools.ietf.org/html/rfc7636). */ export class CryptoOps implements ICrypto { - private browserCrypto: BrowserCrypto; - private pkceGenerator: PkceGenerator; private logger: Logger; /** @@ -53,8 +49,7 @@ export class CryptoOps implements ICrypto { constructor(logger: Logger, performanceClient?: IPerformanceClient) { this.logger = logger; // Browser crypto needs to be validated first before any other classes can be set. - this.browserCrypto = new BrowserCrypto(this.logger); - this.pkceGenerator = new PkceGenerator(this.browserCrypto); + BrowserCrypto.validateCryptoAvailable(logger); this.cache = new CryptoKeyStore(this.logger); this.performanceClient = performanceClient; } @@ -64,7 +59,7 @@ export class CryptoOps implements ICrypto { * @returns string (GUID) */ createNewGuid(): string { - return window.crypto.randomUUID(); + return BrowserCrypto.createNewGuid(); } /** @@ -83,13 +78,6 @@ export class CryptoOps implements ICrypto { return base64Decode(input); } - /** - * Generates PKCE codes used in Authorization Code Flow. - */ - async generatePkceCodes(): Promise { - return this.pkceGenerator.generateCodes(); - } - /** * Generates a keypair, stores it and returns a thumbprint * @param request @@ -104,13 +92,13 @@ export class CryptoOps implements ICrypto { ); // Generate Keypair - const keyPair: CryptoKeyPair = await this.browserCrypto.generateKeyPair( + const keyPair: CryptoKeyPair = await BrowserCrypto.generateKeyPair( CryptoOps.EXTRACTABLE, CryptoOps.POP_KEY_USAGES ); // Generate Thumbprint for Public Key - const publicKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk( + const publicKeyJwk: JsonWebKey = await BrowserCrypto.exportJwk( keyPair.publicKey ); @@ -125,12 +113,12 @@ export class CryptoOps implements ICrypto { const publicJwkHash = await this.hashString(publicJwkString); // Generate Thumbprint for Private Key - const privateKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk( + const privateKeyJwk: JsonWebKey = await BrowserCrypto.exportJwk( keyPair.privateKey ); // Re-import private key to make it unextractable const unextractablePrivateKey: CryptoKey = - await this.browserCrypto.importJwk(privateKeyJwk, false, ["sign"]); + await BrowserCrypto.importJwk(privateKeyJwk, false, ["sign"]); // Store Keypair data in keystore await this.cache.asymmetricKeys.setItem(publicJwkHash, { @@ -189,7 +177,7 @@ export class CryptoOps implements ICrypto { } // Get public key as JWK - const publicKeyJwk = await this.browserCrypto.exportJwk( + const publicKeyJwk = await BrowserCrypto.exportJwk( cachedKeyPair.publicKey ); const publicKeyJwkString = @@ -216,7 +204,7 @@ export class CryptoOps implements ICrypto { // Sign token const tokenBuffer = BrowserStringUtils.stringToUtf8Arr(tokenString); - const signatureBuffer = await this.browserCrypto.sign( + const signatureBuffer = await BrowserCrypto.sign( cachedKeyPair.privateKey, tokenBuffer ); @@ -238,7 +226,7 @@ export class CryptoOps implements ICrypto { * @param plainText */ async hashString(plainText: string): Promise { - const hashBuffer: ArrayBuffer = await this.browserCrypto.sha256Digest( + const hashBuffer: ArrayBuffer = await BrowserCrypto.sha256Digest( plainText ); const hashBytes = new Uint8Array(hashBuffer); diff --git a/lib/msal-browser/src/crypto/ISubtleCrypto.ts b/lib/msal-browser/src/crypto/ISubtleCrypto.ts deleted file mode 100644 index 0cedfad81a..0000000000 --- a/lib/msal-browser/src/crypto/ISubtleCrypto.ts +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -export interface ISubtleCrypto { - initPrng?(entropy: Uint8Array): void; - getRandomValues(dataBuffer: Uint8Array): Uint8Array; - generateKey( - algorithm: RsaHashedKeyGenParams, - extractable: boolean, - keyUsages: KeyUsage[] - ): Promise; - exportKey(key: CryptoKey): Promise; - importKey( - keyData: JsonWebKey, - algorithm: RsaHashedImportParams, - extractable: boolean, - keyUsages: KeyUsage[] - ): Promise; - sign( - algorithm: AlgorithmIdentifier, - key: CryptoKey, - data: ArrayBuffer - ): Promise; - digest( - algorithm: AlgorithmIdentifier, - data: Uint8Array - ): Promise; -} diff --git a/lib/msal-browser/src/crypto/ModernBrowserCrypto.ts b/lib/msal-browser/src/crypto/ModernBrowserCrypto.ts deleted file mode 100644 index 59a798b698..0000000000 --- a/lib/msal-browser/src/crypto/ModernBrowserCrypto.ts +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -import { KEY_FORMAT_JWK } from "../utils/BrowserConstants"; -import { ISubtleCrypto } from "./ISubtleCrypto"; - -export class ModernBrowserCrypto implements ISubtleCrypto { - getRandomValues(dataBuffer: Uint8Array): Uint8Array { - return window.crypto.getRandomValues(dataBuffer); - } - - async generateKey( - algorithm: RsaHashedKeyGenParams, - extractable: boolean, - keyUsages: KeyUsage[] - ): Promise { - return window.crypto.subtle.generateKey( - algorithm, - extractable, - keyUsages - ) as Promise; - } - - async exportKey(key: CryptoKey): Promise { - return window.crypto.subtle.exportKey( - KEY_FORMAT_JWK, - key - ) as Promise; - } - - async importKey( - keyData: JsonWebKey, - algorithm: RsaHashedImportParams, - extractable: boolean, - keyUsages: KeyUsage[] - ): Promise { - return window.crypto.subtle.importKey( - KEY_FORMAT_JWK, - keyData, - algorithm, - extractable, - keyUsages - ) as Promise; - } - - async sign( - algorithm: AlgorithmIdentifier, - key: CryptoKey, - data: ArrayBuffer - ): Promise { - return window.crypto.subtle.sign( - algorithm, - key, - data - ) as Promise; - } - - async digest( - algorithm: AlgorithmIdentifier, - data: Uint8Array - ): Promise { - return window.crypto.subtle.digest( - algorithm, - data - ) as Promise; - } -} diff --git a/lib/msal-browser/src/crypto/PkceGenerator.ts b/lib/msal-browser/src/crypto/PkceGenerator.ts index 703f11c3ad..1fa73cbe6d 100644 --- a/lib/msal-browser/src/crypto/PkceGenerator.ts +++ b/lib/msal-browser/src/crypto/PkceGenerator.ts @@ -3,74 +3,113 @@ * Licensed under the MIT License. */ -import { PkceCodes } from "@azure/msal-common"; +import { + IPerformanceClient, + Logger, + PerformanceEvents, + PkceCodes, + invoke, + invokeAsync, +} from "@azure/msal-common"; import { createBrowserAuthError, BrowserAuthErrorCodes, } from "../error/BrowserAuthError"; import { urlEncodeArr } from "../encode/Base64Encode"; -import { BrowserCrypto } from "./BrowserCrypto"; +import { getRandomValues, sha256Digest } from "./BrowserCrypto"; // Constant byte array length const RANDOM_BYTE_ARR_LENGTH = 32; /** - * Class which exposes APIs to generate PKCE codes and code verifiers. + * This file defines APIs to generate PKCE codes and code verifiers. */ -export class PkceGenerator { - private cryptoObj: BrowserCrypto; - - constructor(cryptoObj: BrowserCrypto) { - this.cryptoObj = cryptoObj; - } - /** - * Generates PKCE Codes. See the RFC for more information: https://tools.ietf.org/html/rfc7636 - */ - async generateCodes(): Promise { - const codeVerifier = this.generateCodeVerifier(); - const codeChallenge = await this.generateCodeChallengeFromVerifier( - codeVerifier - ); - return { - verifier: codeVerifier, - challenge: codeChallenge, - }; - } +/** + * Generates PKCE Codes. See the RFC for more information: https://tools.ietf.org/html/rfc7636 + */ +export async function generatePkceCodes( + performanceClient: IPerformanceClient, + logger: Logger, + correlationId: string +): Promise { + performanceClient.addQueueMeasurement( + PerformanceEvents.GeneratePkceCodes, + correlationId + ); + const codeVerifier = invoke( + generateCodeVerifier, + PerformanceEvents.GenerateCodeVerifier, + logger, + performanceClient, + correlationId + )(performanceClient, logger, correlationId); + const codeChallenge = await invokeAsync( + generateCodeChallengeFromVerifier, + PerformanceEvents.GenerateCodeChallengeFromVerifier, + logger, + performanceClient, + correlationId + )(codeVerifier, performanceClient, logger, correlationId); + return { + verifier: codeVerifier, + challenge: codeChallenge, + }; +} - /** - * Generates a random 32 byte buffer and returns the base64 - * encoded string to be used as a PKCE Code Verifier - */ - private generateCodeVerifier(): string { - try { - // Generate random values as utf-8 - const buffer: Uint8Array = new Uint8Array(RANDOM_BYTE_ARR_LENGTH); - this.cryptoObj.getRandomValues(buffer); - // encode verifier as base64 - const pkceCodeVerifierB64: string = urlEncodeArr(buffer); - return pkceCodeVerifierB64; - } catch (e) { - throw createBrowserAuthError(BrowserAuthErrorCodes.pkceNotCreated); - } +/** + * Generates a random 32 byte buffer and returns the base64 + * encoded string to be used as a PKCE Code Verifier + */ +function generateCodeVerifier( + performanceClient: IPerformanceClient, + logger: Logger, + correlationId: string +): string { + try { + // Generate random values as utf-8 + const buffer: Uint8Array = new Uint8Array(RANDOM_BYTE_ARR_LENGTH); + invoke( + getRandomValues, + PerformanceEvents.GetRandomValues, + logger, + performanceClient, + correlationId + )(buffer); + // encode verifier as base64 + const pkceCodeVerifierB64: string = urlEncodeArr(buffer); + return pkceCodeVerifierB64; + } catch (e) { + throw createBrowserAuthError(BrowserAuthErrorCodes.pkceNotCreated); } +} - /** - * Creates a base64 encoded PKCE Code Challenge string from the - * hash created from the PKCE Code Verifier supplied - */ - private async generateCodeChallengeFromVerifier( - pkceCodeVerifier: string - ): Promise { - try { - // hashed verifier - const pkceHashedCodeVerifier = await this.cryptoObj.sha256Digest( - pkceCodeVerifier - ); - // encode hash as base64 - return urlEncodeArr(new Uint8Array(pkceHashedCodeVerifier)); - } catch (e) { - throw createBrowserAuthError(BrowserAuthErrorCodes.pkceNotCreated); - } +/** + * Creates a base64 encoded PKCE Code Challenge string from the + * hash created from the PKCE Code Verifier supplied + */ +async function generateCodeChallengeFromVerifier( + pkceCodeVerifier: string, + performanceClient: IPerformanceClient, + logger: Logger, + correlationId: string +): Promise { + performanceClient.addQueueMeasurement( + PerformanceEvents.GenerateCodeChallengeFromVerifier, + correlationId + ); + try { + // hashed verifier + const pkceHashedCodeVerifier = await invokeAsync( + sha256Digest, + PerformanceEvents.Sha256Digest, + logger, + performanceClient, + correlationId + )(pkceCodeVerifier, performanceClient, correlationId); + // encode hash as base64 + return urlEncodeArr(new Uint8Array(pkceHashedCodeVerifier)); + } catch (e) { + throw createBrowserAuthError(BrowserAuthErrorCodes.pkceNotCreated); } } diff --git a/lib/msal-browser/src/event/EventHandler.ts b/lib/msal-browser/src/event/EventHandler.ts index 1cf072c4eb..46796d33d3 100644 --- a/lib/msal-browser/src/event/EventHandler.ts +++ b/lib/msal-browser/src/event/EventHandler.ts @@ -17,6 +17,7 @@ import { EventPayload, } from "./EventMessage"; import { EventType } from "./EventType"; +import { createNewGuid } from "../crypto/BrowserCrypto"; export class EventHandler { // Callback for subscribing to events @@ -40,7 +41,7 @@ export class EventHandler { */ addEventCallback(callback: EventCallbackFunction): string | null { if (typeof window !== "undefined") { - const callbackId = this.browserCrypto.createNewGuid(); + const callbackId = createNewGuid(); this.eventCallbacks.set(callbackId, callback); this.logger.verbose( `Event callback registered with id: ${callbackId}` diff --git a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts index b4cc6d957f..e758521c30 100644 --- a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts @@ -37,6 +37,7 @@ import { INavigationClient } from "../navigation/INavigationClient"; import { NativeMessageHandler } from "../broker/nativeBroker/NativeMessageHandler"; import { AuthenticationResult } from "../response/AuthenticationResult"; import { ClearCacheRequest } from "../request/ClearCacheRequest"; +import { createNewGuid } from "../crypto/BrowserCrypto"; export abstract class BaseInteractionClient { protected config: BrowserConfiguration; @@ -68,8 +69,7 @@ export abstract class BaseInteractionClient { this.eventHandler = eventHandler; this.navigationClient = navigationClient; this.nativeMessageHandler = nativeMessageHandler; - this.correlationId = - correlationId || this.browserCrypto.createNewGuid(); + this.correlationId = correlationId || createNewGuid(); this.logger = logger.clone( BrowserConstants.MSAL_SKU, version, diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 8117fcb143..96709eb4b7 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -41,6 +41,8 @@ import { BrowserUtils } from "../utils/BrowserUtils"; import { RedirectRequest } from "../request/RedirectRequest"; import { PopupRequest } from "../request/PopupRequest"; import { SsoSilentRequest } from "../request/SsoSilentRequest"; +import { generatePkceCodes } from "../crypto/PkceGenerator"; +import { createNewGuid } from "../crypto/BrowserCrypto"; /** * Defines the class structure and helper functions used by the "standard", non-brokered auth flows (popup, redirect, silent (RT), silent (iframe)) @@ -57,8 +59,13 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { PerformanceEvents.StandardInteractionClientInitializeAuthorizationCodeRequest, request.correlationId ); - const generatedPkceParams = - await this.browserCrypto.generatePkceCodes(); + const generatedPkceParams = await invokeAsync( + generatePkceCodes, + PerformanceEvents.GeneratePkceCodes, + this.logger, + this.performanceClient, + this.correlationId + )(this.performanceClient, this.logger, this.correlationId); const authCodeRequest: CommonAuthorizationCodeRequest = { ...request, @@ -86,8 +93,7 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { ); const validLogoutRequest: CommonEndSessionRequest = { - correlationId: - this.correlationId || this.browserCrypto.createNewGuid(), + correlationId: this.correlationId || createNewGuid(), ...logoutRequest, }; @@ -412,7 +418,7 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { ...baseRequest, redirectUri: redirectUri, state: state, - nonce: request.nonce || this.browserCrypto.createNewGuid(), + nonce: request.nonce || createNewGuid(), responseMode: this.config.auth.OIDCOptions .serverResponseType as ResponseMode, }; diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index ee4d695aa5..6a9c5d074e 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -64,6 +64,8 @@ import { NativeConstants, } from "../../src/utils/BrowserConstants"; import { CryptoOps } from "../../src/crypto/CryptoOps"; +import * as BrowserCrypto from "../../src/crypto/BrowserCrypto"; +import * as PkceGenerator from "../../src/crypto/PkceGenerator"; import { EventType } from "../../src/event/EventType"; import { SilentRequest } from "../../src/request/SilentRequest"; import { RedirectRequest } from "../../src/request/RedirectRequest"; @@ -148,7 +150,6 @@ function stubProvider(pca: PublicClientApplication) { pca.getLogger(), 2000, perfClient, - new CryptoOps(new Logger({})), "test-extensionId" ); }); @@ -181,6 +182,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { afterEach(() => { sinon.restore(); + jest.restoreAllMocks(); window.location.hash = ""; window.sessionStorage.clear(); window.localStorage.clear(); @@ -936,13 +938,13 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { ).toBeTruthy(); return Promise.resolve(done()); }); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); // @ts-ignore pca.loginRedirect(null); @@ -2417,9 +2419,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const silentClientSpy = sinon .stub(SilentIframeClient.prototype, "acquireToken") .resolves(testTokenResponse); @@ -2458,9 +2460,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const silentClientSpy = sinon .stub(SilentIframeClient.prototype, "acquireToken") .resolves(testTokenResponse); @@ -2503,9 +2505,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const silentClientSpy = sinon .stub(SilentIframeClient.prototype, "acquireToken") .rejects({ @@ -2856,9 +2858,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const silentClientSpy = sinon .stub(SilentAuthCodeClient.prototype, "acquireToken") .resolves(testTokenResponse); @@ -2900,9 +2902,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const silentClientSpy = sinon .stub(SilentAuthCodeClient.prototype, "acquireToken") .resolves(testTokenResponse); @@ -2956,9 +2958,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const silentClientSpy = sinon .stub(SilentAuthCodeClient.prototype, "acquireToken") .rejects({ @@ -3368,9 +3370,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); sinon .stub(CryptoOps.prototype, "hashString") .resolves(TEST_CRYPTO_VALUES.TEST_SHA256_HASH); @@ -3474,9 +3476,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); sinon .stub(CryptoOps.prototype, "hashString") .resolves(TEST_CRYPTO_VALUES.TEST_SHA256_HASH); @@ -3570,9 +3572,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); sinon .stub(CryptoOps.prototype, "hashString") .resolves(TEST_CRYPTO_VALUES.TEST_SHA256_HASH); @@ -3788,9 +3790,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { account: testAccount, tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); sinon .stub(CryptoOps.prototype, "hashString") .resolves(TEST_CRYPTO_VALUES.TEST_SHA256_HASH); @@ -4159,13 +4161,13 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { const silentTokenHelperStub = sinon .stub(SilentIframeClient.prototype, "silentTokenHelper") .resolves(testTokenResponse); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); sinon .stub(ProtocolUtils, "setRequestState") .returns(TEST_STATE_VALUES.TEST_STATE_SILENT); @@ -4245,9 +4247,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); sinon .stub(ProtocolUtils, "setRequestState") .returns(TEST_STATE_VALUES.TEST_STATE_SILENT); @@ -4321,9 +4323,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { .stub(SilentIframeClient.prototype, "acquireToken") .resolves(testTokenResponse); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const callbackId = pca.addPerformanceCallback((events) => { expect(events[0].correlationId).toBe(RANDOM_TEST_GUID); expect(events[0].success).toBe(true); @@ -4380,9 +4382,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { .stub(SilentIframeClient.prototype, "acquireToken") .resolves(testTokenResponse); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const callbackId = pca.addPerformanceCallback((events) => { expect(events[0].correlationId).toBe(RANDOM_TEST_GUID); expect(events[0].success).toBe(true); @@ -4450,9 +4452,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { tokenType: AuthenticationScheme.BEARER, }; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); sinon .stub(ProtocolUtils, "setRequestState") .returns(TEST_STATE_VALUES.TEST_STATE_SILENT); diff --git a/lib/msal-browser/test/broker/NativeMessageHandler.spec.ts b/lib/msal-browser/test/broker/NativeMessageHandler.spec.ts index 9c720945fa..80b6575d76 100644 --- a/lib/msal-browser/test/broker/NativeMessageHandler.spec.ts +++ b/lib/msal-browser/test/broker/NativeMessageHandler.spec.ts @@ -76,8 +76,7 @@ describe("NativeMessageHandler Tests", () => { const wamMessageHandler = await NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ); expect(wamMessageHandler).toBeInstanceOf(NativeMessageHandler); @@ -126,8 +125,7 @@ describe("NativeMessageHandler Tests", () => { NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ).then(() => { window.removeEventListener("message", eventHandler, true); }); @@ -164,8 +162,7 @@ describe("NativeMessageHandler Tests", () => { const wamMessageHandler = await NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ); expect(wamMessageHandler).toBeInstanceOf(NativeMessageHandler); @@ -176,8 +173,7 @@ describe("NativeMessageHandler Tests", () => { NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ).catch((e) => { expect(e).toBeInstanceOf(BrowserAuthError); expect(e.errorCode).toBe( @@ -200,8 +196,7 @@ describe("NativeMessageHandler Tests", () => { NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ) .catch((e) => { expect(e).toBeInstanceOf(BrowserAuthError); @@ -239,8 +234,7 @@ describe("NativeMessageHandler Tests", () => { NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ).catch(() => { if (callbackDone) { done(); @@ -296,8 +290,7 @@ describe("NativeMessageHandler Tests", () => { const wamMessageHandler = await NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ); expect(wamMessageHandler).toBeInstanceOf(NativeMessageHandler); @@ -354,8 +347,7 @@ describe("NativeMessageHandler Tests", () => { NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ) .then((wamMessageHandler) => { wamMessageHandler @@ -421,8 +413,7 @@ describe("NativeMessageHandler Tests", () => { NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ) .then((wamMessageHandler) => { wamMessageHandler @@ -486,8 +477,7 @@ describe("NativeMessageHandler Tests", () => { NativeMessageHandler.createProvider( new Logger({}), 2000, - performanceClient, - cryptoInterface + performanceClient ) .then((wamMessageHandler) => { wamMessageHandler diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index c95c8218fc..1cae423469 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -1,14 +1,15 @@ import { CryptoOps } from "../../src/crypto/CryptoOps"; -import { BrowserCrypto } from "../../src/crypto/BrowserCrypto"; +import * as BrowserCrypto from "../../src/crypto/BrowserCrypto"; import { createHash } from "crypto"; import { PkceCodes, BaseAuthRequest, Logger } from "@azure/msal-common"; -import { TEST_URIS } from "../utils/StringConstants"; +import { RANDOM_TEST_GUID, TEST_URIS } from "../utils/StringConstants"; import { createBrowserAuthError, BrowserAuthErrorCodes, } from "../../src/error/BrowserAuthError"; -import { ModernBrowserCrypto } from "../../src/crypto/ModernBrowserCrypto"; import { DatabaseStorage } from "../../src/cache/DatabaseStorage"; +import { generatePkceCodes } from "../../src/crypto/PkceGenerator"; +import { StubPerformanceClient } from "@azure/msal-common"; let mockDatabase = { "TestDB.keys": {}, @@ -16,10 +17,8 @@ let mockDatabase = { describe("CryptoOps.ts Unit Tests", () => { let cryptoObj: CryptoOps; - let browserCrypto: BrowserCrypto; beforeEach(() => { - browserCrypto = new BrowserCrypto(new Logger({})); cryptoObj = new CryptoOps(new Logger({})); // Mock DatabaseStorage @@ -228,9 +227,8 @@ describe("CryptoOps.ts Unit Tests", () => { }); it("generatePkceCode() creates a valid Pkce code", async () => { - //@ts-ignore jest.spyOn( - BrowserCrypto.prototype as any, + BrowserCrypto, "sha256Digest" // @ts-ignore ).mockImplementation((data: Uint8Array): Promise => { @@ -243,16 +241,19 @@ describe("CryptoOps.ts Unit Tests", () => { * Contains alphanumeric, dash '-', underscore '_', plus '+', or slash '/' with length of 43. */ const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); - const generatedCodes: PkceCodes = await cryptoObj.generatePkceCodes(); + const generatedCodes: PkceCodes = await generatePkceCodes( + new StubPerformanceClient(), + new Logger({}), + RANDOM_TEST_GUID + ); expect(regExp.test(generatedCodes.challenge)).toBe(true); expect(regExp.test(generatedCodes.verifier)).toBe(true); }); it("getPublicKeyThumbprint() generates a valid request thumbprint", async () => { jest.setTimeout(30000); - //@ts-ignore jest.spyOn( - BrowserCrypto.prototype as any, + BrowserCrypto, "sha256Digest" // @ts-ignore ).mockImplementation((data: Uint8Array): Promise => { @@ -260,11 +261,8 @@ describe("CryptoOps.ts Unit Tests", () => { createHash("SHA256").update(Buffer.from(data)).digest() ); }); - const generateKeyPairSpy = jest.spyOn( - BrowserCrypto.prototype, - "generateKeyPair" - ); - const exportJwkSpy = jest.spyOn(BrowserCrypto.prototype, "exportJwk"); + const generateKeyPairSpy = jest.spyOn(BrowserCrypto, "generateKeyPair"); + const exportJwkSpy = jest.spyOn(BrowserCrypto, "exportJwk"); const pkThumbprint = await cryptoObj.getPublicKeyThumbprint({ resourceRequestMethod: "POST", resourceRequestUri: TEST_URIS.TEST_AUTH_ENDPT_WITH_PARAMS, @@ -284,9 +282,8 @@ describe("CryptoOps.ts Unit Tests", () => { }, 30000); it("removeTokenBindingKey() removes the specified key from storage", async () => { - //@ts-ignore jest.spyOn( - BrowserCrypto.prototype as any, + BrowserCrypto, "sha256Digest" // @ts-ignore ).mockImplementation((data: Uint8Array): Promise => { @@ -312,8 +309,7 @@ describe("CryptoOps.ts Unit Tests", () => { }, 30000); it("hashString() returns a valid SHA-256 hash of an input string", async () => { - //@ts-ignore - jest.spyOn(BrowserCrypto.prototype, "sha256Digest").mockImplementation( + jest.spyOn(BrowserCrypto, "sha256Digest").mockImplementation( // @ts-ignore (data: Uint8Array): Promise => { return Promise.resolve( @@ -325,23 +321,12 @@ describe("CryptoOps.ts Unit Tests", () => { const result = await cryptoObj.hashString("testString"); expect(regExp.test(result)).toBe(true); }); + it("throws if crypto is unavailable", () => { + const mockedWindow = window; + //@ts-ignore + delete mockedWindow.crypto; + jest.spyOn(global, "window", "get").mockReturnValue(mockedWindow); - describe("Browser Crypto Interfaces", () => { - it("uses modern crypto if available", () => { - const crypto = new BrowserCrypto(new Logger({})); - // @ts-ignore - expect(crypto.subtleCrypto).toBeInstanceOf(ModernBrowserCrypto); - }); - - it("throws if crypto is unavailable", () => { - jest.spyOn( - BrowserCrypto.prototype, - // @ts-ignore - "hasBrowserCrypto" - // @ts-ignore - ).mockReturnValue(false); - - expect(() => new BrowserCrypto(new Logger({}))).toThrow(); - }); + expect(() => new CryptoOps(new Logger({}))).toThrow(); }); }); diff --git a/lib/msal-browser/test/crypto/PkceGenerator.spec.ts b/lib/msal-browser/test/crypto/PkceGenerator.spec.ts index 4453fd798c..7fe88886e4 100644 --- a/lib/msal-browser/test/crypto/PkceGenerator.spec.ts +++ b/lib/msal-browser/test/crypto/PkceGenerator.spec.ts @@ -1,8 +1,10 @@ -import { BrowserCrypto } from "../../src/crypto/BrowserCrypto"; import { createHash } from "crypto"; -import { PkceGenerator } from "../../src/crypto/PkceGenerator"; -import { Logger, PkceCodes } from "@azure/msal-common"; -import { NUM_TESTS } from "../utils/StringConstants"; +import { PkceCodes } from "@azure/msal-common"; +import { NUM_TESTS, RANDOM_TEST_GUID } from "../utils/StringConstants"; +import * as BrowserCrypto from "../../src/crypto/BrowserCrypto"; +import { generatePkceCodes } from "../../src/crypto/PkceGenerator"; +import { StubPerformanceClient } from "@azure/msal-common"; +import { Logger } from "@azure/msal-common"; describe("PkceGenerator.ts Unit Tests", () => { afterEach(() => { @@ -10,8 +12,7 @@ describe("PkceGenerator.ts Unit Tests", () => { }); it("generateCodes() generates valid pkce codes", async () => { - //@ts-ignore - jest.spyOn(BrowserCrypto.prototype, "sha256Digest").mockImplementation( + jest.spyOn(BrowserCrypto, "sha256Digest").mockImplementation( //@ts-ignore (data: Uint8Array): Promise => { return Promise.resolve( @@ -19,16 +20,16 @@ describe("PkceGenerator.ts Unit Tests", () => { ); } ); - const browserCrypto = new BrowserCrypto(new Logger({})); - - const pkceGenerator = new PkceGenerator(browserCrypto); /** * Contains alphanumeric, dash '-', underscore '_', plus '+', or slash '/' with length of 43. */ const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); for (let i = 0; i < NUM_TESTS; i++) { - const generatedCodes: PkceCodes = - await pkceGenerator.generateCodes(); + const generatedCodes: PkceCodes = await generatePkceCodes( + new StubPerformanceClient(), + new Logger({}), + RANDOM_TEST_GUID + ); expect(regExp.test(generatedCodes.challenge)).toBe(true); expect(regExp.test(generatedCodes.verifier)).toBe(true); } diff --git a/lib/msal-browser/test/crypto/SignedHttpRequest.spec.ts b/lib/msal-browser/test/crypto/SignedHttpRequest.spec.ts index 422484f597..8a4f4d8241 100644 --- a/lib/msal-browser/test/crypto/SignedHttpRequest.spec.ts +++ b/lib/msal-browser/test/crypto/SignedHttpRequest.spec.ts @@ -1,6 +1,5 @@ import { SignedHttpRequest } from "../../src/crypto/SignedHttpRequest"; -import { BrowserCrypto } from "../../src/crypto/BrowserCrypto"; -import { CryptoOps } from "../../src/crypto/CryptoOps"; +import * as BrowserCrypto from "../../src/crypto/BrowserCrypto"; import { createHash } from "crypto"; import { AuthToken, Logger } from "@azure/msal-common"; import { DatabaseStorage } from "../../src/cache/DatabaseStorage"; @@ -14,7 +13,7 @@ describe("SignedHttpRequest.ts Unit Tests", () => { jest.setTimeout(30000); beforeEach(() => { - jest.spyOn(BrowserCrypto.prototype, "sha256Digest").mockImplementation( + jest.spyOn(BrowserCrypto, "sha256Digest").mockImplementation( (): Promise => { return Promise.resolve( createHash("SHA256") diff --git a/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts index 7f965c9a84..e236ff7558 100644 --- a/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts @@ -124,8 +124,7 @@ describe("NativeInteractionClient Tests", () => { wamProvider = new NativeMessageHandler( pca.getLogger(), 2000, - getDefaultPerformanceClient(), - new CryptoOps(new Logger({})) + getDefaultPerformanceClient() ); nativeInteractionClient = new NativeInteractionClient( diff --git a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index 7984347f6a..2a16b18de8 100644 --- a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts @@ -47,7 +47,8 @@ import { ApiId, BrowserConstants, } from "../../src/utils/BrowserConstants"; -import { CryptoOps } from "../../src/crypto/CryptoOps"; +import * as BrowserCrypto from "../../src/crypto/BrowserCrypto"; +import * as PkceGenerator from "../../src/crypto/PkceGenerator"; import { NavigationClient } from "../../src/navigation/NavigationClient"; import { EndSessionPopupRequest } from "../../src/request/EndSessionPopupRequest"; import { PopupClient } from "../../src/interaction_client/PopupClient"; @@ -217,7 +218,7 @@ describe("PopupClient", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -266,7 +267,7 @@ describe("PopupClient", () => { pca.nativeInternalStorage ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -380,19 +381,18 @@ describe("PopupClient", () => { sinon .stub(NativeInteractionClient.prototype, "acquireToken") .resolves(testTokenResponse); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const nativeMessageHandler = new NativeMessageHandler( //@ts-ignore pca.logger, 2000, - getDefaultPerformanceClient(), - new CryptoOps(new Logger({})) + getDefaultPerformanceClient() ); //@ts-ignore popupClient = new PopupClient( @@ -491,13 +491,13 @@ describe("PopupClient", () => { sinon .stub(NativeInteractionClient.prototype, "acquireToken") .resolves(testTokenResponse); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); //@ts-ignore popupClient = new PopupClient( //@ts-ignore @@ -597,13 +597,13 @@ describe("PopupClient", () => { "handleCodeResponseFromHash" ) .resolves(testTokenResponse); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const tokenResp = await popupClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: TEST_CONFIG.DEFAULT_SCOPES, @@ -627,6 +627,13 @@ describe("PopupClient", () => { NetworkManager.prototype, "sendPostRequest" ).mockResolvedValue(TEST_TOKEN_RESPONSE); + jest.spyOn( + PkceGenerator, + "generatePkceCodes" + ).mockResolvedValue({ + challenge: TEST_CONFIG.TEST_CHALLENGE, + verifier: TEST_CONFIG.TEST_VERIFIER, + }); }); it("does not store idToken if storeInCache.idToken = false", async () => { @@ -716,13 +723,13 @@ describe("PopupClient", () => { sinon .stub(PopupClient.prototype, "initiateAuthRequest") .throws(testError); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); try { await popupClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, diff --git a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts index 8e39a4fc9c..ca71bf4d69 100644 --- a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts @@ -65,6 +65,8 @@ import { } from "../../src/error/BrowserAuthError"; import { RedirectHandler } from "../../src/interaction_handler/RedirectHandler"; import { CryptoOps } from "../../src/crypto/CryptoOps"; +import * as BrowserCrypto from "../../src/crypto/BrowserCrypto"; +import * as PkceGenerator from "../../src/crypto/PkceGenerator"; import { BrowserCacheManager } from "../../src/cache/BrowserCacheManager"; import { RedirectRequest } from "../../src/request/RedirectRequest"; import { NavigationClient } from "../../src/navigation/NavigationClient"; @@ -122,9 +124,9 @@ describe("RedirectClient", () => { pca = (pca as any).controller; await pca.initialize(); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); // @ts-ignore browserStorage = pca.browserStorage; @@ -1727,7 +1729,7 @@ describe("RedirectClient", () => { Promise.reject(err); } }); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -1759,7 +1761,7 @@ describe("RedirectClient", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -1830,7 +1832,7 @@ describe("RedirectClient", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -1944,7 +1946,7 @@ describe("RedirectClient", () => { loginHint: testIdTokenClaims.preferred_username || "", }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2035,7 +2037,7 @@ describe("RedirectClient", () => { account: testAccount, }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2104,7 +2106,7 @@ describe("RedirectClient", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2173,7 +2175,7 @@ describe("RedirectClient", () => { browserCrypto, testLogger ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2233,7 +2235,7 @@ describe("RedirectClient", () => { AuthorizationCodeClient.prototype, "getAuthCodeUrl" ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2305,7 +2307,7 @@ describe("RedirectClient", () => { AuthorizationCodeClient.prototype, "getAuthCodeUrl" ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2378,7 +2380,7 @@ describe("RedirectClient", () => { AuthorizationCodeClient.prototype, "getAuthCodeUrl" ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2451,7 +2453,7 @@ describe("RedirectClient", () => { AuthorizationCodeClient.prototype, "getAuthCodeUrl" ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2512,7 +2514,7 @@ describe("RedirectClient", () => { AuthorizationCodeClient.prototype, "getAuthCodeUrl" ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2563,7 +2565,7 @@ describe("RedirectClient", () => { expect(navigateUrl).toEqual(testNavUrl); return Promise.resolve(done()); }); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2605,7 +2607,7 @@ describe("RedirectClient", () => { return Promise.resolve(); } ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2631,7 +2633,7 @@ describe("RedirectClient", () => { authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2692,7 +2694,7 @@ describe("RedirectClient", () => { authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2759,7 +2761,7 @@ describe("RedirectClient", () => { browserCrypto, testLogger ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2820,7 +2822,7 @@ describe("RedirectClient", () => { AuthorizationCodeClient.prototype, "getAuthCodeUrl" ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2883,7 +2885,7 @@ describe("RedirectClient", () => { AuthorizationCodeClient.prototype, "getAuthCodeUrl" ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); @@ -2939,6 +2941,13 @@ describe("RedirectClient", () => { NetworkManager.prototype, "sendPostRequest" ).mockResolvedValue(TEST_TOKEN_RESPONSE); + jest.spyOn( + PkceGenerator, + "generatePkceCodes" + ).mockResolvedValue({ + challenge: TEST_CONFIG.TEST_CHALLENGE, + verifier: TEST_CONFIG.TEST_VERIFIER, + }); }); it("does not store idToken if storeInCache.idToken = false", async () => { diff --git a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts index 0b42e50454..4acb3fb023 100644 --- a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts @@ -36,7 +36,8 @@ import { BrowserAuthErrorCodes, } from "../../src/error/BrowserAuthError"; import { SilentHandler } from "../../src/interaction_handler/SilentHandler"; -import { CryptoOps } from "../../src/crypto/CryptoOps"; +import * as BrowserCrypto from "../../src/crypto/BrowserCrypto"; +import * as PkceGenerator from "../../src/crypto/PkceGenerator"; import { SilentIframeClient } from "../../src/interaction_client/SilentIframeClient"; import { BrowserCacheManager } from "../../src/cache/BrowserCacheManager"; import { ApiId, AuthenticationResult } from "../../src"; @@ -87,6 +88,7 @@ describe("SilentIframeClient", () => { afterEach(() => { sinon.restore(); + jest.restoreAllMocks(); window.location.hash = ""; window.sessionStorage.clear(); window.localStorage.clear(); @@ -128,13 +130,13 @@ describe("SilentIframeClient", () => { BrowserAuthErrorCodes.monitorWindowTimeout ) ); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const telemetryStub = sinon .stub(ServerTelemetryManager.prototype, "cacheFailedRequest") .callsFake((e) => { @@ -172,13 +174,13 @@ describe("SilentIframeClient", () => { sinon .stub(SilentHandler.prototype, "monitorIframeForHash") .rejects(testError); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); silentIframeClient .acquireToken({ @@ -248,13 +250,13 @@ describe("SilentIframeClient", () => { sinon .stub(SilentHandler.prototype, "handleCodeResponseFromHash") .resolves(testTokenResponse); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint", @@ -320,13 +322,13 @@ describe("SilentIframeClient", () => { sinon .stub(SilentHandler.prototype, "handleCodeResponseFromHash") .resolves(testTokenResponse); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: TEST_CONFIG.DEFAULT_SCOPES, @@ -434,13 +436,13 @@ describe("SilentIframeClient", () => { sinon .stub(NativeInteractionClient.prototype, "acquireToken") .resolves(testTokenResponse); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: TEST_CONFIG.DEFAULT_SCOPES, @@ -536,13 +538,13 @@ describe("SilentIframeClient", () => { sinon .stub(NativeInteractionClient.prototype, "acquireToken") .resolves(testTokenResponse); - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); silentIframeClient .acquireToken({ scopes: TEST_CONFIG.DEFAULT_SCOPES, @@ -573,6 +575,13 @@ describe("SilentIframeClient", () => { NetworkManager.prototype, "sendPostRequest" ).mockResolvedValue(TEST_TOKEN_RESPONSE); + jest.spyOn( + PkceGenerator, + "generatePkceCodes" + ).mockResolvedValue({ + challenge: TEST_CONFIG.TEST_CHALLENGE, + verifier: TEST_CONFIG.TEST_VERIFIER, + }); }); it("does not store idToken if storeInCache.idToken = false", async () => { diff --git a/lib/msal-browser/test/interaction_client/SilentRefreshClient.spec.ts b/lib/msal-browser/test/interaction_client/SilentRefreshClient.spec.ts index ea302668df..2a0d12decf 100644 --- a/lib/msal-browser/test/interaction_client/SilentRefreshClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/SilentRefreshClient.spec.ts @@ -25,7 +25,7 @@ import { RefreshTokenEntity, AccountEntity, } from "@azure/msal-common"; -import { CryptoOps } from "../../src/crypto/CryptoOps"; +import * as BrowserCrypto from "../../src/crypto/BrowserCrypto"; import { createBrowserAuthError, BrowserAuthErrorCodes, @@ -68,9 +68,9 @@ describe("SilentRefreshClient", () => { //@ts-ignore browserCacheManager = pca.browserStorage; - sinon - .stub(CryptoOps.prototype, "createNewGuid") - .returns(RANDOM_TEST_GUID); + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); // @ts-ignore silentRefreshClient = new SilentRefreshClient( //@ts-ignore diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index dda0c96e14..9f85f30cb4 100644 --- a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts @@ -25,7 +25,7 @@ import { } from "../utils/StringConstants"; import { AuthorizationUrlRequest } from "../../src/request/AuthorizationUrlRequest"; import { RedirectRequest } from "../../src/request/RedirectRequest"; -import { CryptoOps } from "../../src/crypto/CryptoOps"; +import * as PkceGenerator from "../../src/crypto/PkceGenerator"; import { FetchClient } from "../../src/network/FetchClient"; import { InteractionType } from "../../src/utils/BrowserConstants"; @@ -114,6 +114,7 @@ describe("StandardInteractionClient", () => { }); afterEach(() => { + jest.restoreAllMocks(); sinon.restore(); }); @@ -131,7 +132,7 @@ describe("StandardInteractionClient", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, }; - sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({ + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER, }); diff --git a/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts b/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts index 45424806a4..57ce011e2c 100644 --- a/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts @@ -145,9 +145,6 @@ describe("RedirectHandler.ts Unit Tests", () => { base64Encode: (input: string): string => { return "testEncodedString"; }, - generatePkceCodes: async (): Promise => { - return testPkceCodes; - }, getPublicKeyThumbprint: async (): Promise => { return TEST_POP_VALUES.ENCODED_REQ_CNF; }, diff --git a/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts b/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts index 25bbf17d08..ae3e5b4d0f 100644 --- a/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts @@ -122,9 +122,6 @@ describe("SilentHandler.ts Unit Tests", () => { base64Encode: (input: string): string => { return "testEncodedString"; }, - generatePkceCodes: async (): Promise => { - return testPkceCodes; - }, getPublicKeyThumbprint: async (): Promise => { return TEST_POP_VALUES.ENCODED_REQ_CNF; }, diff --git a/lib/msal-common/src/crypto/ICrypto.ts b/lib/msal-common/src/crypto/ICrypto.ts index 7b72cbf309..61d875831f 100644 --- a/lib/msal-common/src/crypto/ICrypto.ts +++ b/lib/msal-common/src/crypto/ICrypto.ts @@ -45,10 +45,6 @@ export interface ICrypto { * @param input */ base64Decode(input: string): string; - /** - * Generate PKCE codes for OAuth. See RFC here: https://tools.ietf.org/html/rfc7636 - */ - generatePkceCodes(): Promise; /** * Generates an JWK RSA S256 Thumbprint * @param request @@ -91,9 +87,6 @@ export const DEFAULT_CRYPTO_IMPLEMENTATION: ICrypto = { base64Encode: (): string => { throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); }, - async generatePkceCodes(): Promise { - throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); - }, async getPublicKeyThumbprint(): Promise { throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); }, diff --git a/lib/msal-common/src/telemetry/performance/PerformanceEvent.ts b/lib/msal-common/src/telemetry/performance/PerformanceEvent.ts index 5ced64c2e5..8d5fb298e6 100644 --- a/lib/msal-common/src/telemetry/performance/PerformanceEvent.ts +++ b/lib/msal-common/src/telemetry/performance/PerformanceEvent.ts @@ -268,6 +268,15 @@ export const PerformanceEvents = { * Cache operations */ ClearTokensAndKeysWithClaims: "clearTokensAndKeysWithClaims", + + /** + * Crypto Operations + */ + GeneratePkceCodes: "generatePkceCodes", + GenerateCodeVerifier: "generateCodeVerifier", + GenerateCodeChallengeFromVerifier: "generateCodeChallengeFromVerifier", + Sha256Digest: "sha256Digest", + GetRandomValues: "getRandomValues", } as const; export type PerformanceEvents = (typeof PerformanceEvents)[keyof typeof PerformanceEvents]; diff --git a/lib/msal-common/src/utils/FunctionWrappers.ts b/lib/msal-common/src/utils/FunctionWrappers.ts index fb825724ff..c780b3257a 100644 --- a/lib/msal-common/src/utils/FunctionWrappers.ts +++ b/lib/msal-common/src/utils/FunctionWrappers.ts @@ -31,7 +31,6 @@ export const invoke = , U>( eventName, correlationId ); - telemetryClient?.setPreQueueTime(eventName, correlationId); try { const result = callback(...args); inProgressEvent?.end({ diff --git a/lib/msal-common/test/account/AuthToken.spec.ts b/lib/msal-common/test/account/AuthToken.spec.ts index 4c882b1504..93458535b0 100644 --- a/lib/msal-common/test/account/AuthToken.spec.ts +++ b/lib/msal-common/test/account/AuthToken.spec.ts @@ -1,6 +1,5 @@ import * as AuthToken from "../../src/account/AuthToken"; import { - TEST_CONFIG, TEST_DATA_CLIENT_INFO, RANDOM_TEST_GUID, TEST_URIS, @@ -8,7 +7,7 @@ import { TEST_CRYPTO_VALUES, TEST_TOKENS, } from "../test_kit/StringConstants"; -import { PkceCodes, ICrypto } from "../../src/crypto/ICrypto"; +import { ICrypto } from "../../src/crypto/ICrypto"; import { ClientAuthErrorMessage, ClientAuthError, @@ -62,12 +61,6 @@ describe("AuthToken.ts Class Unit Tests", () => { return input; } }, - async generatePkceCodes(): Promise { - return { - challenge: TEST_CONFIG.TEST_CHALLENGE, - verifier: TEST_CONFIG.TEST_VERIFIER, - }; - }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/account/ClientInfo.spec.ts b/lib/msal-common/test/account/ClientInfo.spec.ts index e66a56f4a0..daada80e54 100644 --- a/lib/msal-common/test/account/ClientInfo.spec.ts +++ b/lib/msal-common/test/account/ClientInfo.spec.ts @@ -4,13 +4,12 @@ import { ClientInfo, } from "../../src/account/ClientInfo"; import { - TEST_CONFIG, TEST_DATA_CLIENT_INFO, RANDOM_TEST_GUID, TEST_POP_VALUES, TEST_CRYPTO_VALUES, } from "../test_kit/StringConstants"; -import { PkceCodes, ICrypto } from "../../src/crypto/ICrypto"; +import { ICrypto } from "../../src/crypto/ICrypto"; import { ClientAuthError, ClientAuthErrorCodes, @@ -49,12 +48,6 @@ describe("ClientInfo.ts Class Unit Tests", () => { return input; } }, - async generatePkceCodes(): Promise { - return { - challenge: TEST_CONFIG.TEST_CHALLENGE, - verifier: TEST_CONFIG.TEST_VERIFIER, - }; - }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/cache/entities/AccountEntity.spec.ts b/lib/msal-common/test/cache/entities/AccountEntity.spec.ts index 5a543535c7..63cc2bf229 100644 --- a/lib/msal-common/test/cache/entities/AccountEntity.spec.ts +++ b/lib/msal-common/test/cache/entities/AccountEntity.spec.ts @@ -7,11 +7,10 @@ import { NetworkRequestOptions, INetworkModule, } from "../../../src/network/INetworkModule"; -import { ICrypto, PkceCodes } from "../../../src/crypto/ICrypto"; +import { ICrypto } from "../../../src/crypto/ICrypto"; import { RANDOM_TEST_GUID, TEST_DATA_CLIENT_INFO, - TEST_CONFIG, TEST_TOKENS, TEST_URIS, TEST_POP_VALUES, @@ -55,12 +54,6 @@ const cryptoInterface: ICrypto = { return input; } }, - async generatePkceCodes(): Promise { - return { - challenge: TEST_CONFIG.TEST_CHALLENGE, - verifier: TEST_CONFIG.TEST_VERIFIER, - }; - }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/client/ClientTestUtils.ts b/lib/msal-common/test/client/ClientTestUtils.ts index 7a91792623..948d2c9eaf 100644 --- a/lib/msal-common/test/client/ClientTestUtils.ts +++ b/lib/msal-common/test/client/ClientTestUtils.ts @@ -6,7 +6,6 @@ import { ClientConfiguration, Constants, - PkceCodes, AccountEntity, AppMetadataEntity, ThrottlingEntity, @@ -220,12 +219,6 @@ export const mockCrypto = { base64Encode(input: string): string { return Buffer.from(input, "utf-8").toString("base64"); }, - async generatePkceCodes(): Promise { - return { - challenge: TEST_CONFIG.TEST_CHALLENGE, - verifier: TEST_CONFIG.TEST_VERIFIER, - }; - }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/config/ClientConfiguration.spec.ts b/lib/msal-common/test/config/ClientConfiguration.spec.ts index 368f77e1e8..d58fb6b275 100644 --- a/lib/msal-common/test/config/ClientConfiguration.spec.ts +++ b/lib/msal-common/test/config/ClientConfiguration.spec.ts @@ -2,7 +2,6 @@ import { CommonClientConfiguration, buildClientConfiguration, } from "../../src/config/ClientConfiguration"; -import { PkceCodes } from "../../src/crypto/ICrypto"; import { AuthError } from "../../src/error/AuthError"; import { NetworkRequestOptions } from "../../src/network/INetworkModule"; import { Logger, LogLevel } from "../../src/logger/Logger"; @@ -47,12 +46,6 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { expect(() => emptyConfig.cryptoInterface.base64Encode("test input") ).toThrowError(AuthError); - expect(emptyConfig.cryptoInterface.generatePkceCodes).not.toBeNull(); - await expect( - emptyConfig.cryptoInterface.generatePkceCodes() - ).rejects.toMatchObject( - createClientAuthError(ClientAuthErrorCodes.methodNotImplemented) - ); // Storage interface checks expect(emptyConfig.storageInterface).not.toBeNull(); expect(emptyConfig.storageInterface.clear).not.toBeNull(); @@ -148,11 +141,6 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { new Logger({}) ); - const testPkceCodes = { - challenge: "TestChallenge", - verifier: "TestVerifier", - } as PkceCodes; - const testNetworkResult = { testParam: "testValue", }; @@ -173,9 +161,6 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { base64Encode: (input: string): string => { return "testEncodedString"; }, - generatePkceCodes: async (): Promise => { - return testPkceCodes; - }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, @@ -242,10 +227,6 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { expect(newConfig.cryptoInterface.base64Encode("testString")).toBe( "testEncodedString" ); - expect(newConfig.cryptoInterface.generatePkceCodes).not.toBeNull(); - expect(newConfig.cryptoInterface.generatePkceCodes()).resolves.toBe( - testPkceCodes - ); expect(newConfig.cryptoInterface.removeTokenBindingKey).not.toBeNull(); expect( newConfig.cryptoInterface.removeTokenBindingKey("testString") diff --git a/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts b/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts index 5c50f7e7c2..9369d93e30 100644 --- a/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts +++ b/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts @@ -8,7 +8,7 @@ import { TEST_CRYPTO_VALUES, } from "../test_kit/StringConstants"; import { PopTokenGenerator } from "../../src/crypto/PopTokenGenerator"; -import { ICrypto, PkceCodes } from "../../src/crypto/ICrypto"; +import { ICrypto } from "../../src/crypto/ICrypto"; import { BaseAuthRequest } from "../../src/request/BaseAuthRequest"; import { TimeUtils } from "../../src/utils/TimeUtils"; import { UrlString } from "../../src/url/UrlString"; @@ -51,12 +51,6 @@ describe("PopTokenGenerator Unit Tests", () => { return input; } }, - async generatePkceCodes(): Promise { - return { - challenge: TEST_CONFIG.TEST_CHALLENGE, - verifier: TEST_CONFIG.TEST_VERIFIER, - }; - }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/response/ResponseHandler.spec.ts b/lib/msal-common/test/response/ResponseHandler.spec.ts index 4cf78b024d..26564a15a0 100644 --- a/lib/msal-common/test/response/ResponseHandler.spec.ts +++ b/lib/msal-common/test/response/ResponseHandler.spec.ts @@ -20,7 +20,7 @@ import { INetworkModule, NetworkRequestOptions, } from "../../src/network/INetworkModule"; -import { ICrypto, PkceCodes } from "../../src/crypto/ICrypto"; +import { ICrypto } from "../../src/crypto/ICrypto"; import { ServerAuthorizationCodeResponse } from "../../src/response/ServerAuthorizationCodeResponse"; import { MockStorageClass } from "../client/ClientTestUtils"; import { TokenClaims } from "../../src/account/TokenClaims"; @@ -79,12 +79,6 @@ const cryptoInterface: ICrypto = { return input; } }, - async generatePkceCodes(): Promise { - return { - challenge: TEST_CONFIG.TEST_CHALLENGE, - verifier: TEST_CONFIG.TEST_VERIFIER, - }; - }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/utils/ProtocolUtils.spec.ts b/lib/msal-common/test/utils/ProtocolUtils.spec.ts index 8bf698766d..814a455d4c 100644 --- a/lib/msal-common/test/utils/ProtocolUtils.spec.ts +++ b/lib/msal-common/test/utils/ProtocolUtils.spec.ts @@ -1,11 +1,10 @@ import { ProtocolUtils } from "../../src/utils/ProtocolUtils"; import { RANDOM_TEST_GUID, - TEST_CONFIG, TEST_CRYPTO_VALUES, TEST_POP_VALUES, } from "../test_kit/StringConstants"; -import { ICrypto, PkceCodes } from "../../src/crypto/ICrypto"; +import { ICrypto } from "../../src/crypto/ICrypto"; import { Constants } from "../../src/utils/Constants"; import sinon from "sinon"; import { @@ -45,12 +44,6 @@ describe("ProtocolUtils.ts Class Unit Tests", () => { return input; } }, - async generatePkceCodes(): Promise { - return { - challenge: TEST_CONFIG.TEST_CHALLENGE, - verifier: TEST_CONFIG.TEST_VERIFIER, - }; - }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-node/test/utils/TestConstants.ts b/lib/msal-node/test/utils/TestConstants.ts index 9a5f2b81f0..8d92acaec1 100644 --- a/lib/msal-node/test/utils/TestConstants.ts +++ b/lib/msal-node/test/utils/TestConstants.ts @@ -6,7 +6,6 @@ import { AccountInfo, ICrypto, - PkceCodes, AuthenticationResult, createClientAuthError, ClientAuthErrorCodes, @@ -105,9 +104,6 @@ export const DEFAULT_CRYPTO_IMPLEMENTATION: ICrypto = { base64Encode: (): string => { throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); }, - async generatePkceCodes(): Promise { - throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); - }, async getPublicKeyThumbprint(): Promise { throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); },