Routes are using non existing global permission

I have noticed other issues with the access to the routes and the group permission entity. I am investigating the issue. I will upload another patch when I am done.

I have added MR to fix all issues listed above...

Hello @LOBsTerr
I agree with the first 2 points.
I wonder if the last one is the way to go. Here is why:

1. Usually the access to entity routes are handled by adding _entity_access as a requirements. It is not possible to do this with some group permission routes (e.g. delete route) because we do not have the group permission entity in the route
2. The _entity_access grantees a correct access check on the route based on the entity access handler. So we don't have to add other access requirements (e.g. permissions) to the route explicitly. This grantees that any change that is being applied to the entity access handler, will automatically be applied to the route access without having to change the access requirements on the route.
3. Because of the above 2 points, I have added a custom GroupPermissionEntityAccessCheck.
4. Without applying the entity access check on routes (e.g. delete route and revisions route) and having only the _group_permission requirements, users will get fatal error (see below) when visiting the delete/revisions pages for not existing group permission entity. This is because the user has the access to the route anyway. Meaning the user has access to the route even if the group permission entity is not exist!

Here are the fatal errors:

The website encountered an unexpected error. Please try again later.
Error: Call to a member function getGroup() on null in Drupal\group_permissions\Form\GroupPermissionDeleteForm->getQuestion() (line 64 of modules/contrib/group_permissions/src/Form/GroupPermissionDeleteForm.php). 

The website encountered an unexpected error. Please try again later.
Error: Call to a member function getGroup() on null in Drupal\group_permissions\Controller\GroupPermissionsController->revisionOverview() (line 316 of modules/contrib/group_permissions/src/Controller/GroupPermissionsController.php).

I think we have 2 options to solve the issue:

1. Keep the GroupPermissionEntityAccessCheck. But we should simplify it, as now we have one access point check "one group permission". So we no longer need to pass the operations but instead we should pass TRUE (e.g. ->setRequirement('_group_permission_entity_access', 'TRUE'))
2. Add the group permission entity to all routes and then use _entity_access.

Let my know your thoughts please.

@LOBsTerr
Thanks for moving on with this issue. Here is my feedback about the latest changes and my previous comment #13

Are you sure you checked the latest changes, we have group entity in every route (including revisions), and _group_permission access check works without any fatal errros.

Yes, I have checked and test it the last changes. I know we have the group entity in all routes :) BUT what I have mentioned in #13 is the "group permission" entity!
See below for more info. about the fatal errors.

Also we can drop GroupPermissionAccessControlHandler, since it is not used.

I wonder if this is ideal! By doing this, we lose the access check on the entity type. Meaning websites that are using a different way to create "group permission" entities for example using "rest/json api", might end up not being able to do so or might have vulnerability impact.
This is because they might end up using the "administer group permission entities" global permission. Granting this permission to all global roles that should create "group permissions" entities, will skip the permission granted in the group level. This will have a vulnerability impact. Because if they do so, unprivileged users, will then be able to create "group permissions" entities in the groups that are not suppose to do so! On the other hand, a members who should be able to create "group permissions" entities based on the permission in the group, might not be able to create entities because they do not have the global permission "administer group permission entities" granted to them!

My idea is still to use "override group permissions" to control access to all routes related to group permissions

I am still in favor of applying the entity access on the routes :) This is because of the reasons I mentioned in #13...
On the other hand, every time we need to apply new access rules to the entity, we have to apply them in the routes! Example #3276738: Enabling per-group permissions by group type

I can't see any fatal errors, because we have group entity for all group permissions related routes. I have tested.

Here are the steps to reproduce:

• Create new group. Make sure there is no group permission entity for that entity
• Go to: Group permissions by clicking on the "Group permissions" tab
• You should see the delete and revisions tabs.
• When you click on the tabs or visit group/XX/permissions/delete, you should see the fatal errors.

The issue here is that, the user has access to the delete and revisions pages for a non existing "group permission" entity
!
Let my know your thoughts please.

@LOBsTerr
I finally had the chance to review the MR. Please have a look...

@LOBsTerr Thanks for the quick fix...
Just one has been missed... see here https://git.drupalcode.org/project/group_permissions/-/merge_requests/27...

Basically, I believe we won't need group_permissions_menu_local_tasks_alter anymore

@LOBsTerr Sorry I didn't noticed that the other PR has been merged... Thanks
I wonder what is the plan for Group permission 2.0?