From 621db00de7b5b233c5dedaa4265e0278a8f1853e Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 15 Nov 2024 17:18:56 -0500 Subject: [PATCH 1/7] added token revocation funcitonality to managed identity --- .../src/client/ManagedIdentityApplication.ts | 5 ++- .../src/client/ManagedIdentityClient.ts | 6 ++- .../BaseManagedIdentitySource.ts | 14 +++++- lib/msal-node/src/config/Configuration.ts | 4 ++ .../ManagedIdentitySources/Imds.spec.ts | 44 ++++++++++++++++++- 5 files changed, 68 insertions(+), 5 deletions(-) diff --git a/lib/msal-node/src/client/ManagedIdentityApplication.ts b/lib/msal-node/src/client/ManagedIdentityApplication.ts index 9b2353a427..b2a673ae1b 100644 --- a/lib/msal-node/src/client/ManagedIdentityApplication.ts +++ b/lib/msal-node/src/client/ManagedIdentityApplication.ts @@ -140,6 +140,7 @@ export class ManagedIdentityApplication { ], authority: this.fakeAuthority.canonicalAuthority, correlationId: this.cryptoProvider.createNewGuid(), + claims: managedIdentityRequestParams.claims, }; if ( @@ -150,7 +151,9 @@ export class ManagedIdentityApplication { return this.managedIdentityClient.sendManagedIdentityTokenRequest( managedIdentityRequest, this.config.managedIdentityId, - this.fakeAuthority + this.fakeAuthority, + undefined, + this.config.clientCapabilities ); } diff --git a/lib/msal-node/src/client/ManagedIdentityClient.ts b/lib/msal-node/src/client/ManagedIdentityClient.ts index 555f116992..9890ef66da 100644 --- a/lib/msal-node/src/client/ManagedIdentityClient.ts +++ b/lib/msal-node/src/client/ManagedIdentityClient.ts @@ -54,7 +54,8 @@ export class ManagedIdentityClient { managedIdentityRequest: ManagedIdentityRequest, managedIdentityId: ManagedIdentityId, fakeAuthority: Authority, - refreshAccessToken?: boolean + refreshAccessToken?: boolean, + clientCapabilities?: Array ): Promise { if (!ManagedIdentityClient.identitySource) { ManagedIdentityClient.identitySource = @@ -71,7 +72,8 @@ export class ManagedIdentityClient { managedIdentityRequest, managedIdentityId, fakeAuthority, - refreshAccessToken + refreshAccessToken, + clientCapabilities ); } diff --git a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts index 39d7a3e968..3bb424c6a1 100644 --- a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts +++ b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts @@ -125,7 +125,8 @@ export abstract class BaseManagedIdentitySource { managedIdentityRequest: ManagedIdentityRequest, managedIdentityId: ManagedIdentityId, fakeAuthority: Authority, - refreshAccessToken?: boolean + refreshAccessToken?: boolean, + clientCapabilities?: Array ): Promise { const networkRequest: ManagedIdentityRequestParameters = this.createRequest( @@ -133,6 +134,17 @@ export abstract class BaseManagedIdentitySource { managedIdentityId ); + // if claims are present, token proxies will clear their token cache and request a new token from ESTS + if (managedIdentityRequest.claims) { + networkRequest.queryParameters.bypass_cache = "true"; + } + + // an optional non-empty string query parameter used to send client capabilities to ESTS + if (clientCapabilities) { + networkRequest.queryParameters.xms_cc = + clientCapabilities.toString(); + } + const headers: Record = networkRequest.headers; headers[HeaderNames.CONTENT_TYPE] = Constants.URL_FORM_CONTENT_TYPE; diff --git a/lib/msal-node/src/config/Configuration.ts b/lib/msal-node/src/config/Configuration.ts index fbde1def2b..73bb3d5f64 100644 --- a/lib/msal-node/src/config/Configuration.ts +++ b/lib/msal-node/src/config/Configuration.ts @@ -136,6 +136,7 @@ export type ManagedIdentityIdParams = { /** @public */ export type ManagedIdentityConfiguration = { + clientCapabilities?: Array; managedIdentityIdParams?: ManagedIdentityIdParams; system?: NodeSystemOptions; }; @@ -247,6 +248,7 @@ export function buildAppConfiguration({ /** @internal */ export type ManagedIdentityNodeConfiguration = { + clientCapabilities: Array; managedIdentityId: ManagedIdentityId; system: Required< Pick @@ -254,6 +256,7 @@ export type ManagedIdentityNodeConfiguration = { }; export function buildManagedIdentityConfiguration({ + clientCapabilities, managedIdentityIdParams, system, }: ManagedIdentityConfiguration): ManagedIdentityNodeConfiguration { @@ -290,6 +293,7 @@ export function buildManagedIdentityConfiguration({ } return { + clientCapabilities: clientCapabilities || [], managedIdentityId: managedIdentityId, system: { loggerOptions, diff --git a/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts b/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts index 2e4318e3eb..edb6569aa4 100644 --- a/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts +++ b/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts @@ -55,6 +55,7 @@ import { } from "../../../src"; // NodeJS 16+ provides a built-in version of setTimeout that is promise-based import { setTimeout } from "timers/promises"; +import { CAE_CONSTANTS } from "../../test_kit/StringConstants.js"; describe("Acquires a token successfully via an IMDS Managed Identity", () => { // IMDS doesn't need environment variables because there is a default IMDS endpoint @@ -549,7 +550,44 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { ); }); - test("ignores a cached token when claims are provided", async () => { + test('ensures "xms=client-capabilites-string" was sent to ESTS', async () => { + const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( + networkClient, + "sendGetRequestAsync" + ); + + const systemAssignedManagedIdentityApplicationWithClientCapabilities: ManagedIdentityApplication = + new ManagedIdentityApplication({ + ...systemAssignedConfig, + clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, + }); + + const networkManagedIdentityResult: AuthenticationResult = + await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( + { + claims: TEST_CONFIG.CLAIMS, + resource: MANAGED_IDENTITY_RESOURCE, + } + ); + expect(networkManagedIdentityResult.accessToken).toEqual( + DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken + ); + + const tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; + const url = tokenRequest[0]; + expect( + url.includes( + `xms_cc=${CAE_CONSTANTS.CLIENT_CAPABILITIES.toString()}` + ) + ).toBe(true); + }); + + test('ignores a cached token when claims are provided, and ensures "bypass_cache=true" was sent to ESTS', async () => { + const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( + networkClient, + "sendGetRequestAsync" + ); + let networkManagedIdentityResult: AuthenticationResult = await systemAssignedManagedIdentityApplication.acquireToken({ resource: MANAGED_IDENTITY_RESOURCE, @@ -578,6 +616,10 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { expect(networkManagedIdentityResult.accessToken).toEqual( DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken ); + + const tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; + const url = tokenRequest[0]; + expect(url.includes("bypass_cache=true")).toBe(true); }); test("ignores a cached token when forceRefresh is set to true", async () => { From eb8715684e91785e80a304f25e391c1d9391f4e1 Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 15 Nov 2024 17:26:31 -0500 Subject: [PATCH 2/7] Change files --- ...ure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json diff --git a/change/@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json b/change/@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json new file mode 100644 index 0000000000..221a877da3 --- /dev/null +++ b/change/@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "added token revocation funcitonality to managed identity", + "packageName": "@azure/msal-node", + "email": "rginsburg@microsoft.com", + "dependentChangeType": "patch" +} From 122d4a2f262d65e0b498abffc780aeb70bb2aaf7 Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 15 Nov 2024 17:27:14 -0500 Subject: [PATCH 3/7] beachball + msal-node.api.md --- .../@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json | 2 +- lib/msal-node/apiReview/msal-node.api.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/change/@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json b/change/@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json index 221a877da3..905a07020a 100644 --- a/change/@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json +++ b/change/@azure-msal-node-20888f8d-8ed5-4418-80dd-03944efa17c7.json @@ -1,6 +1,6 @@ { "type": "minor", - "comment": "added token revocation funcitonality to managed identity", + "comment": "Added token revocation functionality to managed identity (#7422)", "packageName": "@azure/msal-node", "email": "rginsburg@microsoft.com", "dependentChangeType": "patch" diff --git a/lib/msal-node/apiReview/msal-node.api.md b/lib/msal-node/apiReview/msal-node.api.md index 1a20e8c01e..02456267e7 100644 --- a/lib/msal-node/apiReview/msal-node.api.md +++ b/lib/msal-node/apiReview/msal-node.api.md @@ -389,6 +389,7 @@ export class ManagedIdentityApplication { // @public (undocumented) export type ManagedIdentityConfiguration = { + clientCapabilities?: Array; managedIdentityIdParams?: ManagedIdentityIdParams; system?: NodeSystemOptions; }; From 93f7c61f646dd0795f161540255a7bc994507063 Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 15 Nov 2024 17:56:05 -0500 Subject: [PATCH 4/7] Improved code + unit tests --- .../src/client/ManagedIdentityApplication.ts | 7 +- .../src/client/ManagedIdentityClient.ts | 6 +- .../BaseManagedIdentitySource.ts | 11 ++- .../request/ManagedIdentityRequestParams.ts | 8 +- .../ManagedIdentitySources/Imds.spec.ts | 73 ++++++++++++++++++- 5 files changed, 86 insertions(+), 19 deletions(-) diff --git a/lib/msal-node/src/client/ManagedIdentityApplication.ts b/lib/msal-node/src/client/ManagedIdentityApplication.ts index b2a673ae1b..d072476def 100644 --- a/lib/msal-node/src/client/ManagedIdentityApplication.ts +++ b/lib/msal-node/src/client/ManagedIdentityApplication.ts @@ -141,6 +141,9 @@ export class ManagedIdentityApplication { authority: this.fakeAuthority.canonicalAuthority, correlationId: this.cryptoProvider.createNewGuid(), claims: managedIdentityRequestParams.claims, + clientCapabilities: + managedIdentityRequestParams.clientCapabilities || + this.config.clientCapabilities, }; if ( @@ -151,9 +154,7 @@ export class ManagedIdentityApplication { return this.managedIdentityClient.sendManagedIdentityTokenRequest( managedIdentityRequest, this.config.managedIdentityId, - this.fakeAuthority, - undefined, - this.config.clientCapabilities + this.fakeAuthority ); } diff --git a/lib/msal-node/src/client/ManagedIdentityClient.ts b/lib/msal-node/src/client/ManagedIdentityClient.ts index 9890ef66da..555f116992 100644 --- a/lib/msal-node/src/client/ManagedIdentityClient.ts +++ b/lib/msal-node/src/client/ManagedIdentityClient.ts @@ -54,8 +54,7 @@ export class ManagedIdentityClient { managedIdentityRequest: ManagedIdentityRequest, managedIdentityId: ManagedIdentityId, fakeAuthority: Authority, - refreshAccessToken?: boolean, - clientCapabilities?: Array + refreshAccessToken?: boolean ): Promise { if (!ManagedIdentityClient.identitySource) { ManagedIdentityClient.identitySource = @@ -72,8 +71,7 @@ export class ManagedIdentityClient { managedIdentityRequest, managedIdentityId, fakeAuthority, - refreshAccessToken, - clientCapabilities + refreshAccessToken ); } diff --git a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts index 3bb424c6a1..4b4b119eb0 100644 --- a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts +++ b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts @@ -125,8 +125,7 @@ export abstract class BaseManagedIdentitySource { managedIdentityRequest: ManagedIdentityRequest, managedIdentityId: ManagedIdentityId, fakeAuthority: Authority, - refreshAccessToken?: boolean, - clientCapabilities?: Array + refreshAccessToken?: boolean ): Promise { const networkRequest: ManagedIdentityRequestParameters = this.createRequest( @@ -134,15 +133,15 @@ export abstract class BaseManagedIdentitySource { managedIdentityId ); - // if claims are present, token proxies will clear their token cache and request a new token from ESTS + // if claims are present, ESTS will get a new token if (managedIdentityRequest.claims) { networkRequest.queryParameters.bypass_cache = "true"; } - // an optional non-empty string query parameter used to send client capabilities to ESTS - if (clientCapabilities) { + // if client capabilities are present, send them to ESTS + if (managedIdentityRequest.clientCapabilities) { networkRequest.queryParameters.xms_cc = - clientCapabilities.toString(); + managedIdentityRequest.clientCapabilities.toString(); } const headers: Record = networkRequest.headers; diff --git a/lib/msal-node/src/request/ManagedIdentityRequestParams.ts b/lib/msal-node/src/request/ManagedIdentityRequestParams.ts index 9ba9ef4fcd..0ba1a3c8dc 100644 --- a/lib/msal-node/src/request/ManagedIdentityRequestParams.ts +++ b/lib/msal-node/src/request/ManagedIdentityRequestParams.ts @@ -5,13 +5,15 @@ /** * ManagedIdentityRequest - * - claims - a stringified claims request which will be used to determine whether or not the cache should be skipped - * - forceRefresh - forces managed identity requests to skip the cache and make network calls if true - * - resource - resource requested to access the protected API. It should be of the form "ResourceIdUri" or "ResourceIdUri/.default". For instance https://management.azure.net or, for Microsoft Graph, https://graph.microsoft.com/.default + * - claims - a stringified claims request which will be used to determine whether or not the cache should be skipped + * - clientCapabilities - an array of client capabilities to be sent to ESTS + * - forceRefresh - forces managed identity requests to skip the cache and make network calls if true + * - resource - resource requested to access the protected API. It should be of the form "ResourceIdUri" or "ResourceIdUri/.default". For instance https://management.azure.net or, for Microsoft Graph, https://graph.microsoft.com/.default * @public */ export type ManagedIdentityRequestParams = { claims?: string; + clientCapabilities?: Array; forceRefresh?: boolean; resource: string; }; diff --git a/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts b/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts index edb6569aa4..681edc5b03 100644 --- a/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts +++ b/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts @@ -550,7 +550,7 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { ); }); - test('ensures "xms=client-capabilites-string" was sent to ESTS', async () => { + test('ignores a cached token when claims are provided, ensures "bypass_cache=true" was sent to ESTS, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided', async () => { const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( networkClient, "sendGetRequestAsync" @@ -562,19 +562,87 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, }); - const networkManagedIdentityResult: AuthenticationResult = + let networkManagedIdentityResult: AuthenticationResult = + await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( + { + resource: MANAGED_IDENTITY_RESOURCE, + } + ); + expect(networkManagedIdentityResult.fromCache).toBe(false); + expect(networkManagedIdentityResult.accessToken).toEqual( + DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken + ); + + const cachedManagedIdentityResult: AuthenticationResult = + await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( + { + resource: MANAGED_IDENTITY_RESOURCE, + } + ); + expect(cachedManagedIdentityResult.fromCache).toBe(true); + expect(cachedManagedIdentityResult.accessToken).toEqual( + DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken + ); + + networkManagedIdentityResult = await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( { claims: TEST_CONFIG.CLAIMS, resource: MANAGED_IDENTITY_RESOURCE, } ); + expect(networkManagedIdentityResult.fromCache).toBe(false); expect(networkManagedIdentityResult.accessToken).toEqual( DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken ); const tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; const url = tokenRequest[0]; + expect(url.includes("bypass_cache=true")).toBe(true); + expect( + url.includes( + `xms_cc=${CAE_CONSTANTS.CLIENT_CAPABILITIES.toString()}` + ) + ).toBe(true); + }); + + test('does not ignore a cached token when claims are not provided, ensures "bypass_cache=true" was not sent to ESTS, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided', async () => { + const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( + networkClient, + "sendGetRequestAsync" + ); + + const systemAssignedManagedIdentityApplicationWithClientCapabilities: ManagedIdentityApplication = + new ManagedIdentityApplication({ + ...systemAssignedConfig, + clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, + }); + + let networkManagedIdentityResult: AuthenticationResult = + await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( + { + resource: MANAGED_IDENTITY_RESOURCE, + } + ); + expect(networkManagedIdentityResult.fromCache).toBe(false); + expect(networkManagedIdentityResult.accessToken).toEqual( + DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken + ); + + const cachedManagedIdentityResult: AuthenticationResult = + await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( + { + resource: MANAGED_IDENTITY_RESOURCE, + } + ); + expect(cachedManagedIdentityResult.fromCache).toBe(true); + expect(cachedManagedIdentityResult.accessToken).toEqual( + DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken + ); + + const tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; + const url = tokenRequest[0]; + expect(url.includes("bypass_cache=true")).toBe(false); expect( url.includes( `xms_cc=${CAE_CONSTANTS.CLIENT_CAPABILITIES.toString()}` @@ -593,7 +661,6 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { resource: MANAGED_IDENTITY_RESOURCE, }); expect(networkManagedIdentityResult.fromCache).toBe(false); - expect(networkManagedIdentityResult.accessToken).toEqual( DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken ); From b41584e1c399ac7914839f926f63a4d9abc088f1 Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 15 Nov 2024 17:58:53 -0500 Subject: [PATCH 5/7] small improvement --- .../client/ManagedIdentitySources/BaseManagedIdentitySource.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts index 4b4b119eb0..b0f9de2bc1 100644 --- a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts +++ b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts @@ -139,7 +139,7 @@ export abstract class BaseManagedIdentitySource { } // if client capabilities are present, send them to ESTS - if (managedIdentityRequest.clientCapabilities) { + if (managedIdentityRequest.clientCapabilities.length) { networkRequest.queryParameters.xms_cc = managedIdentityRequest.clientCapabilities.toString(); } From 53b01bbc86d3874c0a1f8fb65468ad278313e28e Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 15 Nov 2024 18:30:02 -0500 Subject: [PATCH 6/7] Improved unit tests --- .../BaseManagedIdentitySource.ts | 2 +- .../ManagedIdentitySources/Imds.spec.ts | 77 ++++++++++++------- 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts index b0f9de2bc1..cf3e5487d5 100644 --- a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts +++ b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts @@ -139,7 +139,7 @@ export abstract class BaseManagedIdentitySource { } // if client capabilities are present, send them to ESTS - if (managedIdentityRequest.clientCapabilities.length) { + if (managedIdentityRequest.clientCapabilities?.length) { networkRequest.queryParameters.xms_cc = managedIdentityRequest.clientCapabilities.toString(); } diff --git a/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts b/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts index 681edc5b03..71fdb69def 100644 --- a/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts +++ b/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts @@ -550,54 +550,52 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { ); }); - test('ignores a cached token when claims are provided, ensures "bypass_cache=true" was sent to ESTS, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided', async () => { + test('checks if a token was cached or not and if was "bypass_cache=true" was sent to ESTS depending on whether or not claims are provided, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided via the Managed Identity Request Parameters', async () => { const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( networkClient, "sendGetRequestAsync" ); - const systemAssignedManagedIdentityApplicationWithClientCapabilities: ManagedIdentityApplication = - new ManagedIdentityApplication({ - ...systemAssignedConfig, + let networkManagedIdentityResult: AuthenticationResult = + await systemAssignedManagedIdentityApplication.acquireToken({ clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, + resource: MANAGED_IDENTITY_RESOURCE, }); - - let networkManagedIdentityResult: AuthenticationResult = - await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( - { - resource: MANAGED_IDENTITY_RESOURCE, - } - ); expect(networkManagedIdentityResult.fromCache).toBe(false); expect(networkManagedIdentityResult.accessToken).toEqual( DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken ); + let tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; + let url = tokenRequest[0]; + expect(url.includes("bypass_cache=true")).toBe(false); + expect( + url.includes( + `xms_cc=${CAE_CONSTANTS.CLIENT_CAPABILITIES.toString()}` + ) + ).toBe(true); const cachedManagedIdentityResult: AuthenticationResult = - await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( - { - resource: MANAGED_IDENTITY_RESOURCE, - } - ); + await systemAssignedManagedIdentityApplication.acquireToken({ + clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, + resource: MANAGED_IDENTITY_RESOURCE, + }); expect(cachedManagedIdentityResult.fromCache).toBe(true); expect(cachedManagedIdentityResult.accessToken).toEqual( DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken ); networkManagedIdentityResult = - await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( - { - claims: TEST_CONFIG.CLAIMS, - resource: MANAGED_IDENTITY_RESOURCE, - } - ); + await systemAssignedManagedIdentityApplication.acquireToken({ + claims: TEST_CONFIG.CLAIMS, + clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, + resource: MANAGED_IDENTITY_RESOURCE, + }); expect(networkManagedIdentityResult.fromCache).toBe(false); expect(networkManagedIdentityResult.accessToken).toEqual( DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken ); - - const tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; - const url = tokenRequest[0]; + tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; + url = tokenRequest[0]; expect(url.includes("bypass_cache=true")).toBe(true); expect( url.includes( @@ -606,7 +604,7 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { ).toBe(true); }); - test('does not ignore a cached token when claims are not provided, ensures "bypass_cache=true" was not sent to ESTS, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided', async () => { + test('checks if a token was cached or not and if was "bypass_cache=true" was sent to ESTS depending on whether or not claims are provided, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided via the Managed Identity configuration object', async () => { const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( networkClient, "sendGetRequestAsync" @@ -629,6 +627,15 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken ); + let tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; + let url = tokenRequest[0]; + expect(url.includes("bypass_cache=true")).toBe(false); + expect( + url.includes( + `xms_cc=${CAE_CONSTANTS.CLIENT_CAPABILITIES.toString()}` + ) + ).toBe(true); + const cachedManagedIdentityResult: AuthenticationResult = await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( { @@ -640,9 +647,21 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken ); - const tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; - const url = tokenRequest[0]; - expect(url.includes("bypass_cache=true")).toBe(false); + networkManagedIdentityResult = + await systemAssignedManagedIdentityApplicationWithClientCapabilities.acquireToken( + { + claims: TEST_CONFIG.CLAIMS, + resource: MANAGED_IDENTITY_RESOURCE, + } + ); + expect(networkManagedIdentityResult.fromCache).toBe(false); + expect(networkManagedIdentityResult.accessToken).toEqual( + DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken + ); + + tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; + url = tokenRequest[0]; + expect(url.includes("bypass_cache=true")).toBe(true); expect( url.includes( `xms_cc=${CAE_CONSTANTS.CLIENT_CAPABILITIES.toString()}` From cd6981458fa76cc8dd6891321b8c78f7f3d10149 Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Wed, 27 Nov 2024 12:12:30 -0500 Subject: [PATCH 7/7] Implemented GitHub Feedback --- .../src/client/ManagedIdentityApplication.ts | 4 +- .../BaseManagedIdentitySource.ts | 4 +- .../src/request/ManagedIdentityRequest.ts | 7 ++- .../request/ManagedIdentityRequestParams.ts | 8 +-- .../ManagedIdentitySources/Imds.spec.ts | 58 +------------------ 5 files changed, 12 insertions(+), 69 deletions(-) diff --git a/lib/msal-node/src/client/ManagedIdentityApplication.ts b/lib/msal-node/src/client/ManagedIdentityApplication.ts index d072476def..14a76ce06c 100644 --- a/lib/msal-node/src/client/ManagedIdentityApplication.ts +++ b/lib/msal-node/src/client/ManagedIdentityApplication.ts @@ -141,9 +141,7 @@ export class ManagedIdentityApplication { authority: this.fakeAuthority.canonicalAuthority, correlationId: this.cryptoProvider.createNewGuid(), claims: managedIdentityRequestParams.claims, - clientCapabilities: - managedIdentityRequestParams.clientCapabilities || - this.config.clientCapabilities, + clientCapabilities: this.config.clientCapabilities, }; if ( diff --git a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts index cf3e5487d5..4a67869369 100644 --- a/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts +++ b/lib/msal-node/src/client/ManagedIdentitySources/BaseManagedIdentitySource.ts @@ -133,12 +133,12 @@ export abstract class BaseManagedIdentitySource { managedIdentityId ); - // if claims are present, ESTS will get a new token + // if claims are present, the MSI will get a new token if (managedIdentityRequest.claims) { networkRequest.queryParameters.bypass_cache = "true"; } - // if client capabilities are present, send them to ESTS + // if client capabilities are present, send them to the MSI if (managedIdentityRequest.clientCapabilities?.length) { networkRequest.queryParameters.xms_cc = managedIdentityRequest.clientCapabilities.toString(); diff --git a/lib/msal-node/src/request/ManagedIdentityRequest.ts b/lib/msal-node/src/request/ManagedIdentityRequest.ts index a99ae12b3a..a737d80ab8 100644 --- a/lib/msal-node/src/request/ManagedIdentityRequest.ts +++ b/lib/msal-node/src/request/ManagedIdentityRequest.ts @@ -8,8 +8,9 @@ import { ManagedIdentityRequestParams } from "./ManagedIdentityRequestParams.js" /** * ManagedIdentityRequest - * - forceRefresh - forces managed identity requests to skip the cache and make network calls if true - * - resource - resource requested to access the protected API. It should be of the form "{ResourceIdUri}" or {ResourceIdUri/.default}. For instance https://management.azure.net or, for Microsoft Graph, https://graph.microsoft.com/.default + * - clientCapabilities - an array of client capabilities to be sent to ESTS */ export type ManagedIdentityRequest = ManagedIdentityRequestParams & - CommonClientCredentialRequest; + CommonClientCredentialRequest & { + clientCapabilities?: Array; + }; diff --git a/lib/msal-node/src/request/ManagedIdentityRequestParams.ts b/lib/msal-node/src/request/ManagedIdentityRequestParams.ts index 0ba1a3c8dc..9ba9ef4fcd 100644 --- a/lib/msal-node/src/request/ManagedIdentityRequestParams.ts +++ b/lib/msal-node/src/request/ManagedIdentityRequestParams.ts @@ -5,15 +5,13 @@ /** * ManagedIdentityRequest - * - claims - a stringified claims request which will be used to determine whether or not the cache should be skipped - * - clientCapabilities - an array of client capabilities to be sent to ESTS - * - forceRefresh - forces managed identity requests to skip the cache and make network calls if true - * - resource - resource requested to access the protected API. It should be of the form "ResourceIdUri" or "ResourceIdUri/.default". For instance https://management.azure.net or, for Microsoft Graph, https://graph.microsoft.com/.default + * - claims - a stringified claims request which will be used to determine whether or not the cache should be skipped + * - forceRefresh - forces managed identity requests to skip the cache and make network calls if true + * - resource - resource requested to access the protected API. It should be of the form "ResourceIdUri" or "ResourceIdUri/.default". For instance https://management.azure.net or, for Microsoft Graph, https://graph.microsoft.com/.default * @public */ export type ManagedIdentityRequestParams = { claims?: string; - clientCapabilities?: Array; forceRefresh?: boolean; resource: string; }; diff --git a/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts b/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts index 71fdb69def..55bb673267 100644 --- a/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts +++ b/lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts @@ -550,61 +550,7 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { ); }); - test('checks if a token was cached or not and if was "bypass_cache=true" was sent to ESTS depending on whether or not claims are provided, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided via the Managed Identity Request Parameters', async () => { - const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( - networkClient, - "sendGetRequestAsync" - ); - - let networkManagedIdentityResult: AuthenticationResult = - await systemAssignedManagedIdentityApplication.acquireToken({ - clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, - resource: MANAGED_IDENTITY_RESOURCE, - }); - expect(networkManagedIdentityResult.fromCache).toBe(false); - expect(networkManagedIdentityResult.accessToken).toEqual( - DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken - ); - let tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; - let url = tokenRequest[0]; - expect(url.includes("bypass_cache=true")).toBe(false); - expect( - url.includes( - `xms_cc=${CAE_CONSTANTS.CLIENT_CAPABILITIES.toString()}` - ) - ).toBe(true); - - const cachedManagedIdentityResult: AuthenticationResult = - await systemAssignedManagedIdentityApplication.acquireToken({ - clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, - resource: MANAGED_IDENTITY_RESOURCE, - }); - expect(cachedManagedIdentityResult.fromCache).toBe(true); - expect(cachedManagedIdentityResult.accessToken).toEqual( - DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken - ); - - networkManagedIdentityResult = - await systemAssignedManagedIdentityApplication.acquireToken({ - claims: TEST_CONFIG.CLAIMS, - clientCapabilities: CAE_CONSTANTS.CLIENT_CAPABILITIES, - resource: MANAGED_IDENTITY_RESOURCE, - }); - expect(networkManagedIdentityResult.fromCache).toBe(false); - expect(networkManagedIdentityResult.accessToken).toEqual( - DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken - ); - tokenRequest = sendGetRequestAsyncSpy.mock.lastCall; - url = tokenRequest[0]; - expect(url.includes("bypass_cache=true")).toBe(true); - expect( - url.includes( - `xms_cc=${CAE_CONSTANTS.CLIENT_CAPABILITIES.toString()}` - ) - ).toBe(true); - }); - - test('checks if a token was cached or not and if was "bypass_cache=true" was sent to ESTS depending on whether or not claims are provided, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided via the Managed Identity configuration object', async () => { + test('checks if a token was cached or not and if was "bypass_cache=true" was sent to the MSI depending on whether or not claims are provided, and ensures "xms=client-capabilites-string" was sent to ESTS when client capabilities are provided via the Managed Identity configuration object', async () => { const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( networkClient, "sendGetRequestAsync" @@ -669,7 +615,7 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { ).toBe(true); }); - test('ignores a cached token when claims are provided, and ensures "bypass_cache=true" was sent to ESTS', async () => { + test('ignores a cached token when claims are provided, and ensures "bypass_cache=true" was sent to the MSI', async () => { const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( networkClient, "sendGetRequestAsync"