Skip to content

Commit

Permalink
feat(utils): split use-ghost-user in two fn
Browse files Browse the repository at this point in the history
  • Loading branch information
@aileen @acburdine
aileen authored and acburdine committed Mar 26, 2018
1 parent 9fbd3fd commit e8e2fb4
Show file tree
Hide file tree
Showing 9 changed files with 179 additions and 121 deletions.
4 changes: 2 additions & 2 deletions lib/commands/doctor/checks/content-folder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
const path = require('path');

const checkPermissions = require('./check-permissions');
const shouldUseGhostUser = require('../../../utils/use-ghost-user');
const ghostUser = require('../../../utils/use-ghost-user');

const taskTitle = 'Checking content folder ownership';

module.exports = {
title: taskTitle,
enabled: () => shouldUseGhostUser(path.join(process.cwd(), 'content')),
enabled: () => ghostUser.shouldUseGhostUser(path.join(process.cwd(), 'content')),
task: () => checkPermissions('owner', taskTitle),
category: ['start', 'update']
};
4 changes: 2 additions & 2 deletions lib/commands/run.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const Command = require('../command');
class RunCommand extends Command {
run() {
const path = require('path');
const shouldUseGhostUser = require('../utils/use-ghost-user');
const ghostUser = require('../utils/use-ghost-user');

// If the user is running this command directly, output a little note
// telling them they're likely looking for `ghost start`
Expand All @@ -20,7 +20,7 @@ class RunCommand extends Command {
instance.config.set('paths.contentPath', path.join(instance.dir, 'content')).save();
}

if (shouldUseGhostUser(path.join(instance.dir, 'content'))) {
if (ghostUser.shouldUseGhostUser(path.join(instance.dir, 'content'))) {
return this.useGhostUser(instance);
}

Expand Down
4 changes: 2 additions & 2 deletions lib/commands/uninstall.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class UninstallCommand extends Command {
const path = require('path');

const StopCommand = require('./stop');
const shouldUseGhostUser = require('../utils/use-ghost-user');
const ghostUser = require('../utils/use-ghost-user');

if (!argv.force) {
this.ui.log('WARNING: Running this command will delete all of your themes, images, data, any files related to this Ghost instance, and the contents of this folder!\n' +
Expand Down Expand Up @@ -37,7 +37,7 @@ class UninstallCommand extends Command {
}
}, {
title: 'Removing content folder',
enabled: () => shouldUseGhostUser(path.join(instance.dir, 'content')),
enabled: () => ghostUser.shouldUseGhostUser(path.join(instance.dir, 'content')),
task: () => this.ui.sudo(`rm -rf ${path.join(instance.dir, 'content')}`)
}, {
title: 'Removing related configuration',
Expand Down
5 changes: 2 additions & 3 deletions lib/tasks/migrate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const path = require('path');
const execa = require('execa');

const errors = require('../errors');
const shouldUseGhostUser = require('../utils/use-ghost-user');
const ghostUser = require('../utils/use-ghost-user');

module.exports = function runMigrations(context) {
const config = context.instance.config;
Expand All @@ -23,7 +23,7 @@ module.exports = function runMigrations(context) {

// If we're using sqlite and the ghost user owns the content folder, then
// we should run sudo, otherwise run normally
if (shouldUseGhostUser(contentDir)) {
if (ghostUser.shouldUseGhostUser(contentDir)) {
const knexMigratorPath = path.resolve(context.instance.dir, 'current/node_modules/.bin/knex-migrator-migrate');
knexMigratorPromise = context.ui.sudo(`${knexMigratorPath} ${args.join(' ')}`, {sudoArgs: '-E -u ghost'});
} else {
Expand Down Expand Up @@ -76,4 +76,3 @@ module.exports = function runMigrations(context) {
return Promise.reject(error);
});
}

38 changes: 34 additions & 4 deletions lib/utils/use-ghost-user.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const execa = require('execa');

const errors = require('../errors');

module.exports = function useGhostUser(contentDir) {
const getGhostUid = function getGhostUid() {
if (os.platform() !== 'linux') {
return false;
}
Expand All @@ -16,21 +16,51 @@ module.exports = function useGhostUser(contentDir) {
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
if (!e.message.match(/no such user/i)) {
throw new errors.ProcessError(e);
}

return false;
return false
}

ghostuid = parseInt(ghostuid);
ghostgid = parseInt(ghostgid);

return {
uid: ghostuid,
gid: ghostgid
};
}

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;
}

if (!ghostUser) {
return false;
}

const stats = fs.lstatSync(contentDir);

if (stats.uid !== ghostuid && stats.gid !== ghostgid) {
if (stats.uid !== ghostUser.uid && stats.gid !== ghostUser.gid) {
return false;
}

return process.getuid() !== ghostuid;
return process.getuid() !== ghostUser.uid;
}

module.exports = {
getGhostUid: getGhostUid,
shouldUseGhostUser: shouldUseGhostUser
}
4 changes: 2 additions & 2 deletions test/unit/commands/run-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('Unit: Commands > Run', function () {
const fakeInstance = {config: config, dir: '/var/www/ghost'};
const getInstanceStub = sandbox.stub().returns(fakeInstance);
const RunCommand = proxyquire(modulePath, {
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});
const instance = new RunCommand({log: logStub}, {getInstance: getInstanceStub});
const useGhostUser = sandbox.stub(instance, 'useGhostUser').resolves();
Expand Down Expand Up @@ -76,7 +76,7 @@ describe('Unit: Commands > Run', function () {
const fakeInstance = {config: config, dir: '/var/www/ghost'};
const getInstanceStub = sandbox.stub().returns(fakeInstance);
const RunCommand = proxyquire(modulePath, {
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});
const instance = new RunCommand({log: logStub}, {getInstance: getInstanceStub});
const useGhostUser = sandbox.stub(instance, 'useGhostUser').resolves();
Expand Down
2 changes: 1 addition & 1 deletion test/unit/commands/uninstall-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ describe('Unit: Commands > Uninstall', function () {
it('step 2 (removing content folder)', function () {
const useGhostUserStub = sandbox.stub().returns(false);
const command = createInstance({
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});
command.system.getInstance.returns({dir: '/var/www/ghost'});
command.ui.sudo.resolves();
Expand Down
14 changes: 7 additions & 7 deletions test/unit/tasks/migrate-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Unit: Tasks > Migrate', function () {

const migrate = proxyquire(migratePath, {
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});

const sudoStub = sinon.stub().resolves();
Expand All @@ -55,7 +55,7 @@ describe('Unit: Tasks > Migrate', function () {

const migrate = proxyquire(migratePath, {
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});

const sudoStub = sinon.stub().resolves();
Expand All @@ -78,7 +78,7 @@ describe('Unit: Tasks > Migrate', function () {

const migrate = proxyquire(migratePath, {
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});

return migrate({instance: {config: config, dir: '/some-dir', system: {environment: 'testing'}}}).then(() => {
Expand All @@ -100,7 +100,7 @@ describe('Unit: Tasks > Migrate', function () {

const migrate = proxyquire(migratePath, {
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});

return migrate({instance: {config: config, dir: '/some-dir', system: {environment: 'testing'}}}).then(() => {
Expand All @@ -122,7 +122,7 @@ describe('Unit: Tasks > Migrate', function () {

const migrate = proxyquire(migratePath, {
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});

return migrate({instance: {config: config, dir: '/some-dir', system: {environment: 'testing'}}}).then(() => {
Expand All @@ -148,7 +148,7 @@ describe('Unit: Tasks > Migrate', function () {

const migrate = proxyquire(migratePath, {
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});

return migrate({instance: {config: config, dir: '/some-dir', system: {environment: 'testing'}}}).then(() => {
Expand All @@ -175,7 +175,7 @@ describe('Unit: Tasks > Migrate', function () {

const migrate = proxyquire(migratePath, {
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
'../utils/use-ghost-user': {shouldUseGhostUser: useGhostUserStub}
});

return migrate({instance: {config: config, dir: '/some-dir', system: {environment: 'testing'}}}).then(() => {
Expand Down
Loading

0 comments on commit e8e2fb4

Please sign in to comment.