Skip to content

Commit

Permalink
🐛 be able to serve locked users (#8711)
Browse files Browse the repository at this point in the history
closes #8645, closes #8710

- locked users were once part of the category "active users", but were moved to the inactive category
  -> we have added a protection of not being able to edit yourself when you are either suspended or locked
- but they are not really active users, they are restricted, because they have no access to the admin panel
- support three categories: active, inactive, restricted

* - revert restricted states
- instead, update permission layer: fallback to `all` by default, because you are able to serve any user status
- add more tests

- ATTENTION: there is a behaviour change, that a blog owner's author page can be served before setting up the blog, see conversation on slack
   -> LTS serves 404
   -> 1.0 would serve 200
  • Loading branch information
kirrg001 authored and kevinansfield committed Jul 20, 2017
1 parent 59d7302 commit 60558a7
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 42 deletions.
35 changes: 9 additions & 26 deletions core/server/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ var _ = require('lodash'),
bcryptGenSalt = Promise.promisify(bcrypt.genSalt),
bcryptHash = Promise.promisify(bcrypt.hash),
bcryptCompare = Promise.promisify(bcrypt.compare),

activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4'],
inactiveStates = ['inactive', 'locked'],
allStates = activeStates.concat(inactiveStates),
activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4'],
/**
* inactive: owner user before blog setup, suspended users
* locked user: imported users, they get a random passport
*/
inactiveStates = ['inactive', 'locked'],
allStates = activeStates.concat(inactiveStates),
User,
Users;

Expand Down Expand Up @@ -86,7 +89,7 @@ User = ghostBookshelf.Model.extend({
},

isActive: function isActive() {
return inactiveStates.indexOf(this.get('status')) === -1;
return activeStates.indexOf(this.get('status')) !== -1;
},

isLocked: function isLocked() {
Expand Down Expand Up @@ -359,6 +362,7 @@ User = ghostBookshelf.Model.extend({
options = this.filterOptions(options, 'findOne');
delete options.include;
options.include = optInc;

return query.fetch(options);
},

Expand Down Expand Up @@ -619,27 +623,6 @@ User = ghostBookshelf.Model.extend({
return Promise.reject(new errors.NoPermissionError({message: i18n.t('errors.models.user.notEnoughPermission')}));
},

setWarning: function setWarning(user, options) {
var status = user.get('status'),
regexp = /warn-(\d+)/i,
level;

if (status === 'active') {
user.set('status', 'warn-1');
level = 1;
} else {
level = parseInt(status.match(regexp)[1], 10) + 1;
if (level > 4) {
user.set('status', 'locked');
} else {
user.set('status', 'warn-' + level);
}
}
return Promise.resolve(user.save(options)).then(function then() {
return 5 - level;
});
},

// Finds the user by email, and checks the password
// @TODO: shorten this function and rename...
check: function check(object) {
Expand Down
4 changes: 1 addition & 3 deletions core/server/permissions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ function applyStatusRules(docName, method, opts) {
// Enforce status 'active' for users
if (docName === 'users') {
if (!opts.status) {
return 'active';
} else if (opts.status !== 'active') {
throw err;
return 'all';
}
}

Expand Down
56 changes: 55 additions & 1 deletion core/test/functional/routes/channel_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,12 +409,28 @@ describe('Channel Routes', function () {
});

describe('Author', function () {
var lockedUser = {
slug: 'locked-so-what',
email: 'locked@example.com',
status: 'locked'
},
suspendedUser = {
slug: 'suspended-meeh',
email: 'suspended@example.com',
status: 'inactive'
},
ownerSlug = 'ghost-owner';

before(function (done) {
testUtils.clearData().then(function () {
// we initialise data, but not a user. No user should be required for navigating the frontend
return testUtils.initData();
}).then(function () {
return testUtils.fixtures.overrideOwnerUser('ghost-owner');
return testUtils.fixtures.overrideOwnerUser(ownerSlug);
}).then(function () {
return testUtils.fixtures.insertOneUser(lockedUser);
}).then(function () {
return testUtils.fixtures.insertOneUser(suspendedUser);
}).then(function () {
done();
}).catch(done);
Expand Down Expand Up @@ -446,6 +462,44 @@ describe('Channel Routes', function () {
.end(doEnd(done));
});

it('[success] author is locked', function (done) {
request.get('/author/' + lockedUser.slug + '/')
.expect('Cache-Control', testUtils.cacheRules.public)
.expect(200)
.end(doEnd(done));
});

it('[success] author is suspended', function (done) {
request.get('/author/' + suspendedUser.slug + '/')
.expect('Cache-Control', testUtils.cacheRules.public)
.expect(200)
.end(doEnd(done));
});

it('[failure] ghost owner before blog setup', function (done) {
testUtils.fixtures.changeOwnerUserStatus({
slug: ownerSlug,
status: 'inactive'
}).then(function () {
request.get('/author/ghost-owner/')
.expect('Cache-Control', testUtils.cacheRules.public)
.expect(200)
.end(doEnd(done));
}).catch(done);
});

it('[success] ghost owner after blog setup', function (done) {
testUtils.fixtures.changeOwnerUserStatus({
slug: ownerSlug,
status: 'active'
}).then(function () {
request.get('/author/ghost-owner/')
.expect('Cache-Control', testUtils.cacheRules.public)
.expect(200)
.end(doEnd(done));
});
});

describe('RSS', function () {
it('should redirect without slash', function (done) {
request.get('/author/ghost-owner/rss')
Expand Down
16 changes: 16 additions & 0 deletions core/test/integration/api/api_users_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,22 @@ describe('Users API', function () {
(err instanceof errors.NoPermissionError).should.eql(true);
});
});

it('[failure] can\' change my own status to locked', function () {
return UserAPI.edit(
{
users: [
{
status: 'locked'
}
]
}, _.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 () {
Expand Down
13 changes: 1 addition & 12 deletions core/test/unit/permissions_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ describe('Permissions', function () {
result.should.not.eql(publicContext);
result.should.eql({
context: {},
status: 'active'
status: 'all'
});

return permissions.applyPublicRules('users', 'browse', _.extend({}, _.cloneDeep(publicContext), {status: 'active'}));
Expand All @@ -266,17 +266,6 @@ describe('Permissions', function () {
done();
}).catch(done);
});

it('should throw an error for an inactive user', function (done) {
var inactive = {context: {}, status: 'inactive'};

permissions.applyPublicRules('users', 'browse', _.cloneDeep(inactive)).then(function () {
done('Did not throw an error for inactive');
}).catch(function (err) {
(err instanceof errors.NoPermissionError).should.eql(true);
done();
});
});
});

// @TODO: move to integrations or stub
Expand Down
18 changes: 18 additions & 0 deletions core/test/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,14 @@ fixtures = {
.update(user);
},

changeOwnerUserStatus: function changeOwnerUserStatus(options) {
return db.knex('users')
.where('slug', '=', options.slug)
.update({
status: options.status
});
},

createUsersWithRoles: function createUsersWithRoles() {
return db.knex('roles').insert(DataGenerator.forKnex.roles).then(function () {
return db.knex('users').insert(DataGenerator.forKnex.users);
Expand Down Expand Up @@ -285,6 +293,16 @@ fixtures = {
});
},

insertOneUser: function insertOneUser(options) {
options = options || {};

return db.knex('users').insert(DataGenerator.forKnex.createUser({
email: options.email,
slug: options.slug,
status: options.status
}));
},

// 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 () {
Expand Down

0 comments on commit 60558a7

Please sign in to comment.