From d9b24522e5c5ab11301ebc9c3addbcedf8e9229b Mon Sep 17 00:00:00 2001 From: Lewis Sanchez <87730006+lewis-sanchez@users.noreply.github.com> Date: Tue, 14 Feb 2023 20:49:55 -0800 Subject: [PATCH] Fix missing password in Connection pane for Server connections with remembered passwords (#21813) * Fix missing password in Connection pane * Get saved password for SQL login default auth type * Clean up * Fix build hygiene errors * Captures input * Add timeout waiting for all promises to resolve * Add missing semicolon * Code review feedback * Minor clean up * Code review feedback * Improved error messaging * Update src/sql/workbench/services/connection/browser/connectionDialogService.ts Co-authored-by: Charles Gagnon * Improves UX around loading passwords * Remove unused import * Uses await instead of promise chaining. * Removes async * Revert back to resolving password promise. * Asserts controller map and model have values. --------- Co-authored-by: Charles Gagnon --- .../browser/connectionDialogService.ts | 33 +++++++++++++------ .../connection/browser/connectionWidget.ts | 14 +++++++- .../browser/connectionDialogService.test.ts | 9 ++++- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/sql/workbench/services/connection/browser/connectionDialogService.ts b/src/sql/workbench/services/connection/browser/connectionDialogService.ts index 289957c9e5..d1111c1c2b 100644 --- a/src/sql/workbench/services/connection/browser/connectionDialogService.ts +++ b/src/sql/workbench/services/connection/browser/connectionDialogService.ts @@ -361,15 +361,20 @@ export class ConnectionDialogService implements IConnectionDialogService { this.uiController.initDialog(this._params && this._params.providers, this._model); } - private handleFillInConnectionInputs(connectionInfo: IConnectionProfile): void { - this._connectionManagementService.addSavedPassword(connectionInfo).then(connectionWithPassword => { + private async handleFillInConnectionInputs(connectionInfo: IConnectionProfile): Promise { + try { + const connectionWithPassword = await this.createModel(connectionInfo); + if (this._model) { this._model.dispose(); } - this._model = this.createModel(connectionWithPassword); - + this._model = connectionWithPassword; this.uiController.fillInConnectionInputs(this._model); - }).catch(err => onUnexpectedError(err)); + } + catch (err) { + this._logService.error(`Error filling in connection inputs with password. Original error message: ${err}`); + } + this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[connectionInfo.providerName]); } @@ -381,11 +386,11 @@ export class ConnectionDialogService implements IConnectionDialogService { this.uiController.handleOnConnecting(); } - private updateModelServerCapabilities(model: IConnectionProfile) { + private async updateModelServerCapabilities(model: IConnectionProfile) { if (this._model) { this._model.dispose(); } - this._model = this.createModel(model); + this._model = await this.createModel(model); if (this._model.providerName) { this._currentProviderType = this._model.providerName; if (this._connectionDialog) { @@ -394,7 +399,7 @@ export class ConnectionDialogService implements IConnectionDialogService { } } - private createModel(model: IConnectionProfile): ConnectionProfile { + private async createModel(model: IConnectionProfile): Promise { const defaultProvider = this.getDefaultProviderName(); let providerName = model ? model.providerName : defaultProvider; providerName = providerName ? providerName : defaultProvider; @@ -410,6 +415,14 @@ export class ConnectionDialogService implements IConnectionDialogService { } let newProfile = new ConnectionProfile(this._capabilitiesService, model || providerName); + if (!model.password) { + try { + await this._connectionManagementService.addSavedPassword(newProfile); + } + catch (err) { + this._logService.error(`Error filling in password for connection dialog model. Original error message: ${err}`); + } + } newProfile.saveProfile = true; newProfile.generateNewId(); // If connecting from a query editor set "save connection" to false @@ -420,7 +433,7 @@ export class ConnectionDialogService implements IConnectionDialogService { } private async showDialogWithModel(): Promise { - this.updateModelServerCapabilities(this._inputModel); + await this.updateModelServerCapabilities(this._inputModel); await this.doShowDialog(this._params); } @@ -459,7 +472,7 @@ export class ConnectionDialogService implements IConnectionDialogService { this._params = params; this._inputModel = model; - this.updateModelServerCapabilities(model); + await this.updateModelServerCapabilities(model); // If connecting from a query editor set "save connection" to false if (params && (params.input && params.connectionType === ConnectionType.editor || diff --git a/src/sql/workbench/services/connection/browser/connectionWidget.ts b/src/sql/workbench/services/connection/browser/connectionWidget.ts index fb001ba81a..c2e183512f 100644 --- a/src/sql/workbench/services/connection/browser/connectionWidget.ts +++ b/src/sql/workbench/services/connection/browser/connectionWidget.ts @@ -44,6 +44,7 @@ import { createCSSRule } from 'vs/base/browser/dom'; const ConnectionStringText = localize('connectionWidget.connectionString', "Connection string"); export class ConnectionWidget extends lifecycle.Disposable { + private _initialConnectionInfo: IConnectionProfile; private _defaultInputOptionRadioButton: RadioButton; private _connectionStringRadioButton: RadioButton; private _previousGroupOption: string; @@ -612,7 +613,7 @@ export class ConnectionWidget extends lifecycle.Disposable { this._callbacks.onSetConnectButton(shouldEnableConnectButton); } - protected onAuthTypeSelected(selectedAuthType: string) { + protected onAuthTypeSelected(selectedAuthType: string): void { let currentAuthType = this.getMatchingAuthType(selectedAuthType); if (currentAuthType !== AuthenticationType.SqlLogin) { if (currentAuthType !== AuthenticationType.AzureMFA && currentAuthType !== AuthenticationType.AzureMFAAndUser) { @@ -668,6 +669,16 @@ export class ConnectionWidget extends lifecycle.Disposable { this._userNameInputBox.enable(); this._passwordInputBox.enable(); this._rememberPasswordCheckBox.enabled = true; + + this._initialConnectionInfo.authenticationType = AuthenticationType.SqlLogin; + if (this._initialConnectionInfo && this._initialConnectionInfo.userName) { + const setPasswordInputBox = (profile: IConnectionProfile) => { + this._passwordInputBox.value = profile.password; + }; + + this._rememberPasswordCheckBox.checked = this._initialConnectionInfo.savePassword; + this._connectionManagementService.addSavedPassword(this._initialConnectionInfo, true).then(setPasswordInputBox) + } } } @@ -822,6 +833,7 @@ export class ConnectionWidget extends lifecycle.Disposable { } public initDialog(connectionInfo: IConnectionProfile): void { + this._initialConnectionInfo = connectionInfo; this.fillInConnectionInputs(connectionInfo); } diff --git a/src/sql/workbench/services/connection/test/browser/connectionDialogService.test.ts b/src/sql/workbench/services/connection/test/browser/connectionDialogService.test.ts index 8e65154ebc..a4ce4433c9 100644 --- a/src/sql/workbench/services/connection/test/browser/connectionDialogService.test.ts +++ b/src/sql/workbench/services/connection/test/browser/connectionDialogService.test.ts @@ -306,9 +306,16 @@ suite('ConnectionDialogService tests', () => { mockWidget.setup(x => x.fillInConnectionInputs(TypeMoq.It.isAny())).returns(() => { called = true; }); + await connectionDialogService.showDialog(mockConnectionManagementService.object, testConnectionParams, connectionProfile); await (connectionDialogService as any).handleFillInConnectionInputs(connectionProfile); - let returnedModel = ((connectionDialogService as any)._connectionControllerMap['MSSQL'] as any)._model; + + let connectionControllerMap = (connectionDialogService as any)._connectionControllerMap; + assert(!!connectionControllerMap); + + let returnedModel = (connectionControllerMap['MSSQL'] as any)._model; + assert(!!returnedModel); + assert.strictEqual(returnedModel._groupName, 'testGroup'); assert(called); });