From 9cefed840e76f066bc1a3c0cc72ac2a8b02f05b4 Mon Sep 17 00:00:00 2001 From: Vasu Bhog Date: Thu, 19 May 2022 10:41:11 -0700 Subject: [PATCH] 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 --- .../src/common/azureFunctionsUtils.ts | 45 ++--- .../sql-bindings/src/common/constants.ts | 3 +- .../src/services/azureFunctionsService.ts | 8 +- .../test/common/azureFunctionsUtils.test.ts | 162 ++++++++++++++++++ 4 files changed, 193 insertions(+), 25 deletions(-) diff --git a/extensions/sql-bindings/src/common/azureFunctionsUtils.ts b/extensions/sql-bindings/src/common/azureFunctionsUtils.ts index 4f237f1fd2..64e904629b 100644 --- a/extensions/sql-bindings/src/common/azureFunctionsUtils.ts +++ b/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; -} \ No newline at end of file +} diff --git a/extensions/sql-bindings/src/common/constants.ts b/extensions/sql-bindings/src/common/constants.ts index 388802ba79..9cde3d5b03 100644 --- a/extensions/sql-bindings/src/common/constants.ts +++ b/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'); diff --git a/extensions/sql-bindings/src/services/azureFunctionsService.ts b/extensions/sql-bindings/src/services/azureFunctionsService.ts index 804ef289b8..a9b514842b 100644 --- a/extensions/sql-bindings/src/services/azureFunctionsService.ts +++ b/extensions/sql-bindings/src/services/azureFunctionsService.ts @@ -119,7 +119,13 @@ export async function createAzureFunction(node?: ITreeNodeInfo): Promise { 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; diff --git a/extensions/sql-bindings/src/test/common/azureFunctionsUtils.test.ts b/extensions/sql-bindings/src/test/common/azureFunctionsUtils.test.ts index c3b9ba0e1b..399e718a9c 100644 --- a/extensions/sql-bindings/src/test/common/azureFunctionsUtils.test.ts +++ b/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 { sinon.restore(); });