From 926671aa6a048a6413c0156f8b980bc046386ff5 Mon Sep 17 00:00:00 2001 From: Tiago Pascoal Date: Wed, 14 Oct 2020 10:02:04 +0100 Subject: [PATCH] Make database parameter optional. (#22) By making the database optional opens the scenario where we just want to issue commands on the server (eg: to create a database), - Fix: If the database name had spaces `mysql` call would fail - Pass the password as an environment variable instead of command line. Eliminates warning that password method passing is insecure. --- __tests__/AzureMySqlAction.test.ts | 59 +++++++++++++++---- .../MySqlConnectionStringBuilder.test.ts | 14 +++++ lib/AzureMySqlAction.js | 15 ++++- lib/MySqlConnectionStringBuilder.js | 4 +- lib/MySqlUtils.js | 5 +- src/AzureMySqlAction.ts | 21 ++++++- src/MySqlConnectionStringBuilder.ts | 4 +- src/MySqlUtils.ts | 5 +- 8 files changed, 105 insertions(+), 22 deletions(-) diff --git a/__tests__/AzureMySqlAction.test.ts b/__tests__/AzureMySqlAction.test.ts index 09861d8..80ce0ac 100644 --- a/__tests__/AzureMySqlAction.test.ts +++ b/__tests__/AzureMySqlAction.test.ts @@ -3,19 +3,9 @@ import AzureMySqlActionHelper from '../src/AzureMySqlActionHelper'; import MySqlConnectionStringBuilder from '../src/MySqlConnectionStringBuilder'; import AzureMySqlAction, { IActionInputs } from '../src/AzureMySqlAction'; -jest.mock('../src/MySqlConnectionStringBuilder', () => { - return jest.fn().mockImplementation(() => { - return { - server: 'testmysqlserver.mysql.database.azure.com', - userId: 'testuser@testmysqlserver', - password: 'testpassword', - database: 'testdb' - } - }) -}); - describe('AzureMySqlAction tests', () => { it('executes sql file on target database', async () => { + let inputs = getInputs(); let action = new AzureMySqlAction(inputs); @@ -26,7 +16,41 @@ describe('AzureMySqlAction tests', () => { expect(getMySqlClientPathSpy).toHaveBeenCalledTimes(1); expect(execSpy).toHaveBeenCalledTimes(1); - expect(execSpy).toHaveBeenCalledWith(`"mysql.exe" -h testmysqlserver.mysql.database.azure.com -D testdb -u testuser@testmysqlserver -t 10 -e "source ./testsqlfile.sql"`, ['--password=testpassword']); + expect(execSpy).toHaveBeenCalledWith(`"mysql.exe" -t 10`, + [ + "-h", "testmysqlserver.mysql.database.azure.com", + "-u", "testuser@testmysqlserver", + "-e", "source ./testsqlfile.sql", + "-D", "testdb" + ], { + env: { + "MYSQL_PWD": "testpassword" + } + }) + }) + + it('executes sql file on target server', async () => { + + let inputsNoDB = getInputsNoDatabase(); + let action = new AzureMySqlAction(inputsNoDB); + + let getMySqlClientPathSpy = jest.spyOn(AzureMySqlActionHelper, 'getMySqlClientPath').mockResolvedValue('mysql.exe'); + let execSpy = jest.spyOn(exec, 'exec').mockResolvedValue(0); + + await action.execute(); + + expect(getMySqlClientPathSpy).toHaveBeenCalledTimes(1); + expect(execSpy).toHaveBeenCalledTimes(1); + expect(execSpy).toHaveBeenCalledWith(`"mysql.exe" -t 10`, + [ + "-h", "testmysqlserver.mysql.database.azure.com", + "-u", "testuser@testmysqlserver", + "-e", "source ./testsqlfile.sql" + ], { + env: { + "MYSQL_PWD": "testpassword" + } + }) }) it('throws if mysql client fails to execute', async () => { @@ -46,7 +70,16 @@ describe('AzureMySqlAction tests', () => { function getInputs() { return { serverName: 'testmysqlserver.mysql.database.azure.com', - connectionString: new MySqlConnectionStringBuilder('testmysqlserver.mysql.database.azure.com; Port=3306; Database=testdb; Uid=testuser@testmysqlserver; Pwd=testpassword; SslMode=Preferred'), + connectionString: new MySqlConnectionStringBuilder('Server=testmysqlserver.mysql.database.azure.com; Port=3306; Database=testdb; Uid=testuser@testmysqlserver; Pwd=testpassword; SslMode=Preferred'), + sqlFile: './testsqlfile.sql', + additionalArguments: '-t 10' + } as IActionInputs; +} + +function getInputsNoDatabase() { + return { + serverName: 'testmysqlserver.mysql.database.azure.com', + connectionString: new MySqlConnectionStringBuilder('Server=testmysqlserver.mysql.database.azure.com; Port=3306; Uid=testuser@testmysqlserver; Pwd=testpassword; SslMode=Preferred'), sqlFile: './testsqlfile.sql', additionalArguments: '-t 10' } as IActionInputs; diff --git a/__tests__/MySqlConnectionStringBuilder.test.ts b/__tests__/MySqlConnectionStringBuilder.test.ts index a499c53..e43f783 100644 --- a/__tests__/MySqlConnectionStringBuilder.test.ts +++ b/__tests__/MySqlConnectionStringBuilder.test.ts @@ -22,6 +22,20 @@ describe('MySqlConnectionStringBuilder tests', () => { }); }) + describe('validate no database specified', () => { + let validConnectionStrings = [ + [`Server=testserver;User Id=user;Password=JustANormal123@#$password;`, 'validates values no db specified', `JustANormal123@#$password`] + ]; + + it.each(validConnectionStrings)('Input `%s` %s', (connectionStringInput, testDescription, passwordOutput) => { + let connectionString = new MySqlConnectionStringBuilder(connectionStringInput); + + expect(connectionString.connectionString).toMatch(connectionStringInput); + expect(connectionString.password).toMatch(passwordOutput); + expect(connectionString.userId).toMatch(`user`); + }); + }) + describe('throw for invalid connection strings', () => { let invalidConnectionStrings = [ [`Server=testserver;User Id=user;Password="ab'=abcdf''c;123;Database=testdb`, 'validates values beginning with double quotes but not ending with double quotes'], diff --git a/lib/AzureMySqlAction.js b/lib/AzureMySqlAction.js index 6c8a730..71f7ff8 100644 --- a/lib/AzureMySqlAction.js +++ b/lib/AzureMySqlAction.js @@ -29,7 +29,20 @@ class AzureMySqlAction { return __awaiter(this, void 0, void 0, function* () { core.debug('Begin executing action...'); let mySqlClientPath = yield AzureMySqlActionHelper_1.default.getMySqlClientPath(); - yield exec.exec(`"${mySqlClientPath}" -h ${this._inputs.serverName} -D ${this._inputs.connectionString.database} -u ${this._inputs.connectionString.userId} ${this._inputs.additionalArguments} -e "source ${this._inputs.sqlFile}"`, [`--password=${this._inputs.connectionString.password}`]); + let parameters = [ + "-h", this._inputs.serverName, + "-u", this._inputs.connectionString.userId, + "-e", `source ${this._inputs.sqlFile}`, + ]; + if (this._inputs.connectionString.database) { + parameters.push("-D"); + parameters.push(this._inputs.connectionString.database); + } + yield exec.exec(`"${mySqlClientPath}" ${this._inputs.additionalArguments}`, parameters, { + env: { + "MYSQL_PWD": this._inputs.connectionString.password + } + }); console.log('Successfully executed sql file on target database'); }); } diff --git a/lib/MySqlConnectionStringBuilder.js b/lib/MySqlConnectionStringBuilder.js index 3bbbb75..2dc4078 100644 --- a/lib/MySqlConnectionStringBuilder.js +++ b/lib/MySqlConnectionStringBuilder.js @@ -101,8 +101,8 @@ class MySqlConnectionStringBuilder { } } } - if (!parsedConnectionString.server || !parsedConnectionString.userId || !parsedConnectionString.password || !parsedConnectionString.database) { - throw new Error(`Missing required keys in connection string. Please ensure that the keys 'Server', 'User Id', 'Password', 'Database' are provided in the connection string.`); + if (!parsedConnectionString.server || !parsedConnectionString.userId || !parsedConnectionString.password) { + throw new Error(`Missing required keys in connection string. Please ensure that the keys 'Server', 'User Id', 'Password' are provided in the connection string.`); } return parsedConnectionString; } diff --git a/lib/MySqlUtils.js b/lib/MySqlUtils.js index a78fa3b..30e3643 100644 --- a/lib/MySqlUtils.js +++ b/lib/MySqlUtils.js @@ -31,10 +31,13 @@ class MySqlUtils { try { core.debug(`Validating if client has access to MySql Server '${serverName}'.`); core.debug(`"${mySqlClientPath}" -h ${serverName} -u "${connectionString.userId}" -e "show databases"`); - yield exec.exec(`"${mySqlClientPath}" -h ${serverName} -u "${connectionString.userId}" -e "show databases"`, [`--password=${connectionString.password}`], { + yield exec.exec(`"${mySqlClientPath}" -h ${serverName} -u "${connectionString.userId}" -e "show databases"`, [], { silent: true, listeners: { stderr: (data) => mySqlError += data.toString() + }, + env: { + "MYSQL_PWD": connectionString.password } }); } diff --git a/src/AzureMySqlAction.ts b/src/AzureMySqlAction.ts index 40eff72..c7b1b94 100644 --- a/src/AzureMySqlAction.ts +++ b/src/AzureMySqlAction.ts @@ -17,9 +17,26 @@ export default class AzureMySqlAction { public async execute() { core.debug('Begin executing action...'); - + let mySqlClientPath = await AzureMySqlActionHelper.getMySqlClientPath(); - await exec.exec(`"${mySqlClientPath}" -h ${this._inputs.serverName} -D ${this._inputs.connectionString.database} -u ${this._inputs.connectionString.userId} ${this._inputs.additionalArguments} -e "source ${this._inputs.sqlFile}"`, [`--password=${this._inputs.connectionString.password}`]); + + let parameters = [ + "-h", this._inputs.serverName, + "-u", this._inputs.connectionString.userId, + "-e", `source ${this._inputs.sqlFile}`, + ] + + if (this._inputs.connectionString.database) { + parameters.push("-D") + parameters.push(this._inputs.connectionString.database) + } + + await exec.exec(`"${mySqlClientPath}" ${this._inputs.additionalArguments}`, + parameters, { + env: { + "MYSQL_PWD": this._inputs.connectionString.password + } + }); console.log('Successfully executed sql file on target database'); } diff --git a/src/MySqlConnectionStringBuilder.ts b/src/MySqlConnectionStringBuilder.ts index 8024104..8693edf 100644 --- a/src/MySqlConnectionStringBuilder.ts +++ b/src/MySqlConnectionStringBuilder.ts @@ -113,8 +113,8 @@ export default class MySqlConnectionStringBuilder { } } - if (!parsedConnectionString.server || !parsedConnectionString.userId || !parsedConnectionString.password || !parsedConnectionString.database) { - throw new Error(`Missing required keys in connection string. Please ensure that the keys 'Server', 'User Id', 'Password', 'Database' are provided in the connection string.`); + if (!parsedConnectionString.server || !parsedConnectionString.userId || !parsedConnectionString.password) { + throw new Error(`Missing required keys in connection string. Please ensure that the keys 'Server', 'User Id', 'Password' are provided in the connection string.`); } return parsedConnectionString; diff --git a/src/MySqlUtils.ts b/src/MySqlUtils.ts index 4922e5c..35995f9 100644 --- a/src/MySqlUtils.ts +++ b/src/MySqlUtils.ts @@ -11,10 +11,13 @@ export default class MySqlUtils { try { core.debug(`Validating if client has access to MySql Server '${serverName}'.`); core.debug(`"${mySqlClientPath}" -h ${serverName} -u "${connectionString.userId}" -e "show databases"`); - await exec.exec(`"${mySqlClientPath}" -h ${serverName} -u "${connectionString.userId}" -e "show databases"`, [`--password=${connectionString.password}`], { + await exec.exec(`"${mySqlClientPath}" -h ${serverName} -u "${connectionString.userId}" -e "show databases"`, [], { silent: true, listeners: { stderr: (data: Buffer) => mySqlError += data.toString() + }, + env: { + "MYSQL_PWD": connectionString.password } }); }