-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added token revocation functionality to Managed Identity #7422
base: dev
Are you sure you want to change the base?
Changes from all commits
621db00
eb87156
122d4a2
93f7c61
b41584e
53b01bb
535b335
aacbdae
cd69814
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "minor", | ||
"comment": "Added token revocation functionality to managed identity (#7422)", | ||
"packageName": "@azure/msal-node", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,6 +133,17 @@ export abstract class BaseManagedIdentitySource { | |
managedIdentityId | ||
); | ||
|
||
// if claims are present, the MSI will get a new token | ||
if (managedIdentityRequest.claims) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add logs to these two conditions? i.e. when claims or capabilities are present? |
||
networkRequest.queryParameters.bypass_cache = "true"; | ||
} | ||
|
||
// if client capabilities are present, send them to the MSI | ||
if (managedIdentityRequest.clientCapabilities?.length) { | ||
networkRequest.queryParameters.xms_cc = | ||
managedIdentityRequest.clientCapabilities.toString(); | ||
} | ||
|
||
const headers: Record<string, string> = networkRequest.headers; | ||
headers[HeaderNames.CONTENT_TYPE] = Constants.URL_FORM_CONTENT_TYPE; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,7 @@ import { | |
} from "../../../src"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests aren't related to IMDS? Is this the right location for them? |
||
// 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,13 +550,82 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => { | |
); | ||
}); | ||
|
||
test("ignores a cached token when claims are provided", 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 () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No tests in the PR to validate that the MSI V1 endpoint version is updated to 2024-09-30 |
||
const sendGetRequestAsyncSpy: jest.SpyInstance = jest.spyOn( | ||
networkClient, | ||
<any>"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 | ||
); | ||
|
||
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, | ||
} | ||
); | ||
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 | ||
); | ||
|
||
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('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, | ||
<any>"sendGetRequestAsync" | ||
); | ||
|
||
let networkManagedIdentityResult: AuthenticationResult = | ||
await systemAssignedManagedIdentityApplication.acquireToken({ | ||
resource: MANAGED_IDENTITY_RESOURCE, | ||
}); | ||
expect(networkManagedIdentityResult.fromCache).toBe(false); | ||
|
||
expect(networkManagedIdentityResult.accessToken).toEqual( | ||
DEFAULT_SYSTEM_ASSIGNED_MANAGED_IDENTITY_AUTHENTICATION_RESULT.accessToken | ||
); | ||
|
@@ -578,6 +648,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 () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest using const here,