feat(doctor): use new getGhostUid util

Author: aileen

lib/commands/doctor/checks/logged-in-user-owner.js

@@ -1,76 +1,52 @@
 'use strict';
-const os = require('os');
-const execa = require('execa');
 const path = require('path');
 const errors = require('../../../errors');
 const chalk = require('chalk');
 const fs = require('fs');
+const ghostUser = require('../../../utils/use-ghost-user');
 
 const taskTitle = 'Checking logged in user and directory owner';
 
-function checkGhostUser() {
-    let ghostuid;
-    let ghostgid;
-
-    try {
-        ghostuid = execa.shellSync('id -u ghost').stdout;
-        ghostgid = execa.shellSync('id -g ghost').stdout;
-    } catch (e) {
-        // CASE: the ghost user doesn't exist, hence can't be used
-        return false
-    }
-
-    ghostuid = parseInt(ghostuid);
-    ghostgid = parseInt(ghostgid);
-
-    return {
-        uid: ghostuid,
-        gid: ghostgid
-    };
-}
-
 function loggedInUserOwner(ctx) {
     const uid = process.getuid();
     const gid = process.getgroups();
-    const ghostStats = checkGhostUser();
     const dirStats = fs.lstatSync(path.join(process.cwd()));
     const contentDirStats = fs.lstatSync(path.join(process.cwd(), 'content'));
 
+    const ghostStats = ghostUser.getGhostUid();
+
     // CASE 1: check if ghost user exists and if it's currently used
     if (ghostStats && ghostStats.uid && ghostStats.uid === uid) {
         // The ghost user might have been set up on the system and also used,
         // but only when it owns the content folder, it's an indication that it's also used
         // as the linux user and shall not be used as current user.
         if (contentDirStats.uid === ghostStats.uid) {
-            return Promise.reject(new errors.SystemError({
+            throw new errors.SystemError({
                 message: 'You can\'t use Ghost with the ghost user. Please log in with your own user.',
                 help: `${chalk.green('https://docs.ghost.org/docs/install#section-create-a-new-user')}`,
                 task: taskTitle
-            }));
+            });
         }
     }
 
     // CASE 2: check if the current user is the owner of the current dir
     if (dirStats.uid !== uid) {
         if (gid.indexOf(dirStats.gid) < 0) {
-            return Promise.reject(new errors.SystemError({
+            throw new errors.SystemError({
                 message: `Your current user is not the owner of the Ghost directory and also not part of the same group.
 Please log in with the user that owns the directory or add your user to the same group.`,
                 help: `${chalk.green('https://docs.ghost.org/docs/install#section-create-a-new-user')}`,
                 task: taskTitle
-            }));
+            });
         }
         // Yup current user is not the owner, but in the same group, so just show a warning
-        ctx.ui.log('The current user is not the owner of the Ghost directory. This might cause problems.');
+        ctx.ui.log(`${chalk.yellow('The current user is not the owner of the Ghost directory. This might cause problems.')}`);
     }
-
-    return Promise.resolve();
 }
 
 module.exports = {
     title: taskTitle,
     task: loggedInUserOwner,
-    enabled: (ctx) => ctx.instance && ctx.instance.process.name !== 'local',
-    skip: () => os.platform() !== 'linux',
+    enabled: (ctx) => ctx.system.platform.linux,
     category: ['start', 'update']
 }

lib/utils/use-ghost-user.js

@@ -3,8 +3,6 @@ const fs = require('fs');
 const os = require('os');
 const execa = require('execa');
 
-const errors = require('../errors');
-
 const getGhostUid = function getGhostUid() {
     if (os.platform() !== 'linux') {
         return false;
@@ -17,11 +15,11 @@ const getGhostUid = function getGhostUid() {
         ghostgid = execa.shellSync('id -g ghost').stdout;
     } catch (e) {
         // CASE: the ghost user doesn't exist, hence can't be used
-        if (!e.message.match(/no such user/i)) {
-            throw new errors.ProcessError(e);
-        }
-
-        return false
+        // We just return false and not doing anything with the error,
+        // as it would either mean, that the user doesn't exist (this
+        // is exactly what we want), or the command is not known on a
+        // Linux system.
+        return false;
     }
 
     ghostuid = parseInt(ghostuid);
@@ -34,18 +32,12 @@ const getGhostUid = function getGhostUid() {
 }
 
 const shouldUseGhostUser = function shouldUseGhostUser(contentDir) {
-    let ghostUser;
-
     if (os.platform() !== 'linux') {
         return false;
     }
 
     // get the ghost uid and gid
-    try {
-        ghostUser = getGhostUid();
-    } catch (e) {
-        throw e;
-    }
+    const ghostUser = getGhostUid();
 
     if (!ghostUser) {
         return false;

test/unit/commands/doctor/checks/logged-in-user-owner-spec.js

@@ -1,139 +1,115 @@
 'use strict';
 const expect = require('chai').expect;
 const sinon = require('sinon');
-const os = require('os');
 const fs = require('fs');
 
-const execa = require('execa');
 const errors = require('../../../../../lib/errors');
+const ghostUser = require('../../../../../lib/utils/use-ghost-user');
 
 const loggedInUserOwner = require('../../../../../lib/commands/doctor/checks/logged-in-user-owner');
 
 describe('Unit: Doctor Checks > loggedInUserOwner', function () {
     const sandbox = sinon.sandbox.create();
-    let osStub;
-
-    beforeEach(() => {
-        osStub = sandbox.stub(os, 'platform').returns('linux');
-    })
 
     afterEach(() => {
         sandbox.restore();
     });
 
     it('enabled works', function () {
         expect(loggedInUserOwner.enabled({
-            instance: {process: {name: 'local'}}
-        }), 'false if process name is local').to.be.false;
-    });
-
-    it('skip works', function () {
-        osStub.returns('win32');
-        expect(loggedInUserOwner.skip(), 'true if platform is not linux').to.be.true;
-        expect(osStub.calledOnce).to.be.true;
+            system: {platform: {linux: false}}
+        }), 'false if platform is not linux').to.be.false;
     });
 
     describe('Ghost user', function () {
         it('rejects if user is logged in as ghost and ghost owns content folder', function () {
             const uidStub = sandbox.stub(process, 'getuid').returns(1002);
             const gidStub = sandbox.stub(process, 'getgroups').returns([30, 1002]);
-            const execaStub = sandbox.stub(execa, 'shellSync');
+            const ghostUserStub = sandbox.stub(ghostUser, 'getGhostUid').returns({uid: 1002, guid: 1002});
             const fsStub = sandbox.stub(fs, 'lstatSync');
 
-            execaStub.onFirstCall().returns({stdout: '1002'});
-            execaStub.onSecondCall().returns({stdout: '1002'});
             fsStub.onFirstCall().returns({uid: 1000, gid: 1000});
             fsStub.onSecondCall().returns({uid: 1002, gid: 1002});
 
-            return loggedInUserOwner.task().then(() => {
+            try {
+                loggedInUserOwner.task()
                 expect(false, 'error should have been thrown').to.be.true;
-            }).catch((error) => {
+            } catch (error) {
                 expect(error).to.exist;
                 expect(uidStub.calledOnce).to.be.true;
                 expect(gidStub.calledOnce).to.be.true;
-                expect(execaStub.calledWithExactly('id -u ghost')).to.be.true;
-                expect(execaStub.calledWithExactly('id -g ghost')).to.be.true;
+                expect(ghostUserStub.calledOnce).to.be.true;
                 expect(error).to.be.an.instanceof(errors.SystemError);
                 expect(error.message).to.match(/You can't use Ghost with the ghost user./);
-            });
+            }
         });
 
         it('resolves if user is logged in as ghost but ghost doesn\'t own the content folder', function () {
             const uidStub = sandbox.stub(process, 'getuid').returns(1002);
             const gidStub = sandbox.stub(process, 'getgroups').returns([30, 1002]);
-            const execaStub = sandbox.stub(execa, 'shellSync');
+            const ghostUserStub = sandbox.stub(ghostUser, 'getGhostUid').returns({uid: 1002, guid: 1002});
             const fsStub = sandbox.stub(fs, 'lstatSync');
 
-            execaStub.onFirstCall().returns({stdout: '1002'});
-            execaStub.onSecondCall().returns({stdout: '1002'});
             fsStub.onFirstCall().returns({uid: 1002, gid: 1002});
             fsStub.onSecondCall().returns({uid: 1001, gid: 1001});
 
-            return loggedInUserOwner.task().then(() => {
-                expect(uidStub.calledOnce).to.be.true;
-                expect(gidStub.calledOnce).to.be.true;
-                expect(execaStub.calledWithExactly('id -u ghost')).to.be.true;
-                expect(execaStub.calledWithExactly('id -g ghost')).to.be.true;
-            });
+            loggedInUserOwner.task();
+            expect(uidStub.calledOnce).to.be.true;
+            expect(gidStub.calledOnce).to.be.true;
+            expect(ghostUserStub.calledOnce).to.be.true;
         });
     });
 
     describe('Other users', function () {
         it('rejects if current user is not owner and not in the same group as owner', function () {
             const uidStub = sandbox.stub(process, 'getuid').returns(1000);
             const gidStub = sandbox.stub(process, 'getgroups').returns([30, 1000]);
-            const execaStub = sandbox.stub(execa, 'shellSync');
+            const ghostUserStub = sandbox.stub(ghostUser, 'getGhostUid').returns({uid: 1002, guid: 1002});
             const fsStub = sandbox.stub(fs, 'lstatSync');
 
-            execaStub.onFirstCall().returns({stdout: '1002'});
-            execaStub.onSecondCall().returns({stdout: '1002'});
             fsStub.onFirstCall().returns({uid: 1001, gid: 1001});
             fsStub.onSecondCall().returns({uid: 1002, gid: 1002});
 
-            return loggedInUserOwner.task().then(() => {
+            try {
+                loggedInUserOwner.task();
                 expect(false, 'error should have been thrown').to.be.true;
-            }).catch((error) => {
+            } catch (error) {
                 expect(error).to.exist;
                 expect(uidStub.calledOnce).to.be.true;
                 expect(gidStub.calledOnce).to.be.true;
-                expect(execaStub.calledWithExactly('id -u ghost')).to.be.true;
-                expect(execaStub.calledWithExactly('id -g ghost')).to.be.true;
+                expect(ghostUserStub.calledOnce).to.be.true;
                 expect(error).to.be.an.instanceof(errors.SystemError);
                 expect(error.message).to.match(/Your current user is not the owner of the Ghost directory and also not part of the same group./);
-            });
+            }
         });
 
         it('shows a warning message, if user is logged in as different user than owner, but same group', function () {
             const uidStub = sandbox.stub(process, 'getuid').returns(1001);
             const gidStub = sandbox.stub(process, 'getgroups').returns([30, 1000]);
-            const execaStub = sandbox.stub(execa, 'shellSync');
+            const ghostUserStub = sandbox.stub(ghostUser, 'getGhostUid').returns({uid: 1002, guid: 1002});
             const fsStub = sandbox.stub(fs, 'lstatSync');
             const logStub = sandbox.stub();
 
             const ctx = {
                 ui: {log: logStub}
             };
 
-            execaStub.onFirstCall().returns({stdout: '1002'});
-            execaStub.onSecondCall().returns({stdout: '1002'});
             fsStub.onFirstCall().returns({uid: 1000, gid: 1000});
             fsStub.onSecondCall().returns({uid: 1002, gid: 1002});
 
-            return loggedInUserOwner.task(ctx).then(() => {
-                expect(uidStub.calledOnce).to.be.true;
-                expect(gidStub.calledOnce).to.be.true;
-                expect(logStub.calledOnce).to.be.true;
-                expect(logStub.args[0][0]).to.match(/The current user is not the owner of the Ghost directory. This might cause problems./);
-                expect(execaStub.calledWithExactly('id -u ghost')).to.be.true;
-                expect(execaStub.calledWithExactly('id -g ghost')).to.be.true;
-            });
+            loggedInUserOwner.task(ctx);
+            expect(uidStub.calledOnce).to.be.true;
+            expect(gidStub.calledOnce).to.be.true;
+            expect(logStub.calledOnce).to.be.true;
+            expect(logStub.args[0][0]).to.match(/The current user is not the owner of the Ghost directory. This might cause problems./);
+            expect(ghostUserStub.calledOnce).to.be.true;
         });
 
         it('resolves if current user is also the owner', function () {
             const uidStub = sandbox.stub(process, 'getuid').returns(1000);
             const gidStub = sandbox.stub(process, 'getgroups').returns([30, 1000]);
             // Ghost user doesn't exist this time
-            const execaStub = sandbox.stub(execa, 'shellSync').throws(new Error('no such user'));
+            const ghostUserStub = sandbox.stub(ghostUser, 'getGhostUid').returns(false);
             const fsStub = sandbox.stub(fs, 'lstatSync');
             const logStub = sandbox.stub();
 
@@ -144,12 +120,27 @@ describe('Unit: Doctor Checks > loggedInUserOwner', function () {
             fsStub.onFirstCall().returns({uid: 1000, gid: 1000});
             fsStub.onSecondCall().returns({uid: 1002, gid: 1002});
 
-            return loggedInUserOwner.task(ctx).then(() => {
-                expect(uidStub.calledOnce).to.be.true;
-                expect(gidStub.calledOnce).to.be.true;
-                expect(logStub.calledOnce).to.be.false;
-                expect(execaStub.calledOnce).to.be.true;
-            });
+            loggedInUserOwner.task(ctx);
+            expect(uidStub.calledOnce).to.be.true;
+            expect(gidStub.calledOnce).to.be.true;
+            expect(logStub.calledOnce).to.be.false;
+            expect(ghostUserStub.calledOnce).to.be.true;
+        });
+
+        it('rejects and passes the error if ghostUser util throws error', function () {
+            const uidStub = sandbox.stub(process, 'getuid').returns(1000);
+            const gidStub = sandbox.stub(process, 'getgroups').returns([30, 1000]);
+            // getGhostUid throws error this time
+            const ghostUserStub = sandbox.stub(ghostUser, 'getGhostUid').returns(false);
+            const fsStub = sandbox.stub(fs, 'lstatSync');
+
+            fsStub.onFirstCall().returns({uid: 1000, gid: 1000});
+            fsStub.onSecondCall().returns({uid: 1002, gid: 1002});
+
+            loggedInUserOwner.task();
+            expect(uidStub.calledOnce).to.be.true;
+            expect(gidStub.calledOnce).to.be.true;
+            expect(ghostUserStub.calledOnce).to.be.true;
         });
     });
 });