Skip to content
This repository has been archived by the owner on Aug 18, 2024. It is now read-only.

Remove $ignore_admin flag on OgAccess::userAccess() #652

Closed
pfrenssen opened this issue Jul 7, 2020 · 2 comments · Fixed by #653
Closed

Remove $ignore_admin flag on OgAccess::userAccess() #652

pfrenssen opened this issue Jul 7, 2020 · 2 comments · Fixed by #653

Comments

@pfrenssen
Copy link
Contributor

pfrenssen commented Jul 7, 2020

We currently have an $ignore_admin flag on OgAccess::userAccess() which has been ported over from Drupal 7 but is currently not used anywhere. Also in D7 this was barely used, in the 8 years after it was added it was never mentioned in any issue except the original one.

The motivation for adding this originally was as follows (ref. issue #1524480):

Currently, if a user is given administer group they are authorized for every group permission. I feel this is a limitation for anyone who adds permissions with hook_og_permission and wants to check for that permission specifically. There is no way to use hook_og_user_access_alter to do this.

This use case is now covered in D8 since if you want to check if a particular user has a particular permission in a particular group you can retrieve it like this:

$membership_manager = \Drupal::service('og.membership_manager');
$has_permission = $membership_manager->getMembership($group, $user_id)->hasPermission('the permission to check');
@MPParsley
Copy link
Collaborator

This use case is now covered in D8 since if you want to check if a particular user has a particular permission in a particular group you can retrieve it like this:

Shouldn't we update the dochead to reflect this?

   * Determines whether a user has a given privilege.
   *
   * All permission checks in OG should go through this function. This way we
   * guarantee consistent behavior, and ensure that the superuser and group
   * administrators can perform all actions.

@pfrenssen
Copy link
Contributor Author

I think this is still correct, except maybe that it should say "All access checks" instead of "All permission checks". Access includes more things than just checking if a permission is set.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants