[Identity] InteractiveBrowserCredential for browser requires a client ID (Azure#13844)

* wip

* missed InteractiveBrowserCredentialCommonOptions on the API review file

* better explained first paragraph

Author: sadasant

sdk/identity/identity/CHANGELOG.md

@@ -4,7 +4,9 @@
 
 ## 1.2.4-beta.2 (Unreleased)
 
-- Breaking change: `DefaultAzureCredential` now only uses `InteractiveBrowserCredential` in the browser since it's the only credential intended to be used to authenticate within browsers.
+- `DefaultAzureCredential`'s implementation for browsers was simplified to throw a simple error instead of trying credentials that were already not supported for the browser.
+- Breaking Change: `InteractiveBrowserCredential` for the browser now requires the client ID to be provided.
+- Documentation was added to elaborate on how to configure an AAD application to support `InteractiveBrowserCredential`.
 
 ## 1.2.4-beta.1 (2021-02-12)
 

sdk/identity/identity/interactive-browser-credential.md

@@ -1,18 +1,60 @@
 # Interactive Browser Credential
 
-The `InteractiveBrowserCredential` uses [Auth Code Flow][AuthCodeFlow], which uses [Proof Key for Code Exchange (PKCE)](https://tools.ietf.org/html/rfc7636), uses [MSAL v2.x](https://github.com/AzureAD/microsoft-authentication-library-for-js), and is enabled by default. It also allows switching back to the [Implicit Grant Flow][ImplicitGrantFlow] by passing the `flow` property with the value `implicit-grant` through to the constructor of the `InteractiveBrowserCredential`.
+The `InteractiveBrowserCredential` uses [Authorization Code Flow][AuthCodeFlow], which uses [Proof Key for Code Exchange (PKCE)](https://tools.ietf.org/html/rfc7636) both on the browser and on NodeJS. Under the hood it uses [@azure/msal-node](https://www.npmjs.com/package/@azure/msal-node) for NodeJS. For the browser it uses [MSAL v2.x](https://github.com/AzureAD/microsoft-authentication-library-for-js) by default (which also uses Authorization Code Flow), while it also allows switching back to the older [Implicit Grant Flow][ImplicitGrantFlow] by passing the `flow` property with the value `implicit-grant` through to the constructor of the `InteractiveBrowserCredential`, as follows:
+
+```ts
+const credential = new InteractiveBrowserCredential({
+  // Authorization Code Flow is recommended and used by default.
+  // But you can switch batch to the Implicit Grant Flow if you need to:
+  flow: "implicit-grant",
+});
+```
 
 Follow the instructions for [creating your single-page application](https://docs.microsoft.com/azure/active-directory/develop/scenario-spa-app-registration#redirect-uri-msaljs-20-with-auth-code-flow) to correctly mark your redirect URI as enabled for CORS.
 
-If you attempt to use the authorization code flow and see this error:
+When using `InteractiveBrowserCredential` on Node, you may specify a `clientId` and `tenantId`, but otherwise we try to authenticate using a public client that's available for all Azure accounts and the default tenant of your account. For Node, this credential uses a web server to fulfill the redirection. This web server tries to use the port `80` by default. A `redirectUri` can be provided to determine the proper redirection URI with the adequate port, as follows:
+
+```ts
+const credential = new InteractiveBrowserCredential({
+  // You may provide a client ID if you have an application configured.
+  clientId: "my-client-id",
+  // You may provide a tenant ID based on the resource you are trying to access.
+  tenantId: "my-tenant-id",
+  // You may provide a redirectUri based on the redirectUri configured in your AAD application:
+  redirectUri: "http://localhost:8080/"
+});
+```
+
+When using `InteractiveBrowserCredential` on the browser, you will be required to pass a `clientId` in the constructor parameters, such as:
+
+```ts
+// If you've bundled Identity for the browser...
+
+const credential = new InteractiveBrowserCredential({
+  // You MUST provide a client ID if you have an application configured.
+  clientId: "my-client-id",
+  // You may provide a tenant ID based on the resource you are trying to access.
+  tenantId: "my-tenant-id",
+  // You may provide a redirectUri based on the redirectUri configured in your AAD application:
+  redirectUri: "http://localhost:8080/"
+});
+```
+
+Azure Active Directory enterprise applications configured with redirect URIs for `Web` environments are no longer supported by the Authorization Code Flow. You can either configure your AAD application to use Single Page Application redirect URis (type `spa`), or switch to the `implicit-grant` flow.
+
+## CORS error
+
+If you attempt to use the Authorization Code Flow and you get an error similar to this one:
 
 ```
 access to XMLHttpRequest at 'https://login.microsoftonline.com/common/v2.0/oauth2/token' from origin 'yourApp.com' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.
 ```
 
-Then you need to visit your app registration and update the redirect URI for your app to type `spa` (for "single page application").
+Then you need to visit your app registration and update the redirect URI you're using to the type `spa` (for "single page application").
+
+## Sample code
 
-You can see a sample project that uses `InteractiveBrowserCredential` with both [Auth Code Flow][AuthCodeFlow] and [Implicit Grant Flow][ImplicitGrantFlow] here: [link to the sample project](https://github.com/Azure/azure-sdk-for-js/tree/master/sdk/identity/identity/test/manual).
+You can see a sample project that uses `InteractiveBrowserCredential` with both [Authorization Code Flow][AuthCodeFlow] and [Implicit Grant Flow][ImplicitGrantFlow] here: [link to the sample project](https://github.com/Azure/azure-sdk-for-js/tree/master/sdk/identity/identity/test/manual).
 
 [AuthCodeFlow]: https://docs.microsoft.com/azure/active-directory/develop/v2-oauth2-auth-code-flow
 [ImplicitGrantFlow]: https://docs.microsoft.com/azure/active-directory/develop/v2-oauth2-implicit-grant-flow

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

@@ -155,9 +155,13 @@ export class InteractiveBrowserCredential implements TokenCredential {
     }
 
 // @public
-export interface InteractiveBrowserCredentialOptions extends TokenCredentialOptions {
+export interface InteractiveBrowserCredentialBrowserOptions extends InteractiveBrowserCredentialCommonOptions {
+    clientId: string;
+}
+
+// @public
+export interface InteractiveBrowserCredentialCommonOptions extends TokenCredentialOptions {
     authenticationRecord?: AuthenticationRecord;
-    clientId?: string;
     correlationId?: string;
     flow?: InteractiveBrowserAuthenticationFlow;
     loginStyle?: BrowserLoginStyle;
@@ -166,6 +170,11 @@ export interface InteractiveBrowserCredentialOptions extends TokenCredentialOpti
     tenantId?: string;
 }
 
+// @public
+export interface InteractiveBrowserCredentialOptions extends InteractiveBrowserCredentialCommonOptions {
+    clientId?: string;
+}
+
 // @public
 export const logger: AzureLogger;
 

sdk/identity/identity/src/credentials/defaultAzureCredential.browser.ts

@@ -1,27 +1,36 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
+import { AccessToken } from "@azure/core-http";
 import { TokenCredentialOptions } from "../client/identityClient";
+import { credentialLogger, formatError } from "../util/logging";
 import { ChainedTokenCredential } from "./chainedTokenCredential";
-import { InteractiveBrowserCredential } from "./interactiveBrowserCredential";
+
+const BrowserNotSupportedError = new Error(
+  "DefaultAzureCredential is not supported in the browser. Use InteractiveBrowserCredential instead."
+);
+const logger = credentialLogger("DefaultAzureCredential");
 
 /**
  * Provides a default {@link ChainedTokenCredential} configuration for
- * applications that will be deployed to Azure.  The following credential
- * types will be tried, in order:
- *
- * - {@link InteractiveBrowserCredential}
+ * applications that will be deployed to Azure.
  *
- * Consult the documentation of these credential types for more information
- * on how they attempt authentication.
+ * Only available in NodeJS.
  */
 export class DefaultAzureCredential extends ChainedTokenCredential {
   /**
    * Creates an instance of the DefaultAzureCredential class.
    *
    * @param options - Options for configuring the client which makes the authentication request.
    */
-  constructor(tokenCredentialOptions?: TokenCredentialOptions) {
-    super(new InteractiveBrowserCredential(tokenCredentialOptions));
+  constructor(_tokenCredentialOptions?: TokenCredentialOptions) {
+    super();
+    logger.info(formatError("", BrowserNotSupportedError));
+    throw BrowserNotSupportedError;
+  }
+
+  public getToken(): Promise<AccessToken | null> {
+    logger.getToken.info(formatError("", BrowserNotSupportedError));
+    throw BrowserNotSupportedError;
   }
 }

sdk/identity/identity/src/credentials/interactiveBrowserCredential.browser.ts

@@ -5,11 +5,11 @@ import { AccessToken, TokenCredential, GetTokenOptions } from "@azure/core-http"
 import { IdentityClient } from "../client/identityClient";
 import {
   BrowserLoginStyle,
-  InteractiveBrowserCredentialOptions
+  InteractiveBrowserCredentialBrowserOptions
 } from "./interactiveBrowserCredentialOptions";
 import { createSpan } from "../util/tracing";
 import { CanonicalCode } from "@opentelemetry/api";
-import { DefaultTenantId, DeveloperSignOnClientId } from "../constants";
+import { DefaultTenantId } from "../constants";
 import { credentialLogger, formatSuccess, formatError } from "../util/logging";
 import { MSALAuthCode } from "./msalBrowser/msalAuthCode";
 import { MSALImplicit } from "./msalBrowser/msalImplicit";
@@ -35,13 +35,17 @@ export class InteractiveBrowserCredential implements TokenCredential {
    *
    * @param options - Options for configuring the client which makes the authentication request.
    */
-  constructor(options?: InteractiveBrowserCredentialOptions) {
-    this.tenantId = (options && options.tenantId) || DefaultTenantId;
+  constructor(options: InteractiveBrowserCredentialBrowserOptions) {
+    this.tenantId = options.tenantId || DefaultTenantId;
 
-    // TODO: temporary - this is the Azure CLI clientID - we'll replace it when
-    // Developer Sign On application is available
-    // https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/identity/Azure.Identity/src/Constants.cs#L9
-    this.clientId = (options && options.clientId) || DeveloperSignOnClientId;
+    if (!options?.clientId) {
+      const error = new Error(
+        "The parameter `clientId` cannot be left undefined for the `InteractiveBrowserCredential`"
+      );
+      logger.info(formatError("", error));
+      throw error;
+    }
+    this.clientId = options.clientId;
 
     options = {
       ...IdentityClient.getDefaultOptions(),

sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts

@@ -28,16 +28,16 @@ export class InteractiveBrowserCredential implements TokenCredential {
   private port: number;
   private msalClient: MsalClient;
 
-  constructor(options?: InteractiveBrowserCredentialOptions) {
-    const tenantId = (options && options.tenantId) || DefaultTenantId;
-    const clientId = (options && options.clientId) || DeveloperSignOnClientId;
+  constructor(options: InteractiveBrowserCredentialOptions = {}) {
+    const tenantId = options.tenantId || DefaultTenantId;
+    const clientId = options.clientId || DeveloperSignOnClientId;
 
     checkTenantId(logger, tenantId);
 
     // const persistenceEnabled = options?.persistenceEnabled ? options?.persistenceEnabled : false;
     // const authenticationRecord = options?.authenticationRecord;
 
-    if (options && options.redirectUri) {
+    if (options.redirectUri) {
       if (typeof options.redirectUri === "string") {
         this.redirectUri = options.redirectUri;
       } else {
@@ -54,7 +54,7 @@ export class InteractiveBrowserCredential implements TokenCredential {
     }
 
     let authorityHost;
-    if (options && options.authorityHost) {
+    if (options.authorityHost) {
       if (options.authorityHost.endsWith("/")) {
         authorityHost = options.authorityHost + tenantId;
       } else {

sdk/identity/identity/src/credentials/interactiveBrowserCredentialOptions.ts

@@ -23,9 +23,9 @@ export type BrowserLoginStyle = "redirect" | "popup";
 export type InteractiveBrowserAuthenticationFlow = "implicit-grant" | "auth-code";
 
 /**
- * Defines options for the InteractiveBrowserCredential class.
+ * Defines the common options for the InteractiveBrowserCredential class.
  */
-export interface InteractiveBrowserCredentialOptions extends TokenCredentialOptions {
+export interface InteractiveBrowserCredentialCommonOptions extends TokenCredentialOptions {
   /**
    * (Only available if used from a browser)
    * Specifies whether a redirect or a popup window should be used to
@@ -52,11 +52,6 @@ export interface InteractiveBrowserCredentialOptions extends TokenCredentialOpti
    */
   tenantId?: string;
 
-  /**
-   * The client (application) ID of an App Registration in the tenant.
-   */
-  clientId?: string;
-
   /**
    * Correlation ID that can be customized to keep track of the browser authentication requests.
    */
@@ -84,6 +79,29 @@ export interface InteractiveBrowserCredentialOptions extends TokenCredentialOpti
   flow?: InteractiveBrowserAuthenticationFlow;
 }
 
+/**
+ * Defines options for the InteractiveBrowserCredential class for NodeJS.
+ */
+export interface InteractiveBrowserCredentialOptions
+  extends InteractiveBrowserCredentialCommonOptions {
+  /**
+   * The client (application) ID of an App Registration in the tenant.
+   */
+  clientId?: string;
+}
+
+/**
+ * Defines options for the InteractiveBrowserCredential class for the browser.
+ */
+export interface InteractiveBrowserCredentialBrowserOptions
+  extends InteractiveBrowserCredentialCommonOptions {
+  /**
+   * The client (application) ID of an App Registration in the tenant.
+   * This parameter is required on the browser.
+   */
+  clientId: string;
+}
+
 /**
  * Optional parameters to the InteractiveBrowserCredential authenticate() method.
  */

sdk/identity/identity/src/index.ts

@@ -20,6 +20,8 @@ export { AzureCliCredential } from "./credentials/azureCliCredential";
 export { AuthenticationRecord } from "./client/msalClient";
 export {
   InteractiveBrowserCredentialOptions,
+  InteractiveBrowserCredentialBrowserOptions,
+  InteractiveBrowserCredentialCommonOptions,
   BrowserLoginStyle,
   InteractiveBrowserAuthenticationFlow
 } from "./credentials/interactiveBrowserCredentialOptions";