diff --git a/core/server/data/importer/importers/data/users.js b/core/server/data/importer/importers/data/users.js index 86bef00950..269bd45770 100644 --- a/core/server/data/importer/importers/data/users.js +++ b/core/server/data/importer/importers/data/users.js @@ -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)) { diff --git a/core/server/data/schema/fixtures/fixtures.json b/core/server/data/schema/fixtures/fixtures.json index f916a24e50..b19dd1f537 100644 --- a/core/server/data/schema/fixtures/fixtures.json +++ b/core/server/data/schema/fixtures/fixtures.json @@ -470,7 +470,7 @@ "name": "Ghost", "email": "ghost-author@example.com", "status": "active", - "roles": ["Author"], + "roles": ["Contributor"], "twitter": "ghost", "facebook": "ghost", "location": "The Internet", diff --git a/core/server/models/invite.js b/core/server/models/invite.js index ed315045c8..d4002e87c9 100644 --- a/core/server/models/invite.js +++ b/core/server/models/invite.js @@ -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(); diff --git a/core/server/models/user.js b/core/server/models/user.js index e962fdcfe4..a69d6770f9 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -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) { diff --git a/test/unit/data/schema/integrity_spec.js b/test/unit/data/schema/integrity_spec.js index 41dafd6aa8..5e548ccb41 100644 --- a/test/unit/data/schema/integrity_spec.js +++ b/test/unit/data/schema/integrity_spec.js @@ -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';