Check group content entity operation access

[pfrenssen]

This is a followup for #192. Now that we can assign default roles to users which are populated with a set of default permissions for doing CRUD operations on group content, we should use these when checking access.

This PR will e.g. allow members to edit nodes in a group if they have the OG permission "edit any node_type node".

[pfrenssen]

This builds on #192 so it contains all that code. The actual implementation is in the last commit 6aca08c.

This still needs tests.

[pfrenssen]

For the moment to get OG permissions we get them from the OgMembership object. It's probably a good idea to make some specific helper methods to retrieve the set of OG permissions for group content for a particular user, whether they are member or not.

[pfrenssen]

Just noticed that this test is wrong, we should use array_merge() instead of the array union operator + for indexed arrays. If I fix this the test fails. The group content permissions are not added :(

[pfrenssen]

I'm using static methods because I'm following the established pattern, but this is now going to break the unit tests again, since I am statically calling the PermissionManager service. This is going to require some trickery again to make the tests pass :(

[pfrenssen]

I have converted OgAccess into a service in #226. This solves the problem with the unit tests.

Merging this was a bit awkward, I'm going to rebase this branch on top of #226 so that the commit progression is more logical.

[amitaibu]

This will require a re-roll

[pfrenssen]

Picking this back up, will first merge in latest changes and try to get a green test run, then I'll have a look to make a list of what still needs to be done here.

[pfrenssen]

I am thinking how we can best provide the entity operation permissions.

We have two types of permissions:

1. "Group level permissions", such as 'subscribe without approval' and 'manage users'. These are currently provided by the PermissionEvent subscribers.
2. "Entity operation level permissions" on group content, so that we can allow members to create and edit group content.

In D7 we only supplied a hard coded set of permissions for the Node module, but in D8's everything-is-an-entity world we should extend this to all entities. A good example use case is the OG Menu module, in which each menu is group content type of the custom OgMenuInstance entity. In order to allow certain group roles to create or edit OG Menus we need know that OG Menu exposes this entity type, and that it supports certain operations on it.

In core we don't have this information currently (ref. #222). We need to retrieve it somehow, through a hook or event listener.

I am now thinking of extending the current PermissionEvent to handle both the group level and entity operation level permissions. This seems better from a DX perspective, so that modules don't need to implement two subscribers to declare a set of permissions.

The problem with the entity operation permissions in Drupal is that there is no formal framework, and the permissions are based on human language. A good example is the basic 'edit own article content' permission - neither the operation ('update') or the entity type ('node') are even mentioned in the permission, it's impossile to parse the needed information out of it.

So we need to be able to associate a permission with an operation, and with an entity type and bundle. We also need to distinguish between a user having permission to operate on their own content, or on all content: ('edit own article content' vs 'edit any article content'). We also need the machine name and the title.

It seems like it would be a good idea to use a class for this, so something like the 'edit any article content' can be provided in the permission subscriber using a new method like setEntityOperationPermission():

  public function provideDefaultOgPermissions(PermissionEventInterface $event) {
    $permission = new EntityOperationPermission();
    $permission
      ->setName('edit any article content')
      ->setTitle(t('Article: edit any content'))
      ->setDescription(t('Allows to edit any node of type article'))
      ->setEntityType('node')
      ->setBundle('article')
      ->setOwnership('any')
      ->setOperation('update');
    $event->setEntityOperationPermission($permission);

    // Here is an example of a normal group-level permission being set in the
    // same event subscriber.
    $event->setPermissions([
      'administer group' => [
        'title' => t('Administer group'),
        'description' => t('Manage group members and content in the group.'),
        'default roles' => [OgRoleInterface::ADMINISTRATOR],
        'restrict access' => TRUE,
      ],
    ]);
  }

For basic CRUD operations we can supply all permissions out of the box with generic wording such as update own $bundle_id $entity_type_id. We can then allow modules to alter this if the generic wording is not to their liking.

[pfrenssen]

On second thought, I'm going to use an array structure for the moment to pass the data, to be consistent with the group level permissions. This would make it a lot less work to implement. We can later on still make an object to store the permissions.

If we do it like this we don't even need two separate methods, we can pass everything in the same method, and distinguish between the two if the 'operation' key is present.

So the example implementation above would instead look like this:

  public function provideDefaultOgPermissions(PermissionEventInterface $event) {
    $event->setPermissions([
      // Example entity operation type permission.
      'edit any article content' => [
        'title' => t('Article: edit any content'),
        'description' => t('Allows to edit any node of type article'),
        'entity type' => 'node',
        'bundle' => 'article',
        'ownership' => 'any',
        'operation' => 'update',
      ],

      // Example group-level type permission.
      'administer group' => [
        'title' => t('Administer group'),
        'description' => t('Manage group members and content in the group.'),
        'default roles' => [OgRoleInterface::ADMINISTRATOR],
        'restrict access' => TRUE,
      ],
    ]);
  }

[amitaibu]

This should be split into two methods. One is declaring the permissions that might be used. And the second is the permissions associated with a certain role.

(As you could have a role with no permissions at all)

[amitaibu]

Also my thoughts for providing default roles is that we maybe should pass a partially applied OgRole instead of the array.

And when we create the roles we just need to fill in the group type and bundle/group I'd

[pfrenssen]

I'm ready with this! I have addressed all the remarks (I think :) )

[amitaibu]

As indicated in another comment getRole should still remain (although we can move it to OgRole itself ) as You may want to set the role's permissions before any group as actually created

Also, just as a heads up, the entire OgRole will still undergo changes, once we'll have to introduce the concept of OgRolePerBundle and OgRolePerGroup (as we have in OG7)

[pfrenssen]

OK will restore it! I will make a followup to move it to OgRole, because it will need some discussion about the correct naming of the function. Burying this in here will cause it to be missed by everyone who is involved in OG atm.

[pfrenssen]

Followup: #292

[amitaibu]

Thanks for keeping up the good work!

Made comments about the "dummy non-member memberships" (dnmm in short 😜).
(Also it will need a re-roll)

[pfrenssen]

Addressed the remarks, rerolled, and tests are green :)

[amitaibu]

👍

[amitaibu]

Wonderful, thanks!

[pfrenssen]

Woohoo!! Thanks a lot!!