Skip to content
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

Implemented functionality to skip the cache for MI when claims are provided #7207

Open
wants to merge 28 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
dc5eed5
Implemented functionality to skip the cache for MI when claims are pr…
Robbie-Microsoft Jul 17, 2024
e2b3281
Change files
Robbie-Microsoft Jul 17, 2024
0308ae4
beachball + apiExtractor
Robbie-Microsoft Jul 17, 2024
271094e
Change files
Robbie-Microsoft Jul 17, 2024
695efec
Added claims to the MI endpoint
Robbie-Microsoft Jul 17, 2024
35cd7bc
removed comment
Robbie-Microsoft Jul 17, 2024
c3bc890
Merge branch 'dev' into mi_claims
Robbie-Microsoft Jul 22, 2024
480c318
Merge branch 'dev' into mi_claims
Robbie-Microsoft Jul 29, 2024
3eaf74d
Added claims to clientAssertionCallback
Robbie-Microsoft Jul 29, 2024
622869d
Deprecated client assertion strings
Robbie-Microsoft Jul 30, 2024
c3e1b6b
Merge branch 'dev' into mi_claims
Robbie-Microsoft Aug 7, 2024
e04d663
Merge branch 'dev' into mi_claims
Robbie-Microsoft Aug 8, 2024
a10e633
merged dev
Robbie-Microsoft Aug 21, 2024
9abe98a
Undid clientAssertion string deprecation
Robbie-Microsoft Aug 22, 2024
414a456
Added JSDocs to RequestValidator.ts
Robbie-Microsoft Aug 22, 2024
6449f9e
Merge branch 'dev' into mi_claims
Robbie-Microsoft Aug 26, 2024
51938d7
Implemented Feedback
Robbie-Microsoft Aug 29, 2024
e1176e8
Merge branch 'dev' into mi_claims
Robbie-Microsoft Aug 29, 2024
fc82433
undid changes to api files
Robbie-Microsoft Aug 29, 2024
e6f7c1f
Merge branch 'dev' into mi_claims
Robbie-Microsoft Aug 30, 2024
f947bb1
Merge branch 'dev' into mi_claims
Robbie-Microsoft Sep 3, 2024
0501ada
Merge branch 'dev' into mi_claims
Robbie-Microsoft Sep 11, 2024
8181612
api extractor
Robbie-Microsoft Sep 11, 2024
206985f
Merge branch 'dev' into mi_claims
Robbie-Microsoft Sep 16, 2024
76c858f
Merge branch 'dev' into mi_claims
Robbie-Microsoft Sep 23, 2024
20eca62
Merge branch 'dev' into mi_claims
Robbie-Microsoft Sep 26, 2024
a725048
removed functionality sending claims to the network
Robbie-Microsoft Sep 26, 2024
59989f4
fixed comment
Robbie-Microsoft Sep 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Implemented functionality to skip the cache for MI when claims are provided #7207",
Robbie-Microsoft marked this conversation as resolved.
Show resolved Hide resolved
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Implemented functionality to skip the cache for MI when claims are provided #7207",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
3 changes: 2 additions & 1 deletion lib/msal-common/apiReview/msal-common.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,7 @@ export type ClientAssertionCallback = (config: ClientAssertionConfig) => Promise
export type ClientAssertionConfig = {
clientId: string;
tokenEndpoint?: string;
claims?: string;
};

declare namespace ClientAssertionUtils {
Expand Down Expand Up @@ -1996,7 +1997,7 @@ function generateCredentialKey(credentialEntity: CredentialEntity): string;
// Warning: (ae-missing-release-tag) "getClientAssertion" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
export function getClientAssertion(clientAssertion: string | ClientAssertionCallback, clientId: string, tokenEndpoint?: string): Promise<string>;
export function getClientAssertion(clientAssertion: string | ClientAssertionCallback, clientId: string, tokenEndpoint?: string, claims?: string): Promise<string>;

// Warning: (ae-missing-release-tag) "getDeserializedResponse" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
Expand Down
1 change: 1 addition & 0 deletions lib/msal-common/src/account/ClientCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
export type ClientAssertionConfig = {
clientId: string;
tokenEndpoint?: string;
claims?: string;
};

export type ClientAssertionCallback = (
Expand Down
4 changes: 3 additions & 1 deletion lib/msal-common/src/utils/ClientAssertionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ import {
export async function getClientAssertion(
clientAssertion: string | ClientAssertionCallback,
clientId: string,
tokenEndpoint?: string
tokenEndpoint?: string,
claims?: string
): Promise<string> {
if (typeof clientAssertion === "string") {
return clientAssertion;
} else {
const config: ClientAssertionConfig = {
clientId: clientId,
tokenEndpoint: tokenEndpoint,
claims: claims,
};
return clientAssertion(config);
}
Expand Down
1 change: 1 addition & 0 deletions lib/msal-node/apiReview/msal-node.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ export type ManagedIdentityIdParams = {
//
// @public
export type ManagedIdentityRequestParams = {
claims?: string;
Robbie-Microsoft marked this conversation as resolved.
Show resolved Hide resolved
forceRefresh?: boolean;
resource: string;
};
Expand Down
6 changes: 5 additions & 1 deletion lib/msal-node/src/client/ManagedIdentityApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ export class ManagedIdentityApplication {
}

const managedIdentityRequest: ManagedIdentityRequest = {
claims: managedIdentityRequestParams.claims,
forceRefresh: managedIdentityRequestParams.forceRefresh,
resource: managedIdentityRequestParams.resource.replace(
"/.default",
Expand All @@ -142,7 +143,10 @@ export class ManagedIdentityApplication {
correlationId: this.cryptoProvider.createNewGuid(),
};

if (managedIdentityRequest.forceRefresh) {
if (
managedIdentityRequest.claims ||
managedIdentityRequest.forceRefresh
) {
// make a network call to the managed identity source
return this.managedIdentityClient.sendManagedIdentityTokenRequest(
managedIdentityRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
createClientAuthError,
AuthenticationResult,
UrlString,
AADServerParamKeys,
} from "@azure/msal-common";
import { ManagedIdentityId } from "../../config/ManagedIdentityId";
import { ManagedIdentityRequestParameters } from "../../config/ManagedIdentityRequestParameters";
Expand Down Expand Up @@ -138,6 +139,11 @@ export abstract class BaseManagedIdentitySource {

const networkRequestOptions: NetworkRequestOptions = { headers };

if (managedIdentityRequest.claims) {
networkRequest.bodyParameters[AADServerParamKeys.CLAIMS] =
Robbie-Microsoft marked this conversation as resolved.
Show resolved Hide resolved
encodeURIComponent(managedIdentityRequest.claims);
}

if (Object.keys(networkRequest.bodyParameters).length) {
networkRequestOptions.body =
networkRequest.computeParametersBodyString();
Expand Down
4 changes: 3 additions & 1 deletion lib/msal-node/src/request/ManagedIdentityRequestParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@

/**
* ManagedIdentityRequest
* - claims - a stringified claims request which will be added to all /authorize and /token calls
* - 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
* - 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
*/
export type ManagedIdentityRequestParams = {
claims?: string;
forceRefresh?: boolean;
resource: string;
};
47 changes: 47 additions & 0 deletions lib/msal-node/test/client/ManagedIdentitySources/Imds.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
MANAGED_IDENTITY_RESOURCE_ID,
MANAGED_IDENTITY_RESOURCE_ID_2,
MANAGED_IDENTITY_TOKEN_RETRIEVAL_ERROR_MESSAGE,
TEST_CONFIG,
THREE_SECONDS_IN_MILLI,
getCacheKey,
} from "../../test_kit/StringConstants";
Expand All @@ -31,6 +32,7 @@ import {
ManagedIdentitySourceNames,
} from "../../../src/utils/Constants";
import {
AADServerParamKeys,
AccessTokenEntity,
AuthenticationResult,
CacheHelpers,
Expand Down Expand Up @@ -548,6 +550,51 @@ describe("Acquires a token successfully via an IMDS Managed Identity", () => {
);
});

test("ignores a cached token when claims are provided", 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
);

const cachedManagedIdentityResult: AuthenticationResult =
await systemAssignedManagedIdentityApplication.acquireToken({
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,
resource: MANAGED_IDENTITY_RESOURCE,
});
expect(networkManagedIdentityResult.fromCache).toBe(false);

expect(
sendGetRequestAsyncSpy.mock.lastCall[1].body.includes(
`${AADServerParamKeys.CLAIMS}=${encodeURIComponent(
TEST_CONFIG.CLAIMS
)}`
)
).toBe(true);

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

test("ignores a cached token when forceRefresh is set to true", async () => {
let networkManagedIdentityResult: AuthenticationResult =
await systemAssignedManagedIdentityApplication.acquireToken({
Expand Down
Loading