Show cancellable, long running notification when signing in (#480)

src/cloudConsole/ipc.ts

@@ -5,10 +5,12 @@
 
 'use strict';
 
+import { parseError } from '@microsoft/vscode-azext-utils';
 import * as crypto from 'crypto';
 import * as http from 'http';
 import * as os from 'os';
 import * as path from 'path';
+import { ext } from '../extensionVariables';
 
 export async function createServer(ipcHandlePrefix: string, onRequest: http.RequestListener): Promise<Server> {
 	const buffer = await randomBytes(20);
@@ -37,7 +39,7 @@ export class Server {
 	}
 
 	dispose(): void {
-		this.server.close();
+		this.server.close(error => error && ext.outputChannel.appendLog(parseError(error).message));
 	}
 }
 

src/constants.ts

@@ -4,6 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { Environment } from "@azure/ms-rest-azure-env";
+import { localize } from "./utils/localize";
 
 export type AuthLibrary = 'ADAL' | 'MSAL';
 
@@ -16,6 +17,8 @@ export const resourceFilterSetting: string = 'resourceFilter';
 export const showSignedInEmailSetting: string = 'showSignedInEmail';
 export const tenantSetting: string = 'tenant';
 
+export const authTimeoutSeconds: number = 5 * 60;
+export const authTimeoutMs: number = authTimeoutSeconds * 1000;
 export const azureCustomCloud: string = 'AzureCustomCloud';
 export const azurePPE: string = 'AzurePPE';
 export const cacheKey: string = 'cache';
@@ -25,10 +28,20 @@ export const displayName: string = 'Azure Account';
 export const redirectUrlAAD: string = 'https://vscode-redirect.azurewebsites.net/';
 export const portADFS: number = 19472;
 export const redirectUrlADFS: string = `http://127.0.0.1:${portADFS}/callback`;
+export const stoppedAuthTaskMessage: string = localize('azure-account.stoppedAuthTask', 'Stopped authentication task.');
 
 export const staticEnvironments: Environment[] = [
 	Environment.AzureCloud,
 	Environment.ChinaCloud,
 	Environment.GermanCloud,
 	Environment.USGovernment
 ];
+
+export const environmentLabels: Record<string, string> = {
+	AzureCloud: localize('azure-account.azureCloud', 'Azure'),
+	AzureChinaCloud: localize('azure-account.azureChinaCloud', 'Azure China'),
+	AzureGermanCloud: localize('azure-account.azureGermanyCloud', 'Azure Germany'),
+	AzureUSGovernment: localize('azure-account.azureUSCloud', 'Azure US Government'),
+	[azureCustomCloud]: localize('azure-account.azureCustomCloud', 'Azure Custom Cloud'),
+	[azurePPE]: localize('azure-account.azurePPE', 'Azure PPE'),
+};

src/login/AuthProviderBase.ts

@@ -6,13 +6,14 @@
 import { TokenCredential } from "@azure/core-auth";
 import { Environment } from "@azure/ms-rest-azure-env";
 import { AccountInfo } from "@azure/msal-node";
-import { parseError } from "@microsoft/vscode-azext-utils";
+import { IActionContext, parseError } from "@microsoft/vscode-azext-utils";
 import { randomBytes } from "crypto";
 import { ServerResponse } from "http";
 import { DeviceTokenCredentials } from "ms-rest-azure";
-import { env, MessageItem, UIKind, Uri, window } from "vscode";
+import { CancellationToken, env, MessageItem, UIKind, Uri, window } from "vscode";
 import { AzureAccountExtensionApi, AzureSession } from "../azure-account.api";
 import { redirectUrlAAD, redirectUrlADFS } from "../constants";
+import { ext } from "../extensionVariables";
 import { localize } from "../utils/localize";
 import { logErrorMessage } from "../utils/logErrorMessage";
 import { openUri } from "../utils/openUri";
@@ -31,14 +32,14 @@ export abstract class AuthProviderBase<TLoginResult> {
 	private terminateServer: (() => Promise<void>) | undefined;
 
 	public abstract loginWithAuthCode(code: string, redirectUrl: string, clientId: string, environment: Environment, tenantId: string): Promise<TLoginResult>;
-	public abstract loginWithDeviceCode(environment: Environment, tenantId: string): Promise<TLoginResult>;
+	public abstract loginWithDeviceCode(context: IActionContext, environment: Environment, tenantId: string, cancellationToken: CancellationToken): Promise<TLoginResult>;
 	public abstract loginSilent(environment: Environment, tenantId: string): Promise<TLoginResult>;
 	public abstract getCredentials(environment: string, userId: string, tenantId: string): AbstractCredentials;
 	public abstract getCredentials2(environment: Environment, userId: string, tenantId: string, accountInfo?: AccountInfo): AbstractCredentials2;
 	public abstract updateSessions(environment: Environment, loginResult: TLoginResult, sessions: AzureSession[]): Promise<void>;
 	public abstract clearTokenCache(): Promise<void>;
 
-	public async login(clientId: string, environment: Environment, isAdfs: boolean, tenantId: string, openUri: (url: string) => Promise<void>, redirectTimeout: () => Promise<void>): Promise<TLoginResult> {
+	public async login(context: IActionContext, clientId: string, environment: Environment, isAdfs: boolean, tenantId: string, openUri: (url: string) => Promise<void>, redirectTimeout: () => Promise<void>, cancellationToken: CancellationToken): Promise<TLoginResult> {
 		if (env.uiKind === UIKind.Web) {
 			return await this.loginWithoutLocalServer(clientId, environment, isAdfs, tenantId);
 		}
@@ -48,7 +49,14 @@ export abstract class AuthProviderBase<TLoginResult> {
 		}
 
 		const nonce: string = randomBytes(16).toString('base64');
-		const { server, redirectPromise, codePromise } = createServer(nonce);
+		const { server, redirectPromise, codePromise, codeTimer } = createServer(context, nonce);
+
+		cancellationToken.onCancellationRequested(() => {
+			server.close(error => error && ext.outputChannel.appendLog(parseError(error).message));
+			clearTimeout(codeTimer);
+			context.telemetry.properties.serverClosed = 'true';
+			ext.outputChannel.appendLog(localize('azure-account.authProcessCancelled', 'Authentication process cancelled.'));
+		});
 
 		if (isAdfs) {
 			this.terminateServer = createTerminateServer(server);
@@ -102,7 +110,7 @@ export abstract class AuthProviderBase<TLoginResult> {
 			}
 		} finally {
 			setTimeout(() => {
-				server.close();
+				server.close(error => error && ext.outputChannel.appendLog(parseError(error).message));
 			}, 5000);
 		}
 	}
@@ -160,8 +168,6 @@ export abstract class AuthProviderBase<TLoginResult> {
 		if (response === copyAndOpen) {
 			void env.clipboard.writeText(userCode);
 			await openUri(verificationUrl);
-		} else {
-			return Promise.reject('user canceled');
 		}
 	}
 }

src/login/AzureLoginHelper.ts

@@ -5,9 +5,9 @@
 
 import { Environment } from '@azure/ms-rest-azure-env';
 import { callWithTelemetryAndErrorHandling, IActionContext, registerCommand } from '@microsoft/vscode-azext-utils';
-import { CancellationTokenSource, commands, EventEmitter, ExtensionContext, MessageItem, window, workspace } from 'vscode';
+import { CancellationTokenSource, commands, EventEmitter, ExtensionContext, MessageItem, ProgressLocation, window, workspace } from 'vscode';
 import { AzureLoginStatus, AzureResourceFilter, AzureSession, AzureSubscription } from '../azure-account.api';
-import { AuthLibrary, authLibrarySetting, cacheKey, clientId, commonTenantId, resourceFilterSetting, tenantSetting } from '../constants';
+import { AuthLibrary, authLibrarySetting, cacheKey, clientId, commonTenantId, environmentLabels, resourceFilterSetting, tenantSetting } from '../constants';
 import { AzureLoginError, getErrorMessage } from '../errors';
 import { ext } from '../extensionVariables';
 import { localize } from '../utils/localize';
@@ -95,42 +95,56 @@ export class AzureAccountLoginHelper {
 	}
 
 	public async login(context: IActionContext, trigger: LoginTrigger): Promise<void> {
-		await ext.loginHelper.logout();
+		await ext.loginHelper.logout(true /* forceLogout */);
 
 		let codePath: CodePath = 'newLogin';
 		let environmentName: string = 'uninitialized';
 		const cancelSource: CancellationTokenSource = new CancellationTokenSource();
 		try {
 			const environment: Environment = await getSelectedEnvironment();
 			environmentName = environment.name;
-			const onlineTask: Promise<void> = waitUntilOnline(environment, 2000, cancelSource.token);
-			const timerTask: Promise<boolean | PromiseLike<boolean> | undefined> = delay(2000, true);
+			const environmentLabel: string = environmentLabels[environmentName] || localize('azure-account.unknownCloud', 'unknown cloud');
 
-			if (await Promise.race([onlineTask, timerTask])) {
-				const cancel: MessageItem = { title: localize('azure-account.cancel', "Cancel") };
-				await Promise.race([
-					onlineTask,
-					window.showInformationMessage(localize('azure-account.checkNetwork', "You appear to be offline. Please check your network connection."), cancel)
-						.then(result => {
-							if (result === cancel) {
-								throw new AzureLoginError(localize('azure-account.offline', "Offline"));
-							}
-						})
-				]);
-				await onlineTask;
-			}
+			await window.withProgress({
+				title: localize('azure-account.signingIn', 'Signing in to {0}...', environmentLabel), 
+				location: ProgressLocation.Notification,
+				cancellable: true
+			}, async (_progress, cancellationToken) => {
+				cancellationToken.onCancellationRequested(async () => {
+					await this.logout(true /* forceLogout */);
+					context.telemetry.properties.signInCancelled = 'true';
+					ext.outputChannel.appendLog(localize('azure-account.signInCancelled', 'Sign in cancelled.'));
+				});
+
+				const onlineTask: Promise<void> = waitUntilOnline(environment, 2000, cancelSource.token);
+				const timerTask: Promise<boolean | PromiseLike<boolean> | undefined> = delay(2000, true);
 
-			this.beginLoggingIn();
+				if (await Promise.race([onlineTask, timerTask])) {
+					const cancel: MessageItem = { title: localize('azure-account.cancel', "Cancel") };
+					await Promise.race([
+						onlineTask,
+						window.showInformationMessage(localize('azure-account.checkNetwork', "You appear to be offline. Please check your network connection."), cancel)
+							.then(result => {
+								if (result === cancel) {
+									throw new AzureLoginError(localize('azure-account.offline', "Offline"));
+								}
+							})
+					]);
+					await onlineTask;
+				}
+
+				this.beginLoggingIn();
 
-			const tenantId: string = getSettingValue(tenantSetting) || commonTenantId;
-			const isAdfs: boolean = isADFS(environment);
-			const useCodeFlow: boolean = trigger !== 'loginWithDeviceCode' && await checkRedirectServer(isAdfs);
-			codePath = useCodeFlow ? 'newLoginCodeFlow' : 'newLoginDeviceCode';
-			const loginResult = useCodeFlow ?
-				await this.authProvider.login(clientId, environment, isAdfs, tenantId, openUri, redirectTimeout) :
-				await this.authProvider.loginWithDeviceCode(environment, tenantId);
-			await this.updateSessions(this.authProvider, environment, loginResult);
-			void this.sendLoginTelemetry(context, { trigger, codePath, environmentName, outcome: 'success' }, true);
+				const tenantId: string = getSettingValue(tenantSetting) || commonTenantId;
+				const isAdfs: boolean = isADFS(environment);
+				const useCodeFlow: boolean = trigger !== 'loginWithDeviceCode' && await checkRedirectServer(isAdfs);
+				codePath = useCodeFlow ? 'newLoginCodeFlow' : 'newLoginDeviceCode';
+				const loginResult = useCodeFlow ?
+					await this.authProvider.login(context, clientId, environment, isAdfs, tenantId, openUri, redirectTimeout, cancellationToken) :
+					await this.authProvider.loginWithDeviceCode(context, environment, tenantId, cancellationToken);
+				await this.updateSessions(this.authProvider, environment, loginResult);
+				void this.sendLoginTelemetry(context, { trigger, codePath, environmentName, outcome: 'success' }, true);
+			});
 		} catch (err) {
 			if (err instanceof AzureLoginError && err.reason) {
 				ext.outputChannel.appendLog(err.reason);
@@ -157,8 +171,10 @@ export class AzureAccountLoginHelper {
 		}
 	}
 
-	public async logout(): Promise<void> {
-		await this.api.waitForLogin();
+	public async logout(forceLogout?: boolean): Promise<void> {
+		if (!forceLogout) {
+			await this.api.waitForLogin();
+		}
 		await this.clearSessions();
 		this.updateLoginStatus();
 	}

src/login/adal/AdalAuthProvider.ts

@@ -4,14 +4,16 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { Environment } from "@azure/ms-rest-azure-env";
+import { IActionContext } from "@microsoft/vscode-azext-utils";
 import { Logging, MemoryCache, TokenResponse, UserCodeInfo } from "adal-node";
 import { DeviceTokenCredentials } from "ms-rest-azure";
+import { CancellationToken } from "vscode";
 import { AzureSession } from "../../azure-account.api";
-import { azureCustomCloud, azurePPE, clientId, staticEnvironments } from "../../constants";
+import { authTimeoutMs, azureCustomCloud, azurePPE, clientId, staticEnvironments, stoppedAuthTaskMessage } from "../../constants";
 import { AzureLoginError } from "../../errors";
 import { ext } from "../../extensionVariables";
 import { localize } from "../../utils/localize";
-import { timeout } from "../../utils/timeUtils";
+import { Deferred } from "../../utils/promiseUtils";
 import { AbstractCredentials, AuthProviderBase } from "../AuthProviderBase";
 import { DeviceTokenCredentials2 } from "./DeviceTokenCredentials2";
 import { getUserCode } from "./getUserCode";
@@ -53,11 +55,27 @@ export class AdalAuthProvider extends AuthProviderBase<TokenResponse[]> {
 		return getTokensFromToken(environment, tenantId, tokenResponse);
 	}
 
-	public async loginWithDeviceCode(environment: Environment, tenantId: string): Promise<TokenResponse[]> {
+	public async loginWithDeviceCode(context: IActionContext, environment: Environment, tenantId: string, cancellationToken: CancellationToken): Promise<TokenResponse[]> {
+		// Used for prematurely ending the `tokenResponseTask`.
+		let deferredTaskRegulator: Deferred<TokenResponse>;
+		const taskRegulator = new Promise<TokenResponse>((resolve, reject) => deferredTaskRegulator = { resolve, reject });
+
+		const timeout = setTimeout(() => {
+			context.errorHandling.suppressDisplay = true;
+			deferredTaskRegulator.reject(new Error(localize('azure-account.timeoutWaitingForDeviceCode', 'Timeout waiting for device code.')));
+		}, authTimeoutMs);
+
+		cancellationToken.onCancellationRequested(() => {
+			clearTimeout(timeout);
+			deferredTaskRegulator.reject(new Error(stoppedAuthTaskMessage));
+		});
+
 		const userCode: UserCodeInfo = await getUserCode(environment, tenantId);
 		const messageTask: Promise<void> = this.showDeviceCodeMessage(userCode.message, userCode.userCode, userCode.verificationUrl);
 		const tokenResponseTask: Promise<TokenResponse> = getTokenResponse(environment, tenantId, userCode);
-		const tokenResponse: TokenResponse = await Promise.race([tokenResponseTask, messageTask.then(() => Promise.race([tokenResponseTask, timeout(3 * 60 * 1000)]))]); // 3 minutes
+		const tokenResponse: TokenResponse = await Promise.race([tokenResponseTask, messageTask.then(() => Promise.race([tokenResponseTask, taskRegulator]))]);
+
+		clearTimeout(timeout);
 
 		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 		await storeRefreshToken(environment, tokenResponse.refreshToken!);

src/login/commands/loginToCloud.ts

@@ -6,21 +6,12 @@
 import { Environment } from "@azure/ms-rest-azure-env";
 import { IActionContext } from "@microsoft/vscode-azext-utils";
 import { QuickPickItem, window, workspace, WorkspaceConfiguration } from "vscode";
-import { azureCustomCloud, azurePPE, cloudSetting, commonTenantId, customCloudArmUrlSetting, extensionPrefix, tenantSetting } from "../../constants";
+import { azureCustomCloud, cloudSetting, commonTenantId, customCloudArmUrlSetting, environmentLabels, extensionPrefix, tenantSetting } from "../../constants";
 import { ext } from "../../extensionVariables";
 import { localize } from "../../utils/localize";
 import { getEnvironments, getSelectedEnvironment } from "../environments";
 import { getCurrentTarget } from "../getCurrentTarget";
 
-const environmentLabels: Record<string, string> = {
-	AzureCloud: localize('azure-account.azureCloud', 'Azure'),
-	AzureChinaCloud: localize('azure-account.azureChinaCloud', 'Azure China'),
-	AzureGermanCloud: localize('azure-account.azureGermanyCloud', 'Azure Germany'),
-	AzureUSGovernment: localize('azure-account.azureUSCloud', 'Azure US Government'),
-	[azureCustomCloud]: localize('azure-account.azureCustomCloud', 'Azure Custom Cloud'),
-	[azurePPE]: localize('azure-account.azurePPE', 'Azure PPE'),
-};
-
 export async function loginToCloud(context: IActionContext): Promise<void> {
 	const current: Environment = await getSelectedEnvironment();
 	const selected: QuickPickItem & { environment: Environment } | undefined = await window.showQuickPick<QuickPickItem & { environment: Environment }>(getEnvironments(true /* includePartial */)

src/login/msal/MsalAuthProvider.ts

@@ -6,11 +6,14 @@
 import { Environment } from "@azure/ms-rest-azure-env";
 import { DeviceCodeResponse } from "@azure/msal-common";
 import { AccountInfo, AuthenticationResult, Configuration, LogLevel, PublicClientApplication, TokenCache } from "@azure/msal-node";
+import { IActionContext, IParsedError, parseError, UserCancelledError } from "@microsoft/vscode-azext-utils";
+import { CancellationToken } from "vscode";
 import { AzureSession } from "../../azure-account.api";
-import { clientId } from "../../constants";
+import { authTimeoutSeconds, clientId, stoppedAuthTaskMessage } from "../../constants";
 import { AzureLoginError } from "../../errors";
 import { ext } from "../../extensionVariables";
 import { localize } from "../../utils/localize";
+import { Deferred } from "../../utils/promiseUtils";
 import { AbstractCredentials, AbstractCredentials2, AuthProviderBase } from "../AuthProviderBase";
 import { AzureSessionInternal } from "../AzureSessionInternal";
 import { cachePlugin } from "./cachePlugin";
@@ -57,16 +60,48 @@ export class MsalAuthProvider extends AuthProviderBase<AuthenticationResult> {
 		return authResult;
 	}
 
-	public async loginWithDeviceCode(environment: Environment, tenantId: string): Promise<AuthenticationResult> {
-		const authResult: AuthenticationResult | null = await this.publicClientApp.acquireTokenByDeviceCode({
+	public async loginWithDeviceCode(context: IActionContext, environment: Environment, tenantId: string, cancellationToken: CancellationToken): Promise<AuthenticationResult> {
+		// Used for prematurely ending the `authResultTask`.
+		let deferredTaskRegulator: Deferred<AuthenticationResult>;
+		const taskRegulator = new Promise<AuthenticationResult>((resolve, reject) => deferredTaskRegulator = { resolve, reject });
+
+		cancellationToken.onCancellationRequested(() => {
+			deferredTaskRegulator.reject(new Error(stoppedAuthTaskMessage));
+		});
+
+		const authResultTask: Promise<AuthenticationResult | null> = this.publicClientApp.acquireTokenByDeviceCode({
 			scopes: getDefaultMsalScopes(environment),
 			deviceCodeCallback: (response: DeviceCodeResponse) => this.showDeviceCodeMessage(response.message, response.userCode, response.verificationUri),
 			azureCloudOptions: {
 				azureCloudInstance: getAzureCloudInstance(environment),
 				tenant: tenantId
+			},
+			timeout: authTimeoutSeconds,
+			// This will immediately halt the promise if cancellation has already been requested.
+			// If cancellation is requested while awaiting the promise this will be ignored.
+			cancel: cancellationToken.isCancellationRequested
+		}).then(async result => {
+			if (cancellationToken.isCancellationRequested) {
+				context.telemetry.properties.authFlowCompletedAfterCancellation = 'true';
+
+				// Acquiring the token has already saved it in the cache so remove it.
+				await this.clearTokenCache();
+				throw new UserCancelledError();
 			}
+			return result;
 		});
 
+		let authResult: AuthenticationResult | null;
+		try {
+			authResult = await Promise.race([authResultTask, taskRegulator]);
+		} catch (error) {
+			const parsedError: IParsedError = parseError(error);
+			if (/user_timeout_reached/i.test(parsedError.errorType)) {
+				context.errorHandling.suppressDisplay = true;
+			}
+			throw error;
+		}
+
 		if (!authResult) {
 			throw new Error(localize('azure-account.msalDeviceCodeFailed', 'MSAL device code login failed.'));
 		}