From c9f551eb9648e756f3ac0c5ce49e257a7ac7b85b Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Mon, 13 Mar 2017 13:03:26 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20suspend=20user=20feature=20(#8114)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #8111 - Ghost returns now all (active+none active) users by default - protect login with suspended status - test permissions and add extra protection for suspending myself - if a user is suspended and tries to activate himself, he won't be able to proceed the login to get a new token --- core/server/api/users.js | 19 +- core/server/auth/auth-strategies.js | 120 +++++-- core/server/auth/oauth.js | 4 +- core/server/models/base/listeners.js | 19 ++ core/server/models/user.js | 87 +++-- core/server/translations/en.json | 2 + core/test/functional/routes/api/users_spec.js | 17 +- core/test/integration/api/api_users_spec.js | 311 +++++++++++++++++- .../integration/model/base/listeners_spec.js | 35 +- core/test/unit/auth/auth-strategies_spec.js | 78 ++++- core/test/unit/auth/oauth_spec.js | 2 +- core/test/utils/index.js | 32 +- 12 files changed, 628 insertions(+), 98 deletions(-) diff --git a/core/server/api/users.js b/core/server/api/users.js index 302a998b77..9cdef17d42 100644 --- a/core/server/api/users.js +++ b/core/server/api/users.js @@ -135,7 +135,16 @@ users = { } return canThis(options.context).edit.user(options.id).then(function () { - // if roles aren't in the payload, proceed with the edit + // CASE: can't edit my own status to inactive or locked + if (options.id === options.context.user) { + if (dataProvider.User.inactiveStates.indexOf(options.data.users[0].status) !== -1) { + return Promise.reject(new errors.NoPermissionError({ + message: i18n.t('errors.api.users.cannotChangeStatus') + })); + } + } + + // CASE: if roles aren't in the payload, proceed with the edit if (!(options.data.users[0].roles && options.data.users[0].roles[0])) { return options; } @@ -151,14 +160,18 @@ users = { var contextRoleId = contextUser.related('roles').toJSON(options)[0].id; if (roleId !== contextRoleId && editedUserId === contextUser.id) { - return Promise.reject(new errors.NoPermissionError({message: i18n.t('errors.api.users.cannotChangeOwnRole')})); + return Promise.reject(new errors.NoPermissionError({ + message: i18n.t('errors.api.users.cannotChangeOwnRole') + })); } return dataProvider.User.findOne({role: 'Owner'}).then(function (owner) { if (contextUser.id !== owner.id) { if (editedUserId === owner.id) { if (owner.related('roles').at(0).id !== roleId) { - return Promise.reject(new errors.NoPermissionError({message: i18n.t('errors.api.users.cannotChangeOwnersRole')})); + return Promise.reject(new errors.NoPermissionError({ + message: i18n.t('errors.api.users.cannotChangeOwnersRole') + })); } } else if (roleId !== contextRoleId) { return canThis(options.context).assign.role(role).then(function () { diff --git a/core/server/auth/auth-strategies.js b/core/server/auth/auth-strategies.js index 5b1ce19443..468b3682f5 100644 --- a/core/server/auth/auth-strategies.js +++ b/core/server/auth/auth-strategies.js @@ -1,8 +1,8 @@ -var models = require('../models'), +var _ = require('lodash'), + models = require('../models'), utils = require('../utils'), i18n = require('../i18n'), errors = require('../errors'), - _ = require('lodash'), strategies; strategies = { @@ -45,12 +45,23 @@ strategies = { if (token.expires > Date.now()) { return models.User.findOne({id: token.user_id}) .then(function then(model) { - if (model) { - var user = model.toJSON(), - info = {scope: '*'}; - return done(null, {id: user.id}, info); + if (!model) { + return done(null, false); } - return done(null, false); + + if (!model.isActive()) { + throw new errors.NoPermissionError({ + message: i18n.t('errors.models.user.accountSuspended') + }); + } + + var user = model.toJSON(), + info = {scope: '*'}; + + return done(null, {id: user.id}, info); + }) + .catch(function (err) { + return done(err); }); } else { return done(null, false); @@ -67,13 +78,13 @@ strategies = { * * CASES: * - via invite token - * - via normal auth + * - via normal sign in * - via setup */ ghostStrategy: function ghostStrategy(req, ghostAuthAccessToken, ghostAuthRefreshToken, profile, done) { var inviteToken = req.body.inviteToken, options = {context: {internal: true}}, - handleInviteToken, handleSetup; + handleInviteToken, handleSetup, handleSignIn; // CASE: socket hangs up for example if (!ghostAuthAccessToken || !profile) { @@ -91,18 +102,25 @@ strategies = { invite = _invite; if (!invite) { - throw new errors.NotFoundError({message: i18n.t('errors.api.invites.inviteNotFound')}); + throw new errors.NotFoundError({ + message: i18n.t('errors.api.invites.inviteNotFound') + }); } if (invite.get('expires') < Date.now()) { - throw new errors.NotFoundError({message: i18n.t('errors.api.invites.inviteExpired')}); + throw new errors.NotFoundError({ + message: i18n.t('errors.api.invites.inviteExpired') + }); } return models.User.add({ email: profile.email, name: profile.email, password: utils.uid(50), - roles: [invite.toJSON().role_id] + roles: [invite.toJSON().role_id], + ghost_auth_id: profile.id, + ghost_auth_access_token: ghostAuthAccessToken + }, options); }) .then(function destroyInvite(_user) { @@ -118,41 +136,75 @@ strategies = { return models.User.findOne({slug: 'ghost-owner', status: 'inactive'}, options) .then(function fetchedOwner(owner) { if (!owner) { - throw new errors.NotFoundError({message: i18n.t('errors.models.user.userNotFound')}); + throw new errors.NotFoundError({ + message: i18n.t('errors.models.user.userNotFound') + }); } return models.User.edit({ email: profile.email, - status: 'active' + status: 'active', + ghost_auth_id: profile.id, + ghost_auth_access_token: ghostAuthAccessToken }, _.merge({id: owner.id}, options)); }); }; - models.User.findOne({ghost_auth_id: profile.id}, options) - .then(function fetchedUser(user) { - if (user) { + handleSignIn = function handleSignIn() { + var user; + + return models.User.findOne({ghost_auth_id: profile.id}, options) + .then(function (_user) { + user = _user; + + if (!user) { + throw new errors.NotFoundError(); + } + + if (!user.isActive()) { + throw new errors.NoPermissionError({ + message: i18n.t('errors.models.user.accountSuspended') + }); + } + + return models.User.edit({ + email: profile.email, + ghost_auth_id: profile.id, + ghost_auth_access_token: ghostAuthAccessToken + }, _.merge({id: user.id}, options)); + }) + .then(function () { return user; - } + }); + }; - if (inviteToken) { - return handleInviteToken(); - } + if (inviteToken) { + return handleInviteToken() + .then(function (user) { + done(null, user, profile); + }) + .catch(function (err) { + done(err); + }); + } - return handleSetup(); - }) - .then(function updateGhostAuthToken(user) { - options.id = user.id; - - return models.User.edit({ - email: profile.email, - ghost_auth_id: profile.id, - ghost_auth_access_token: ghostAuthAccessToken - }, options); - }) - .then(function returnResponse(user) { + handleSignIn() + .then(function (user) { done(null, user, profile); }) - .catch(done); + .catch(function (err) { + if (!(err instanceof errors.NotFoundError)) { + return done(err); + } + + handleSetup() + .then(function (user) { + done(null, user, profile); + }) + .catch(function (err) { + done(err); + }); + }); } }; diff --git a/core/server/auth/oauth.js b/core/server/auth/oauth.js index 044475ffee..df273abbd7 100644 --- a/core/server/auth/oauth.js +++ b/core/server/auth/oauth.js @@ -74,9 +74,7 @@ function exchangeAuthorizationCode(req, res, next) { passport.authenticate('ghost', {session: false, failWithError: false}, function authenticate(err, user) { if (err) { - return next(new errors.UnauthorizedError({ - err: err - })); + return next(err); } if (!user) { diff --git a/core/server/models/base/listeners.js b/core/server/models/base/listeners.js index 6219048b21..9c0509aab1 100644 --- a/core/server/models/base/listeners.js +++ b/core/server/models/base/listeners.js @@ -16,6 +16,25 @@ events.on('token.added', function (tokenModel) { }); }); +/** + * WHEN user get's suspended (status=inactive), we delete his tokens to ensure + * he can't login anymore + */ +events.on('user.deactivated', function (userModel) { + var options = {id: userModel.id}; + + models.Accesstoken.destroyByUser(options) + .then(function () { + return models.Refreshtoken.destroyByUser(options); + }) + .catch(function (err) { + logging.error(new errors.GhostError({ + err: err, + level: 'critical' + })); + }); +}); + /** * WHEN timezone changes, we will: * - reschedule all scheduled posts diff --git a/core/server/models/user.js b/core/server/models/user.js index 4c3a5c4a5b..55ac734860 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -17,7 +17,9 @@ var _ = require('lodash'), bcryptHash = Promise.promisify(bcrypt.hash), bcryptCompare = Promise.promisify(bcrypt.compare), - activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked'], + activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4'], + inactiveStates = ['inactive', 'locked'], + allStates = activeStates.concat(inactiveStates), User, Users; @@ -82,6 +84,18 @@ User = ghostBookshelf.Model.extend({ model.emitChange('edited'); }, + isActive: function isActive() { + return inactiveStates.indexOf(this.get('status')) === -1; + }, + + isLocked: function isLocked() { + return this.get('status') === 'locked'; + }, + + isInactive: function isInactive() { + return this.get('status') === 'inactive'; + }, + /** * Lookup Gravatar if email changes to update image url * Generating a slug requires a db call to look for conflicting slugs @@ -214,7 +228,7 @@ User = ghostBookshelf.Model.extend({ return null; } - return this.isPublicContext() ? 'status:[' + activeStates.join(',') + ']' : null; + return this.isPublicContext() ? 'status:[' + allStates.join(',') + ']' : null; }, defaultFilters: function defaultFilters() { @@ -222,7 +236,7 @@ User = ghostBookshelf.Model.extend({ return null; } - return this.isPublicContext() ? null : 'status:[' + activeStates.join(',') + ']'; + return this.isPublicContext() ? null : 'status:[' + allStates.join(',') + ']'; } }, { orderDefaultOptions: function orderDefaultOptions() { @@ -244,7 +258,7 @@ User = ghostBookshelf.Model.extend({ // This is the only place that 'options.where' is set now options.where = {statements: []}; - var allStates = activeStates, value; + var value; // Filter on the status. A status of 'all' translates to no filter since we want all statuses if (options.status !== 'all') { @@ -309,7 +323,7 @@ User = ghostBookshelf.Model.extend({ delete data.role; data = _.defaults(data || {}, { - status: 'active' + status: 'all' }); status = data.status; @@ -595,35 +609,45 @@ User = ghostBookshelf.Model.extend({ return this.getByEmail(object.email).then(function then(user) { if (!user) { - return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.models.user.noUserWithEnteredEmailAddr')})); + return Promise.reject(new errors.NotFoundError({ + message: i18n.t('errors.models.user.noUserWithEnteredEmailAddr') + })); } - if (user.get('status') !== 'locked') { - return self.isPasswordCorrect({plainPassword: object.password, hashedPassword: user.get('password')}) - .then(function then() { - return Promise.resolve(user.set({status: 'active', last_login: new Date()}).save({validate: false})) - .catch(function handleError(err) { - // If we get a validation or other error during this save, catch it and log it, but don't - // cause a login error because of it. The user validation is not important here. - logging.error(new errors.GhostError({ - err: err, - context: i18n.t('errors.models.user.userUpdateError.context'), - help: i18n.t('errors.models.user.userUpdateError.help') - })); - - return user; - }); - }) - .catch(function onError(err) { - return Promise.reject(new errors.UnauthorizedError({ - err: err, - context: i18n.t('errors.models.user.incorrectPassword'), - help: i18n.t('errors.models.user.userUpdateError.help') - })); - }); + if (user.isLocked()) { + return Promise.reject(new errors.NoPermissionError({ + message: i18n.t('errors.models.user.accountLocked') + })); } - return Promise.reject(new errors.NoPermissionError({message: i18n.t('errors.models.user.accountLocked')})); + if (user.isInactive()) { + return Promise.reject(new errors.NoPermissionError({ + message: i18n.t('errors.models.user.accountSuspended') + })); + } + + return self.isPasswordCorrect({plainPassword: object.password, hashedPassword: user.get('password')}) + .then(function then() { + return Promise.resolve(user.set({status: 'active', last_login: new Date()}).save({validate: false})) + .catch(function handleError(err) { + // If we get a validation or other error during this save, catch it and log it, but don't + // cause a login error because of it. The user validation is not important here. + logging.error(new errors.GhostError({ + err: err, + context: i18n.t('errors.models.user.userUpdateError.context'), + help: i18n.t('errors.models.user.userUpdateError.help') + })); + + return user; + }); + }) + .catch(function onError(err) { + return Promise.reject(new errors.UnauthorizedError({ + err: err, + context: i18n.t('errors.models.user.incorrectPassword'), + help: i18n.t('errors.models.user.userUpdateError.help') + })); + }); }, function handleError(error) { if (error.message === 'NotFound' || error.message === 'EmptyResponse') { return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.models.user.noUserWithEnteredEmailAddr')})); @@ -746,7 +770,8 @@ User = ghostBookshelf.Model.extend({ return userWithEmail; } }); - } + }, + inactiveStates: inactiveStates }); Users = ghostBookshelf.Collection.extend({ diff --git a/core/server/translations/en.json b/core/server/translations/en.json index da77a6eb3e..83bb45ccfe 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -233,6 +233,7 @@ }, "incorrectPassword": "Your password is incorrect.", "accountLocked": "Your account is locked. Please reset your password to log in again by clicking the \"Forgotten password?\" link!", + "accountSuspended": "Your account was suspended.", "newPasswordsDoNotMatch": "Your new passwords do not match", "passwordRequiredForOperation": "Password is required for this operation", "expiredToken": "Expired token", @@ -380,6 +381,7 @@ "users": { "userNotFound": "User not found.", "cannotChangeOwnRole": "You cannot change your own role.", + "cannotChangeStatus": "You cannot change your own status.", "cannotChangeOwnersRole": "Cannot change Owner's role", "noPermissionToEditUser": "You do not have permission to edit this user", "noPermissionToAddUser": "You do not have permission to add this user", diff --git a/core/test/functional/routes/api/users_spec.js b/core/test/functional/routes/api/users_spec.js index 494f4321f2..06f35b425a 100644 --- a/core/test/functional/routes/api/users_spec.js +++ b/core/test/functional/routes/api/users_spec.js @@ -10,7 +10,7 @@ describe('User API', function () { var ownerAccessToken = '', editorAccessToken = '', authorAccessToken = '', - editor, author, ghostServer; + editor, author, ghostServer, inactiveUser; before(function (done) { // starting ghost automatically populates the db @@ -37,6 +37,14 @@ describe('User API', function () { }).then(function (_user2) { author = _user2; + // create inactive user + return testUtils.createUser({ + user: testUtils.DataGenerator.forKnex.createUser({email: 'test+3@ghost.org', status: 'inactive'}), + role: testUtils.DataGenerator.Content.roles[2] + }); + }).then(function (_user3) { + inactiveUser = _user3; + // by default we login with the owner return testUtils.doAuth(request); }).then(function (token) { @@ -81,7 +89,7 @@ describe('User API', function () { // owner use when Ghost starts // and two extra users, see createUser in before - jsonResponse.users.should.have.length(3); + jsonResponse.users.should.have.length(4); testUtils.API.checkResponse(jsonResponse.users[0], 'user'); testUtils.API.isISO8601(jsonResponse.users[0].last_login).should.be.true(); @@ -112,8 +120,9 @@ describe('User API', function () { should.exist(jsonResponse.users); testUtils.API.checkResponse(jsonResponse, 'users'); - jsonResponse.users.should.have.length(3); + jsonResponse.users.should.have.length(4); testUtils.API.checkResponse(jsonResponse.users[0], 'user'); + jsonResponse.users[3].status.should.eql(inactiveUser.status); done(); }); }); @@ -134,7 +143,7 @@ describe('User API', function () { should.exist(jsonResponse.users); testUtils.API.checkResponse(jsonResponse, 'users'); - jsonResponse.users.should.have.length(3); + jsonResponse.users.should.have.length(4); testUtils.API.checkResponse(jsonResponse.users[0], 'user', 'roles'); done(); }); diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index 297228d120..9f3446398b 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -1,23 +1,44 @@ var testUtils = require('../../utils'), should = require('should'), + sinon = require('sinon'), Promise = require('bluebird'), _ = require('lodash'), models = require('../../../server/models'), + errors = require('../../../server/errors'), + events = require('../../../server/events'), UserAPI = require('../../../server/api/users'), db = require('../../../server/data/db'), + sandbox = sinon.sandbox.create(), context = testUtils.context, userIdFor = testUtils.users.ids, roleIdFor = testUtils.roles.ids; describe('Users API', function () { + var eventsTriggered; + // Keep the DB clean before(testUtils.teardown); + beforeEach(function () { + eventsTriggered = {}; + + sandbox.stub(events, 'emit', function (eventName, eventObj) { + if (!eventsTriggered[eventName]) { + eventsTriggered[eventName] = []; + } + + eventsTriggered[eventName].push(eventObj); + }); + }); + beforeEach(testUtils.setup( - 'users:roles', 'users', 'user:token', 'perms:user', 'perms:role', 'perms:setting', 'perms:init', 'posts' + 'users:roles', 'users', 'user-token', 'perms:user', 'perms:role', 'perms:setting', 'perms:init', 'posts' )); - afterEach(testUtils.teardown); + afterEach(function () { + sandbox.restore(); + return testUtils.teardown(); + }); function checkForErrorType(type, done) { return function checkForErrorType(error) { @@ -459,6 +480,292 @@ describe('Users API', function () { }); }).catch(done); }); + + describe('Change status', function () { + describe('as owner', function () { + it('[success] can change status to inactive for admin', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.owner, {id: userIdFor.admin}) + ).then(function () { + Object.keys(eventsTriggered).length.should.eql(2); + should.exist(eventsTriggered['user.edited']); + should.exist(eventsTriggered['user.deactivated']); + + return models.User.findOne({id: userIdFor.admin, status: 'all'}).then(function (response) { + response.get('status').should.eql('inactive'); + }); + }); + }); + + it('[success] can change status to inactive for editor', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.owner, {id: userIdFor.editor}) + ).then(function () { + return models.User.findOne({id: userIdFor.editor, status: 'all'}).then(function (response) { + response.get('status').should.eql('inactive'); + }); + }); + }); + + it('[failure] can\' change my own status to inactive', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.owner, {id: userIdFor.owner}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + }); + + describe('as admin', function () { + it('[failure] can\'t change status to inactive for owner', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.admin, {id: userIdFor.owner}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can\'t change status to inactive for admin', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.admin, {id: userIdFor.admin2}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can\' change my own status to inactive', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.admin, {id: userIdFor.admin}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[success] can change status to inactive for editor', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.admin, {id: userIdFor.editor}) + ).then(function () { + return models.User.findOne({id: userIdFor.editor, status: 'all'}).then(function (response) { + response.get('status').should.eql('inactive'); + }); + }); + }); + }); + + describe('as editor', function () { + it('[failure] can\'t change status to inactive for owner', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.editor, {id: userIdFor.owner}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can\' change my own status to inactive', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.editor, {id: userIdFor.editor}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can\'t change status to inactive for admin', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.editor, {id: userIdFor.admin}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can\'t change status to inactive for editor', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.editor, {id: userIdFor.editor2}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[success] can change status to inactive for author', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.editor, {id: userIdFor.author}) + ).then(function () { + return models.User.findOne({id: userIdFor.author, status: 'all'}).then(function (response) { + response.get('status').should.eql('inactive'); + }); + }); + }); + }); + + describe('as author', function () { + it('[failure] can\'t change status to inactive for owner', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.author, {id: userIdFor.owner}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can\' change my own status to inactive', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.author, {id: userIdFor.author}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can\'t change status to inactive for admin', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.author, {id: userIdFor.admin}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can\'t change status to inactive for editor', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.author, {id: userIdFor.editor}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + + it('[failure] can change status to inactive for author', function () { + return UserAPI.edit( + { + users: [ + { + status: 'inactive' + } + ] + }, _.extend({}, context.author, {id: userIdFor.author2}) + ).then(function () { + throw new Error('this is not allowed'); + }).catch(function (err) { + (err instanceof errors.NoPermissionError).should.eql(true); + }); + }); + }); + }); }); describe('Destroy', function () { diff --git a/core/test/integration/model/base/listeners_spec.js b/core/test/integration/model/base/listeners_spec.js index 44942995c3..a0fc729923 100644 --- a/core/test/integration/model/base/listeners_spec.js +++ b/core/test/integration/model/base/listeners_spec.js @@ -25,7 +25,7 @@ describe('Models: listeners', function () { }; before(testUtils.teardown); - beforeEach(testUtils.setup()); + beforeEach(testUtils.setup('owner', 'user-token:0')); beforeEach(function () { sinon.stub(events, 'on', function (eventName, callback) { @@ -176,4 +176,37 @@ describe('Models: listeners', function () { }); }); }); + + describe('on user is deactived', function () { + it('ensure tokens get deleted', function (done) { + var userId = testUtils.DataGenerator.Content.users[0].id, + timeout, + retries = 0; + + (function retry() { + Promise.props({ + accesstokens: models.Accesstoken.findAll({context: {internal: true}, id: userId}), + refreshtokens: models.Refreshtoken.findAll({context: {internal: true}, id: userId}) + }).then(function (result) { + if (retries === 0) { + // trigger event after first check how many tokens the user has + eventsToRemember['user.deactivated']({ + id: userId + }); + + result.accesstokens.length.should.eql(1); + result.refreshtokens.length.should.eql(1); + } + + if (!result.accesstokens.length && !result.refreshtokens.length) { + return done(); + } + + retries = retries + 1; + clearTimeout(timeout); + timeout = setTimeout(retry, 500); + }).catch(done); + })(); + }); + }); }); diff --git a/core/test/unit/auth/auth-strategies_spec.js b/core/test/unit/auth/auth-strategies_spec.js index e380b1574d..75a7cddec4 100644 --- a/core/test/unit/auth/auth-strategies_spec.js +++ b/core/test/unit/auth/auth-strategies_spec.js @@ -114,7 +114,7 @@ describe('Auth Strategies', function () { }); describe('Bearer Strategy', function () { - var tokenStub, userStub; + var tokenStub, userStub, userIsActive; beforeEach(function () { tokenStub = sandbox.stub(Models.Accesstoken, 'findOne'); @@ -124,6 +124,7 @@ describe('Auth Strategies', function () { return fakeValidToken; } })); + tokenStub.withArgs({token: fakeInvalidToken.token}).returns(new Promise.resolve({ toJSON: function () { return fakeInvalidToken; @@ -135,6 +136,9 @@ describe('Auth Strategies', function () { userStub.withArgs({id: 3}).returns(new Promise.resolve({ toJSON: function () { return {id: 3}; + }, + isActive: function () { + return userIsActive; } })); }); @@ -143,6 +147,8 @@ describe('Auth Strategies', function () { var accessToken = 'valid-token', userId = 3; + userIsActive = true; + authStrategies.bearerStrategy(accessToken, next).then(function () { tokenStub.calledOnce.should.be.true(); tokenStub.calledWith({token: accessToken}).should.be.true(); @@ -155,6 +161,25 @@ describe('Auth Strategies', function () { }).catch(done); }); + it('should find user with valid token, but user is suspended', function (done) { + var accessToken = 'valid-token', + userId = 3; + + userIsActive = false; + + authStrategies.bearerStrategy(accessToken, next).then(function () { + tokenStub.calledOnce.should.be.true(); + tokenStub.calledWith({token: accessToken}).should.be.true(); + userStub.calledOnce.should.be.true(); + userStub.calledWith({id: userId}).should.be.true(); + next.calledOnce.should.be.true(); + next.firstCall.args.length.should.eql(1); + (next.firstCall.args[0] instanceof errors.NoPermissionError).should.eql(true); + next.firstCall.args[0].message.should.eql('Your account was suspended.'); + done(); + }).catch(done); + }); + it('shouldn\'t find user with invalid token', function (done) { var accessToken = 'invalid_token'; @@ -221,7 +246,7 @@ describe('Auth Strategies', function () { authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, profile, function (err) { should.exist(err); (err instanceof errors.NotFoundError).should.eql(true); - userFindOneStub.calledOnce.should.be.true(); + userFindOneStub.calledOnce.should.be.false(); inviteStub.calledOnce.should.be.true(); done(); }); @@ -242,7 +267,7 @@ describe('Auth Strategies', function () { authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, profile, function (err) { should.exist(err); (err instanceof errors.NotFoundError).should.eql(true); - userFindOneStub.calledOnce.should.be.true(); + userFindOneStub.calledOnce.should.be.false(); inviteStub.calledOnce.should.be.true(); done(); }); @@ -272,7 +297,7 @@ describe('Auth Strategies', function () { user.should.eql(invitedUser); profile.should.eql(invitedProfile); - userFindOneStub.calledOnce.should.be.true(); + userFindOneStub.calledOnce.should.be.false(); inviteStub.calledOnce.should.be.true(); done(); }); @@ -290,7 +315,12 @@ describe('Auth Strategies', function () { userFindOneStub.withArgs({slug: 'ghost-owner', status: 'inactive'}) .returns(Promise.resolve(_.merge({}, {status: 'inactive'}, owner))); - userEditStub.withArgs({status: 'active', email: 'test@example.com'}, { + userEditStub.withArgs({ + status: 'active', + email: 'test@example.com', + ghost_auth_id: ownerProfile.id, + ghost_auth_access_token: ghostAuthAccessToken + }, { context: {internal: true}, id: owner.id }).returns(Promise.resolve(owner)); @@ -317,11 +347,13 @@ describe('Auth Strategies', function () { }); }); - it('auth', function (done) { + it('sign in', function (done) { var ghostAuthAccessToken = '12345', req = {body: {}}, ownerProfile = {email: 'test@example.com', id: '12345'}, - owner = {id: 2}; + owner = {id: 2, isActive: function () { + return true; + }}; userFindOneStub.returns(Promise.resolve(owner)); userEditStub.withArgs({ @@ -346,5 +378,37 @@ describe('Auth Strategies', function () { done(); }); }); + + it('sign in, but user is suspended', function (done) { + var ghostAuthAccessToken = '12345', + req = {body: {}}, + ownerProfile = {email: 'test@example.com', id: '12345'}, + owner = {id: 2, isActive: function () { + return false; + }}; + + userFindOneStub.returns(Promise.resolve(owner)); + userEditStub.withArgs({ + ghost_auth_access_token: ghostAuthAccessToken, + ghost_auth_id: ownerProfile.id, + email: ownerProfile.email + }, { + context: {internal: true}, + id: owner.id + }).returns(Promise.resolve(owner)); + + authStrategies.ghostStrategy(req, ghostAuthAccessToken, null, ownerProfile, function (err, user, profile) { + should.exist(err); + err.message.should.eql('Your account was suspended.'); + + userFindOneStub.calledOnce.should.be.true(); + userEditStub.calledOnce.should.be.false(); + inviteStub.calledOnce.should.be.false(); + + should.not.exist(user); + should.not.exist(profile); + done(); + }); + }); }); }); diff --git a/core/test/unit/auth/oauth_spec.js b/core/test/unit/auth/oauth_spec.js index 9c198c3efb..f438a52f5d 100644 --- a/core/test/unit/auth/oauth_spec.js +++ b/core/test/unit/auth/oauth_spec.js @@ -346,7 +346,7 @@ describe('OAuth', function () { sandbox.stub(passport, 'authenticate', function (name, options, onSuccess) { return function () { - onSuccess(new Error('validation error')); + onSuccess(new errors.UnauthorizedError()); }; }); diff --git a/core/test/utils/index.js b/core/test/utils/index.js index ba79ec584d..f235ba6fce 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -285,12 +285,16 @@ fixtures = { }); }, - // Creates a client, and access and refresh tokens for user 3 (author) - createTokensForUser: function createTokensForUser() { + // Creates a client, and access and refresh tokens for user with index or 2 by default + createTokensForUser: function createTokensForUser(index) { return db.knex('clients').insert(DataGenerator.forKnex.clients).then(function () { - return db.knex('accesstokens').insert(DataGenerator.forKnex.createToken({user_id: DataGenerator.Content.users[2].id})); + return db.knex('accesstokens').insert(DataGenerator.forKnex.createToken({ + user_id: DataGenerator.Content.users[index || 2].id + })); }).then(function () { - return db.knex('refreshtokens').insert(DataGenerator.forKnex.createToken({user_id: DataGenerator.Content.users[2].id})); + return db.knex('refreshtokens').insert(DataGenerator.forKnex.createToken({ + user_id: DataGenerator.Content.users[index || 2].id + })); }); }, @@ -480,8 +484,8 @@ toDoList = { users: function createExtraUsers() { return fixtures.createExtraUsers(); }, - 'user:token': function createTokensForUser() { - return fixtures.createTokensForUser(); + 'user-token': function createTokensForUser(index) { + return fixtures.createTokensForUser(index); }, owner: function insertOwnerUser() { return fixtures.insertOwnerUser(); @@ -496,9 +500,7 @@ toDoList = { return permissions.init(); }, perms: function permissionsFor(obj) { - return function permissionsForObj() { - return fixtures.permissionsFor(obj); - }; + return fixtures.permissionsFor(obj); }, clients: function insertClients() { return fixtures.insertClients(); @@ -553,9 +555,12 @@ getFixtureOps = function getFixtureOps(toDos) { _.each(toDos, function (value, toDo) { var tmp; - if (toDo !== 'perms:init' && toDo.indexOf('perms:') !== -1) { + if ((toDo !== 'perms:init' && toDo.indexOf('perms:') !== -1) || toDo.indexOf('user-token:') !== -1) { tmp = toDo.split(':'); - fixtureOps.push(toDoList[tmp[0]](tmp[1])); + + fixtureOps.push(function addCustomFixture() { + return toDoList[tmp[0]](tmp[1]); + }); } else { if (!toDoList[toDo]) { throw new Error('setup todo does not exist - spell mistake?'); @@ -839,7 +844,10 @@ module.exports = { owner: DataGenerator.Content.users[0].id, admin: DataGenerator.Content.users[1].id, editor: DataGenerator.Content.users[2].id, - author: DataGenerator.Content.users[3].id + author: DataGenerator.Content.users[3].id, + admin2: DataGenerator.Content.users[6].id, + editor2: DataGenerator.Content.users[4].id, + author2: DataGenerator.Content.users[5].id } }, roles: {