diff --git a/packages/core/admin/server/controllers/role.js b/packages/core/admin/server/controllers/role.js index c0ab3d4bf0..4e5ab40b0c 100644 --- a/packages/core/admin/server/controllers/role.js +++ b/packages/core/admin/server/controllers/role.js @@ -146,7 +146,7 @@ module.exports = { return ctx.notFound('role.notFound'); } - const permissions = await roleService.partialPermissionUpdate(role.id, input.permissions); + const permissions = await roleService.assignPermissions(role.id, input.permissions); const sanitizedPermissions = permissions.map(permissionService.sanitizePermission); diff --git a/packages/core/admin/server/services/permission/queries.js b/packages/core/admin/server/services/permission/queries.js index 221a27d6f5..58930a0541 100644 --- a/packages/core/admin/server/services/permission/queries.js +++ b/packages/core/admin/server/services/permission/queries.js @@ -182,7 +182,6 @@ const cleanPermissionsInDatabase = async () => { module.exports = { createMany, - update, findMany, deleteByRolesIds, deleteByIds, diff --git a/packages/core/admin/server/services/role.js b/packages/core/admin/server/services/role.js index 1bc55d5ece..0c98b48fd8 100644 --- a/packages/core/admin/server/services/role.js +++ b/packages/core/admin/server/services/role.js @@ -1,22 +1,11 @@ 'use strict'; const _ = require('lodash'); -const { - set, - omit, - pick, - prop, - isArray, - differenceWith, - differenceBy, - flow, - map, -} = require('lodash/fp'); +const { set, omit, pick, prop, isArray, differenceWith, differenceBy } = require('lodash/fp'); const deepEqual = require('fast-deep-equal'); const { generateTimestampCode, stringIncludes, - mapAsync, hooks: { createAsyncSeriesWaterfallHook }, } = require('@strapi/utils'); const { ApplicationError } = require('@strapi/utils').errors; @@ -326,78 +315,6 @@ const displayWarningIfNoSuperAdmin = async () => { } }; -/** - * Partially update a role permissions in database. - * Permissions will use the format of: - * { - * connect: [permission1, permission2], - * disconnect: [permission3, permission4] - * } - * - * @param {*} roleId - * @param {*} permissions - */ -const partialPermissionUpdate = async (roleId, permissions = {}) => { - const superAdmin = await getService('role').getSuperAdmin(); - const isSuperAdmin = superAdmin && superAdmin.id === roleId; - const permissionsWithRole = flow( - map(set('role', roleId)), // Assign role - map(permissionDomain.create) // Map permission to domain - ); - - const connect = permissionsWithRole(permissions.connect) || []; - const disconnect = permissionsWithRole(permissions.disconnect) || []; - - await validatePermissionsExist(connect); - - const permissionsToCreate = connect.filter((permission) => !permission.id); - const permissionsToUpdate = connect.filter((permission) => permission.id); - const permissionsToDelete = disconnect; - - const existingPermissions = await getService('permission').findMany({ - where: { role: { id: roleId } }, - populate: ['role'], - }); - - // Find permissions that do not exist in db - const invalidUpdatePermissions = differenceBy('id', permissionsToUpdate, existingPermissions); - const invalidDeletePermissions = differenceBy('id', permissionsToDelete, existingPermissions); - - if (invalidUpdatePermissions.length !== 0) { - throw new ApplicationError('Some permissions to update do not exist'); - } - - if (invalidDeletePermissions.length !== 0) { - throw new ApplicationError('Some permissions to delete do not exist'); - } - - // Array of final permissions to return - const permissionsToReturn = differenceBy('id', existingPermissions, [ - ...permissionsToDelete, - ...permissionsToUpdate, - ]); - - if (permissionsToDelete.length > 0) { - await getService('permission').deleteByIds(permissionsToDelete.map(prop('id'))); - } - - if (permissionsToCreate.length > 0) { - const newPermissions = await addPermissions(roleId, permissionsToCreate); - permissionsToReturn.push(...newPermissions); - } - - if (permissionsToUpdate.length > 0) { - const updatedPermissions = await updatePermissions(roleId, permissionsToUpdate); - permissionsToReturn.push(...updatedPermissions); - } - - if (!isSuperAdmin && (connect.length || disconnect.length)) { - await getService('metrics').sendDidUpdateRolePermissions(); - } - - return permissionsToReturn; -}; - /** * Assign permissions to a role * @param {string|int} roleId - role ID @@ -406,6 +323,13 @@ const partialPermissionUpdate = async (roleId, permissions = {}) => { const assignPermissions = async (roleId, permissions = []) => { await validatePermissionsExist(permissions); + // Internal actions are not handled by the role service, so any permission + // with an internal action is filtered out + const internalActions = getService('permission') + .actionProvider.values() + .filter((action) => action.section === 'internal') + .map((action) => action.actionId); + const superAdmin = await getService('role').getSuperAdmin(); const isSuperAdmin = superAdmin && superAdmin.id === roleId; const assignRole = set('role', roleId); @@ -425,13 +349,13 @@ const assignPermissions = async (roleId, permissions = []) => { arePermissionsEqual, permissionsWithRole, existingPermissions - ); + ).filter((permission) => !internalActions.includes(permission.action)); const permissionsToDelete = differenceWith( arePermissionsEqual, existingPermissions, permissionsWithRole - ); + ).filter((permission) => !internalActions.includes(permission.action)); const permissionsToReturn = differenceBy('id', permissionsToDelete, existingPermissions); @@ -463,22 +387,6 @@ const addPermissions = async (roleId, permissions) => { return createMany(permissionsWithRole); }; -const updatePermissions = async (roleId, permissions) => { - // TODO: Update many - const { conditionProvider, update } = getService('permission'); - const { sanitizeConditions } = permissionDomain; - - const permissionsWithRole = permissions - .map(set('role', roleId)) - .map(sanitizeConditions(conditionProvider)) - .map(permissionDomain.create); - - return mapAsync(permissionsWithRole, (permission) => { - const { id, ...attributes } = permission; - return update({ id }, attributes); - }); -}; - const isContentTypeAction = (action) => action.section === CONTENT_TYPE_SECTION; /** @@ -554,7 +462,6 @@ module.exports = { displayWarningIfNoSuperAdmin, addPermissions, hasSuperAdminRole, - partialPermissionUpdate, assignPermissions, resetSuperAdminPermissions, checkRolesIdForDeletion, diff --git a/packages/core/admin/server/validation/permission.js b/packages/core/admin/server/validation/permission.js index b1d2618538..a73f73fe95 100644 --- a/packages/core/admin/server/validation/permission.js +++ b/packages/core/admin/server/validation/permission.js @@ -4,21 +4,6 @@ const { yup, validateYupSchema } = require('@strapi/utils'); const { getService } = require('../utils'); const validators = require('./common-validators'); -const updatePermissions = yup.object().shape({ - permissions: yup - .object() - .shape({ - connect: yup.array().of( - yup.object().shape({ - id: yup.strapiID(), // If id is not provided, it will be generated - ...validators.permission.fields, - }) - ), - disconnect: yup.array().of(yup.object().shape({ id: yup.strapiID().required() })), - }) - .required(), -}); - const checkPermissionsSchema = yup.object().shape({ permissions: yup.array().of( yup @@ -61,9 +46,8 @@ const actionsExistSchema = yup .test('actions-exist', '', checkPermissionsExist); // exports - module.exports = { - validatedUpdatePermissionsInput: validateYupSchema(updatePermissions), + validatedUpdatePermissionsInput: validateYupSchema(validators.updatePermissions), validatePermissionsExist: validateYupSchema(actionsExistSchema), validateCheckPermissionsInput: validateYupSchema(checkPermissionsSchema), };