Added token revocation functionality to Managed Identity's App Service and Service Fabric sources #7679

Robbie-Microsoft · 2025-04-02T19:15:31Z

Token Revocation spec

lib/msal-node/test/client/ManagedIdentitySources/AppService.spec.ts

lib/msal-node/src/request/ManagedIdentityRequest.ts

lib/msal-node/test/client/ManagedIdentitySources/AppService.spec.ts

Robbie-Microsoft · 2025-04-07T15:55:28Z

lib/msal-common/src/utils/Constants.ts

@@ -389,3 +389,8 @@ export const ONE_DAY_IN_MS = 86400000;

 // Token renewal offset default in seconds
 export const DEFAULT_TOKEN_RENEWAL_OFFSET_SEC = 300;
+
+export const EncodingTypes = {
+    HEX: "hex",


I'll be making another PR where I replace every instance of "hex" with this new constant, and make "base64" value as well.

I would recommend to create a github issue and link it here for reference

neha-bhargava · 2025-04-10T16:33:09Z

lib/msal-common/apiReview/msal-common.api.md

@@ -2215,6 +2215,17 @@ const emptyInputScopesError = "empty_input_scopes_error";
 // @public (undocumented)
 const emptyInputScopeSet = "empty_input_scopeset";

+// Warning: (ae-missing-release-tag) "EncodingTypes" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
+// Warning: (ae-missing-release-tag) "EncodingTypes" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)


nit: Duplicate line.

Everything in this file is auto-generated; we don't touch this file. But I'll re-run the apiExtractor and see if it removes the duplicate line.

neha-bhargava · 2025-04-10T16:39:42Z

lib/msal-node/src/client/ManagedIdentityApplication.ts

+     * @param refreshAccessToken - Optional flag indicating whether to force a refresh of the access token.
+     * @returns A promise that resolves to an AuthenticationResult containing the acquired token and related information.
+     */
+    private async acquireTokenFromManagedIdentity(


Why is this method needed?

I extracted the functionality for the network request since it's being used multiple times.

This allowed me to write better docs for it

neha-bhargava · 2025-04-10T16:56:34Z

lib/msal-node/test/client/ManagedIdentitySources/AppService.spec.ts

@@ -176,6 +178,70 @@ describe("Acquires a token successfully via an App Service Managed Identity", ()
        });
    });

+    describe("Miscellaneous", () => {
+        test("ignores a cached token when claims are provided and the Managed Identity does support token revocation, and ensures the token revocation query parameter token_sha256_to_refresh was included in the network request to the Managed Identity", async () => {


Add test when just the capabilities are set and claims are not passed?

gladjohn · 2025-04-10T19:14:16Z

lib/msal-node/src/client/ManagedIdentitySources/AppService.ts

 } from "../../utils/Constants.js";
 import { CryptoProvider } from "../../crypto/CryptoProvider.js";
 import { ManagedIdentityRequestParameters } from "../../config/ManagedIdentityRequestParameters.js";
 import { ManagedIdentityId } from "../../config/ManagedIdentityId.js";
 import { NodeStorage } from "../../cache/NodeStorage.js";

 // MSI Constants. Docs for MSI are available here https://docs.microsoft.com/azure/app-service/overview-managed-identity
-const APP_SERVICE_MSI_API_VERSION: string = "2019-08-01";
+const APP_SERVICE_MSI_API_VERSION: string = "2025-03-30";


please do not merge this

gladjohn

Overall, the PR looks solid. It adds a clear, consistent approach for token revocation. If you remove 'app service' the PR can be merged. Else,. please mark it as 'Do Not Merge'

gladjohn · 2025-04-10T19:17:31Z

lib/msal-node/src/client/ManagedIdentityApplication.ts

+
+const SOURCES_THAT_SUPPORT_TOKEN_REVOCATION: Array<ManagedIdentitySourceNames> =
+    [
+        ManagedIdentitySourceNames.APP_SERVICE,


If you remove app service, you can merge the PR. we can add app service later

gladjohn · 2025-04-10T19:19:16Z

lib/msal-node/src/client/ManagedIdentityApplication.ts

+             * Check if there is a cached token and if the Managed Identity source supports token revocation.
+             * If so, hash the cached access token and add it to the request.
+             */
+            if (


GOOD - if claims are set and you have a cached token, you skip the cache for that request, if the source supports revocation, you also send the old token’s SHA-256 to the server. This ensures the old token is invalidated on the RP

gladjohn · 2025-04-10T19:20:35Z

lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts

            expect(networkManagedIdentityResult.accessToken).toEqual(
                DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken
            );

+            expect(sendGetRequestAsyncSpy.mock.calls.length).toEqual(1);
+            const firstNetworkRequestUrlParams: URLSearchParams =


Good use of spies and verifying the final URLSearchParams

gladjohn · 2025-04-10T19:22:07Z

lib/msal-node/src/client/ManagedIdentityApplication.ts

+import { ManagedIdentityId } from "../config/ManagedIdentityId.js";
+import { HashUtils } from "../crypto/HashUtils.js";
+
+const SOURCES_THAT_SUPPORT_TOKEN_REVOCATION: Array<ManagedIdentitySourceNames> =


Great that you have a dedicated list of sources that handle revocation. If more services adopt it, it should be trivial to add them to SOURCES_THAT_SUPPORT_TOKEN_REVOCATION.

Robbie-Microsoft added 2 commits April 2, 2025 15:08

Initial Implementation

920bac3

Change files

73ffd22

github-actions bot added documentation Related to documentation. msal-node Related to msal-node package labels Apr 2, 2025

Improvements

f4f002a

Robbie-Microsoft changed the title ~~Added token revocation functionality to Managed Identity's App Service and Service Fabric Sources~~ Added token revocation functionality to Managed Identity's App Service and Service Fabric sources Apr 2, 2025

Robbie-Microsoft added 3 commits April 2, 2025 18:47

Improvements

468de01

Added unit tests

dcd382f

Merge branch 'dev' into msi_v1_token_revocation

d8fbbca

Robbie-Microsoft marked this pull request as ready for review April 3, 2025 22:08

Robbie-Microsoft requested review from sameerag, tnorling, hectormmg, peterzenz and a team as code owners April 3, 2025 22:08

bgavrilMS reviewed Apr 3, 2025

View reviewed changes

lib/msal-node/test/client/ManagedIdentitySources/AppService.spec.ts Outdated Show resolved Hide resolved

bgavrilMS reviewed Apr 3, 2025

View reviewed changes

lib/msal-node/test/client/ManagedIdentitySources/AppService.spec.ts Show resolved Hide resolved

bgavrilMS reviewed Apr 3, 2025

View reviewed changes

lib/msal-node/test/client/ManagedIdentitySources/AppService.spec.ts Outdated Show resolved Hide resolved

bgavrilMS reviewed Apr 3, 2025

View reviewed changes

lib/msal-node/src/request/ManagedIdentityRequest.ts Outdated Show resolved Hide resolved

Robbie-Microsoft added 3 commits April 3, 2025 18:46

Implemented some github feedback

b09a464

Implemented github feedback

0f7b2b9

package-lock

b32f45e

Robbie-Microsoft requested review from jo-arroyo, lalimasharda, konstantin-msft and shylasummers as code owners April 3, 2025 23:48

bgavrilMS reviewed Apr 4, 2025

View reviewed changes

lib/msal-node/test/client/ManagedIdentitySources/AppService.spec.ts Outdated Show resolved Hide resolved

Defined sha256 hash for test access token

604194d

Robbie-Microsoft requested a review from bgavrilMS April 4, 2025 19:12

Switched to hex encoding, instead of base64, for the sha256 hash

3405bce

Robbie-Microsoft added 2 commits April 7, 2025 11:53

Change files

f1d096f

Merge branch 'dev' into msi_v1_token_revocation

203d5d0

github-actions bot added the msal-common Related to msal-common package label Apr 7, 2025

Robbie-Microsoft commented Apr 7, 2025

View reviewed changes

neha-bhargava reviewed Apr 10, 2025

View reviewed changes

AzureAD deleted a comment from julesblancphin Apr 10, 2025

neha-bhargava reviewed Apr 10, 2025

View reviewed changes

gladjohn reviewed Apr 10, 2025

View reviewed changes

gladjohn approved these changes Apr 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added token revocation functionality to Managed Identity's App Service and Service Fabric sources #7679

Added token revocation functionality to Managed Identity's App Service and Service Fabric sources #7679

Robbie-Microsoft commented Apr 2, 2025 •

edited

Loading

Robbie-Microsoft Apr 7, 2025

neha-bhargava Apr 10, 2025

neha-bhargava Apr 10, 2025

Robbie-Microsoft Apr 10, 2025

neha-bhargava Apr 10, 2025

Robbie-Microsoft Apr 10, 2025

Robbie-Microsoft Apr 10, 2025

neha-bhargava Apr 10, 2025

gladjohn Apr 10, 2025

gladjohn left a comment

gladjohn Apr 10, 2025

gladjohn Apr 10, 2025

gladjohn Apr 10, 2025

gladjohn Apr 10, 2025

Added token revocation functionality to Managed Identity's App Service and Service Fabric sources #7679

Are you sure you want to change the base?

Added token revocation functionality to Managed Identity's App Service and Service Fabric sources #7679

Conversation

Robbie-Microsoft commented Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gladjohn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Robbie-Microsoft commented Apr 2, 2025 •

edited

Loading