SQL Bindings refactor password prompting (#19371)

* remove password enter manually - since we prompt user to include it prior

* go back to connection profile prompt

* add manual entry if connection info password is not saved

* add tests that verify all password prompt scenarios

* nit

extensions/sql-bindings/src/common/azureFunctionsUtils.ts

@@ -325,7 +325,7 @@ export async function promptForObjectName(bindingType: BindingType, connectionIn
 
 	while (true) {
 		if (!connectionInfo) {
-			// prompt is shown when user selects an existing connection string setting 
+			// prompt is shown when user selects an existing connection string setting
 			// or manually enters a connection string
 			return promptToManuallyEnterObjectName(bindingType);
 		}
@@ -335,7 +335,7 @@ export async function promptForObjectName(bindingType: BindingType, connectionIn
 		// get connectionURI and selectedDatabase to be used for listing tables query request
 		connectionURI = await getConnectionURI(connectionInfo);
 		if (!connectionURI) {
-			// User cancelled or mssql connection error 
+			// User cancelled or mssql connection error
 			// we will then prompt user to choose a connection profile again
 			continue;
 		}
@@ -357,7 +357,7 @@ export async function promptForObjectName(bindingType: BindingType, connectionIn
 /**
  * Prompts the user to enter connection setting and updates it from AF project
  * @param projectUri Azure Function project uri
- * @param connectionInfo (optional) connection info from the user to update the connection string, 
+ * @param connectionInfo (optional) connection info from the user to update the connection string,
  * if left undefined we prompt the user for the connection info
  * @returns connection string setting name to be used for the createFunction API
  */
@@ -559,32 +559,33 @@ export async function promptConnectionStringPasswordAndUpdateConnectionString(co
 	let includePassword: string | undefined;
 	let connectionString: string = '';
 	let connectionDetails: ConnectionDetails;
+	let userPassword: string | undefined;
 	const vscodeMssqlApi = await utils.getVscodeMssqlApi();
 	connectionDetails = { options: connectionInfo };
 
 	try {
-		// Prompt to include password in connection string if authentication type is SqlLogin and connection has password saved
 		if (connectionInfo.authenticationType === 'SqlLogin' && connectionInfo.password) {
+			// Prompt to include password in connection string if authentication type is SqlLogin and connection has password saved
 			includePassword = await vscode.window.showQuickPick([constants.yesString, constants.noString], {
 				title: constants.includePassword,
 				canPickMany: false,
 				ignoreFocusOut: true
 			});
 			if (includePassword === constants.yesString) {
-				// set connection string to include password
+				// get connection string to include password
 				connectionString = await vscodeMssqlApi.getConnectionString(connectionDetails, true, false);
 			}
 		}
-		// set connection string to not include the password if connection info does not include password, or user chooses to not include password, or authentication type is not sql login
-		let userPassword: string | undefined;
-		if (includePassword !== constants.yesString) {
+
+		if (includePassword !== constants.yesString || !connectionInfo.password || connectionInfo.authenticationType !== 'SqlLogin') {
+			// get connection string to not include the password if connection info does not include password,
+			// or user chooses to not include password (or if user cancels out of include password prompt), or authentication type is not SQL login
 			connectionString = await vscodeMssqlApi.getConnectionString(connectionDetails, false, false);
 
-			// Ask user to enter password if auth type is sql login and password is not saved
-			if (connectionInfo.authenticationType === 'SqlLogin' && connectionInfo.password) {
+			if (!connectionInfo.password && connectionInfo.authenticationType === 'SqlLogin') {
+				// if a connection exists but does not have password saved we ask user if they would like to enter it and save it in local.settings.json
 				userPassword = await vscode.window.showInputBox({
 					prompt: constants.enterPasswordPrompt,
-					placeHolder: constants.enterPasswordManually,
 					ignoreFocusOut: true,
 					password: true,
 					validateInput: input => input ? undefined : constants.valueMustNotBeEmpty
@@ -594,20 +595,20 @@ export async function promptConnectionStringPasswordAndUpdateConnectionString(co
 					connectionString = connectionString.replace(constants.passwordPlaceholder, userPassword);
 				}
 			}
-		}
 
-		if (includePassword !== constants.yesString && !userPassword && connectionInfo?.authenticationType === 'SqlLogin') {
-			// if user does not want to include password or user does not enter password, show warning message that they will have to enter it manually later in local.settings.json
-			void vscode.window.showWarningMessage(constants.userPasswordLater, constants.openFile, constants.closeButton).then(async (result) => {
-				if (result === constants.openFile) {
-					// open local.settings.json file (if it exists)
-					void vscode.commands.executeCommand(constants.vscodeOpenCommand, vscode.Uri.file(localSettingsPath));
-				}
-			});
+			if (!userPassword && connectionInfo.authenticationType === 'SqlLogin') {
+				// show warning message that user will have to enter password manually later in local.settings.json
+				// if they choose to not to include password, if connection info does not include password
+				void vscode.window.showWarningMessage(constants.userPasswordLater, constants.openFile, constants.closeButton).then(async (result) => {
+					if (result === constants.openFile) {
+						// open local.settings.json file (if it exists)
+						void vscode.commands.executeCommand(constants.vscodeOpenCommand, vscode.Uri.file(localSettingsPath));
+					}
+				});
+			}
 		}
 
 		return connectionString;
-
 	} catch (e) {
 		// failed to get connection string for selected connection and will go back to prompt for connection string methods
 		console.warn(e);
@@ -705,4 +706,4 @@ export async function promptToManuallyEnterObjectName(bindingType: BindingType):
 		ignoreFocusOut: true
 	});
 	return selectedObject;
-}
+}

extensions/sql-bindings/src/common/constants.ts

@@ -69,11 +69,10 @@ export const connectionProfile = localize('connectionProfile', 'Select a connect
 export const userConnectionString = localize('userConnectionString', 'Enter connection string');
 export const selectConnectionString = localize('selectConnectionString', 'Select SQL connection string method');
 export const includePassword = localize('includePassword', 'Do you want to include the password from this connection in your local.settings.json file?');
-export const enterPasswordPrompt = localize('enterPasswordPrompt', 'Enter the password to be used for the connection string');
-export const enterPasswordManually = localize('enterPasswordManually', 'Enter password or press escape to cancel');
 export const userPasswordLater = localize('userPasswordLater', 'In order to user the SQL connection string later you will need to manually enter the password in your local.settings.json file.');
 export const openFile = localize('openFile', "Open File");
 export const closeButton = localize('closeButton', "Close");
+export const enterPasswordPrompt = localize('enterPasswordPrompt', '(Optional) Enter connection password to save in local.settings.json');
 export const connectionProgressTitle = localize('connectionProgressTitle', "Testing SQL Server connection...");
 export const enterObjectName = localize('enterObjectName', 'Enter SQL table or view to query');
 export const enterObjectNameToUpsert = localize('enterObjectNameToUpsert', 'Enter SQL table to upsert into');

extensions/sql-bindings/src/services/azureFunctionsService.ts

@@ -119,7 +119,13 @@ export async function createAzureFunction(node?: ITreeNodeInfo): Promise<void> {
 			telemetryStep = CreateAzureFunctionStep.launchFromCommandPalette;
 			// prompt user for connection profile to get connection info
 			while (true) {
-				connectionInfo = await vscodeMssqlApi.promptForConnection(true);
+				try {
+					connectionInfo = await vscodeMssqlApi.promptForConnection(true);
+				} catch (e) {
+					// user cancelled while creating connection profile
+					// show the connection profile selection prompt again
+					continue;
+				}
 				if (!connectionInfo) {
 					// User cancelled
 					return;

extensions/sql-bindings/src/test/common/azureFunctionsUtils.test.ts

@@ -9,11 +9,18 @@ import * as should from 'should';
 import * as sinon from 'sinon';
 import * as constants from '../../common/constants';
 import * as azureFunctionsUtils from '../../common/azureFunctionsUtils';
+import * as utils from '../../common/utils';
+import { IConnectionInfo } from 'vscode-mssql';
+import { createTestCredentials, createTestUtils, TestUtils } from '../testUtils';
 
 const rootFolderPath = 'test';
 const localSettingsPath: string = path.join(rootFolderPath, 'local.settings.json');
+let testUtils: TestUtils;
 
 describe('Tests to verify Azure Functions Utils functions', function (): void {
+	beforeEach(function (): void {
+		testUtils = createTestUtils();
+	});
 
 	it('Should correctly parse local.settings.json', async () => {
 		sinon.stub(fs, 'existsSync').withArgs(localSettingsPath).returns(true);
@@ -71,6 +78,161 @@ describe('Tests to verify Azure Functions Utils functions', function (): void {
 		should(writeFileStub.calledWithExactly(localSettingsPath, `{\n  "IsEncrypted": false,\n  "Values": {\n    "test1": "test1",\n    "test2": "test2",\n    "test3": "test3",\n    "SqlConnectionString": "testConnectionString"\n  }\n}`)).equals(true);
 	});
 
+	// Password Prompts
+	it('Should include password if user includes password and connection info contains the password and auth type is SQL', async () => {
+		sinon.stub(utils, 'getVscodeMssqlApi').resolves(testUtils.vscodeMssqlIExtension.object);
+		let connectionInfo: IConnectionInfo = createTestCredentials();// Mocks promptForConnection
+		let connectionDetails = { options: connectionInfo };
+
+		// getConnectionString should return a connection string with the password
+		testUtils.vscodeMssqlIExtension.setup(x => x.getConnectionString(connectionDetails, true, false)).returns(() => Promise.resolve(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${connectionInfo.password};`));
+
+		// Include Password Prompt - Yes to include password
+		let quickPickStub = sinon.stub(vscode.window, 'showQuickPick').onFirstCall().returns(Promise.resolve(constants.yesString) as any);
+		// Manually enter password
+		let quickInputSpy = sinon.spy(vscode.window, 'showInputBox');
+		// show warning window
+		const warningSpy = sinon.spy(vscode.window, 'showWarningMessage');
+
+		let getConnectionString = await azureFunctionsUtils.promptConnectionStringPasswordAndUpdateConnectionString(connectionInfo, localSettingsPath);
+
+		// manually entered password prompt and warning prompt should not be called
+		should(quickPickStub.calledOnce).be.true('showQuickPick should have been called');
+		should(quickInputSpy.notCalled).be.true('showInputBox should not have been called');
+		should(warningSpy.notCalled).be.true('showWarningMessage should not have been called');
+		// get connection string result
+		should(getConnectionString).equals(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${connectionInfo.password};`);
+	});
+
+	it('Should not include password and show warning if user does not want to include password prompt and connection info contains the password and auth type is SQL', async () => {
+		sinon.stub(utils, 'getVscodeMssqlApi').resolves(testUtils.vscodeMssqlIExtension.object);
+		let connectionInfo: IConnectionInfo = createTestCredentials();// Mocks promptForConnection
+		let connectionDetails = { options: connectionInfo };
+
+		// getConnectionString should return a connection string with password placeholder
+		testUtils.vscodeMssqlIExtension.setup(x => x.getConnectionString(connectionDetails, false, false)).returns(() => Promise.resolve(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`));
+
+		// Include Password Prompt - NO to include password
+		let quickPickStub = sinon.stub(vscode.window, 'showQuickPick').onFirstCall().returns(Promise.resolve(constants.noString) as any);
+		// Manually enter password
+		let quickInputSpy = sinon.spy(vscode.window, 'showInputBox');
+		// show warning window
+		const warningSpy = sinon.spy(vscode.window, 'showWarningMessage');
+
+		let getConnectionString = await azureFunctionsUtils.promptConnectionStringPasswordAndUpdateConnectionString(connectionInfo, localSettingsPath);
+
+		// warning prompt should be shown and manually entered password prompt should not be called (since user indicated they do not want to include password)
+		should(quickPickStub.calledOnce).be.true('showQuickPick should have been called');
+		should(quickInputSpy.notCalled).be.true('showInputBox should not have been called');
+		should(warningSpy.calledOnce).be.true('showWarningMessage should have been called');
+		// returned connection string should NOT include password
+		should(getConnectionString).equals(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`);
+	});
+
+	it('Should not include password and show warning if user cancels include password prompt and connection info contains the password and auth type is SQL', async () => {
+		sinon.stub(utils, 'getVscodeMssqlApi').resolves(testUtils.vscodeMssqlIExtension.object);
+		let connectionInfo: IConnectionInfo = createTestCredentials();// Mocks promptForConnection
+		let connectionDetails = { options: connectionInfo };
+
+		// getConnectionString should return a connection string with password placeholder
+		testUtils.vscodeMssqlIExtension.setup(x => x.getConnectionString(connectionDetails, false, false)).returns(() => Promise.resolve(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`));
+
+		// Include Password Prompt - cancels out of include password prompt
+		let quickPickStub = sinon.stub(vscode.window, 'showQuickPick').onFirstCall().resolves(undefined);
+		// Manually enter password
+		let quickInputSpy = sinon.spy(vscode.window, 'showInputBox');
+		// show warning window
+		const warningSpy = sinon.spy(vscode.window, 'showWarningMessage');
+
+		let getConnectionString = await azureFunctionsUtils.promptConnectionStringPasswordAndUpdateConnectionString(connectionInfo, localSettingsPath);
+
+		// warning prompt should be shown and manually entered password prompt should not be called (since user indicated they do not want to include password)
+		should(quickPickStub.calledOnce).be.true('showQuickPick should have been called');
+		should(quickInputSpy.notCalled).be.true('showInputBox should not have been called');
+		should(warningSpy.calledOnce).be.true('showWarningMessage should have been called');
+		// returned connection string should NOT include password
+		should(getConnectionString).equals(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`);
+	});
+
+	it('Should return connection string with no password saved when connection auth type is not SQL', async () => {
+		sinon.stub(utils, 'getVscodeMssqlApi').resolves(testUtils.vscodeMssqlIExtension.object);
+		let connectionInfo: IConnectionInfo = createTestCredentials();// Mocks promptForConnection
+		connectionInfo.authenticationType = 'TestAuth'; // auth type is not SQL
+		let connectionDetails = { options: connectionInfo };
+
+		// getConnectionString should return a connection string with password placeholder
+		testUtils.vscodeMssqlIExtension.setup(x => x.getConnectionString(connectionDetails, false, false)).returns(() => Promise.resolve(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`));
+
+		// Include password prompt
+		let quickpickStub = sinon.stub(vscode.window, 'showQuickPick');
+		// Manually enter password
+		let quickInputSpy = sinon.spy(vscode.window, 'showInputBox');
+		// show warning window
+		const warningSpy = sinon.spy(vscode.window, 'showWarningMessage');
+
+		let getConnectionString = await azureFunctionsUtils.promptConnectionStringPasswordAndUpdateConnectionString(connectionInfo, localSettingsPath);
+
+		// should not call any of the prompts (include password, manually enter password, or show warning) since connection auth type is not SQL
+		should(quickpickStub.notCalled).be.true('showQuickPick should not have been called');
+		should(quickInputSpy.notCalled).be.true('showInputBox should not have been called');
+		should(warningSpy.notCalled).be.true('showWarningMessage should not have been called');
+		// returned connection string should NOT include password
+		should(getConnectionString).equals(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`);
+	});
+
+	it('Should ask user to enter password and set password to connection string when connection info does not contain password and auth type is SQL', async () => {
+		sinon.stub(utils, 'getVscodeMssqlApi').resolves(testUtils.vscodeMssqlIExtension.object);
+		let connectionInfo: IConnectionInfo = createTestCredentials();// Mocks promptForConnection
+		connectionInfo.password = ''; // password is not saved for the connection
+		let connectionDetails = { options: connectionInfo };
+
+		// getConnectionString should return a connection string with password placeholder
+		testUtils.vscodeMssqlIExtension.setup(x => x.getConnectionString(connectionDetails, false, false)).returns(() => Promise.resolve(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`));
+
+		// Include password prompt
+		let quickpickStub = sinon.stub(vscode.window, 'showQuickPick');
+		// Manually enter password
+		let enteredPassword = 'testPassword';
+		let quickInputSpy = sinon.stub(vscode.window, 'showInputBox').onFirstCall().resolves(enteredPassword);
+		// show warning window
+		const warningSpy = sinon.spy(vscode.window, 'showWarningMessage');
+
+		let getConnectionString = await azureFunctionsUtils.promptConnectionStringPasswordAndUpdateConnectionString(connectionInfo, localSettingsPath);
+
+		// manually entered password prompt should be called while warning prompt and include password prompt should not be called (since user connection info does not contain password)
+		should(quickpickStub.notCalled).be.true('showQuickPick should not have been called');
+		should(quickInputSpy.calledOnce).be.true('showInputBox should have been called');
+		should(warningSpy.notCalled).be.true('showWarningMessage should have been called');
+		// returned connection string should have the entered password
+		should(getConnectionString).equals(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${enteredPassword};`);
+	});
+
+	it('Should ask user to enter password and not include password to connection string since user cancelled prompt when connection info does not contain password and auth type is SQL', async () => {
+		sinon.stub(utils, 'getVscodeMssqlApi').resolves(testUtils.vscodeMssqlIExtension.object);
+		let connectionInfo: IConnectionInfo = createTestCredentials();// Mocks promptForConnection
+		connectionInfo.password = ''; // password is not saved for the connection
+		let connectionDetails = { options: connectionInfo };
+
+		// getConnectionString should return a connection string with password placeholder
+		testUtils.vscodeMssqlIExtension.setup(x => x.getConnectionString(connectionDetails, false, false)).returns(() => Promise.resolve(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`));
+
+		// Include password prompt
+		let quickpickStub = sinon.stub(vscode.window, 'showQuickPick');
+		// Manually enter password
+		let quickInputSpy = sinon.stub(vscode.window, 'showInputBox').onFirstCall().resolves(undefined);
+		// show warning window
+		const warningSpy = sinon.spy(vscode.window, 'showWarningMessage');
+
+		let getConnectionString = await azureFunctionsUtils.promptConnectionStringPasswordAndUpdateConnectionString(connectionInfo, localSettingsPath);
+
+		// manually entered password prompt and warning prompt should be shown and include password prompt should not be called (since user cancelled manually enter password prompt)
+		should(quickpickStub.notCalled).be.true('showQuickPick should not have been called');
+		should(quickInputSpy.calledOnce).be.true('showInputBox should have been called');
+		should(warningSpy.calledOnce).be.true('showWarningMessage should have been called ');
+		// returned connection string should have the entered password
+		should(getConnectionString).equals(`Server=${connectionInfo.server};Initial Catalog=${connectionInfo.database};User ID=${connectionInfo.user};Password=${constants.passwordPlaceholder};`);
+	});
+
 	afterEach(async function (): Promise<void> {
 		sinon.restore();
 	});