[Identity] Removed allowMultiTenantAuthentication

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,31 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+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 has been disabled by the environment variable AZURE_IDENTITY_DISABLE_MULTITENANTAUTH.";
+
+/**
+ * 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 (
+    // Only if a getTokenOptions.tenantId was received
+    getTokenOptions?.tenantId &&
+    // Disabled if the credential has been configured with `adfs` as the tenant Id.
+    tenantId === "adfs"
+  ) {
+    throw new Error(multiTenantErrorMessage);
+  }
+  return getTokenOptions?.tenantId || tenantId;
+}

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

@@ -7,28 +7,27 @@ 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.";
+  "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.
+ * 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 &&
+    // Only if a getTokenOptions.tenantId was received
     getTokenOptions?.tenantId &&
-    tenantId &&
-    getTokenOptions.tenantId !== tenantId
+    // Disabled if the AZURE_IDENTITY_DISABLE_MULTITENANTAUTH environment variable is set.
+    (process.env.AZURE_IDENTITY_DISABLE_MULTITENANTAUTH ||
+      // Disabled if the credential has been configured with `adfs` as the tenant Id.
+      tenantId === "adfs")
   ) {
     throw new Error(multiTenantErrorMessage);
   }
-  if (allowMultiTenantAuthentication && getTokenOptions?.tenantId) {
-    return getTokenOptions.tenantId;
-  }
-  return tenantId;
+  return getTokenOptions?.tenantId || 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 {
+  multiTenantErrorMessage,
+  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, multiTenantErrorMessage);
+    });
+  });
+});

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

@@ -9,76 +9,44 @@ import {
 
 describe("Identity utilities", function() {
   describe("validateMultiTenantRequest", function() {
-    it("throws if multi-tenant authentication is disallowed, and the tenants don't match", async function() {
-      let error: Error | undefined;
-      try {
-        processMultiTenantRequest("credential-options-tenant-id", false, {
-          tenantId: "get-token-options-tenant-id"
-        });
-      } catch (e) {
-        error = e;
-      }
-      assert.ok(
-        error,
-        "validateMultiTenantRequest should throw if multi-tenant is disallowed and the tenants don't match"
-      );
-      assert.equal(error!.message, multiTenantErrorMessage);
+    it("returns the original tenant if getTokenOptions does not have a tenantId", async function() {
+      const originalTenant = "credential-options-tenant-id";
+      const resultingTenant = processMultiTenantRequest(originalTenant);
+      assert.equal(resultingTenant, originalTenant);
     });
 
-    it("throws if multi-tenant authentication is undefined, and the tenants don't match", async function() {
+    it("throws if a tenant is provided through getTokenoptions and the original tenant Id is 'asdf'", async function() {
       let error: Error | undefined;
       try {
-        processMultiTenantRequest("credential-options-tenant-id", undefined, {
+        processMultiTenantRequest("adfs", {
           tenantId: "get-token-options-tenant-id"
         });
       } catch (e) {
         error = e;
       }
       assert.ok(
         error,
-        "validateMultiTenantRequest should throw if multi-tenant is disallowed and the tenants don't match"
+        "validateMultiTenantRequest should throw if a tenant is provided through getTokenoptions and the original tenant Id is 'asdf'"
       );
       assert.equal(error!.message, multiTenantErrorMessage);
     });
 
-    it("If allowMultiTenantAuthentication is disallowed, it shouldn't throw if the tenant received is the same as the tenant we already had, that same tenant should be returned", async function() {
+    it("it shouldn't throw if the tenant received is the same as the tenant we already had", async function() {
       assert.equal(
-        processMultiTenantRequest("same-tenant", false, {
-          tenantId: "same-tenant"
-        }),
-        "same-tenant"
-      );
-      assert.equal(
-        processMultiTenantRequest("same-tenant", undefined, {
+        processMultiTenantRequest("same-tenant", {
           tenantId: "same-tenant"
         }),
         "same-tenant"
       );
     });
 
-    it("If we had a tenant and the options have another tenant, we pick the tenant from the options", async function() {
+    it("should pick the tenant from the options", async function() {
       assert.equal(
-        processMultiTenantRequest("credential-options-tenant-id", true, {
+        processMultiTenantRequest("credential-options-tenant-id", {
           tenantId: "get-token-options-tenant-id"
         }),
         "get-token-options-tenant-id"
       );
     });
-
-    it("If we had a tenant and there is no tenant in the options, we pick the tenant we already had", async function() {
-      assert.equal(
-        processMultiTenantRequest("credential-options-tenant-id", true, {}),
-        "credential-options-tenant-id"
-      );
-    });
-
-    it("If the tenant received is the same as the tenant we already had, that same tenant should be returned", async function() {
-      assert.equal(
-        processMultiTenantRequest("same-tenant", true, {
-          tenantId: "same-tenant"
-        }),
-        "same-tenant"
-      );
-    });
   });
 });