From 650c0837d15eb6a3d8323b73a2495ea272e5f182 Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Tue, 30 Apr 2024 16:02:17 -0400 Subject: [PATCH] removed thrown error claimsBasedCaching. Marked it as deprecated in msal-node --- ...-2f219ed5-baf1-4c17-bb00-4a6f0d3d5f04.json | 7 - ...-c7f27295-64bd-44e2-b07f-d4bf9c4b7d6a.json | 7 - lib/msal-browser/src/config/Configuration.ts | 6 - .../test/app/PublicClientApplication.spec.ts | 381 ++++++++++++++++++ .../test/config/Configuration.spec.ts | 23 +- .../src/config/ClientConfiguration.ts | 10 - .../src/error/ClientConfigurationError.ts | 2 - .../error/ClientConfigurationErrorCodes.ts | 1 - .../test/client/SilentFlowClient.spec.ts | 65 +++ .../test/config/ClientConfiguration.spec.ts | 54 +-- lib/msal-node/src/config/Configuration.ts | 11 +- .../client/ClientCredentialClient.spec.ts | 16 - .../client/PublicClientApplication.spec.ts | 45 ++- .../test/config/ClientConfiguration.spec.ts | 17 - 14 files changed, 513 insertions(+), 132 deletions(-) delete mode 100644 change/@azure-msal-browser-2f219ed5-baf1-4c17-bb00-4a6f0d3d5f04.json delete mode 100644 change/@azure-msal-common-c7f27295-64bd-44e2-b07f-d4bf9c4b7d6a.json diff --git a/change/@azure-msal-browser-2f219ed5-baf1-4c17-bb00-4a6f0d3d5f04.json b/change/@azure-msal-browser-2f219ed5-baf1-4c17-bb00-4a6f0d3d5f04.json deleted file mode 100644 index cb4452e7e0..0000000000 --- a/change/@azure-msal-browser-2f219ed5-baf1-4c17-bb00-4a6f0d3d5f04.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "patch", - "comment": "Removed claimsBasedCaching from msal-browser", - "packageName": "@azure/msal-browser", - "email": "rginsburg@microsoft.com", - "dependentChangeType": "patch" -} diff --git a/change/@azure-msal-common-c7f27295-64bd-44e2-b07f-d4bf9c4b7d6a.json b/change/@azure-msal-common-c7f27295-64bd-44e2-b07f-d4bf9c4b7d6a.json deleted file mode 100644 index 2c8295975f..0000000000 --- a/change/@azure-msal-common-c7f27295-64bd-44e2-b07f-d4bf9c4b7d6a.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "patch", - "comment": "ClientCredential and OBO acquireToken requests with claims will now skip the cache", - "packageName": "@azure/msal-common", - "email": "rginsburg@microsoft.com", - "dependentChangeType": "patch" -} diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts index eba6c9c3e9..bbd53b4cfa 100644 --- a/lib/msal-browser/src/config/Configuration.ts +++ b/lib/msal-browser/src/config/Configuration.ts @@ -252,12 +252,6 @@ export function buildConfiguration( }: Configuration, isBrowserEnvironment: boolean ): BrowserConfiguration { - if (userInputCache?.claimsBasedCachingEnabled) { - throw createClientConfigurationError( - ClientConfigurationErrorCodes.claimsBasedCachingEnabled - ); - } - // Default auth options for browser const DEFAULT_AUTH_OPTIONS: InternalAuthOptions = { clientId: Constants.EMPTY_STRING, diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index fe0ac929c4..1fd1a6b3ab 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -3933,6 +3933,115 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { expect(parallelResponse).toHaveLength(3); }); + it("makes one network request with multiple parallel silent requests with same request including claims when claimsBasedCaching is enabled", async () => { + pca = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + system: { + allowNativeBroker: false, + }, + cache: { + claimsBasedCachingEnabled: true, + }, + }); + + await pca.initialize(); + const testServerTokenResponse = { + token_type: TEST_CONFIG.TOKEN_TYPE_BEARER, + scope: TEST_CONFIG.DEFAULT_SCOPES.join(" "), + expires_in: TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN, + ext_expires_in: TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN, + access_token: TEST_TOKENS.ACCESS_TOKEN, + refresh_token: TEST_TOKENS.REFRESH_TOKEN, + id_token: TEST_TOKENS.IDTOKEN_V2, + }; + const testIdTokenClaims: TokenClaims = { + ver: "2.0", + iss: "https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0", + sub: "AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ", + name: "Abe Lincoln", + preferred_username: "AbeLi@microsoft.com", + oid: "00000000-0000-0000-66f3-3332eca7ea81", + tid: "3338040d-6c67-4c5b-b112-36a304b66dad", + nonce: "123523", + }; + const testAccount: AccountInfo = { + homeAccountId: TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID, + localAccountId: TEST_DATA_CLIENT_INFO.TEST_UID, + environment: "login.windows.net", + tenantId: testIdTokenClaims.tid || "", + username: testIdTokenClaims.preferred_username || "", + }; + const testTokenResponse: AuthenticationResult = { + authority: TEST_CONFIG.validAuthority, + uniqueId: testIdTokenClaims.oid || "", + tenantId: testIdTokenClaims.tid || "", + scopes: [...TEST_CONFIG.DEFAULT_SCOPES, "User.Read"], + idToken: testServerTokenResponse.id_token, + idTokenClaims: testIdTokenClaims, + accessToken: testServerTokenResponse.access_token, + fromCache: false, + correlationId: RANDOM_TEST_GUID, + expiresOn: new Date( + Date.now() + testServerTokenResponse.expires_in * 1000 + ), + account: testAccount, + tokenType: AuthenticationScheme.BEARER, + }; + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); + sinon + .stub(CryptoOps.prototype, "hashString") + .resolves(TEST_CRYPTO_VALUES.TEST_SHA256_HASH); + const atsSpy = sinon.spy( + StandardController.prototype, + "acquireTokenSilentAsync" + ); + const silentATStub = sinon + .stub( + RefreshTokenClient.prototype, + "acquireTokenByRefreshToken" + ) + .resolves(testTokenResponse); + const tokenRequest: CommonSilentFlowRequest = { + scopes: ["User.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.BEARER, + claims: JSON.stringify({ claim: "claim" }), + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + const expectedTokenRequest: CommonSilentFlowRequest = { + ...tokenRequest, + scopes: ["User.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + claims: JSON.stringify({ claim: "claim" }), + requestedClaimsHash: TEST_CRYPTO_VALUES.TEST_SHA256_HASH, + forceRefresh: false, + }; + + const silentRequest1 = pca.acquireTokenSilent(tokenRequest); + const silentRequest2 = pca.acquireTokenSilent(tokenRequest); + const silentRequest3 = pca.acquireTokenSilent(tokenRequest); + const parallelResponse = await Promise.all([ + silentRequest1, + silentRequest2, + silentRequest3, + ]); + + expect(silentATStub.calledWith(expectedTokenRequest)).toBeTruthy(); + expect(atsSpy.calledOnce).toBe(true); + expect(silentATStub.callCount).toEqual(1); + expect(parallelResponse[0]).toEqual(testTokenResponse); + expect(parallelResponse[1]).toEqual(testTokenResponse); + expect(parallelResponse[2]).toEqual(testTokenResponse); + expect(parallelResponse).toHaveLength(3); + }); + it("makes network requests for each distinct request when acquireTokenSilent is called in parallel", async () => { const testServerTokenResponse = { token_type: TEST_CONFIG.TOKEN_TYPE_BEARER, @@ -4138,6 +4247,278 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { expect(silentATStub.callCount).toEqual(6); }); + it("makes network requests for each distinct request including claims when acquireTokenSilent is called in parallel with claimsBasedCaching is enabled", async () => { + pca = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + system: { + allowNativeBroker: false, + }, + cache: { + claimsBasedCachingEnabled: true, + }, + }); + + await pca.initialize(); + const testServerTokenResponse = { + token_type: TEST_CONFIG.TOKEN_TYPE_BEARER, + scope: TEST_CONFIG.DEFAULT_SCOPES.join(" "), + expires_in: TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN, + ext_expires_in: TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN, + access_token: TEST_TOKENS.ACCESS_TOKEN, + refresh_token: TEST_TOKENS.REFRESH_TOKEN, + id_token: TEST_TOKENS.IDTOKEN_V2, + }; + const testIdTokenClaims: TokenClaims = { + ver: "2.0", + iss: "https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0", + sub: "AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ", + name: "Abe Lincoln", + preferred_username: "AbeLi@microsoft.com", + oid: "00000000-0000-0000-66f3-3332eca7ea81", + tid: "3338040d-6c67-4c5b-b112-36a304b66dad", + nonce: "123523", + }; + const testAccount: AccountInfo = { + homeAccountId: TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID, + localAccountId: TEST_DATA_CLIENT_INFO.TEST_UID, + environment: "login.windows.net", + tenantId: testIdTokenClaims.tid || "", + username: testIdTokenClaims.preferred_username || "", + }; + const testTokenResponse: AuthenticationResult = { + authority: TEST_CONFIG.validAuthority, + uniqueId: testIdTokenClaims.oid || "", + tenantId: testIdTokenClaims.tid || "", + scopes: [...TEST_CONFIG.DEFAULT_SCOPES, "User.Read"], + idToken: testServerTokenResponse.id_token, + idTokenClaims: testIdTokenClaims, + accessToken: testServerTokenResponse.access_token, + fromCache: false, + correlationId: RANDOM_TEST_GUID, + expiresOn: new Date( + Date.now() + testServerTokenResponse.expires_in * 1000 + ), + account: testAccount, + tokenType: AuthenticationScheme.BEARER, + }; + jest.spyOn(BrowserCrypto, "createNewGuid").mockReturnValue( + RANDOM_TEST_GUID + ); + sinon + .stub(BrowserCrypto, "hashString") + .resolves(TEST_CRYPTO_VALUES.TEST_SHA256_HASH); + const silentATStub = sinon + .stub( + RefreshTokenClient.prototype, + "acquireTokenByRefreshToken" + ) + .resolves(testTokenResponse); + // Beaerer requests + const tokenRequest1: CommonSilentFlowRequest = { + scopes: ["User.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.BEARER, + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + const expectedTokenRequest1: CommonSilentFlowRequest = { + ...tokenRequest1, + scopes: ["User.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + forceRefresh: false, + }; + const tokenRequest2: CommonSilentFlowRequest = { + scopes: ["Mail.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.BEARER, + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + const expectedTokenRequest2: CommonSilentFlowRequest = { + ...tokenRequest1, + scopes: ["Mail.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + forceRefresh: false, + }; + + // PoP requests + const popTokenRequest1: CommonSilentFlowRequest = { + scopes: ["User.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.POP, + resourceRequestMethod: "GET", + resourceRequestUri: "https://testUri.com/user.read", + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + + const popTokenRequest2: CommonSilentFlowRequest = { + scopes: ["Mail.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.POP, + resourceRequestMethod: "GET", + resourceRequestUri: "https://testUri.com/mail.read", + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + const expectedPopTokenRequest1: CommonSilentFlowRequest = { + ...popTokenRequest1, + scopes: ["User.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + forceRefresh: false, + }; + + const expectedPopTokenRequest2: CommonSilentFlowRequest = { + ...popTokenRequest2, + scopes: ["Mail.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + forceRefresh: false, + }; + + // SSH Certificate requests + const sshCertRequest1: CommonSilentFlowRequest = { + scopes: ["User.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.SSH, + sshJwk: TEST_SSH_VALUES.ENCODED_SSH_JWK, + sshKid: TEST_SSH_VALUES.SSH_KID, + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + + const sshCertRequest2: CommonSilentFlowRequest = { + scopes: ["Mail.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.SSH, + sshJwk: TEST_SSH_VALUES.ALTERNATE_ENCODED_SSH_JWK, + sshKid: TEST_SSH_VALUES.ALTERNATE_SSH_KID, + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + + const expectedSshCertificateRequest1: CommonSilentFlowRequest = { + ...sshCertRequest1, + scopes: ["User.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + forceRefresh: false, + }; + + const expectedSshCertificateRequest2: CommonSilentFlowRequest = { + ...sshCertRequest2, + scopes: ["Mail.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + forceRefresh: false, + }; + + // Requests with claims + const claimsRequest1: CommonSilentFlowRequest = { + scopes: ["User.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.BEARER, + claims: JSON.stringify({ claim1: "claim1" }), + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + + const claimsRequest2: CommonSilentFlowRequest = { + scopes: ["User.Read"], + account: testAccount, + authority: TEST_CONFIG.validAuthority, + authenticationScheme: AuthenticationScheme.BEARER, + claims: JSON.stringify({ claim2: "claim2" }), + requestedClaimsHash: TEST_CRYPTO_VALUES.TEST_SHA256_HASH, + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + }; + + const expectedClaimsRequest1: CommonSilentFlowRequest = { + ...claimsRequest1, + scopes: ["User.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + claims: JSON.stringify({ claim1: "claim1" }), + requestedClaimsHash: TEST_CRYPTO_VALUES.TEST_SHA256_HASH, + forceRefresh: false, + }; + + const expectedClaimsRequest2: CommonSilentFlowRequest = { + ...claimsRequest2, + scopes: ["User.Read"], + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: RANDOM_TEST_GUID, + claims: JSON.stringify({ claim2: "claim2" }), + requestedClaimsHash: TEST_CRYPTO_VALUES.TEST_SHA256_HASH, + forceRefresh: false, + }; + + const silentRequest1 = pca.acquireTokenSilent(tokenRequest1); + const silentRequest2 = pca.acquireTokenSilent(tokenRequest1); + const silentRequest3 = pca.acquireTokenSilent(tokenRequest2); + const popSilentRequest1 = pca.acquireTokenSilent(popTokenRequest1); + const popSilentRequest2 = pca.acquireTokenSilent(popTokenRequest1); + const popSilentRequest3 = pca.acquireTokenSilent(popTokenRequest2); + const sshCertSilentRequest1 = + pca.acquireTokenSilent(sshCertRequest1); + const sshCertSilentRequest2 = + pca.acquireTokenSilent(sshCertRequest1); + const sshCertSilentRequest3 = + pca.acquireTokenSilent(sshCertRequest2); + const claimsSilentRequest1 = pca.acquireTokenSilent(claimsRequest1); + const claimsSilentRequest2 = pca.acquireTokenSilent(claimsRequest1); + const claimsSilentRequest3 = pca.acquireTokenSilent(claimsRequest2); + await Promise.all([ + silentRequest1, + silentRequest2, + silentRequest3, + popSilentRequest1, + popSilentRequest2, + popSilentRequest3, + sshCertSilentRequest1, + sshCertSilentRequest2, + sshCertSilentRequest3, + claimsSilentRequest1, + claimsSilentRequest2, + claimsSilentRequest3, + ]); + + expect(silentATStub.calledWith(expectedTokenRequest1)).toBeTruthy(); + expect(silentATStub.calledWith(expectedTokenRequest2)).toBeTruthy(); + expect( + silentATStub.calledWith(expectedPopTokenRequest1) + ).toBeTruthy(); + expect( + silentATStub.calledWith(expectedPopTokenRequest2) + ).toBeTruthy(); + expect( + silentATStub.calledWith(expectedSshCertificateRequest1) + ).toBeTruthy(); + expect( + silentATStub.calledWith(expectedSshCertificateRequest2) + ).toBeTruthy(); + expect( + silentATStub.calledWith(expectedClaimsRequest1) + ).toBeTruthy(); + expect( + silentATStub.calledWith(expectedClaimsRequest2) + ).toBeTruthy(); + expect(silentATStub.callCount).toEqual(8); + }); + it("throws error that SilentFlowClient.acquireToken() throws", async () => { const testError: AuthError = new AuthError( "create_login_url_error", diff --git a/lib/msal-browser/test/config/Configuration.spec.ts b/lib/msal-browser/test/config/Configuration.spec.ts index 0e76ed51be..343a62b233 100644 --- a/lib/msal-browser/test/config/Configuration.spec.ts +++ b/lib/msal-browser/test/config/Configuration.spec.ts @@ -12,8 +12,6 @@ import { ProtocolMode, ServerResponseType, Logger, - createClientConfigurationError, - ClientConfigurationErrorCodes, } from "@azure/msal-common"; import sinon from "sinon"; import { BrowserCacheLocation } from "../../src/utils/BrowserConstants"; @@ -249,7 +247,7 @@ describe("Configuration.ts Class Unit Tests", () => { cacheLocation: BrowserCacheLocation.LocalStorage, storeAuthStateInCookie: true, secureCookies: true, - claimsBasedCachingEnabled: false, + claimsBasedCachingEnabled: true, }, system: { windowHashTimeout: TEST_POPUP_TIMEOUT_MS, @@ -281,7 +279,7 @@ describe("Configuration.ts Class Unit Tests", () => { expect(newConfig.cache?.storeAuthStateInCookie).not.toBeNull(); expect(newConfig.cache?.storeAuthStateInCookie).toBe(true); expect(newConfig.cache?.secureCookies).toBe(true); - expect(newConfig.cache?.claimsBasedCachingEnabled).toBe(false); + expect(newConfig.cache?.claimsBasedCachingEnabled).toBe(true); // System config checks expect(newConfig.system).not.toBeNull(); expect(newConfig.system?.windowHashTimeout).not.toBeNull(); @@ -313,21 +311,4 @@ describe("Configuration.ts Class Unit Tests", () => { ); expect(loggerSpy).toBeCalled(); }); - - it("throws an error when claimsBasedCaching is enabled", () => { - expect(() => { - buildConfiguration( - { - // @ts-ignore - auth: null, - cache: { claimsBasedCachingEnabled: true }, - }, - true - ); - }).toThrow( - createClientConfigurationError( - ClientConfigurationErrorCodes.claimsBasedCachingEnabled - ) - ); - }); }); diff --git a/lib/msal-common/src/config/ClientConfiguration.ts b/lib/msal-common/src/config/ClientConfiguration.ts index a5a27ed582..e400d39543 100644 --- a/lib/msal-common/src/config/ClientConfiguration.ts +++ b/lib/msal-common/src/config/ClientConfiguration.ts @@ -23,10 +23,6 @@ import { ClientAuthErrorCodes, createClientAuthError, } from "../error/ClientAuthError"; -import { - ClientConfigurationErrorCodes, - createClientConfigurationError, -} from "../error/ClientConfigurationError"; /** * Use the configuration object to configure MSAL Modules and initialize the base interfaces for MSAL. @@ -237,12 +233,6 @@ export function buildClientConfiguration({ persistencePlugin: persistencePlugin, serializableCache: serializableCache, }: ClientConfiguration): CommonClientConfiguration { - if (userCacheOptions?.claimsBasedCachingEnabled) { - throw createClientConfigurationError( - ClientConfigurationErrorCodes.claimsBasedCachingEnabled - ); - } - const loggerOptions = { ...DEFAULT_LOGGER_IMPLEMENTATION, ...userLoggerOption, diff --git a/lib/msal-common/src/error/ClientConfigurationError.ts b/lib/msal-common/src/error/ClientConfigurationError.ts index 2ea8806701..838ee19737 100644 --- a/lib/msal-common/src/error/ClientConfigurationError.ts +++ b/lib/msal-common/src/error/ClientConfigurationError.ts @@ -51,8 +51,6 @@ export const ClientConfigurationErrorMessages = { "Cannot set allowNativeBroker parameter to true when not in AAD protocol mode.", [ClientConfigurationErrorCodes.authorityMismatch]: "Authority mismatch error. Authority provided in login request or PublicClientApplication config does not match the environment of the provided account. Please use a matching account or make an interactive request to login to this authority.", - [ClientConfigurationErrorCodes.claimsBasedCachingEnabled]: - "Claims based caching is not supported in MSALJS.", }; /** diff --git a/lib/msal-common/src/error/ClientConfigurationErrorCodes.ts b/lib/msal-common/src/error/ClientConfigurationErrorCodes.ts index 5895d4c58d..c60c03a95a 100644 --- a/lib/msal-common/src/error/ClientConfigurationErrorCodes.ts +++ b/lib/msal-common/src/error/ClientConfigurationErrorCodes.ts @@ -26,4 +26,3 @@ export const invalidAuthenticationHeader = "invalid_authentication_header"; export const cannotSetOIDCOptions = "cannot_set_OIDCOptions"; export const cannotAllowNativeBroker = "cannot_allow_native_broker"; export const authorityMismatch = "authority_mismatch"; -export const claimsBasedCachingEnabled = "claims_based_caching_enabled"; diff --git a/lib/msal-common/test/client/SilentFlowClient.spec.ts b/lib/msal-common/test/client/SilentFlowClient.spec.ts index 5f155d3255..a41fb8e86f 100644 --- a/lib/msal-common/test/client/SilentFlowClient.spec.ts +++ b/lib/msal-common/test/client/SilentFlowClient.spec.ts @@ -366,6 +366,71 @@ describe("SilentFlowClient unit tests", () => { ); }); + it("acquireCachedToken does not throw when given valid claims with claimsBasedCachingEnabled", async () => { + const testScopes = [ + Constants.OPENID_SCOPE, + Constants.PROFILE_SCOPE, + ...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + ]; + testAccessTokenEntity.target = testScopes.join(" "); + sinon + .stub( + Authority.prototype, + "getEndpointMetadataFromNetwork" + ) + .resolves(DEFAULT_OPENID_CONFIG_RESPONSE.body); + sinon + .stub(CacheManager.prototype, "readAccountFromCache") + .returns(testAccountEntity); + sinon + .stub(CacheManager.prototype, "getIdToken") + .returns(testIdToken); + sinon + .stub(CacheManager.prototype, "getAccessToken") + .returns(testAccessTokenEntity); + sinon + .stub(CacheManager.prototype, "getRefreshToken") + .returns(testRefreshTokenEntity); + const config = + await ClientTestUtils.createTestClientConfiguration(); + const client = new SilentFlowClient( + { + ...config, + cacheOptions: { + ...config.cacheOptions, + claimsBasedCachingEnabled: true, + }, + }, + stubPerformanceClient + ); + sinon.stub(TimeUtils, "isTokenExpired").returns(false); + + const silentFlowRequest: CommonSilentFlowRequest = { + scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE, + account: testAccount, + authority: TEST_CONFIG.validAuthority, + correlationId: TEST_CONFIG.CORRELATION_ID, + forceRefresh: false, + claims: `{ "access_token": { "xms_cc":{"values":["cp1"] } }}`, + }; + + const response = await client.acquireCachedToken(silentFlowRequest); + const authResult: AuthenticationResult = response[0]; + expect(authResult.authority).toEqual( + `${TEST_URIS.DEFAULT_INSTANCE}${TEST_CONFIG.TENANT}/` + ); + expect(authResult.uniqueId).toEqual(ID_TOKEN_CLAIMS.oid); + expect(authResult.tenantId).toEqual(ID_TOKEN_CLAIMS.tid); + expect(authResult.scopes).toEqual(testScopes); + expect(authResult.account).toEqual(testAccount); + expect(authResult.idToken).toEqual(testIdToken.secret); + expect(authResult.idTokenClaims).toEqual(ID_TOKEN_CLAIMS); + expect(authResult.accessToken).toEqual( + testAccessTokenEntity.secret + ); + expect(authResult.state).toBe(""); + }); + it("acquireCachedToken returns correct token when max age is provided and has not transpired yet", async () => { const testScopes = [ Constants.OPENID_SCOPE, diff --git a/lib/msal-common/test/config/ClientConfiguration.spec.ts b/lib/msal-common/test/config/ClientConfiguration.spec.ts index 544ab0a539..c7cb0ac533 100644 --- a/lib/msal-common/test/config/ClientConfiguration.spec.ts +++ b/lib/msal-common/test/config/ClientConfiguration.spec.ts @@ -14,12 +14,7 @@ import { import { MockStorageClass, mockCrypto } from "../client/ClientTestUtils"; import { MockCache } from "../cache/entities/cacheConstants"; import { Constants } from "../../src/utils/Constants"; -import { - ClientAuthErrorCodes, - ClientConfigurationErrorCodes, - createClientAuthError, - createClientConfigurationError, -} from "../../src"; +import { ClientAuthErrorCodes, createClientAuthError } from "../../src"; describe("ClientConfiguration.ts Class Unit Tests", () => { it("buildConfiguration assigns default functions", async () => { @@ -36,21 +31,21 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { expect(emptyConfig.cryptoInterface.base64Decode).not.toBeNull(); expect(() => emptyConfig.cryptoInterface.base64Decode("test input") - ).toThrow( + ).toThrowError( createClientAuthError(ClientAuthErrorCodes.methodNotImplemented) ); expect(() => emptyConfig.cryptoInterface.base64Decode("test input") - ).toThrow(AuthError); + ).toThrowError(AuthError); expect(emptyConfig.cryptoInterface.base64Encode).not.toBeNull(); expect(() => emptyConfig.cryptoInterface.base64Encode("test input") - ).toThrow( + ).toThrowError( createClientAuthError(ClientAuthErrorCodes.methodNotImplemented) ); expect(() => emptyConfig.cryptoInterface.base64Encode("test input") - ).toThrow(AuthError); + ).toThrowError(AuthError); // Storage interface checks expect(emptyConfig.storageInterface).not.toBeNull(); expect(emptyConfig.storageInterface.clear).not.toBeNull(); @@ -62,35 +57,37 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { expect(emptyConfig.storageInterface.getAccount).not.toBeNull(); expect(() => emptyConfig.storageInterface.getAccount("testKey") - ).toThrow( + ).toThrowError( createClientAuthError(ClientAuthErrorCodes.methodNotImplemented) ); expect(() => emptyConfig.storageInterface.getAccount("testKey") - ).toThrow(AuthError); + ).toThrowError(AuthError); expect(emptyConfig.storageInterface.getKeys).not.toBeNull(); - expect(() => emptyConfig.storageInterface.getKeys()).toThrow( + expect(() => emptyConfig.storageInterface.getKeys()).toThrowError( createClientAuthError(ClientAuthErrorCodes.methodNotImplemented) ); - expect(() => emptyConfig.storageInterface.getKeys()).toThrow(AuthError); + expect(() => emptyConfig.storageInterface.getKeys()).toThrowError( + AuthError + ); expect(emptyConfig.storageInterface.removeItem).not.toBeNull(); expect(() => emptyConfig.storageInterface.removeItem("testKey") - ).toThrow( + ).toThrowError( createClientAuthError(ClientAuthErrorCodes.methodNotImplemented) ); expect(() => emptyConfig.storageInterface.removeItem("testKey") - ).toThrow(AuthError); + ).toThrowError(AuthError); expect(emptyConfig.storageInterface.setAccount).not.toBeNull(); expect(() => emptyConfig.storageInterface.setAccount(MockCache.acc) - ).toThrow( + ).toThrowError( createClientAuthError(ClientAuthErrorCodes.methodNotImplemented) ); expect(() => emptyConfig.storageInterface.setAccount(MockCache.acc) - ).toThrow(AuthError); + ).toThrowError(AuthError); // Network interface checks expect(emptyConfig.networkInterface).not.toBeNull(); expect(emptyConfig.networkInterface.sendGetRequestAsync).not.toBeNull(); @@ -194,6 +191,9 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { ): void => {}, piiLoggingEnabled: true, }, + cacheOptions: { + claimsBasedCachingEnabled: true, + }, libraryInfo: { sku: TEST_CONFIG.TEST_SKU, version: TEST_CONFIG.TEST_VERSION, @@ -265,7 +265,7 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { expect(newConfig.loggerOptions.piiLoggingEnabled).toBe(true); // Cache options tests expect(newConfig.cacheOptions).not.toBeNull(); - expect(newConfig.cacheOptions.claimsBasedCachingEnabled).toBe(false); + expect(newConfig.cacheOptions.claimsBasedCachingEnabled).toBe(true); // Client info tests expect(newConfig.libraryInfo.sku).toBe(TEST_CONFIG.TEST_SKU); expect(newConfig.libraryInfo.version).toBe(TEST_CONFIG.TEST_VERSION); @@ -279,20 +279,4 @@ describe("ClientConfiguration.ts Class Unit Tests", () => { TEST_CONFIG.TEST_APP_VER ); }); - - test("throws an error when claimsBasedCaching is enabled", async () => { - expect(() => { - buildClientConfiguration({ - //@ts-ignore - authOptions: { - clientId: TEST_CONFIG.MSAL_CLIENT_ID, - }, - cacheOptions: { claimsBasedCachingEnabled: true }, - }); - }).toThrow( - createClientConfigurationError( - ClientConfigurationErrorCodes.claimsBasedCachingEnabled - ) - ); - }); }); diff --git a/lib/msal-node/src/config/Configuration.ts b/lib/msal-node/src/config/Configuration.ts index fdb8a979d9..ba57eb4118 100644 --- a/lib/msal-node/src/config/Configuration.ts +++ b/lib/msal-node/src/config/Configuration.ts @@ -14,8 +14,6 @@ import { AzureCloudOptions, ApplicationTelemetry, INativeBrokerPlugin, - createClientConfigurationError, - ClientConfigurationErrorCodes, } from "@azure/msal-common"; import { HttpClient } from "../network/HttpClient.js"; import http from "http"; @@ -67,6 +65,9 @@ export type NodeAuthOptions = { */ export type CacheOptions = { cachePlugin?: ICachePlugin; + /** + * @deprecated claims-based-caching functionality will be removed in the next version of MSALJS + */ claimsBasedCachingEnabled?: boolean; }; @@ -205,12 +206,6 @@ export function buildAppConfiguration({ system, telemetry, }: Configuration): NodeConfiguration { - if (cache?.claimsBasedCachingEnabled) { - throw createClientConfigurationError( - ClientConfigurationErrorCodes.claimsBasedCachingEnabled - ); - } - const systemOptions: Required = { ...DEFAULT_SYSTEM_OPTIONS, networkClient: new HttpClient( diff --git a/lib/msal-node/test/client/ClientCredentialClient.spec.ts b/lib/msal-node/test/client/ClientCredentialClient.spec.ts index f764776542..6bbdc80a7a 100644 --- a/lib/msal-node/test/client/ClientCredentialClient.spec.ts +++ b/lib/msal-node/test/client/ClientCredentialClient.spec.ts @@ -20,8 +20,6 @@ import { createClientAuthError, ClientAuthErrorCodes, CacheHelpers, - createClientConfigurationError, - ClientConfigurationErrorCodes, } from "@azure/msal-common"; import { ClientCredentialClient, UsernamePasswordClient } from "../../src"; import { @@ -502,20 +500,6 @@ describe("ClientCredentialClient unit tests", () => { ); }); - it("An error is thrown when claims based caching is enabled", async () => { - expect(() => { - // will use msal-common's buildAppConfiguration - new ClientCredentialClient({ - ...config, - cacheOptions: { claimsBasedCachingEnabled: true }, - }); - }).toThrow( - createClientConfigurationError( - ClientConfigurationErrorCodes.claimsBasedCachingEnabled - ) - ); - }); - it("Does not add claims when empty object provided", async () => { const client: ClientCredentialClient = new ClientCredentialClient( config diff --git a/lib/msal-node/test/client/PublicClientApplication.spec.ts b/lib/msal-node/test/client/PublicClientApplication.spec.ts index a8287a64c2..b2c7e426b8 100644 --- a/lib/msal-node/test/client/PublicClientApplication.spec.ts +++ b/lib/msal-node/test/client/PublicClientApplication.spec.ts @@ -868,6 +868,46 @@ describe("PublicClientApplication", () => { }); }); + test("initializeBaseRequest passes a requested claims hash to acquireToken when claimsBasedHashing is enabled", async () => { + const account: AccountInfo = { + homeAccountId: "", + environment: "", + tenantId: "", + username: "", + localAccountId: "", + name: "", + idTokenClaims: ID_TOKEN_CLAIMS, + }; + const request: SilentFlowRequest = { + account: account, + scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE, + claims: TEST_CONSTANTS.CLAIMS, + }; + + const silentFlowClient = getMsalCommonAutoMock().SilentFlowClient; + jest.spyOn(msalCommon, "SilentFlowClient").mockImplementation( + (config) => new silentFlowClient(config) + ); + + const acquireTokenSpy = jest.spyOn( + silentFlowClient.prototype, + "acquireToken" + ); + const authApp = new PublicClientApplication({ + ...appConfig, + cache: { claimsBasedCachingEnabled: true }, + }); + await authApp.acquireTokenSilent(request); + expect(silentFlowClient.prototype.acquireToken).toHaveBeenCalledWith( + expect.objectContaining({ requestedClaimsHash: expect.any(String) }) + ); + + const submittedRequest = acquireTokenSpy.mock.calls[0][0]; + expect( + (submittedRequest as any)?.requestedClaimsHash?.length + ).toBeGreaterThan(0); + }); + test("initializeBaseRequest doesn't pass a claims hash to acquireToken when claimsBasedHashing is disabled by default", async () => { const account: AccountInfo = { homeAccountId: "", @@ -1096,7 +1136,7 @@ describe("PublicClientApplication", () => { expect(authApp.getLogger().info("Test logger")).toEqual(undefined); }); - test("throws an error if state is not provided", async () => { + test("should throw an error if state is not provided", async () => { const cryptoProvider = new CryptoProvider(); const request: AuthorizationCodeRequest = { scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE, @@ -1129,11 +1169,12 @@ describe("PublicClientApplication", () => { }); const authApp = new PublicClientApplication(appConfig); + await authApp.acquireTokenByCode(request, authCodePayLoad); try { await authApp.acquireTokenByCode(request, authCodePayLoad); } catch (e) { - expect(mockInfo).toHaveBeenCalledWith("acquireTokenByCode called"); + expect(mockInfo).toBeCalledWith("acquireTokenByCode called"); expect(mockInfo).toHaveBeenCalledWith( "acquireTokenByCode - validating state" ); diff --git a/lib/msal-node/test/config/ClientConfiguration.spec.ts b/lib/msal-node/test/config/ClientConfiguration.spec.ts index 134464ed7e..42722ce5fa 100644 --- a/lib/msal-node/test/config/ClientConfiguration.spec.ts +++ b/lib/msal-node/test/config/ClientConfiguration.spec.ts @@ -12,8 +12,6 @@ import { LogLevel, NetworkRequestOptions, AzureCloudInstance, - createClientConfigurationError, - ClientConfigurationErrorCodes, } from "@azure/msal-common"; import { ClientCredentialRequest, @@ -266,19 +264,4 @@ describe("ClientConfiguration tests", () => { }) ); }); - - test("throws an error when claimsBasedCaching is enabled", async () => { - expect(() => { - buildAppConfiguration({ - auth: { - clientId: TEST_CONSTANTS.CLIENT_ID, - }, - cache: { claimsBasedCachingEnabled: true }, - }); - }).toThrow( - createClientConfigurationError( - ClientConfigurationErrorCodes.claimsBasedCachingEnabled - ) - ); - }); });