feat: filter out internal actions instead of doing partial updates

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);
 

packages/core/admin/server/services/permission/queries.js

@@ -182,7 +182,6 @@ const cleanPermissionsInDatabase = async () => {
 
 module.exports = {
   createMany,
-  update,
   findMany,
   deleteByRolesIds,
   deleteByIds,

packages/core/admin/server/services/role.js

@@ -1,22 +1,11 @@
 'use strict';
 
 const _ = require('lodash');
-const {
+const { set, omit, pick, prop, isArray, differenceWith, differenceBy } = require('lodash/fp');
-  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;
@@ -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,

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),
 };