Skip to content

Commit

Permalink
🎨 deny auto switch (#8086)
Browse files Browse the repository at this point in the history
* 🎨  deny auto switch

no issue

- deny auth switch after the blog was setup
- setup completed depends on the status of the user right now, see comments

* Updates from comments

- re-use statuses in user model
- update error message
  • Loading branch information
kirrg001 authored and ErisDS committed Mar 2, 2017
1 parent 144544e commit 9fafc38
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 11 deletions.
10 changes: 2 additions & 8 deletions core/server/api/authentication.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,16 +472,10 @@ authentication = {
* @return {Promise}
*/
isSetup: function isSetup() {
var tasks,
validStatuses = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked'];
var tasks;

function checkSetupStatus() {
return models.User
.where('status', 'in', validStatuses)
.count('id')
.then(function (count) {
return !!count;
});
return models.User.isSetup();
}

function formatResponse(isSetup) {
Expand Down
2 changes: 2 additions & 0 deletions core/server/auth/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var passport = require('./passport'),
authenticate = require('./authenticate'),
sync = require('./sync'),
oauth = require('./oauth'),
validation = require('./validation'),
ghostAuth = require('./ghost-auth');

exports.init = function (options) {
Expand All @@ -15,6 +16,7 @@ exports.init = function (options) {
});
};

exports.validation = validation;
exports.oauth = oauth;
exports.authorize = authorize;
exports.authenticate = authenticate;
Expand Down
15 changes: 14 additions & 1 deletion core/server/auth/passport.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,21 @@ exports.init = function initPassport(options) {
passport.use(new ClientPasswordStrategy(authStrategies.clientPasswordStrategy));
passport.use(new BearerStrategy(authStrategies.bearerStrategy));

// CASE: use switches from password to ghost and back
// If we don't clean up the database, it can happen that the auth switch validation fails
if (authType !== 'ghost') {
return resolve({passport: passport.initialize()});
return models.Client.findOne({slug: 'ghost-auth'})
.then(function (client) {
if (!client) {
return;
}

return models.Client.destroy({id: client.id});
})
.then(function () {
resolve({passport: passport.initialize()});
})
.catch(reject);
}

var ghostOAuth2Strategy = new GhostOAuth2Strategy({
Expand Down
31 changes: 31 additions & 0 deletions core/server/auth/validation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
var Promise = require('bluebird'),
models = require('../models'),
errors = require('../errors');

/**
* If the setup is completed and...
* 1. the public client does exist, deny to switch to local
* 2. the public client does not exist, deny to switch to remote
*/
exports.switch = function validate(options) {
var authType = options.authType;

return models.User.isSetup()
.then(function (isSetup) {
if (!isSetup) {
return;
}

return models.Client.findOne({slug: 'ghost-auth'}, {columns: 'id'})
.then(function (client) {
if ((client && authType === 'password') || !client && authType === 'ghost') {
return Promise.reject(new errors.InternalServerError({
code: 'AUTH_SWITCH',
message: 'Switching the auth strategy is not allowed.',
context: 'Please reset your database and start from scratch.',
help: 'NODE_ENV=production|development knex-migrator reset && NODE_ENV=production|development knex-migrator init\n'
}));
}
});
});
};
6 changes: 5 additions & 1 deletion core/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ function init() {
parentApp = require('./app')();

debug('Express Apps done');

}).then(function () {
return auth.validation.switch({
authType: config.get('auth:type')
});
}).then(function () {
// runs asynchronous
auth.init({
authType: config.get('auth:type'),
Expand Down
13 changes: 13 additions & 0 deletions core/server/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,19 @@ User = ghostBookshelf.Model.extend({
return self.edit.call(self, userData, options);
},

/**
* Right now the setup of the blog depends on the user status.
* @TODO: see https://github.com/TryGhost/Ghost/issues/8003
*/
isSetup: function isSetup() {
return this
.where('status', 'in', activeStates)
.count('id')
.then(function (count) {
return !!count;
});
},

permissible: function permissible(userModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
userModel = userModelOrId,
Expand Down
28 changes: 27 additions & 1 deletion core/test/unit/auth/passport_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ describe('Ghost Passport', function () {
return Promise.resolve(client);
});

sandbox.stub(models.Client, 'destroy').returns(Promise.resolve());

sandbox.stub(models.Client, 'add', function () {
client = new models.Client(testUtils.DataGenerator.forKnex.createClient());
return Promise.resolve(client);
Expand Down Expand Up @@ -92,13 +94,37 @@ describe('Ghost Passport', function () {
should.exist(response.passport);
passport.use.callCount.should.eql(2);

models.Client.findOne.called.should.eql(false);
models.Client.findOne.called.should.eql(true);
models.Client.destroy.called.should.eql(false);
models.Client.add.called.should.eql(false);
FakeGhostOAuth2Strategy.prototype.setClient.called.should.eql(false);
FakeGhostOAuth2Strategy.prototype.registerClient.called.should.eql(false);
FakeGhostOAuth2Strategy.prototype.updateClient.called.should.eql(false);
});
});

it('initialise passport with passport auth type [auth client exists]', function () {
return models.Client.add({slug: 'ghost-auth'})
.then(function () {
models.Client.add.called.should.eql(true);
models.Client.add.reset();

return GhostPassport.init({
authType: 'passport'
});
})
.then(function (response) {
should.exist(response.passport);
passport.use.callCount.should.eql(2);

models.Client.findOne.called.should.eql(true);
models.Client.destroy.called.should.eql(true);
models.Client.add.called.should.eql(false);
FakeGhostOAuth2Strategy.prototype.setClient.called.should.eql(false);
FakeGhostOAuth2Strategy.prototype.registerClient.called.should.eql(false);
FakeGhostOAuth2Strategy.prototype.updateClient.called.should.eql(false);
});
});
});

describe('auth_type: ghost', function () {
Expand Down
78 changes: 78 additions & 0 deletions core/test/unit/auth/validation_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
var should = require('should'),
sinon = require('sinon'),
Promise = require('bluebird'),
auth = require('../../../server/auth'),
models = require('../../../server/models'),
sandbox = sinon.sandbox.create();

describe('UNIT: auth validation', function () {
before(function () {
models.init();
});

beforeEach(function () {
sandbox.restore();
});

describe('ghost is enabled', function () {
it('[success]', function () {
sandbox.stub(models.User, 'isSetup').returns(Promise.resolve(false));

return auth.validation.switch({
authType: 'ghost'
});
});

it('[success]', function () {
sandbox.stub(models.User, 'isSetup').returns(Promise.resolve(true));
sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(true));

return auth.validation.switch({
authType: 'ghost'
});
});

it('[failure]', function () {
sandbox.stub(models.User, 'isSetup').returns(Promise.resolve(true));
sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(true));

return auth.validation.switch({
authType: 'password'
}).catch(function (err) {
should.exist(err);
err.code.should.eql('AUTH_SWITCH');
});
});
});

describe('password is enabled', function () {
it('[success]', function () {
sandbox.stub(models.User, 'isSetup').returns(Promise.resolve(false));

return auth.validation.switch({
authType: 'password'
});
});

it('[success]', function () {
sandbox.stub(models.User, 'isSetup').returns(Promise.resolve(true));
sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(false));

return auth.validation.switch({
authType: 'password'
});
});

it('[failure]', function () {
sandbox.stub(models.User, 'isSetup').returns(Promise.resolve(true));
sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(true));

return auth.validation.switch({
authType: 'ghost'
}).catch(function (err) {
should.exist(err);
err.code.should.eql('AUTH_SWITCH');
});
});
});
});

0 comments on commit 9fafc38

Please sign in to comment.