[Identity] Removed allowMultiTenantAuthentication (#17915)

* [Identity] Removed allowMultiTenantAuthentication

* small changelog improvement

* review file

* adfs error message

sdk/identity/identity/CHANGELOG.md

@@ -6,6 +6,11 @@
 
 ### Breaking Changes
 
+#### Breaking Changes from 2.0.0-beta.4
+
+- Removed the `allowMultiTenantAuthentication` option from all of the credentials. Multi-tenant authentication is now enabled by default. On Node.js, it can be disabled with the `AZURE_IDENTITY_DISABLE_MULTITENANTAUTH` environment variable.
+
+
 ### Bugs Fixed
 
 ### Other Changes

sdk/identity/identity/package.json

@@ -24,6 +24,7 @@
     "./dist-esm/src/credentials/applicationCredential.js": "./dist-esm/src/credentials/applicationCredential.browser.js",
     "./dist-esm/src/credentials/onBehalfOfCredential.js": "./dist-esm/src/credentials/onBehalfOfCredential.browser.js",
     "./dist-esm/src/util/authHostEnv.js": "./dist-esm/src/util/authHostEnv.browser.js",
+    "./dist-esm/src/util/validateMultiTenant.js": "./dist-esm/src/util/validateMultiTenant.browser.js",
     "./dist-esm/src/tokenCache/TokenCachePersistence.js": "./dist-esm/src/tokenCache/TokenCachePersistence.browser.js",
     "./dist-esm/src/plugins/consumer.js": "./dist-esm/src/plugins/consumer.browser.js",
     "./dist-esm/test/httpRequests.js": "./dist-esm/test/httpRequests.browser.js"

sdk/identity/identity/review/identity.api.md

@@ -346,7 +346,6 @@ export { TokenCredential }
 
 // @public
 export interface TokenCredentialOptions extends CommonClientOptions {
-    allowMultiTenantAuthentication?: boolean;
     authorityHost?: string;
 }
 

sdk/identity/identity/src/client/identityClient.ts

@@ -307,9 +307,4 @@ export interface TokenCredentialOptions extends CommonClientOptions {
    * The default is "https://login.microsoftonline.com".
    */
   authorityHost?: string;
-
-  /**
-   * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call.
-   */
-  allowMultiTenantAuthentication?: boolean;
 }

sdk/identity/identity/src/credentials/azureCliCredential.ts

@@ -82,7 +82,6 @@ const logger = credentialLogger("AzureCliCredential");
  */
 export class AzureCliCredential implements TokenCredential {
   private tenantId?: string;
-  private allowMultiTenantAuthentication?: boolean;
 
   /**
    * Creates an instance of the {@link AzureCliCredential}.
@@ -91,7 +90,6 @@ export class AzureCliCredential implements TokenCredential {
    */
   constructor(options?: AzureCliCredentialOptions) {
     this.tenantId = options?.tenantId;
-    this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication;
   }
 
   /**
@@ -106,11 +104,7 @@ export class AzureCliCredential implements TokenCredential {
     scopes: string | string[],
     options?: GetTokenOptions
   ): Promise<AccessToken> {
-    const tenantId = processMultiTenantRequest(
-      this.tenantId,
-      this.allowMultiTenantAuthentication,
-      options
-    );
+    const tenantId = processMultiTenantRequest(this.tenantId, options);
     if (tenantId) {
       checkTenantId(logger, tenantId);
     }

sdk/identity/identity/src/credentials/azurePowerShellCredential.ts

@@ -96,7 +96,6 @@ if (isWindows) {
  */
 export class AzurePowerShellCredential implements TokenCredential {
   private tenantId?: string;
-  private allowMultiTenantAuthentication?: boolean;
 
   /**
    * Creates an instance of the {@link AzurePowershellCredential}.
@@ -105,7 +104,6 @@ export class AzurePowerShellCredential implements TokenCredential {
    */
   constructor(options?: AzurePowerShellCredentialOptions) {
     this.tenantId = options?.tenantId;
-    this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication;
   }
 
   /**
@@ -167,11 +165,7 @@ export class AzurePowerShellCredential implements TokenCredential {
     options: GetTokenOptions = {}
   ): Promise<AccessToken> {
     return trace(`${this.constructor.name}.getToken`, options, async () => {
-      const tenantId = processMultiTenantRequest(
-        this.tenantId,
-        this.allowMultiTenantAuthentication,
-        options
-      );
+      const tenantId = processMultiTenantRequest(this.tenantId, options);
       if (tenantId) {
         checkTenantId(logger, tenantId);
       }

sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts

@@ -103,7 +103,6 @@ export class VisualStudioCodeCredential implements TokenCredential {
   private identityClient: IdentityClient;
   private tenantId: string;
   private cloudName: VSCodeCloudNames;
-  private allowMultiTenantAuthentication?: boolean;
 
   /**
    * Creates an instance of VisualStudioCodeCredential to use for automatically authenticating via VSCode.
@@ -134,7 +133,6 @@ export class VisualStudioCodeCredential implements TokenCredential {
     } else {
       this.tenantId = CommonTenantId;
     }
-    this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication;
 
     checkUnsupportedTenant(this.tenantId);
   }
@@ -180,9 +178,7 @@ export class VisualStudioCodeCredential implements TokenCredential {
   ): Promise<AccessToken> {
     await this.prepareOnce();
 
-    const tenantId =
-      processMultiTenantRequest(this.tenantId, this.allowMultiTenantAuthentication, options) ||
-      this.tenantId;
+    const tenantId = processMultiTenantRequest(this.tenantId, options) || this.tenantId;
 
     if (findCredentials === undefined) {
       throw new CredentialUnavailableError(

sdk/identity/identity/src/msal/browserFlows/browserCommon.ts

@@ -23,7 +23,6 @@ import { processMultiTenantRequest } from "../../util/validateMultiTenant";
 export interface MsalBrowserFlowOptions extends MsalFlowOptions {
   redirectUri?: string;
   loginStyle: BrowserLoginStyle;
-  allowMultiTenantAuthentication?: boolean;
   loginHint?: string;
 }
 
@@ -71,7 +70,6 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows
   protected loginStyle: BrowserLoginStyle;
   protected clientId: string;
   protected tenantId: string;
-  protected allowMultiTenantAuthentication?: boolean;
   protected authorityHost?: string;
   protected account: AuthenticationRecord | undefined;
   protected msalConfig: msalBrowser.Configuration;
@@ -87,7 +85,6 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows
     }
     this.clientId = options.clientId;
     this.tenantId = resolveTenantId(this.logger, options.tenantId, options.clientId);
-    this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication;
     this.authorityHost = options.authorityHost;
     this.msalConfig = defaultBrowserMsalConfig(options);
     this.disableAutomaticAuthentication = options.disableAutomaticAuthentication;
@@ -146,9 +143,7 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows
     scopes: string[],
     options: CredentialFlowGetTokenOptions = {}
   ): Promise<AccessToken> {
-    const tenantId =
-      processMultiTenantRequest(this.tenantId, this.allowMultiTenantAuthentication, options) ||
-      this.tenantId;
+    const tenantId = processMultiTenantRequest(this.tenantId, options) || this.tenantId;
 
     if (!options.authority) {
       options.authority = getAuthority(tenantId, this.authorityHost);

sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts

@@ -33,7 +33,6 @@ import { processMultiTenantRequest } from "../../util/validateMultiTenant";
 export interface MsalNodeOptions extends MsalFlowOptions {
   tokenCachePersistenceOptions?: TokenCachePersistenceOptions;
   tokenCredentialOptions: TokenCredentialOptions;
-  allowMultiTenantAuthentication?: boolean;
   /**
    * Specifies a regional authority. Please refer to the {@link RegionalAuthority} type for the accepted values.
    * If {@link RegionalAuthority.AutoDiscoverRegion} is specified, we will try to discover the regional authority endpoint.
@@ -75,7 +74,6 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow {
   protected msalConfig: msalNode.Configuration;
   protected clientId: string;
   protected tenantId: string;
-  protected allowMultiTenantAuthentication?: boolean;
   protected authorityHost?: string;
   protected identityClient?: IdentityClient;
   protected requiresConfidential: boolean = false;
@@ -86,7 +84,6 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow {
     super(options);
     this.msalConfig = this.defaultNodeMsalConfig(options);
     this.tenantId = resolveTenantId(options.logger, options.tenantId, options.clientId);
-    this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication;
     this.clientId = this.msalConfig.auth.clientId;
 
     // If persistence has been configured
@@ -280,9 +277,7 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
     scopes: string[],
     options: CredentialFlowGetTokenOptions = {}
   ): Promise<AccessToken> {
-    const tenantId =
-      processMultiTenantRequest(this.tenantId, this.allowMultiTenantAuthentication, options) ||
-      this.tenantId;
+    const tenantId = processMultiTenantRequest(this.tenantId, options) || this.tenantId;
 
     options.authority = getAuthority(tenantId, this.authorityHost);
 

sdk/identity/identity/src/util/validateMultiTenant.browser.ts

@@ -0,0 +1,29 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+import { GetTokenOptions } from "@azure/core-auth";
+
+/**
+ * @internal
+ */
+export const multiTenantADFSErrorMessage =
+  "A new tenant Id can't be assigned through the GetTokenOptions when a credential has been originally configured to use the tenant `adfs`.";
+
+/**
+ * Of getToken contains a tenantId, this functions allows picking this tenantId as the appropriate for authentication,
+ * unless multitenant authentication has been disabled through the AZURE_IDENTITY_DISABLE_MULTITENANTAUTH (on Node.js),
+ * or unless the original tenant Id is `adfs`.
+ * @internal
+ */
+export function processMultiTenantRequest(
+  tenantId?: string,
+  getTokenOptions?: GetTokenOptions
+): string | undefined {
+  if (!getTokenOptions?.tenantId) {
+    return tenantId;
+  }
+  if (tenantId === "adfs") {
+    throw new Error(multiTenantADFSErrorMessage);
+  }
+  return getTokenOptions?.tenantId;
+}

sdk/identity/identity/src/util/validateMultiTenant.ts

@@ -6,29 +6,33 @@ import { GetTokenOptions } from "@azure/core-auth";
 /**
  * @internal
  */
-export const multiTenantErrorMessage =
-  "A getToken request was attempted with a tenant different than the tenant configured at the initialization of the credential, but multi-tenant authentication was not enabled in this credential instance.";
+export const multiTenantDisabledErrorMessage =
+  "A getToken request was attempted with a tenant different than the tenant configured at the initialization of the credential, but multi-tenant authentication has been disabled by the environment variable AZURE_IDENTITY_DISABLE_MULTITENANTAUTH.";
 
 /**
- * Verifies whether locally assigned tenants are equal to tenants received through getToken.
- * Returns the appropriate tenant.
+ * @internal
+ */
+export const multiTenantADFSErrorMessage =
+  "A new tenant Id can't be assigned through the GetTokenOptions when a credential has been originally configured to use the tenant `adfs`.";
+
+/**
+ * Of getToken contains a tenantId, this functions allows picking this tenantId as the appropriate for authentication,
+ * unless multitenant authentication has been disabled through the AZURE_IDENTITY_DISABLE_MULTITENANTAUTH (on Node.js),
+ * or unless the original tenant Id is `adfs`.
  * @internal
  */
 export function processMultiTenantRequest(
   tenantId?: string,
-  allowMultiTenantAuthentication?: boolean,
   getTokenOptions?: GetTokenOptions
 ): string | undefined {
-  if (
-    !allowMultiTenantAuthentication &&
-    getTokenOptions?.tenantId &&
-    tenantId &&
-    getTokenOptions.tenantId !== tenantId
-  ) {
-    throw new Error(multiTenantErrorMessage);
+  if (!getTokenOptions?.tenantId) {
+    return tenantId;
+  }
+  if (process.env.AZURE_IDENTITY_DISABLE_MULTITENANTAUTH) {
+    throw new Error(multiTenantDisabledErrorMessage);
   }
-  if (allowMultiTenantAuthentication && getTokenOptions?.tenantId) {
-    return getTokenOptions.tenantId;
+  if (tenantId === "adfs") {
+    throw new Error(multiTenantADFSErrorMessage);
   }
-  return tenantId;
+  return getTokenOptions?.tenantId;
 }

sdk/identity/identity/test/internal/node/utils.spec.ts

@@ -0,0 +1,40 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+import { assert } from "chai";
+import {
+  multiTenantDisabledErrorMessage,
+  processMultiTenantRequest
+} from "../../../src/util/validateMultiTenant";
+
+describe("Identity utilities (Node.js only)", function() {
+  describe("validateMultiTenantRequest (Node.js only)", function() {
+    afterEach(() => {
+      delete process.env.AZURE_IDENTITY_DISABLE_MULTITENANTAUTH;
+    });
+
+    it("returns the original tenant and doesn't throw if getTokenOptions does not have a tenantId, even if AZURE_IDENTITY_DISABLE_MULTITENANTAUTH is defined", async function() {
+      process.env.AZURE_IDENTITY_DISABLE_MULTITENANTAUTH = "true";
+      const originalTenant = "credential-options-tenant-id";
+      const resultingTenant = processMultiTenantRequest(originalTenant);
+      assert.equal(resultingTenant, originalTenant);
+    });
+
+    it("throws if multi-tenant authentication is disabled via AZURE_IDENTITY_DISABLE_MULTITENANTAUTH", async function() {
+      let error: Error | undefined;
+      process.env.AZURE_IDENTITY_DISABLE_MULTITENANTAUTH = "true";
+      try {
+        processMultiTenantRequest("credential-options-tenant-id", {
+          tenantId: "get-token-options-tenant-id"
+        });
+      } catch (e) {
+        error = e;
+      }
+      assert.ok(
+        error,
+        "validateMultiTenantRequest should throw if multi-tenant authentication is disabled"
+      );
+      assert.equal(error!.message, multiTenantDisabledErrorMessage);
+    });
+  });
+});