fix(mysql): re-order create user statements

refs TryGhost#511
- always run create user with hashed password to fix requirements with
secure installations

Author: acburdine

extensions/mysql/index.js

@@ -79,35 +79,47 @@ class MySQLExtension extends cli.Extension {
     createUser(ctx, dbconfig) {
         const randomPassword = crypto.randomBytes(10).toString('hex');
 
-        // IMPORTANT: we generate random MySQL usernames
-        // e.g. you delete all your Ghost instances from your droplet and start from scratch, the MySQL users would remain and the CLI has to generate a random user name to work
-        // e.g. if we would rely on the instance name, the instance naming only auto increments if there are existing instances
-        // the most important fact is, that if a MySQL user exists, we have no access to the password, which we need to autofill the Ghost config
-        // disadvantage: the CLI could potentially create lot's of MySQL users (but this should only happen if the user installs Ghost over and over again with root credentials)
-        const username = 'ghost-' + Math.floor(Math.random() * 1000);
-
-        return this._query(`CREATE USER '${username}'@'${dbconfig.host}' IDENTIFIED WITH mysql_native_password;`).then(() => {
-            this.ui.logVerbose(`MySQL: successfully created new user ${username}`, 'green');
+        let username;
 
-            return this._query('SET old_passwords = 0;');
-        }).then(() => {
+        // Ensure old passwords is set to 0
+        return this._query('SET old_passwords = 0;').then(() => {
             this.ui.logVerbose('MySQL: successfully disabled old_password', 'green');
 
-            return this._query(`SET PASSWORD FOR '${username}'@'${dbconfig.host}' = PASSWORD('${randomPassword}');`);
+            return this._query(`SELECT PASSWORD('${randomPassword}') AS password;`);
+        }).then((result) => {
+            this.ui.logVerbose('MySQL: successfully created password hash.', 'green');
+
+            const tryCreateUser = () => {
+                // IMPORTANT: we generate random MySQL usernames
+                // e.g. you delete all your Ghost instances from your droplet and start from scratch, the MySQL users would remain and the CLI has to generate a random user name to work
+                // e.g. if we would rely on the instance name, the instance naming only auto increments if there are existing instances
+                // the most important fact is, that if a MySQL user exists, we have no access to the password, which we need to autofill the Ghost config
+                // disadvantage: the CLI could potentially create lot's of MySQL users (but this should only happen if the user installs Ghost over and over again with root credentials)
+                username = `ghost-${Math.floor(Math.random() * 1000)}`;
+
+                return this._query(
+                    `CREATE USER '${username}'@'${dbconfig.host}' ` +
+                    `IDENTIFIED WITH mysql_native_password AS '${result[0].password}';`
+                ).catch((error) => {
+                    // User already exists, run this method again
+                    if (error.errno === 1396) {
+                        this.ui.logVerbose('MySQL: user exists, re-trying user creation with new username', 'yellow');
+                        return tryCreateUser();
+                    }
+
+                    return Promise.reject(error);
+                });
+            };
+
+            return tryCreateUser();
         }).then(() => {
-            this.ui.logVerbose(`MySQL: successfully created password for user ${username}`, 'green');
+            this.ui.logVerbose(`MySQL: successfully created new user ${username}`, 'green');
 
             ctx.mysql = {
                 username: username,
                 password: randomPassword
             };
         }).catch((error) => {
-            // User already exists, run this method again
-            if (error.errno === 1396) {
-                this.ui.logVerbose('MySQL: user exists, re-trying user creation with new username', 'yellow');
-                return this.createUser(ctx, dbconfig);
-            }
-
             this.ui.logVerbose('MySQL: Unable to create custom Ghost user', 'red');
             this.connection.end(); // Ensure we end the connection
             return Promise.reject(new cli.errors.SystemError(`Creating new mysql user errored with message: ${error.message}`));

extensions/mysql/test/extension-spec.js

@@ -186,17 +186,18 @@ describe('Unit: Mysql extension', function () {
             const logStub = sinon.stub();
             const instance = new MysqlExtension({logVerbose: logStub}, {}, {}, '/some/dir');
             const queryStub = sinon.stub(instance, '_query').resolves();
+            queryStub.onSecondCall().resolves([{password: '*2470C0C06DEE42FD1618BB99005ADCA2EC9D1E19'}]);
             const ctx = {};
 
             return instance.createUser(ctx, {host: 'localhost'}).then(() => {
                 expect(queryStub.calledThrice).to.be.true;
-                expect(queryStub.args[0][0]).to.match(/^CREATE USER 'ghost-[0-9]{1,4}'@'localhost' IDENTIFIED WITH mysql_native_password;$/);
-                expect(queryStub.args[1][0]).to.equal('SET old_passwords = 0;');
-                expect(queryStub.args[2][0]).to.match(/^SET PASSWORD FOR 'ghost-[0-9]{1,4}'@'localhost' = PASSWORD\('[0-9A-Fa-f]*'\);$/);
+                expect(queryStub.args[0][0]).to.equal('SET old_passwords = 0;');
+                expect(queryStub.args[1][0]).to.match(/^SELECT PASSWORD\('[0-9A-Fa-f]*'\) AS password;$/);
+                expect(queryStub.args[2][0]).to.match(/^CREATE USER 'ghost-[0-9]{1,4}'@'localhost' IDENTIFIED WITH mysql_native_password AS '\*[0-9A-F]*';$/);
                 expect(logStub.calledThrice).to.be.true;
-                expect(logStub.args[0][0]).to.match(/created new user/);
-                expect(logStub.args[1][0]).to.match(/disabled old_password/);
-                expect(logStub.args[2][0]).to.match(/successfully created password for user/);
+                expect(logStub.args[0][0]).to.match(/disabled old_password/);
+                expect(logStub.args[1][0]).to.match(/created password hash/);
+                expect(logStub.args[2][0]).to.match(/successfully created new user/);
                 expect(ctx.mysql).to.exist;
                 expect(ctx.mysql.username).to.match(/^ghost-[0-9]{1,4}$/);
                 expect(ctx.mysql.password).to.match(/^[0-9A-Fa-f]*$/);
@@ -207,7 +208,8 @@ describe('Unit: Mysql extension', function () {
             const logStub = sinon.stub();
             const instance = new MysqlExtension({logVerbose: logStub}, {}, {}, '/some/dir');
             const queryStub = sinon.stub(instance, '_query').resolves();
-            queryStub.onFirstCall().callsFake(() => {
+            queryStub.onSecondCall().resolves([{password: '*2470C0C06DEE42FD1618BB99005ADCA2EC9D1E19'}]);
+            queryStub.onThirdCall().callsFake(() => {
                 const error = new Error();
                 error.errno = 1396;
                 return Promise.reject(error);
@@ -216,21 +218,47 @@ describe('Unit: Mysql extension', function () {
 
             return instance.createUser(ctx, {host: 'localhost'}).then(() => {
                 expect(queryStub.callCount).to.equal(4);
-                expect(queryStub.args[0][0]).to.match(/^CREATE USER 'ghost-[0-9]{1,4}'@'localhost' IDENTIFIED WITH mysql_native_password;$/);
-                expect(queryStub.args[1][0]).to.match(/^CREATE USER 'ghost-[0-9]{1,4}'@'localhost' IDENTIFIED WITH mysql_native_password;$/);
-                expect(queryStub.args[2][0]).to.equal('SET old_passwords = 0;');
-                expect(queryStub.args[3][0]).to.match(/^SET PASSWORD FOR 'ghost-[0-9]{1,4}'@'localhost' = PASSWORD\('[0-9A-Fa-f]*'\);$/);
+                expect(queryStub.args[0][0]).to.equal('SET old_passwords = 0;');
+                expect(queryStub.args[1][0]).to.match(/^SELECT PASSWORD\('[0-9A-Fa-f]*'\) AS password;$/);
+                expect(queryStub.args[2][0]).to.match(/^CREATE USER 'ghost-[0-9]{1,4}'@'localhost' IDENTIFIED WITH mysql_native_password AS '\*[0-9A-F]*';$/);
+                expect(queryStub.args[3][0]).to.match(/^CREATE USER 'ghost-[0-9]{1,4}'@'localhost' IDENTIFIED WITH mysql_native_password AS '\*[0-9A-F]*';$/);
                 expect(logStub.callCount).to.equal(4);
-                expect(logStub.args[0][0]).to.match(/user exists, re-trying user creation/);
-                expect(logStub.args[1][0]).to.match(/created new user/);
-                expect(logStub.args[2][0]).to.match(/disabled old_password/);
-                expect(logStub.args[3][0]).to.match(/successfully created password for user/);
+                expect(logStub.args[0][0]).to.match(/disabled old_password/);
+                expect(logStub.args[1][0]).to.match(/created password hash/);
+                expect(logStub.args[2][0]).to.match(/user exists, re-trying user creation/);
+                expect(logStub.args[3][0]).to.match(/successfully created new user/);
                 expect(ctx.mysql).to.exist;
                 expect(ctx.mysql.username).to.match(/^ghost-[0-9]{1,4}$/);
                 expect(ctx.mysql.password).to.match(/^[0-9A-Fa-f]*$/);
             });
         });
 
+        it('rejects if error occurs during user creation', function () {
+            const logStub = sinon.stub();
+            const instance = new MysqlExtension({logVerbose: logStub}, {}, {}, '/some/dir');
+            const queryStub = sinon.stub(instance, '_query').resolves();
+            queryStub.onSecondCall().resolves([{password: '*2470C0C06DEE42FD1618BB99005ADCA2EC9D1E19'}]);
+            queryStub.onThirdCall().rejects();
+            const endStub = sinon.stub();
+            instance.connection = {end: endStub};
+
+            return instance.createUser({}, {host: 'localhost'}).then(() => {
+                expect(false, 'error should have been thrown').to.be.true;
+            }).catch((error) => {
+                expect(error).to.be.an.instanceof(errors.SystemError);
+                expect(error.message).to.match(/Creating new mysql user errored/);
+                expect(queryStub.callCount).to.equal(3);
+                expect(queryStub.args[0][0]).to.equal('SET old_passwords = 0;');
+                expect(queryStub.args[1][0]).to.match(/^SELECT PASSWORD\('[0-9A-Fa-f]*'\) AS password;$/);
+                expect(queryStub.args[2][0]).to.match(/^CREATE USER 'ghost-[0-9]{1,4}'@'localhost' IDENTIFIED WITH mysql_native_password AS '\*[0-9A-F]*';$/);
+                expect(logStub.callCount).to.equal(3);
+                expect(logStub.args[0][0]).to.match(/disabled old_password/);
+                expect(logStub.args[1][0]).to.match(/created password hash/);
+                expect(logStub.args[2][0]).to.match(/Unable to create custom Ghost user/);
+                expect(endStub.calledOnce).to.be.true;
+            });
+        });
+
         it('rejects with SystemError and ends connection if any query fails', function () {
             const logStub = sinon.stub();
             const instance = new MysqlExtension({logVerbose: logStub}, {}, {}, '/some/dir');
@@ -244,7 +272,7 @@ describe('Unit: Mysql extension', function () {
                 expect(error).to.be.an.instanceof(errors.SystemError);
                 expect(error.message).to.match(/Creating new mysql user errored/);
                 expect(queryStub.calledOnce).to.be.true;
-                expect(queryStub.args[0][0]).to.match(/^CREATE USER 'ghost-[0-9]{1,4}'@'localhost' IDENTIFIED WITH mysql_native_password;$/);
+                expect(queryStub.args[0][0]).to.equal('SET old_passwords = 0;');
                 expect(logStub.calledOnce).to.be.true;
                 expect(logStub.args[0][0]).to.match(/Unable to create custom Ghost user/);
                 expect(endStub.calledOnce).to.be.true;