From d315ccff6870077338a0b06a66586babec8fea48 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 29 Oct 2019 07:27:31 -0700 Subject: [PATCH] Fix BDC remember password and reprompting connection (#7957) * Fix remember password and reprompting connection * comment * Fix to remember password for session * Fix floating promises --- .../controller/clusterControllerApi.ts | 87 +++++++++++-------- .../dialog/addControllerDialog.ts | 6 +- .../src/bigDataCluster/dialog/bdcDashboard.ts | 12 ++- .../dialog/bdcDashboardModel.ts | 73 +++++++++++----- .../bigDataCluster/dialog/hdfsDialogBase.ts | 10 ++- .../tree/controllerTreeDataProvider.ts | 28 +++--- .../bigDataCluster/tree/controllerTreeNode.ts | 18 ++-- extensions/big-data-cluster/src/extension.ts | 46 +++++----- 8 files changed, 183 insertions(+), 97 deletions(-) diff --git a/extensions/big-data-cluster/src/bigDataCluster/controller/clusterControllerApi.ts b/extensions/big-data-cluster/src/bigDataCluster/controller/clusterControllerApi.ts index c238513f29..ecb4c7d50d 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/controller/clusterControllerApi.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/controller/clusterControllerApi.ts @@ -88,35 +88,51 @@ class DefaultApiWrapper extends DefaultApi { export class ClusterController { - private authPromise: Promise; + private _authPromise: Promise; private _url: string; - private readonly dialog: ConnectControllerDialog; - private connectionPromise: Promise; + private readonly _dialog: ConnectControllerDialog; + private _connectionPromise: Promise; constructor(url: string, - private authType: AuthType, - private username?: string, - private password?: string, + private _authType: AuthType, + private _username?: string, + private _password?: string, ignoreSslVerification?: boolean ) { - if (!url || (authType === 'basic' && (!username || !password))) { + if (!url || (_authType === 'basic' && (!_username || !_password))) { throw new Error('Missing required inputs for Cluster controller API (URL, username, password)'); } this._url = adjustUrl(url); - if (this.authType === 'basic') { - this.authPromise = Promise.resolve(new BasicAuth(username, password, !!ignoreSslVerification)); + if (this._authType === 'basic') { + this._authPromise = Promise.resolve(new BasicAuth(_username, _password, !!ignoreSslVerification)); } else { - this.authPromise = this.requestTokenUsingKerberos(ignoreSslVerification); + this._authPromise = this.requestTokenUsingKerberos(ignoreSslVerification); } - this.dialog = new ConnectControllerDialog(new ConnectControllerModel( + this._dialog = new ConnectControllerDialog(new ConnectControllerModel( { url: this._url, - auth: this.authType, - username: this.username, - password: this.password + auth: this._authType, + username: this._username, + password: this._password })); } + public get url(): string { + return this._url; + } + + public get authType(): AuthType { + return this._authType; + } + + public get username(): string | undefined { + return this._username; + } + + public get password(): string | undefined { + return this._password; + } + private async requestTokenUsingKerberos(ignoreSslVerification?: boolean): Promise { let supportsKerberos = await this.verifyKerberosSupported(ignoreSslVerification); if (!supportsKerberos) { @@ -166,8 +182,8 @@ export class ClusterController { } private async getEndpointsImpl(self: ClusterController): Promise { - let auth = await self.authPromise; - let endPointApi = new BdcApiWrapper(self.username, self.password, self._url, auth); + let auth = await self._authPromise; + let endPointApi = new BdcApiWrapper(self._username, self._password, self._url, auth); let options: any = {}; let result = await endPointApi.endpointsGet(options); @@ -185,8 +201,8 @@ export class ClusterController { } private async getBdcStatusImpl(self: ClusterController): Promise { - let auth = await self.authPromise; - const bdcApi = new BdcApiWrapper(self.username, self.password, self._url, auth); + let auth = await self._authPromise; + const bdcApi = new BdcApiWrapper(self._username, self._password, self._url, auth); const bdcStatus = await bdcApi.getBdcStatus('', '', /*all*/ true); return { @@ -206,8 +222,8 @@ export class ClusterController { } private async mountHdfsImpl(self: ClusterController, mountPath: string, remoteUri: string, credentials: {}): Promise { - let auth = await self.authPromise; - const api = new DefaultApiWrapper(self.username, self.password, self._url, auth); + let auth = await self._authPromise; + const api = new DefaultApiWrapper(self._username, self._password, self._url, auth); const mountStatus = await api.createMount('', '', remoteUri, mountPath, credentials); return { @@ -225,8 +241,8 @@ export class ClusterController { } private async getMountStatusImpl(self: ClusterController, mountPath?: string): Promise { - const auth = await self.authPromise; - const api = new DefaultApiWrapper(self.username, self.password, self._url, auth); + const auth = await self._authPromise; + const api = new DefaultApiWrapper(self._username, self._password, self._url, auth); const mountStatus = await api.listMounts('', '', mountPath); return { @@ -244,8 +260,8 @@ export class ClusterController { } private async refreshMountImpl(self: ClusterController, mountPath: string): Promise { - const auth = await self.authPromise; - const api = new DefaultApiWrapper(self.username, self.password, self._url, auth); + const auth = await self._authPromise; + const api = new DefaultApiWrapper(self._username, self._password, self._url, auth); const mountStatus = await api.refreshMount('', '', mountPath); return { @@ -263,8 +279,8 @@ export class ClusterController { } private async deleteMountImpl(mountPath: string): Promise { - let auth = await this.authPromise; - const api = new DefaultApiWrapper(this.username, this.password, this._url, auth); + let auth = await this._authPromise; + const api = new DefaultApiWrapper(this._username, this._password, this._url, auth); const mountStatus = await api.deleteMount('', '', mountPath); return { @@ -291,17 +307,17 @@ export class ClusterController { // We don't want to open multiple dialogs here if multiple calls come in the same time so check // and see if we have are actively waiting on an open dialog to return and if so then just wait // on that promise. - if (!this.connectionPromise) { - this.connectionPromise = this.dialog.showDialog(); + if (!this._connectionPromise) { + this._connectionPromise = this._dialog.showDialog(); } - const controller = await this.connectionPromise; - this.connectionPromise = undefined; + const controller = await this._connectionPromise; + this._connectionPromise = undefined; if (controller) { - this.username = controller.username; - this.password = controller.password; + this._username = controller._username; + this._password = controller._password; this._url = controller._url; - this.authType = controller.authType; - this.authPromise = controller.authPromise; + this._authType = controller._authType; + this._authPromise = controller._authPromise; } return await f(this, args); } @@ -378,7 +394,7 @@ export class ControllerError extends Error { public code?: number; public reason?: string; public address?: string; - + public statusMessage?: string; /** * * @param error The original error to wrap @@ -391,6 +407,7 @@ export class ControllerError extends Error { this.code = error.response.statusCode; this.message += `${error.response.statusMessage ? ` - ${error.response.statusMessage}` : ''}` || ''; this.address = error.response.url || ''; + this.statusMessage = error.response.statusMessage; } else if (error.message) { this.message += ` - ${error.message}`; diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/addControllerDialog.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/addControllerDialog.ts index 22ce084a45..44b0878827 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/dialog/addControllerDialog.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/addControllerDialog.ts @@ -6,11 +6,14 @@ 'use strict'; import * as azdata from 'azdata'; +import * as vscode from 'vscode'; import * as nls from 'vscode-nls'; import { ClusterController, ControllerError } from '../controller/clusterControllerApi'; import { ControllerTreeDataProvider } from '../tree/controllerTreeDataProvider'; import { TreeNode } from '../tree/treeNode'; import { AuthType } from '../constants'; +import { ManageControllerCommand } from '../../extension'; +import { BdcDashboardOptions } from './bdcDashboardModel'; const localize = nls.loadMessageBundle(); @@ -73,7 +76,8 @@ export class AddControllerDialogModel { if (this._canceled) { return; } - this.treeDataProvider.addController(url, auth, username, password, rememberPassword); + this.treeDataProvider.addOrUpdateController(url, auth, username, password, rememberPassword); + vscode.commands.executeCommand(ManageControllerCommand, { url: url, auth: auth, username: username, password: password }); await this.treeDataProvider.saveControllers(); } } catch (error) { diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboard.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboard.ts index 0c4c25586f..34fa54db40 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboard.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboard.ts @@ -13,7 +13,8 @@ import { IconPathHelper, cssStyles } from '../constants'; import { BdcServiceStatusPage } from './bdcServiceStatusPage'; import { BdcDashboardOverviewPage } from './bdcDashboardOverviewPage'; import { BdcStatusModel, ServiceStatusModel } from '../controller/apiGenerated'; -import { getHealthStatusDot, getServiceNameDisplayText } from '../utils'; +import { getHealthStatusDot, getServiceNameDisplayText, showErrorMessage } from '../utils'; +import { HdfsDialogCancelledError } from './hdfsDialogBase'; const localize = nls.loadMessageBundle(); @@ -43,6 +44,7 @@ export class BdcDashboard { constructor(private title: string, private model: BdcDashboardModel) { this.model.onDidUpdateBdcStatus(bdcStatus => this.handleBdcStatusUpdate(bdcStatus)); + this.model.onError(error => this.handleError(error)); } public showDashboard(): void { @@ -165,6 +167,14 @@ export class BdcDashboard { this.updateServiceNavTabs(bdcStatus.services); } + private handleError(error: Error): void { + // We don't want to show an error for the connection dialog being + // canceled since that's a normal case. + if (!(error instanceof HdfsDialogCancelledError)) { + showErrorMessage(error.message); + } + } + private async doRefresh(): Promise { try { this.refreshButton.enabled = false; diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardModel.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardModel.ts index 76c419d714..ab4c99b734 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardModel.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/bdcDashboardModel.ts @@ -7,8 +7,10 @@ import * as azdata from 'azdata'; import * as vscode from 'vscode'; import { ClusterController } from '../controller/clusterControllerApi'; import { EndpointModel, BdcStatusModel } from '../controller/apiGenerated'; -import { showErrorMessage, Endpoint, Service } from '../utils'; +import { Endpoint, Service } from '../utils'; import { AuthType } from '../constants'; +import { ConnectControllerDialog, ConnectControllerModel } from './connectControllerDialog'; +import { ControllerTreeDataProvider } from '../tree/controllerTreeDataProvider'; export type BdcDashboardOptions = { url: string, auth: AuthType, username: string, password: string }; @@ -21,12 +23,23 @@ export class BdcDashboardModel { private _endpointsLastUpdated: Date; private readonly _onDidUpdateEndpoints = new vscode.EventEmitter(); private readonly _onDidUpdateBdcStatus = new vscode.EventEmitter(); + private readonly _onError = new vscode.EventEmitter(); public onDidUpdateEndpoints = this._onDidUpdateEndpoints.event; public onDidUpdateBdcStatus = this._onDidUpdateBdcStatus.event; + public onError = this._onError.event; - constructor(private options: BdcDashboardOptions, ignoreSslVerification = true) { - this._clusterController = new ClusterController(options.url, options.auth, options.username, options.password, ignoreSslVerification); - this.refresh(); + constructor(private _options: BdcDashboardOptions, private _treeDataProvider: ControllerTreeDataProvider, ignoreSslVerification = true) { + try { + this._clusterController = new ClusterController(_options.url, _options.auth, _options.username, _options.password, ignoreSslVerification); + // tslint:disable-next-line:no-floating-promises + this.refresh(); + } catch { + this.promptReconnect().then(async () => { + await this.refresh(); + }).catch(error => { + this._onError.fire(error); + }); + } } public get bdcStatus(): BdcStatusModel | undefined { @@ -46,21 +59,28 @@ export class BdcDashboardModel { } public async refresh(): Promise { - await Promise.all([ - this._clusterController.getBdcStatus(true).then(response => { - this._bdcStatus = response.bdcStatus; - this._bdcStatusLastUpdated = new Date(); - this._onDidUpdateBdcStatus.fire(this.bdcStatus); - }), - this._clusterController.getEndPoints(true).then(response => { - this._endpoints = response.endPoints || []; - fixEndpoints(this._endpoints); - this._endpointsLastUpdated = new Date(); - this._onDidUpdateEndpoints.fire(this.serviceEndpoints); - }) - ]).catch(error => { - showErrorMessage(error); - }); + try { + if (!this._clusterController) { + // If this succeeds without error we know we have a clusterController at this point + await this.promptReconnect(); + } + + await Promise.all([ + this._clusterController.getBdcStatus(true).then(response => { + this._bdcStatus = response.bdcStatus; + this._bdcStatusLastUpdated = new Date(); + this._onDidUpdateBdcStatus.fire(this.bdcStatus); + }), + this._clusterController.getEndPoints(true).then(response => { + this._endpoints = response.endPoints || []; + fixEndpoints(this._endpoints); + this._endpointsLastUpdated = new Date(); + this._onDidUpdateEndpoints.fire(this.serviceEndpoints); + }) + ]); + } catch (error) { + this._onError.fire(error); + } } /** @@ -81,7 +101,7 @@ export class BdcDashboardModel { serverName: sqlServerMasterEndpoint.endpoint, databaseName: undefined, userName: 'sa', - password: this.options.password, + password: this._options.password, authenticationType: '', savePassword: true, groupFullName: undefined, @@ -92,6 +112,19 @@ export class BdcDashboardModel { options: {} }; } + + /** + * Opens up a dialog prompting the user to re-enter credentials for the controller + */ + private async promptReconnect(): Promise { + this._clusterController = await new ConnectControllerDialog(new ConnectControllerModel(this._options)).showDialog(); + this._treeDataProvider.addOrUpdateController( + this._clusterController.url, + this._clusterController.authType, + this._clusterController.username, + this._clusterController.password, + /* Remember password */false); + } } /** diff --git a/extensions/big-data-cluster/src/bigDataCluster/dialog/hdfsDialogBase.ts b/extensions/big-data-cluster/src/bigDataCluster/dialog/hdfsDialogBase.ts index b3943ea1b5..bcc1cb7387 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/dialog/hdfsDialogBase.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/dialog/hdfsDialogBase.ts @@ -28,6 +28,12 @@ export interface HdfsDialogProperties { password?: string; } +export class HdfsDialogCancelledError extends Error { + constructor(message: string = 'Dialog cancelled') { + super(message); + } +} + export abstract class HdfsDialogModelBase { protected _canceled = false; private _authTypes: azdata.CategoryValue[]; @@ -87,7 +93,7 @@ export abstract class HdfsDialogModelBase { throw new Error(localize('mount.hdfs.loginerror1', "Login to controller failed")); } } catch (err) { - throw new Error(localize('mount.hdfs.loginerror2', "Login to controller failed: {0}", err.message)); + throw new Error(localize('mount.hdfs.loginerror2', "Login to controller failed: {0}", err.statusMessage || err.message)); } return controller; } @@ -224,7 +230,7 @@ export abstract class HdfsDialogBase { if (this.model && this.model.onCancel) { await this.model.onCancel(); } - this.returnPromise.reject(new Error('Dialog cancelled')); + this.returnPromise.reject(new HdfsDialogCancelledError()); } protected async reportError(error: any): Promise { diff --git a/extensions/big-data-cluster/src/bigDataCluster/tree/controllerTreeDataProvider.ts b/extensions/big-data-cluster/src/bigDataCluster/tree/controllerTreeDataProvider.ts index ef85ff08fe..c28c8028fa 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/tree/controllerTreeDataProvider.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/tree/controllerTreeDataProvider.ts @@ -45,7 +45,7 @@ export class ControllerTreeDataProvider implements vscode.TreeDataProvider { - let controllers = this.root.children.map((e): IControllerInfoSlim => { - let controller = e as ControllerNode; + const controllers = this.root.children.map((e): IControllerInfoSlim => { + const controller = e as ControllerNode; return { url: controller.url, auth: controller.auth, username: controller.username, password: controller.password, - rememberPassword: !!controller.rememberPassword + rememberPassword: controller.rememberPassword }; }); - let controllersWithoutPassword = controllers.map((e): IControllerInfoSlim => { + const controllersWithoutPassword = controllers.map((e): IControllerInfoSlim => { return { url: e.url, auth: e.auth, @@ -164,7 +172,7 @@ export class ControllerTreeDataProvider implements vscode.TreeDataProvider { + private async getPassword(url: string, username: string): Promise { let provider = await this.getCredentialProvider(); let id = this.createId(url, username); let credential = await provider.readCredential(id); diff --git a/extensions/big-data-cluster/src/bigDataCluster/tree/controllerTreeNode.ts b/extensions/big-data-cluster/src/bigDataCluster/tree/controllerTreeNode.ts index 5e7eda8b92..c9aaf553bc 100644 --- a/extensions/big-data-cluster/src/bigDataCluster/tree/controllerTreeNode.ts +++ b/extensions/big-data-cluster/src/bigDataCluster/tree/controllerTreeNode.ts @@ -103,7 +103,15 @@ export class ControllerRootNode extends ControllerTreeNode { return this.children as ControllerNode[]; } - public addControllerNode( + /** + * Creates or updates a node in the tree with the specified connection information + * @param url The URL for the BDC management endpoint + * @param auth The type of auth to use + * @param username The username (if basic auth) + * @param password The password (if basic auth) + * @param rememberPassword Whether to store the password in the password store when saving + */ + public addOrUpdateControllerNode( url: string, auth: AuthType, username: string, @@ -201,6 +209,10 @@ export class ControllerNode extends ControllerTreeNode { this._password = pw; } + public set label(label: string) { + super.label = label || this.generateLabel(); + } + public get rememberPassword() { return this._rememberPassword; } @@ -209,10 +221,6 @@ export class ControllerNode extends ControllerTreeNode { this._rememberPassword = rememberPassword; } - public set label(label: string) { - super.label = label || this.generateLabel(); - } - private generateLabel(): string { let label = `controller: ${ControllerNode.toIpAndPort(this._url)}`; if (this._auth === 'basic') { diff --git a/extensions/big-data-cluster/src/extension.ts b/extensions/big-data-cluster/src/extension.ts index 27fd17a0c2..fa721b8a61 100644 --- a/extensions/big-data-cluster/src/extension.ts +++ b/extensions/big-data-cluster/src/extension.ts @@ -20,13 +20,13 @@ import { getControllerEndpoint } from './bigDataCluster/utils'; const localize = nls.loadMessageBundle(); -const AddControllerCommand = 'bigDataClusters.command.addController'; -const DeleteControllerCommand = 'bigDataClusters.command.deleteController'; -const RefreshControllerCommand = 'bigDataClusters.command.refreshController'; -const ManageControllerCommand = 'bigDataClusters.command.manageController'; -const MountHdfsCommand = 'bigDataClusters.command.mount'; -const RefreshMountCommand = 'bigDataClusters.command.refreshmount'; -const DeleteMountCommand = 'bigDataClusters.command.deletemount'; +export const AddControllerCommand = 'bigDataClusters.command.addController'; +export const DeleteControllerCommand = 'bigDataClusters.command.deleteController'; +export const RefreshControllerCommand = 'bigDataClusters.command.refreshController'; +export const ManageControllerCommand = 'bigDataClusters.command.manageController'; +export const MountHdfsCommand = 'bigDataClusters.command.mount'; +export const RefreshMountCommand = 'bigDataClusters.command.refreshmount'; +export const DeleteMountCommand = 'bigDataClusters.command.deletemount'; const endpointNotFoundError = localize('mount.error.endpointNotFound', "Controller endpoint information was not found"); @@ -51,8 +51,8 @@ function registerCommands(context: vscode.ExtensionContext, treeDataProvider: Co runThrottledAction(AddControllerCommand, () => addBdcController(treeDataProvider, node)); }); - vscode.commands.registerCommand(DeleteControllerCommand, (node: TreeNode) => { - deleteBdcController(treeDataProvider, node); + vscode.commands.registerCommand(DeleteControllerCommand, async (node: TreeNode) => { + await deleteBdcController(treeDataProvider, node); }); vscode.commands.registerCommand(RefreshControllerCommand, (node: TreeNode) => { @@ -64,7 +64,7 @@ function registerCommands(context: vscode.ExtensionContext, treeDataProvider: Co vscode.commands.registerCommand(ManageControllerCommand, async (info: ControllerNode | BdcDashboardOptions) => { const title: string = `${localize('bdc.dashboard.title', "Big Data Cluster Dashboard -")} ${ControllerNode.toIpAndPort(info.url)}`; - const dashboard: BdcDashboard = new BdcDashboard(title, new BdcDashboardModel(info)); + const dashboard: BdcDashboard = new BdcDashboard(title, new BdcDashboardModel(info, treeDataProvider)); dashboard.showDashboard(); }); @@ -80,26 +80,26 @@ function registerCommands(context: vscode.ExtensionContext, treeDataProvider: Co } async function mountHdfs(explorerContext?: azdata.ObjectExplorerContext): Promise { - let mountProps = await getMountProps(explorerContext); + const mountProps = await getMountProps(explorerContext); if (mountProps) { - let dialog = new MountHdfsDialog(new MountHdfsModel(mountProps)); - dialog.showDialog(); + const dialog = new MountHdfsDialog(new MountHdfsModel(mountProps)); + await dialog.showDialog(); } } async function refreshMount(explorerContext?: azdata.ObjectExplorerContext): Promise { - let mountProps = await getMountProps(explorerContext); + const mountProps = await getMountProps(explorerContext); if (mountProps) { - let dialog = new RefreshMountDialog(new RefreshMountModel(mountProps)); - dialog.showDialog(); + const dialog = new RefreshMountDialog(new RefreshMountModel(mountProps)); + await dialog.showDialog(); } } async function deleteMount(explorerContext?: azdata.ObjectExplorerContext): Promise { - let mountProps = await getMountProps(explorerContext); + const mountProps = await getMountProps(explorerContext); if (mountProps) { - let dialog = new DeleteMountDialog(new DeleteMountModel(mountProps)); - dialog.showDialog(); + const dialog = new DeleteMountDialog(new DeleteMountModel(mountProps)); + await dialog.showDialog(); } } @@ -169,15 +169,15 @@ async function deleteBdcController(treeDataProvider: ControllerTreeDataProvider, let result = await vscode.window.showQuickPick(Object.keys(choices), options); let remove: boolean = !!(result && choices[result]); if (remove) { - deleteControllerInternal(treeDataProvider, controllerNode); + await deleteControllerInternal(treeDataProvider, controllerNode); } return remove; } -function deleteControllerInternal(treeDataProvider: ControllerTreeDataProvider, controllerNode: ControllerNode): void { - let deleted = treeDataProvider.deleteController(controllerNode.url, controllerNode.auth, controllerNode.username); +async function deleteControllerInternal(treeDataProvider: ControllerTreeDataProvider, controllerNode: ControllerNode): Promise { + const deleted = treeDataProvider.deleteController(controllerNode.url, controllerNode.auth, controllerNode.username); if (deleted) { - treeDataProvider.saveControllers(); + await treeDataProvider.saveControllers(); } }