[identity] DeviceCodeCredential uses MSALClient (#29405)

### Packages impacted by this PR @azure/identity ### Issues associated with this PR Resolves #29377 ### Describe the problem that is addressed by this PR This PR makes the following changes to the Identity library: 1. Migrates DeviceCodeCredential to the MSALClient flow 2. Skips silent authentication for client credential based flows 3. Allows passing disableAutomaticAuthentication as an option param instead of during client construction The reason this is important and exciting is because DeviceCodeCredential is the first PublicClientApplication based flow that is being migrated to MSALClient!
Azure · May 8, 2024 · 6b099a2 · 6b099a2
1 parent b2b8254
commit 6b099a2
Show file tree

Hide file tree

Showing 6 changed files with 251 additions and 62 deletions.
diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md
@@ -10,8 +10,12 @@
 
 ### Bug Fixes
 
+- `ClientSecretCredential`, `ClientCertificateCredential`, and `ClientAssertionCredential` no longer try silent authentication unnecessarily as per the MSAL guidelines. For more information please refer to [the Entra documentation on token caching](https://learn.microsoft.com/entra/identity-platform/msal-acquire-cache-tokens#recommended-call-pattern-for-public-client-applications). [#29405](https://github.com/Azure/azure-sdk-for-js/pull/29405)
+
 ### Other Changes
 
+- `DeviceCodeCredential` migrated to use MSALClient internally instead of MSALNode flow. This is an internal refactoring and should not result in any behavioral changes. [#29405](https://github.com/Azure/azure-sdk-for-js/pull/29405)
+
 ## 4.2.0 (2024-04-30)
 
 ### Features Added

diff --git a/sdk/identity/identity/assets.json b/sdk/identity/identity/assets.json
@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "js",
   "TagPrefix": "js/identity/identity",
-  "Tag": "js/identity/identity_58e656fd32"
+  "Tag": "js/identity/identity_72abb85c88"
 }
diff --git a/sdk/identity/identity/src/credentials/deviceCodeCredential.ts b/sdk/identity/identity/src/credentials/deviceCodeCredential.ts
@@ -5,14 +5,19 @@ import { AccessToken, GetTokenOptions, TokenCredential } from "@azure/core-auth"
 import {
   processMultiTenantRequest,
   resolveAdditionallyAllowedTenantIds,
+  resolveTenantId,
 } from "../util/tenantIdUtils";
-import { DeviceCodeCredentialOptions, DeviceCodeInfo } from "./deviceCodeCredentialOptions";
+import {
+  DeviceCodeCredentialOptions,
+  DeviceCodeInfo,
+  DeviceCodePromptCallback,
+} from "./deviceCodeCredentialOptions";
 import { AuthenticationRecord } from "../msal/types";
-import { MsalDeviceCode } from "../msal/nodeFlows/msalDeviceCode";
-import { MsalFlow } from "../msal/flows";
 import { credentialLogger } from "../util/logging";
 import { ensureScopes } from "../util/scopeUtils";
 import { tracingClient } from "../util/tracing";
+import { MsalClient, createMsalClient } from "../msal/nodeFlows/msalClient";
+import { DeveloperSignOnClientId } from "../constants";
 
 const logger = credentialLogger("DeviceCodeCredential");
 
@@ -31,8 +36,9 @@ export function defaultDeviceCodePromptCallback(deviceCodeInfo: DeviceCodeInfo):
 export class DeviceCodeCredential implements TokenCredential {
   private tenantId?: string;
   private additionallyAllowedTenantIds: string[];
-  private msalFlow: MsalFlow;
   private disableAutomaticAuthentication?: boolean;
+  private msalClient: MsalClient;
+  private userPromptCallback: DeviceCodePromptCallback;
 
   /**
    * Creates an instance of DeviceCodeCredential with the details needed
@@ -59,10 +65,11 @@ export class DeviceCodeCredential implements TokenCredential {
     this.additionallyAllowedTenantIds = resolveAdditionallyAllowedTenantIds(
       options?.additionallyAllowedTenants,
     );
-    this.msalFlow = new MsalDeviceCode({
+    const clientId = options?.clientId ?? DeveloperSignOnClientId;
+    const tenantId = resolveTenantId(logger, options?.tenantId, clientId);
+    this.userPromptCallback = options?.userPromptCallback ?? defaultDeviceCodePromptCallback;
+    this.msalClient = createMsalClient(clientId, tenantId, {
       ...options,
-      logger,
-      userPromptCallback: options?.userPromptCallback || defaultDeviceCodePromptCallback,
       tokenCredentialOptions: options || {},
     });
     this.disableAutomaticAuthentication = options?.disableAutomaticAuthentication;
@@ -93,7 +100,7 @@ export class DeviceCodeCredential implements TokenCredential {
         );
 
         const arrayScopes = ensureScopes(scopes);
-        return this.msalFlow.getToken(arrayScopes, {
+        return this.msalClient.getTokenByDeviceCode(arrayScopes, this.userPromptCallback, {
           ...newOptions,
           disableAutomaticAuthentication: this.disableAutomaticAuthentication,
         });
@@ -120,8 +127,11 @@ export class DeviceCodeCredential implements TokenCredential {
       options,
       async (newOptions) => {
         const arrayScopes = Array.isArray(scopes) ? scopes : [scopes];
-        await this.msalFlow.getToken(arrayScopes, newOptions);
-        return this.msalFlow.getActiveAccount();
+        await this.msalClient.getTokenByDeviceCode(arrayScopes, this.userPromptCallback, {
+          ...newOptions,
+          disableAutomaticAuthentication: false, // this method should always allow user interaction
+        });
+        return this.msalClient.getActiveAccount();
       },
     );
   }

diff --git a/sdk/identity/identity/src/msal/nodeFlows/msalClient.ts b/sdk/identity/identity/src/msal/nodeFlows/msalClient.ts
@@ -13,26 +13,44 @@ import {
   getKnownAuthorities,
   getMSALLogLevel,
   handleMsalError,
+  msalToPublic,
   publicToMsal,
 } from "../utils";
 
 import { AuthenticationRequiredError } from "../../errors";
-import { CertificateParts } from "../types";
+import { AuthenticationRecord, CertificateParts } from "../types";
 import { IdentityClient } from "../../client/identityClient";
 import { MsalNodeOptions } from "./msalNodeCommon";
 import { calculateRegionalAuthority } from "../../regionalAuthority";
 import { getLogLevel } from "@azure/logger";
 import { resolveTenantId } from "../../util/tenantIdUtils";
+import { DeviceCodePromptCallback } from "../../credentials/deviceCodeCredentialOptions";
 
 /**
  * The logger for all MsalClient instances.
  */
 const msalLogger = credentialLogger("MsalClient");
 
+export interface GetTokenWithSilentAuthOptions extends GetTokenOptions {
+  /**
+   * Disables automatic authentication. If set to true, the method will throw an error if the user needs to authenticate.
+   *
+   * @remarks
+   *
+   * This option will be set to `false` when the user calls `authenticate` directly on a credential that supports it.
+   */
+  disableAutomaticAuthentication?: boolean;
+}
+
 /**
  * Represents a client for interacting with the Microsoft Authentication Library (MSAL).
  */
 export interface MsalClient {
+  getTokenByDeviceCode(
+    arrayScopes: string[],
+    userPromptCallback: DeviceCodePromptCallback,
+    options?: GetTokenWithSilentAuthOptions,
+  ): Promise<AccessToken>;
   /**
    * Retrieves an access token by using a client certificate.
    *
@@ -74,12 +92,21 @@ export interface MsalClient {
     clientSecret: string,
     options?: GetTokenOptions,
   ): Promise<AccessToken>;
+
+  /**
+   * Retrieves the last authenticated account. This method expects an authentication record to have been previously loaded.
+   *
+   * An authentication record could be loaded by calling the `getToken` method, or by providing an `authenticationRecord` when creating a credential.
+   */
+  getActiveAccount(): AuthenticationRecord | undefined;
 }
 
 /**
  * Options for creating an instance of the MsalClient.
  */
-export type MsalClientOptions = Partial<Omit<MsalNodeOptions, "clientId" | "tenantId">>;
+export type MsalClientOptions = Partial<
+  Omit<MsalNodeOptions, "clientId" | "tenantId" | "disableAutomaticAuthentication">
+>;
 
 /**
  * Generates the configuration for MSAL (Microsoft Authentication Library).
@@ -173,6 +200,40 @@ export function createMsalClient(
     pluginConfiguration: msalPlugins.generatePluginConfiguration(createMsalClientOptions),
   };
 
+  const publicApps: Map<string, msal.PublicClientApplication> = new Map();
+  async function getPublicApp(
+    options: GetTokenOptions = {},
+  ): Promise<msal.PublicClientApplication> {
+    const appKey = options.enableCae ? "CAE" : "default";
+
+    let publicClientApp = publicApps.get(appKey);
+    if (publicClientApp) {
+      msalLogger.getToken.info("Existing PublicClientApplication found in cache, returning it.");
+      return publicClientApp;
+    }
+
+    // Initialize a new app and cache it
+    msalLogger.getToken.info(
+      `Creating new PublicClientApplication with CAE ${options.enableCae ? "enabled" : "disabled"}.`,
+    );
+
+    const cachePlugin = options.enableCae
+      ? state.pluginConfiguration.cache.cachePluginCae
+      : state.pluginConfiguration.cache.cachePlugin;
+
+    state.msalConfig.auth.clientCapabilities = options.enableCae ? ["cp1"] : undefined;
+
+    publicClientApp = new msal.PublicClientApplication({
+      ...state.msalConfig,
+      broker: { nativeBrokerPlugin: state.pluginConfiguration.broker.nativeBrokerPlugin },
+      cache: { cachePlugin: await cachePlugin },
+    });
+
+    publicApps.set(appKey, publicClientApp);
+
+    return publicClientApp;
+  }
+
   const confidentialApps: Map<string, msal.ConfidentialClientApplication> = new Map();
   async function getConfidentialApp(
     options: GetTokenOptions = {},
@@ -272,7 +333,7 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
   async function withSilentAuthentication(
     msalApp: msal.ConfidentialClientApplication | msal.PublicClientApplication,
     scopes: Array<string>,
-    options: GetTokenOptions,
+    options: GetTokenWithSilentAuthOptions,
     onAuthenticationRequired: () => Promise<msal.AuthenticationResult | null>,
   ): Promise<AccessToken> {
     let response: msal.AuthenticationResult | null = null;
@@ -282,7 +343,7 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
       if (e.name !== "AuthenticationRequiredError") {
         throw e;
       }
-      if (createMsalClientOptions.disableAutomaticAuthentication) {
+      if (options.disableAutomaticAuthentication) {
         throw new AuthenticationRequiredError({
           scopes,
           getTokenOptions: options,
@@ -324,14 +385,24 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
 
     const msalApp = await getConfidentialApp(options);
 
-    return withSilentAuthentication(msalApp, scopes, options, () =>
-      msalApp.acquireTokenByClientCredential({
+    try {
+      const response = await msalApp.acquireTokenByClientCredential({
         scopes,
         authority: state.msalConfig.auth.authority,
         azureRegion: calculateRegionalAuthority(),
         claims: options?.claims,
-      }),
-    );
+      });
+      ensureValidMsalToken(scopes, response, options);
+
+      msalLogger.getToken.info(formatSuccess(scopes));
+
+      return {
+        token: response.accessToken,
+        expiresOnTimestamp: response.expiresOn.getTime(),
+      };
+    } catch (err: any) {
+      throw handleMsalError(scopes, err, options);
+    }
   }
 
   async function getTokenByClientAssertion(
@@ -345,15 +416,25 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
 
     const msalApp = await getConfidentialApp(options);
 
-    return withSilentAuthentication(msalApp, scopes, options, () =>
-      msalApp.acquireTokenByClientCredential({
+    try {
+      const response = await msalApp.acquireTokenByClientCredential({
         scopes,
         authority: state.msalConfig.auth.authority,
         azureRegion: calculateRegionalAuthority(),
         claims: options?.claims,
         clientAssertion,
-      }),
-    );
+      });
+      ensureValidMsalToken(scopes, response, options);
+
+      msalLogger.getToken.info(formatSuccess(scopes));
+
+      return {
+        token: response.accessToken,
+        expiresOnTimestamp: response.expiresOn.getTime(),
+      };
+    } catch (err: any) {
+      throw handleMsalError(scopes, err, options);
+    }
   }
 
   async function getTokenByClientCertificate(
@@ -366,20 +447,66 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
     state.msalConfig.auth.clientCertificate = certificate;
 
     const msalApp = await getConfidentialApp(options);
-
-    return withSilentAuthentication(msalApp, scopes, options, () =>
-      msalApp.acquireTokenByClientCredential({
+    try {
+      const response = await msalApp.acquireTokenByClientCredential({
         scopes,
+        authority: state.msalConfig.auth.authority,
         azureRegion: calculateRegionalAuthority(),
+        claims: options?.claims,
+      });
+      ensureValidMsalToken(scopes, response, options);
+
+      msalLogger.getToken.info(formatSuccess(scopes));
+
+      return {
+        token: response.accessToken,
+        expiresOnTimestamp: response.expiresOn.getTime(),
+      };
+    } catch (err: any) {
+      throw handleMsalError(scopes, err, options);
+    }
+  }
+
+  async function getTokenByDeviceCode(
+    scopes: string[],
+    deviceCodeCallback: DeviceCodePromptCallback,
+    options: GetTokenWithSilentAuthOptions = {},
+  ): Promise<AccessToken> {
+    msalLogger.getToken.info(`Attempting to acquire token using device code`);
+
+    const msalApp = await getPublicApp(options);
+
+    return withSilentAuthentication(msalApp, scopes, options, () => {
+      const requestOptions: msal.DeviceCodeRequest = {
+        scopes,
+        cancel: options?.abortSignal?.aborted ?? false,
+        deviceCodeCallback,
         authority: state.msalConfig.auth.authority,
         claims: options?.claims,
-      }),
-    );
+      };
+      const deviceCodeRequest = msalApp.acquireTokenByDeviceCode(requestOptions);
+      if (options.abortSignal) {
+        options.abortSignal.addEventListener("abort", () => {
+          requestOptions.cancel = true;
+        });
+      }
+
+      return deviceCodeRequest;
+    });
+  }
+
+  function getActiveAccount(): AuthenticationRecord | undefined {
+    if (!state.cachedAccount) {
+      return undefined;
+    }
+    return msalToPublic(clientId, state.cachedAccount);
   }
 
   return {
+    getActiveAccount,
     getTokenByClientSecret,
     getTokenByClientAssertion,
     getTokenByClientCertificate,
+    getTokenByDeviceCode,
   };
 }