Added staff user limit

refs: https://github.com/TryGhost/Team/issues/510

- In the case that host config is provided, keep staff users within the limiti
- The definition of a staff user is a user with a role other than Contributor, and whose status is not inactive
   - Contributors don't count
   - Suspended (status inactive) users don't count
   - Locked users DO count
   - Invited users DO count
- You can't invite more staff users whilst there are pending invites
- You can't unsuspend a user, or change the role on a user in such a way as will take you over your limit
- You can't import staff users - all imported users are automatically set to Contributors
- As part of this work, we are changing the default Ghost user to a Contributor otherwise it uses up a staff user

Note: there is one known active bug with this commit.
- Assume you have one remaining user within your limit. You send an invite, this works.
- You cannot "resend" that invite, it will think you're sending a new invite and hit the limit
- You must "revoke" that invite first, and create a new one
- This bug exists because the resend function uses the add endpoint & does a delete+add, but this hits the permission check before the delete
This commit is contained in:
Hannah Wolfe 2021-03-03 12:48:04 +00:00
parent 7e2da2adee
commit e30b9735fa
5 changed files with 34 additions and 4 deletions

View File

@ -2,6 +2,7 @@ const debug = require('ghost-ignition').debug('importer:users');
const _ = require('lodash');
const BaseImporter = require('./base');
const models = require('../../../../models');
const limitService = require('../../../../services/limits');
class UsersImporter extends BaseImporter {
constructor(allDataFromFile) {
@ -58,6 +59,14 @@ class UsersImporter extends BaseImporter {
role = {name: 'Author'};
}
// If this site has any sort of staff limit, set all imported users to contributors
// Any other sort of logic for counting staff users would be too complex in this scenario
// So we essentially don't allow importing staff users
// The roles can be changed afterwards if the limit permits
if (limitService.isLimited('staff')) {
role = {name: 'Contributor'};
}
_.each(this.dataToImport, (obj) => {
if (attachedRole.user_id === obj.id) {
if (!_.isArray(obj.roles)) {

View File

@ -470,7 +470,7 @@
"name": "Ghost",
"email": "ghost-author@example.com",
"status": "active",
"roles": ["Author"],
"roles": ["Contributor"],
"twitter": "ghost",
"facebook": "ghost",
"location": "The Internet",

View File

@ -5,6 +5,7 @@ const errors = require('@tryghost/errors');
const constants = require('@tryghost/constants');
const security = require('@tryghost/security');
const settingsCache = require('../services/settings/cache');
const limitService = require('../services/limits');
const ghostBookshelf = require('./base');
let Invite;
@ -43,9 +44,15 @@ Invite = ghostBookshelf.Model.extend({
return ghostBookshelf.Model.add.call(this, data, options);
},
permissible(inviteModel, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
async permissible(inviteModel, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
const isAdd = (action === 'add');
if (isAdd && limitService.isLimited('staff')) {
// CASE: if your site is limited to a certain number of staff users
// Inviting a new user requires we check we won't go over the limit
await limitService.errorIfWouldGoOverLimit('staff');
}
if (!isAdd) {
if (hasUserPermission && hasApiKeyPermission) {
return Promise.resolve();

View File

@ -4,6 +4,7 @@ const validator = require('validator');
const ObjectId = require('bson-objectid');
const ghostBookshelf = require('./base');
const baseUtils = require('./base/utils');
const limitService = require('../services/limits');
const {i18n} = require('../lib/common');
const errors = require('@tryghost/errors');
const security = require('@tryghost/security');
@ -655,7 +656,7 @@ User = ghostBookshelf.Model.extend({
});
},
permissible: function permissible(userModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
permissible: async function permissible(userModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
const self = this;
const userModel = userModelOrId;
let origArgs;
@ -687,6 +688,11 @@ User = ghostBookshelf.Model.extend({
});
}
// If we have a staff user limit & the user is being unsuspended
if (limitService.isLimited('staff') && action === 'edit' && unsafeAttrs.status && unsafeAttrs.status === 'active' && userModel.get('status') === 'inactive') {
await limitService.errorIfWouldGoOverLimit('staff');
}
if (action === 'edit') {
// Users with the role 'Editor', 'Author', and 'Contributor' have complex permissions when the action === 'edit'
// We now have all the info we need to construct the permissions
@ -741,6 +747,13 @@ User = ghostBookshelf.Model.extend({
}));
}
if (limitService.isLimited('staff') && userModel.hasRole('Contributor') && role.name !== 'Contributor') {
// CASE: if your site is limited to a certain number of staff users
// Trying to change the role of a contributor, who doesn't count towards the limit, to any other role requires a limit check
// To check if it's OK to add one more staff user
await limitService.errorIfWouldGoOverLimit('staff');
}
return User.getOwnerUser()
.then((owner) => {
// CASE: owner can assign role to any user
@ -765,6 +778,7 @@ User = ghostBookshelf.Model.extend({
// CASE: you are trying to change a role, but you are not owner
// @NOTE: your role is not the same than the role you try to change (!)
// e.g. admin can assign admin role to a user, but not owner
return permissions.canThis(context).assign.role(role)
.then(() => {
if (hasUserPermission && hasApiKeyPermission) {

View File

@ -33,7 +33,7 @@ const defaultSettings = require('../../../../core/server/data/schema/default-set
describe('DB version integrity', function () {
// Only these variables should need updating
const currentSchemaHash = '559cdbb49a7eeb5758caf0c6e3bf790d';
const currentFixturesHash = '370d0da0ab7c45050b2ff30bce8896ba';
const currentFixturesHash = '5f6f69931811c407dff01da9ef9667f4';
const currentSettingsHash = 'e1f85186a7c7ed76064b6026f68c6321';
const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';