diff --git a/packages/core/admin/server/controllers/role.js b/packages/core/admin/server/controllers/role.js index 4e5ab40b0c..6cb54d1313 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.assignPermissions(role.id, input.permissions); + const permissions = await roleService.updatePermissions(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 58930a0541..221a27d6f5 100644 --- a/packages/core/admin/server/services/permission/queries.js +++ b/packages/core/admin/server/services/permission/queries.js @@ -182,6 +182,7 @@ 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 d9cbd715f6..45501f840d 100644 --- a/packages/core/admin/server/services/role.js +++ b/packages/core/admin/server/services/role.js @@ -1,11 +1,22 @@ 'use strict'; const _ = require('lodash'); -const { set, omit, pick, prop, isArray, differenceWith, differenceBy } = require('lodash/fp'); +const { + set, + omit, + pick, + prop, + isArray, + differenceWith, + differenceBy, + flow, + map, +} = require('lodash/fp'); const deepEqual = require('fast-deep-equal'); const { generateTimestampCode, stringIncludes, + mapAsync, hooks: { createAsyncSeriesWaterfallHook }, } = require('@strapi/utils'); const { ApplicationError } = require('@strapi/utils').errors; @@ -315,6 +326,78 @@ 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 partialAssignPermissions = 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 @@ -368,7 +451,6 @@ const assignPermissions = async (roleId, permissions = []) => { return permissionsToReturn; }; - const addPermissions = async (roleId, permissions) => { const { conditionProvider, createMany } = getService('permission'); const { sanitizeConditions } = permissionDomain; @@ -381,6 +463,21 @@ 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; @@ -457,6 +554,7 @@ module.exports = { displayWarningIfNoSuperAdmin, addPermissions, hasSuperAdminRole, + updatePermissions: partialAssignPermissions, assignPermissions, resetSuperAdminPermissions, checkRolesIdForDeletion, diff --git a/packages/core/admin/server/validation/common-validators.js b/packages/core/admin/server/validation/common-validators.js index 42193d7ced..a1627bd72c 100644 --- a/packages/core/admin/server/validation/common-validators.js +++ b/packages/core/admin/server/validation/common-validators.js @@ -89,112 +89,102 @@ const fieldsPropertyValidation = (action) => checkNilFields(action) ); +const permission = yup + .object() + .shape({ + action: yup + .string() + .required() + .test('action-validity', 'action is not an existing permission action', function (actionId) { + // If the action field is Nil, ignore the test and let the required check handle the error + if (isNil(actionId)) { + return true; + } + + return !!getActionFromProvider(actionId); + }), + subject: yup + .string() + .nullable() + .test('subject-validity', 'Invalid subject submitted', function (subject) { + const action = getActionFromProvider(this.options.parent.action); + + if (!action) { + return true; + } + + if (isNil(action.subjects)) { + return isNil(subject); + } + + if (isArray(action.subjects)) { + return action.subjects.includes(subject); + } + + return false; + }), + properties: yup + .object() + .test('properties-structure', 'Invalid property set at ${path}', function (properties) { + const action = getActionFromProvider(this.options.parent.action); + const hasNoProperties = isEmpty(properties) || isNil(properties); + + if (!has('options.applyToProperties', action)) { + return hasNoProperties; + } + + if (hasNoProperties) { + return true; + } + + const { applyToProperties } = action.options; + + if (!isArray(applyToProperties)) { + return false; + } + + return Object.keys(properties).every((property) => applyToProperties.includes(property)); + }) + .test( + 'fields-property', + 'Invalid fields property at ${path}', + async function (properties = {}) { + const action = getActionFromProvider(this.options.parent.action); + + if (!action || !properties) { + return true; + } + + if (!actionDomain.appliesToProperty('fields', action)) { + return true; + } + + try { + await fieldsPropertyValidation(action).validate(properties.fields, { + strict: true, + abortEarly: false, + }); + return true; + } catch (e) { + // Propagate fieldsPropertyValidation error with updated path + throw this.createError({ + message: e.message, + path: `${this.path}.fields`, + }); + } + } + ), + conditions: yup.array().of(yup.string()), + }) + .noUnknown(); + const updatePermissions = yup .object() .shape({ permissions: yup .array() .required() - .of( - yup - .object() - .shape({ - action: yup - .string() - .required() - .test( - 'action-validity', - 'action is not an existing permission action', - function (actionId) { - // If the action field is Nil, ignore the test and let the required check handle the error - if (isNil(actionId)) { - return true; - } - - return !!getActionFromProvider(actionId); - } - ), - subject: yup - .string() - .nullable() - .test('subject-validity', 'Invalid subject submitted', function (subject) { - const action = getActionFromProvider(this.options.parent.action); - - if (!action) { - return true; - } - - if (isNil(action.subjects)) { - return isNil(subject); - } - - if (isArray(action.subjects)) { - return action.subjects.includes(subject); - } - - return false; - }), - properties: yup - .object() - .test( - 'properties-structure', - 'Invalid property set at ${path}', - function (properties) { - const action = getActionFromProvider(this.options.parent.action); - const hasNoProperties = isEmpty(properties) || isNil(properties); - - if (!has('options.applyToProperties', action)) { - return hasNoProperties; - } - - if (hasNoProperties) { - return true; - } - - const { applyToProperties } = action.options; - - if (!isArray(applyToProperties)) { - return false; - } - - return Object.keys(properties).every((property) => - applyToProperties.includes(property) - ); - } - ) - .test( - 'fields-property', - 'Invalid fields property at ${path}', - async function (properties = {}) { - const action = getActionFromProvider(this.options.parent.action); - - if (!action || !properties) { - return true; - } - - if (!actionDomain.appliesToProperty('fields', action)) { - return true; - } - - try { - await fieldsPropertyValidation(action).validate(properties.fields, { - strict: true, - abortEarly: false, - }); - return true; - } catch (e) { - // Propagate fieldsPropertyValidation error with updated path - throw this.createError({ - message: e.message, - path: `${this.path}.fields`, - }); - } - } - ), - conditions: yup.array().of(yup.string()), - }) - .noUnknown() - ) + .of(permission) .test( 'duplicated-permissions', 'Some permissions are duplicated (same action and subject)', @@ -213,5 +203,6 @@ module.exports = { roles, isAPluginName, arrayOfConditionNames, + permission, updatePermissions, }; diff --git a/packages/core/admin/server/validation/permission.js b/packages/core/admin/server/validation/permission.js index 15d5e9e8a3..9fedad6a03 100644 --- a/packages/core/admin/server/validation/permission.js +++ b/packages/core/admin/server/validation/permission.js @@ -4,6 +4,12 @@ const { yup, validateYupSchema } = require('@strapi/utils'); const { getService } = require('../utils'); const validators = require('./common-validators'); +const updatePermissions = yup.object().shape({ + // TODO: Add id + connect: yup.array().of(validators.permission), + disconnect: yup.array().of(yup.object().shape({ id: yup.strapiID().required() })), +}); + const checkPermissionsSchema = yup.object().shape({ permissions: yup.array().of( yup @@ -50,7 +56,7 @@ const actionsExistSchema = yup // exports module.exports = { - validatedUpdatePermissionsInput: validateYupSchema(validators.updatePermissions), + validatedUpdatePermissionsInput: validateYupSchema(updatePermissions), validatePermissionsExist: validateYupSchema(actionsExistSchema), validateCheckPermissionsInput: validateYupSchema(checkPermissionsSchema), };