From 75ede548281026a8e0eb256713bf9a8002980e56 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Fri, 16 Apr 2021 13:58:22 -0700 Subject: [PATCH 01/44] Add KeyManager class --- lib/msal-common/src/crypto/ICrypto.ts | 10 +++++++++ lib/msal-common/src/crypto/KeyManager.ts | 27 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 lib/msal-common/src/crypto/KeyManager.ts diff --git a/lib/msal-common/src/crypto/ICrypto.ts b/lib/msal-common/src/crypto/ICrypto.ts index db96bcb4d4..07a1ed7089 100644 --- a/lib/msal-common/src/crypto/ICrypto.ts +++ b/lib/msal-common/src/crypto/ICrypto.ts @@ -49,6 +49,12 @@ export interface ICrypto { * @param accessToken */ signJwt(payload: SignedHttpRequest, kid: string): Promise; + /** + * Returns the public key from an asymmetric key pair stored in IndexedDB based on the + * public key thumbprint parameter + * @param keyThumbprint + */ + getAsymmetricPublicKey(keyThumbprint: string): Promise; } export const DEFAULT_CRYPTO_IMPLEMENTATION: ICrypto = { @@ -75,5 +81,9 @@ export const DEFAULT_CRYPTO_IMPLEMENTATION: ICrypto = { async signJwt(): Promise { const notImplErr = "Crypto interface - signJwt() has not been implemented"; throw AuthError.createUnexpectedError(notImplErr); + }, + async getAsymmetricPublicKey(): Promise { + const notImplErr = "Crypto interface - getAssymetricPublicKey() has not been implemented"; + throw AuthError.createUnexpectedError(notImplErr); } }; diff --git a/lib/msal-common/src/crypto/KeyManager.ts b/lib/msal-common/src/crypto/KeyManager.ts new file mode 100644 index 0000000000..432fbcc3ee --- /dev/null +++ b/lib/msal-common/src/crypto/KeyManager.ts @@ -0,0 +1,27 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { Logger } from "../logger/Logger"; +import { ICrypto } from "./ICrypto"; + +export class KeyManager { + private cryptoUtils: ICrypto; + private logger: Logger; + + constructor(cryptoUtils: ICrypto, logger: Logger) { + this.cryptoUtils = cryptoUtils; + this.logger = logger; + } + + /** + * Returns the public key from an asymmetric key pair stored in IndexedDB based on the + * public key thumbprint parameter + * @param keyThumbprint + * @returns Public Key JWK string + */ + async retrieveAsymmetricPublicKey(keyThumbprint: string): Promise { + return await this.cryptoUtils.getAsymmetricPublicKey(keyThumbprint); + } +} \ No newline at end of file From 09ac0e762bc875c86c807fb5f4a510062537165a Mon Sep 17 00:00:00 2001 From: HectorMg Date: Fri, 16 Apr 2021 14:02:55 -0700 Subject: [PATCH 02/44] Add STK JWK to BaseAuthRequest --- lib/msal-common/src/request/BaseAuthRequest.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/msal-common/src/request/BaseAuthRequest.ts b/lib/msal-common/src/request/BaseAuthRequest.ts index 97ccd05c2a..eb13a33952 100644 --- a/lib/msal-common/src/request/BaseAuthRequest.ts +++ b/lib/msal-common/src/request/BaseAuthRequest.ts @@ -14,7 +14,8 @@ import { AuthenticationScheme } from "../utils/Constants"; * - claims - A stringified claims request which will be added to all /authorize and /token calls * - shrClaims - A stringified claims object which will be added to a Signed HTTP Request * - resourceRequestMethod - HTTP Request type used to request data from the resource (i.e. "GET", "POST", etc.). Used for proof-of-possession flows. - * - resourceRequestUri - URI that token will be used for. Used for proof-of-possession flows. + * - resourceRequestUri - URI that token will be used for. Used for proof-of-possession flows.\ + * - stkJwk - String representatin of the session transport key JWK used for Refresh Token Binding */ export type BaseAuthRequest = { authority: string; @@ -25,4 +26,5 @@ export type BaseAuthRequest = { shrClaims?: string; resourceRequestMethod?: string; resourceRequestUri?: string; + stkJwk?: string }; From b8f8be0b5ab0840ac197478364c4aa58918c5a5f Mon Sep 17 00:00:00 2001 From: HectorMg Date: Fri, 16 Apr 2021 15:03:22 -0700 Subject: [PATCH 03/44] Add STK generation logic to common and browser --- lib/msal-browser/src/app/ClientApplication.ts | 6 +- lib/msal-browser/src/crypto/BrowserCrypto.ts | 22 +++++-- lib/msal-browser/src/crypto/CryptoOps.ts | 65 ++++++++++++++++++- .../src/utils/BrowserConstants.ts | 28 ++++++++ lib/msal-common/src/crypto/ICrypto.ts | 2 +- 5 files changed, 113 insertions(+), 10 deletions(-) diff --git a/lib/msal-browser/src/app/ClientApplication.ts b/lib/msal-browser/src/app/ClientApplication.ts index 0adb541478..ff49b76be7 100644 --- a/lib/msal-browser/src/app/ClientApplication.ts +++ b/lib/msal-browser/src/app/ClientApplication.ts @@ -7,7 +7,7 @@ import { CryptoOps } from "../crypto/CryptoOps"; import { Authority, StringUtils, UrlString, ServerAuthorizationCodeResponse, CommonAuthorizationCodeRequest, AuthorizationCodeClient, PromptValue, ServerError, InteractionRequiredAuthError, AccountInfo, AuthorityFactory, ServerTelemetryManager, SilentFlowClient, ClientConfiguration, BaseAuthRequest, ServerTelemetryRequest, PersistentCacheKeys, IdToken, ProtocolUtils, ResponseMode, Constants, INetworkModule, AuthenticationResult, Logger, ThrottlingUtils, RefreshTokenClient, AuthenticationScheme, CommonSilentFlowRequest, CommonEndSessionRequest, AccountEntity, ICrypto, DEFAULT_CRYPTO_IMPLEMENTATION, AuthorityOptions } from "@azure/msal-common"; import { BrowserCacheManager, DEFAULT_BROWSER_CACHE_MANAGER } from "../cache/BrowserCacheManager"; import { BrowserConfiguration, buildConfiguration, Configuration } from "../config/Configuration"; -import { TemporaryCacheKeys, InteractionType, ApiId, BrowserConstants, BrowserCacheLocation, WrapperSKU } from "../utils/BrowserConstants"; +import { TemporaryCacheKeys, InteractionType, ApiId, BrowserConstants, BrowserCacheLocation, WrapperSKU, CryptoKeyTypes } from "../utils/BrowserConstants"; import { BrowserUtils } from "../utils/BrowserUtils"; import { BrowserStateObject, BrowserProtocolUtils } from "../utils/BrowserProtocolUtils"; import { RedirectHandler } from "../interaction_handler/RedirectHandler"; @@ -1197,6 +1197,10 @@ export abstract class ClientApplication { protected async initializeAuthorizationCodeRequest(request: AuthorizationUrlRequest): Promise { const generatedPkceParams = await this.browserCrypto.generatePkceCodes(); + // Generate Session Transport Key for Refresh Token Binding + const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.stk_jwk); + request.stkJwk = sessionTransportKeyThumbprint; + const authCodeRequest: CommonAuthorizationCodeRequest = { ...request, redirectUri: request.redirectUri, diff --git a/lib/msal-browser/src/crypto/BrowserCrypto.ts b/lib/msal-browser/src/crypto/BrowserCrypto.ts index b1f332a303..d8225fb960 100644 --- a/lib/msal-browser/src/crypto/BrowserCrypto.ts +++ b/lib/msal-browser/src/crypto/BrowserCrypto.ts @@ -6,6 +6,7 @@ import { BrowserStringUtils } from "../utils/BrowserStringUtils"; import { BrowserAuthError } from "../error/BrowserAuthError"; import { KEY_FORMAT_JWK } from "../utils/BrowserConstants"; +import { CryptoKeyOptions } from "./CryptoOps"; /** * See here for more info on RsaHashedKeyGenParams: https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedKeyGenParams */ @@ -66,11 +67,16 @@ export class BrowserCrypto { * @param extractable * @param usages */ - async generateKeyPair(extractable: boolean, usages: Array): Promise { + async generateKeyPair(keyOptions: CryptoKeyOptions, extractable: boolean): Promise { + const keyGenAlgorithmOptions = keyOptions.keyGenAlgorithmOptions; return ( this.hasIECrypto() ? - this.msCryptoGenerateKey(extractable, usages) - : window.crypto.subtle.generateKey(this._keygenAlgorithmOptions, extractable, usages) + this.msCryptoGenerateKey(keyOptions, extractable) + : window.crypto.subtle.generateKey( + keyGenAlgorithmOptions, + extractable, + keyOptions.keypairUsages + ) ) as Promise; } @@ -162,9 +168,15 @@ export class BrowserCrypto { * @param extractable * @param usages */ - private async msCryptoGenerateKey(extractable: boolean, usages: Array): Promise { + private async msCryptoGenerateKey(keyOptions: CryptoKeyOptions, extractable: boolean): Promise { return new Promise((resolve: any, reject: any) => { - const msGenerateKey = window["msCrypto"].subtle.generateKey(this._keygenAlgorithmOptions, extractable, usages); + + const msGenerateKey = window["msCrypto"].subtle.generateKey( + keyOptions.keyGenAlgorithmOptions, + extractable, + keyOptions.keypairUsages + ); + msGenerateKey.addEventListener("complete", (e: { target: { result: CryptoKeyPair | PromiseLike; }; }) => { resolve(e.target.result); }); diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index 83d119acc3..15c6f2d806 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -11,7 +11,10 @@ import { PkceGenerator } from "./PkceGenerator"; import { BrowserCrypto } from "./BrowserCrypto"; import { DatabaseStorage } from "../cache/DatabaseStorage"; import { BrowserStringUtils } from "../utils/BrowserStringUtils"; -import { KEY_FORMAT_JWK } from "../utils/BrowserConstants"; +import { BROWSER_CRYPTO, CryptoKeyTypes, KEY_FORMAT_JWK, KEY_USAGES } from "../utils/BrowserConstants"; + +// Public Exponent used in Key Generation +const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]); export type CachedKeyPair = { publicKey: CryptoKey, @@ -20,6 +23,12 @@ export type CachedKeyPair = { requestUri?: string }; +export type CryptoKeyOptions = { + keyGenAlgorithmOptions: RsaHashedKeyGenParams, + keypairUsages: KeyUsage[], + privateKeyUsage: KeyUsage[] +}; + /** * This class implements MSAL's crypto interface, which allows it to perform base64 encoding and decoding, generating cryptographically random GUIDs and * implementing Proof Key for Code Exchange specs for the OAuth Authorization Code Flow using PKCE (rfc here: https://tools.ietf.org/html/rfc7636). @@ -31,6 +40,8 @@ export class CryptoOps implements ICrypto { private b64Encode: Base64Encode; private b64Decode: Base64Decode; private pkceGenerator: PkceGenerator; + private _atBindingKeyOptions: CryptoKeyOptions; + private _rtBindingKeyOptions: CryptoKeyOptions; private static POP_KEY_USAGES: Array = ["sign", "verify"]; private static EXTRACTABLE: boolean = true; @@ -48,6 +59,28 @@ export class CryptoOps implements ICrypto { this.guidGenerator = new GuidGenerator(this.browserCrypto); this.pkceGenerator = new PkceGenerator(this.browserCrypto); this.cache = new DatabaseStorage(CryptoOps.DB_NAME, CryptoOps.TABLE_NAME, CryptoOps.DB_VERSION); + + this._atBindingKeyOptions = { + keyGenAlgorithmOptions: { + name: BROWSER_CRYPTO.PKCS1_V15_KEYGEN_ALG, + hash: BROWSER_CRYPTO.S256_HASH_ALG, + modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, + publicExponent: PUBLIC_EXPONENT + }, + keypairUsages: KEY_USAGES.AT_BINDING.KEYPAIR as KeyUsage[], + privateKeyUsage: KEY_USAGES.AT_BINDING.PRIVATE_KEY as KeyUsage[] + }; + + this._rtBindingKeyOptions = { + keyGenAlgorithmOptions: { + name: BROWSER_CRYPTO.RSA_OAEP, + hash: BROWSER_CRYPTO.S256_HASH_ALG, + modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, + publicExponent: PUBLIC_EXPONENT + }, + keypairUsages: KEY_USAGES.RT_BINDING.KEYPAIR as KeyUsage[], + privateKeyUsage: KEY_USAGES.RT_BINDING.PRIVATE_KEY as KeyUsage[] + }; } /** @@ -85,17 +118,30 @@ export class CryptoOps implements ICrypto { * Generates a keypair, stores it and returns a thumbprint * @param request */ - async getPublicKeyThumbprint(request: BaseAuthRequest): Promise { + async getPublicKeyThumbprint(request: BaseAuthRequest, keyType?: string): Promise { + let keyOptions: CryptoKeyOptions; + + switch(keyType) { + case CryptoKeyTypes.stk_jwk: + keyOptions = this._rtBindingKeyOptions; + break; + default: + keyOptions = this._atBindingKeyOptions; + } + // Generate Keypair - const keyPair = await this.browserCrypto.generateKeyPair(CryptoOps.EXTRACTABLE, CryptoOps.POP_KEY_USAGES); + const keyPair = await this.browserCrypto.generateKeyPair(keyOptions, CryptoOps.EXTRACTABLE); // Generate Thumbprint for Public Key const publicKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.publicKey); + + // Build JSON Web Key const pubKeyThumprintObj: JsonWebKey = { e: publicKeyJwk.e, kty: publicKeyJwk.kty, n: publicKeyJwk.n }; + const publicJwkString: string = BrowserCrypto.getJwkString(pubKeyThumprintObj); const publicJwkBuffer: ArrayBuffer = await this.browserCrypto.sha256Digest(publicJwkString); const publicJwkHash: string = this.b64Encode.urlEncodeArr(new Uint8Array(publicJwkBuffer)); @@ -152,4 +198,17 @@ export class CryptoOps implements ICrypto { return `${tokenString}.${encodedSignature}`; } + + /** + * Returns the public key from an asymmetric key pair stored in IndexedDB based on the + * public key thumbprint parameter + * @param keyThumbprint + * @returns Public Key JWK string + */ + async getAsymmetricPublicKey(keyThumbprint: string): Promise { + const cachedKeyPair: CachedKeyPair = await this.cache.get(keyThumbprint); + // Get public key as JWK + const publicKeyJwk = await this.browserCrypto.exportJwk(cachedKeyPair.publicKey); + return BrowserCrypto.getJwkString(publicKeyJwk); + } } diff --git a/lib/msal-browser/src/utils/BrowserConstants.ts b/lib/msal-browser/src/utils/BrowserConstants.ts index f48ee70294..086e35656a 100644 --- a/lib/msal-browser/src/utils/BrowserConstants.ts +++ b/lib/msal-browser/src/utils/BrowserConstants.ts @@ -147,3 +147,31 @@ export enum WrapperSKU { React = "@azure/msal-react", Angular = "@azure/msal-angular" } + +// Supported Cryptographic Key Types +export enum CryptoKeyTypes { + req_cnf = "req_cnf", + stk_jwk = "stk_jwk" +} + +// Crypto Key Usage sets +export const KEY_USAGES = { + AT_BINDING: { + KEYPAIR: ["sign", "verify"], + PRIVATE_KEY: ["sign"] + }, + RT_BINDING: { + KEYPAIR: ["encrypt", "decrypt"], + PRIVATE_KEY: ["decrypt"] + } +}; + +// Cryptographic Constants +export const BROWSER_CRYPTO = { + PKCS1_V15_KEYGEN_ALG: "RSASSA-PKCS1-v1_5", + RSA_OAEP: "RSA-OAEP", + AES_GCM: "AES-GCM", + DIRECT: "dir", + S256_HASH_ALG: "SHA-256", + MODULUS_LENGTH: 2048 +}; diff --git a/lib/msal-common/src/crypto/ICrypto.ts b/lib/msal-common/src/crypto/ICrypto.ts index 07a1ed7089..864edb8f5c 100644 --- a/lib/msal-common/src/crypto/ICrypto.ts +++ b/lib/msal-common/src/crypto/ICrypto.ts @@ -43,7 +43,7 @@ export interface ICrypto { * Generates an JWK RSA S256 Thumbprint * @param request */ - getPublicKeyThumbprint(request: BaseAuthRequest): Promise; + getPublicKeyThumbprint(request: BaseAuthRequest, keyType?: string): Promise; /** * Returns a signed proof-of-possession token with a given acces token that contains a cnf claim with the required kid. * @param accessToken From 10729529a1f07b1fc4ee297320a2b8aab761b1ad Mon Sep 17 00:00:00 2001 From: HectorMg Date: Mon, 19 Apr 2021 11:10:19 -0700 Subject: [PATCH 04/44] Update PublicClientApplication tests to mock out STK generation from Auth Code requests --- lib/msal-browser/package-lock.json | 12 ++++-- .../test/app/PublicClientApplication.spec.ts | 39 +++++++++++++++---- .../test/utils/StringConstants.ts | 7 +++- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/lib/msal-browser/package-lock.json b/lib/msal-browser/package-lock.json index 44abc82913..50f21aa9d5 100644 --- a/lib/msal-browser/package-lock.json +++ b/lib/msal-browser/package-lock.json @@ -175,6 +175,14 @@ "tslib": "^1.9.3" } }, + "@azure/msal-common": { + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/@azure/msal-common/-/msal-common-4.2.0.tgz", + "integrity": "sha512-dOImswKoo0E0t/j6ePcWYBZ2oPrt9I7LeuXfW9zxbPBRwfqpd0MBHjTXkCFZinn0xW8UbzCnWT7DxP/4UsOQLA==", + "requires": { + "debug": "^4.1.1" + } + }, "@azure/storage-blob": { "version": "12.2.1", "resolved": "https://registry.npmjs.org/@azure/storage-blob/-/storage-blob-12.2.1.tgz", @@ -2235,7 +2243,6 @@ "version": "4.1.1", "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", - "dev": true, "requires": { "ms": "^2.1.1" } @@ -3737,8 +3744,7 @@ "ms": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", - "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", - "dev": true + "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, "nice-try": { "version": "1.0.5", diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index a3119544ec..f6a8451792 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -8,7 +8,7 @@ import chai, { config } from "chai"; import chaiAsPromised from "chai-as-promised"; import sinon from "sinon"; import { PublicClientApplication } from "../../src/app/PublicClientApplication"; -import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, DEFAULT_OPENID_CONFIG_RESPONSE, testNavUrl, testLogoutUrl, TEST_STATE_VALUES, testNavUrlNoRequest } from "../utils/StringConstants"; +import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, DEFAULT_OPENID_CONFIG_RESPONSE, testNavUrl, testLogoutUrl, TEST_STATE_VALUES, testNavUrlNoRequest, TEST_POP_VALUES } from "../utils/StringConstants"; import { ServerError, Constants, AccountInfo, TokenClaims, PromptValue, AuthenticationResult, AuthorizationCodeRequest, AuthorizationUrlRequest, AuthToken, PersistentCacheKeys, AuthorizationCodeClient, ResponseMode, AccountEntity, ProtocolUtils, AuthenticationScheme, RefreshTokenClient, Logger, ServerTelemetryEntity, SilentFlowRequest, EndSessionRequest as CommonEndSessionRequest, LogLevel, NetworkResponse, ServerAuthorizationTokenResponse } from "@azure/msal-common"; import { BrowserUtils } from "../../src/utils/BrowserUtils"; import { BrowserConstants, TemporaryCacheKeys, ApiId, InteractionType, BrowserCacheLocation, WrapperSKU } from "../../src/utils/BrowserConstants"; @@ -940,6 +940,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { }); describe("loginRedirect", () => { + beforeEach(() => { + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); + }); it("loginRedirect throws an error if interaction is currently in progress", async () => { window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.INTERACTION_STATUS_KEY}`, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE); @@ -995,6 +998,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER }); + sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); pca.loginRedirect(null); @@ -1144,7 +1148,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { correlationId: TEST_CONFIG.CORRELATION_ID, responseMode: TEST_CONFIG.RESPONSE_MODE as ResponseMode, nonce: "", - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, }; await pca.loginRedirect(emptyRequest); const validatedRequest: AuthorizationUrlRequest = { @@ -1157,7 +1161,8 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { authority: `${Constants.DEFAULT_AUTHORITY}`, responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD + codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, + stkJwk: TEST_POP_VALUES.KID }; expect(loginUrlSpy.calledWith(validatedRequest)).to.be.true; }); @@ -1201,6 +1206,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { nonce: "", authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme }; + await pca.loginRedirect(loginRequest); const validatedRequest: AuthorizationUrlRequest = { ...loginRequest, @@ -1211,13 +1217,17 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { nonce: RANDOM_TEST_GUID, responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD + codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, + stkJwk: TEST_POP_VALUES.KID }; expect(loginUrlSpy.calledWith(validatedRequest)).to.be.true; }); }); describe("acquireTokenRedirect", () => { + beforeEach(() => { + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); + }); it("throws error if called in a popup", (done) => { const oldWindow = global.window; @@ -1439,7 +1449,8 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { correlationId: TEST_CONFIG.CORRELATION_ID, responseMode: TEST_CONFIG.RESPONSE_MODE as ResponseMode, nonce: "", - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + stkJwk: TEST_POP_VALUES.KID }; await pca.acquireTokenRedirect(emptyRequest); const validatedRequest: AuthorizationUrlRequest = { @@ -1454,6 +1465,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD }; + expect(acquireTokenUrlSpy.calledWith(validatedRequest)).to.be.true; }); @@ -1495,7 +1507,8 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { correlationId: TEST_CONFIG.CORRELATION_ID, responseMode: TEST_CONFIG.RESPONSE_MODE as ResponseMode, nonce: "", - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + stkJwk: TEST_POP_VALUES.KID }; await pca.acquireTokenRedirect(loginRequest); const validatedRequest: AuthorizationUrlRequest = { @@ -1519,6 +1532,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { describe("loginPopup", () => { beforeEach(() => { sinon.stub(window, "open").returns(window); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); }); afterEach(() => { @@ -1628,6 +1642,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { describe("acquireTokenPopup", () => { beforeEach(() => { sinon.stub(window, "open").returns(window); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); }); afterEach(() => { @@ -1823,6 +1838,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { describe("ssoSilent() Tests", () => { + beforeEach(() => { + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); + }); + it("throws error if loginHint or sid are empty", async () => { await expect(pca.ssoSilent({ redirectUri: TEST_URIS.TEST_REDIR_URI, @@ -1969,6 +1988,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { }); describe("Acquire Token Silent (Iframe) Tests", () => { + beforeEach(() => { + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); + }); it("successfully renews token", async () => { const testServerTokenResponse = { @@ -2102,7 +2124,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { fromCache: false, expiresOn: new Date(Date.now() + (testServerTokenResponse.expires_in * 1000)), account: testAccount, - tokenType: AuthenticationScheme.BEARER + tokenType: AuthenticationScheme.BEARER, }; const createAcqTokenStub = sinon.stub(AuthorizationCodeClient.prototype, "getAuthCodeUrl").resolves(testNavUrl); const silentTokenHelperStub = sinon.stub(pca, "silentTokenHelper").resolves(testTokenResponse); @@ -2132,7 +2154,8 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + stkJwk: TEST_POP_VALUES.KID }; const tokenResp = await pca.acquireTokenSilent(silentFlowRequest); diff --git a/lib/msal-browser/test/utils/StringConstants.ts b/lib/msal-browser/test/utils/StringConstants.ts index 7406723700..42ec7303a5 100644 --- a/lib/msal-browser/test/utils/StringConstants.ts +++ b/lib/msal-browser/test/utils/StringConstants.ts @@ -90,7 +90,12 @@ export const TEST_DATA_CLIENT_INFO = { export const TEST_POP_VALUES = { KID: "NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs", ENCODED_REQ_CNF: "eyJraWQiOiJOemJMc1hoOHVEQ2NkLTZNTndYRjRXXzdub1dYRlpBZkhreFpzUkdDOVhzIiwieG1zX2tzbCI6InN3In0=", - DECODED_REQ_CNF: "{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\",\"xms_ksl\":\"sw\"}" + DECODED_REQ_CNF: "{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\",\"xms_ksl\":\"sw\"}", + SAMPLE_POP_AT: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + SAMPLE_POP_AT_PAYLOAD_ENCODED: "eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ", + SAMPLE_POP_AT_PAYLOAD_DECODED: "{\"sub\":\"1234567890\",\"name\":\"John Doe\",\"iat\":1516239022,\"cnf\":{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\"}}", + ENCODED_STK_JWK_THUMBPRINT: "%7B%22kid%22%3A%22NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs%22%7D", + DECODED_STK_JWK_THUMBPRINT: "{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\"}" }; export const TEST_STATE_VALUES = { From c4c209d931b16790991fc7438e3c5c06c755c1db Mon Sep 17 00:00:00 2001 From: HectorMg Date: Mon, 19 Apr 2021 11:21:06 -0700 Subject: [PATCH 05/44] Undo msal-node-samples changes --- samples/msal-node-samples/auth-code/test/auth-code-aad.spec.ts | 2 +- samples/msal-node-samples/auth-code/test/auth-code-adfs.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/msal-node-samples/auth-code/test/auth-code-aad.spec.ts b/samples/msal-node-samples/auth-code/test/auth-code-aad.spec.ts index 53bd28d649..af82fd4cfe 100644 --- a/samples/msal-node-samples/auth-code/test/auth-code-aad.spec.ts +++ b/samples/msal-node-samples/auth-code/test/auth-code-aad.spec.ts @@ -32,7 +32,7 @@ const cachePlugin = require("../../cachePlugin.js")(TEST_CACHE_LOCATION); const config = require("../config/AAD.json"); describe("Auth Code AAD PPE Tests", () => { - jest.setTimeout(25000); + jest.setTimeout(15000); let browser: puppeteer.Browser; let context: puppeteer.BrowserContext; let page: puppeteer.Page; diff --git a/samples/msal-node-samples/auth-code/test/auth-code-adfs.spec.ts b/samples/msal-node-samples/auth-code/test/auth-code-adfs.spec.ts index 4efaf71dd5..6b11958ba5 100644 --- a/samples/msal-node-samples/auth-code/test/auth-code-adfs.spec.ts +++ b/samples/msal-node-samples/auth-code/test/auth-code-adfs.spec.ts @@ -24,7 +24,7 @@ let username: string; let accountPwd: string; describe('Auth Code ADFS PPE Tests', () => { - jest.setTimeout(25000); + jest.setTimeout(15000); let browser: puppeteer.Browser; let context: puppeteer.BrowserContext; let page: puppeteer.Page; From a80561c941d0ac5d05b40f6b43cba6f6713e4a6f Mon Sep 17 00:00:00 2001 From: HectorMg Date: Mon, 19 Apr 2021 13:06:21 -0700 Subject: [PATCH 06/44] Move generateCnf from PopTokenGenerator to KeyManager --- .../src/client/AuthorizationCodeClient.ts | 6 +- .../src/client/RefreshTokenClient.ts | 8 +- lib/msal-common/src/crypto/KeyManager.ts | 39 +++++++++- .../src/crypto/PopTokenGenerator.ts | 26 ------- lib/msal-common/src/utils/Constants.ts | 9 +++ .../test/crypto/KeyManager.spec.ts | 78 +++++++++++++++++++ .../test/crypto/PopTokenGenerator.spec.ts | 18 +---- lib/msal-node/package-lock.json | 9 ++- 8 files changed, 140 insertions(+), 53 deletions(-) create mode 100644 lib/msal-common/test/crypto/KeyManager.spec.ts diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 0a6d1b74b9..949b2bd033 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -21,12 +21,12 @@ import { ServerAuthorizationCodeResponse } from "../response/ServerAuthorization import { AccountEntity } from "../cache/entities/AccountEntity"; import { CommonEndSessionRequest } from "../request/CommonEndSessionRequest"; import { ClientConfigurationError } from "../error/ClientConfigurationError"; -import { PopTokenGenerator } from "../crypto/PopTokenGenerator"; import { RequestThumbprint } from "../network/RequestThumbprint"; import { AuthorizationCodePayload } from "../response/AuthorizationCodePayload"; import { TimeUtils } from "../utils/TimeUtils"; import { TokenClaims } from "../account/TokenClaims"; import { AccountInfo } from "../account/AccountInfo"; +import { KeyManager } from "../crypto/KeyManager"; /** * Oauth2.0 Authorization Code client @@ -206,8 +206,8 @@ export class AuthorizationCodeClient extends BaseClient { parameterBuilder.addClientInfo(); if (request.authenticationScheme === AuthenticationScheme.POP) { - const popTokenGenerator = new PopTokenGenerator(this.cryptoUtils); - const cnfString = await popTokenGenerator.generateCnf(request); + const keyManager = new KeyManager(this.cryptoUtils); + const cnfString = await keyManager.generateCnf(request); parameterBuilder.addPopToken(cnfString); } diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 7fb98fa79a..318407914a 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -12,7 +12,6 @@ import { RequestParameterBuilder } from "../request/RequestParameterBuilder"; import { GrantType, AuthenticationScheme, Errors } from "../utils/Constants"; import { ResponseHandler } from "../response/ResponseHandler"; import { AuthenticationResult } from "../response/AuthenticationResult"; -import { PopTokenGenerator } from "../crypto/PopTokenGenerator"; import { StringUtils } from "../utils/StringUtils"; import { RequestThumbprint } from "../network/RequestThumbprint"; import { NetworkResponse } from "../network/NetworkManager"; @@ -21,6 +20,7 @@ import { ClientConfigurationError } from "../error/ClientConfigurationError"; import { ClientAuthError, ClientAuthErrorMessage } from "../error/ClientAuthError"; import { ServerError } from "../error/ServerError"; import { TimeUtils } from "../utils/TimeUtils"; +import { KeyManager } from "../crypto/KeyManager"; /** * OAuth2.0 refresh token client @@ -185,9 +185,9 @@ export class RefreshTokenClient extends BaseClient { } if (request.authenticationScheme === AuthenticationScheme.POP) { - const popTokenGenerator = new PopTokenGenerator(this.cryptoUtils); - - parameterBuilder.addPopToken(await popTokenGenerator.generateCnf(request)); + const keyManager = new KeyManager(this.cryptoUtils); + const cnfString = await keyManager.generateCnf(request); + parameterBuilder.addPopToken(cnfString); } if (!StringUtils.isEmpty(request.claims) || this.config.authOptions.clientCapabilities && this.config.authOptions.clientCapabilities.length > 0) { diff --git a/lib/msal-common/src/crypto/KeyManager.ts b/lib/msal-common/src/crypto/KeyManager.ts index 432fbcc3ee..ee2b32a7b0 100644 --- a/lib/msal-common/src/crypto/KeyManager.ts +++ b/lib/msal-common/src/crypto/KeyManager.ts @@ -3,16 +3,47 @@ * Licensed under the MIT License. */ -import { Logger } from "../logger/Logger"; +import { BaseAuthRequest } from "../request/BaseAuthRequest"; +import { CryptoKeyTypes } from "../utils/Constants"; import { ICrypto } from "./ICrypto"; +/** + * See eSTS docs for more info. + * - A kid element, with the value containing an RFC 7638-compliant JWK thumbprint that is base64 encoded. + * - xms_ksl element, representing the storage location of the key's secret component on the client device. One of two values: + * - sw: software storage + * - uhw: hardware storage + */ + type ReqCnf = { + kid: string; + xms_ksl: KeyLocation; +}; + +enum KeyLocation { + SW = "sw", + UHW = "uhw" +} + export class KeyManager { private cryptoUtils: ICrypto; - private logger: Logger; - constructor(cryptoUtils: ICrypto, logger: Logger) { + constructor(cryptoUtils: ICrypto) { this.cryptoUtils = cryptoUtils; - this.logger = logger; + } + + /** + * Generates an asymmetric crypto keypair and returns the base64 encoded stringified + * req_cnf parameter from the public key's thumbprint + * @param request + * @returns Public Key JWK string + */ + async generateCnf(request: BaseAuthRequest): Promise { + const kidThumbprint = await this.cryptoUtils.getPublicKeyThumbprint(request, CryptoKeyTypes.req_cnf); + const reqCnf: ReqCnf = { + kid: kidThumbprint, + xms_ksl: KeyLocation.SW + }; + return this.cryptoUtils.base64Encode(JSON.stringify(reqCnf)); } /** diff --git a/lib/msal-common/src/crypto/PopTokenGenerator.ts b/lib/msal-common/src/crypto/PopTokenGenerator.ts index 0c18ec5a8e..49d09ead9a 100644 --- a/lib/msal-common/src/crypto/PopTokenGenerator.ts +++ b/lib/msal-common/src/crypto/PopTokenGenerator.ts @@ -11,23 +11,6 @@ import { UrlString } from "../url/UrlString"; import { ClientAuthError } from "../error/ClientAuthError"; import { BaseAuthRequest } from "../request/BaseAuthRequest"; -/** - * See eSTS docs for more info. - * - A kid element, with the value containing an RFC 7638-compliant JWK thumbprint that is base64 encoded. - * - xms_ksl element, representing the storage location of the key's secret component on the client device. One of two values: - * - sw: software storage - * - uhw: hardware storage - */ -type ReqCnf = { - kid: string; - xms_ksl: KeyLocation; -}; - -enum KeyLocation { - SW = "sw", - UHW = "uhw" -} - export class PopTokenGenerator { private cryptoUtils: ICrypto; @@ -36,15 +19,6 @@ export class PopTokenGenerator { this.cryptoUtils = cryptoUtils; } - async generateCnf(request: BaseAuthRequest): Promise { - const kidThumbprint = await this.cryptoUtils.getPublicKeyThumbprint(request); - const reqCnf: ReqCnf = { - kid: kidThumbprint, - xms_ksl: KeyLocation.SW - }; - return this.cryptoUtils.base64Encode(JSON.stringify(reqCnf)); - } - async signPopToken(accessToken: string, request: BaseAuthRequest): Promise { const tokenClaims: TokenClaims | null = AuthToken.extractTokenClaims(accessToken, this.cryptoUtils); diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index ac7dd85285..79887dc47e 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -332,3 +332,12 @@ export enum PasswordGrantConstants { username = "username", password = "password" } + +/** + * Supported Cryptographic Key Types + */ +export enum CryptoKeyTypes { + req_cnf = "req_cnf", + stk_jwk = "stk_jwk" +} + diff --git a/lib/msal-common/test/crypto/KeyManager.spec.ts b/lib/msal-common/test/crypto/KeyManager.spec.ts new file mode 100644 index 0000000000..00eaae5dbe --- /dev/null +++ b/lib/msal-common/test/crypto/KeyManager.spec.ts @@ -0,0 +1,78 @@ +import * as Mocha from "mocha"; +import * as chai from "chai"; +import sinon from "sinon"; +import chaiAsPromised from "chai-as-promised"; +const expect = chai.expect; +chai.use(chaiAsPromised); +import { ICrypto, PkceCodes, Logger } from "../../src"; +import { RANDOM_TEST_GUID, TEST_POP_VALUES, TEST_DATA_CLIENT_INFO, TEST_CONFIG, TEST_URIS } from "../utils/StringConstants"; +import { KeyManager } from "../../src/crypto/KeyManager"; + +describe("KeyManager Unit Tests", () => { + + afterEach(() => { + sinon.restore(); + }); + + const cryptoInterface: ICrypto = { + createNewGuid(): string { + return RANDOM_TEST_GUID; + }, + base64Decode(input: string): string { + switch (input) { + case TEST_POP_VALUES.ENCODED_REQ_CNF: + return TEST_POP_VALUES.DECODED_REQ_CNF; + case TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO: + return TEST_DATA_CLIENT_INFO.TEST_DECODED_CLIENT_INFO; + case TEST_POP_VALUES.SAMPLE_POP_AT_PAYLOAD_ENCODED: + return TEST_POP_VALUES.SAMPLE_POP_AT_PAYLOAD_DECODED; + default: + return input; + } + }, + base64Encode(input: string): string { + switch (input) { + case "123-test-uid": + return "MTIzLXRlc3QtdWlk"; + case "456-test-uid": + return "NDU2LXRlc3QtdWlk"; + case TEST_POP_VALUES.DECODED_REQ_CNF: + return TEST_POP_VALUES.ENCODED_REQ_CNF; + case TEST_POP_VALUES.SAMPLE_POP_AT_PAYLOAD_DECODED: + return TEST_POP_VALUES.SAMPLE_POP_AT_PAYLOAD_ENCODED; + default: + return input; + } + }, + async generatePkceCodes(): Promise { + return { + challenge: TEST_CONFIG.TEST_CHALLENGE, + verifier: TEST_CONFIG.TEST_VERIFIER + } + }, + async getPublicKeyThumbprint(): Promise { + return TEST_POP_VALUES.KID; + }, + async signJwt(): Promise { + return ""; + }, + async getAsymmetricPublicKey(): Promise { + return TEST_POP_VALUES.KID; + } + }; + + describe("generateCnf", () => { + const testRequest = { + authority: TEST_CONFIG.validAuthority, + scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + correlationId: TEST_CONFIG.CORRELATION_ID, + resourceRequestMethod:"POST", + resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS + }; + it("Generates the req_cnf correctly", async () => { + const keyManager = new KeyManager(cryptoInterface); + const req_cnf = await keyManager.generateCnf(testRequest); + expect(req_cnf).to.be.eq(TEST_POP_VALUES.ENCODED_REQ_CNF); + }); + }); +}); \ No newline at end of file diff --git a/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts b/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts index ec5e5f266d..f5e8068485 100644 --- a/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts +++ b/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts @@ -55,24 +55,12 @@ describe("PopTokenGenerator Unit Tests", () => { }, async signJwt(): Promise { return ""; + }, + async getAsymmetricPublicKey(): Promise { + return TEST_POP_VALUES.KID; } }; - describe("generateCnf", () => { - const testRequest = { - authority: TEST_CONFIG.validAuthority, - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, - correlationId: TEST_CONFIG.CORRELATION_ID, - resourceRequestMethod:"POST", - resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS - }; - it("Generates the req_cnf correctly", async () => { - const popTokenGenerator = new PopTokenGenerator(cryptoInterface); - const req_cnf = await popTokenGenerator.generateCnf(testRequest); - expect(req_cnf).to.be.eq(TEST_POP_VALUES.ENCODED_REQ_CNF); - }); - }); - describe("signPopToken", () => { let popTokenGenerator: PopTokenGenerator; let accessToken: string; diff --git a/lib/msal-node/package-lock.json b/lib/msal-node/package-lock.json index ef01c1af85..a6c2510b85 100644 --- a/lib/msal-node/package-lock.json +++ b/lib/msal-node/package-lock.json @@ -4,6 +4,14 @@ "lockfileVersion": 1, "requires": true, "dependencies": { + "@azure/msal-common": { + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/@azure/msal-common/-/msal-common-4.2.0.tgz", + "integrity": "sha512-dOImswKoo0E0t/j6ePcWYBZ2oPrt9I7LeuXfW9zxbPBRwfqpd0MBHjTXkCFZinn0xW8UbzCnWT7DxP/4UsOQLA==", + "requires": { + "debug": "^4.1.1" + } + }, "@babel/code-frame": { "version": "7.10.4", "resolved": "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.10.4.tgz", @@ -2885,7 +2893,6 @@ "version": "4.2.0", "resolved": "https://registry.npmjs.org/debug/-/debug-4.2.0.tgz", "integrity": "sha512-IX2ncY78vDTjZMFUdmsvIRFY2Cf4FnD0wRs+nQwJU8Lu99/tPFdb0VybiiMTPe3I6rQmwsqQqRBvxU+bZ/I8sg==", - "dev": true, "requires": { "ms": "2.1.2" } From da248511fa3765e96d57c5104dec46266f51f03e Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 20 Apr 2021 11:23:18 -0700 Subject: [PATCH 07/44] Refactor crypto key generation to use different key generation algorithm options for AT and RT PoP --- lib/msal-browser/src/crypto/BrowserCrypto.ts | 42 +++------- lib/msal-browser/src/crypto/CryptoOps.ts | 14 ++-- .../test/crypto/CryptoOps.spec.ts | 79 +++++++++++++++++-- .../test/utils/StringConstants.ts | 24 ++++++ .../test/crypto/KeyManager.spec.ts | 4 +- 5 files changed, 122 insertions(+), 41 deletions(-) diff --git a/lib/msal-browser/src/crypto/BrowserCrypto.ts b/lib/msal-browser/src/crypto/BrowserCrypto.ts index d8225fb960..ff83e38531 100644 --- a/lib/msal-browser/src/crypto/BrowserCrypto.ts +++ b/lib/msal-browser/src/crypto/BrowserCrypto.ts @@ -5,39 +5,21 @@ import { BrowserStringUtils } from "../utils/BrowserStringUtils"; import { BrowserAuthError } from "../error/BrowserAuthError"; -import { KEY_FORMAT_JWK } from "../utils/BrowserConstants"; +import { BROWSER_CRYPTO, KEY_FORMAT_JWK } from "../utils/BrowserConstants"; import { CryptoKeyOptions } from "./CryptoOps"; /** * See here for more info on RsaHashedKeyGenParams: https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedKeyGenParams */ -// RSA KeyGen Algorithm -const PKCS1_V15_KEYGEN_ALG = "RSASSA-PKCS1-v1_5"; -// SHA-256 hashing algorithm -const S256_HASH_ALG = "SHA-256"; -// MOD length for PoP tokens -const MODULUS_LENGTH = 2048; -// Public Exponent -const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]); /** * 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. */ export class BrowserCrypto { - - private _keygenAlgorithmOptions: RsaHashedKeyGenParams; - constructor() { if (!(this.hasCryptoAPI())) { throw BrowserAuthError.createCryptoNotAvailableError("Browser crypto or msCrypto object not available."); } - - this._keygenAlgorithmOptions = { - name: PKCS1_V15_KEYGEN_ALG, - hash: S256_HASH_ALG, - modulusLength: MODULUS_LENGTH, - publicExponent: PUBLIC_EXPONENT - }; } /** @@ -47,7 +29,7 @@ export class BrowserCrypto { async sha256Digest(dataString: string): Promise { const data = BrowserStringUtils.stringToUtf8Arr(dataString); - return this.hasIECrypto() ? this.getMSCryptoDigest(S256_HASH_ALG, data) : this.getSubtleCryptoDigest(S256_HASH_ALG, data); + return this.hasIECrypto() ? this.getMSCryptoDigest(BROWSER_CRYPTO.S256_HASH_ALG, data) : this.getSubtleCryptoDigest(BROWSER_CRYPTO.S256_HASH_ALG, data); } /** @@ -96,13 +78,13 @@ export class BrowserCrypto { * @param extractable * @param usages */ - async importJwk(key: JsonWebKey, extractable: boolean, usages: Array): Promise { + async importJwk(keyOptions: CryptoKeyOptions, key: JsonWebKey, extractable: boolean, usages: Array): Promise { const keyString = BrowserCrypto.getJwkString(key); const keyBuffer = BrowserStringUtils.stringToArrayBuffer(keyString); return this.hasIECrypto() ? - this.msCryptoImportKey(keyBuffer, extractable, usages) - : window.crypto.subtle.importKey(KEY_FORMAT_JWK, key, this._keygenAlgorithmOptions, extractable, usages); + this.msCryptoImportKey(keyOptions, keyBuffer, extractable, usages) + : window.crypto.subtle.importKey(KEY_FORMAT_JWK, key, keyOptions.keyGenAlgorithmOptions, extractable, usages); } /** @@ -110,10 +92,10 @@ export class BrowserCrypto { * @param key * @param data */ - async sign(key: CryptoKey, data: ArrayBuffer): Promise { + async sign(keyOptions: CryptoKeyOptions, key: CryptoKey, data: ArrayBuffer): Promise { return this.hasIECrypto() ? - this.msCryptoSign(key, data) - : window.crypto.subtle.sign(this._keygenAlgorithmOptions, key, data); + this.msCryptoSign(keyOptions, key, data) + : window.crypto.subtle.sign(keyOptions.keyGenAlgorithmOptions, key, data); } /** @@ -225,9 +207,9 @@ export class BrowserCrypto { * @param extractable * @param usages */ - private async msCryptoImportKey(keyBuffer: ArrayBuffer, extractable: boolean, usages: Array): Promise { + private async msCryptoImportKey(keyOptions: CryptoKeyOptions, keyBuffer: ArrayBuffer, extractable: boolean, usages: Array): Promise { return new Promise((resolve: any, reject: any) => { - const msImportKey = window["msCrypto"].subtle.importKey(KEY_FORMAT_JWK, keyBuffer, this._keygenAlgorithmOptions, extractable, usages); + const msImportKey = window["msCrypto"].subtle.importKey(KEY_FORMAT_JWK, keyBuffer, keyOptions.keyGenAlgorithmOptions, extractable, usages); msImportKey.addEventListener("complete", (e: { target: { result: CryptoKey | PromiseLike; }; }) => { resolve(e.target.result); }); @@ -243,9 +225,9 @@ export class BrowserCrypto { * @param key * @param data */ - private async msCryptoSign(key: CryptoKey, data: ArrayBuffer): Promise { + private async msCryptoSign(keyOptions: CryptoKeyOptions, key: CryptoKey, data: ArrayBuffer): Promise { return new Promise((resolve: any, reject: any) => { - const msSign = window["msCrypto"].subtle.sign(this._keygenAlgorithmOptions, key, data); + const msSign = window["msCrypto"].subtle.sign(keyOptions, key, data); msSign.addEventListener("complete", (e: { target: { result: ArrayBuffer | PromiseLike; }; }) => { resolve(e.target.result); }); diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index 15c6f2d806..291718715b 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -63,7 +63,9 @@ export class CryptoOps implements ICrypto { this._atBindingKeyOptions = { keyGenAlgorithmOptions: { name: BROWSER_CRYPTO.PKCS1_V15_KEYGEN_ALG, - hash: BROWSER_CRYPTO.S256_HASH_ALG, + hash: { + name: BROWSER_CRYPTO.S256_HASH_ALG + }, modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, publicExponent: PUBLIC_EXPONENT }, @@ -74,7 +76,9 @@ export class CryptoOps implements ICrypto { this._rtBindingKeyOptions = { keyGenAlgorithmOptions: { name: BROWSER_CRYPTO.RSA_OAEP, - hash: BROWSER_CRYPTO.S256_HASH_ALG, + hash: { + name: BROWSER_CRYPTO.S256_HASH_ALG + }, modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, publicExponent: PUBLIC_EXPONENT }, @@ -128,7 +132,7 @@ export class CryptoOps implements ICrypto { default: keyOptions = this._atBindingKeyOptions; } - + // Generate Keypair const keyPair = await this.browserCrypto.generateKeyPair(keyOptions, CryptoOps.EXTRACTABLE); @@ -149,7 +153,7 @@ export class CryptoOps implements ICrypto { // Generate Thumbprint for Private Key const privateKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.privateKey); // Re-import private key to make it unextractable - const unextractablePrivateKey: CryptoKey = await this.browserCrypto.importJwk(privateKeyJwk, false, ["sign"]); + const unextractablePrivateKey: CryptoKey = await this.browserCrypto.importJwk(keyOptions, privateKeyJwk, false, keyOptions.privateKeyUsage); // Store Keypair data in keystore this.cache.put(publicJwkHash, { @@ -193,7 +197,7 @@ export class CryptoOps implements ICrypto { // Sign token const tokenBuffer = BrowserStringUtils.stringToArrayBuffer(tokenString); - const signatureBuffer = await this.browserCrypto.sign(cachedKeyPair.privateKey, tokenBuffer); + const signatureBuffer = await this.browserCrypto.sign(this._atBindingKeyOptions, cachedKeyPair.privateKey, tokenBuffer); const encodedSignature = this.b64Encode.urlEncodeArr(new Uint8Array(signatureBuffer)); return `${tokenString}.${encodedSignature}`; diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index d24ea31150..f669321525 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -4,10 +4,38 @@ import { CryptoOps, CachedKeyPair } from "../../src/crypto/CryptoOps"; import { GuidGenerator } from "../../src/crypto/GuidGenerator"; import { BrowserCrypto } from "../../src/crypto/BrowserCrypto"; import crypto from "crypto"; -import { PkceCodes } from "@azure/msal-common"; -import { TEST_URIS } from "../utils/StringConstants"; +import { AuthenticationScheme, PkceCodes } from "@azure/msal-common"; +import { TEST_CONFIG, TEST_URIS, BROWSER_CRYPTO, KEY_USAGES, TEST_POP_VALUES } from "../utils/StringConstants"; import { DatabaseStorage } from "../../src/cache/DatabaseStorage"; +const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]); + +const AT_BINDING_KEY_OPTIONS = { + keyGenAlgorithmOptions: { + name: BROWSER_CRYPTO.PKCS1_V15_KEYGEN_ALG, + hash: { + name: BROWSER_CRYPTO.S256_HASH_ALG + }, + modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, + publicExponent: PUBLIC_EXPONENT + }, + keypairUsages: KEY_USAGES.AT_BINDING.KEYPAIR as KeyUsage[], + privateKeyUsage: KEY_USAGES.AT_BINDING.PRIVATE_KEY as KeyUsage[] +}; + +const RT_BINDING_KEY_OPTIONS = { + keyGenAlgorithmOptions: { + name: BROWSER_CRYPTO.RSA_OAEP, + hash: { + name: BROWSER_CRYPTO.S256_HASH_ALG + }, + modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, + publicExponent: PUBLIC_EXPONENT + }, + keypairUsages: KEY_USAGES.RT_BINDING.KEYPAIR as KeyUsage[], + privateKeyUsage: KEY_USAGES.RT_BINDING.PRIVATE_KEY as KeyUsage[] +}; + describe("CryptoOps.ts Unit Tests", () => { let cryptoObj: CryptoOps; let dbStorage = {}; @@ -85,19 +113,60 @@ describe("CryptoOps.ts Unit Tests", () => { expect(regExp.test(generatedCodes.verifier)).to.be.true; }); - it("getPublicKeyThumbprint() generates a valid request thumbprint", async () => { + it("getPublicKeyThumbprint() generates a valid req_cnf thumbprint", async () => { + sinon.stub(BrowserCrypto.prototype, "getSubtleCryptoDigest").callsFake(async (algorithm: string, data: Uint8Array): Promise => { + expect(algorithm).to.be.eq("SHA-256"); + return crypto.createHash("SHA256").update(Buffer.from(data)).digest(); + }); + const generateKeyPairSpy = sinon.spy(BrowserCrypto.prototype, "generateKeyPair"); + const exportJwkSpy = sinon.spy(BrowserCrypto.prototype, "exportJwk"); + + const keyType = "req_cnf"; + + const testRequest = { + authority: TEST_CONFIG.validAuthority, + scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + correlationId: TEST_CONFIG.CORRELATION_ID, + authenticationScheme: AuthenticationScheme.POP, + resourceRequestMethod:"POST", + resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS, + stkJwk: TEST_POP_VALUES.KID + }; + const pkThumbprint = await cryptoObj.getPublicKeyThumbprint(testRequest, keyType); + /** + * Contains alphanumeric, dash '-', underscore '_', plus '+', or slash '/' with length of 43. + */ + const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); + expect(generateKeyPairSpy.calledWith(AT_BINDING_KEY_OPTIONS, true)); + expect(generateKeyPairSpy.getCall(0).args[0].keyGenAlgorithmOptions.name).to.eq(AT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name); + expect(exportJwkSpy.calledWith((await generateKeyPairSpy.returnValues[0]).publicKey)); + expect(regExp.test(pkThumbprint)).to.be.true; + expect(dbStorage[pkThumbprint]).to.be.not.empty; + }).timeout(0); + + it("getPublicKeyThumbprint() generates a valid stk_jwk thumbprint", async () => { sinon.stub(BrowserCrypto.prototype, "getSubtleCryptoDigest").callsFake(async (algorithm: string, data: Uint8Array): Promise => { expect(algorithm).to.be.eq("SHA-256"); return crypto.createHash("SHA256").update(Buffer.from(data)).digest(); }); const generateKeyPairSpy = sinon.spy(BrowserCrypto.prototype, "generateKeyPair"); const exportJwkSpy = sinon.spy(BrowserCrypto.prototype, "exportJwk"); - const pkThumbprint = await cryptoObj.getPublicKeyThumbprint("POST", TEST_URIS.TEST_AUTH_ENDPT_WITH_PARAMS); + + const keyType = "stk_jwk"; + + const testRequest = { + authority: TEST_CONFIG.validAuthority, + scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + correlationId: TEST_CONFIG.CORRELATION_ID, + stkJwk: TEST_POP_VALUES.KID + }; + const pkThumbprint = await cryptoObj.getPublicKeyThumbprint(testRequest, keyType); /** * Contains alphanumeric, dash '-', underscore '_', plus '+', or slash '/' with length of 43. */ const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); - expect(generateKeyPairSpy.calledWith(true, ["sign", "verify"])); + expect(generateKeyPairSpy.calledWith(AT_BINDING_KEY_OPTIONS, true)); + expect(generateKeyPairSpy.getCall(0).args[0].keyGenAlgorithmOptions.name).to.eq(RT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name); expect(exportJwkSpy.calledWith((await generateKeyPairSpy.returnValues[0]).publicKey)); expect(regExp.test(pkThumbprint)).to.be.true; expect(dbStorage[pkThumbprint]).to.be.not.empty; diff --git a/lib/msal-browser/test/utils/StringConstants.ts b/lib/msal-browser/test/utils/StringConstants.ts index 42ec7303a5..3b5ecca07c 100644 --- a/lib/msal-browser/test/utils/StringConstants.ts +++ b/lib/msal-browser/test/utils/StringConstants.ts @@ -24,6 +24,7 @@ export const TEST_URIS = { TEST_AUTH_ENDPT: "https://login.microsoftonline.com/common/oauth2/v2.0/authorize", TEST_END_SESSION_ENDPOINT: "https://login.microsoftonline.com/common/oauth2/v2.0/logout", TEST_AUTH_ENDPT_WITH_PARAMS: "https://login.microsoftonline.com/common/oauth2/v2.0/authorize?param1=value1¶m2=value2", + TEST_RESOURCE_ENDPT_WITH_PARAMS: "https://localhost:8081/endpoint?param1=value1¶m2=value2", }; // Test MSAL config params @@ -43,6 +44,7 @@ export const TEST_CONFIG = { TOKEN_TYPE_BEARER: "Bearer", TOKEN_TYPE_POP: "pop", DEFAULT_SCOPES: ["openid", "profile"], + DEFAULT_GRAPH_SCOPE: ["User.Read"], CORRELATION_ID: RANDOM_TEST_GUID, SID: "session-id", OID: "test-oid", @@ -201,3 +203,25 @@ export const testNavUrl = `https://login.microsoftonline.com/common/oauth2/v2.0/ export const testNavUrlNoRequest = `https://login.microsoftonline.com/common/oauth2/v2.0/authorize?client_id=${encodeURIComponent(`${TEST_CONFIG.MSAL_CLIENT_ID}`)}&scope=openid%20profile%20offline_access&redirect_uri=https%3A%2F%2Flocalhost%3A8081%2Findex.html&client-request-id=${encodeURIComponent(`${RANDOM_TEST_GUID}`)}&response_mode=fragment&response_type=code&x-client-SKU=msal.js.browser&x-client-VER=${version}&x-client-OS=&x-client-CPU=&client_info=1&code_challenge=JsjesZmxJwehdhNY9kvyr0QOeSMEvryY_EHZo3BKrqg&code_challenge_method=S256&nonce=${encodeURIComponent(`${RANDOM_TEST_GUID}`)}&state=`; export const testLogoutUrl = `https://login.microsoftonline.com/common/oauth2/v2.0/logout?post_logout_redirect_uri=${encodeURIComponent(`${TEST_URIS.TEST_REDIR_URI}`)}`; + +// Crypto Key Usage sets +export const KEY_USAGES = { + AT_BINDING: { + KEYPAIR: ["sign", "verify"], + PRIVATE_KEY: ["sign"] + }, + RT_BINDING: { + KEYPAIR: ["encrypt", "decrypt"], + PRIVATE_KEY: ["decrypt"] + } +}; + +// Cryptographic Constants +export const BROWSER_CRYPTO = { + PKCS1_V15_KEYGEN_ALG: "RSASSA-PKCS1-v1_5", + RSA_OAEP: "RSA-OAEP", + AES_GCM: "AES-GCM", + DIRECT: "dir", + S256_HASH_ALG: "SHA-256", + MODULUS_LENGTH: 2048 +}; diff --git a/lib/msal-common/test/crypto/KeyManager.spec.ts b/lib/msal-common/test/crypto/KeyManager.spec.ts index 00eaae5dbe..688e49b830 100644 --- a/lib/msal-common/test/crypto/KeyManager.spec.ts +++ b/lib/msal-common/test/crypto/KeyManager.spec.ts @@ -4,7 +4,7 @@ import sinon from "sinon"; import chaiAsPromised from "chai-as-promised"; const expect = chai.expect; chai.use(chaiAsPromised); -import { ICrypto, PkceCodes, Logger } from "../../src"; +import { ICrypto, PkceCodes, Logger, AuthenticationScheme } from "../../src"; import { RANDOM_TEST_GUID, TEST_POP_VALUES, TEST_DATA_CLIENT_INFO, TEST_CONFIG, TEST_URIS } from "../utils/StringConstants"; import { KeyManager } from "../../src/crypto/KeyManager"; @@ -66,9 +66,11 @@ describe("KeyManager Unit Tests", () => { authority: TEST_CONFIG.validAuthority, scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, correlationId: TEST_CONFIG.CORRELATION_ID, + authenticationScheme: AuthenticationScheme.POP, resourceRequestMethod:"POST", resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS }; + it("Generates the req_cnf correctly", async () => { const keyManager = new KeyManager(cryptoInterface); const req_cnf = await keyManager.generateCnf(testRequest); From 20d70abc101cdd17aca9318a68438e7dbebccdbb Mon Sep 17 00:00:00 2001 From: HectorMg Date: Wed, 21 Apr 2021 14:05:18 -0700 Subject: [PATCH 08/44] Add missing API from Crypto Interface to msal-node --- lib/msal-node/src/crypto/CryptoProvider.ts | 9 +++++++++ lib/msal-node/test/utils/TestConstants.ts | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/lib/msal-node/src/crypto/CryptoProvider.ts b/lib/msal-node/src/crypto/CryptoProvider.ts index 2a31f7d409..d27846bb81 100644 --- a/lib/msal-node/src/crypto/CryptoProvider.ts +++ b/lib/msal-node/src/crypto/CryptoProvider.ts @@ -65,4 +65,13 @@ export class CryptoProvider implements ICrypto { signJwt(): Promise { throw new Error("Method not implemented."); } + + /** + * Returns the public key from an asymmetric key pair stored in IndexedDB based on the + * public key thumbprint parameter + * @param keyThumbprint + */ + getAsymmetricPublicKey(): Promise { + throw new Error("Method not implemented."); + } } diff --git a/lib/msal-node/test/utils/TestConstants.ts b/lib/msal-node/test/utils/TestConstants.ts index 00d9d7b374..d05f305fb8 100644 --- a/lib/msal-node/test/utils/TestConstants.ts +++ b/lib/msal-node/test/utils/TestConstants.ts @@ -72,6 +72,10 @@ export const DEFAULT_CRYPTO_IMPLEMENTATION: ICrypto = { async signJwt(): Promise { const notImplErr = "Crypto interface - signJwt() has not been implemented"; throw AuthError.createUnexpectedError(notImplErr); + }, + async getAsymmetricPublicKey(): Promise { + const notImplErr = "Crypto interface - signJwt() has not been implemented"; + throw AuthError.createUnexpectedError(notImplErr); } }; From fbfbe3e34f04e469421bd30f6a34afa872be3664 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Wed, 21 Apr 2021 14:21:01 -0700 Subject: [PATCH 09/44] Fix linter issues --- lib/msal-common/src/crypto/KeyManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/msal-common/src/crypto/KeyManager.ts b/lib/msal-common/src/crypto/KeyManager.ts index ee2b32a7b0..115aeb3398 100644 --- a/lib/msal-common/src/crypto/KeyManager.ts +++ b/lib/msal-common/src/crypto/KeyManager.ts @@ -14,7 +14,7 @@ import { ICrypto } from "./ICrypto"; * - sw: software storage * - uhw: hardware storage */ - type ReqCnf = { +type ReqCnf = { kid: string; xms_ksl: KeyLocation; }; @@ -55,4 +55,4 @@ export class KeyManager { async retrieveAsymmetricPublicKey(keyThumbprint: string): Promise { return await this.cryptoUtils.getAsymmetricPublicKey(keyThumbprint); } -} \ No newline at end of file +} From 8df91ebcac3221cc280d7e82df86c0e974a037c8 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Wed, 21 Apr 2021 14:25:35 -0700 Subject: [PATCH 10/44] Add rtPop msal-browser sample --- .../VanillaJSTestApp2.0/app/rtPop/auth.js | 95 +++++++++++++++++++ .../app/rtPop/authConfig.js | 62 ++++++++++++ .../VanillaJSTestApp2.0/app/rtPop/graph.js | 64 +++++++++++++ .../VanillaJSTestApp2.0/app/rtPop/index.html | 70 ++++++++++++++ .../VanillaJSTestApp2.0/app/rtPop/ui.js | 70 ++++++++++++++ 5 files changed, 361 insertions(+) create mode 100644 samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/auth.js create mode 100644 samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js create mode 100644 samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/graph.js create mode 100644 samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/index.html create mode 100644 samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/ui.js diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/auth.js new file mode 100644 index 0000000000..3e6b00c4b5 --- /dev/null +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/auth.js @@ -0,0 +1,95 @@ +// Browser check variables +// If you support IE, our recommendation is that you sign-in using Redirect APIs +// If you as a developer are testing using Edge InPrivate mode, please add "isEdge" to the if check +const ua = window.navigator.userAgent; +const msie = ua.indexOf("MSIE "); +const msie11 = ua.indexOf("Trident/"); +const msedge = ua.indexOf("Edge/"); +const isIE = msie > 0 || msie11 > 0; +const isEdge = msedge > 0; + +let signInType; +let accountId = ""; + +// Create the main myMSALObj instance +// configuration parameters are located at authConfig.js +const myMSALObj = new msal.PublicClientApplication(msalConfig); + +// Redirect: once login is successful and redirects with tokens, call Graph API +myMSALObj.handleRedirectPromise().then(handleResponse).catch(err => { + console.error(err); +}); + +function handleResponse(resp) { + if (resp !== null) { + accountId = resp.account.homeAccountId; + myMSALObj.setActiveAccount(resp.account); + showWelcomeMessage(resp.account); + } else { + // need to call getAccount here? + const currentAccounts = myMSALObj.getAllAccounts(); + if (!currentAccounts || currentAccounts.length < 1) { + return; + } else if (currentAccounts.length > 1) { + // Add choose account code here + } else if (currentAccounts.length === 1) { + const activeAccount = currentAccounts[0]; + myMSALObj.setActiveAccount(activeAccount); + accountId = activeAccount.homeAccountId; + showWelcomeMessage(activeAccount); + } + } +} + +async function signIn(method) { + signInType = isIE ? "redirect" : method; + if (signInType === "popup") { + return myMSALObj.loginPopup(loginRequest).then(handleResponse).catch(function (error) { + console.log(error); + }); + } else if (signInType === "redirect") { + return myMSALObj.loginRedirect(loginRequest) + } +} + +function signOut(interactionType) { + const logoutRequest = { + account: myMSALObj.getAccountByHomeId(accountId) + }; + + if (interactionType === "popup") { + myMSALObj.logoutPopup(logoutRequest).then(() => { + window.location.reload(); + }); + } else { + myMSALObj.logoutRedirect(logoutRequest); + } +} + +async function getTokenPopup(request, account) { + return await myMSALObj.acquireTokenSilent(request).catch(async (error) => { + console.log("silent token acquisition fails."); + if (error instanceof msal.InteractionRequiredAuthError) { + console.log("acquiring token using popup"); + return myMSALObj.acquireTokenPopup(request).catch(error => { + console.error(error); + }); + } else { + console.error(error); + } + }); +} + +// This function can be removed if you do not need to support IE +async function getTokenRedirect(request, account) { + return await myMSALObj.acquireTokenSilent(request).catch(async (error) => { + console.log("silent token acquisition fails."); + if (error instanceof msal.InteractionRequiredAuthError) { + // fallback to interaction when silent call fails + console.log("acquiring token using redirect"); + myMSALObj.acquireTokenRedirect(request); + } else { + console.error(error); + } + }); +} diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js new file mode 100644 index 0000000000..7d1a633572 --- /dev/null +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js @@ -0,0 +1,62 @@ +// Config object to be passed to Msal on creation +const msalConfig = { + auth: { + clientId: "bc77b0a7-16aa-4af4-884b-41b968c9c71a", + authority: "https://login.microsoftonline.com/5d97b14d-c396-4aee-b524-c86d33e9b660" + }, + cache: { + cacheLocation: "sessionStorage", // This configures where your cache will be stored + storeAuthStateInCookie: false, // Set this to "true" if you are having issues on IE11 or Edge + }, + system: { + loggerOptions: { + loggerCallback: (level, message, containsPii) => { + if (containsPii) { + return; + } + switch (level) { + case msal.LogLevel.Error: + console.error(message); + return; + case msal.LogLevel.Info: + console.info(message); + return; + case msal.LogLevel.Verbose: + console.debug(message); + return; + case msal.LogLevel.Warning: + console.warn(message); + return; + } + } + } + } +}; + +// RT PoP Test Slice params +const extraQueryParams = { + dc: "ESTS-PUB-WUS2-AZ1-TEST1" +}; + +// Add here scopes for id token to be used at MS Identity Platform endpoints. +const loginRequest = { + scopes: ["User.Read"] +}; + +// Add here the endpoints for MS Graph API services you would like to use. +const graphConfig = { + graphMeEndpoint: "https://graph.microsoft-ppe.com/v1.0/me", + graphMailEndpoint: "https://graph.microsoft-ppe.com/v1.0/me/messages" +}; + +// Add here scopes for access token to be used at MS Graph API endpoints. +const tokenRequest = { + scopes: ["Mail.Read"], + forceRefresh: false // Set this to "true" to skip a cached token and go to the server to get a new token +}; + +const silentRequest = { + scopes: ["openid", "profile", "User.Read", "Mail.Read"] +}; + +const logoutRequest = {} diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/graph.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/graph.js new file mode 100644 index 0000000000..0c5f6ba224 --- /dev/null +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/graph.js @@ -0,0 +1,64 @@ +// Helper function to call MS Graph API endpoint +// using authorization bearer token scheme +function callMSGraph(endpoint, accessToken, callback) { + const headers = new Headers(); + const bearer = `Bearer ${accessToken}`; + + headers.append("Authorization", bearer); + + const options = { + method: "GET", + headers: headers + }; + + console.log('request made to Graph API at: ' + new Date().toString()); + + fetch(endpoint, options) + .then(response => response.json()) + .then(response => callback(response, endpoint)) + .catch(error => console.log(error)); +} + +async function seeProfile() { + const currentAcc = myMSALObj.getAccountByHomeId(accountId); + if (currentAcc) { + const response = await getTokenPopup(loginRequest, currentAcc).catch(error => { + console.log(error); + }); + callMSGraph(graphConfig.graphMeEndpoint, response.accessToken, updateUI); + profileButton.style.display = 'none'; + } +} + +async function readMail() { + const currentAcc = myMSALObj.getAccountByHomeId(accountId); + if (currentAcc) { + const response = await getTokenPopup(tokenRequest, currentAcc).catch(error => { + console.log(error); + }); + callMSGraph(graphConfig.graphMailEndpoint, response.accessToken, updateUI); + mailButton.style.display = 'none'; + } +} + +async function seeProfileRedirect() { + const currentAcc = myMSALObj.getAccountByHomeId(accountId); + if (currentAcc) { + const response = await getTokenRedirect(loginRequest, currentAcc).catch(error => { + console.log(error); + }); + callMSGraph(graphConfig.graphMeEndpoint, response.accessToken, updateUI); + profileButton.style.display = 'none'; + } +} + +async function readMailRedirect() { + const currentAcc = myMSALObj.getAccountByHomeId(accountId); + if (currentAcc) { + const response = await getTokenRedirect(tokenRequest, currentAcc).catch(error => { + console.log(error); + }); + callMSGraph(graphConfig.graphMailEndpoint, response.accessToken, updateUI); + mailButton.style.display = 'none'; + } +} diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/index.html b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/index.html new file mode 100644 index 0000000000..cc58ba0e1c --- /dev/null +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/index.html @@ -0,0 +1,70 @@ + + + + + + Quickstart | MSAL.JS Vanilla JavaScript SPA + + + + + + + + + +
+
Vanilla JavaScript SPA calling MS Graph API with MSAL.JS
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+
+ + + + + + + + + + + + + diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/ui.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/ui.js new file mode 100644 index 0000000000..e248605008 --- /dev/null +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/ui.js @@ -0,0 +1,70 @@ +// Select DOM elements to work with +const welcomeDiv = document.getElementById("WelcomeMessage"); +const signInButton = document.getElementById("SignIn"); +const popupButton = document.getElementById("popup"); +const redirectButton = document.getElementById("redirect"); +const cardDiv = document.getElementById("card-div"); +const mailButton = document.getElementById("readMail"); +const profileButton = document.getElementById("seeProfile"); +const profileDiv = document.getElementById("profile-div"); + +function showWelcomeMessage(account) { + // Reconfiguring DOM elements + cardDiv.style.display = 'initial'; + welcomeDiv.innerHTML = `Welcome ${account.username}`; + signInButton.setAttribute('class', "btn btn-success dropdown-toggle"); + signInButton.innerHTML = "Sign Out"; + popupButton.setAttribute('onClick', "signOut(this.id)"); + popupButton.innerHTML = "Sign Out with Popup"; + redirectButton.setAttribute('onClick', "signOut(this.id)"); + redirectButton.innerHTML = "Sign Out with Redirect"; +} + +function updateUI(data, endpoint) { + console.log('Graph API responded at: ' + new Date().toString()); + + if (endpoint === graphConfig.graphMeEndpoint) { + const title = document.createElement('p'); + title.innerHTML = "Title: " + data.jobTitle; + const email = document.createElement('p'); + email.innerHTML = "Mail: " + data.mail; + const phone = document.createElement('p'); + phone.innerHTML = "Phone: " + data.businessPhones[0]; + const address = document.createElement('p'); + address.innerHTML = "Location: " + data.officeLocation; + profileDiv.appendChild(title); + profileDiv.appendChild(email); + profileDiv.appendChild(phone); + profileDiv.appendChild(address); + } else if (endpoint === graphConfig.graphMailEndpoint) { + if (data.value.length < 1) { + alert("Your mailbox is empty!") + } else { + const tabList = document.getElementById("list-tab"); + const tabContent = document.getElementById("nav-tabContent"); + + data.value.map((d, i) => { + // Keeping it simple + if (i < 10) { + const listItem = document.createElement("a"); + listItem.setAttribute("class", "list-group-item list-group-item-action") + listItem.setAttribute("id", "list" + i + "list") + listItem.setAttribute("data-toggle", "list") + listItem.setAttribute("href", "#list" + i) + listItem.setAttribute("role", "tab") + listItem.setAttribute("aria-controls", i) + listItem.innerHTML = d.subject; + tabList.appendChild(listItem) + + const contentItem = document.createElement("div"); + contentItem.setAttribute("class", "tab-pane fade") + contentItem.setAttribute("id", "list" + i) + contentItem.setAttribute("role", "tabpanel") + contentItem.setAttribute("aria-labelledby", "list" + i + "list") + contentItem.innerHTML = " from: " + d.from.emailAddress.address + "

" + d.bodyPreview + "..."; + tabContent.appendChild(contentItem); + } + }); + } + } +} From d557d11324ade651d865054291a00a9edc9c4676 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Wed, 21 Apr 2021 14:45:51 -0700 Subject: [PATCH 11/44] Add stk_jwk to /authorize call in AuthorizationCodeClient to initiate bound rt flow --- lib/msal-browser/test/utils/StringConstants.ts | 2 +- .../src/client/AuthorizationCodeClient.ts | 4 ++++ lib/msal-common/src/crypto/KeyManager.ts | 6 +++++- .../src/request/RequestParameterBuilder.ts | 15 +++++++++++++++ lib/msal-common/src/utils/Constants.ts | 1 + 5 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/msal-browser/test/utils/StringConstants.ts b/lib/msal-browser/test/utils/StringConstants.ts index 57e747365d..3629a7d9cb 100644 --- a/lib/msal-browser/test/utils/StringConstants.ts +++ b/lib/msal-browser/test/utils/StringConstants.ts @@ -211,7 +211,7 @@ export const ALTERNATE_OPENID_CONFIG_RESPONSE = { } }; -export const testNavUrl = `https://login.microsoftonline.com/common/oauth2/v2.0/authorize?client_id=${encodeURIComponent(`${TEST_CONFIG.MSAL_CLIENT_ID}`)}&scope=user.read%20openid%20profile%20offline_access&redirect_uri=https%3A%2F%2Flocalhost%3A8081%2Findex.html&client-request-id=${encodeURIComponent(`${RANDOM_TEST_GUID}`)}&response_mode=fragment&response_type=code&x-client-SKU=msal.js.browser&x-client-VER=${version}&x-client-OS=&x-client-CPU=&client_info=1&code_challenge=JsjesZmxJwehdhNY9kvyr0QOeSMEvryY_EHZo3BKrqg&code_challenge_method=S256&nonce=${encodeURIComponent(`${RANDOM_TEST_GUID}`)}&state=${encodeURIComponent(`${TEST_STATE_VALUES.TEST_STATE_REDIRECT}`)}`; +export const testNavUrl = `https://login.microsoftonline.com/common/oauth2/v2.0/authorize?client_id=${encodeURIComponent(`${TEST_CONFIG.MSAL_CLIENT_ID}`)}&scope=user.read%20openid%20profile%20offline_access&redirect_uri=https%3A%2F%2Flocalhost%3A8081%2Findex.html&client-request-id=${encodeURIComponent(`${RANDOM_TEST_GUID}`)}&response_mode=fragment&response_type=code&x-client-SKU=msal.js.browser&x-client-VER=${version}&x-client-OS=&x-client-CPU=&client_info=1&code_challenge=JsjesZmxJwehdhNY9kvyr0QOeSMEvryY_EHZo3BKrqg&code_challenge_method=S256&nonce=${encodeURIComponent(`${RANDOM_TEST_GUID}`)}&state=${encodeURIComponent(`${TEST_STATE_VALUES.TEST_STATE_REDIRECT}`)}&stk_jwk=${TEST_POP_VALUES.ENCODED_STK_JWK_THUMBPRINT}`; export const testNavUrlNoRequest = `https://login.microsoftonline.com/common/oauth2/v2.0/authorize?client_id=${encodeURIComponent(`${TEST_CONFIG.MSAL_CLIENT_ID}`)}&scope=openid%20profile%20offline_access&redirect_uri=https%3A%2F%2Flocalhost%3A8081%2Findex.html&client-request-id=${encodeURIComponent(`${RANDOM_TEST_GUID}`)}&response_mode=fragment&response_type=code&x-client-SKU=msal.js.browser&x-client-VER=${version}&x-client-OS=&x-client-CPU=&client_info=1&code_challenge=JsjesZmxJwehdhNY9kvyr0QOeSMEvryY_EHZo3BKrqg&code_challenge_method=S256&nonce=${encodeURIComponent(`${RANDOM_TEST_GUID}`)}&state=`; diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 0cd74c4fbc..e1079eedcf 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -320,6 +320,10 @@ export class AuthorizationCodeClient extends BaseClient { parameterBuilder.addExtraQueryParameters(request.extraQueryParameters); } + if (request.stkJwk) { + parameterBuilder.addStkJwkThumbprint(request.stkJwk); + } + return parameterBuilder.createQueryString(); } diff --git a/lib/msal-common/src/crypto/KeyManager.ts b/lib/msal-common/src/crypto/KeyManager.ts index 115aeb3398..a488a19d94 100644 --- a/lib/msal-common/src/crypto/KeyManager.ts +++ b/lib/msal-common/src/crypto/KeyManager.ts @@ -14,11 +14,15 @@ import { ICrypto } from "./ICrypto"; * - sw: software storage * - uhw: hardware storage */ -type ReqCnf = { +export type ReqCnf = { kid: string; xms_ksl: KeyLocation; }; +export type StkJwkThumbprint = { + kid: string; +}; + enum KeyLocation { SW = "sw", UHW = "uhw" diff --git a/lib/msal-common/src/request/RequestParameterBuilder.ts b/lib/msal-common/src/request/RequestParameterBuilder.ts index 79d12fdf75..14002330f1 100644 --- a/lib/msal-common/src/request/RequestParameterBuilder.ts +++ b/lib/msal-common/src/request/RequestParameterBuilder.ts @@ -11,6 +11,7 @@ import { RequestValidator } from "./RequestValidator"; import { LibraryInfo } from "../config/ClientConfiguration"; import { StringUtils } from "../utils/StringUtils"; import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManager"; +import { StkJwkThumbprint } from "../crypto/KeyManager"; export class RequestParameterBuilder { @@ -356,6 +357,20 @@ export class RequestParameterBuilder { this.parameters.set(AADServerParamKeys.X_MS_LIB_CAPABILITY, ThrottlingConstants.X_MS_LIB_CAPABILITY_VALUE); } + /** + * Add stk_jwk thumbprint to query params + * @param stkJwkKid + */ + addStkJwkThumbprint(stkJwkKid: string): void { + if(!StringUtils.isEmpty(stkJwkKid)) { + const stkJwkThumbprint: StkJwkThumbprint = { + kid: stkJwkKid + }; + + this.parameters.set(AADServerParamKeys.STK_JWK, encodeURIComponent(JSON.stringify(stkJwkThumbprint))); + } + } + /** * Utility to create a URL from the params map */ diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index 0f1de2909f..44b3d5bbce 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -128,6 +128,7 @@ export enum AADServerParamKeys { CLIENT_ASSERTION_TYPE = "client_assertion_type", TOKEN_TYPE = "token_type", REQ_CNF = "req_cnf", + STK_JWK = "stk_jwk", OBO_ASSERTION = "assertion", REQUESTED_TOKEN_USE = "requested_token_use", ON_BEHALF_OF = "on_behalf_of", From afb5ceabd892bf182aee5f707cec850d9f2ea29a Mon Sep 17 00:00:00 2001 From: HectorMg Date: Wed, 21 Apr 2021 15:28:51 -0700 Subject: [PATCH 12/44] Add STK JWK to /token request to obtain bound rt response --- lib/msal-common/src/client/AuthorizationCodeClient.ts | 9 ++++++--- lib/msal-common/src/client/BaseClient.ts | 7 +++++++ lib/msal-common/src/request/RequestParameterBuilder.ts | 10 ++++++++++ .../VanillaJSTestApp2.0/app/rtPop/authConfig.js | 8 +++++++- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index e1079eedcf..6d0d2e5a04 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -26,7 +26,6 @@ import { AuthorizationCodePayload } from "../response/AuthorizationCodePayload"; import { TimeUtils } from "../utils/TimeUtils"; import { TokenClaims } from "../account/TokenClaims"; import { AccountInfo } from "../account/AccountInfo"; -import { KeyManager } from "../crypto/KeyManager"; /** * Oauth2.0 Authorization Code client @@ -216,11 +215,15 @@ export class AuthorizationCodeClient extends BaseClient { parameterBuilder.addClientInfo(); if (request.authenticationScheme === AuthenticationScheme.POP) { - const keyManager = new KeyManager(this.cryptoUtils); - const cnfString = await keyManager.generateCnf(request); + const cnfString = await this.keyManager.generateCnf(request); parameterBuilder.addPopToken(cnfString); } + if (request.stkJwk) { + const stkJwk = await this.keyManager.retrieveAsymmetricPublicKey(request.stkJwk); + parameterBuilder.addStkJwk(stkJwk); + } + const correlationId = request.correlationId || this.config.cryptoInterface.createNewGuid(); parameterBuilder.addCorrelationId(correlationId); diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 003fcf82b3..96e1bfbc5b 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -16,6 +16,7 @@ import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManag import { RequestThumbprint } from "../network/RequestThumbprint"; import { version, name } from "../packageMetadata"; import { ClientAuthError } from "../error/ClientAuthError"; +import { KeyManager } from "../crypto/KeyManager"; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. @@ -45,6 +46,9 @@ export abstract class BaseClient { // Default authority object public authority: Authority; + // Define Key Manager object + protected keyManager: KeyManager; + protected constructor(configuration: ClientConfiguration) { // Set the configuration this.config = buildClientConfiguration(configuration); @@ -69,6 +73,9 @@ export abstract class BaseClient { // set Authority this.authority = this.config.authOptions.authority; + + // set KeyManager + this.keyManager = new KeyManager(this.cryptoUtils); } /** diff --git a/lib/msal-common/src/request/RequestParameterBuilder.ts b/lib/msal-common/src/request/RequestParameterBuilder.ts index 14002330f1..bb2a07dbcc 100644 --- a/lib/msal-common/src/request/RequestParameterBuilder.ts +++ b/lib/msal-common/src/request/RequestParameterBuilder.ts @@ -371,6 +371,16 @@ export class RequestParameterBuilder { } } + /** + * Add stk_jwk public key to query params + * @param stkJwk + */ + addStkJwk(stkJwk: string): void { + if(!StringUtils.isEmpty(stkJwk)) { + this.parameters.set(AADServerParamKeys.STK_JWK, encodeURIComponent(stkJwk)); + } + } + /** * Utility to create a URL from the params map */ diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js index 7d1a633572..6f94db751b 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js @@ -38,9 +38,15 @@ const extraQueryParams = { dc: "ESTS-PUB-WUS2-AZ1-TEST1" }; +const tokenQueryParams = { + slice: "TestSlice&dc=ESTS-PUB-WUS2-AZ1-TEST1" +} + // Add here scopes for id token to be used at MS Identity Platform endpoints. const loginRequest = { - scopes: ["User.Read"] + scopes: ["User.Read"], + extraQueryParameters: extraQueryParams, + tokenQueryParameters: tokenQueryParams }; // Add here the endpoints for MS Graph API services you would like to use. From 82241e0316e492f9e6335509f5a52613a1eefd10 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Wed, 2 Jun 2021 15:26:44 -0700 Subject: [PATCH 13/44] Add stkJwk to AuthorizationCodeClient tests --- .../test/crypto/CryptoOps.spec.ts | 4 +- .../client/AuthorizationCodeClient.spec.ts | 67 +++++++++++++------ .../test/client/ClientTestUtils.ts | 3 + .../test/crypto/KeyManager.spec.ts | 7 +- .../test/test_kit/StringConstants.ts | 4 +- 5 files changed, 56 insertions(+), 29 deletions(-) diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index 5b2f89a043..d28036ffd1 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -124,7 +124,7 @@ describe("CryptoOps.ts Unit Tests", () => { }); it("getPublicKeyThumbprint() generates a valid request thumbprint", async () => { - jest.setTimeout(10000); + jest.setTimeout(30000); //@ts-ignore jest.spyOn(BrowserCrypto.prototype as any, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { expect(algorithm).toBe("SHA-256"); @@ -158,7 +158,7 @@ describe("CryptoOps.ts Unit Tests", () => { }); it("getPublicKeyThumbprint() generates a valid stk_jwk thumbprint", async () => { - jest.setTimeout(10000); + jest.setTimeout(30000); //@ts-ignore jest.spyOn(BrowserCrypto.prototype as any, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { expect(algorithm).toBe("SHA-256"); diff --git a/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts b/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts index 726603a759..bbdd85f4d4 100644 --- a/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts +++ b/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts @@ -69,7 +69,8 @@ describe("AuthorizationCodeClient unit tests", () => { codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, correlationId: RANDOM_TEST_GUID, - authenticationScheme: AuthenticationScheme.BEARER + authenticationScheme: AuthenticationScheme.BEARER, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(Constants.DEFAULT_AUTHORITY)).toBe(true); @@ -81,6 +82,7 @@ describe("AuthorizationCodeClient unit tests", () => { expect(loginUrl.includes(`${AADServerParamKeys.RESPONSE_MODE}=${encodeURIComponent(ResponseMode.QUERY)}`)).toBe(true); expect(loginUrl.includes(`${AADServerParamKeys.CODE_CHALLENGE}=${encodeURIComponent(TEST_CONFIG.TEST_CHALLENGE)}`)).toBe(true); expect(loginUrl.includes(`${AADServerParamKeys.CODE_CHALLENGE_METHOD}=${encodeURIComponent(Constants.S256_CODE_CHALLENGE_METHOD)}`)).toBe(true); + expect(loginUrl.includes(`${AADServerParamKeys.STK_JWK}=${encodeURIComponent(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT)}`)).toBe(true); }); it("Creates an authorization url passing in optional parameters", async () => { @@ -104,7 +106,8 @@ describe("AuthorizationCodeClient unit tests", () => { claims: TEST_CONFIG.CLAIMS, nonce: TEST_CONFIG.NONCE, correlationId: RANDOM_TEST_GUID, - authenticationScheme: AuthenticationScheme.BEARER + authenticationScheme: AuthenticationScheme.BEARER, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(TEST_CONFIG.validAuthority)).toBe(true); @@ -122,6 +125,7 @@ describe("AuthorizationCodeClient unit tests", () => { expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=${encodeURIComponent(TEST_CONFIG.LOGIN_HINT)}`)).toBe(true); expect(loginUrl.includes(`${SSOTypes.DOMAIN_HINT}=${encodeURIComponent(TEST_CONFIG.DOMAIN_HINT)}`)).toBe(true); expect(loginUrl.includes(`${AADServerParamKeys.CLAIMS}=${encodeURIComponent(TEST_CONFIG.CLAIMS)}`)).toBe(true); + expect(loginUrl.includes(`${AADServerParamKeys.STK_JWK}=${encodeURIComponent(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT)}`)).toBe(true); }); it("Prefers sid over loginHint if both provided and prompt=None", async () => { @@ -140,7 +144,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl).toEqual(expect.not.arrayContaining([`${SSOTypes.LOGIN_HINT}=`])); @@ -163,7 +168,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=${encodeURIComponent(TEST_CONFIG.LOGIN_HINT)}`)).toBe(true); @@ -185,7 +191,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=`)).toBe(false); @@ -207,7 +214,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=${encodeURIComponent(TEST_CONFIG.LOGIN_HINT)}`)).toBe(true); @@ -246,7 +254,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.SID}=${encodeURIComponent(testTokenClaims.sid)}`)).toBe(true); @@ -283,7 +292,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.SID}=`)).toBe(false); @@ -318,7 +328,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=${encodeURIComponent(TEST_CONFIG.LOGIN_HINT)}`)).toBe(true); @@ -339,7 +350,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=${encodeURIComponent(TEST_ACCOUNT_INFO.username)}`)).toBe(true); @@ -361,7 +373,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=`)).toBe(false); @@ -383,7 +396,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=`)).toBe(false); @@ -405,7 +419,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); expect(loginUrl.includes(`${SSOTypes.LOGIN_HINT}=`)).toBe(false); @@ -429,7 +444,8 @@ describe("AuthorizationCodeClient unit tests", () => { correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, authority: TEST_CONFIG.validAuthority, - responseMode: ResponseMode.FRAGMENT + responseMode: ResponseMode.FRAGMENT, + stkJwk: TEST_POP_VALUES.KID }; const loginUrl = await client.getAuthCodeUrl(loginRequest); @@ -594,7 +610,8 @@ describe("AuthorizationCodeClient unit tests", () => { code: "", correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, - authority: TEST_CONFIG.validAuthority + authority: TEST_CONFIG.validAuthority, + stkJwk: TEST_POP_VALUES.KID }; // @ts-ignore await expect(client.acquireToken(codeRequest, null)).rejects.toMatchObject(ClientAuthError.createTokenRequestCannotBeMadeError()); @@ -677,7 +694,8 @@ describe("AuthorizationCodeClient unit tests", () => { codeVerifier: TEST_CONFIG.TEST_VERIFIER, claims: TEST_CONFIG.CLAIMS, correlationId: RANDOM_TEST_GUID, - authenticationScheme: AuthenticationScheme.BEARER + authenticationScheme: AuthenticationScheme.BEARER, + stkJwk: TEST_POP_VALUES.KID }; await client.acquireToken(authCodeRequest, { @@ -753,7 +771,8 @@ describe("AuthorizationCodeClient unit tests", () => { codeVerifier: TEST_CONFIG.TEST_VERIFIER, claims: TEST_CONFIG.CLAIMS, correlationId: RANDOM_TEST_GUID, - authenticationScheme: AuthenticationScheme.BEARER + authenticationScheme: AuthenticationScheme.BEARER, + stkJwk: TEST_POP_VALUES.KID }; const authenticationResult = await client.acquireToken(authCodeRequest, { @@ -791,6 +810,8 @@ describe("AuthorizationCodeClient unit tests", () => { expect(returnVal.includes(`${AADServerParamKeys.X_CLIENT_CPU}=${TEST_CONFIG.TEST_CPU}`) ).toBe(true); expect(returnVal.includes(`${AADServerParamKeys.X_MS_LIB_CAPABILITY}=${ThrottlingConstants.X_MS_LIB_CAPABILITY_VALUE}`)).toBe(true); + expect(returnVal.includes(`${AADServerParamKeys.STK_JWK}=${TEST_POP_VALUES.ENCODED_STK_JWK_THUMBPRINT}`) + ).toBe(true); }); it("Adds tokenQueryParameters to the /token request", (done) => { @@ -811,6 +832,7 @@ describe("AuthorizationCodeClient unit tests", () => { claims: TEST_CONFIG.CLAIMS, correlationId: RANDOM_TEST_GUID, authenticationScheme: AuthenticationScheme.BEARER, + stkJwk: TEST_POP_VALUES.KID, tokenQueryParameters: { testParam: "testValue" } @@ -907,7 +929,8 @@ describe("AuthorizationCodeClient unit tests", () => { resourceRequestMethod: "POST", resourceRequestUri: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS, claims: TEST_CONFIG.CLAIMS, - correlationId: RANDOM_TEST_GUID + correlationId: RANDOM_TEST_GUID, + stkJwk: TEST_POP_VALUES.KID }; const authenticationResult = await client.acquireToken(authCodeRequest, { @@ -931,6 +954,8 @@ describe("AuthorizationCodeClient unit tests", () => { expect(returnVal.includes(`${AADServerParamKeys.TOKEN_TYPE}=${AuthenticationScheme.POP}`)).toBe(true); expect(returnVal.includes(`${AADServerParamKeys.REQ_CNF}=${encodeURIComponent(TEST_POP_VALUES.ENCODED_REQ_CNF)}`)).toBe(true); expect(returnVal.includes(`${AADServerParamKeys.CLAIMS}=${encodeURIComponent(TEST_CONFIG.CLAIMS)}`)).toBe(true); + expect(returnVal.includes(`${AADServerParamKeys.STK_JWK}=${TEST_POP_VALUES.ENCODED_STK_JWK_THUMBPRINT}`)).toBe(true); + }); it("properly handles expiration timestamps as strings", async () => { @@ -1003,7 +1028,8 @@ describe("AuthorizationCodeClient unit tests", () => { codeVerifier: TEST_CONFIG.TEST_VERIFIER, claims: TEST_CONFIG.CLAIMS, correlationId: RANDOM_TEST_GUID, - authenticationScheme: AuthenticationScheme.BEARER + authenticationScheme: AuthenticationScheme.BEARER, + stkJwk: TEST_POP_VALUES.KID }; const authenticationResult = await client.acquireToken(authCodeRequest, { @@ -1085,7 +1111,8 @@ describe("AuthorizationCodeClient unit tests", () => { codeVerifier: TEST_CONFIG.TEST_VERIFIER, claims: TEST_CONFIG.CLAIMS, correlationId: RANDOM_TEST_GUID, - authenticationScheme: AuthenticationScheme.BEARER + authenticationScheme: AuthenticationScheme.BEARER, + stkJwk: TEST_POP_VALUES.KID }; const authenticationResult = await client.acquireToken(authCodeRequest, { diff --git a/lib/msal-common/test/client/ClientTestUtils.ts b/lib/msal-common/test/client/ClientTestUtils.ts index b1b0883aca..f20dc250ec 100644 --- a/lib/msal-common/test/client/ClientTestUtils.ts +++ b/lib/msal-common/test/client/ClientTestUtils.ts @@ -150,6 +150,9 @@ export const mockCrypto = { async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, + async getAsymmetricPublicKey(): Promise { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; + }, async signJwt(): Promise { return ""; } diff --git a/lib/msal-common/test/crypto/KeyManager.spec.ts b/lib/msal-common/test/crypto/KeyManager.spec.ts index 181f82e8f1..e82917bb44 100644 --- a/lib/msal-common/test/crypto/KeyManager.spec.ts +++ b/lib/msal-common/test/crypto/KeyManager.spec.ts @@ -1,9 +1,4 @@ -import * as Mocha from "mocha"; -import * as chai from "chai"; import sinon from "sinon"; -import chaiAsPromised from "chai-as-promised"; -const expect = chai.expect; -chai.use(chaiAsPromised); import { ICrypto, PkceCodes, AuthenticationScheme } from "../../src"; import { RANDOM_TEST_GUID, TEST_POP_VALUES, TEST_DATA_CLIENT_INFO, TEST_CONFIG, TEST_URIS } from "../test_kit/StringConstants"; import { KeyManager } from "../../src/crypto/KeyManager"; @@ -74,7 +69,7 @@ describe("KeyManager Unit Tests", () => { it("Generates the req_cnf correctly", async () => { const keyManager = new KeyManager(cryptoInterface); const req_cnf = await keyManager.generateCnf(testRequest); - expect(req_cnf).to.be.eq(TEST_POP_VALUES.ENCODED_REQ_CNF); + expect(req_cnf).toEqual(TEST_POP_VALUES.ENCODED_REQ_CNF); }); }); }); \ No newline at end of file diff --git a/lib/msal-common/test/test_kit/StringConstants.ts b/lib/msal-common/test/test_kit/StringConstants.ts index 5582f99015..195b8a5b64 100644 --- a/lib/msal-common/test/test_kit/StringConstants.ts +++ b/lib/msal-common/test/test_kit/StringConstants.ts @@ -147,7 +147,9 @@ export const TEST_POP_VALUES = { DECODED_REQ_CNF: "{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\",\"xms_ksl\":\"sw\"}", SAMPLE_POP_AT: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", SAMPLE_POP_AT_PAYLOAD_ENCODED: "eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ", - SAMPLE_POP_AT_PAYLOAD_DECODED: "{\"sub\":\"1234567890\",\"name\":\"John Doe\",\"iat\":1516239022,\"cnf\":{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\"}}" + SAMPLE_POP_AT_PAYLOAD_DECODED: "{\"sub\":\"1234567890\",\"name\":\"John Doe\",\"iat\":1516239022,\"cnf\":{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\"}}", + ENCODED_STK_JWK_THUMBPRINT: "%7B%22kid%22%3A%22NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs%22%7D", + DECODED_STK_JWK_THUMBPRINT: "{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\"}" }; export const TEST_ACCOUNT_INFO: AccountInfo = { From a2a8479d148354bc417678038abb50e4dd4c84e6 Mon Sep 17 00:00:00 2001 From: technical-boy Date: Thu, 10 Jun 2021 15:52:19 -0500 Subject: [PATCH 14/44] Update mock crypto interface in tests --- .../test/interaction_handler/InteractionHandler.spec.ts | 3 +++ lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts | 3 +++ .../test/interaction_handler/RedirectHandler.spec.ts | 3 +++ .../test/interaction_handler/SilentHandler.spec.ts | 3 +++ lib/msal-common/test/account/AuthToken.spec.ts | 3 +++ lib/msal-common/test/account/ClientInfo.spec.ts | 3 +++ lib/msal-common/test/cache/entities/AccountEntity.spec.ts | 3 +++ lib/msal-common/test/config/ClientConfiguration.spec.ts | 3 +++ lib/msal-common/test/response/ResponseHandler.spec.ts | 3 +++ lib/msal-common/test/utils/ProtocolUtils.spec.ts | 3 +++ 10 files changed, 30 insertions(+) diff --git a/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts b/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts index b5fe41e50a..292017d3d1 100644 --- a/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts @@ -96,6 +96,9 @@ const cryptoInterface = { }, signJwt: async (): Promise => { return "signedJwt"; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } } diff --git a/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts b/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts index 82819f8d09..1e38ff9f23 100644 --- a/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts @@ -89,6 +89,9 @@ describe("PopupHandler.ts Unit Tests", () => { }, signJwt: async (): Promise => { return "signedJwt"; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }, networkInterface: { diff --git a/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts b/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts index 81a20a5ade..043550967a 100644 --- a/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts @@ -105,6 +105,9 @@ describe("RedirectHandler.ts Unit Tests", () => { }, signJwt: async (): Promise => { return "signedJwt"; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }, storageInterface: browserStorage, diff --git a/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts b/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts index 5e33ac379c..3c96a5e1b8 100644 --- a/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts @@ -89,6 +89,9 @@ describe("SilentHandler.ts Unit Tests", () => { }, signJwt: async (): Promise => { return "signedJwt"; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }, networkInterface: { diff --git a/lib/msal-common/test/account/AuthToken.spec.ts b/lib/msal-common/test/account/AuthToken.spec.ts index a6c163d8d8..c57698792b 100644 --- a/lib/msal-common/test/account/AuthToken.spec.ts +++ b/lib/msal-common/test/account/AuthToken.spec.ts @@ -62,6 +62,9 @@ describe("AuthToken.ts Class Unit Tests", () => { }, async signJwt(): Promise { return ""; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }; }); diff --git a/lib/msal-common/test/account/ClientInfo.spec.ts b/lib/msal-common/test/account/ClientInfo.spec.ts index 30e214f609..d6f59ec2d3 100644 --- a/lib/msal-common/test/account/ClientInfo.spec.ts +++ b/lib/msal-common/test/account/ClientInfo.spec.ts @@ -45,6 +45,9 @@ describe("ClientInfo.ts Class Unit Tests", () => { }, async signJwt(): Promise { return ""; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }; }); diff --git a/lib/msal-common/test/cache/entities/AccountEntity.spec.ts b/lib/msal-common/test/cache/entities/AccountEntity.spec.ts index 7fb83e7f39..f68a91f6e2 100644 --- a/lib/msal-common/test/cache/entities/AccountEntity.spec.ts +++ b/lib/msal-common/test/cache/entities/AccountEntity.spec.ts @@ -50,6 +50,9 @@ const cryptoInterface: ICrypto = { }, async signJwt(): Promise { return ""; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }; diff --git a/lib/msal-common/test/config/ClientConfiguration.spec.ts b/lib/msal-common/test/config/ClientConfiguration.spec.ts index ce2c90cafe..545642bca0 100644 --- a/lib/msal-common/test/config/ClientConfiguration.spec.ts +++ b/lib/msal-common/test/config/ClientConfiguration.spec.ts @@ -117,6 +117,9 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { }, async signJwt(): Promise { return "signedJwt"; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }, storageInterface: cacheStorageMock, diff --git a/lib/msal-common/test/response/ResponseHandler.spec.ts b/lib/msal-common/test/response/ResponseHandler.spec.ts index 580efe5d61..14ee44b3a4 100644 --- a/lib/msal-common/test/response/ResponseHandler.spec.ts +++ b/lib/msal-common/test/response/ResponseHandler.spec.ts @@ -71,6 +71,9 @@ const cryptoInterface: ICrypto = { }, async signJwt(): Promise { return signedJwt; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }; diff --git a/lib/msal-common/test/utils/ProtocolUtils.spec.ts b/lib/msal-common/test/utils/ProtocolUtils.spec.ts index 5c89b11562..2bfe9a5ea5 100644 --- a/lib/msal-common/test/utils/ProtocolUtils.spec.ts +++ b/lib/msal-common/test/utils/ProtocolUtils.spec.ts @@ -49,6 +49,9 @@ describe("ProtocolUtils.ts Class Unit Tests", () => { }, async signJwt(): Promise { return ""; + }, + getAsymmetricPublicKey: async(): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }; }); From f68caff161632032a5549b525f1d4fba4babca27 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 3 Aug 2021 13:42:56 -0700 Subject: [PATCH 15/44] Fix merge conflicts --- .../src/interaction_client/StandardInteractionClient.ts | 6 +++++- lib/msal-common/test/account/AuthToken.spec.ts | 3 +++ lib/msal-common/test/account/ClientInfo.spec.ts | 3 +++ lib/msal-common/test/cache/entities/AccountEntity.spec.ts | 3 +++ lib/msal-common/test/client/ClientTestUtils.ts | 3 +++ lib/msal-common/test/config/ClientConfiguration.spec.ts | 3 +++ lib/msal-common/test/crypto/KeyManager.spec.ts | 7 +------ lib/msal-common/test/response/ResponseHandler.spec.ts | 3 +++ lib/msal-common/test/utils/ProtocolUtils.spec.ts | 3 +++ 9 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 91ebd51483..9a12840c4e 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -9,7 +9,7 @@ import { BrowserConfiguration } from "../config/Configuration"; import { AuthorizationUrlRequest } from "../request/AuthorizationUrlRequest"; import { BrowserCacheManager } from "../cache/BrowserCacheManager"; import { EventHandler } from "../event/EventHandler"; -import { BrowserConstants, InteractionType, TemporaryCacheKeys } from "../utils/BrowserConstants"; +import { BrowserConstants, CryptoKeyTypes, InteractionType, TemporaryCacheKeys } from "../utils/BrowserConstants"; import { version } from "../packageMetadata"; import { BrowserAuthError } from "../error/BrowserAuthError"; import { BrowserProtocolUtils, BrowserStateObject } from "../utils/BrowserProtocolUtils"; @@ -39,6 +39,10 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { this.logger.verbose("initializeAuthorizationRequest called", request.correlationId); const generatedPkceParams = await this.browserCrypto.generatePkceCodes(); + // Generate Session Transport Key for Refresh Token Binding + const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.stk_jwk); + request.stkJwk = sessionTransportKeyThumbprint; + const authCodeRequest: CommonAuthorizationCodeRequest = { ...request, redirectUri: request.redirectUri, diff --git a/lib/msal-common/test/account/AuthToken.spec.ts b/lib/msal-common/test/account/AuthToken.spec.ts index 5ca3750d99..3ca974032c 100644 --- a/lib/msal-common/test/account/AuthToken.spec.ts +++ b/lib/msal-common/test/account/AuthToken.spec.ts @@ -63,6 +63,9 @@ describe("AuthToken.ts Class Unit Tests", () => { }, async signJwt(): Promise { return ""; + }, + async getAsymmetricPublicKey(): 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 0bf94543b1..c6da2fc79b 100644 --- a/lib/msal-common/test/account/ClientInfo.spec.ts +++ b/lib/msal-common/test/account/ClientInfo.spec.ts @@ -46,6 +46,9 @@ describe("ClientInfo.ts Class Unit Tests", () => { }, async signJwt(): Promise { return ""; + }, + async getAsymmetricPublicKey(): 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 4cb98ec556..dcb8fcc152 100644 --- a/lib/msal-common/test/cache/entities/AccountEntity.spec.ts +++ b/lib/msal-common/test/cache/entities/AccountEntity.spec.ts @@ -55,6 +55,9 @@ const cryptoInterface: ICrypto = { }, async signJwt(): Promise { return ""; + }, + async getAsymmetricPublicKey(): 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 1dd4415e65..e473fc11f3 100644 --- a/lib/msal-common/test/client/ClientTestUtils.ts +++ b/lib/msal-common/test/client/ClientTestUtils.ts @@ -152,6 +152,9 @@ export const mockCrypto = { }, async signJwt(): Promise { return ""; + }, + async getAsymmetricPublicKey(): 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 be9121bcef..4a9643bed3 100644 --- a/lib/msal-common/test/config/ClientConfiguration.spec.ts +++ b/lib/msal-common/test/config/ClientConfiguration.spec.ts @@ -117,6 +117,9 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { }, async signJwt(): Promise { return "signedJwt"; + }, + async getAsymmetricPublicKey(): Promise { + return TEST_POP_VALUES.KID } }, storageInterface: cacheStorageMock, diff --git a/lib/msal-common/test/crypto/KeyManager.spec.ts b/lib/msal-common/test/crypto/KeyManager.spec.ts index 181f82e8f1..d66ba51686 100644 --- a/lib/msal-common/test/crypto/KeyManager.spec.ts +++ b/lib/msal-common/test/crypto/KeyManager.spec.ts @@ -1,9 +1,4 @@ -import * as Mocha from "mocha"; -import * as chai from "chai"; import sinon from "sinon"; -import chaiAsPromised from "chai-as-promised"; -const expect = chai.expect; -chai.use(chaiAsPromised); import { ICrypto, PkceCodes, AuthenticationScheme } from "../../src"; import { RANDOM_TEST_GUID, TEST_POP_VALUES, TEST_DATA_CLIENT_INFO, TEST_CONFIG, TEST_URIS } from "../test_kit/StringConstants"; import { KeyManager } from "../../src/crypto/KeyManager"; @@ -74,7 +69,7 @@ describe("KeyManager Unit Tests", () => { it("Generates the req_cnf correctly", async () => { const keyManager = new KeyManager(cryptoInterface); const req_cnf = await keyManager.generateCnf(testRequest); - expect(req_cnf).to.be.eq(TEST_POP_VALUES.ENCODED_REQ_CNF); + expect(req_cnf).toBe(TEST_POP_VALUES.ENCODED_REQ_CNF); }); }); }); \ No newline at end of file diff --git a/lib/msal-common/test/response/ResponseHandler.spec.ts b/lib/msal-common/test/response/ResponseHandler.spec.ts index 29329f81af..b206befa2f 100644 --- a/lib/msal-common/test/response/ResponseHandler.spec.ts +++ b/lib/msal-common/test/response/ResponseHandler.spec.ts @@ -71,6 +71,9 @@ const cryptoInterface: ICrypto = { }, async signJwt(): Promise { return signedJwt; + }, + async getAsymmetricPublicKey(): 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 00df85eda6..c3752e6ded 100644 --- a/lib/msal-common/test/utils/ProtocolUtils.spec.ts +++ b/lib/msal-common/test/utils/ProtocolUtils.spec.ts @@ -49,6 +49,9 @@ describe("ProtocolUtils.ts Class Unit Tests", () => { }, async signJwt(): Promise { return ""; + }, + async getAsymmetricPublicKey(): Promise { + return TEST_POP_VALUES.KID } }; }); From 4d1d627f730b3a998c2a2977099b2d31ddef84f9 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 3 Aug 2021 17:56:41 -0700 Subject: [PATCH 16/44] Cleanup tests --- lib/msal-browser/test/crypto/CryptoOps.spec.ts | 3 +-- lib/msal-common/test/client/ClientTestUtils.ts | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index b9c9c9247d..7775fcfcd7 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -157,7 +157,6 @@ describe("CryptoOps.ts Unit Tests", () => { }); it("getPublicKeyThumbprint() generates a valid stk_jwk thumbprint", async () => { - jest.setTimeout(30000); //@ts-ignore jest.spyOn(BrowserCrypto.prototype as any, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { expect(algorithm).toBe("SHA-256"); @@ -187,5 +186,5 @@ describe("CryptoOps.ts Unit Tests", () => { expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); expect(regExp.test(pkThumbprint)).toBe(true); expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); - }, 20000); + }, 30000); }); diff --git a/lib/msal-common/test/client/ClientTestUtils.ts b/lib/msal-common/test/client/ClientTestUtils.ts index 4038f51bac..d11ae69396 100644 --- a/lib/msal-common/test/client/ClientTestUtils.ts +++ b/lib/msal-common/test/client/ClientTestUtils.ts @@ -155,9 +155,6 @@ export const mockCrypto = { }, async signJwt(): Promise { return ""; - }, - async getAsymmetricPublicKey(): Promise { - return TEST_POP_VALUES.KID } }; From 9aa503bfef55fe42c89c47af448ce5f1378ed075 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 3 Aug 2021 19:25:10 -0700 Subject: [PATCH 17/44] Refactor Cryptographic constants out of BrowserConstants and CryptoOps --- lib/msal-browser/src/crypto/BrowserCrypto.ts | 13 ++-- lib/msal-browser/src/crypto/CryptoOps.ts | 44 ++--------- .../StandardInteractionClient.ts | 5 +- .../src/utils/BrowserConstants.ts | 33 --------- lib/msal-browser/src/utils/CryptoConstants.ts | 73 +++++++++++++++++++ .../test/app/PublicClientApplication.spec.ts | 9 ++- .../test/crypto/CryptoOps.spec.ts | 6 +- .../interaction_client/PopupClient.spec.ts | 10 ++- .../interaction_client/RedirectClient.spec.ts | 31 ++++++-- .../SilentIframeClient.spec.ts | 6 +- .../InteractionHandler.spec.ts | 3 + .../interaction_handler/PopupHandler.spec.ts | 3 + .../RedirectHandler.spec.ts | 3 + .../interaction_handler/SilentHandler.spec.ts | 3 + 14 files changed, 148 insertions(+), 94 deletions(-) create mode 100644 lib/msal-browser/src/utils/CryptoConstants.ts diff --git a/lib/msal-browser/src/crypto/BrowserCrypto.ts b/lib/msal-browser/src/crypto/BrowserCrypto.ts index 9bdb2055b0..307205da98 100644 --- a/lib/msal-browser/src/crypto/BrowserCrypto.ts +++ b/lib/msal-browser/src/crypto/BrowserCrypto.ts @@ -5,7 +5,7 @@ import { BrowserStringUtils } from "../utils/BrowserStringUtils"; import { BrowserAuthError } from "../error/BrowserAuthError"; -import { BROWSER_CRYPTO, KEY_FORMAT_JWK } from "../utils/BrowserConstants"; +import { ALGORITHMS, KEY_FORMATS } from "../utils/CryptoConstants"; import { CryptoKeyOptions } from "./CryptoOps"; /** * See here for more info on RsaHashedKeyGenParams: https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedKeyGenParams @@ -28,8 +28,7 @@ export class BrowserCrypto { */ async sha256Digest(dataString: string): Promise { const data = BrowserStringUtils.stringToUtf8Arr(dataString); - - return this.hasIECrypto() ? this.getMSCryptoDigest(BROWSER_CRYPTO.S256_HASH_ALG, data) : this.getSubtleCryptoDigest(BROWSER_CRYPTO.S256_HASH_ALG, data); + return this.hasIECrypto() ? this.getMSCryptoDigest(ALGORITHMS.S256_HASH_ALG, data) : this.getSubtleCryptoDigest(ALGORITHMS.S256_HASH_ALG, data); } /** @@ -68,7 +67,7 @@ export class BrowserCrypto { * @param format */ async exportJwk(key: CryptoKey): Promise { - return this.hasIECrypto() ? this.msCryptoExportJwk(key) : window.crypto.subtle.exportKey(KEY_FORMAT_JWK, key); + return this.hasIECrypto() ? this.msCryptoExportJwk(key) : window.crypto.subtle.exportKey(KEY_FORMATS.JWK, key); } /** @@ -84,7 +83,7 @@ export class BrowserCrypto { return this.hasIECrypto() ? this.msCryptoImportKey(keyOptions, keyBuffer, extractable, usages) - : window.crypto.subtle.importKey(KEY_FORMAT_JWK, key, keyOptions.keyGenAlgorithmOptions, extractable, usages); + : window.crypto.subtle.importKey(KEY_FORMATS.JWK, key, keyOptions.keyGenAlgorithmOptions, extractable, usages); } /** @@ -175,7 +174,7 @@ export class BrowserCrypto { */ private async msCryptoExportJwk(key: CryptoKey): Promise { return new Promise((resolve: Function, reject: Function) => { - const msExportKey = window["msCrypto"].subtle.exportKey(KEY_FORMAT_JWK, key); + const msExportKey = window["msCrypto"].subtle.exportKey(KEY_FORMATS.JWK, key); msExportKey.addEventListener("complete", (e: { target: { result: ArrayBuffer; }; }) => { const resultBuffer: ArrayBuffer = e.target.result; @@ -208,7 +207,7 @@ export class BrowserCrypto { */ private async msCryptoImportKey(keyOptions: CryptoKeyOptions, keyBuffer: ArrayBuffer, extractable: boolean, usages: Array): Promise { return new Promise((resolve: Function, reject: Function) => { - const msImportKey = window["msCrypto"].subtle.importKey(KEY_FORMAT_JWK, keyBuffer, keyOptions.keyGenAlgorithmOptions, extractable, usages); + const msImportKey = window["msCrypto"].subtle.importKey(KEY_FORMATS.JWK, keyBuffer, keyOptions.keyGenAlgorithmOptions, extractable, usages); msImportKey.addEventListener("complete", (e: { target: { result: CryptoKey | PromiseLike; }; }) => { resolve(e.target.result); }); diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index 291718715b..10744565ce 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -11,10 +11,7 @@ import { PkceGenerator } from "./PkceGenerator"; import { BrowserCrypto } from "./BrowserCrypto"; import { DatabaseStorage } from "../cache/DatabaseStorage"; import { BrowserStringUtils } from "../utils/BrowserStringUtils"; -import { BROWSER_CRYPTO, CryptoKeyTypes, KEY_FORMAT_JWK, KEY_USAGES } from "../utils/BrowserConstants"; - -// Public Exponent used in Key Generation -const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]); +import { CryptoKeyTypes, KEY_FORMATS, CRYPTO_KEY_CONFIG } from "../utils/CryptoConstants"; export type CachedKeyPair = { publicKey: CryptoKey, @@ -40,10 +37,7 @@ export class CryptoOps implements ICrypto { private b64Encode: Base64Encode; private b64Decode: Base64Decode; private pkceGenerator: PkceGenerator; - private _atBindingKeyOptions: CryptoKeyOptions; - private _rtBindingKeyOptions: CryptoKeyOptions; - private static POP_KEY_USAGES: Array = ["sign", "verify"]; private static EXTRACTABLE: boolean = true; private static DB_VERSION = 1; @@ -59,32 +53,6 @@ export class CryptoOps implements ICrypto { this.guidGenerator = new GuidGenerator(this.browserCrypto); this.pkceGenerator = new PkceGenerator(this.browserCrypto); this.cache = new DatabaseStorage(CryptoOps.DB_NAME, CryptoOps.TABLE_NAME, CryptoOps.DB_VERSION); - - this._atBindingKeyOptions = { - keyGenAlgorithmOptions: { - name: BROWSER_CRYPTO.PKCS1_V15_KEYGEN_ALG, - hash: { - name: BROWSER_CRYPTO.S256_HASH_ALG - }, - modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, - publicExponent: PUBLIC_EXPONENT - }, - keypairUsages: KEY_USAGES.AT_BINDING.KEYPAIR as KeyUsage[], - privateKeyUsage: KEY_USAGES.AT_BINDING.PRIVATE_KEY as KeyUsage[] - }; - - this._rtBindingKeyOptions = { - keyGenAlgorithmOptions: { - name: BROWSER_CRYPTO.RSA_OAEP, - hash: { - name: BROWSER_CRYPTO.S256_HASH_ALG - }, - modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, - publicExponent: PUBLIC_EXPONENT - }, - keypairUsages: KEY_USAGES.RT_BINDING.KEYPAIR as KeyUsage[], - privateKeyUsage: KEY_USAGES.RT_BINDING.PRIVATE_KEY as KeyUsage[] - }; } /** @@ -126,11 +94,11 @@ export class CryptoOps implements ICrypto { let keyOptions: CryptoKeyOptions; switch(keyType) { - case CryptoKeyTypes.stk_jwk: - keyOptions = this._rtBindingKeyOptions; + case CryptoKeyTypes.STK_JWK: + keyOptions = CRYPTO_KEY_CONFIG.RT_BINDING; break; default: - keyOptions = this._atBindingKeyOptions; + keyOptions = CRYPTO_KEY_CONFIG.AT_BINDING; } // Generate Keypair @@ -182,7 +150,7 @@ export class CryptoOps implements ICrypto { // Generate header const header = { alg: publicKeyJwk.alg, - type: KEY_FORMAT_JWK + type: KEY_FORMATS.JWK }; const encodedHeader = this.b64Encode.urlEncode(JSON.stringify(header)); @@ -197,7 +165,7 @@ export class CryptoOps implements ICrypto { // Sign token const tokenBuffer = BrowserStringUtils.stringToArrayBuffer(tokenString); - const signatureBuffer = await this.browserCrypto.sign(this._atBindingKeyOptions, cachedKeyPair.privateKey, tokenBuffer); + const signatureBuffer = await this.browserCrypto.sign(CRYPTO_KEY_CONFIG.AT_BINDING, cachedKeyPair.privateKey, tokenBuffer); const encodedSignature = this.b64Encode.urlEncodeArr(new Uint8Array(signatureBuffer)); return `${tokenString}.${encodedSignature}`; diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 9a12840c4e..7ffcd15c3d 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -9,7 +9,7 @@ import { BrowserConfiguration } from "../config/Configuration"; import { AuthorizationUrlRequest } from "../request/AuthorizationUrlRequest"; import { BrowserCacheManager } from "../cache/BrowserCacheManager"; import { EventHandler } from "../event/EventHandler"; -import { BrowserConstants, CryptoKeyTypes, InteractionType, TemporaryCacheKeys } from "../utils/BrowserConstants"; +import { BrowserConstants, InteractionType, TemporaryCacheKeys } from "../utils/BrowserConstants"; import { version } from "../packageMetadata"; import { BrowserAuthError } from "../error/BrowserAuthError"; import { BrowserProtocolUtils, BrowserStateObject } from "../utils/BrowserProtocolUtils"; @@ -19,6 +19,7 @@ import { INavigationClient } from "../navigation/INavigationClient"; import { RedirectRequest } from "../request/RedirectRequest"; import { PopupRequest } from "../request/PopupRequest"; import { SsoSilentRequest } from "../request/SsoSilentRequest"; +import { CryptoKeyTypes } from "../utils/CryptoConstants"; /** * Defines the class structure and helper functions used by the "standard", non-brokered auth flows (popup, redirect, silent (RT), silent (iframe)) @@ -40,7 +41,7 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { const generatedPkceParams = await this.browserCrypto.generatePkceCodes(); // Generate Session Transport Key for Refresh Token Binding - const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.stk_jwk); + const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.STK_JWK); request.stkJwk = sessionTransportKeyThumbprint; const authCodeRequest: CommonAuthorizationCodeRequest = { diff --git a/lib/msal-browser/src/utils/BrowserConstants.ts b/lib/msal-browser/src/utils/BrowserConstants.ts index d56ed4b6c1..da564e9e70 100644 --- a/lib/msal-browser/src/utils/BrowserConstants.ts +++ b/lib/msal-browser/src/utils/BrowserConstants.ts @@ -146,41 +146,8 @@ export const DEFAULT_REQUEST: RedirectRequest|PopupRequest = { scopes: OIDC_DEFAULT_SCOPES }; -/** - * JWK Key Format string (Type MUST be defined for window crypto APIs) - */ -export const KEY_FORMAT_JWK = "jwk"; - // Supported wrapper SKUs export enum WrapperSKU { React = "@azure/msal-react", Angular = "@azure/msal-angular" } - -// Supported Cryptographic Key Types -export enum CryptoKeyTypes { - req_cnf = "req_cnf", - stk_jwk = "stk_jwk" -} - -// Crypto Key Usage sets -export const KEY_USAGES = { - AT_BINDING: { - KEYPAIR: ["sign", "verify"], - PRIVATE_KEY: ["sign"] - }, - RT_BINDING: { - KEYPAIR: ["encrypt", "decrypt"], - PRIVATE_KEY: ["decrypt"] - } -}; - -// Cryptographic Constants -export const BROWSER_CRYPTO = { - PKCS1_V15_KEYGEN_ALG: "RSASSA-PKCS1-v1_5", - RSA_OAEP: "RSA-OAEP", - AES_GCM: "AES-GCM", - DIRECT: "dir", - S256_HASH_ALG: "SHA-256", - MODULUS_LENGTH: 2048 -}; diff --git a/lib/msal-browser/src/utils/CryptoConstants.ts b/lib/msal-browser/src/utils/CryptoConstants.ts new file mode 100644 index 0000000000..16307e6a1b --- /dev/null +++ b/lib/msal-browser/src/utils/CryptoConstants.ts @@ -0,0 +1,73 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +// Cryptographic Algorithms used/supported +export enum ALGORITHMS { + PKCS1_V15_KEYGEN_ALG = "RSASSA-PKCS1-v1_5", + RSA_OAEP = "RSA-OAEP", + AES_GCM = "AES-GCM", + DIRECT = "dir", + S256_HASH_ALG = "SHA-256", +} + +// Numerical constants relating to biy/bytelength +export enum LENGTHS { + MODULUS = 2048 +} + +// Public Exponent used in Key Generation +export const PUBLIC_EXPONENT = new Uint8Array([0x01, 0x00, 0x01]); + +// Supported Cryptographic Key Types +export enum CryptoKeyTypes { + REQ_CNF = "req_cnf", + STK_JWK = "stk_jwk" +} + +/** + * JWK Key Format string (Type MUST be defined for window crypto APIs) + */ +export enum KEY_FORMATS { + JWK = "jwk" +} + +// Crypto Key Usage sets +export const KEY_USAGES = { + AT_BINDING: { + KEYPAIR: ["sign", "verify"], + PRIVATE_KEY: ["sign"] + }, + RT_BINDING: { + KEYPAIR: ["encrypt", "decrypt"], + PRIVATE_KEY: ["decrypt"] + } +}; + +export const CRYPTO_KEY_CONFIG = { + AT_BINDING: { + keyGenAlgorithmOptions: { + name: ALGORITHMS.PKCS1_V15_KEYGEN_ALG, + hash: { + name: ALGORITHMS.S256_HASH_ALG + }, + modulusLength: LENGTHS.MODULUS, + publicExponent: PUBLIC_EXPONENT + }, + keypairUsages: KEY_USAGES.AT_BINDING.KEYPAIR as KeyUsage[], + privateKeyUsage: KEY_USAGES.AT_BINDING.PRIVATE_KEY as KeyUsage[] + }, + RT_BINDING: { + keyGenAlgorithmOptions: { + name: ALGORITHMS.RSA_OAEP, + hash: { + name: ALGORITHMS.S256_HASH_ALG + }, + modulusLength: LENGTHS.MODULUS, + publicExponent: PUBLIC_EXPONENT + }, + keypairUsages: KEY_USAGES.RT_BINDING.KEYPAIR as KeyUsage[], + privateKeyUsage: KEY_USAGES.RT_BINDING.PRIVATE_KEY as KeyUsage[] + } +}; diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index 1b597456fc..a64d87dc7f 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -5,7 +5,7 @@ import sinon from "sinon"; import { PublicClientApplication } from "../../src/app/PublicClientApplication"; -import { TEST_CONFIG, TEST_URIS, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, testNavUrl, testLogoutUrl, TEST_STATE_VALUES, TEST_HASHES, DEFAULT_TENANT_DISCOVERY_RESPONSE, DEFAULT_OPENID_CONFIG_RESPONSE, testNavUrlNoRequest } from "../utils/StringConstants"; +import { TEST_CONFIG, TEST_URIS, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, testNavUrl, testLogoutUrl, TEST_STATE_VALUES, TEST_HASHES, DEFAULT_TENANT_DISCOVERY_RESPONSE, DEFAULT_OPENID_CONFIG_RESPONSE, testNavUrlNoRequest, TEST_POP_VALUES } from "../utils/StringConstants"; import { ServerError, Constants, AccountInfo, TokenClaims, AuthenticationResult, CommonAuthorizationUrlRequest, AuthorizationCodeClient, ResponseMode, AccountEntity, ProtocolUtils, AuthenticationScheme, RefreshTokenClient, Logger, ServerTelemetryEntity, CommonSilentFlowRequest, LogLevel, CommonAuthorizationCodeRequest } from "@azure/msal-common"; import { ApiId, InteractionType, WrapperSKU, TemporaryCacheKeys, BrowserConstants, BrowserCacheLocation } from "../../src/utils/BrowserConstants"; import { CryptoOps } from "../../src/crypto/CryptoOps"; @@ -303,7 +303,8 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + // @ts-ignore pca.loginRedirect(null); }); @@ -1066,6 +1067,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); sinon.stub(ProtocolUtils, "setRequestState").returns(TEST_STATE_VALUES.TEST_STATE_SILENT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const CommonSilentFlowRequest: SilentRequest = { scopes: ["User.Read"], account: testAccount, @@ -1086,7 +1088,8 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT }; const tokenResp = await pca.acquireTokenSilent(CommonSilentFlowRequest); diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index daa183aa71..0b72a75f05 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -154,10 +154,9 @@ describe("CryptoOps.ts Unit Tests", () => { expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); expect(regExp.test(pkThumbprint)).toBe(true); expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); - }); + }, 30000); it("getPublicKeyThumbprint() generates a valid stk_jwk thumbprint", async () => { - jest.setTimeout(10000); //@ts-ignore jest.spyOn(BrowserCrypto.prototype as any, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { expect(algorithm).toBe("SHA-256"); @@ -172,7 +171,6 @@ describe("CryptoOps.ts Unit Tests", () => { authority: TEST_CONFIG.validAuthority, scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, correlationId: TEST_CONFIG.CORRELATION_ID, - stkJwk: TEST_POP_VALUES.KID }; const pkThumbprint = await cryptoObj.getPublicKeyThumbprint(testRequest, keyType); @@ -187,5 +185,5 @@ describe("CryptoOps.ts Unit Tests", () => { expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); expect(regExp.test(pkThumbprint)).toBe(true); expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); - }, 20000); + }, 30000); }); diff --git a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index 48a46c88c0..3b800dfe81 100644 --- a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts @@ -5,7 +5,7 @@ import sinon from "sinon"; import { PublicClientApplication } from "../../src/app/PublicClientApplication"; -import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, testNavUrl, TEST_STATE_VALUES } from "../utils/StringConstants"; +import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, testNavUrl, TEST_STATE_VALUES, TEST_POP_VALUES } from "../utils/StringConstants"; import { Constants, AccountInfo, TokenClaims, AuthenticationResult, CommonAuthorizationUrlRequest, AuthorizationCodeClient, ResponseMode, AuthenticationScheme, ServerTelemetryEntity } from "@azure/msal-common"; import { BrowserConstants, TemporaryCacheKeys, ApiId } from "../../src/utils/BrowserConstants"; import { BrowserAuthErrorMessage, BrowserAuthError } from "../../src/error/BrowserAuthError"; @@ -75,6 +75,8 @@ describe("PopupClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + const popupSpy = sinon.stub(PopupUtils, "openSizedPopup"); try { @@ -99,6 +101,8 @@ describe("PopupClient", () => { challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER }); + + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const request: CommonAuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, @@ -177,6 +181,8 @@ describe("PopupClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + const tokenResp = await popupClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: TEST_CONFIG.DEFAULT_SCOPES @@ -196,6 +202,8 @@ describe("PopupClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + 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 b9ddcf54cf..a3c5b35557 100644 --- a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts @@ -5,7 +5,7 @@ import sinon from "sinon"; import { PublicClientApplication } from "../../src/app/PublicClientApplication"; -import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, DEFAULT_OPENID_CONFIG_RESPONSE, testNavUrl, TEST_STATE_VALUES, DEFAULT_TENANT_DISCOVERY_RESPONSE, testLogoutUrl } from "../utils/StringConstants"; +import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, DEFAULT_OPENID_CONFIG_RESPONSE, testNavUrl, TEST_STATE_VALUES, DEFAULT_TENANT_DISCOVERY_RESPONSE, testLogoutUrl, TEST_POP_VALUES } from "../utils/StringConstants"; import { ServerError, Constants, AccountInfo, TokenClaims, AuthenticationResult, CommonAuthorizationCodeRequest, CommonAuthorizationUrlRequest, AuthToken, PersistentCacheKeys, AuthorizationCodeClient, ResponseMode, ProtocolUtils, AuthenticationScheme, Logger, ServerTelemetryEntity, LogLevel, NetworkResponse, ServerAuthorizationTokenResponse, CcsCredential, CcsCredentialType, CommonEndSessionRequest, ServerTelemetryManager } from "@azure/msal-common"; import { BrowserUtils } from "../../src/utils/BrowserUtils"; import { BrowserConstants, TemporaryCacheKeys, ApiId, BrowserCacheLocation, InteractionType } from "../../src/utils/BrowserConstants"; @@ -745,6 +745,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const loginRequest: CommonAuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: ["user.read"], @@ -777,6 +778,7 @@ describe("RedirectClient", () => { }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -825,6 +827,7 @@ describe("RedirectClient", () => { }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -881,6 +884,7 @@ describe("RedirectClient", () => { }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -920,6 +924,7 @@ describe("RedirectClient", () => { expect(urlNavigate).not.toBe(""); return Promise.resolve(true); }); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const browserCrypto = new CryptoOps(); const testLogger = new Logger(loggerOptions); @@ -931,6 +936,7 @@ describe("RedirectClient", () => { expect(cachedRequest.authority).toEqual(`${Constants.DEFAULT_AUTHORITY}`); expect(cachedRequest.correlationId).toEqual(RANDOM_TEST_GUID); expect(cachedRequest.authenticationScheme).toEqual(TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme); + expect(cachedRequest.stkJwk).toEqual(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); }); it("Cleans cache before error is thrown", async () => { @@ -952,6 +958,7 @@ describe("RedirectClient", () => { challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER }); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const testError = { errorCode: "create_login_url_error", @@ -995,6 +1002,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1021,7 +1029,8 @@ describe("RedirectClient", () => { authority: `${Constants.DEFAULT_AUTHORITY}`, responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD + codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, + stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT }; expect(loginUrlSpy.calledWith(validatedRequest)).toBeTruthy(); }); @@ -1049,6 +1058,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1075,7 +1085,8 @@ describe("RedirectClient", () => { nonce: RANDOM_TEST_GUID, responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD + codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, + stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT }; expect(loginUrlSpy.calledWith(validatedRequest)).toBeTruthy(); }); @@ -1090,6 +1101,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const loginRequest: RedirectRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: ["user.read", "openid", "profile"], @@ -1121,6 +1133,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const loginRequest: RedirectRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: ["user.read", "openid", "profile"], @@ -1147,6 +1160,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1178,6 +1192,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1193,6 +1208,7 @@ describe("RedirectClient", () => { expect(cachedRequest.authority).toEqual(`${Constants.DEFAULT_AUTHORITY}`); expect(cachedRequest.correlationId).toEqual(RANDOM_TEST_GUID); expect(cachedRequest.authenticationScheme).toEqual(TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme); + expect(cachedRequest.stkJwk).toEqual(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); }); it("Cleans cache before error is thrown", async () => { @@ -1214,6 +1230,7 @@ describe("RedirectClient", () => { challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER }); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const testError = { errorCode: "create_login_url_error", @@ -1258,6 +1275,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1284,7 +1302,8 @@ describe("RedirectClient", () => { nonce: RANDOM_TEST_GUID, responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD + codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, + stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT }; expect(acquireTokenUrlSpy.calledWith(validatedRequest)).toBeTruthy(); }); @@ -1312,6 +1331,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1339,7 +1359,8 @@ describe("RedirectClient", () => { nonce: RANDOM_TEST_GUID, responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, - codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD + codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, + stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT }; expect(acquireTokenUrlSpy.calledWith(validatedRequest)).toBeTruthy(); }); diff --git a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts index b531d1ef22..c503938b4a 100644 --- a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts @@ -5,7 +5,7 @@ import sinon from "sinon"; import { PublicClientApplication } from "../../src/app/PublicClientApplication"; -import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, testNavUrl } from "../utils/StringConstants"; +import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, testNavUrl, TEST_POP_VALUES } from "../utils/StringConstants"; import { AccountInfo, TokenClaims, PromptValue, AuthenticationResult, CommonAuthorizationUrlRequest, AuthorizationCodeClient, ResponseMode, AuthenticationScheme, ServerTelemetryManager } from "@azure/msal-common"; import { BrowserAuthError } from "../../src/error/BrowserAuthError"; import { SilentHandler } from "../../src/interaction_handler/SilentHandler"; @@ -66,6 +66,7 @@ describe("SilentIframeClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const telemetryStub = sinon.stub(ServerTelemetryManager.prototype, "cacheFailedRequest").callsFake((e) => { expect(e).toMatchObject(BrowserAuthError.createMonitorIframeTimeoutError()); }); @@ -130,6 +131,8 @@ describe("SilentIframeClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint" @@ -187,6 +190,7 @@ describe("SilentIframeClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: TEST_CONFIG.DEFAULT_SCOPES, diff --git a/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts b/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts index 121de99717..022ad04a58 100644 --- a/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts @@ -100,6 +100,9 @@ const cryptoInterface = { }, signJwt: async (): Promise => { return "signedJwt"; + }, + getAsymmetricPublicKey: async (): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT } } diff --git a/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts b/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts index 82819f8d09..f95452d725 100644 --- a/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts @@ -89,6 +89,9 @@ describe("PopupHandler.ts Unit Tests", () => { }, signJwt: async (): Promise => { return "signedJwt"; + }, + getAsymmetricPublicKey: async (): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT } }, networkInterface: { diff --git a/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts b/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts index d26866e4b7..b58979fb8f 100644 --- a/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts @@ -105,6 +105,9 @@ describe("RedirectHandler.ts Unit Tests", () => { }, signJwt: async (): Promise => { return "signedJwt"; + }, + getAsymmetricPublicKey: async (): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT } }, storageInterface: browserStorage, diff --git a/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts b/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts index 5e33ac379c..d11e018223 100644 --- a/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts @@ -89,6 +89,9 @@ describe("SilentHandler.ts Unit Tests", () => { }, signJwt: async (): Promise => { return "signedJwt"; + }, + getAsymmetricPublicKey: async (): Promise => { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT } }, networkInterface: { From b98766e8523975db6f48b8838c8c8eb0659cfb8b Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 3 Aug 2021 19:40:30 -0700 Subject: [PATCH 18/44] Fix generatePublicKeyThumbprint stubs and expected values on tests --- .../test/app/PublicClientApplication.spec.ts | 6 +-- .../interaction_client/PopupClient.spec.ts | 8 ++-- .../interaction_client/RedirectClient.spec.ts | 42 +++++++++---------- .../SilentIframeClient.spec.ts | 6 +-- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index a64d87dc7f..5f891cbc13 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -303,7 +303,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); // @ts-ignore pca.loginRedirect(null); @@ -1067,7 +1067,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); sinon.stub(ProtocolUtils, "setRequestState").returns(TEST_STATE_VALUES.TEST_STATE_SILENT); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const CommonSilentFlowRequest: SilentRequest = { scopes: ["User.Read"], account: testAccount, @@ -1089,7 +1089,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, - stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT + stkJwk: TEST_POP_VALUES.KID }; const tokenResp = await pca.acquireTokenSilent(CommonSilentFlowRequest); diff --git a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index 3b800dfe81..56af2a6137 100644 --- a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts @@ -75,7 +75,7 @@ describe("PopupClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const popupSpy = sinon.stub(PopupUtils, "openSizedPopup"); @@ -102,7 +102,7 @@ describe("PopupClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const request: CommonAuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, @@ -181,7 +181,7 @@ describe("PopupClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const tokenResp = await popupClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, @@ -202,7 +202,7 @@ describe("PopupClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); try { await popupClient.acquireToken({ diff --git a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts index a3c5b35557..a94ddf854a 100644 --- a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts @@ -745,7 +745,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const loginRequest: CommonAuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: ["user.read"], @@ -778,7 +778,7 @@ describe("RedirectClient", () => { }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -827,7 +827,7 @@ describe("RedirectClient", () => { }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -884,7 +884,7 @@ describe("RedirectClient", () => { }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -924,7 +924,7 @@ describe("RedirectClient", () => { expect(urlNavigate).not.toBe(""); return Promise.resolve(true); }); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const browserCrypto = new CryptoOps(); const testLogger = new Logger(loggerOptions); @@ -936,7 +936,7 @@ describe("RedirectClient", () => { expect(cachedRequest.authority).toEqual(`${Constants.DEFAULT_AUTHORITY}`); expect(cachedRequest.correlationId).toEqual(RANDOM_TEST_GUID); expect(cachedRequest.authenticationScheme).toEqual(TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme); - expect(cachedRequest.stkJwk).toEqual(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + expect(cachedRequest.stkJwk).toEqual(TEST_POP_VALUES.KID); }); it("Cleans cache before error is thrown", async () => { @@ -958,7 +958,7 @@ describe("RedirectClient", () => { challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER }); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const testError = { errorCode: "create_login_url_error", @@ -1002,7 +1002,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1030,7 +1030,7 @@ describe("RedirectClient", () => { responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, - stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT + stkJwk: TEST_POP_VALUES.KID }; expect(loginUrlSpy.calledWith(validatedRequest)).toBeTruthy(); }); @@ -1058,7 +1058,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1086,7 +1086,7 @@ describe("RedirectClient", () => { responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, - stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT + stkJwk: TEST_POP_VALUES.KID }; expect(loginUrlSpy.calledWith(validatedRequest)).toBeTruthy(); }); @@ -1101,7 +1101,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const loginRequest: RedirectRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: ["user.read", "openid", "profile"], @@ -1133,7 +1133,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const loginRequest: RedirectRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: ["user.read", "openid", "profile"], @@ -1160,7 +1160,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1192,7 +1192,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1208,7 +1208,7 @@ describe("RedirectClient", () => { expect(cachedRequest.authority).toEqual(`${Constants.DEFAULT_AUTHORITY}`); expect(cachedRequest.correlationId).toEqual(RANDOM_TEST_GUID); expect(cachedRequest.authenticationScheme).toEqual(TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme); - expect(cachedRequest.stkJwk).toEqual(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + expect(cachedRequest.stkJwk).toEqual(TEST_POP_VALUES.KID); }); it("Cleans cache before error is thrown", async () => { @@ -1230,7 +1230,7 @@ describe("RedirectClient", () => { challenge: TEST_CONFIG.TEST_CHALLENGE, verifier: TEST_CONFIG.TEST_VERIFIER }); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const testError = { errorCode: "create_login_url_error", @@ -1275,7 +1275,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1303,7 +1303,7 @@ describe("RedirectClient", () => { responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, - stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT + stkJwk: TEST_POP_VALUES.KID }; expect(acquireTokenUrlSpy.calledWith(validatedRequest)).toBeTruthy(); }); @@ -1331,7 +1331,7 @@ describe("RedirectClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise => { expect(options.noHistory).toBeFalsy(); expect(urlNavigate).not.toBe(""); @@ -1360,7 +1360,7 @@ describe("RedirectClient", () => { responseMode: ResponseMode.FRAGMENT, codeChallenge: TEST_CONFIG.TEST_CHALLENGE, codeChallengeMethod: Constants.S256_CODE_CHALLENGE_METHOD, - stkJwk: TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT + stkJwk: TEST_POP_VALUES.KID }; expect(acquireTokenUrlSpy.calledWith(validatedRequest)).toBeTruthy(); }); diff --git a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts index c503938b4a..e66f6097e9 100644 --- a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts @@ -66,7 +66,7 @@ describe("SilentIframeClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const telemetryStub = sinon.stub(ServerTelemetryManager.prototype, "cacheFailedRequest").callsFake((e) => { expect(e).toMatchObject(BrowserAuthError.createMonitorIframeTimeoutError()); }); @@ -131,7 +131,7 @@ describe("SilentIframeClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, @@ -190,7 +190,7 @@ describe("SilentIframeClient", () => { verifier: TEST_CONFIG.TEST_VERIFIER }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); - sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT); + sinon.stub(CryptoOps.prototype, "getPublicKeyThumbprint").resolves(TEST_POP_VALUES.KID); const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, scopes: TEST_CONFIG.DEFAULT_SCOPES, From 1f7b0a1440a9fd2bf6ec50959c5abefb1f4f2b7b Mon Sep 17 00:00:00 2001 From: HectorMg Date: Mon, 9 Aug 2021 13:34:30 -0700 Subject: [PATCH 19/44] Fix tests after merge --- lib/msal-browser/test/crypto/CryptoOps.spec.ts | 2 +- lib/msal-common/test/account/ClientInfo.spec.ts | 8 +++----- lib/msal-common/test/crypto/KeyManager.spec.ts | 8 +++++++- lib/msal-common/test/test_kit/StringConstants.ts | 4 +++- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index 59df539de5..a234c3b178 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -3,7 +3,7 @@ import { GuidGenerator } from "../../src/crypto/GuidGenerator"; import { BrowserCrypto } from "../../src/crypto/BrowserCrypto"; import { TEST_CONFIG, TEST_URIS, BROWSER_CRYPTO, KEY_USAGES, TEST_POP_VALUES } from "../utils/StringConstants"; import { createHash } from "crypto"; -import { AuthenticationScheme, PkceCodes } from "@azure/msal-common"; +import { AuthenticationScheme, BaseAuthRequest, PkceCodes } from "@azure/msal-common"; import { DatabaseStorage } from "../../src/cache/DatabaseStorage"; import { BrowserAuthError, BrowserAuthErrorMessage } from "../../src"; const msrCrypto = require("../polyfills/msrcrypto.min"); diff --git a/lib/msal-common/test/account/ClientInfo.spec.ts b/lib/msal-common/test/account/ClientInfo.spec.ts index 4ba63279a2..1bfb131f6c 100644 --- a/lib/msal-common/test/account/ClientInfo.spec.ts +++ b/lib/msal-common/test/account/ClientInfo.spec.ts @@ -47,16 +47,14 @@ describe("ClientInfo.ts Class Unit Tests", () => { async signJwt(): Promise { return ""; }, -<<<<<<< HEAD - async getAsymmetricPublicKey(): Promise { - return TEST_POP_VALUES.KID; -======= async removeTokenBindingKey(): Promise { return Promise.resolve(true); }, async clearKeystore(): Promise { return Promise.resolve(true); ->>>>>>> dev + }, + async getAsymmetricPublicKey(): Promise { + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }; }); diff --git a/lib/msal-common/test/crypto/KeyManager.spec.ts b/lib/msal-common/test/crypto/KeyManager.spec.ts index d66ba51686..1ce4fd1e0a 100644 --- a/lib/msal-common/test/crypto/KeyManager.spec.ts +++ b/lib/msal-common/test/crypto/KeyManager.spec.ts @@ -50,9 +50,15 @@ describe("KeyManager Unit Tests", () => { }, async signJwt(): Promise { return ""; + }, + async removeTokenBindingKey(): Promise { + return Promise.resolve(true); + }, + async clearKeystore(): Promise { + return Promise.resolve(true); }, async getAsymmetricPublicKey(): Promise { - return TEST_POP_VALUES.KID; + return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; } }; diff --git a/lib/msal-common/test/test_kit/StringConstants.ts b/lib/msal-common/test/test_kit/StringConstants.ts index 9afda5cb90..7e4c9ae60d 100644 --- a/lib/msal-common/test/test_kit/StringConstants.ts +++ b/lib/msal-common/test/test_kit/StringConstants.ts @@ -149,7 +149,9 @@ export const TEST_POP_VALUES = { DECODED_REQ_CNF: "{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\",\"xms_ksl\":\"sw\"}", SAMPLE_POP_AT: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", SAMPLE_POP_AT_PAYLOAD_ENCODED: "eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ", - SAMPLE_POP_AT_PAYLOAD_DECODED: "{\"sub\":\"1234567890\",\"name\":\"John Doe\",\"iat\":1516239022,\"cnf\":{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\"}}" + SAMPLE_POP_AT_PAYLOAD_DECODED: "{\"sub\":\"1234567890\",\"name\":\"John Doe\",\"iat\":1516239022,\"cnf\":{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\"}}", + ENCODED_STK_JWK_THUMBPRINT: "%7B%22kid%22%3A%22NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs%22%7D", + DECODED_STK_JWK_THUMBPRINT: "{\"kid\":\"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs\"}" }; export const TEST_ACCOUNT_INFO: AccountInfo = { From 80ac691704efdb3cef5a3eacf79ba6a4e7be9003 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Mon, 4 Oct 2021 18:08:44 -0700 Subject: [PATCH 20/44] Add feature flag to make RT Binding opt-in --- lib/msal-browser/src/config/Configuration.ts | 5 ++++- lib/msal-browser/test/app/PublicClientApplication.spec.ts | 3 ++- lib/msal-browser/test/interaction_client/PopupClient.spec.ts | 3 ++- .../test/interaction_client/RedirectClient.spec.ts | 3 ++- lib/msal-common/src/config/ClientConfiguration.ts | 3 +++ 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts index d66731dc22..ea45a8daa8 100644 --- a/lib/msal-browser/src/config/Configuration.ts +++ b/lib/msal-browser/src/config/Configuration.ts @@ -26,6 +26,7 @@ export const DEFAULT_REDIRECT_TIMEOUT_MS = 30000; * - navigateToLoginRequestUrl - Boolean indicating whether to navigate to the original request URL after the auth server navigates to the redirect URL. * - clientCapabilities - Array of capabilities which will be added to the claims.access_token.xms_cc request property on every network request. * - protocolMode - Enum that represents the protocol that msal follows. Used for configuring proper endpoints. + * - refreshTokenBinding - Boolean that enables refresh token binding (a.k.a refresh token proof-of-possession) for authorization requests */ export type BrowserAuthOptions = { clientId: string; @@ -38,6 +39,7 @@ export type BrowserAuthOptions = { navigateToLoginRequestUrl?: boolean; clientCapabilities?: Array; protocolMode?: ProtocolMode; + refreshTokenBinding?: boolean; }; /** @@ -122,7 +124,8 @@ export function buildConfiguration({ auth: userInputAuth, cache: userInputCache, postLogoutRedirectUri: "", navigateToLoginRequestUrl: true, clientCapabilities: [], - protocolMode: ProtocolMode.AAD + protocolMode: ProtocolMode.AAD, + refreshTokenBinding: false }; // Default cache options for browser diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index 5cf570210b..490daa077d 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -33,7 +33,8 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { beforeEach(() => { pca = new PublicClientApplication({ auth: { - clientId: TEST_CONFIG.MSAL_CLIENT_ID + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + refreshTokenBinding: true } }); }); diff --git a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index e86911d8b8..668956c45c 100644 --- a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts @@ -22,7 +22,8 @@ describe("PopupClient", () => { beforeEach(() => { const pca = new PublicClientApplication({ auth: { - clientId: TEST_CONFIG.MSAL_CLIENT_ID + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + refreshTokenBinding: true } }); //@ts-ignore diff --git a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts index 3f093e35c7..ce2ad3e6da 100644 --- a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts @@ -44,7 +44,8 @@ describe("RedirectClient", () => { beforeEach(() => { const pca = new PublicClientApplication({ auth: { - clientId: TEST_CONFIG.MSAL_CLIENT_ID + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + refreshTokenBinding: true } }); sinon.stub(CryptoOps.prototype, "createNewGuid").returns(RANDOM_TEST_GUID); diff --git a/lib/msal-common/src/config/ClientConfiguration.ts b/lib/msal-common/src/config/ClientConfiguration.ts index 98b5d9a8f2..f7624695ac 100644 --- a/lib/msal-common/src/config/ClientConfiguration.ts +++ b/lib/msal-common/src/config/ClientConfiguration.ts @@ -68,11 +68,13 @@ export type CommonClientConfiguration = { * - cloudDiscoveryMetadata - A string containing the cloud discovery response. Used in AAD scenarios. * - clientCapabilities - Array of capabilities which will be added to the claims.access_token.xms_cc request property on every network request. * - protocolMode - Enum that represents the protocol that msal follows. Used for configuring proper endpoints. + * - refreshTokenBinding - Boolean that enables refresh token binding (a.k.a refresh token proof-of-possession) for authorization requests */ export type AuthOptions = { clientId: string; authority: Authority; clientCapabilities?: Array; + refreshTokenBinding?: boolean; }; /** @@ -204,6 +206,7 @@ export function buildClientConfiguration( function buildAuthOptions(authOptions: AuthOptions): Required { return { clientCapabilities: [], + refreshTokenBinding: false, ...authOptions }; } From 94f0451ad7053d2f5d1788ea55dde193536bbbb0 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 10:37:17 -0700 Subject: [PATCH 21/44] Add error handling to STK generation step --- lib/msal-browser/src/cache/DatabaseStorage.ts | 4 +++- lib/msal-browser/src/error/BrowserAuthError.ts | 11 +++++++++++ .../StandardInteractionClient.ts | 17 ++++++++++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/msal-browser/src/cache/DatabaseStorage.ts b/lib/msal-browser/src/cache/DatabaseStorage.ts index 49587fb917..943245dc36 100644 --- a/lib/msal-browser/src/cache/DatabaseStorage.ts +++ b/lib/msal-browser/src/cache/DatabaseStorage.ts @@ -52,7 +52,9 @@ export class DatabaseStorage{ resolve(); }); - openDB.addEventListener("error", error => reject(error)); + openDB.addEventListener("error", () => { + reject(BrowserAuthError.createDatabaseUnavailableError()); + }); }); } diff --git a/lib/msal-browser/src/error/BrowserAuthError.ts b/lib/msal-browser/src/error/BrowserAuthError.ts index 1b7ee50807..626dcbcb16 100644 --- a/lib/msal-browser/src/error/BrowserAuthError.ts +++ b/lib/msal-browser/src/error/BrowserAuthError.ts @@ -152,6 +152,10 @@ export const BrowserAuthErrorMessage = { signingKeyNotFoundInStorage: { code: "crypto_key_not_found", desc: "Cryptographic Key or Keypair not found in browser storage." + }, + datdabaseUnavailable: { + code: "database_unavailable", + desc: "IndexedDB, which is required for cryptographic key storage, is unavailable. This may happen when browsing in private mode." } }; @@ -441,4 +445,11 @@ export class BrowserAuthError extends AuthError { static createSigningKeyNotFoundInStorageError(keyId: string): BrowserAuthError { return new BrowserAuthError(BrowserAuthErrorMessage.signingKeyNotFoundInStorage.code, `${BrowserAuthErrorMessage.signingKeyNotFoundInStorage.desc} | No match found for KeyId: ${keyId}`); } + + /** + * Create an error when IndexedDB is unavailable + */ + static createDatabaseUnavailableError(): BrowserAuthError { + return new BrowserAuthError(BrowserAuthErrorMessage.datdabaseUnavailable.code, BrowserAuthErrorMessage.datdabaseUnavailable.desc); + } } diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index ae57bc07c0..5d9b9c614f 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -39,9 +39,20 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { this.logger.verbose("initializeAuthorizationRequest called", request.correlationId); const generatedPkceParams = await this.browserCrypto.generatePkceCodes(); - // Generate Session Transport Key for Refresh Token Binding - const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.stk_jwk); - request.stkJwk = sessionTransportKeyThumbprint; + // Generate Session Transport Key if Refresh Token Binding is enabled + if (this.config.auth.refreshTokenBinding) { + this.logger.verbose("Refresh token binding enabled, attempting to generate Session Transport Key"); + try { + const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.stk_jwk); + request.stkJwk = sessionTransportKeyThumbprint; + this.logger.verbose("Successfully generated and stored Session Transport Key"); + } catch(error) { + if(error instanceof BrowserAuthError) { + this.logger.error(error.message); + this.logger.verbose("Could not generate Session Transport Key, refresh token will be unbound"); + } + } + } const authCodeRequest: CommonAuthorizationCodeRequest = { ...request, From 7ca16e8ace00681960f11e99fc9b47559c306b30 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 10:55:40 -0700 Subject: [PATCH 22/44] Refactor crypto enum names --- lib/msal-browser/src/crypto/BrowserCrypto.ts | 12 +++++----- lib/msal-browser/src/crypto/CryptoOps.ts | 4 ++-- lib/msal-browser/src/utils/CryptoConstants.ts | 22 +++++++++---------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/msal-browser/src/crypto/BrowserCrypto.ts b/lib/msal-browser/src/crypto/BrowserCrypto.ts index 307205da98..7e4c713b4f 100644 --- a/lib/msal-browser/src/crypto/BrowserCrypto.ts +++ b/lib/msal-browser/src/crypto/BrowserCrypto.ts @@ -5,7 +5,7 @@ import { BrowserStringUtils } from "../utils/BrowserStringUtils"; import { BrowserAuthError } from "../error/BrowserAuthError"; -import { ALGORITHMS, KEY_FORMATS } from "../utils/CryptoConstants"; +import { Algorithms, CryptoKeyFormats } from "../utils/CryptoConstants"; import { CryptoKeyOptions } from "./CryptoOps"; /** * See here for more info on RsaHashedKeyGenParams: https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedKeyGenParams @@ -28,7 +28,7 @@ export class BrowserCrypto { */ async sha256Digest(dataString: string): Promise { const data = BrowserStringUtils.stringToUtf8Arr(dataString); - return this.hasIECrypto() ? this.getMSCryptoDigest(ALGORITHMS.S256_HASH_ALG, data) : this.getSubtleCryptoDigest(ALGORITHMS.S256_HASH_ALG, data); + return this.hasIECrypto() ? this.getMSCryptoDigest(Algorithms.S256_HASH_ALG, data) : this.getSubtleCryptoDigest(Algorithms.S256_HASH_ALG, data); } /** @@ -67,7 +67,7 @@ export class BrowserCrypto { * @param format */ async exportJwk(key: CryptoKey): Promise { - return this.hasIECrypto() ? this.msCryptoExportJwk(key) : window.crypto.subtle.exportKey(KEY_FORMATS.JWK, key); + return this.hasIECrypto() ? this.msCryptoExportJwk(key) : window.crypto.subtle.exportKey(CryptoKeyFormats.jwk, key); } /** @@ -83,7 +83,7 @@ export class BrowserCrypto { return this.hasIECrypto() ? this.msCryptoImportKey(keyOptions, keyBuffer, extractable, usages) - : window.crypto.subtle.importKey(KEY_FORMATS.JWK, key, keyOptions.keyGenAlgorithmOptions, extractable, usages); + : window.crypto.subtle.importKey(CryptoKeyFormats.jwk, key, keyOptions.keyGenAlgorithmOptions, extractable, usages); } /** @@ -174,7 +174,7 @@ export class BrowserCrypto { */ private async msCryptoExportJwk(key: CryptoKey): Promise { return new Promise((resolve: Function, reject: Function) => { - const msExportKey = window["msCrypto"].subtle.exportKey(KEY_FORMATS.JWK, key); + const msExportKey = window["msCrypto"].subtle.exportKey(CryptoKeyFormats.jwk, key); msExportKey.addEventListener("complete", (e: { target: { result: ArrayBuffer; }; }) => { const resultBuffer: ArrayBuffer = e.target.result; @@ -207,7 +207,7 @@ export class BrowserCrypto { */ private async msCryptoImportKey(keyOptions: CryptoKeyOptions, keyBuffer: ArrayBuffer, extractable: boolean, usages: Array): Promise { return new Promise((resolve: Function, reject: Function) => { - const msImportKey = window["msCrypto"].subtle.importKey(KEY_FORMATS.JWK, keyBuffer, keyOptions.keyGenAlgorithmOptions, extractable, usages); + const msImportKey = window["msCrypto"].subtle.importKey(CryptoKeyFormats.jwk, keyBuffer, keyOptions.keyGenAlgorithmOptions, extractable, usages); msImportKey.addEventListener("complete", (e: { target: { result: CryptoKey | PromiseLike; }; }) => { resolve(e.target.result); }); diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index 61b3edf900..167b5dbe4e 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -11,7 +11,7 @@ import { PkceGenerator } from "./PkceGenerator"; import { BrowserCrypto } from "./BrowserCrypto"; import { DatabaseStorage } from "../cache/DatabaseStorage"; import { BrowserStringUtils } from "../utils/BrowserStringUtils"; -import { KEY_FORMATS, CRYPTO_KEY_CONFIG } from "../utils/CryptoConstants"; +import { CryptoKeyFormats, CRYPTO_KEY_CONFIG } from "../utils/CryptoConstants"; import { BrowserAuthError } from "../error/BrowserAuthError"; export type CachedKeyPair = { @@ -169,7 +169,7 @@ export class CryptoOps implements ICrypto { // Generate header const header = { alg: publicKeyJwk.alg, - type: KEY_FORMATS.JWK + type: CryptoKeyFormats.jwk }; const encodedHeader = this.b64Encode.urlEncode(JSON.stringify(header)); diff --git a/lib/msal-browser/src/utils/CryptoConstants.ts b/lib/msal-browser/src/utils/CryptoConstants.ts index 5f592d8474..8f5c710406 100644 --- a/lib/msal-browser/src/utils/CryptoConstants.ts +++ b/lib/msal-browser/src/utils/CryptoConstants.ts @@ -4,7 +4,7 @@ */ // Cryptographic Algorithms used/supported -export enum ALGORITHMS { +export enum Algorithms { PKCS1_V15_KEYGEN_ALG = "RSASSA-PKCS1-v1_5", RSA_OAEP = "RSA-OAEP", AES_GCM = "AES-GCM", @@ -13,8 +13,8 @@ export enum ALGORITHMS { } // Numerical constants relating to biy/bytelength -export enum LENGTHS { - MODULUS = 2048 +export enum Lengths { + modulus = 2048 } // Public Exponent used in Key Generation @@ -23,8 +23,8 @@ export const PUBLIC_EXPONENT = new Uint8Array([0x01, 0x00, 0x01]); /** * JWK Key Format string (Type MUST be defined for window crypto APIs) */ -export enum KEY_FORMATS { - JWK = "jwk" +export enum CryptoKeyFormats { + jwk = "jwk" } // Crypto Key Usage sets @@ -42,11 +42,11 @@ export const KEY_USAGES = { export const CRYPTO_KEY_CONFIG = { AT_BINDING: { keyGenAlgorithmOptions: { - name: ALGORITHMS.PKCS1_V15_KEYGEN_ALG, + name: Algorithms.PKCS1_V15_KEYGEN_ALG, hash: { - name: ALGORITHMS.S256_HASH_ALG + name: Algorithms.S256_HASH_ALG }, - modulusLength: LENGTHS.MODULUS, + modulusLength: Lengths.modulus, publicExponent: PUBLIC_EXPONENT }, keypairUsages: KEY_USAGES.AT_BINDING.KEYPAIR as KeyUsage[], @@ -54,11 +54,11 @@ export const CRYPTO_KEY_CONFIG = { }, RT_BINDING: { keyGenAlgorithmOptions: { - name: ALGORITHMS.RSA_OAEP, + name: Algorithms.RSA_OAEP, hash: { - name: ALGORITHMS.S256_HASH_ALG + name: Algorithms.S256_HASH_ALG }, - modulusLength: LENGTHS.MODULUS, + modulusLength: Lengths.modulus, publicExponent: PUBLIC_EXPONENT }, keypairUsages: KEY_USAGES.RT_BINDING.KEYPAIR as KeyUsage[], From 5aa90092d091bcb8b1494ec5e35902e739fc84cd Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 11:31:14 -0700 Subject: [PATCH 23/44] Add error handling for crypto key generation --- lib/msal-browser/src/crypto/BrowserCrypto.ts | 7 ++++- lib/msal-browser/src/crypto/CryptoOps.ts | 31 ++++++++++--------- .../src/error/BrowserAuthError.ts | 11 +++++++ 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/lib/msal-browser/src/crypto/BrowserCrypto.ts b/lib/msal-browser/src/crypto/BrowserCrypto.ts index 7e4c713b4f..2a1f0671a8 100644 --- a/lib/msal-browser/src/crypto/BrowserCrypto.ts +++ b/lib/msal-browser/src/crypto/BrowserCrypto.ts @@ -6,10 +6,15 @@ import { BrowserStringUtils } from "../utils/BrowserStringUtils"; import { BrowserAuthError } from "../error/BrowserAuthError"; import { Algorithms, CryptoKeyFormats } from "../utils/CryptoConstants"; -import { CryptoKeyOptions } from "./CryptoOps"; + /** * See here for more info on RsaHashedKeyGenParams: https://developer.mozilla.org/en-US/docs/Web/API/RsaHashedKeyGenParams */ +export type CryptoKeyOptions = { + keyGenAlgorithmOptions: RsaHashedKeyGenParams, + keypairUsages: KeyUsage[], + privateKeyUsage: KeyUsage[] +}; /** * This class implements functions used by the browser library to perform cryptography operations such as diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index 167b5dbe4e..98254da27d 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -8,7 +8,7 @@ import { GuidGenerator } from "./GuidGenerator"; import { Base64Encode } from "../encode/Base64Encode"; import { Base64Decode } from "../encode/Base64Decode"; import { PkceGenerator } from "./PkceGenerator"; -import { BrowserCrypto } from "./BrowserCrypto"; +import { BrowserCrypto, CryptoKeyOptions } from "./BrowserCrypto"; import { DatabaseStorage } from "../cache/DatabaseStorage"; import { BrowserStringUtils } from "../utils/BrowserStringUtils"; import { CryptoKeyFormats, CRYPTO_KEY_CONFIG } from "../utils/CryptoConstants"; @@ -21,12 +21,6 @@ export type CachedKeyPair = { requestUri?: string }; -export type CryptoKeyOptions = { - keyGenAlgorithmOptions: RsaHashedKeyGenParams, - keypairUsages: KeyUsage[], - privateKeyUsage: KeyUsage[] -}; - /** * This class implements MSAL's crypto interface, which allows it to perform base64 encoding and decoding, generating cryptographically random GUIDs and * implementing Proof Key for Code Exchange specs for the OAuth Authorization Code Flow using PKCE (rfc here: https://tools.ietf.org/html/rfc7636). @@ -105,6 +99,10 @@ export class CryptoOps implements ICrypto { // Generate Keypair const keyPair = await this.browserCrypto.generateKeyPair(keyOptions, CryptoOps.EXTRACTABLE); + if (!keyPair || !keyPair.publicKey) { + throw BrowserAuthError.createKeyGenerationFailedError(); + } + // Generate Thumbprint for Public Key const publicKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.publicKey); @@ -118,20 +116,25 @@ export class CryptoOps implements ICrypto { const publicJwkString: string = BrowserCrypto.getJwkString(pubKeyThumprintObj); const publicJwkBuffer: ArrayBuffer = await this.browserCrypto.sha256Digest(publicJwkString); const publicJwkHash: string = this.b64Encode.urlEncodeArr(new Uint8Array(publicJwkBuffer)); - - // Generate Thumbprint for Private Key - const privateKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.privateKey); - // Re-import private key to make it unextractable - const unextractablePrivateKey: CryptoKey = await this.browserCrypto.importJwk(keyOptions, privateKeyJwk, false, keyOptions.privateKeyUsage); + + // When asymmetric keypair is generated, there is also a private key + let unextractablePrivateKey; + + if (keyPair.privateKey) { + // Generate Thumbprint for Private Key + const privateKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.privateKey); + // Re-import private key to make it unextractable + unextractablePrivateKey = await this.browserCrypto.importJwk(keyOptions, privateKeyJwk, false, keyOptions.privateKeyUsage); + } // Store Keypair data in keystore await this.cache.put(publicJwkHash, { - privateKey: unextractablePrivateKey, + privateKey: unextractablePrivateKey as CryptoKey, publicKey: keyPair.publicKey, requestMethod: request.resourceRequestMethod, requestUri: request.resourceRequestUri }); - + return publicJwkHash; } diff --git a/lib/msal-browser/src/error/BrowserAuthError.ts b/lib/msal-browser/src/error/BrowserAuthError.ts index c703516faf..654d3680e3 100644 --- a/lib/msal-browser/src/error/BrowserAuthError.ts +++ b/lib/msal-browser/src/error/BrowserAuthError.ts @@ -152,6 +152,10 @@ export const BrowserAuthErrorMessage = { datdabaseUnavailable: { code: "database_unavailable", desc: "IndexedDB, which is required for cryptographic key storage, is unavailable. This may happen when browsing in private mode." + }, + keyGenerationFailed: { + code: "key_generation_failed", + desc: "Failed to generate cryptographic key" } }; @@ -441,4 +445,11 @@ export class BrowserAuthError extends AuthError { static createDatabaseUnavailableError(): BrowserAuthError { return new BrowserAuthError(BrowserAuthErrorMessage.datdabaseUnavailable.code, BrowserAuthErrorMessage.datdabaseUnavailable.desc); } + + /** + * Create an error when key generation failed + */ + static createKeyGenerationFailedError(): BrowserAuthError { + return new BrowserAuthError(BrowserAuthErrorMessage.keyGenerationFailed.code, BrowserAuthErrorMessage.keyGenerationFailed.desc); + } } From 93d5145fdd41d112bcf77fc7987e5aaaf40e63fb Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 11:49:01 -0700 Subject: [PATCH 24/44] Put KeyManager instance in BaseClient instead of AuthCode and Refresh Clients --- lib/msal-browser/test/utils/StringConstants.ts | 2 +- lib/msal-common/src/client/AuthorizationCodeClient.ts | 4 +--- lib/msal-common/src/client/BaseClient.ts | 7 +++++++ lib/msal-common/src/client/RefreshTokenClient.ts | 4 +--- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/msal-browser/test/utils/StringConstants.ts b/lib/msal-browser/test/utils/StringConstants.ts index 9a6f9245ee..1de441165c 100644 --- a/lib/msal-browser/test/utils/StringConstants.ts +++ b/lib/msal-browser/test/utils/StringConstants.ts @@ -23,7 +23,7 @@ export const TEST_URIS = { TEST_AUTH_ENDPT: "https://login.microsoftonline.com/common/oauth2/v2.0/authorize", TEST_END_SESSION_ENDPOINT: "https://login.microsoftonline.com/common/oauth2/v2.0/logout", TEST_AUTH_ENDPT_WITH_PARAMS: "https://login.microsoftonline.com/common/oauth2/v2.0/authorize?param1=value1¶m2=value2", - TEST_RESOURCE_ENDPT_WITH_PARAMS: "https://localhost:8081/endpoint?param1=value1¶m2=value2", + TEST_RESOURCE_ENDPT_WITH_PARAMS: "https://localhost:8081/endpoint?param1=value1¶m2=value2" }; // Test MSAL config params diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 8601b994f9..a217a71063 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -27,7 +27,6 @@ import { TokenClaims } from "../account/TokenClaims"; import { AccountInfo } from "../account/AccountInfo"; import { buildClientInfoFromHomeAccountId, buildClientInfo } from "../account/ClientInfo"; import { CcsCredentialType, CcsCredential } from "../account/CcsCredential"; -import { KeyManager } from "../crypto/KeyManager"; /** * Oauth2.0 Authorization Code client @@ -222,8 +221,7 @@ export class AuthorizationCodeClient extends BaseClient { parameterBuilder.addClientInfo(); if (request.authenticationScheme === AuthenticationScheme.POP) { - const keyManager = new KeyManager(this.cryptoUtils); - const cnfString = await keyManager.generateCnf(request); + const cnfString = await this.keyManager.generateCnf(request); parameterBuilder.addPopToken(cnfString); } diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 9b10e675e5..d40ce782fc 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -18,6 +18,7 @@ import { version, name } from "../packageMetadata"; import { ClientAuthError } from "../error/ClientAuthError"; import { CcsCredential, CcsCredentialType } from "../account/CcsCredential"; import { buildClientInfoFromHomeAccountId } from "../account/ClientInfo"; +import { KeyManager } from ".."; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. @@ -47,6 +48,9 @@ export abstract class BaseClient { // Default authority object public authority: Authority; + // Define Key Manager object + protected keyManager: KeyManager; + protected constructor(configuration: ClientConfiguration) { // Set the configuration this.config = buildClientConfiguration(configuration); @@ -71,6 +75,9 @@ export abstract class BaseClient { // set Authority this.authority = this.config.authOptions.authority; + + // set KeyManager + this.keyManager = new KeyManager(this.cryptoUtils); } /** diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 3e3513e9c0..4c136a43f5 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -20,7 +20,6 @@ import { ClientConfigurationError } from "../error/ClientConfigurationError"; import { ClientAuthError } from "../error/ClientAuthError"; import { ServerError } from "../error/ServerError"; import { TimeUtils } from "../utils/TimeUtils"; -import { KeyManager } from "../crypto/KeyManager"; import { UrlString } from "../url/UrlString"; import { CcsCredentialType } from "../account/CcsCredential"; import { buildClientInfoFromHomeAccountId } from "../account/ClientInfo"; @@ -204,8 +203,7 @@ export class RefreshTokenClient extends BaseClient { } if (request.authenticationScheme === AuthenticationScheme.POP) { - const keyManager = new KeyManager(this.cryptoUtils); - const cnfString = await keyManager.generateCnf(request); + const cnfString = await this.keyManager.generateCnf(request); parameterBuilder.addPopToken(cnfString); } From 90e19c53a31eb8512d4f3259865f24d2232a81e5 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 11:53:45 -0700 Subject: [PATCH 25/44] Fix import in BaseClient --- lib/msal-common/src/client/BaseClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index d40ce782fc..33fa93b03e 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -18,7 +18,7 @@ import { version, name } from "../packageMetadata"; import { ClientAuthError } from "../error/ClientAuthError"; import { CcsCredential, CcsCredentialType } from "../account/CcsCredential"; import { buildClientInfoFromHomeAccountId } from "../account/ClientInfo"; -import { KeyManager } from ".."; +import { KeyManager } from "../crypto/KeyManager"; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. From 38ca6876456eecf7c660f0304296a9531402463d Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 12:04:47 -0700 Subject: [PATCH 26/44] Extend KeyManager tests --- .../test/crypto/KeyManager.spec.ts | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/lib/msal-common/test/crypto/KeyManager.spec.ts b/lib/msal-common/test/crypto/KeyManager.spec.ts index 1ce4fd1e0a..043ca8baee 100644 --- a/lib/msal-common/test/crypto/KeyManager.spec.ts +++ b/lib/msal-common/test/crypto/KeyManager.spec.ts @@ -1,5 +1,5 @@ import sinon from "sinon"; -import { ICrypto, PkceCodes, AuthenticationScheme } from "../../src"; +import { ICrypto, PkceCodes, AuthenticationScheme, CryptoKeyTypes } from "../../src"; import { RANDOM_TEST_GUID, TEST_POP_VALUES, TEST_DATA_CLIENT_INFO, TEST_CONFIG, TEST_URIS } from "../test_kit/StringConstants"; import { KeyManager } from "../../src/crypto/KeyManager"; @@ -62,20 +62,32 @@ describe("KeyManager Unit Tests", () => { } }; - describe("generateCnf", () => { - const testRequest = { - authority: TEST_CONFIG.validAuthority, - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, - correlationId: TEST_CONFIG.CORRELATION_ID, - authenticationScheme: AuthenticationScheme.POP, - resourceRequestMethod:"POST", - resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS - }; + let keyManager: KeyManager; + + const testPopRequest = { + authority: TEST_CONFIG.validAuthority, + scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + correlationId: TEST_CONFIG.CORRELATION_ID, + authenticationScheme: AuthenticationScheme.POP, + resourceRequestMethod:"POST", + resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS + }; + + beforeEach(() => { + keyManager = new KeyManager(cryptoInterface); + }); - it("Generates the req_cnf correctly", async () => { - const keyManager = new KeyManager(cryptoInterface); - const req_cnf = await keyManager.generateCnf(testRequest); + describe("generateCnf", () => { + it("generates the req_cnf correctly", async () => { + const req_cnf = await keyManager.generateCnf(testPopRequest); expect(req_cnf).toBe(TEST_POP_VALUES.ENCODED_REQ_CNF); }); }); + + describe("generateKid", () => { + it("returns the correct kid and key storage location", async () => { + const req_cnf = await keyManager.generateKid(testPopRequest, CryptoKeyTypes.req_cnf); + expect(req_cnf).toStrictEqual(JSON.parse(TEST_POP_VALUES.DECODED_REQ_CNF)); + }); + }); }); \ No newline at end of file From 5030726b6e22a5d07ff42f1dce7770d2c77f6be8 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 13:14:41 -0700 Subject: [PATCH 27/44] Increase test coverage --- .../src/error/BrowserAuthError.ts | 4 +- .../test/crypto/CryptoOps.spec.ts | 128 ++++++++++-------- .../test/error/BrowserAuthError.spec.ts | 26 ++++ .../test/client/RefreshTokenClient.spec.ts | 58 ++++++-- 4 files changed, 147 insertions(+), 69 deletions(-) diff --git a/lib/msal-browser/src/error/BrowserAuthError.ts b/lib/msal-browser/src/error/BrowserAuthError.ts index 654d3680e3..6d55471995 100644 --- a/lib/msal-browser/src/error/BrowserAuthError.ts +++ b/lib/msal-browser/src/error/BrowserAuthError.ts @@ -149,7 +149,7 @@ export const BrowserAuthErrorMessage = { code: "crypto_key_not_found", desc: "Cryptographic Key or Keypair not found in browser storage." }, - datdabaseUnavailable: { + databaseUnavailable: { code: "database_unavailable", desc: "IndexedDB, which is required for cryptographic key storage, is unavailable. This may happen when browsing in private mode." }, @@ -443,7 +443,7 @@ export class BrowserAuthError extends AuthError { * Create an error when IndexedDB is unavailable */ static createDatabaseUnavailableError(): BrowserAuthError { - return new BrowserAuthError(BrowserAuthErrorMessage.datdabaseUnavailable.code, BrowserAuthErrorMessage.datdabaseUnavailable.desc); + return new BrowserAuthError(BrowserAuthErrorMessage.databaseUnavailable.code, BrowserAuthErrorMessage.databaseUnavailable.desc); } /** diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index 85d8ed2965..693354cf02 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -1,11 +1,11 @@ import { CryptoOps, CachedKeyPair } from "../../src/crypto/CryptoOps"; import { GuidGenerator } from "../../src/crypto/GuidGenerator"; import { BrowserCrypto } from "../../src/crypto/BrowserCrypto"; -import { TEST_CONFIG, TEST_URIS, BROWSER_CRYPTO, KEY_USAGES, TEST_POP_VALUES } from "../utils/StringConstants"; +import { TEST_URIS, BROWSER_CRYPTO, KEY_USAGES, TEST_POP_VALUES } from "../utils/StringConstants"; import { createHash } from "crypto"; import { AuthenticationScheme, BaseAuthRequest, CryptoKeyTypes, PkceCodes } from "@azure/msal-common"; import { DatabaseStorage } from "../../src/cache/DatabaseStorage"; -import { BrowserAuthError, BrowserAuthErrorMessage } from "../../src"; +import { BrowserAuthError } from "../../src"; const msrCrypto = require("../polyfills/msrcrypto.min"); const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]); @@ -130,62 +130,6 @@ describe("CryptoOps.ts Unit Tests", () => { 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, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { - expect(algorithm).toBe("SHA-256"); - return Promise.resolve(createHash("SHA256").update(Buffer.from(data)).digest()); - }); - const generateKeyPairSpy = jest.spyOn(BrowserCrypto.prototype, "generateKeyPair"); - const exportJwkSpy = jest.spyOn(BrowserCrypto.prototype, "exportJwk"); - - const keyType = CryptoKeyTypes.req_cnf; - - const testRequest = { - authenticationScheme: AuthenticationScheme.POP, - resourceRequestMethod:"POST", - resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS, - stkJwk: TEST_POP_VALUES.KID - }; - - const pkThumbprint = await cryptoObj.getPublicKeyThumbprint(testRequest, keyType); - /** - * Contains alphanumeric, dash '-', underscore '_', plus '+', or slash '/' with length of 43. - */ - const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); - const result = await generateKeyPairSpy.mock.results[0].value; - expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(AT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); - expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); - expect(regExp.test(pkThumbprint)).toBe(true); - expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); - }, 30000); - - it("getPublicKeyThumbprint() generates a valid stk_jwk thumbprint", async () => { - //@ts-ignore - jest.spyOn(BrowserCrypto.prototype as any, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { - expect(algorithm).toBe("SHA-256"); - return Promise.resolve(createHash("SHA256").update(Buffer.from(data)).digest()); - }); - const generateKeyPairSpy = jest.spyOn(BrowserCrypto.prototype, "generateKeyPair"); - const exportJwkSpy = jest.spyOn(BrowserCrypto.prototype, "exportJwk"); - - const keyType = CryptoKeyTypes.stk_jwk; - - const pkThumbprint = await cryptoObj.getPublicKeyThumbprint({}, keyType); - /** - * Contains alphanumeric, dash '-', underscore '_', plus '+', or slash '/' with length of 43. - */ - const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); - const result = await generateKeyPairSpy.mock.results[0].value; - - expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(RT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); - expect(result.privateKey.algorithm.name.toLowerCase()).toEqual(RT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); - expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); - expect(regExp.test(pkThumbprint)).toBe(true); - expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); - }, 30000); - it("removeTokenBindingKey() removes the specified key from storage", async () => { //@ts-ignore jest.spyOn(BrowserCrypto.prototype as any, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { @@ -202,4 +146,72 @@ describe("CryptoOps.ts Unit Tests", () => { jest.spyOn(DatabaseStorage.prototype, "get").mockResolvedValue(undefined); return await expect(cryptoObj.signJwt({}, "testString")).rejects.toThrow(BrowserAuthError.createSigningKeyNotFoundInStorageError("testString")); }, 30000); + + describe("getPublicKeyThumbprint", () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("generates a valid request thumbprint", async () => { + jest.setTimeout(30000); + //@ts-ignore + jest.spyOn(BrowserCrypto.prototype as any, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { + expect(algorithm).toBe("SHA-256"); + return Promise.resolve(createHash("SHA256").update(Buffer.from(data)).digest()); + }); + const generateKeyPairSpy = jest.spyOn(BrowserCrypto.prototype, "generateKeyPair"); + const exportJwkSpy = jest.spyOn(BrowserCrypto.prototype, "exportJwk"); + + const keyType = CryptoKeyTypes.req_cnf; + + const testRequest = { + authenticationScheme: AuthenticationScheme.POP, + resourceRequestMethod:"POST", + resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS, + stkJwk: TEST_POP_VALUES.KID + }; + + const pkThumbprint = await cryptoObj.getPublicKeyThumbprint(testRequest, keyType); + /** + * Contains alphanumeric, dash '-', underscore '_', plus '+', or slash '/' with length of 43. + */ + const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); + const result = await generateKeyPairSpy.mock.results[0].value; + expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(AT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); + expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); + expect(regExp.test(pkThumbprint)).toBe(true); + expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); + }, 30000); + + it("generates a valid stk_jwk thumbprint", async () => { + //@ts-ignore + jest.spyOn(BrowserCrypto.prototype as any, "getSubtleCryptoDigest").mockImplementation((algorithm: string, data: Uint8Array): Promise => { + expect(algorithm).toBe("SHA-256"); + return Promise.resolve(createHash("SHA256").update(Buffer.from(data)).digest()); + }); + const generateKeyPairSpy = jest.spyOn(BrowserCrypto.prototype, "generateKeyPair"); + const exportJwkSpy = jest.spyOn(BrowserCrypto.prototype, "exportJwk"); + + const keyType = CryptoKeyTypes.stk_jwk; + + const pkThumbprint = await cryptoObj.getPublicKeyThumbprint({}, keyType); + /** + * Contains alphanumeric, dash '-', underscore '_', plus '+', or slash '/' with length of 43. + */ + const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); + const result = await generateKeyPairSpy.mock.results[0].value; + + expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(RT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); + expect(result.privateKey.algorithm.name.toLowerCase()).toEqual(RT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); + expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); + expect(regExp.test(pkThumbprint)).toBe(true); + expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); + }, 30000); + + it("throws error if key generation fails", async () => { + //@ts-ignore + jest.spyOn(BrowserCrypto.prototype, "generateKeyPair").mockReturnValue(undefined); + expect(() => cryptoObj.getPublicKeyThumbprint({})).rejects.toThrow(BrowserAuthError.createKeyGenerationFailedError()); + }); + }); }); diff --git a/lib/msal-browser/test/error/BrowserAuthError.spec.ts b/lib/msal-browser/test/error/BrowserAuthError.spec.ts index 6c66c128d8..e0ee7a4b87 100644 --- a/lib/msal-browser/test/error/BrowserAuthError.spec.ts +++ b/lib/msal-browser/test/error/BrowserAuthError.spec.ts @@ -283,4 +283,30 @@ describe("BrowserAuthError Unit Tests", () => { expect(err.name).toBe("BrowserAuthError"); expect(err.stack?.includes("BrowserAuthError.spec.ts")).toBe(true); }); + + it("createDatabaseUnavailableError()", () => { + const err: BrowserAuthError = BrowserAuthError.createDatabaseUnavailableError(); + + expect(err instanceof BrowserAuthError).toBe(true); + expect(err instanceof AuthError).toBe(true); + expect(err instanceof Error).toBe(true); + expect(err.errorCode).toBe(BrowserAuthErrorMessage.databaseUnavailable.code); + expect(err.errorMessage?.includes(BrowserAuthErrorMessage.databaseUnavailable.desc)).toBe(true); + expect(err.message?.includes(BrowserAuthErrorMessage.databaseUnavailable.desc)).toBe(true); + expect(err.name).toBe("BrowserAuthError"); + expect(err.stack?.includes("BrowserAuthError.spec.ts")).toBe(true); + }); + + it("createKeyGenerationFailedError()", () => { + const err: BrowserAuthError = BrowserAuthError.createKeyGenerationFailedError(); + + expect(err instanceof BrowserAuthError).toBe(true); + expect(err instanceof AuthError).toBe(true); + expect(err instanceof Error).toBe(true); + expect(err.errorCode).toBe(BrowserAuthErrorMessage.keyGenerationFailed.code); + expect(err.errorMessage?.includes(BrowserAuthErrorMessage.keyGenerationFailed.desc)).toBe(true); + expect(err.message?.includes(BrowserAuthErrorMessage.keyGenerationFailed.desc)).toBe(true); + expect(err.name).toBe("BrowserAuthError"); + expect(err.stack?.includes("BrowserAuthError.spec.ts")).toBe(true); + }); }); diff --git a/lib/msal-common/test/client/RefreshTokenClient.spec.ts b/lib/msal-common/test/client/RefreshTokenClient.spec.ts index 3f61e8c600..d5f54582ee 100644 --- a/lib/msal-common/test/client/RefreshTokenClient.spec.ts +++ b/lib/msal-common/test/client/RefreshTokenClient.spec.ts @@ -12,7 +12,8 @@ import { TEST_DATA_CLIENT_INFO, ID_TOKEN_CLAIMS, AUTHENTICATION_RESULT_WITH_FOCI, - CORS_SIMPLE_REQUEST_HEADERS + CORS_SIMPLE_REQUEST_HEADERS, + TEST_POP_VALUES } from "../test_kit/StringConstants"; import { BaseClient} from "../../src/client/BaseClient"; import { AADServerParamKeys, GrantType, Constants, CredentialType, AuthenticationScheme, ThrottlingConstants } from "../../src/utils/Constants"; @@ -101,7 +102,7 @@ describe("RefreshTokenClient unit tests", () => { claims: TEST_CONFIG.CLAIMS, authority: TEST_CONFIG.validAuthority, correlationId: TEST_CONFIG.CORRELATION_ID, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + authenticationScheme: AuthenticationScheme.BEARER, tokenQueryParameters: { testParam: "testValue" } @@ -165,7 +166,7 @@ describe("RefreshTokenClient unit tests", () => { claims: TEST_CONFIG.CLAIMS, authority: TEST_CONFIG.validAuthority, correlationId: TEST_CONFIG.CORRELATION_ID, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: AuthenticationScheme.BEARER }; client.acquireToken(refreshTokenRequest); @@ -181,7 +182,7 @@ describe("RefreshTokenClient unit tests", () => { claims: TEST_CONFIG.CLAIMS, authority: TEST_CONFIG.validAuthority, correlationId: TEST_CONFIG.CORRELATION_ID, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: AuthenticationScheme.BEARER }; const authResult: AuthenticationResult = await client.acquireToken(refreshTokenRequest); @@ -223,7 +224,7 @@ describe("RefreshTokenClient unit tests", () => { const expectedRefreshRequest: CommonRefreshTokenRequest = { ...silentFlowRequest, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + authenticationScheme: AuthenticationScheme.BEARER, refreshToken: testRefreshTokenEntity.secret, ccsCredential: { credential: testAccount.homeAccountId, @@ -245,7 +246,7 @@ describe("RefreshTokenClient unit tests", () => { refreshToken: TEST_TOKENS.REFRESH_TOKEN, authority: TEST_CONFIG.validAuthority, correlationId: TEST_CONFIG.CORRELATION_ID, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: AuthenticationScheme.BEARER }; const authResult: AuthenticationResult = await client.acquireToken(refreshTokenRequest); @@ -285,7 +286,7 @@ describe("RefreshTokenClient unit tests", () => { authority: TEST_CONFIG.validAuthority, claims: "{}", correlationId: TEST_CONFIG.CORRELATION_ID, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: AuthenticationScheme.BEARER }; const authResult: AuthenticationResult = await client.acquireToken(refreshTokenRequest); @@ -314,6 +315,45 @@ describe("RefreshTokenClient unit tests", () => { expect(result.includes(`${AADServerParamKeys.X_CLIENT_CPU}=${TEST_CONFIG.TEST_CPU}`)).toBe(true); expect(result.includes(`${AADServerParamKeys.X_MS_LIB_CAPABILITY}=${ThrottlingConstants.X_MS_LIB_CAPABILITY_VALUE}`)).toBe(true); }); + + it.only("adds req_cnf if authenticationScheme is POP", async () => { + sinon.stub(RefreshTokenClient.prototype, "executePostToTokenEndpoint").resolves(AUTHENTICATION_RESULT); + const createTokenRequestBodySpy = sinon.spy(RefreshTokenClient.prototype, "createTokenRequestBody"); + const client = new RefreshTokenClient(config); + const refreshTokenRequest: CommonRefreshTokenRequest = { + scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + refreshToken: TEST_TOKENS.REFRESH_TOKEN, + authority: TEST_CONFIG.validAuthority, + correlationId: TEST_CONFIG.CORRELATION_ID, + authenticationScheme: AuthenticationScheme.POP + }; + + const authResult: AuthenticationResult = await client.acquireToken(refreshTokenRequest); + const expectedScopes = [Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, TEST_CONFIG.DEFAULT_GRAPH_SCOPE[0], "email"]; + + expect(authResult.uniqueId).toEqual(ID_TOKEN_CLAIMS.oid); + expect(authResult.tenantId).toEqual(ID_TOKEN_CLAIMS.tid); + expect(authResult.scopes).toEqual(expectedScopes); + expect(authResult.account).toEqual(testAccount); + expect(authResult.idToken).toEqual(AUTHENTICATION_RESULT.body.id_token); + expect(authResult.idTokenClaims).toEqual(ID_TOKEN_CLAIMS); + expect(authResult.accessToken).toEqual(AUTHENTICATION_RESULT.body.access_token); + expect(authResult.state).toBe(""); + expect(createTokenRequestBodySpy.calledWith(refreshTokenRequest)).toBe(true); + + const result = await createTokenRequestBodySpy.returnValues[0] as string; + expect(result.includes(`${TEST_CONFIG.DEFAULT_GRAPH_SCOPE[0]}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.CLIENT_ID}=${TEST_CONFIG.MSAL_CLIENT_ID}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.REFRESH_TOKEN}=${TEST_TOKENS.REFRESH_TOKEN}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.GRANT_TYPE}=${GrantType.REFRESH_TOKEN_GRANT}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.CLIENT_SECRET}=${TEST_CONFIG.MSAL_CLIENT_SECRET}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.REQ_CNF}=${TEST_POP_VALUES.ENCODED_REQ_CNF}`)); + expect(result.includes(`${AADServerParamKeys.X_CLIENT_SKU}=${Constants.SKU}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.X_CLIENT_VER}=${TEST_CONFIG.TEST_VERSION}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.X_CLIENT_OS}=${TEST_CONFIG.TEST_OS}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.X_CLIENT_CPU}=${TEST_CONFIG.TEST_CPU}`)).toBe(true); + expect(result.includes(`${AADServerParamKeys.X_MS_LIB_CAPABILITY}=${ThrottlingConstants.X_MS_LIB_CAPABILITY_VALUE}`)).toBe(true); + }) }); describe("acquireToken APIs with FOCI enabled", () => { @@ -359,7 +399,7 @@ describe("RefreshTokenClient unit tests", () => { claims: TEST_CONFIG.CLAIMS, authority: TEST_CONFIG.validAuthority, correlationId: TEST_CONFIG.CORRELATION_ID, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + authenticationScheme: AuthenticationScheme.BEARER }; const authResult: AuthenticationResult = await client.acquireToken(refreshTokenRequest); @@ -397,7 +437,7 @@ describe("RefreshTokenClient unit tests", () => { const expectedRefreshRequest: CommonRefreshTokenRequest = { ...silentFlowRequest, refreshToken: testRefreshTokenEntity.secret, - authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + authenticationScheme: AuthenticationScheme.BEARER, ccsCredential: { credential: testAccount.homeAccountId, type: CcsCredentialType.HOME_ACCOUNT_ID From 13d12a716f34d7ba897ba19f9d53c11577e48d61 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Tue, 5 Oct 2021 13:26:55 -0700 Subject: [PATCH 28/44] Update lib/msal-browser/src/utils/CryptoConstants.ts --- lib/msal-browser/src/utils/CryptoConstants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-browser/src/utils/CryptoConstants.ts b/lib/msal-browser/src/utils/CryptoConstants.ts index 8f5c710406..63d933197e 100644 --- a/lib/msal-browser/src/utils/CryptoConstants.ts +++ b/lib/msal-browser/src/utils/CryptoConstants.ts @@ -12,7 +12,7 @@ export enum Algorithms { S256_HASH_ALG = "SHA-256", } -// Numerical constants relating to biy/bytelength +// Numerical constants relating to bit/byte length export enum Lengths { modulus = 2048 } From a9ee76e68e47452acc640dcb8ebf8cd668cdf57f Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 13:50:23 -0700 Subject: [PATCH 29/44] Fix merge conflicts --- lib/msal-common/src/client/BaseClient.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 463c8350e3..587ce8483a 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -19,7 +19,6 @@ import { ClientAuthError } from "../error/ClientAuthError"; import { KeyManager } from "../crypto/KeyManager"; import { CcsCredential, CcsCredentialType } from "../account/CcsCredential"; import { buildClientInfoFromHomeAccountId } from "../account/ClientInfo"; -import { KeyManager } from "../crypto/KeyManager"; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. From bfb6d4badc1c1c4bc5bb3554eb710a0c49073fb1 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 14:04:28 -0700 Subject: [PATCH 30/44] Add boundRT sample --- lib/msal-common/src/client/BaseClient.ts | 2 +- .../VanillaJSTestApp2.0/app/{rtPop => boundRt}/auth.js | 0 .../app/{rtPop => boundRt}/authConfig.js | 6 ++++-- .../VanillaJSTestApp2.0/app/{rtPop => boundRt}/graph.js | 0 .../VanillaJSTestApp2.0/app/{rtPop => boundRt}/index.html | 0 .../VanillaJSTestApp2.0/app/{rtPop => boundRt}/ui.js | 0 6 files changed, 5 insertions(+), 3 deletions(-) rename samples/msal-browser-samples/VanillaJSTestApp2.0/app/{rtPop => boundRt}/auth.js (100%) rename samples/msal-browser-samples/VanillaJSTestApp2.0/app/{rtPop => boundRt}/authConfig.js (94%) rename samples/msal-browser-samples/VanillaJSTestApp2.0/app/{rtPop => boundRt}/graph.js (100%) rename samples/msal-browser-samples/VanillaJSTestApp2.0/app/{rtPop => boundRt}/index.html (100%) rename samples/msal-browser-samples/VanillaJSTestApp2.0/app/{rtPop => boundRt}/ui.js (100%) diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 587ce8483a..33fa93b03e 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -16,9 +16,9 @@ import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManag import { RequestThumbprint } from "../network/RequestThumbprint"; import { version, name } from "../packageMetadata"; import { ClientAuthError } from "../error/ClientAuthError"; -import { KeyManager } from "../crypto/KeyManager"; import { CcsCredential, CcsCredentialType } from "../account/CcsCredential"; import { buildClientInfoFromHomeAccountId } from "../account/ClientInfo"; +import { KeyManager } from "../crypto/KeyManager"; /** * 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/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/auth.js similarity index 100% rename from samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/auth.js rename to samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/auth.js diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/authConfig.js similarity index 94% rename from samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js rename to samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/authConfig.js index 6f94db751b..f9733f82ea 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/authConfig.js +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/authConfig.js @@ -2,7 +2,8 @@ const msalConfig = { auth: { clientId: "bc77b0a7-16aa-4af4-884b-41b968c9c71a", - authority: "https://login.microsoftonline.com/5d97b14d-c396-4aee-b524-c86d33e9b660" + authority: "https://login.microsoftonline.com/5d97b14d-c396-4aee-b524-c86d33e9b660", + refreshTokenBinding: true }, cache: { cacheLocation: "sessionStorage", // This configures where your cache will be stored @@ -28,7 +29,8 @@ const msalConfig = { console.warn(message); return; } - } + }, + logLevel: msal.LogLevel.Verbose } } }; diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/graph.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/graph.js similarity index 100% rename from samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/graph.js rename to samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/graph.js diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/index.html b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/index.html similarity index 100% rename from samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/index.html rename to samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/index.html diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/ui.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/ui.js similarity index 100% rename from samples/msal-browser-samples/VanillaJSTestApp2.0/app/rtPop/ui.js rename to samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/ui.js From c364a752c938230985a803c38b5c8047848f3807 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 14:12:38 -0700 Subject: [PATCH 31/44] Undo unnecessary method position change --- lib/msal-common/test/client/ClientTestUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/msal-common/test/client/ClientTestUtils.ts b/lib/msal-common/test/client/ClientTestUtils.ts index 0d7f113c68..93a817d331 100644 --- a/lib/msal-common/test/client/ClientTestUtils.ts +++ b/lib/msal-common/test/client/ClientTestUtils.ts @@ -150,12 +150,12 @@ export const mockCrypto = { async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, - async signJwt(): Promise { - return ""; - }, async removeTokenBindingKey(keyId: string): Promise { return Promise.resolve(true); }, + async signJwt(): Promise { + return ""; + }, async clearKeystore(): Promise { return Promise.resolve(true); }, From a4d4c6235f2e3baa5ba73484d7cc2190b31043c9 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 14:46:27 -0700 Subject: [PATCH 32/44] Add initial e2e tests for RT PoP --- .../VanillaJSTestApp2.0/app/boundRt/auth.js | 6 +- .../app/boundRt/index.html | 4 +- .../app/boundRt/test/browser.spec.ts | 73 +++++++++++++++++++ 3 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/auth.js index 3e6b00c4b5..00d1caa114 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/auth.js +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/auth.js @@ -42,12 +42,12 @@ function handleResponse(resp) { } async function signIn(method) { - signInType = isIE ? "redirect" : method; - if (signInType === "popup") { + signInType = isIE ? "loginRedirect" : method; + if (signInType === "loginPopup") { return myMSALObj.loginPopup(loginRequest).then(handleResponse).catch(function (error) { console.log(error); }); - } else if (signInType === "redirect") { + } else if (signInType === "loginRedirect") { return myMSALObj.loginRedirect(loginRequest) } } diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/index.html b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/index.html index cc58ba0e1c..f4c2db3ac1 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/index.html +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/index.html @@ -19,8 +19,8 @@ Sign In
- - + +
diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts new file mode 100644 index 0000000000..43dc3ff059 --- /dev/null +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts @@ -0,0 +1,73 @@ +import "mocha"; +import puppeteer from "puppeteer"; +import { expect } from "chai"; +import { Screenshot, createFolder, setupCredentials, enterCredentials } from "../../../../../e2eTestUtils/TestUtils"; +import { BrowserCacheUtils } from "../../../../../e2eTestUtils/BrowserCacheTestUtils"; +import { LabApiQueryParams } from "../../../../../e2eTestUtils/LabApiQueryParams"; +import { AzureEnvironments, AppTypes } from "../../../../../e2eTestUtils/Constants"; +import { LabClient } from "../../../../../e2eTestUtils/LabClient"; + +const SCREENSHOT_BASE_FOLDER_NAME = `${__dirname}/screenshots`; +const SAMPLE_HOME_URL = "http://localhost:30662/"; +let username = ""; +let accountPwd = ""; + +describe("Browser tests", function () { + this.timeout(0); + this.retries(1); + + let browser: puppeteer.Browser; + before(async () => { + createFolder(SCREENSHOT_BASE_FOLDER_NAME); + const labApiParams: LabApiQueryParams = { + azureEnvironment: AzureEnvironments.PPE, + appType: AppTypes.CLOUD + }; + + const labClient = new LabClient(); + const envResponse = await labClient.getVarsByCloudEnvironment(labApiParams); + [username, accountPwd] = await setupCredentials(envResponse[0], labClient); + + browser = await puppeteer.launch({ + headless: false, + ignoreDefaultArgs: ["--no-sandbox", "–disable-setuid-sandbox"] + }); + }); + + let context: puppeteer.BrowserContext; + let page: puppeteer.Page; + let BrowserCache: BrowserCacheUtils; + beforeEach(async () => { + context = await browser.createIncognitoBrowserContext(); + page = await context.newPage(); + BrowserCache = new BrowserCacheUtils(page, "sessionStorage"); + await page.goto(SAMPLE_HOME_URL); + }); + + afterEach(async () => { + await page.close(); + }); + + after(async () => { + await context.close(); + await browser.close(); + }); + + it("Performs loginRedirect with RT binding", async () => { + const testName = "redirectBaseCase"; + const screenshot = new Screenshot(`${SCREENSHOT_BASE_FOLDER_NAME}/${testName}`); + // Home Page + await page.waitForSelector("#SignIn"); + await screenshot.takeScreenshot(page, "samplePageInit"); + // Click Sign In + await page.click("#SignIn"); + await page.waitForSelector("#loginRedirect"); + await screenshot.takeScreenshot(page, "signInClicked"); + // Click Sign In With Redirect + await page.click("#loginRedirect"); + // Check URL to see if it contains stkJwk + await page.waitForNavigation({ waitUntil: "networkidle0", timeout: 10000}); + await page.waitForSelector("input#i0116.input.text-box"); + expect(page.url()).to.include("stk_jwk"); + }); +}); From ead8a5a2ca0a681c0c3e6814eeea0182216e9a0d Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 15:01:19 -0700 Subject: [PATCH 33/44] Revert to headless false for boundRT e2e test --- .../VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts index 43dc3ff059..35f0fab6b5 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/test/browser.spec.ts @@ -29,7 +29,7 @@ describe("Browser tests", function () { [username, accountPwd] = await setupCredentials(envResponse[0], labClient); browser = await puppeteer.launch({ - headless: false, + headless: true, ignoreDefaultArgs: ["--no-sandbox", "–disable-setuid-sandbox"] }); }); From 60c6f120204bfef2e10f4b1c2367829d6bd770da Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Tue, 5 Oct 2021 15:22:08 -0700 Subject: [PATCH 34/44] Update lib/msal-common/test/client/RefreshTokenClient.spec.ts Co-authored-by: Thomas Norling --- lib/msal-common/test/client/RefreshTokenClient.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-common/test/client/RefreshTokenClient.spec.ts b/lib/msal-common/test/client/RefreshTokenClient.spec.ts index d5f54582ee..05f63844ec 100644 --- a/lib/msal-common/test/client/RefreshTokenClient.spec.ts +++ b/lib/msal-common/test/client/RefreshTokenClient.spec.ts @@ -316,7 +316,7 @@ describe("RefreshTokenClient unit tests", () => { expect(result.includes(`${AADServerParamKeys.X_MS_LIB_CAPABILITY}=${ThrottlingConstants.X_MS_LIB_CAPABILITY_VALUE}`)).toBe(true); }); - it.only("adds req_cnf if authenticationScheme is POP", async () => { + it("adds req_cnf if authenticationScheme is POP", async () => { sinon.stub(RefreshTokenClient.prototype, "executePostToTokenEndpoint").resolves(AUTHENTICATION_RESULT); const createTokenRequestBodySpy = sinon.spy(RefreshTokenClient.prototype, "createTokenRequestBody"); const client = new RefreshTokenClient(config); From 86288dccdc29e0df5def3b82d00c7482033f73bb Mon Sep 17 00:00:00 2001 From: HectorMg Date: Tue, 5 Oct 2021 15:29:16 -0700 Subject: [PATCH 35/44] Fix incorrect typing and checks for private key on getPublicKeyThumbprint --- lib/msal-browser/src/crypto/CryptoOps.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index 98254da27d..63f73923c6 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -99,7 +99,7 @@ export class CryptoOps implements ICrypto { // Generate Keypair const keyPair = await this.browserCrypto.generateKeyPair(keyOptions, CryptoOps.EXTRACTABLE); - if (!keyPair || !keyPair.publicKey) { + if (!keyPair || !keyPair.publicKey || !keyPair.privateKey) { throw BrowserAuthError.createKeyGenerationFailedError(); } @@ -117,19 +117,14 @@ export class CryptoOps implements ICrypto { const publicJwkBuffer: ArrayBuffer = await this.browserCrypto.sha256Digest(publicJwkString); const publicJwkHash: string = this.b64Encode.urlEncodeArr(new Uint8Array(publicJwkBuffer)); - // When asymmetric keypair is generated, there is also a private key - let unextractablePrivateKey; - - if (keyPair.privateKey) { - // Generate Thumbprint for Private Key - const privateKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.privateKey); - // Re-import private key to make it unextractable - unextractablePrivateKey = await this.browserCrypto.importJwk(keyOptions, privateKeyJwk, false, keyOptions.privateKeyUsage); - } + // Generate Thumbprint for Private Key + const privateKeyJwk: JsonWebKey = await this.browserCrypto.exportJwk(keyPair.privateKey); + // Re-import private key to make it unextractable + const unextractablePrivateKey: CryptoKey = await this.browserCrypto.importJwk(keyOptions, privateKeyJwk, false, keyOptions.privateKeyUsage); // Store Keypair data in keystore await this.cache.put(publicJwkHash, { - privateKey: unextractablePrivateKey as CryptoKey, + privateKey: unextractablePrivateKey, publicKey: keyPair.publicKey, requestMethod: request.resourceRequestMethod, requestUri: request.resourceRequestUri From d23e4a372b40021f37d800d9a8eb5adbe6e4e3ad Mon Sep 17 00:00:00 2001 From: HectorMg Date: Wed, 6 Oct 2021 15:06:25 -0700 Subject: [PATCH 36/44] Refactor cryptographic constants to have more consistent casing --- lib/msal-browser/src/crypto/CryptoOps.ts | 10 ++-- .../src/crypto/SignedHttpRequest.ts | 2 +- .../StandardInteractionClient.ts | 10 ++-- lib/msal-browser/src/utils/CryptoConstants.ts | 46 +++++++++++++------ .../test/crypto/CryptoOps.spec.ts | 27 +++++------ .../test/utils/StringConstants.ts | 12 ----- lib/msal-common/src/crypto/KeyManager.ts | 8 ++-- lib/msal-common/src/utils/Constants.ts | 4 +- .../test/crypto/KeyManager.spec.ts | 8 ++-- 9 files changed, 69 insertions(+), 58 deletions(-) diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index 63f73923c6..334bf76cbc 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -11,7 +11,7 @@ import { PkceGenerator } from "./PkceGenerator"; import { BrowserCrypto, CryptoKeyOptions } from "./BrowserCrypto"; import { DatabaseStorage } from "../cache/DatabaseStorage"; import { BrowserStringUtils } from "../utils/BrowserStringUtils"; -import { CryptoKeyFormats, CRYPTO_KEY_CONFIG } from "../utils/CryptoConstants"; +import { CryptoKeyFormats, CryptoKeyConfig } from "../utils/CryptoConstants"; import { BrowserAuthError } from "../error/BrowserAuthError"; export type CachedKeyPair = { @@ -89,11 +89,11 @@ export class CryptoOps implements ICrypto { let keyOptions: CryptoKeyOptions; switch(keyType) { - case CryptoKeyTypes.stk_jwk: - keyOptions = CRYPTO_KEY_CONFIG.RT_BINDING; + case CryptoKeyTypes.StkJwk: + keyOptions = CryptoKeyConfig.RefreshTokenBinding; break; default: - keyOptions = CRYPTO_KEY_CONFIG.AT_BINDING; + keyOptions = CryptoKeyConfig.AccessTokenBinding; } // Generate Keypair @@ -182,7 +182,7 @@ export class CryptoOps implements ICrypto { // Sign token const tokenBuffer = BrowserStringUtils.stringToArrayBuffer(tokenString); - const signatureBuffer = await this.browserCrypto.sign(CRYPTO_KEY_CONFIG.AT_BINDING, cachedKeyPair.privateKey, tokenBuffer); + const signatureBuffer = await this.browserCrypto.sign(CryptoKeyConfig.AccessTokenBinding, cachedKeyPair.privateKey, tokenBuffer); const encodedSignature = this.b64Encode.urlEncodeArr(new Uint8Array(signatureBuffer)); return `${tokenString}.${encodedSignature}`; diff --git a/lib/msal-browser/src/crypto/SignedHttpRequest.ts b/lib/msal-browser/src/crypto/SignedHttpRequest.ts index 2b5bb344f1..3f75b9380d 100644 --- a/lib/msal-browser/src/crypto/SignedHttpRequest.ts +++ b/lib/msal-browser/src/crypto/SignedHttpRequest.ts @@ -24,7 +24,7 @@ export class SignedHttpRequest { * @returns Public key digest, which should be sent to the token issuer. */ async generatePublicKeyThumbprint(): Promise { - const { kid } = await this.keyManager.generateKid(this.shrParameters, CryptoKeyTypes.req_cnf); + const { kid } = await this.keyManager.generateKid(this.shrParameters, CryptoKeyTypes.ReqCnf); return kid; } diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 5d9b9c614f..3adefac765 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -43,13 +43,17 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { if (this.config.auth.refreshTokenBinding) { this.logger.verbose("Refresh token binding enabled, attempting to generate Session Transport Key"); try { - const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.stk_jwk); + const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.StkJwk); request.stkJwk = sessionTransportKeyThumbprint; this.logger.verbose("Successfully generated and stored Session Transport Key"); } catch(error) { if(error instanceof BrowserAuthError) { - this.logger.error(error.message); - this.logger.verbose("Could not generate Session Transport Key, refresh token will be unbound"); + if (error.errorCode === "key_generation_failed") { + this.logger.error("Failed to generate Session Transport Key, refresh token will be unbound."); + } + if (error.errorCode === "database_unavailable") { + this.logger.error("Failed to store Session Transport Key because IndexedDB is unavailable in the current browser environment, refresh token will be unbound."); + } } } } diff --git a/lib/msal-browser/src/utils/CryptoConstants.ts b/lib/msal-browser/src/utils/CryptoConstants.ts index 63d933197e..d5da251369 100644 --- a/lib/msal-browser/src/utils/CryptoConstants.ts +++ b/lib/msal-browser/src/utils/CryptoConstants.ts @@ -3,6 +3,8 @@ * Licensed under the MIT License. */ +import { CryptoKeyOptions } from "../crypto/BrowserCrypto"; + // Cryptographic Algorithms used/supported export enum Algorithms { PKCS1_V15_KEYGEN_ALG = "RSASSA-PKCS1-v1_5", @@ -28,19 +30,35 @@ export enum CryptoKeyFormats { } // Crypto Key Usage sets -export const KEY_USAGES = { - AT_BINDING: { - KEYPAIR: ["sign", "verify"], - PRIVATE_KEY: ["sign"] + +type CryptoKeyUsageSet = { + Keypair: KeyUsage[], + PrivateKey: KeyUsage[] +}; + +interface TokenBindingKeyUsageSets { + AccessTokenBinding: CryptoKeyUsageSet; + RefreshTokenBinding: CryptoKeyUsageSet; +} + +export const CryptoKeyUsageSets: TokenBindingKeyUsageSets = { + AccessTokenBinding: { + Keypair: ["sign", "verify"], + PrivateKey: ["sign"] }, - RT_BINDING: { - KEYPAIR: ["encrypt", "decrypt"], - PRIVATE_KEY: ["decrypt"] + RefreshTokenBinding: { + Keypair: ["encrypt", "decrypt"], + PrivateKey: ["decrypt"] } }; -export const CRYPTO_KEY_CONFIG = { - AT_BINDING: { +interface TokenBindingKeyConfig { + AccessTokenBinding: CryptoKeyOptions; + RefreshTokenBinding: CryptoKeyOptions; +} + +export const CryptoKeyConfig: TokenBindingKeyConfig = { + AccessTokenBinding: { keyGenAlgorithmOptions: { name: Algorithms.PKCS1_V15_KEYGEN_ALG, hash: { @@ -49,10 +67,10 @@ export const CRYPTO_KEY_CONFIG = { modulusLength: Lengths.modulus, publicExponent: PUBLIC_EXPONENT }, - keypairUsages: KEY_USAGES.AT_BINDING.KEYPAIR as KeyUsage[], - privateKeyUsage: KEY_USAGES.AT_BINDING.PRIVATE_KEY as KeyUsage[] + keypairUsages: CryptoKeyUsageSets.AccessTokenBinding.Keypair, + privateKeyUsage: CryptoKeyUsageSets.AccessTokenBinding.PrivateKey }, - RT_BINDING: { + RefreshTokenBinding: { keyGenAlgorithmOptions: { name: Algorithms.RSA_OAEP, hash: { @@ -61,7 +79,7 @@ export const CRYPTO_KEY_CONFIG = { modulusLength: Lengths.modulus, publicExponent: PUBLIC_EXPONENT }, - keypairUsages: KEY_USAGES.RT_BINDING.KEYPAIR as KeyUsage[], - privateKeyUsage: KEY_USAGES.RT_BINDING.PRIVATE_KEY as KeyUsage[] + keypairUsages: CryptoKeyUsageSets.RefreshTokenBinding.Keypair, + privateKeyUsage: CryptoKeyUsageSets.RefreshTokenBinding.PrivateKey } }; diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index 693354cf02..32a6c3199c 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -1,16 +1,17 @@ import { CryptoOps, CachedKeyPair } from "../../src/crypto/CryptoOps"; import { GuidGenerator } from "../../src/crypto/GuidGenerator"; -import { BrowserCrypto } from "../../src/crypto/BrowserCrypto"; -import { TEST_URIS, BROWSER_CRYPTO, KEY_USAGES, TEST_POP_VALUES } from "../utils/StringConstants"; +import { BrowserCrypto, CryptoKeyOptions } from "../../src/crypto/BrowserCrypto"; +import { TEST_URIS, BROWSER_CRYPTO, TEST_POP_VALUES } from "../utils/StringConstants"; import { createHash } from "crypto"; import { AuthenticationScheme, BaseAuthRequest, CryptoKeyTypes, PkceCodes } from "@azure/msal-common"; import { DatabaseStorage } from "../../src/cache/DatabaseStorage"; import { BrowserAuthError } from "../../src"; +import { CryptoKeyUsageSets } from "../../src/utils/CryptoConstants"; const msrCrypto = require("../polyfills/msrcrypto.min"); const PUBLIC_EXPONENT: Uint8Array = new Uint8Array([0x01, 0x00, 0x01]); -const AT_BINDING_KEY_OPTIONS = { +const AccessTokenBindingKeyOptions: CryptoKeyOptions = { keyGenAlgorithmOptions: { name: BROWSER_CRYPTO.PKCS1_V15_KEYGEN_ALG, hash: { @@ -19,11 +20,11 @@ const AT_BINDING_KEY_OPTIONS = { modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, publicExponent: PUBLIC_EXPONENT }, - keypairUsages: KEY_USAGES.AT_BINDING.KEYPAIR as KeyUsage[], - privateKeyUsage: KEY_USAGES.AT_BINDING.PRIVATE_KEY as KeyUsage[] + keypairUsages: CryptoKeyUsageSets.AccessTokenBinding.Keypair, + privateKeyUsage: CryptoKeyUsageSets.AccessTokenBinding.PrivateKey }; -const RT_BINDING_KEY_OPTIONS = { +const RefreshTokenBindingOptions: CryptoKeyOptions = { keyGenAlgorithmOptions: { name: BROWSER_CRYPTO.RSA_OAEP, hash: { @@ -32,8 +33,8 @@ const RT_BINDING_KEY_OPTIONS = { modulusLength: BROWSER_CRYPTO.MODULUS_LENGTH, publicExponent: PUBLIC_EXPONENT }, - keypairUsages: KEY_USAGES.RT_BINDING.KEYPAIR as KeyUsage[], - privateKeyUsage: KEY_USAGES.RT_BINDING.PRIVATE_KEY as KeyUsage[] + keypairUsages: CryptoKeyUsageSets.RefreshTokenBinding.Keypair, + privateKeyUsage: CryptoKeyUsageSets.RefreshTokenBinding.PrivateKey }; describe("CryptoOps.ts Unit Tests", () => { @@ -162,7 +163,7 @@ describe("CryptoOps.ts Unit Tests", () => { const generateKeyPairSpy = jest.spyOn(BrowserCrypto.prototype, "generateKeyPair"); const exportJwkSpy = jest.spyOn(BrowserCrypto.prototype, "exportJwk"); - const keyType = CryptoKeyTypes.req_cnf; + const keyType = CryptoKeyTypes.ReqCnf; const testRequest = { authenticationScheme: AuthenticationScheme.POP, @@ -177,7 +178,7 @@ describe("CryptoOps.ts Unit Tests", () => { */ const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); const result = await generateKeyPairSpy.mock.results[0].value; - expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(AT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); + expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(AccessTokenBindingKeyOptions.keyGenAlgorithmOptions.name.toLowerCase()); expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); expect(regExp.test(pkThumbprint)).toBe(true); expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); @@ -192,7 +193,7 @@ describe("CryptoOps.ts Unit Tests", () => { const generateKeyPairSpy = jest.spyOn(BrowserCrypto.prototype, "generateKeyPair"); const exportJwkSpy = jest.spyOn(BrowserCrypto.prototype, "exportJwk"); - const keyType = CryptoKeyTypes.stk_jwk; + const keyType = CryptoKeyTypes.StkJwk; const pkThumbprint = await cryptoObj.getPublicKeyThumbprint({}, keyType); /** @@ -201,8 +202,8 @@ describe("CryptoOps.ts Unit Tests", () => { const regExp = new RegExp("[A-Za-z0-9-_+/]{43}"); const result = await generateKeyPairSpy.mock.results[0].value; - expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(RT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); - expect(result.privateKey.algorithm.name.toLowerCase()).toEqual(RT_BINDING_KEY_OPTIONS.keyGenAlgorithmOptions.name.toLowerCase()); + expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(RefreshTokenBindingOptions.keyGenAlgorithmOptions.name.toLowerCase()); + expect(result.privateKey.algorithm.name.toLowerCase()).toEqual(RefreshTokenBindingOptions.keyGenAlgorithmOptions.name.toLowerCase()); expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); expect(regExp.test(pkThumbprint)).toBe(true); expect(Object.keys(dbStorage[pkThumbprint])).not.toHaveLength(0); diff --git a/lib/msal-browser/test/utils/StringConstants.ts b/lib/msal-browser/test/utils/StringConstants.ts index 1de441165c..94ebd4b1e5 100644 --- a/lib/msal-browser/test/utils/StringConstants.ts +++ b/lib/msal-browser/test/utils/StringConstants.ts @@ -235,18 +235,6 @@ export const testNavUrlNoRequest = `https://login.microsoftonline.com/common/oau export const testLogoutUrl = `https://login.microsoftonline.com/common/oauth2/v2.0/logout?post_logout_redirect_uri=${encodeURIComponent(`${TEST_URIS.TEST_REDIR_URI}`)}`; -// Crypto Key Usage sets -export const KEY_USAGES = { - AT_BINDING: { - KEYPAIR: ["sign", "verify"], - PRIVATE_KEY: ["sign"] - }, - RT_BINDING: { - KEYPAIR: ["encrypt", "decrypt"], - PRIVATE_KEY: ["decrypt"] - } -}; - // Cryptographic Constants export const BROWSER_CRYPTO = { PKCS1_V15_KEYGEN_ALG: "RSASSA-PKCS1-v1_5", diff --git a/lib/msal-common/src/crypto/KeyManager.ts b/lib/msal-common/src/crypto/KeyManager.ts index 8dfe825a1c..7bfaea803a 100644 --- a/lib/msal-common/src/crypto/KeyManager.ts +++ b/lib/msal-common/src/crypto/KeyManager.ts @@ -19,8 +19,8 @@ type ReqCnf = { }; enum KeyLocation { - SW = "sw", - UHW = "uhw" + SoftwareStorage = "sw", + HardwareStorage = "uhw" } export class KeyManager { @@ -31,7 +31,7 @@ export class KeyManager { } async generateCnf(request: SignedHttpRequestParameters): Promise { - const reqCnf = await this.generateKid(request, CryptoKeyTypes.req_cnf); + const reqCnf = await this.generateKid(request, CryptoKeyTypes.ReqCnf); return this.cryptoUtils.base64Encode(JSON.stringify(reqCnf)); } @@ -40,7 +40,7 @@ export class KeyManager { return { kid: kidThumbprint, - xms_ksl: KeyLocation.SW + xms_ksl: KeyLocation.SoftwareStorage }; } diff --git a/lib/msal-common/src/utils/Constants.ts b/lib/msal-common/src/utils/Constants.ts index c41716f5d9..71db75d5da 100644 --- a/lib/msal-common/src/utils/Constants.ts +++ b/lib/msal-common/src/utils/Constants.ts @@ -350,8 +350,8 @@ export enum PasswordGrantConstants { * Supported Cryptographic Key Types */ export enum CryptoKeyTypes { - req_cnf = "req_cnf", - stk_jwk = "stk_jwk" + ReqCnf = "req_cnf", + StkJwk = "stk_jwk" } /** diff --git a/lib/msal-common/test/crypto/KeyManager.spec.ts b/lib/msal-common/test/crypto/KeyManager.spec.ts index 043ca8baee..48f6d13f52 100644 --- a/lib/msal-common/test/crypto/KeyManager.spec.ts +++ b/lib/msal-common/test/crypto/KeyManager.spec.ts @@ -79,15 +79,15 @@ describe("KeyManager Unit Tests", () => { describe("generateCnf", () => { it("generates the req_cnf correctly", async () => { - const req_cnf = await keyManager.generateCnf(testPopRequest); - expect(req_cnf).toBe(TEST_POP_VALUES.ENCODED_REQ_CNF); + const reqCnf = await keyManager.generateCnf(testPopRequest); + expect(reqCnf).toBe(TEST_POP_VALUES.ENCODED_REQ_CNF); }); }); describe("generateKid", () => { it("returns the correct kid and key storage location", async () => { - const req_cnf = await keyManager.generateKid(testPopRequest, CryptoKeyTypes.req_cnf); - expect(req_cnf).toStrictEqual(JSON.parse(TEST_POP_VALUES.DECODED_REQ_CNF)); + const reqCnf = await keyManager.generateKid(testPopRequest, CryptoKeyTypes.ReqCnf); + expect(reqCnf).toStrictEqual(JSON.parse(TEST_POP_VALUES.DECODED_REQ_CNF)); }); }); }); \ No newline at end of file From ec5c2f6d100c9a48d04e8de0d30f58da79a84473 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Thu, 21 Oct 2021 13:35:29 -0700 Subject: [PATCH 37/44] Fix CryptoOps tests around getPublicKeyThumbprint --- lib/msal-browser/test/crypto/CryptoOps.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index 37d867636f..4c8eb9a8bf 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -200,7 +200,7 @@ describe("CryptoOps.ts Unit Tests", () => { expect(result.publicKey.algorithm.name.toLowerCase()).toEqual(AccessTokenBindingKeyOptions.keyGenAlgorithmOptions.name.toLowerCase()); expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); expect(regExp.test(pkThumbprint)).toBe(true); - expect(mockDatabase["TestDB.keys"][pkThumbprint]).not.toHaveLength(0); + expect(mockDatabase["TestDB.keys"][pkThumbprint]).not.toBe(undefined); }, 30000); it("generates a valid stk_jwk thumbprint", async () => { @@ -225,7 +225,7 @@ describe("CryptoOps.ts Unit Tests", () => { expect(result.privateKey.algorithm.name.toLowerCase()).toEqual(RefreshTokenBindingOptions.keyGenAlgorithmOptions.name.toLowerCase()); expect(exportJwkSpy).toHaveBeenCalledWith(result.publicKey); expect(regExp.test(pkThumbprint)).toBe(true); - expect(Object.keys(mockDatabase["TestDB.keys"][pkThumbprint])).not.toHaveLength(0); + expect(Object.keys(mockDatabase["TestDB.keys"][pkThumbprint])).not.toBe(undefined); }, 30000); it("throws error if key generation fails", async () => { From 138acc49a84ba1da91f247025ec8041b701bfb15 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Thu, 21 Oct 2021 14:03:32 -0700 Subject: [PATCH 38/44] Move refreshTokenBinding feature flag to system config --- .../src/interaction_client/StandardInteractionClient.ts | 2 +- lib/msal-browser/test/app/PublicClientApplication.spec.ts | 2 ++ .../test/interaction_client/PopupClient.spec.ts | 2 ++ .../test/interaction_client/RedirectClient.spec.ts | 2 ++ lib/msal-common/src/config/ClientConfiguration.ts | 8 ++++---- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 3adefac765..66b887bcef 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -40,7 +40,7 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { const generatedPkceParams = await this.browserCrypto.generatePkceCodes(); // Generate Session Transport Key if Refresh Token Binding is enabled - if (this.config.auth.refreshTokenBinding) { + if (this.config.system.refreshTokenBinding) { this.logger.verbose("Refresh token binding enabled, attempting to generate Session Transport Key"); try { const sessionTransportKeyThumbprint = await this.browserCrypto.getPublicKeyThumbprint(request, CryptoKeyTypes.StkJwk); diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index 490daa077d..82483d7fff 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -34,6 +34,8 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { pca = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + system: { refreshTokenBinding: true } }); diff --git a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index 668956c45c..c9a74ffdf0 100644 --- a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts @@ -23,6 +23,8 @@ describe("PopupClient", () => { const pca = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + system: { refreshTokenBinding: true } }); diff --git a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts index ce2ad3e6da..10411b0c34 100644 --- a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts @@ -45,6 +45,8 @@ describe("RedirectClient", () => { const pca = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + system: { refreshTokenBinding: true } }); diff --git a/lib/msal-common/src/config/ClientConfiguration.ts b/lib/msal-common/src/config/ClientConfiguration.ts index f7624695ac..51d9dd968d 100644 --- a/lib/msal-common/src/config/ClientConfiguration.ts +++ b/lib/msal-common/src/config/ClientConfiguration.ts @@ -68,23 +68,23 @@ export type CommonClientConfiguration = { * - cloudDiscoveryMetadata - A string containing the cloud discovery response. Used in AAD scenarios. * - clientCapabilities - Array of capabilities which will be added to the claims.access_token.xms_cc request property on every network request. * - protocolMode - Enum that represents the protocol that msal follows. Used for configuring proper endpoints. - * - refreshTokenBinding - Boolean that enables refresh token binding (a.k.a refresh token proof-of-possession) for authorization requests */ export type AuthOptions = { clientId: string; authority: Authority; clientCapabilities?: Array; - refreshTokenBinding?: boolean; }; /** * Use this to configure token renewal info in the Configuration object * * - tokenRenewalOffsetSeconds - Sets the window of offset needed to renew the token before expiry + * - refreshTokenBinding - Boolean that enables refresh token binding (a.k.a refresh token proof-of-possession) for authorization requests */ export type SystemOptions = { tokenRenewalOffsetSeconds?: number; preventCorsPreflight?: boolean; + refreshTokenBinding?: boolean; }; /** @@ -125,7 +125,8 @@ export type ClientCredentials = { export const DEFAULT_SYSTEM_OPTIONS: Required = { tokenRenewalOffsetSeconds: DEFAULT_TOKEN_RENEWAL_OFFSET_SEC, - preventCorsPreflight: false + preventCorsPreflight: false, + refreshTokenBinding: false }; const DEFAULT_LOGGER_IMPLEMENTATION: Required = { @@ -206,7 +207,6 @@ export function buildClientConfiguration( function buildAuthOptions(authOptions: AuthOptions): Required { return { clientCapabilities: [], - refreshTokenBinding: false, ...authOptions }; } From d8201570d10cd5d0abced7dcebdbd755368f75dc Mon Sep 17 00:00:00 2001 From: HectorMg Date: Thu, 21 Oct 2021 14:12:16 -0700 Subject: [PATCH 39/44] Update browser client config to move refreshTokenBinding flag to system config --- lib/msal-browser/src/config/Configuration.ts | 8 ++++---- lib/msal-browser/test/config/Configuration.spec.ts | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts index ea45a8daa8..b97d3f9cf6 100644 --- a/lib/msal-browser/src/config/Configuration.ts +++ b/lib/msal-browser/src/config/Configuration.ts @@ -26,7 +26,6 @@ export const DEFAULT_REDIRECT_TIMEOUT_MS = 30000; * - navigateToLoginRequestUrl - Boolean indicating whether to navigate to the original request URL after the auth server navigates to the redirect URL. * - clientCapabilities - Array of capabilities which will be added to the claims.access_token.xms_cc request property on every network request. * - protocolMode - Enum that represents the protocol that msal follows. Used for configuring proper endpoints. - * - refreshTokenBinding - Boolean that enables refresh token binding (a.k.a refresh token proof-of-possession) for authorization requests */ export type BrowserAuthOptions = { clientId: string; @@ -39,7 +38,6 @@ export type BrowserAuthOptions = { navigateToLoginRequestUrl?: boolean; clientCapabilities?: Array; protocolMode?: ProtocolMode; - refreshTokenBinding?: boolean; }; /** @@ -68,6 +66,7 @@ export type CacheOptions = { * - redirectNavigationTimeout - Time to wait for redirection to occur before resolving promise * - asyncPopups - Sets whether popups are opened asynchronously. By default, this flag is set to false. When set to false, blank popups are opened before anything else happens. When set to true, popups are opened when making the network request. * - allowRedirectInIframe - Flag to enable redirect opertaions when the app is rendered in an iframe (to support scenarios such as embedded B2C login). + * - refreshTokenBinding - Boolean that enables refresh token binding (a.k.a refresh token proof-of-possession) for authorization requests */ export type BrowserSystemOptions = SystemOptions & { loggerOptions?: LoggerOptions; @@ -80,6 +79,7 @@ export type BrowserSystemOptions = SystemOptions & { redirectNavigationTimeout?: number; asyncPopups?: boolean; allowRedirectInIframe?: boolean; + refreshTokenBinding?: boolean; }; /** @@ -125,7 +125,6 @@ export function buildConfiguration({ auth: userInputAuth, cache: userInputCache, navigateToLoginRequestUrl: true, clientCapabilities: [], protocolMode: ProtocolMode.AAD, - refreshTokenBinding: false }; // Default cache options for browser @@ -156,7 +155,8 @@ export function buildConfiguration({ auth: userInputAuth, cache: userInputCache, navigateFrameWait: isBrowserEnvironment && BrowserUtils.detectIEOrEdge() ? 500 : 0, redirectNavigationTimeout: DEFAULT_REDIRECT_TIMEOUT_MS, asyncPopups: false, - allowRedirectInIframe: false + allowRedirectInIframe: false, + refreshTokenBinding: false }; const overlayedConfig: BrowserConfiguration = { diff --git a/lib/msal-browser/test/config/Configuration.spec.ts b/lib/msal-browser/test/config/Configuration.spec.ts index 5a4f286e16..fb21fb32e8 100644 --- a/lib/msal-browser/test/config/Configuration.spec.ts +++ b/lib/msal-browser/test/config/Configuration.spec.ts @@ -44,6 +44,7 @@ describe("Configuration.ts Class Unit Tests", () => { expect(emptyConfig.system?.navigateFrameWait).toBe(0); expect(emptyConfig.system?.tokenRenewalOffsetSeconds).toBe(300); expect(emptyConfig.system?.asyncPopups).toBe(false); + expect(emptyConfig.system?.refreshTokenBinding).toBe(false); }); it("sets timeouts with loadFrameTimeout", () => { @@ -168,7 +169,8 @@ describe("Configuration.ts Class Unit Tests", () => { loggerCallback: testLoggerCallback, piiLoggingEnabled: true }, - asyncPopups: true + asyncPopups: true, + refreshTokenBinding: true } }, true); // Auth config checks @@ -196,5 +198,6 @@ describe("Configuration.ts Class Unit Tests", () => { expect(newConfig.system?.loggerOptions?.loggerCallback).not.toBeNull(); expect(newConfig.system?.loggerOptions?.piiLoggingEnabled).toBe(true); expect(newConfig.system?.asyncPopups).toBe(true); + expect(newConfig.system?.refreshTokenBinding).toBe(true); }); }); From f6f428e41803b69c207285d2870e9aae29009729 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Thu, 21 Oct 2021 14:37:27 -0700 Subject: [PATCH 40/44] Rename KeyManager to CryptoKeyManager for more specificity --- lib/msal-browser/src/crypto/SignedHttpRequest.ts | 8 ++++---- .../src/client/AuthorizationCodeClient.ts | 2 +- lib/msal-common/src/client/BaseClient.ts | 6 +++--- lib/msal-common/src/client/RefreshTokenClient.ts | 2 +- .../crypto/{KeyManager.ts => CryptoKeyManager.ts} | 2 +- lib/msal-common/src/index.ts | 2 +- .../{KeyManager.spec.ts => CryptoKeyManager.spec.ts} | 12 ++++++------ 7 files changed, 17 insertions(+), 17 deletions(-) rename lib/msal-common/src/crypto/{KeyManager.ts => CryptoKeyManager.ts} (98%) rename lib/msal-common/test/crypto/{KeyManager.spec.ts => CryptoKeyManager.spec.ts} (88%) diff --git a/lib/msal-browser/src/crypto/SignedHttpRequest.ts b/lib/msal-browser/src/crypto/SignedHttpRequest.ts index 3f75b9380d..677ae9b1a8 100644 --- a/lib/msal-browser/src/crypto/SignedHttpRequest.ts +++ b/lib/msal-browser/src/crypto/SignedHttpRequest.ts @@ -4,18 +4,18 @@ */ import { CryptoOps } from "./CryptoOps"; -import { PopTokenGenerator, SignedHttpRequestParameters, KeyManager, CryptoKeyTypes } from "@azure/msal-common"; +import { PopTokenGenerator, SignedHttpRequestParameters, CryptoKeyManager, CryptoKeyTypes } from "@azure/msal-common"; export class SignedHttpRequest { private popTokenGenerator: PopTokenGenerator; private cryptoOps: CryptoOps; private shrParameters: SignedHttpRequestParameters; - private keyManager: KeyManager; + private cryptoKeyManager: CryptoKeyManager; constructor(shrParameters: SignedHttpRequestParameters) { this.cryptoOps = new CryptoOps(); this.popTokenGenerator = new PopTokenGenerator(this.cryptoOps); - this.keyManager = new KeyManager(this.cryptoOps); + this.cryptoKeyManager = new CryptoKeyManager(this.cryptoOps); this.shrParameters = shrParameters; } @@ -24,7 +24,7 @@ export class SignedHttpRequest { * @returns Public key digest, which should be sent to the token issuer. */ async generatePublicKeyThumbprint(): Promise { - const { kid } = await this.keyManager.generateKid(this.shrParameters, CryptoKeyTypes.ReqCnf); + const { kid } = await this.cryptoKeyManager.generateKid(this.shrParameters, CryptoKeyTypes.ReqCnf); return kid; } diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index a217a71063..d4768bf061 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -221,7 +221,7 @@ export class AuthorizationCodeClient extends BaseClient { parameterBuilder.addClientInfo(); if (request.authenticationScheme === AuthenticationScheme.POP) { - const cnfString = await this.keyManager.generateCnf(request); + const cnfString = await this.cryptoKeyManager.generateCnf(request); parameterBuilder.addPopToken(cnfString); } diff --git a/lib/msal-common/src/client/BaseClient.ts b/lib/msal-common/src/client/BaseClient.ts index 33fa93b03e..ec2400bd27 100644 --- a/lib/msal-common/src/client/BaseClient.ts +++ b/lib/msal-common/src/client/BaseClient.ts @@ -18,7 +18,7 @@ import { version, name } from "../packageMetadata"; import { ClientAuthError } from "../error/ClientAuthError"; import { CcsCredential, CcsCredentialType } from "../account/CcsCredential"; import { buildClientInfoFromHomeAccountId } from "../account/ClientInfo"; -import { KeyManager } from "../crypto/KeyManager"; +import { CryptoKeyManager } from "../crypto/CryptoKeyManager"; /** * Base application class which will construct requests to send to and handle responses from the Microsoft STS using the authorization code flow. @@ -49,7 +49,7 @@ export abstract class BaseClient { public authority: Authority; // Define Key Manager object - protected keyManager: KeyManager; + protected cryptoKeyManager: CryptoKeyManager; protected constructor(configuration: ClientConfiguration) { // Set the configuration @@ -77,7 +77,7 @@ export abstract class BaseClient { this.authority = this.config.authOptions.authority; // set KeyManager - this.keyManager = new KeyManager(this.cryptoUtils); + this.cryptoKeyManager = new CryptoKeyManager(this.cryptoUtils); } /** diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 4c136a43f5..1b8b40a131 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -203,7 +203,7 @@ export class RefreshTokenClient extends BaseClient { } if (request.authenticationScheme === AuthenticationScheme.POP) { - const cnfString = await this.keyManager.generateCnf(request); + const cnfString = await this.cryptoKeyManager.generateCnf(request); parameterBuilder.addPopToken(cnfString); } diff --git a/lib/msal-common/src/crypto/KeyManager.ts b/lib/msal-common/src/crypto/CryptoKeyManager.ts similarity index 98% rename from lib/msal-common/src/crypto/KeyManager.ts rename to lib/msal-common/src/crypto/CryptoKeyManager.ts index 7bfaea803a..4c8ae53c78 100644 --- a/lib/msal-common/src/crypto/KeyManager.ts +++ b/lib/msal-common/src/crypto/CryptoKeyManager.ts @@ -23,7 +23,7 @@ enum KeyLocation { HardwareStorage = "uhw" } -export class KeyManager { +export class CryptoKeyManager { private cryptoUtils: ICrypto; constructor(cryptoUtils: ICrypto) { diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index 7f9ee96e19..8ebd4c5d9a 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -55,7 +55,7 @@ export { UrlString } from "./url/UrlString"; // Crypto Interface export { ICrypto, PkceCodes, DEFAULT_CRYPTO_IMPLEMENTATION, SignedHttpRequestParameters } from "./crypto/ICrypto"; export { SignedHttpRequest } from "./crypto/SignedHttpRequest"; -export { KeyManager } from "./crypto/KeyManager"; +export { CryptoKeyManager } from "./crypto/CryptoKeyManager"; // Request and Response export { BaseAuthRequest } from "./request/BaseAuthRequest"; export { CommonAuthorizationUrlRequest } from "./request/CommonAuthorizationUrlRequest"; diff --git a/lib/msal-common/test/crypto/KeyManager.spec.ts b/lib/msal-common/test/crypto/CryptoKeyManager.spec.ts similarity index 88% rename from lib/msal-common/test/crypto/KeyManager.spec.ts rename to lib/msal-common/test/crypto/CryptoKeyManager.spec.ts index 48f6d13f52..a58f0bc353 100644 --- a/lib/msal-common/test/crypto/KeyManager.spec.ts +++ b/lib/msal-common/test/crypto/CryptoKeyManager.spec.ts @@ -1,9 +1,9 @@ import sinon from "sinon"; import { ICrypto, PkceCodes, AuthenticationScheme, CryptoKeyTypes } from "../../src"; import { RANDOM_TEST_GUID, TEST_POP_VALUES, TEST_DATA_CLIENT_INFO, TEST_CONFIG, TEST_URIS } from "../test_kit/StringConstants"; -import { KeyManager } from "../../src/crypto/KeyManager"; +import { CryptoKeyManager } from "../../src/crypto/CryptoKeyManager"; -describe("KeyManager Unit Tests", () => { +describe("CryptoKeyManager Unit Tests", () => { afterEach(() => { sinon.restore(); @@ -62,7 +62,7 @@ describe("KeyManager Unit Tests", () => { } }; - let keyManager: KeyManager; + let cryptoKeyManager: CryptoKeyManager; const testPopRequest = { authority: TEST_CONFIG.validAuthority, @@ -74,19 +74,19 @@ describe("KeyManager Unit Tests", () => { }; beforeEach(() => { - keyManager = new KeyManager(cryptoInterface); + cryptoKeyManager = new CryptoKeyManager(cryptoInterface); }); describe("generateCnf", () => { it("generates the req_cnf correctly", async () => { - const reqCnf = await keyManager.generateCnf(testPopRequest); + const reqCnf = await cryptoKeyManager.generateCnf(testPopRequest); expect(reqCnf).toBe(TEST_POP_VALUES.ENCODED_REQ_CNF); }); }); describe("generateKid", () => { it("returns the correct kid and key storage location", async () => { - const reqCnf = await keyManager.generateKid(testPopRequest, CryptoKeyTypes.ReqCnf); + const reqCnf = await cryptoKeyManager.generateKid(testPopRequest, CryptoKeyTypes.ReqCnf); expect(reqCnf).toStrictEqual(JSON.parse(TEST_POP_VALUES.DECODED_REQ_CNF)); }); }); From a4e2ae55e7bdf5fca53ba0ba29bb5913dc2946cf Mon Sep 17 00:00:00 2001 From: HectorMg Date: Mon, 1 Nov 2021 13:12:40 -0700 Subject: [PATCH 41/44] Update BrowserAuthError to remove keyId from error message and avoid Pii --- lib/msal-browser/src/error/BrowserAuthError.ts | 4 ++-- lib/msal-browser/test/crypto/CryptoOps.spec.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/msal-browser/src/error/BrowserAuthError.ts b/lib/msal-browser/src/error/BrowserAuthError.ts index 9c8a7ab955..6b9381a234 100644 --- a/lib/msal-browser/src/error/BrowserAuthError.ts +++ b/lib/msal-browser/src/error/BrowserAuthError.ts @@ -435,8 +435,8 @@ export class BrowserAuthError extends AuthError { /** * Create an error thrown when the queried cryptographic key is not found in IndexedDB */ - static createSigningKeyNotFoundInStorageError(keyId: string): BrowserAuthError { - return new BrowserAuthError(BrowserAuthErrorMessage.signingKeyNotFoundInStorage.code, `${BrowserAuthErrorMessage.signingKeyNotFoundInStorage.desc} | No match found for KeyId: ${keyId}`); + static createSigningKeyNotFoundInStorageError(): BrowserAuthError { + return new BrowserAuthError(BrowserAuthErrorMessage.signingKeyNotFoundInStorage.code, `${BrowserAuthErrorMessage.signingKeyNotFoundInStorage.desc}`); } /** diff --git a/lib/msal-browser/test/crypto/CryptoOps.spec.ts b/lib/msal-browser/test/crypto/CryptoOps.spec.ts index a0e69df76b..7c8d28d1a1 100644 --- a/lib/msal-browser/test/crypto/CryptoOps.spec.ts +++ b/lib/msal-browser/test/crypto/CryptoOps.spec.ts @@ -165,7 +165,7 @@ describe("CryptoOps.ts Unit Tests", () => { }, 30000); it("signJwt() throws signingKeyNotFoundInStorage error if signing keypair is not found in storage", async () => { - expect(cryptoObj.signJwt({}, "testString")).rejects.toThrow(BrowserAuthError.createSigningKeyNotFoundInStorageError("testString")); + expect(cryptoObj.signJwt({}, "testString")).rejects.toThrow(BrowserAuthError.createSigningKeyNotFoundInStorageError()); }, 30000); describe("getPublicKeyThumbprint", () => { From 306277412bf14c30582078f08935ce6579c1f77c Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 1 Nov 2021 13:18:12 -0700 Subject: [PATCH 42/44] Update lib/msal-browser/src/config/Configuration.ts --- lib/msal-browser/src/config/Configuration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts index b97d3f9cf6..b81311c1a5 100644 --- a/lib/msal-browser/src/config/Configuration.ts +++ b/lib/msal-browser/src/config/Configuration.ts @@ -124,7 +124,7 @@ export function buildConfiguration({ auth: userInputAuth, cache: userInputCache, postLogoutRedirectUri: "", navigateToLoginRequestUrl: true, clientCapabilities: [], - protocolMode: ProtocolMode.AAD, + protocolMode: ProtocolMode.AAD }; // Default cache options for browser From 89d13f4b1fa4dd4d862d49d2f6563ecbcfa18ee4 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Mon, 1 Nov 2021 13:38:45 -0700 Subject: [PATCH 43/44] Update sample and fix merge bugs --- lib/msal-common/src/client/AuthorizationCodeClient.ts | 2 +- lib/msal-common/src/request/RequestParameterBuilder.ts | 2 +- .../VanillaJSTestApp2.0/app/boundRt/authConfig.js | 4 ++-- .../VanillaJSTestApp2.0/app/boundRt/ui.js | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 591b13cc39..52bd44c24c 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -234,7 +234,7 @@ export class AuthorizationCodeClient extends BaseClient { } if (request.stkJwk) { - const stkJwk = await this.keyManager.retrieveAsymmetricPublicKey(request.stkJwk); + const stkJwk = await this.cryptoKeyManager.retrieveAsymmetricPublicKey(request.stkJwk); parameterBuilder.addStkJwk(stkJwk); } diff --git a/lib/msal-common/src/request/RequestParameterBuilder.ts b/lib/msal-common/src/request/RequestParameterBuilder.ts index 75ebaf5e90..db83cadeea 100644 --- a/lib/msal-common/src/request/RequestParameterBuilder.ts +++ b/lib/msal-common/src/request/RequestParameterBuilder.ts @@ -12,7 +12,7 @@ import { LibraryInfo } from "../config/ClientConfiguration"; import { StringUtils } from "../utils/StringUtils"; import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManager"; import { ClientInfo } from "../account/ClientInfo"; -import { StkJwkThumbprint } from "../crypto/KeyManager"; +import { StkJwkThumbprint } from "../crypto/CryptoKeyManager"; export class RequestParameterBuilder { diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/authConfig.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/authConfig.js index f9733f82ea..04d19d9129 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/authConfig.js +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/authConfig.js @@ -3,7 +3,6 @@ const msalConfig = { auth: { clientId: "bc77b0a7-16aa-4af4-884b-41b968c9c71a", authority: "https://login.microsoftonline.com/5d97b14d-c396-4aee-b524-c86d33e9b660", - refreshTokenBinding: true }, cache: { cacheLocation: "sessionStorage", // This configures where your cache will be stored @@ -31,7 +30,8 @@ const msalConfig = { } }, logLevel: msal.LogLevel.Verbose - } + }, + refreshTokenBinding: true } }; diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/ui.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/ui.js index e248605008..a9a6b084bc 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/ui.js +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/boundRt/ui.js @@ -1,8 +1,8 @@ // Select DOM elements to work with const welcomeDiv = document.getElementById("WelcomeMessage"); const signInButton = document.getElementById("SignIn"); -const popupButton = document.getElementById("popup"); -const redirectButton = document.getElementById("redirect"); +const popupButton = document.getElementById("loginPopup"); +const redirectButton = document.getElementById("loginRedirect"); const cardDiv = document.getElementById("card-div"); const mailButton = document.getElementById("readMail"); const profileButton = document.getElementById("seeProfile"); From 589d04a78b8cc2ea01e4b88d5f6c176eeeeece92 Mon Sep 17 00:00:00 2001 From: HectorMg Date: Fri, 28 Jan 2022 15:58:36 -0800 Subject: [PATCH 44/44] Remove CryptoKeyManager class --- .../src/crypto/CryptoKeyManager.ts | 60 ------------ .../src/crypto/PopTokenGenerator.ts | 6 +- lib/msal-common/src/index.ts | 1 - .../src/request/RequestParameterBuilder.ts | 2 +- .../test/crypto/CryptoKeyManager.spec.ts | 93 ------------------- 5 files changed, 6 insertions(+), 156 deletions(-) delete mode 100644 lib/msal-common/src/crypto/CryptoKeyManager.ts delete mode 100644 lib/msal-common/test/crypto/CryptoKeyManager.spec.ts diff --git a/lib/msal-common/src/crypto/CryptoKeyManager.ts b/lib/msal-common/src/crypto/CryptoKeyManager.ts deleted file mode 100644 index f460be65c5..0000000000 --- a/lib/msal-common/src/crypto/CryptoKeyManager.ts +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -import { CryptoKeyTypes } from "../utils/Constants"; -import { ICrypto, SignedHttpRequestParameters } from "./ICrypto"; - -/** - * See eSTS docs for more info. - * - A kid element, with the value containing an RFC 7638-compliant JWK thumbprint that is base64 encoded. - * - xms_ksl element, representing the storage location of the key's secret component on the client device. One of two values: - * - sw: software storage - * - uhw: hardware storage - */ -export type ReqCnf = { - kid: string; - xms_ksl: KeyLocation; -}; - -export type StkJwkThumbprint = { - kid: string; -}; - -enum KeyLocation { - SoftwareStorage = "sw", - HardwareStorage = "uhw" -} - -export class CryptoKeyManager { - private cryptoUtils: ICrypto; - - constructor(cryptoUtils: ICrypto) { - this.cryptoUtils = cryptoUtils; - } - - async generateCnf(request: SignedHttpRequestParameters): Promise { - const reqCnf = await this.generateKid(request, CryptoKeyTypes.ReqCnf); - return this.cryptoUtils.base64Encode(JSON.stringify(reqCnf)); - } - - async generateKid(request: SignedHttpRequestParameters, keyType: CryptoKeyTypes): Promise { - const kidThumbprint = await this.cryptoUtils.getPublicKeyThumbprint(request, keyType); - - return { - kid: kidThumbprint, - xms_ksl: KeyLocation.SoftwareStorage - }; - } - - /** - * Returns the public key from an asymmetric key pair stored in IndexedDB based on the - * public key thumbprint parameter - * @param keyThumbprint - * @returns Public Key JWK string - */ - async retrieveAsymmetricPublicKey(keyThumbprint: string): Promise { - return await this.cryptoUtils.getAsymmetricPublicKey(keyThumbprint); - } -} diff --git a/lib/msal-common/src/crypto/PopTokenGenerator.ts b/lib/msal-common/src/crypto/PopTokenGenerator.ts index 24bb112bd7..7087902a0b 100644 --- a/lib/msal-common/src/crypto/PopTokenGenerator.ts +++ b/lib/msal-common/src/crypto/PopTokenGenerator.ts @@ -18,11 +18,15 @@ import { ClientAuthError } from "../error/ClientAuthError"; * - sw: software storage * - uhw: hardware storage */ -type ReqCnf = { +export type ReqCnf = { kid: string; xms_ksl: KeyLocation; }; +export type StkJwkThumbprint = { + kid: string; +}; + enum KeyLocation { SoftwareStorage = "sw", HardwareStorage = "uhw" diff --git a/lib/msal-common/src/index.ts b/lib/msal-common/src/index.ts index 2df62a645a..94d9ecb411 100644 --- a/lib/msal-common/src/index.ts +++ b/lib/msal-common/src/index.ts @@ -55,7 +55,6 @@ export { UrlString } from "./url/UrlString"; // Crypto Interface export { ICrypto, PkceCodes, DEFAULT_CRYPTO_IMPLEMENTATION, SignedHttpRequestParameters } from "./crypto/ICrypto"; export { SignedHttpRequest } from "./crypto/SignedHttpRequest"; -export { CryptoKeyManager } from "./crypto/CryptoKeyManager"; // Request and Response export { BaseAuthRequest } from "./request/BaseAuthRequest"; export { CommonAuthorizationUrlRequest } from "./request/CommonAuthorizationUrlRequest"; diff --git a/lib/msal-common/src/request/RequestParameterBuilder.ts b/lib/msal-common/src/request/RequestParameterBuilder.ts index db83cadeea..e2f9dd5895 100644 --- a/lib/msal-common/src/request/RequestParameterBuilder.ts +++ b/lib/msal-common/src/request/RequestParameterBuilder.ts @@ -12,7 +12,7 @@ import { LibraryInfo } from "../config/ClientConfiguration"; import { StringUtils } from "../utils/StringUtils"; import { ServerTelemetryManager } from "../telemetry/server/ServerTelemetryManager"; import { ClientInfo } from "../account/ClientInfo"; -import { StkJwkThumbprint } from "../crypto/CryptoKeyManager"; +import { StkJwkThumbprint } from "../crypto/PopTokenGenerator"; export class RequestParameterBuilder { diff --git a/lib/msal-common/test/crypto/CryptoKeyManager.spec.ts b/lib/msal-common/test/crypto/CryptoKeyManager.spec.ts deleted file mode 100644 index a58f0bc353..0000000000 --- a/lib/msal-common/test/crypto/CryptoKeyManager.spec.ts +++ /dev/null @@ -1,93 +0,0 @@ -import sinon from "sinon"; -import { ICrypto, PkceCodes, AuthenticationScheme, CryptoKeyTypes } from "../../src"; -import { RANDOM_TEST_GUID, TEST_POP_VALUES, TEST_DATA_CLIENT_INFO, TEST_CONFIG, TEST_URIS } from "../test_kit/StringConstants"; -import { CryptoKeyManager } from "../../src/crypto/CryptoKeyManager"; - -describe("CryptoKeyManager Unit Tests", () => { - - afterEach(() => { - sinon.restore(); - }); - - const cryptoInterface: ICrypto = { - createNewGuid(): string { - return RANDOM_TEST_GUID; - }, - base64Decode(input: string): string { - switch (input) { - case TEST_POP_VALUES.ENCODED_REQ_CNF: - return TEST_POP_VALUES.DECODED_REQ_CNF; - case TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO: - return TEST_DATA_CLIENT_INFO.TEST_DECODED_CLIENT_INFO; - case TEST_POP_VALUES.SAMPLE_POP_AT_PAYLOAD_ENCODED: - return TEST_POP_VALUES.SAMPLE_POP_AT_PAYLOAD_DECODED; - default: - return input; - } - }, - base64Encode(input: string): string { - switch (input) { - case "123-test-uid": - return "MTIzLXRlc3QtdWlk"; - case "456-test-uid": - return "NDU2LXRlc3QtdWlk"; - case TEST_POP_VALUES.DECODED_REQ_CNF: - return TEST_POP_VALUES.ENCODED_REQ_CNF; - case TEST_POP_VALUES.SAMPLE_POP_AT_PAYLOAD_DECODED: - return TEST_POP_VALUES.SAMPLE_POP_AT_PAYLOAD_ENCODED; - default: - return input; - } - }, - async generatePkceCodes(): Promise { - return { - challenge: TEST_CONFIG.TEST_CHALLENGE, - verifier: TEST_CONFIG.TEST_VERIFIER - } - }, - async getPublicKeyThumbprint(): Promise { - return TEST_POP_VALUES.KID; - }, - async signJwt(): Promise { - return ""; - }, - async removeTokenBindingKey(): Promise { - return Promise.resolve(true); - }, - async clearKeystore(): Promise { - return Promise.resolve(true); - }, - async getAsymmetricPublicKey(): Promise { - return TEST_POP_VALUES.DECODED_STK_JWK_THUMBPRINT; - } - }; - - let cryptoKeyManager: CryptoKeyManager; - - const testPopRequest = { - authority: TEST_CONFIG.validAuthority, - scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, - correlationId: TEST_CONFIG.CORRELATION_ID, - authenticationScheme: AuthenticationScheme.POP, - resourceRequestMethod:"POST", - resourceRequestUrl: TEST_URIS.TEST_RESOURCE_ENDPT_WITH_PARAMS - }; - - beforeEach(() => { - cryptoKeyManager = new CryptoKeyManager(cryptoInterface); - }); - - describe("generateCnf", () => { - it("generates the req_cnf correctly", async () => { - const reqCnf = await cryptoKeyManager.generateCnf(testPopRequest); - expect(reqCnf).toBe(TEST_POP_VALUES.ENCODED_REQ_CNF); - }); - }); - - describe("generateKid", () => { - it("returns the correct kid and key storage location", async () => { - const reqCnf = await cryptoKeyManager.generateKid(testPopRequest, CryptoKeyTypes.ReqCnf); - expect(reqCnf).toStrictEqual(JSON.parse(TEST_POP_VALUES.DECODED_REQ_CNF)); - }); - }); -}); \ No newline at end of file