Improve error handling when acquiring aad tokens (#1746)

* Mostly working - some cosmetic changes remaining.

* Cosmetic changes and other tidy ups.

* More clean up.

* Move msal back to dependencies. Fix typo.

* msal should be prod dependency

* Revert msal package update as it is causing issues with unit test
execution.

* Add tracing for unhandled exceptions when acquiring tokens.

.vscode/settings.json

@@ -20,8 +20,8 @@
   "typescript.tsdk": "node_modules/typescript/lib",
   "editor.formatOnSave": true,
   "editor.codeActionsOnSave": {
-    "source.fixAll.eslint": true,
+    "source.fixAll.eslint": "explicit",
-    "source.organizeImports": true
+    "source.organizeImports": "explicit"
   },
   "typescript.preferences.importModuleSpecifier": "non-relative",
   "editor.defaultFormatter": "esbenp.prettier-vscode"

package.json

@@ -78,8 +78,8 @@
     "mkdirp": "1.0.4",
     "monaco-editor": "0.44.0",
     "ms": "2.1.3",
-    "patch-package": "8.0.0",
     "p-retry": "4.6.2",
+    "patch-package": "8.0.0",
     "plotly.js-cartesian-dist-min": "1.52.3",
     "post-robot": "10.0.42",
     "q": "1.5.1",

src/HostedExplorer.tsx

@@ -1,5 +1,6 @@
 import { initializeIcons } from "@fluentui/react";
 import { useBoolean } from "@fluentui/react-hooks";
+import { AadAuthorizationFailure } from "Platform/Hosted/Components/AadAuthorizationFailure";
 import * as React from "react";
 import { render } from "react-dom";
 import ChevronRight from "../images/chevron-right.svg";
@@ -32,7 +33,8 @@ const App: React.FunctionComponent = () => {
   // For showing/hiding panel
   const [isOpen, { setTrue: openPanel, setFalse: dismissPanel }] = useBoolean(false);
   const config = useConfig();
-  const { isLoggedIn, armToken, graphToken, account, tenantId, logout, login, switchTenant } = useAADAuth();
+  const { isLoggedIn, armToken, graphToken, account, tenantId, logout, login, switchTenant, authFailure } =
+    useAADAuth();
   const [databaseAccount, setDatabaseAccount] = React.useState<DatabaseAccount>();
   const [authType, setAuthType] = React.useState<AuthType>(encryptedToken ? AuthType.EncryptedToken : undefined);
   const [connectionString, setConnectionString] = React.useState<string>();
@@ -136,7 +138,10 @@ const App: React.FunctionComponent = () => {
       {!isLoggedIn && !encryptedTokenMetadata && (
         <ConnectExplorer {...{ login, setEncryptedToken, setAuthType, connectionString, setConnectionString }} />
       )}
-      {isLoggedIn && <DirectoryPickerPanel {...{ isOpen, dismissPanel, armToken, tenantId, switchTenant }} />}
+      {isLoggedIn && authFailure && <AadAuthorizationFailure {...{ authFailure }} />}
+      {isLoggedIn && !authFailure && (
+        <DirectoryPickerPanel {...{ isOpen, dismissPanel, armToken, tenantId, switchTenant }} />
+      )}
     </>
   );
 };

src/Platform/Hosted/AadAuthorizationFailure.less

@@ -0,0 +1,52 @@
+.aadAuthFailureContainer {
+  height: 100%;
+  width: 100%;
+}
+.aadAuthFailureContainer .aadAuthFailureFormContainer {
+  display: -webkit-flex;
+  display: -ms-flexbox;
+  display: -ms-flex;
+  display: flex;
+  -webkit-flex-direction: column;
+  -ms-flex-direction: column;
+  flex-direction: column;
+  height: 100%;
+  width: 100%;
+}
+.aadAuthFailureContainer .aadAuthFailure {
+  text-align: center;
+  display: -webkit-flex;
+  display: -ms-flexbox;
+  display: -ms-flex;
+  display: flex;
+  -webkit-flex-direction: column;
+  -ms-flex-direction: column;
+  flex-direction: column;
+  justify-content: center;
+  height: 100%;
+  margin-bottom: 60px;
+}
+.aadAuthFailureContainer .aadAuthFailure .authFailureTitle {
+  font-size: 16px;
+  font-weight: 500;
+  color: #d12d2d;
+  margin: 16px 8px 8px 8px;
+}
+.aadAuthFailureContainer .aadAuthFailure .authFailureMessage {
+  font-size: 14px;
+  color: #393939;
+  margin: 16px 16px 16px 16px;
+  word-wrap: break-word;
+  white-space: pre-wrap;
+}
+.aadAuthFailureContainer .aadAuthFailure .authFailureLink {
+  margin: 8px;
+  font-size: 14px;
+  color: #0058ad;
+  cursor: pointer;
+}
+
+.aadAuthFailureContainer .aadAuthFailure .aadAuthFailureContent {
+  margin: 8px;
+  color: #393939;
+}

src/Platform/Hosted/Components/AadAuthorizationFailure.tsx

@@ -0,0 +1,29 @@
+import { AadAuthFailure } from "hooks/useAADAuth";
+import * as React from "react";
+import ConnectImage from "../../../../images/HdeConnectCosmosDB.svg";
+import "../AadAuthorizationFailure.less";
+
+interface Props {
+  authFailure: AadAuthFailure;
+}
+
+export const AadAuthorizationFailure: React.FunctionComponent<Props> = ({ authFailure }: Props) => {
+  return (
+    <div id="aadAuthFailure" className="aadAuthFailureContainer" style={{ display: "flex" }}>
+      <div className="aadAuthFailureFormContainer">
+        <div className="aadAuthFailure">
+          <p className="aadAuthFailureContent">
+            <img src={ConnectImage} alt="Azure Cosmos DB" />
+          </p>
+          <p className="authFailureTitle">Authorization Failure</p>
+          <p className="authFailureMessage">{authFailure.failureMessage}</p>
+          {authFailure.failureLinkTitle && (
+            <p className="authFailureLink" onClick={authFailure.failureLinkAction}>
+              {authFailure.failureLinkTitle}
+            </p>
+          )}
+        </div>
+      </div>
+    </div>
+  );
+};

src/Utils/AuthorizationUtils.ts

@@ -1,9 +1,11 @@
 import * as msal from "@azure/msal-browser";
+import { Action } from "Shared/Telemetry/TelemetryConstants";
 import { AuthType } from "../AuthType";
 import * as Constants from "../Common/Constants";
 import * as Logger from "../Common/Logger";
 import { configContext } from "../ConfigContext";
 import * as ViewModels from "../Contracts/ViewModels";
+import { traceFailure } from "../Shared/Telemetry/TelemetryProcessor";
 import { userContext } from "../UserContext";
 
 export function getAuthorizationHeader(): ViewModels.AuthorizationTokenHeaderMetadata {
@@ -43,8 +45,8 @@ export function decryptJWTToken(token: string) {
   return JSON.parse(tokenPayload);
 }
 
-export function getMsalInstance() {
+export async function getMsalInstance() {
-  const config: msal.Configuration = {
+  const msalConfig: msal.Configuration = {
     cache: {
       cacheLocation: "localStorage",
     },
@@ -55,8 +57,46 @@ export function getMsalInstance() {
   };
 
   if (process.env.NODE_ENV === "development") {
-    config.auth.redirectUri = "https://dataexplorer-dev.azurewebsites.net";
+    msalConfig.auth.redirectUri = "https://dataexplorer-dev.azurewebsites.net";
   }
-  const msalInstance = new msal.PublicClientApplication(config);
+
+  const msalInstance = new msal.PublicClientApplication(msalConfig);
   return msalInstance;
 }
+
+export async function acquireTokenWithMsal(msalInstance: msal.IPublicClientApplication, request: msal.SilentRequest) {
+  const tokenRequest = {
+    account: msalInstance.getActiveAccount() || null,
+    ...request,
+  };
+
+  try {
+    // attempt silent acquisition first
+    return (await msalInstance.acquireTokenSilent(tokenRequest)).accessToken;
+  } catch (silentError) {
+    if (silentError instanceof msal.InteractionRequiredAuthError) {
+      try {
+        // The error indicates that we need to acquire the token interactively.
+        // This will display a pop-up to re-establish authorization. If user does not
+        // have pop-ups enabled in their browser, this will fail.
+        return (await msalInstance.acquireTokenPopup(tokenRequest)).accessToken;
+      } catch (interactiveError) {
+        traceFailure(Action.SignInAad, {
+          request: JSON.stringify(tokenRequest),
+          acquireTokenType: "interactive",
+          errorMessage: JSON.stringify(interactiveError),
+        });
+
+        throw interactiveError;
+      }
+    } else {
+      traceFailure(Action.SignInAad, {
+        request: JSON.stringify(tokenRequest),
+        acquireTokenType: "silent",
+        errorMessage: JSON.stringify(silentError),
+      });
+
+      throw silentError;
+    }
+  }
+}

src/hooks/useAADAuth.ts

@@ -2,9 +2,9 @@ import * as msal from "@azure/msal-browser";
 import { useBoolean } from "@fluentui/react-hooks";
 import * as React from "react";
 import { configContext } from "../ConfigContext";
-import { getMsalInstance } from "../Utils/AuthorizationUtils";
+import { acquireTokenWithMsal, getMsalInstance } from "../Utils/AuthorizationUtils";
 
-const msalInstance = getMsalInstance();
+const msalInstance = await getMsalInstance();
 
 const cachedAccount = msalInstance.getAllAccounts()?.[0];
 const cachedTenantId = localStorage.getItem("cachedTenantId");
@@ -18,6 +18,13 @@ interface ReturnType {
   tenantId: string;
   account: msal.AccountInfo;
   switchTenant: (tenantId: string) => void;
+  authFailure: AadAuthFailure;
+}
+
+export interface AadAuthFailure {
+  failureMessage: string;
+  failureLinkTitle?: string;
+  failureLinkAction?: () => void;
 }
 
 export function useAADAuth(): ReturnType {
@@ -28,6 +35,7 @@ export function useAADAuth(): ReturnType {
   const [tenantId, setTenantId] = React.useState<string>(cachedTenantId);
   const [graphToken, setGraphToken] = React.useState<string>();
   const [armToken, setArmToken] = React.useState<string>();
+  const [authFailure, setAuthFailure] = React.useState<AadAuthFailure>(undefined);
 
   msalInstance.setActiveAccount(account);
   const login = React.useCallback(async () => {
@@ -61,24 +69,60 @@ export function useAADAuth(): ReturnType {
     [account, tenantId],
   );
 
-  React.useEffect(() => {
+  const acquireTokens = React.useCallback(async () => {
-    if (account && tenantId) {
+    if (!(account && tenantId)) {
-      Promise.all([
+      return;
-        msalInstance.acquireTokenSilent({
+    }
-          authority: `${configContext.AAD_ENDPOINT}${tenantId}`,
+
-          scopes: [`${configContext.GRAPH_ENDPOINT}/.default`],
+    try {
-        }),
+      const armToken = await acquireTokenWithMsal(msalInstance, {
-        msalInstance.acquireTokenSilent({
         authority: `${configContext.AAD_ENDPOINT}${tenantId}`,
         scopes: [`${configContext.ARM_ENDPOINT}/.default`],
-        }),
+      });
-      ]).then(([graphTokenResponse, armTokenResponse]) => {
+
-        setGraphToken(graphTokenResponse.accessToken);
+      setArmToken(armToken);
-        setArmToken(armTokenResponse.accessToken);
+      setAuthFailure(null);
+    } catch (error) {
+      if (error instanceof msal.AuthError && error.errorCode === msal.BrowserAuthErrorMessage.popUpWindowError.code) {
+        // This error can occur when acquireTokenWithMsal() has attempted to acquire token interactively
+        // and user has popups disabled in browser. This fails as the popup is not the result of a explicit user
+        // action. In this case, we display the failure and a link to repeat the operation. Clicking on the
+        // link is a user action so it will work even if popups have been disabled.
+        // See: https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/76#issuecomment-324787539
+        setAuthFailure({
+          failureMessage:
+            "We were unable to establish authorization for this account, due to pop-ups being disabled in the browser.\nPlease click below to retry authorization without requiring popups being enabled.",
+          failureLinkTitle: "Retry Authorization",
+          failureLinkAction: acquireTokens,
+        });
+      } else {
+        const errorJson = JSON.stringify(error);
+        setAuthFailure({
+          failureMessage: `We were unable to establish authorization for this account, due to the following error: \n${errorJson}`,
         });
       }
+    }
+
+    try {
+      const graphToken = await acquireTokenWithMsal(msalInstance, {
+        authority: `${configContext.AAD_ENDPOINT}${tenantId}`,
+        scopes: [`${configContext.GRAPH_ENDPOINT}/.default`],
+      });
+
+      setGraphToken(graphToken);
+    } catch (error) {
+      // Graph token is used only for retrieving user photo at the moment, so
+      // it's not critical if this fails.
+      console.warn("Error acquiring graph token: " + error);
+    }
   }, [account, tenantId]);
 
+  React.useEffect(() => {
+    if (account && tenantId && !authFailure) {
+      acquireTokens();
+    }
+  }, [account, tenantId, acquireTokens, authFailure]);
+
   return {
     account,
     tenantId,
@@ -88,5 +132,6 @@ export function useAADAuth(): ReturnType {
     login,
     logout,
     switchTenant,
+    authFailure,
   };
 }

src/hooks/useKnockoutExplorer.ts

@@ -6,6 +6,7 @@ import { useCommandBar } from "Explorer/Menus/CommandBar/CommandBarComponentAdap
 import { useSelectedNode } from "Explorer/useSelectedNode";
 import { scheduleRefreshDatabaseResourceToken } from "Platform/Fabric/FabricUtil";
 import { getNetworkSettingsWarningMessage } from "Utils/NetworkUtility";
+import { logConsoleError } from "Utils/NotificationConsoleUtils";
 import { useQueryCopilot } from "hooks/useQueryCopilot";
 import { ReactTabKind, useTabs } from "hooks/useTabs";
 import { useEffect, useState } from "react";
@@ -35,7 +36,7 @@ import {
 import { extractFeatures } from "../Platform/Hosted/extractFeatures";
 import { DefaultExperienceUtility } from "../Shared/DefaultExperienceUtility";
 import { Node, PortalEnv, updateUserContext, userContext } from "../UserContext";
-import { getAuthorizationHeader, getMsalInstance } from "../Utils/AuthorizationUtils";
+import { acquireTokenWithMsal, getAuthorizationHeader, getMsalInstance } from "../Utils/AuthorizationUtils";
 import { isInvalidParentFrameOrigin, shouldProcessMessage } from "../Utils/MessageValidation";
 import { listKeys } from "../Utils/arm/generatedClients/cosmos/databaseAccounts";
 import { DatabaseAccountListKeysResult } from "../Utils/arm/generatedClients/cosmos/types";
@@ -243,16 +244,19 @@ async function configureHostedWithAAD(config: AAD): Promise<Explorer> {
   let keys: DatabaseAccountListKeysResult = {};
   if (account.properties?.documentEndpoint) {
     const hrefEndpoint = new URL(account.properties.documentEndpoint).href.replace(/\/$/, "/.default");
-    const msalInstance = getMsalInstance();
+    const msalInstance = await getMsalInstance();
     const cachedAccount = msalInstance.getAllAccounts()?.[0];
     msalInstance.setActiveAccount(cachedAccount);
     const cachedTenantId = localStorage.getItem("cachedTenantId");
-    const aadTokenResponse = await msalInstance.acquireTokenSilent({
+    try {
+      aadToken = await acquireTokenWithMsal(msalInstance, {
         forceRefresh: true,
         scopes: [hrefEndpoint],
         authority: `${configContext.AAD_ENDPOINT}${cachedTenantId}`,
       });
-    aadToken = aadTokenResponse.accessToken;
+    } catch (authError) {
+      logConsoleError("Failed to acquire authorization token: " + authError);
+    }
   }
   try {
     if (!account.properties.disableLocalAuth) {