diff --git a/change/@azure-msal-browser-6d89bcc9-48e1-495e-bdd5-2d7fda8e13f8.json b/change/@azure-msal-browser-6d89bcc9-48e1-495e-bdd5-2d7fda8e13f8.json new file mode 100644 index 0000000000..3986730cca --- /dev/null +++ b/change/@azure-msal-browser-6d89bcc9-48e1-495e-bdd5-2d7fda8e13f8.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add support for apps to set their own `reqCnf` and correct native flows cnf format #6357", + "packageName": "@azure/msal-browser", + "email": "sameera.gajjarapu@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-msal-common-98b3791f-fcf5-4225-a03c-a379d2312455.json b/change/@azure-msal-common-98b3791f-fcf5-4225-a03c-a379d2312455.json new file mode 100644 index 0000000000..d73e6cd343 --- /dev/null +++ b/change/@azure-msal-common-98b3791f-fcf5-4225-a03c-a379d2312455.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add support for apps to set their own `reqCnf` and correct native flows cnf format #6357", + "packageName": "@azure/msal-common", + "email": "sameera.gajjarapu@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-msal-node-cf83856a-c1aa-4763-8eb2-0b62f5708a27.json b/change/@azure-msal-node-cf83856a-c1aa-4763-8eb2-0b62f5708a27.json new file mode 100644 index 0000000000..e8f24cffd4 --- /dev/null +++ b/change/@azure-msal-node-cf83856a-c1aa-4763-8eb2-0b62f5708a27.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fixed msal-node unit tests for PoP token support #\u0016\u00167119", + "packageName": "@azure/msal-node", + "email": "lalimasharda@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-browser/apiReview/msal-browser.api.md b/lib/msal-browser/apiReview/msal-browser.api.md index ba3032322a..71487d7366 100644 --- a/lib/msal-browser/apiReview/msal-browser.api.md +++ b/lib/msal-browser/apiReview/msal-browser.api.md @@ -234,7 +234,8 @@ declare namespace BrowserAuthErrorCodes { nativeConnectionNotEstablished, uninitializedPublicClientApplication, nativePromptNotSupported, - invalidBase64String + invalidBase64String, + invalidPopTokenRequest } } export { BrowserAuthErrorCodes } @@ -423,6 +424,10 @@ export const BrowserAuthErrorMessage: { code: string; desc: string; }; + invalidPopTokenRequest: { + code: string; + desc: string; + }; }; // Warning: (ae-missing-release-tag) "BrowserAuthOptions" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -1022,6 +1027,11 @@ const invalidBase64String = "invalid_base64_string"; // @public (undocumented) const invalidCacheType = "invalid_cache_type"; +// Warning: (ae-missing-release-tag) "invalidPopTokenRequest" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +const invalidPopTokenRequest = "invalid_pop_token_request"; + export { IPerformanceClient } // Warning: (ae-missing-release-tag) "IPublicClientApplication" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/lib/msal-browser/docs/access-token-proof-of-possession.md b/lib/msal-browser/docs/access-token-proof-of-possession.md index 8801e85ce2..603c1b2472 100644 --- a/lib/msal-browser/docs/access-token-proof-of-possession.md +++ b/lib/msal-browser/docs/access-token-proof-of-possession.md @@ -151,6 +151,14 @@ The Proof-of-Possession authentication scheme relies on an asymmetric cryptograp In the event of refreshing a bound access token, MSAL will delete the cryptographic keypair that was generated when requesting the expired bound access token, generate a new cryptographic keypair for the new access token, and store the new keypair in the keystore. +## Advanced feature: Application managed cryptographic keypair + +> :warning: We do not recommend using this feature unless you are familiar with the [Proof of Possession protocol](https://oauth.net/2/dpop/) and have a specific requirement to generate your own cryptographic keypair. For most cases, we recommend the PoP usage as described in the rest of this document. + +If you choose to generate your own cryptographic keypair, then this feature enables the application to provide the `popKid` as a request parameter. MSAL JS ensures the token issuer embeds the `cnf` in the token but returns the issued token _unsigned_. The onus of signing the access token before it is forwarded to the intended resource will be on the application. + +Please also note to make sure the remaining [pop parameters](#at-pop-request-parameters) except the `AuthenticationScheme` are not set if you choose to leverage this behavior. + ### Why access tokens are saved asynchronously Most MSAL credentials and cache items, like `ID Tokens` for example, can be stored and removed synchronously. This is because these cache items are stored in either `localStorage` or `sessionStorage` (which can be manipulated synchronously), and they have no dependencies on other stored items that have asynchronous access restrictions. diff --git a/lib/msal-browser/src/broker/nativeBroker/NativeRequest.ts b/lib/msal-browser/src/broker/nativeBroker/NativeRequest.ts index bdbbb819cf..7b39633035 100644 --- a/lib/msal-browser/src/broker/nativeBroker/NativeRequest.ts +++ b/lib/msal-browser/src/broker/nativeBroker/NativeRequest.ts @@ -31,6 +31,7 @@ export type NativeTokenRequest = { extendedExpiryToken?: boolean; extraParameters?: StringDict; storeInCache?: StoreInCache; // Object of booleans indicating whether to store tokens in the cache or not (default is true) + signPopToken?: boolean; // Set to true only if token request deos not contain a PoP keyId }; /** diff --git a/lib/msal-browser/src/crypto/CryptoOps.ts b/lib/msal-browser/src/crypto/CryptoOps.ts index 1f76c28f60..ea47705e08 100644 --- a/lib/msal-browser/src/crypto/CryptoOps.ts +++ b/lib/msal-browser/src/crypto/CryptoOps.ts @@ -78,6 +78,23 @@ export class CryptoOps implements ICrypto { return base64Decode(input); } + /** + * Encodes input string to base64 URL safe string. + * @param input + */ + base64UrlEncode(input: string): string { + return urlEncode(input); + } + + /** + * Stringifies and base64Url encodes input public key + * @param inputKid + * @returns Base64Url encoded public key + */ + encodeKid(inputKid: string): string { + return this.base64UrlEncode(JSON.stringify({ kid: inputKid })); + } + /** * Generates a keypair, stores it and returns a thumbprint * @param request diff --git a/lib/msal-browser/src/error/BrowserAuthError.ts b/lib/msal-browser/src/error/BrowserAuthError.ts index 44108202d0..483f03d33b 100644 --- a/lib/msal-browser/src/error/BrowserAuthError.ts +++ b/lib/msal-browser/src/error/BrowserAuthError.ts @@ -90,6 +90,8 @@ export const BrowserAuthErrorMessages = { "The provided prompt is not supported by the native platform. This request should be routed to the web based flow.", [BrowserAuthErrorCodes.invalidBase64String]: "Invalid base64 encoded string.", + [BrowserAuthErrorCodes.invalidPopTokenRequest]: + "Invalid PoP token request. The request should not have both a popKid value and signPopToken set to true.", }; /** @@ -333,6 +335,12 @@ export const BrowserAuthErrorMessage = { BrowserAuthErrorCodes.invalidBase64String ], }, + invalidPopTokenRequest: { + code: BrowserAuthErrorCodes.invalidPopTokenRequest, + desc: BrowserAuthErrorMessages[ + BrowserAuthErrorCodes.invalidPopTokenRequest + ], + }, }; /** diff --git a/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts b/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts index 993cead058..ac099a1db8 100644 --- a/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts +++ b/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts @@ -55,3 +55,4 @@ export const uninitializedPublicClientApplication = "uninitialized_public_client_application"; export const nativePromptNotSupported = "native_prompt_not_supported"; export const invalidBase64String = "invalid_base64_string"; +export const invalidPopTokenRequest = "invalid_pop_token_request"; diff --git a/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts b/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts index 8f273d142f..cb8de49d62 100644 --- a/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts @@ -170,10 +170,12 @@ export class NativeInteractionClient extends BaseInteractionClient { ); } + const { ...nativeTokenRequest } = nativeRequest; + // fall back to native calls const messageBody: NativeExtensionRequestBody = { method: NativeExtensionMethod.GetToken, - request: nativeRequest, + request: nativeTokenRequest, }; const response: object = await this.nativeMessageHandler.sendMessage( @@ -289,9 +291,11 @@ export class NativeInteractionClient extends BaseInteractionClient { ); const nativeRequest = await this.initializeNativeRequest(request); + const { ...nativeTokenRequest } = nativeRequest; + const messageBody: NativeExtensionRequestBody = { method: NativeExtensionMethod.GetToken, - request: nativeRequest, + request: nativeTokenRequest, }; try { @@ -481,7 +485,7 @@ export class NativeInteractionClient extends BaseInteractionClient { request, homeAccountIdentifier, idTokenClaims, - result.accessToken, + response.access_token, result.tenantId, reqTimestamp ); @@ -535,7 +539,10 @@ export class NativeInteractionClient extends BaseInteractionClient { response: NativeResponse, request: NativeTokenRequest ): Promise { - if (request.tokenType === AuthenticationScheme.POP) { + if ( + request.tokenType === AuthenticationScheme.POP && + request.signPopToken + ) { /** * This code prioritizes SHR returned from the native layer. In case of error/SHR not calculated from WAM and the AT * is still received, SHR is calculated locally @@ -725,7 +732,11 @@ export class NativeInteractionClient extends BaseInteractionClient { responseScopes.printScopes(), tokenExpirationSeconds, 0, - base64Decode + base64Decode, + undefined, + request.tokenType as AuthenticationScheme, + undefined, + request.keyId ); const nativeCacheRecord = new CacheRecord( @@ -917,8 +928,16 @@ export class NativeInteractionClient extends BaseInteractionClient { ...request.tokenQueryParameters, }, extendedExpiryToken: false, // Make this configurable? + keyId: request.popKid, }; + // Check for PoP token requests: signPopToken should only be set to true if popKid is not set + if (validatedRequest.signPopToken && !!request.popKid) { + throw createBrowserAuthError( + BrowserAuthErrorCodes.invalidPopTokenRequest + ); + } + this.handleExtraBrokerParams(validatedRequest); validatedRequest.extraParameters = validatedRequest.extraParameters || {}; @@ -935,17 +954,29 @@ export class NativeInteractionClient extends BaseInteractionClient { }; const popTokenGenerator = new PopTokenGenerator(this.browserCrypto); - const reqCnfData = await invokeAsync( - popTokenGenerator.generateCnf.bind(popTokenGenerator), - PerformanceEvents.PopTokenGenerateCnf, - this.logger, - this.performanceClient, - this.correlationId - )(shrParameters, this.logger); - - // to reduce the URL length, it is recommended to send the hash of the req_cnf instead of the whole string - validatedRequest.reqCnf = reqCnfData.reqCnfHash; - validatedRequest.keyId = reqCnfData.kid; + + // generate reqCnf if not provided in the request + let reqCnfData; + if (!validatedRequest.keyId) { + const generatedReqCnfData = await invokeAsync( + popTokenGenerator.generateCnf.bind(popTokenGenerator), + PerformanceEvents.PopTokenGenerateCnf, + this.logger, + this.performanceClient, + request.correlationId + )(shrParameters, this.logger); + reqCnfData = generatedReqCnfData.reqCnfString; + validatedRequest.keyId = generatedReqCnfData.kid; + validatedRequest.signPopToken = true; + } else { + reqCnfData = this.browserCrypto.base64UrlEncode( + JSON.stringify({ kid: validatedRequest.keyId }) + ); + validatedRequest.signPopToken = false; + } + + // SPAs require whole string to be passed to broker + validatedRequest.reqCnf = reqCnfData; } return validatedRequest; diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index 9f85f30cb4..e4a233baff 100644 --- a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts @@ -22,6 +22,7 @@ import { TEST_URIS, DEFAULT_TENANT_DISCOVERY_RESPONSE, DEFAULT_OPENID_CONFIG_RESPONSE, + TEST_REQ_CNF_DATA, } from "../utils/StringConstants"; import { AuthorizationUrlRequest } from "../../src/request/AuthorizationUrlRequest"; import { RedirectRequest } from "../../src/request/RedirectRequest"; @@ -141,6 +142,56 @@ describe("StandardInteractionClient", () => { await testClient.initializeAuthorizationCodeRequest(request); expect(request.codeChallenge).toBe(TEST_CONFIG.TEST_CHALLENGE); expect(authCodeRequest.codeVerifier).toBe(TEST_CONFIG.TEST_VERIFIER); + expect(authCodeRequest.popKid).toBeUndefined; + }); + + it("initializeAuthorizationCodeRequest validates the request and does not influence undefined popKid param", async () => { + const request: AuthorizationUrlRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: ["scope"], + loginHint: "AbeLi@microsoft.com", + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + correlationId: TEST_CONFIG.CORRELATION_ID, + responseMode: TEST_CONFIG.RESPONSE_MODE as ResponseMode, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ + challenge: TEST_CONFIG.TEST_CHALLENGE, + verifier: TEST_CONFIG.TEST_VERIFIER, + }); + + const authCodeRequest = + await testClient.initializeAuthorizationCodeRequest(request); + expect(authCodeRequest.popKid).toBeUndefined; + }); + + it("initializeAuthorizationCodeRequest validates the request and adds reqCnf param when user defined", async () => { + const request: AuthorizationUrlRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: ["scope"], + loginHint: "AbeLi@microsoft.com", + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + correlationId: TEST_CONFIG.CORRELATION_ID, + responseMode: TEST_CONFIG.RESPONSE_MODE as ResponseMode, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + popKid: TEST_REQ_CNF_DATA.kid, + }; + + jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({ + challenge: TEST_CONFIG.TEST_CHALLENGE, + verifier: TEST_CONFIG.TEST_VERIFIER, + }); + + const authCodeRequest = + await testClient.initializeAuthorizationCodeRequest(request); + expect(authCodeRequest.popKid).toEqual(TEST_REQ_CNF_DATA.kid); }); it("getDiscoveredAuthority - request authority only", async () => { diff --git a/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts b/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts index 8b23ec7faa..87f95ccb09 100644 --- a/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/InteractionHandler.spec.ts @@ -129,6 +129,14 @@ const cryptoInterface = { base64Encode: (input: string): string => { return "testEncodedString"; }, + base64UrlEncode(input: string): string { + return Buffer.from(input, "utf-8").toString("base64url"); + }, + encodeKid(input: string): string { + return Buffer.from(JSON.stringify({ kid: input }), "utf-8").toString( + "base64url" + ); + }, generatePkceCodes: async (): Promise => { return testPkceCodes; }, diff --git a/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts b/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts index a1784e1be9..1eadc8b645 100644 --- a/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts +++ b/lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts @@ -146,6 +146,15 @@ describe("RedirectHandler.ts Unit Tests", () => { base64Encode: (input: string): string => { return "testEncodedString"; }, + base64UrlEncode(input: string): string { + return Buffer.from(input, "utf-8").toString("base64url"); + }, + encodeKid(input: string): string { + return Buffer.from( + JSON.stringify({ kid: input }), + "utf-8" + ).toString("base64url"); + }, getPublicKeyThumbprint: async (): Promise => { return TEST_POP_VALUES.ENCODED_REQ_CNF; }, diff --git a/lib/msal-browser/test/utils/BrowserUtils.spec.ts b/lib/msal-browser/test/utils/BrowserUtils.spec.ts index 066f14c21b..3381ad7ea5 100644 --- a/lib/msal-browser/test/utils/BrowserUtils.spec.ts +++ b/lib/msal-browser/test/utils/BrowserUtils.spec.ts @@ -7,8 +7,7 @@ import { TEST_CONFIG, TEST_URIS } from "./StringConstants"; import { BrowserUtils, BrowserAuthError, - BrowserAuthErrorMessage, - InteractionType, + BrowserAuthErrorCodes, } from "../../src"; describe("BrowserUtils.ts Function Unit Tests", () => { @@ -101,7 +100,7 @@ describe("BrowserUtils.ts Function Unit Tests", () => { } catch (e) { const browserAuthError = e as BrowserAuthError; expect(browserAuthError.errorCode).toBe( - BrowserAuthErrorMessage.redirectInIframeError.code + BrowserAuthErrorCodes.redirectInIframe ); done(); } diff --git a/lib/msal-browser/test/utils/StringConstants.ts b/lib/msal-browser/test/utils/StringConstants.ts index 7f8e38c455..6109a4ad14 100644 --- a/lib/msal-browser/test/utils/StringConstants.ts +++ b/lib/msal-browser/test/utils/StringConstants.ts @@ -170,6 +170,11 @@ export const TEST_POP_VALUES = { '{"kid":"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs","xms_ksl":"sw"}', }; +export const TEST_REQ_CNF_DATA = { + kid: TEST_POP_VALUES.KID, + reqCnfString: TEST_POP_VALUES.DECODED_REQ_CNF, +}; + export const TEST_SSH_VALUES = { SSH_JWK: '{"kty":"RSA","n":"wDJwv083ZhGGkpMPVcBMwtSBNLu7qhT2VmKv7AyPEz_dWb8GQzNEnWT1niNjFI0isDMFWQ7X2O-dhTL9J1QguQ==","e":"AQAB"}', diff --git a/lib/msal-common/apiReview/msal-common.api.md b/lib/msal-common/apiReview/msal-common.api.md index 4a04dc4acb..fb1cf2ef49 100644 --- a/lib/msal-common/apiReview/msal-common.api.md +++ b/lib/msal-common/apiReview/msal-common.api.md @@ -600,6 +600,7 @@ export type BaseAuthRequest = { tokenQueryParameters?: StringDict; storeInCache?: StoreInCache; scenarioId?: string; + popKid?: string; }; // Warning: (ae-internal-missing-underscore) The name "BaseClient" should be prefixed with an underscore because the declaration is marked as @internal @@ -2115,9 +2116,12 @@ export interface ICrypto { base64Decode(input: string): string; // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen base64Encode(input: string): string; + base64UrlEncode(input: string): string; clearKeystore(): Promise; createNewGuid(): string; // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen + encodeKid(inputKid: string): string; + // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen getPublicKeyThumbprint(request: SignedHttpRequestParameters): Promise; // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen hashString(plainText: string): Promise; @@ -4247,9 +4251,9 @@ const X_MS_LIB_CAPABILITY = "x-ms-lib-capability"; // src/client/AuthorizationCodeClient.ts:228:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/client/AuthorizationCodeClient.ts:229:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/client/AuthorizationCodeClient.ts:307:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen -// src/client/AuthorizationCodeClient.ts:494:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen -// src/client/AuthorizationCodeClient.ts:702:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen -// src/client/AuthorizationCodeClient.ts:742:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen +// src/client/AuthorizationCodeClient.ts:501:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen +// src/client/AuthorizationCodeClient.ts:716:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen +// src/client/AuthorizationCodeClient.ts:756:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/client/RefreshTokenClient.ts:193:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/client/RefreshTokenClient.ts:277:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/client/RefreshTokenClient.ts:278:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen diff --git a/lib/msal-common/src/cache/CacheManager.ts b/lib/msal-common/src/cache/CacheManager.ts index 5595b391f5..469256ba85 100644 --- a/lib/msal-common/src/cache/CacheManager.ts +++ b/lib/msal-common/src/cache/CacheManager.ts @@ -1905,7 +1905,7 @@ export abstract class CacheManager implements ICacheManager { /** * Returns true if the credential's keyId matches the one in the request, false otherwise * @param entity - * @param tokenType + * @param keyId */ private matchKeyId(entity: CredentialEntity, keyId: string): boolean { return !!(entity.keyId && entity.keyId === keyId); diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 4e914da11a..286764acfc 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -390,15 +390,22 @@ export class AuthorizationCodeClient extends BaseClient { this.performanceClient ); - const reqCnfData = await invokeAsync( - popTokenGenerator.generateCnf.bind(popTokenGenerator), - PerformanceEvents.PopTokenGenerateCnf, - this.logger, - this.performanceClient, - request.correlationId - )(request, this.logger); + let reqCnfData; + if (!request.popKid) { + const generatedReqCnfData = await invokeAsync( + popTokenGenerator.generateCnf.bind(popTokenGenerator), + PerformanceEvents.PopTokenGenerateCnf, + this.logger, + this.performanceClient, + request.correlationId + )(request, this.logger); + reqCnfData = generatedReqCnfData.reqCnfString; + } else { + reqCnfData = this.cryptoUtils.encodeKid(request.popKid); + } + // SPA PoP requires full Base64Url encoded req_cnf string (unhashed) - parameterBuilder.addPopToken(reqCnfData.reqCnfString); + parameterBuilder.addPopToken(reqCnfData); } else if (request.authenticationScheme === AuthenticationScheme.SSH) { if (request.sshJwk) { parameterBuilder.addSshJwk(request.sshJwk); @@ -682,15 +689,22 @@ export class AuthorizationCodeClient extends BaseClient { const popTokenGenerator = new PopTokenGenerator( this.cryptoUtils ); - // to reduce the URL length, it is recommended to send the hash of the req_cnf instead of the whole string - const reqCnfData = await invokeAsync( - popTokenGenerator.generateCnf.bind(popTokenGenerator), - PerformanceEvents.PopTokenGenerateCnf, - this.logger, - this.performanceClient, - request.correlationId - )(request, this.logger); - parameterBuilder.addPopToken(reqCnfData.reqCnfHash); + + // req_cnf is always sent as a string for SPAs + let reqCnfData; + if (!request.popKid) { + const generatedReqCnfData = await invokeAsync( + popTokenGenerator.generateCnf.bind(popTokenGenerator), + PerformanceEvents.PopTokenGenerateCnf, + this.logger, + this.performanceClient, + request.correlationId + )(request, this.logger); + reqCnfData = generatedReqCnfData.reqCnfString; + } else { + reqCnfData = this.cryptoUtils.encodeKid(request.popKid); + } + parameterBuilder.addPopToken(reqCnfData); } } diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts index 0a34396174..45c7d13fcc 100644 --- a/lib/msal-common/src/client/RefreshTokenClient.ts +++ b/lib/msal-common/src/client/RefreshTokenClient.ts @@ -407,15 +407,24 @@ export class RefreshTokenClient extends BaseClient { this.cryptoUtils, this.performanceClient ); - const reqCnfData = await invokeAsync( - popTokenGenerator.generateCnf.bind(popTokenGenerator), - PerformanceEvents.PopTokenGenerateCnf, - this.logger, - this.performanceClient, - request.correlationId - )(request, this.logger); + + let reqCnfData; + if (!request.popKid) { + const generatedReqCnfData = await invokeAsync( + popTokenGenerator.generateCnf.bind(popTokenGenerator), + PerformanceEvents.PopTokenGenerateCnf, + this.logger, + this.performanceClient, + request.correlationId + )(request, this.logger); + + reqCnfData = generatedReqCnfData.reqCnfString; + } else { + reqCnfData = this.cryptoUtils.encodeKid(request.popKid); + } + // SPA PoP requires full Base64Url encoded req_cnf string (unhashed) - parameterBuilder.addPopToken(reqCnfData.reqCnfString); + parameterBuilder.addPopToken(reqCnfData); } else if (request.authenticationScheme === AuthenticationScheme.SSH) { if (request.sshJwk) { parameterBuilder.addSshJwk(request.sshJwk); diff --git a/lib/msal-common/src/crypto/ICrypto.ts b/lib/msal-common/src/crypto/ICrypto.ts index b35fb77cb4..658f33d40c 100644 --- a/lib/msal-common/src/crypto/ICrypto.ts +++ b/lib/msal-common/src/crypto/ICrypto.ts @@ -49,6 +49,16 @@ export interface ICrypto { * @param input */ base64Decode(input: string): string; + /** + * base64 URL safe encoded string + */ + base64UrlEncode(input: string): string; + /** + * Stringifies and base64Url encodes input public key + * @param inputKid + * @returns Base64Url encoded public key + */ + encodeKid(inputKid: string): string; /** * Generates an JWK RSA S256 Thumbprint * @param request @@ -92,6 +102,12 @@ export const DEFAULT_CRYPTO_IMPLEMENTATION: ICrypto = { base64Encode: (): string => { throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); }, + base64UrlEncode: (): string => { + throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); + }, + encodeKid: (): string => { + throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); + }, async getPublicKeyThumbprint(): Promise { throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); }, diff --git a/lib/msal-common/src/crypto/PopTokenGenerator.ts b/lib/msal-common/src/crypto/PopTokenGenerator.ts index b93ffc9d8f..13ada66a20 100644 --- a/lib/msal-common/src/crypto/PopTokenGenerator.ts +++ b/lib/msal-common/src/crypto/PopTokenGenerator.ts @@ -26,7 +26,6 @@ type ReqCnf = { export type ReqCnfData = { kid: string; reqCnfString: string; - reqCnfHash: string; }; const KeyLocation = { @@ -67,14 +66,13 @@ export class PopTokenGenerator { this.performanceClient, request.correlationId )(request); - const reqCnfString: string = this.cryptoUtils.base64Encode( + const reqCnfString: string = this.cryptoUtils.base64UrlEncode( JSON.stringify(reqCnf) ); return { kid: reqCnf.kid, reqCnfString, - reqCnfHash: await this.cryptoUtils.hashString(reqCnfString), }; } diff --git a/lib/msal-common/src/request/BaseAuthRequest.ts b/lib/msal-common/src/request/BaseAuthRequest.ts index b76071627b..541f2c9370 100644 --- a/lib/msal-common/src/request/BaseAuthRequest.ts +++ b/lib/msal-common/src/request/BaseAuthRequest.ts @@ -28,6 +28,7 @@ import { ShrOptions } from "../crypto/SignedHttpRequest"; * - tokenQueryParameters - String to string map of custom query parameters added to the /token call * - storeInCache - Object containing boolean values indicating whether to store tokens in the cache or not (default is true) * - scenarioId - Scenario id to track custom user prompts + * - popKid - Key ID to identify the public key for PoP token request */ export type BaseAuthRequest = { authority: string; @@ -48,4 +49,5 @@ export type BaseAuthRequest = { tokenQueryParameters?: StringDict; storeInCache?: StoreInCache; scenarioId?: string; + popKid?: string; }; diff --git a/lib/msal-common/src/response/ResponseHandler.ts b/lib/msal-common/src/response/ResponseHandler.ts index 6b66b214fb..c5a789a15e 100644 --- a/lib/msal-common/src/response/ResponseHandler.ts +++ b/lib/msal-common/src/response/ResponseHandler.ts @@ -591,8 +591,14 @@ export class ResponseHandler { let familyId: string = Constants.EMPTY_STRING; if (cacheRecord.accessToken) { + /* + * if the request object has `popKid` property, `signPopToken` will be set to false and + * the token will be returned unsigned + */ if ( - cacheRecord.accessToken.tokenType === AuthenticationScheme.POP + cacheRecord.accessToken.tokenType === + AuthenticationScheme.POP && + !request.popKid ) { const popTokenGenerator: PopTokenGenerator = new PopTokenGenerator(cryptoObj); diff --git a/lib/msal-common/test/account/AuthToken.spec.ts b/lib/msal-common/test/account/AuthToken.spec.ts index 5e1b06ba67..ccd778618e 100644 --- a/lib/msal-common/test/account/AuthToken.spec.ts +++ b/lib/msal-common/test/account/AuthToken.spec.ts @@ -47,6 +47,22 @@ describe("AuthToken.ts Class Unit Tests", () => { return input; } }, + base64UrlEncode(input: string): string { + switch (input) { + case '{"kid": "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc"}': + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, + encodeKid(input: string): string { + switch (input) { + case "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc": + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/account/ClientInfo.spec.ts b/lib/msal-common/test/account/ClientInfo.spec.ts index 67eee32423..bc3111ff29 100644 --- a/lib/msal-common/test/account/ClientInfo.spec.ts +++ b/lib/msal-common/test/account/ClientInfo.spec.ts @@ -48,6 +48,22 @@ describe("ClientInfo.ts Class Unit Tests", () => { return input; } }, + base64UrlEncode(input: string): string { + switch (input) { + case '{"kid": "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc"}': + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, + encodeKid(input: string): string { + switch (input) { + case "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc": + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/cache/entities/AccountEntity.spec.ts b/lib/msal-common/test/cache/entities/AccountEntity.spec.ts index e859485783..0dbe7110e8 100644 --- a/lib/msal-common/test/cache/entities/AccountEntity.spec.ts +++ b/lib/msal-common/test/cache/entities/AccountEntity.spec.ts @@ -58,6 +58,22 @@ const cryptoInterface: ICrypto = { return input; } }, + base64UrlEncode(input: string): string { + switch (input) { + case '{"kid": "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc"}': + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, + encodeKid(input: string): string { + switch (input) { + case "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc": + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts b/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts index e2edca58e8..50ad572644 100644 --- a/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts +++ b/lib/msal-common/test/client/AuthorizationCodeClient.spec.ts @@ -1294,6 +1294,57 @@ describe("AuthorizationCodeClient unit tests", () => { }); }); + it("Adds req-cnf as needed", async () => { + // Override with alternate authority openid_config + sinon + .stub(Authority.prototype, "getEndpointMetadataFromNetwork") + .resolves(DEFAULT_OPENID_CONFIG_RESPONSE.body); + + const config: ClientConfiguration = + await ClientTestUtils.createTestClientConfiguration(); + const client = new AuthorizationCodeClient(config); + + if (!config.cryptoInterface) { + throw TestError.createTestSetupError( + "configuration cryptoInterface not initialized correctly." + ); + } + + const authCodeUrlRequest: CommonAuthorizationUrlRequest = { + redirectUri: TEST_URIS.TEST_REDIRECT_URI_LOCALHOST, + scopes: [ + ...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + ...TEST_CONFIG.DEFAULT_SCOPES, + ], + authority: TEST_CONFIG.validAuthority, + responseMode: ResponseMode.FORM_POST, + codeChallenge: TEST_CONFIG.TEST_CHALLENGE, + codeChallengeMethod: TEST_CONFIG.CODE_CHALLENGE_METHOD, + state: TEST_CONFIG.STATE, + prompt: PromptValue.LOGIN, + loginHint: TEST_CONFIG.LOGIN_HINT, + domainHint: TEST_CONFIG.DOMAIN_HINT, + claims: TEST_CONFIG.CLAIMS, + nonce: TEST_CONFIG.NONCE, + correlationId: RANDOM_TEST_GUID, + authenticationScheme: AuthenticationScheme.POP, + nativeBroker: true, + }; + const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest); + expect( + loginUrl.includes( + `${AADServerParamKeys.NATIVE_BROKER}=${encodeURIComponent("1")}` + ) + ).toBe(true); + expect( + loginUrl.includes( + `${AADServerParamKeys.REQ_CNF}=${encodeURIComponent( + TEST_POP_VALUES.ENCODED_REQ_CNF + )}` + ) + ).toBe(true); + }); + describe("handleFragmentResponse()", () => { it("returns valid server code response", async () => { const config: ClientConfiguration = @@ -2579,7 +2630,8 @@ describe("AuthorizationCodeClient unit tests", () => { } }); - const signedJwt = "signedJwt"; + const signedJwt = + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; config.cryptoInterface.signJwt = async ( // @ts-ignore @@ -2706,9 +2758,7 @@ describe("AuthorizationCodeClient unit tests", () => { ).toBe(true); expect( returnVal.includes( - `${AADServerParamKeys.REQ_CNF}=${encodeURIComponent( - TEST_POP_VALUES.ENCODED_REQ_CNF - )}` + `${AADServerParamKeys.REQ_CNF}=${TEST_POP_VALUES.ENCODED_REQ_CNF}` ) ).toBe(true); expect( @@ -2776,7 +2826,8 @@ describe("AuthorizationCodeClient unit tests", () => { return input; } }; - const signedJwt = "signedJwt"; + const signedJwt = + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; config.cryptoInterface.signJwt = async ( // @ts-ignore diff --git a/lib/msal-common/test/client/ClientTestUtils.ts b/lib/msal-common/test/client/ClientTestUtils.ts index aeafc9c8d3..eaf5deb731 100644 --- a/lib/msal-common/test/client/ClientTestUtils.ts +++ b/lib/msal-common/test/client/ClientTestUtils.ts @@ -227,6 +227,14 @@ export const mockCrypto = { base64Encode(input: string): string { return Buffer.from(input, "utf-8").toString("base64"); }, + base64UrlEncode(input: string): string { + return Buffer.from(input, "utf-8").toString("base64url"); + }, + encodeKid(input: string): string { + return Buffer.from(JSON.stringify({ kid: input }), "utf-8").toString( + "base64url" + ); + }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/client/RefreshTokenClient.spec.ts b/lib/msal-common/test/client/RefreshTokenClient.spec.ts index 3a5253bd26..621e4e1f30 100644 --- a/lib/msal-common/test/client/RefreshTokenClient.spec.ts +++ b/lib/msal-common/test/client/RefreshTokenClient.spec.ts @@ -619,6 +619,7 @@ describe("RefreshTokenClient unit tests", () => { ); await client.acquireTokenByRefreshToken(silentFlowRequest); + expect(refreshTokenClientSpy.called).toBe(true); expect( refreshTokenClientSpy.calledWith(expectedRefreshRequest) ).toBe(true); diff --git a/lib/msal-common/test/config/ClientConfiguration.spec.ts b/lib/msal-common/test/config/ClientConfiguration.spec.ts index c7cb0ac533..bcce9f5eb2 100644 --- a/lib/msal-common/test/config/ClientConfiguration.spec.ts +++ b/lib/msal-common/test/config/ClientConfiguration.spec.ts @@ -152,6 +152,22 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { base64Encode: (input: string): string => { return "testEncodedString"; }, + base64UrlEncode(input: string): string { + switch (input) { + case '{"kid": "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc"}': + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, + encodeKid(input: string): string { + switch (input) { + case "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc": + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts b/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts index a89d29c7b4..5da3167022 100644 --- a/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts +++ b/lib/msal-common/test/crypto/PopTokenGenerator.spec.ts @@ -51,6 +51,26 @@ describe("PopTokenGenerator Unit Tests", () => { return input; } }, + base64UrlEncode(input: string): string { + switch (input) { + case '{"kid": "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc"}': + return "e2tpZDogIlhuc3VBdnR0VFBwMG5uMUtfWU1MZVBMRGJwN3N5Q0toTkh0N0hqWUhKWWMifQ"; + case '{"kid":"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs","xms_ksl":"sw"}': + return "eyJraWQiOiJOemJMc1hoOHVEQ2NkLTZNTndYRjRXXzdub1dYRlpBZkhreFpzUkdDOVhzIiwieG1zX2tzbCI6InN3In0"; + default: + return input; + } + }, + encodeKid(input: string): string { + switch (input) { + case "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc": + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + case "NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs": + return "eyJraWQiOiJOemJMc1hoOHVEQ2NkLTZNTndYRjRXXzdub1dYRlpBZkhreFpzUkdDOVhzIiwieG1zX2tzbCI6InN3In0"; + default: + return input; + } + }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, @@ -86,9 +106,6 @@ describe("PopTokenGenerator Unit Tests", () => { TEST_POP_VALUES.ENCODED_REQ_CNF ); expect(reqCnfData.kid).toBe(TEST_POP_VALUES.KID); - expect(reqCnfData.reqCnfHash).toBe( - TEST_CRYPTO_VALUES.TEST_SHA256_HASH - ); }); }); diff --git a/lib/msal-common/test/response/ResponseHandler.spec.ts b/lib/msal-common/test/response/ResponseHandler.spec.ts index a3c0c793a6..2298c5f142 100644 --- a/lib/msal-common/test/response/ResponseHandler.spec.ts +++ b/lib/msal-common/test/response/ResponseHandler.spec.ts @@ -57,7 +57,8 @@ const networkInterface: INetworkModule = { return {} as T; }, }; -const signedJwt = "SignedJwt"; +const signedJwt = + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJjbmYiOnsia2lkIjoiTnpiTHNYaDh1RENjZC02TU53WEY0V183bm9XWEZaQWZIa3hac1JHQzlYcyJ9fQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; const cryptoInterface: ICrypto = { createNewGuid(): string { return RANDOM_TEST_GUID; @@ -86,6 +87,22 @@ const cryptoInterface: ICrypto = { return input; } }, + base64UrlEncode(input: string): string { + switch (input) { + case '{"kid": "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc"}': + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, + encodeKid(input: string): string { + switch (input) { + case "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc": + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, @@ -630,6 +647,53 @@ describe("ResponseHandler.ts", () => { expect(result.accessToken).toBe(signedJwt); }); + it("Does not sign access token when PoP kid is set and PoP scheme enabled", async () => { + const testRequest: BaseAuthRequest = { + authority: testAuthority.canonicalAuthority, + correlationId: "CORRELATION_ID", + scopes: ["openid", "profile", "User.Read", "email"], + popKid: TEST_POP_VALUES.POPKID, + }; + const testResponse: ServerAuthorizationTokenResponse = { + ...POP_AUTHENTICATION_RESULT.body, + }; + claimsStub.callsFake( + (encodedToken: string, crypto: ICrypto): TokenClaims | null => { + switch (encodedToken) { + case testResponse.id_token: + return ID_TOKEN_CLAIMS as TokenClaims; + case testResponse.access_token: + return { + cnf: { + kid: TEST_POP_VALUES.KID, + }, + }; + default: + return null; + } + } + ); + + const responseHandler = new ResponseHandler( + "this-is-a-client-id", + testCacheManager, + cryptoInterface, + logger, + null, + null + ); + const timestamp = TimeUtils.nowSeconds(); + const result = await responseHandler.handleServerTokenResponse( + testResponse, + testAuthority, + timestamp, + testRequest + ); + + expect(result.tokenType).toBe(AuthenticationScheme.POP); + expect(result.accessToken).toBe(testResponse.access_token); + }); + it("sets default value if requestId not provided", async () => { const testRequest: BaseAuthRequest = { authority: testAuthority.canonicalAuthority, diff --git a/lib/msal-common/test/test_kit/StringConstants.ts b/lib/msal-common/test/test_kit/StringConstants.ts index 41e4e78c5f..7ca729c87d 100644 --- a/lib/msal-common/test/test_kit/StringConstants.ts +++ b/lib/msal-common/test/test_kit/StringConstants.ts @@ -234,8 +234,11 @@ export const TEST_POP_VALUES = { CLIENT_CLAIMS: '{"customClaim":"CustomClaimValue","anotherClaim":"AnotherValue"}', KID: "NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs", + POPKID: "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc", + POPKID_OBJ: + '{"kid":"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs","xms_ksl":"sw"}', ENCODED_REQ_CNF: - "eyJraWQiOiJOemJMc1hoOHVEQ2NkLTZNTndYRjRXXzdub1dYRlpBZkhreFpzUkdDOVhzIiwieG1zX2tzbCI6InN3In0=", + "eyJraWQiOiJOemJMc1hoOHVEQ2NkLTZNTndYRjRXXzdub1dYRlpBZkhreFpzUkdDOVhzIiwieG1zX2tzbCI6InN3In0", DECODED_REQ_CNF: '{"kid":"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs","xms_ksl":"sw"}', SAMPLE_POP_AT: diff --git a/lib/msal-common/test/utils/ProtocolUtils.spec.ts b/lib/msal-common/test/utils/ProtocolUtils.spec.ts index 814a455d4c..89b651a506 100644 --- a/lib/msal-common/test/utils/ProtocolUtils.spec.ts +++ b/lib/msal-common/test/utils/ProtocolUtils.spec.ts @@ -44,6 +44,22 @@ describe("ProtocolUtils.ts Class Unit Tests", () => { return input; } }, + base64UrlEncode(input: string): string { + switch (input) { + case '{"kid": "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc"}': + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, + encodeKid(input: string): string { + switch (input) { + case "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc": + return "eyJraWQiOiAiWG5zdUF2dHRUUHAwbm4xS19ZTUxlUExEYnA3c3lDS2hOSHQ3SGpZSEpZYyJ9"; + default: + return input; + } + }, async getPublicKeyThumbprint(): Promise { return TEST_POP_VALUES.KID; }, diff --git a/lib/msal-node/apiReview/msal-node.api.md b/lib/msal-node/apiReview/msal-node.api.md index 7d0498c81a..ce703eb202 100644 --- a/lib/msal-node/apiReview/msal-node.api.md +++ b/lib/msal-node/apiReview/msal-node.api.md @@ -225,8 +225,11 @@ export class CryptoProvider implements ICrypto { constructor(); base64Decode(input: string): string; base64Encode(input: string): string; + base64UrlEncode(): string; clearKeystore(): Promise; createNewGuid(): string; + // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen + encodeKid(): string; generatePkceCodes(): Promise; getPublicKeyThumbprint(): Promise; hashString(plainText: string): Promise; diff --git a/lib/msal-node/src/crypto/CryptoProvider.ts b/lib/msal-node/src/crypto/CryptoProvider.ts index 21464f1bbf..3c260f1494 100644 --- a/lib/msal-node/src/crypto/CryptoProvider.ts +++ b/lib/msal-node/src/crypto/CryptoProvider.ts @@ -26,6 +26,21 @@ export class CryptoProvider implements ICrypto { this.hashUtils = new HashUtils(); } + /** + * base64 URL safe encoded string + */ + base64UrlEncode(): string { + throw new Error("Method not implemented."); + } + /** + * Stringifies and base64Url encodes input public key + * @param inputKid + * @returns Base64Url encoded public key + */ + encodeKid(): string { + throw new Error("Method not implemented."); + } + /** * Creates a new random GUID - used to populate state and nonce. * @returns string (GUID) diff --git a/lib/msal-node/test/client/ClientTestUtils.ts b/lib/msal-node/test/client/ClientTestUtils.ts index 1e87320889..d91dba4ab4 100644 --- a/lib/msal-node/test/client/ClientTestUtils.ts +++ b/lib/msal-node/test/client/ClientTestUtils.ts @@ -246,6 +246,22 @@ export const mockCrypto = { return input; } }, + base64UrlEncode(input: string): string { + switch (input) { + case TEST_POP_VALUES.DECODED_REQ_CNF: + return TEST_POP_VALUES.URLSAFE_ENCODED_REQCNF; + default: + return input; + } + }, + encodeKid(input: string): string { + switch (input) { + case TEST_POP_VALUES.KID: + return TEST_POP_VALUES.URLSAFE_ENCODED_REQCNF; + default: + return input; + } + }, async generatePkceCodes(): Promise { return { challenge: TEST_CONFIG.TEST_CHALLENGE, diff --git a/lib/msal-node/test/test_kit/StringConstants.ts b/lib/msal-node/test/test_kit/StringConstants.ts index 965909e140..f858d6ab6f 100644 --- a/lib/msal-node/test/test_kit/StringConstants.ts +++ b/lib/msal-node/test/test_kit/StringConstants.ts @@ -218,6 +218,8 @@ export const TEST_POP_VALUES = { CLIENT_CLAIMS: '{"customClaim":"CustomClaimValue","anotherClaim":"AnotherValue"}', KID: "NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs", + URLSAFE_ENCODED_REQCNF: + "eyJraWQiOiJOemJMc1hoOHVEQ2NkLTZNTndYRjRXXzdub1dYRlpBZkhreFpzUkdDOVhzIiwieG1zX2tzbCI6InN3In0", ENCODED_REQ_CNF: "eyJraWQiOiJOemJMc1hoOHVEQ2NkLTZNTndYRjRXXzdub1dYRlpBZkhreFpzUkdDOVhzIiwieG1zX2tzbCI6InN3In0=", DECODED_REQ_CNF: diff --git a/lib/msal-node/test/utils/TestConstants.ts b/lib/msal-node/test/utils/TestConstants.ts index 06c81e69bf..13a3abb1e1 100644 --- a/lib/msal-node/test/utils/TestConstants.ts +++ b/lib/msal-node/test/utils/TestConstants.ts @@ -120,6 +120,12 @@ export const DEFAULT_CRYPTO_IMPLEMENTATION: ICrypto = { async hashString(): Promise { throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); }, + base64UrlEncode: function (): string { + throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); + }, + encodeKid: function (): string { + throw createClientAuthError(ClientAuthErrorCodes.methodNotImplemented); + }, }; export const DEFAULT_OPENID_CONFIG_RESPONSE = { diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/authConfig.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/authConfig.js index dfc2f203c3..0eb1aab333 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/authConfig.js +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/authConfig.js @@ -11,6 +11,7 @@ const msalConfig = { system: { allowNativeBroker: false, // Disables WAM Broker loggerOptions: { + logLevel: msal.LogLevel.Verbose, loggerCallback: (level, message, containsPii) => { if (containsPii) { return; @@ -55,6 +56,10 @@ const silentRequest = { scopes: ["openid", "profile", "User.Read"], }; +const bearerTokenRequest = { + scopes: ["openid", "profile", "User.Read"] +} + const popTokenRequest = { scopes: ["openid", "profile", "User.Read"], authenticationScheme: msal.AuthenticationScheme.POP, @@ -62,6 +67,8 @@ const popTokenRequest = { resourceRequestUri: popConfig.endpoint } -const bearerTokenRequest = { - scopes: ["openid", "profile", "User.Read"] -} +const popTokenWithKidRequest = { + scopes: ["openid", "profile", "User.Read"], + authenticationScheme: msal.AuthenticationScheme.POP, + popKid: "XnsuAvttTPp0nn1K_YMLePLDbp7syCKhNHt7HjYHJYc", +}; diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/graph.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/graph.js index 122f660d52..de3ccc5261 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/graph.js +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/graph.js @@ -70,6 +70,20 @@ async function fetchPopToken() { } } +async function fetchPopTokenWithKid() { + const currentAcc = myMSALObj.getAccountByUsername(username); + if (currentAcc) { + return getTokenPopup(popTokenWithKidRequest, currentAcc).then(response => { + if (response.accessToken) { + showPopTokenWithKidAcquired(response.accessToken); + return response.accessToken; + } + }).catch(error => { + console.log(error); + }); + } +} + async function seeProfile() { const currentAcc = myMSALObj.getAccountByUsername(username); if (currentAcc) { diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/index.html b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/index.html index 2ffa8823a8..14e5b1b97b 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/index.html +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/index.html @@ -45,6 +45,9 @@
Please sign-in to see your profile an

+ +
+
diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/test/browser.spec.ts b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/test/browser.spec.ts index d222989711..6de6b4a7c1 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/test/browser.spec.ts +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/test/browser.spec.ts @@ -111,4 +111,46 @@ describe("Browser PoP tests", function () { const storage = await BrowserCache.getWindowStorage(); expect(Object.keys(storage).length).toEqual(7); }); + + it("Performs loginRedirect, acquires and verifies a PoP token is unsigned if PoP kid is provided in request", async () => { + const testName = "redirectBaseCaseWithCnf"; + 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"); + // Enter credentials + await enterCredentials(page, screenshot, username, accountPwd); + await page.waitForSelector("#popCnfToken", { visible: true }); + await screenshot.takeScreenshot(page, "samplePageLoggedIn"); + await page.click("#popCnfToken"); + await page.waitForSelector("#PopTokenWithKidAcquired"); + await screenshot.takeScreenshot(page, "popTokenWithCnfClicked"); + console.log("Waiting for pop token to be generated"); + const tokenStore = await BrowserCache.getTokens(); + expect(tokenStore.idTokens).toHaveLength(1); + // One Bearer Token and one PoP token + expect(tokenStore.accessTokens).toHaveLength(2); + expect(tokenStore.refreshTokens).toHaveLength(1); + const cachedAccount = await BrowserCache.getAccountFromCache( + tokenStore.idTokens[0] + ); + const defaultCachedToken = + await BrowserCache.accessTokenForScopesExists( + tokenStore.accessTokens, + ["openid", "profile", "user.read"] + ); + expect(cachedAccount).toBeDefined(); + expect(defaultCachedToken).toBeTruthy(); + + const storage = await BrowserCache.getWindowStorage(); + expect(Object.keys(storage).length).toEqual(7); + }); }); diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/ui.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/ui.js index 826b718019..2fa9805991 100644 --- a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/ui.js +++ b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/pop/ui.js @@ -6,6 +6,7 @@ const popCardDiv = document.getElementById("pop-card-div"); const profileButton = document.getElementById("seeProfile"); const profileDiv = document.getElementById("profile-div"); const popTokenAcquired = document.getElementById("PopTokenAcquired"); +const popTokenWithCnfAcquired = document.getElementById("PopTokenWithCnfAcquired"); const jwtBodyView = document.getElementById("jwtBodyView"); const jwtHeaderView = document.getElementById("jwtHeaderView"); @@ -35,6 +36,22 @@ function showPopTokenAcquired(encodedJwt) { jwtBodyView.textContent = jwtBody; } +function showPopTokenWithKidAcquired(encodedJwt) { + popCardDiv.style.display = 'initial' + const popTokenWithCnfAcquired = document.createElement('p'); + popTokenWithCnfAcquired.setAttribute("id", "PopTokenWithKidAcquired"); + popTokenWithCnfAcquired.innerHTML = "Successfully acquired PoP Token with kid"; + profileDiv.appendChild(popTokenWithCnfAcquired); + + const jwtWindow = document.getElementById("jwtWindow"); + const splitJwt = encodedJwt.split("."); + const jwtHeader = JSON.stringify(JSON.parse(atob(splitJwt[0])), null, 4); + const jwtBody = JSON.stringify(JSON.parse(atob(splitJwt[1])), null, 4); + jwtBodyView.style = "white-space: pre-wrap"; + jwtHeaderView.textContent = jwtHeader; + jwtBodyView.textContent = jwtBody; +} + function updateUI(data, endpoint) { console.log('Graph API responded at: ' + new Date().toString());