Better error messages from connection failures in MSAL (#22065)

* better error messages from connection failures in MSAL\, fix typo

* cleanup

* pr comments

* rename error interface

* address pr comments

* update api

* fix typings

* change one more typing

* fix build

* fix tests

extensions/azurecore/src/account-provider/auths/azureAuth.ts

@@ -322,7 +322,7 @@ export abstract class AzureAuth implements vscode.Disposable {
 	 * @param azureResource
 	 * @returns The authentication result, including the access token
 	 */
-	public async getTokenMsal(accountId: string, azureResource: azdata.AzureResource, tenantId: string): Promise<AuthenticationResult | null> {
+	public async getTokenMsal(accountId: string, azureResource: azdata.AzureResource, tenantId: string): Promise<AuthenticationResult | azdata.PromptFailedResult | null> {
 		const resource = this.resources.find(s => s.azureResourceId === azureResource);
 		if (!resource) {
 			Logger.error(`Error: Could not fetch the azure resource ${azureResource} `);
@@ -363,11 +363,17 @@ export abstract class AzureAuth implements vscode.Disposable {
 					displayName: ''
 				};
 				return this.handleInteractionRequiredMsal(tenant, resource);
-			} else if (e.name === 'ClientAuthError') {
-				Logger.error(e.message);
+			} else {
+				if (e.name === 'ClientAuthError') {
+					Logger.verbose('[ClientAuthError] Failed to silently acquire token');
+				}
+				return {
+					canceled: false,
+					name: e.name,
+					errorCode: e.errorCode,
+					errorMessage: e.errorMessage || e.message
+				}
 			}
-			Logger.error('Failed to silently acquire token, not InteractionRequiredAuthError');
-			return null;
 		}
 	}
 

extensions/azurecore/src/account-provider/azureAccountProvider.ts

@@ -13,7 +13,7 @@ import {
 	AzureAccount
 } from 'azurecore';
 import { Deferred } from './interfaces';
-import { PublicClientApplication } from '@azure/msal-node';
+import { AuthenticationResult, PublicClientApplication } from '@azure/msal-node';
 import { SimpleTokenCache } from './simpleTokenCache';
 import { Logger } from '../utils/Logger';
 import { MultiTenantTokenResponse, Token, AzureAuth } from './auths/azureAuth';
@@ -148,10 +148,7 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp
 			if (this.authLibrary === Constants.AuthLibrary.MSAL) {
 				tenantId = tenantId || account.properties.owningTenant.id;
 				let authResult = await azureAuth.getTokenMsal(account.key.accountId, resource, tenantId);
-				if (!authResult || !authResult.account || !authResult.account.idTokenClaims) {
-					Logger.error(`MSAL: getToken call failed`);
-					throw Error('Failed to get token');
-				} else {
+				if (this.isAuthenticationResult(authResult) && authResult.account && authResult.account.idTokenClaims) {
 					const token: Token = {
 						key: authResult.account.homeAccountId,
 						token: authResult.accessToken,
@@ -159,6 +156,14 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp
 						expiresOn: authResult.account.idTokenClaims.exp
 					};
 					return token;
+				} else {
+					Logger.error(`MSAL: getToken call failed`);
+					// Throw error with MSAL-specific code/message, else throw generic error message
+					if (this.isProviderError(authResult)) {
+						throw new Error(localize('msalTokenError', `{0} occurred when acquiring token. \n{1}`, authResult.errorCode, authResult.errorMessage));
+					} else {
+						throw new Error(localize('genericTokenError', 'Failed to get token'));
+					}
 				}
 			} else { // fallback to ADAL as default
 				return azureAuth.getAccountSecurityTokenAdal(account, tenantId, resource);
@@ -171,6 +176,23 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp
 		}
 	}
 
+	private isAuthenticationResult(result: AuthenticationResult | azdata.ProviderError | null): result is AuthenticationResult {
+		if (result) {
+			return typeof (<AuthenticationResult>result).accessToken === 'string';
+		} else {
+			return false;
+		}
+	}
+
+	private isProviderError(result: AuthenticationResult | azdata.ProviderError | null): result is azdata.ProviderError {
+		if (result) {
+			return typeof (<azdata.ProviderError>result).errorMessage === 'string';
+		} else {
+			return false;
+		}
+	}
+
+
 	private async _getSecurityToken(account: AzureAccount, resource: azdata.AzureResource): Promise<MultiTenantTokenResponse | undefined> {
 		void vscode.window.showInformationMessage(localize('azure.deprecatedGetSecurityToken', "A call was made to azdata.accounts.getSecurityToken, this method is deprecated and will be removed in future releases. Please use getAccountSecurityToken instead."));
 		const azureAccount = account as AzureAccount;

src/sql/azdata.proposed.d.ts

@@ -432,18 +432,26 @@ declare module 'azdata' {
 		azurePortalEndpoint?: string;
 	}
 
-	export interface PromptFailedResult {
+	export interface PromptFailedResult extends ProviderError { }
+
+	export interface ProviderError {
 		/**
-		 * Error code used for non-user cancelled sign in errors
+		 * Error name
+		 */
+		name?: string;
+
+		/**
+		 * Error code
 		 */
 		errorCode?: string;
 
 		/**
-		 * Error message used for non-user cancelled sign in errors
+		 * Error message
 		 */
 		errorMessage?: string;
 	}
 
+
 	export namespace diagnostics {
 		/**
 		 * Represents a diagnostics provider of accounts.

src/sql/workbench/services/accountManagement/browser/accountManagementService.ts

@@ -128,7 +128,7 @@ export class AccountManagementService implements IAccountManagementService {
 	 */
 	public addAccount(providerId: string): Promise<void> {
 		const closeAction: Action = new Action('closeAddingAccount', localize('accountManagementService.close', "Close"), undefined, true);
-
+		const genericAccountErrorMessage = localize('addAccountFailedGenericMessage', 'Adding account failed, check Azure Accounts log for more info.')
 		const loginNotification: INotification = {
 			severity: Severity.Info,
 			message: localize('loggingIn', "Adding account..."),
@@ -148,13 +148,19 @@ export class AccountManagementService implements IAccountManagementService {
 					if (accountResult.canceled === true) {
 						return;
 					} else {
-						throw new Error(localize('addAccountFailedMessage', `${0} \nError Message: ${1}`, accountResult.errorCode, accountResult.errorMessage));
+						if (accountResult.errorCode && accountResult.errorMessage) {
+							throw new Error(localize('addAccountFailedCodeMessage', `{0} \nError Message: {1}`, accountResult.errorCode, accountResult.errorMessage));
+						} else if (accountResult.errorMessage) {
+							throw new Error(localize('addAccountFailedMessage', `{0}`, accountResult.errorMessage));
+						} else {
+							throw new Error(genericAccountErrorMessage);
+						}
 					}
 				}
 				let result = await this._accountStore.addOrUpdate(accountResult);
 				if (!result) {
 					this._logService.error('adding account failed');
-					throw new Error(localize('addAccountFailedGeneric', 'Adding account failed, check Azure Accounts log for more info.'));
+					throw new Error(genericAccountErrorMessage);
 				}
 				if (result.accountAdded) {
 					// Add the account to the list

src/sql/workbench/services/connection/browser/connectionWidget.ts

@@ -561,7 +561,7 @@ export class ConnectionWidget extends lifecycle.Disposable {
 		if (this._azureAccountDropdown) {
 			this._register(styler.attachSelectBoxStyler(this._azureAccountDropdown, this._themeService));
 			this._register(this._azureAccountDropdown.onDidSelect(() => {
-				this.onAzureAccountSelected().catch(err => this._logService.error(`Unexpeted error handling Azure Account dropdown click : ${err}`));
+				this.onAzureAccountSelected().catch(err => this._logService.error(`Unexpected error handling Azure Account dropdown click : ${err}`));
 			}));
 		}