From 9fafc38b79240b243379f8a53ded635986d8af56 Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Thu, 2 Mar 2017 20:50:58 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20=20deny=20auto=20switch=20(#8086?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🎨 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 --- core/server/api/authentication.js | 10 +--- core/server/auth/index.js | 2 + core/server/auth/passport.js | 15 ++++- core/server/auth/validation.js | 31 ++++++++++ core/server/index.js | 6 +- core/server/models/user.js | 13 +++++ core/test/unit/auth/passport_spec.js | 28 ++++++++- core/test/unit/auth/validation_spec.js | 78 ++++++++++++++++++++++++++ 8 files changed, 172 insertions(+), 11 deletions(-) create mode 100644 core/server/auth/validation.js create mode 100644 core/test/unit/auth/validation_spec.js diff --git a/core/server/api/authentication.js b/core/server/api/authentication.js index dcda4186f5..d46857510b 100644 --- a/core/server/api/authentication.js +++ b/core/server/api/authentication.js @@ -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) { diff --git a/core/server/auth/index.js b/core/server/auth/index.js index 29a6aec9a1..747a9745bd 100644 --- a/core/server/auth/index.js +++ b/core/server/auth/index.js @@ -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) { @@ -15,6 +16,7 @@ exports.init = function (options) { }); }; +exports.validation = validation; exports.oauth = oauth; exports.authorize = authorize; exports.authenticate = authenticate; diff --git a/core/server/auth/passport.js b/core/server/auth/passport.js index d89c3d2610..d18336e871 100644 --- a/core/server/auth/passport.js +++ b/core/server/auth/passport.js @@ -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({ diff --git a/core/server/auth/validation.js b/core/server/auth/validation.js new file mode 100644 index 0000000000..9271d33e8c --- /dev/null +++ b/core/server/auth/validation.js @@ -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' + })); + } + }); + }); +}; diff --git a/core/server/index.js b/core/server/index.js index 57be680aff..11706a83f9 100644 --- a/core/server/index.js +++ b/core/server/index.js @@ -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'), diff --git a/core/server/models/user.js b/core/server/models/user.js index f1371da135..4c3a5c4a5b 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -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, diff --git a/core/test/unit/auth/passport_spec.js b/core/test/unit/auth/passport_spec.js index bda76706a1..dd32eb17af 100644 --- a/core/test/unit/auth/passport_spec.js +++ b/core/test/unit/auth/passport_spec.js @@ -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); @@ -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 () { diff --git a/core/test/unit/auth/validation_spec.js b/core/test/unit/auth/validation_spec.js new file mode 100644 index 0000000000..ccb2eb4126 --- /dev/null +++ b/core/test/unit/auth/validation_spec.js @@ -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'); + }); + }); + }); +});