Change Custom Permissions List permission to administer permissions

Yup, just noticed this issue.

It's a little ironic since a main motivation of Custom Permissions is to make permissions more fine-grained... but yeah 'administer permissions' is the correct permission here not 'access content'!

I'm suggesting to add more granular permission to "Administer custom permissions" which can be provided by default from the module itself.

See related issue here: #3035483: Update [Custom Permissions] module from 2.0-beta1 (~2.0) to 2.0-beta2 (~2.0) .

Of course along with the provided patch here.

Making this critical since it imposes some security implications.

Hi!

I just added one new permission to administer the module and added a hook_update to give this new permission to the roles that already were able to administer the module and a test.

Patch attached.

Thanks @gnuget for the quick action on this.

Allow me to add a couple of reviews on the patch:

1. +++ b/config_perms.install
   @@ -29,3 +29,20 @@ function config_perms_update_8201() {
   +    if ($role->hasPermission('administer site configuration')) {
   

   I believe checking if the role had "administer permissions" makes more sense.

   As "administer site configuration" is unrelated to managing permissions.

2. +++ b/config_perms.permissions.yml
   @@ -1,2 +1,6 @@
   +  title: 'Administer config permissions'
   

   Even though the module machine name is config_perms, the human readable name is "Custom Permissions".
   I think this will create confusion and I suggest renaming it to "Administer custom permissions" instead.

Hi @mohammed.

1. I believe checking if the role had "administer permissions" makes more sense.

   The administer site configuration was the permission used in the CustomPermsentity.php. (yup, it was poorly selected glad that we are going to fix it in this issue) you can see that here:

    index 9e2a1ec..f5ba86f 100755
   --- a/src/Entity/CustomPermsEntity.php
   +++ b/src/Entity/CustomPermsEntity.php
   @@ -17,7 +17,7 @@ use Drupal\config_perms\CustomPermsEntityInterface;
     *     }
     *   },
     *   config_prefix = "custom_perms_entity",
   - *   admin_permission = "administer site configuration",
   + *   admin_permission = "administer config permissions",
     *   entity_keys = {
     *     "id" = "id",
     *     "label" = "label",
   

2. FIxed! in this new patch.

Thank you for the review.

Great! I think we can mark as RTBC now.

Thanks for your work on this!

Yay, fixed!