Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replaced loadedPermissions with role #19896

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Replaced loadedPermissions with role #19896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
67941df
Replaced loadedPermissions with role
allouis Mar 18, 2024
c82f69f
fixup! Replaced loadedPermissions with role
allouis Mar 20, 2024
36e65c1
Merge branch 'main' into remove-loaded-permissions
allouis Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ghost/core/core/server/models/comment.js
View file
Open in desktop
Expand Up @@ -129,7 +129,7 @@ const Comment = ghostBookshelf.Model.extend({
return softDelete();
},

async permissible(commentModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission, hasMemberPermission) {
async permissible(commentModelOrId, action, context, unsafeAttrs, role, hasUserPermission, hasApiKeyPermission, hasMemberPermission) {
const self = this;

if (hasUserPermission) {
Expand Down
21 changes: 11 additions & 10 deletions ghost/core/core/server/models/invite.js
View file
Open in desktop
@@ -1,4 +1,3 @@
const _ = require('lodash');
const tpl = require('@tryghost/tpl');
const errors = require('@tryghost/errors');
const constants = require('@tryghost/constants');
Expand Down Expand Up @@ -50,7 +49,7 @@ Invite = ghostBookshelf.Model.extend({
return ghostBookshelf.Model.add.call(this, data, options);
},

async permissible(inviteModel, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
async permissible(inviteModel, action, context, unsafeAttrs, role, hasUserPermission, hasApiKeyPermission) {
const isAdd = (action === 'add');

if (!isAdd) {
Expand Down Expand Up @@ -86,15 +85,17 @@ Invite = ghostBookshelf.Model.extend({
}

let allowed = [];
if (loadedPermissions.user) {
if (_.some(loadedPermissions.user.roles, {name: 'Owner'}) ||
_.some(loadedPermissions.user.roles, {name: 'Administrator'})) {
allowed = ['Administrator', 'Editor', 'Author', 'Contributor'];
} else if (_.some(loadedPermissions.user.roles, {name: 'Editor'})) {
allowed = ['Author', 'Contributor'];
}
} else if (loadedPermissions.apiKey) {
if (role === 'Owner' || role === 'Administrator') {
allowed = ['Administrator', 'Editor', 'Author', 'Contributor'];
} else if (role === 'Admin Integration') {
allowed = ['Editor', 'Author', 'Contributor'];
} else if (role === 'Editor') {
allowed = ['Author', 'Contributor'];
}

// Cannot invite administrators when using an api key
if (context.api_key) {
allowed = allowed.filter(item => item !== 'Administrator');
}

if (allowed.indexOf(roleToInvite.get('name')) === -1) {
Expand Down
12 changes: 6 additions & 6 deletions ghost/core/core/server/models/post.js
View file
Open in desktop
Expand Up @@ -1474,7 +1474,7 @@ Post = ghostBookshelf.Model.extend({
},

// NOTE: the `authors` extension is the parent of the post model. It also has a permissible function.
permissible: async function permissible(postModel, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
permissible: async function permissible(postModel, action, context, unsafeAttrs, role, hasUserPermission, hasApiKeyPermission) {
let isContributor;
let isOwner;
let isAdmin;
Expand All @@ -1496,11 +1496,11 @@ Post = ghostBookshelf.Model.extend({
return postModel.get('status') === 'draft';
}

isContributor = loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Contributor'});
isOwner = loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Owner'});
isAdmin = loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Administrator'});
isEditor = loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Editor'});
isIntegration = loadedPermissions.apiKey && _.some(loadedPermissions.apiKey.roles, {name: 'Admin Integration'});
isContributor = role === 'Contributor';
isOwner = role === 'Owner';
isAdmin = role === 'Administrator';
isEditor = role === 'Editor';
isIntegration = role === 'Admin Integration';

isEdit = (action === 'edit');
isAdd = (action === 'add');
Expand Down
8 changes: 4 additions & 4 deletions ghost/core/core/server/models/relations/authors.js
View file
Open in desktop
Expand Up @@ -301,7 +301,7 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
return reassignPost();
},

permissible: function permissible(postModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
permissible: function permissible(postModelOrId, action, context, unsafeAttrs, role, hasUserPermission, hasApiKeyPermission) {
const self = this;
const postModel = postModelOrId;
let origArgs;
Expand Down Expand Up @@ -332,8 +332,8 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
});
}

isContributor = loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Contributor'});
isAuthor = loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Author'});
isContributor = role === 'Contributor';
isAuthor = role === 'Author';
isEdit = (action === 'edit');
isAdd = (action === 'add');
isDestroy = (action === 'destroy');
Expand Down Expand Up @@ -392,7 +392,7 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
postModelOrId,
action, context,
unsafeAttrs,
loadedPermissions,
role,
hasUserPermission,
hasApiKeyPermission
).then(({excludedAttrs}) => {
Expand Down
12 changes: 6 additions & 6 deletions ghost/core/core/server/models/role.js
View file
Open in desktop
Expand Up @@ -55,7 +55,7 @@ Role = ghostBookshelf.Model.extend({
return options;
},

permissible: function permissible(roleModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
permissible: function permissible(roleModelOrId, action, context, unsafeAttrs, role, hasUserPermission, hasApiKeyPermission) {
// If we passed in an id instead of a model, get the model
// then check the permissions
if (_.isNumber(roleModelOrId) || _.isString(roleModelOrId)) {
Expand All @@ -77,21 +77,21 @@ Role = ghostBookshelf.Model.extend({

const roleModel = roleModelOrId;

if (action === 'assign' && loadedPermissions.user) {
if (action === 'assign' && hasUserPermission) {
let checkAgainst;
if (_.some(loadedPermissions.user.roles, {name: 'Owner'})) {
if (role === 'Owner') {
checkAgainst = ['Owner', 'Administrator', 'Editor', 'Author', 'Contributor'];
} else if (_.some(loadedPermissions.user.roles, {name: 'Administrator'})) {
} else if (role === 'Administrator') {
checkAgainst = ['Administrator', 'Editor', 'Author', 'Contributor'];
} else if (_.some(loadedPermissions.user.roles, {name: 'Editor'})) {
} else if (role === 'Editor') {
checkAgainst = ['Author', 'Contributor'];
}

// Role in the list of permissible roles
hasUserPermission = roleModelOrId && _.includes(checkAgainst, roleModel.get('name'));
}

if (action === 'assign' && loadedPermissions.apiKey) {
if (action === 'assign' && context.api_key) {
// apiKey cannot 'assign' the 'Owner' role
if (roleModel.get('name') === 'Owner') {
return Promise.reject(new errors.NoPermissionError({
Expand Down
32 changes: 20 additions & 12 deletions ghost/core/core/server/models/user.js
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ const permissions = require('../services/permissions');
const urlUtils = require('../../shared/url-utils');
const activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4'];
const ASSIGNABLE_ROLES = ['Administrator', 'Editor', 'Author', 'Contributor'];
const models = require('./');

const messages = {
valueCannotBeBlank: 'Value in [{tableName}.{columnKey}] cannot be blank.',
Expand Down Expand Up @@ -777,7 +778,7 @@ User = ghostBookshelf.Model.extend({
});
},

permissible: async function permissible(userModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission) {
permissible: async function permissible(userModelOrId, action, context, unsafeAttrs, role, hasUserPermission, hasApiKeyPermission) {
const self = this;
const userModel = userModelOrId;
let origArgs;
Expand Down Expand Up @@ -823,10 +824,10 @@ User = ghostBookshelf.Model.extend({
if (context.user === userModel.get('id')) {
// If this is the same user that requests the operation allow it.
hasUserPermission = true;
} else if (loadedPermissions.user && userModel.hasRole('Owner')) {
} else if (userModel.hasRole('Owner')) {
// Owner can only be edited by owner
hasUserPermission = loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Owner'});
} else if (loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Editor'})) {
hasUserPermission = role === 'Owner';
} else if (role === 'Editor') {
// If the user we are trying to edit is an Author or Contributor, allow it
hasUserPermission = userModel.hasRole('Author') || userModel.hasRole('Contributor');
}
Expand All @@ -841,7 +842,7 @@ User = ghostBookshelf.Model.extend({
}

// Users with the role 'Editor' have complex permissions when the action === 'destroy'
if (loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Editor'})) {
if (role === 'Editor') {
// Alternatively, if the user we are trying to edit is an Author, allow it
hasUserPermission = context.user === userModel.get('id') || userModel.hasRole('Author') || userModel.hasRole('Contributor');
}
Expand All @@ -858,19 +859,26 @@ User = ghostBookshelf.Model.extend({

// CASE: i want to edit roles
if (action === 'edit' && unsafeAttrs.roles && unsafeAttrs.roles[0]) {
let role = unsafeAttrs.roles[0];
let roleId = role.id || role;
let roleToSet = unsafeAttrs.roles[0];
let roleId = roleToSet.id || role;
let roleName = roleToSet.name;
if (!roleName) {
const roleModel = await models.Role.findOne({id: roleId});
if (roleModel) {
roleName = roleModel.get('name');
}
}
let editedUserId = userModel.id;
// @NOTE: role id of logged in user
let contextRoleId = loadedPermissions.user.roles[0].id;
let contextRoleName = role;

if (roleId !== contextRoleId && editedUserId === context.user) {
if (roleName !== contextRoleName && editedUserId === context.user) {
return Promise.reject(new errors.NoPermissionError({
message: tpl(messages.cannotChangeOwnRole)
}));
}

if (limitService.isLimited('staff') && userModel.hasRole('Contributor') && role.name !== 'Contributor') {
if (limitService.isLimited('staff') && userModel.hasRole('Contributor') && roleToSet.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
Expand All @@ -897,12 +905,12 @@ User = ghostBookshelf.Model.extend({
message: tpl(messages.cannotChangeOwnersRole)
}));
}
} else if (roleId !== contextRoleId) {
} else if (roleName !== contextRoleName) {
// 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)
return permissions.canThis(context).assign.role(roleToSet)
.then(() => {
if (hasUserPermission && hasApiKeyPermission) {
return Promise.resolve();
Expand Down
11 changes: 10 additions & 1 deletion ghost/core/core/server/services/permissions/can-this.js
View file
Open in desktop
Expand Up @@ -57,6 +57,7 @@ class CanThisResult {
let hasUserPermission;
let hasApiKeyPermission;
let hasMemberPermission = false;
let role;

const checkPermission = function (perm) {
// Look for a matching action type and object type first
Expand All @@ -67,6 +68,14 @@ class CanThisResult {
return true;
};

if (loadedPermissions.user) {
role = loadedPermissions.user.roles[0].name;
} else if (loadedPermissions.member) {
role = 'Member';
} else if (loadedPermissions.apiKey) {
role = loadedPermissions.apiKey.roles[0].name;
}

if (loadedPermissions.user && _.some(loadedPermissions.user.roles, {name: 'Owner'})) {
hasUserPermission = true;
} else if (!_.isEmpty(userPermissions)) {
Expand All @@ -88,7 +97,7 @@ class CanThisResult {
// Offer a chance for the TargetModel to override the results
if (TargetModel && _.isFunction(TargetModel.permissible)) {
return TargetModel.permissible(
modelId, actType, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission, hasMemberPermission
modelId, actType, context, unsafeAttrs, role, hasUserPermission, hasApiKeyPermission, hasMemberPermission
);
}

Expand Down