Skip to content

Commit

Permalink
ClientCredential and OBO acquireToken requests with claims will now s…
Browse files Browse the repository at this point in the history
…kip the cache (#6999)

ClientCredential and OBO acquireToken requests with claims will now skip
the cache.

claimsBasedCachingEnabled has been marked as deprecated in msal-node.
  • Loading branch information
Robbie-Microsoft authored Apr 30, 2024
1 parent 1c72e3f commit 37633b4
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "ClientCredential and OBO acquireToken requests with claims will now skip the cache",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
2 changes: 1 addition & 1 deletion lib/msal-node/src/client/ClientCredentialClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class ClientCredentialClient extends BaseClient {
public async acquireToken(
request: CommonClientCredentialRequest
): Promise<AuthenticationResult | null> {
if (request.skipCache) {
if (request.skipCache || request.claims) {
return this.executeTokenRequest(request, this.authority);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/msal-node/src/client/OnBehalfOfClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class OnBehalfOfClient extends BaseClient {
request.oboAssertion
);

if (request.skipCache) {
if (request.skipCache || request.claims) {
return this.executeTokenRequest(
request,
this.authority,
Expand Down
3 changes: 3 additions & 0 deletions lib/msal-node/src/config/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,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;
};

Expand Down
54 changes: 52 additions & 2 deletions lib/msal-node/test/client/ClientCredentialClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,14 +420,17 @@ describe("ClientCredentialClient unit tests", () => {
])(
"Validates that claims and client capabilities are correctly merged",
async (claims, mergedClaims) => {
clientCredentialRequest.claims = claims;
// acquire a token with a client that has client capabilities, but no claims in the request
// verify that it comes from the IDP
const authResult = (await client.acquireToken(
clientCredentialRequest
)) as AuthenticationResult;
expect(authResult.accessToken).toEqual(
CONFIDENTIAL_CLIENT_AUTHENTICATION_RESULT.body.access_token
);
expect(authResult.fromCache).toBe(false);

// verify that the client capabilities have been merged with the (empty) claims
const returnVal: string = createTokenRequestBodySpy.mock
.results[0].value as string;
expect(
Expand All @@ -437,15 +440,62 @@ describe("ClientCredentialClient unit tests", () => {
.filter((key: string) => key.includes("claims="))[0]
.split("claims=")[1]
)
).toEqual(mergedClaims);
).toEqual(CAE_CONSTANTS.MERGED_EMPTY_CLAIMS);

// acquire a token (without changing anything) and verify that it comes from the cache
// verify that it comes from the cache
const cachedAuthResult = (await client.acquireToken(
clientCredentialRequest
)) as AuthenticationResult;
expect(cachedAuthResult.accessToken).toEqual(
CONFIDENTIAL_CLIENT_AUTHENTICATION_RESULT.body.access_token
);
expect(cachedAuthResult.fromCache).toBe(true);

// acquire a token with a client that has client capabilities, and has claims in the request
// verify that it comes from the IDP
clientCredentialRequest.claims = claims;
const authResult2 = (await client.acquireToken(
clientCredentialRequest
)) as AuthenticationResult;
expect(authResult2.accessToken).toEqual(
CONFIDENTIAL_CLIENT_AUTHENTICATION_RESULT.body.access_token
);
expect(authResult2.fromCache).toBe(false);

// verify that the client capabilities have been merged with the claims
const returnVal2: string = createTokenRequestBodySpy.mock
.results[1].value as string;
expect(
decodeURIComponent(
returnVal2
.split("&")
.filter((key: string) => key.includes("claims="))[0]
.split("claims=")[1]
)
).toEqual(mergedClaims);

// acquire a token with a client that has client capabilities, but no claims in the request
// verify that it comes from the cache
delete clientCredentialRequest.claims;
const authResult3 = (await client.acquireToken(
clientCredentialRequest
)) as AuthenticationResult;
expect(authResult3.accessToken).toEqual(
CONFIDENTIAL_CLIENT_AUTHENTICATION_RESULT.body.access_token
);
expect(authResult3.fromCache).toBe(true);

// acquire a token with a client that has client capabilities, and has claims in the request
// verify that it comes from the IDP
clientCredentialRequest.claims = claims;
const authResult4 = (await client.acquireToken(
clientCredentialRequest
)) as AuthenticationResult;
expect(authResult4.accessToken).toEqual(
CONFIDENTIAL_CLIENT_AUTHENTICATION_RESULT.body.access_token
);
expect(authResult4.fromCache).toBe(false);
}
);
});
Expand Down
33 changes: 31 additions & 2 deletions lib/msal-node/test/client/ConfidentialClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,18 @@ describe("ConfidentialClientApplication", () => {
])(
"Validates that claims and client capabilities are correctly merged",
async (claims, mergedClaims) => {
authorizationCodeRequest.claims = claims;
// acquire a token with a client that has client capabilities, but no claims in the request
// verify that it comes from the IDP
const authResult = (await client.acquireTokenByCode(
authorizationCodeRequest
)) as AuthenticationResult;
expect(authResult.accessToken).toEqual(
CONFIDENTIAL_CLIENT_AUTHENTICATION_RESULT.body
.access_token
);
expect(authResult.fromCache).toBe(false);

// verify that the client capabilities have been merged with the (empty) claims
const returnVal: string = (await createTokenRequestBodySpy
.mock.results[0].value) as string;
expect(
Expand All @@ -152,9 +155,35 @@ describe("ConfidentialClientApplication", () => {
)[0]
.split("claims=")[1]
)
).toEqual(mergedClaims);
).toEqual(CAE_CONSTANTS.MERGED_EMPTY_CLAIMS);

// skip cache lookup verification because acquireTokenByCode does not pull elements from the cache

// acquire a token with a client that has client capabilities, and has claims in the request
// verify that it comes from the IDP
authorizationCodeRequest.claims = claims;
const authResult2 = (await client.acquireTokenByCode(
authorizationCodeRequest
)) as AuthenticationResult;
expect(authResult2.accessToken).toEqual(
CONFIDENTIAL_CLIENT_AUTHENTICATION_RESULT.body
.access_token
);
expect(authResult2.fromCache).toBe(false);

// verify that the client capabilities have been merged with the claims
const returnVal2: string = (await createTokenRequestBodySpy
.mock.results[1].value) as string;
expect(
decodeURIComponent(
returnVal2
.split("&")
.filter((key: string) =>
key.includes("claims=")
)[0]
.split("claims=")[1]
)
).toEqual(mergedClaims);
}
);
});
Expand Down
56 changes: 54 additions & 2 deletions lib/msal-node/test/client/OnBehalfOfClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,17 @@ describe("OnBehalfOf unit tests", () => {
])(
"Validates that claims and client capabilities are correctly merged",
async (claims, mergedClaims) => {
oboRequest.claims = claims;
// acquire a token with a client that has client capabilities, but no claims in the request
// verify that it comes from the IDP
const authResult = (await client.acquireToken(
oboRequest
)) as AuthenticationResult;
expect(authResult.accessToken).toEqual(
AUTHENTICATION_RESULT.body.access_token
);
expect(authResult.fromCache).toBe(false);

// verify that the client capabilities have been merged with the (empty) claims
const returnVal: string = createTokenRequestBodySpy.mock
.results[0].value as string;
expect(
Expand All @@ -161,15 +164,64 @@ describe("OnBehalfOf unit tests", () => {
)[0]
.split("claims=")[1]
)
).toEqual(mergedClaims);
).toEqual(CAE_CONSTANTS.MERGED_EMPTY_CLAIMS);

// acquire a token (without changing anything) and verify that it comes from the cache
// verify that it comes from the cache
const cachedAuthResult = (await client.acquireToken(
oboRequest
)) as AuthenticationResult;
expect(cachedAuthResult.accessToken).toEqual(
AUTHENTICATION_RESULT.body.access_token
);
expect(cachedAuthResult.fromCache).toBe(true);

// acquire a token with a client that has client capabilities, and has claims in the request
// verify that it comes from the IDP
oboRequest.claims = claims;
const authResult2 = (await client.acquireToken(
oboRequest
)) as AuthenticationResult;
expect(authResult2.accessToken).toEqual(
AUTHENTICATION_RESULT.body.access_token
);
expect(authResult2.fromCache).toBe(false);

// verify that the client capabilities have been merged with the claims
const returnVal2: string = createTokenRequestBodySpy.mock
.results[1].value as string;
expect(
decodeURIComponent(
returnVal2
.split("&")
.filter((key: string) =>
key.includes("claims=")
)[0]
.split("claims=")[1]
)
).toEqual(mergedClaims);

// acquire a token with a client that has client capabilities, but no claims in the request
// verify that it comes from the cache
delete oboRequest.claims;
const authResult3 = (await client.acquireToken(
oboRequest
)) as AuthenticationResult;
expect(authResult3.accessToken).toEqual(
AUTHENTICATION_RESULT.body.access_token
);
expect(authResult3.fromCache).toBe(true);

// acquire a token with a client that has client capabilities, and has claims in the request
// verify that it comes from the IDP
oboRequest.claims = claims;
const authResult4 = (await client.acquireToken(
oboRequest
)) as AuthenticationResult;
expect(authResult4.accessToken).toEqual(
AUTHENTICATION_RESULT.body.access_token
);
expect(authResult4.fromCache).toBe(false);
}
);
});
Expand Down

0 comments on commit 37633b4

Please sign in to comment.