Incorrect Access Check on Media Library

In our case I used hook_entity_create_access, with custom logic. if a user can edit group content, he/she can also add media item.

I'm experiencing this as well. It seems that the permissions to create media on a node creation page looks for both rights to create that media type and rights to create that type of node. If the user doesn't have access to create the content type outside of the group, the media field fails when creating a new piece of media (Selecting existing works fine). It seems we need a patch in groups that overwrites that access check to see if the user has access to create the CT in the group.
@LOBsTerr, do you have sample code for how you got around the issue?

I came across the same issue today. The following should work:

/**
 * Implements hook_field_widget_WIDGET_TYPE_form_alter().
 *
 * Add the group ID and plugin ID to the MediaLibraryState when the media
 * library is being used on the create group content form so it can be used to
 * determine permissions in gnode_node_create_access().
 *
 * @see gnode_node_create_access()
 * @see \Drupal\media_library\Plugin\Field\FieldWidget\MediaLibraryWidget::formElement()
 */
function gnode_field_widget_media_library_widget_form_alter(&$element, FormStateInterface $form_state, $context) {
  /** @var \Drupal\Core\Routing\RouteMatchInterface $route_match */
  $route_match = \Drupal::routeMatch();

  if ($route_match->getRouteName() === 'entity.group_content.create_form') {
    /** @var \Drupal\media_library\MediaLibraryState $state */
    $state = $element['media_library_open_button']['#media_library_state'];
    $openerParameters = $state->getOpenerParameters();
    $openerParameters['group_id'] = $route_match->getParameters()->get('group')->id();
    $openerParameters['plugin_id'] = $route_match->getParameters()->get('plugin_id');
    $new_state = MediaLibraryState::create($state->getOpenerId(), $state->getAllowedTypeIds(), $state->getSelectedTypeId(), $state->getAvailableSlots(), $openerParameters);
    $element['media_library_open_button']['#media_library_state'] = $new_state;
  }
}

/**
 * Implements hook_entity_create_access().
 *
 * Use the group ID and plugin ID to determine if the account has permission to
 * create content in the group.
 *
 * @see Drupal\media_library\MediaLibraryFieldWidgetOpener()
 */
function gnode_node_create_access(AccountInterface $account, array $context, $entity_bundle) {
  $route_name = \Drupal::routeMatch()->getRouteName();

  if ($route_name === 'media_library.ui') {
    /** @var \Drupal\media_library\MediaLibraryState $state */
    $state = MediaLibraryState::fromRequest(\Drupal::request());
    $openerParameters = $state->getOpenerParameters();

    if (isset($openerParameters['group_id']) && isset($openerParameters['plugin_id'])) {
      $group = Group::load($openerParameters['group_id']);
      $plugin_id = $openerParameters['plugin_id'];

      if ($group->hasPermission("create $plugin_id content", $account)) {
        return AccessResult::allowed();
      }
    }
  }

  // No opinion.
  return AccessResult::neutral();
}

I'll test it and write a patch within the next day or so.

Getting an error when creating a node with this patch applied. Looks like the element should be open_button rather than media_library_open_button.

The website encountered an unexpected error. Please try again later.
Error: Call to a member function getOpenerParameters() on null in gnode_field_widget_media_library_widget_form_alter() (line 424 of modules/contrib/group/modules/gnode/gnode.module).

gnode_field_widget_media_library_widget_form_alter(Array, Object, Array) (Line: 539)
Drupal\Core\Extension\ModuleHandler->alter('field_widget_form', Array, Object, Array) (Line: 355)
Drupal\Core\Field\WidgetBase->formSingleElement(Object, 0, Array, Array, Object) (Line: 94)
Drupal\Core\Field\WidgetBase->form(Object, Array, Object, NULL) (Line: 294)
Drupal\media_library\Plugin\Field\FieldWidget\MediaLibraryWidget->form(Object, Array, Object) (Line: 178)
Drupal\Core\Entity\Entity\EntityFormDisplay->buildForm(Object, Array, Object) (Line: 125)
Drupal\Core\Entity\ContentEntityForm->form(Array, Object) (Line: 127)
Drupal\node\NodeForm->form(Array, Object) (Line: 149)
Drupal\Core\Entity\EntityForm->buildForm(Array, Object)
call_user_func_array(Array, Array) (Line: 520)
Drupal\Core\Form\FormBuilder->retrieveForm('node_generic_content_form', Object) (Line: 277)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 61)
Drupal\Core\Entity\EntityFormBuilder->getForm(Object, 'default', Array) (Line: 361)
Drupal\group\Entity\Controller\GroupContentController->createForm(Object, 'group_node:generic_content')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 694)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

The patch in #8 is almost correct but needs a minor change. I think the if condition that tests for the permissions should test for entity not content.

That is replace

if ($group->hasPermission("create $plugin_id content", $account)) {

with

if ($group->hasPermission("create $plugin_id entity", $account)) {

Tested #9 and that solved my issue of adding media to a node entity.

Despite patch #9 allows selecting media. Switching the widget to table / grid is giving me the same Forbidden ajax error

Node grants are now removed from Group, so does this patch still make sense? Maybe it needs to be reworked a bit? A test would be great so we can tell that the patch (still) works or needs to be refactored.

I know you're looking for actual automated tests but in the pickle we had with our client today I tested updating to 1.2 first and that did not resolve the situation with our configuration.

Instead applying this patch (#9) onto 1.0-rc5 did resolve it. So in that sense continued work on this issue is justified.

I'd be happy to help but I would need some guidance as to how I should approach creating the test. I feel like this should be a functional test and I have no prior experience in writing those.

Re #13: You should definitely try updating to 1.3, as you can now switch back to the old group-vs-node_grants interactions by swapping out the GroupNode plugin's access handler (if that's what's holding you back).

As to the use case at hand, I figured out what's wrong:

1. Group does not implement hook_entity_create_access() because that would grant access to creating entities in the global scope, even though you might only be able to add these entities within a group. For that reason, we have special access checks on particular routes instead.
2. Now, Media checks for create access and finds that you have none (because Group is careful re handing out said access) and therefore fails to load the widget properly.

So in order to fix this, we need to overhaul Group so that it does hand out create access in hook_entity_create_access() rather than on the specific routes only, but would check for the current route in the access callback. This is rather nasty, as we'd need to make sure the access result is cached per route, but also because this might lead to unintended behavior in decoupled requests.

Ample test coverage would be required and I'd need to do some investigating into whether other modules already did something similar and got away with it.

The fix proposed in #9 is too specific to this use case only, whereas it turns out there is a bigger issue at play here.

I'm curious to know if this is a new bug or whether it is only presenting itself now because of a core media update?

I manage a number of sites running the same codebase and only recently has it become problematic for group members (without wider node creation permissions) to have this problem with the media library.

Does anyone know?

This has also only recently become an issue for me. I am using group 8.x-1.3 and on drupal 8.9.7, and users with group permission to create a node and group permission to create media, can no longer add a new file via media library. This patch/#9 fixed the issue for me.

Group does not implement hook_entity_create_access() because that would grant access to creating entities in the global scope, even though you might only be able to add these entities within a group. For that reason, we have special access checks on particular routes instead.

I think this explains why I've been banging my head against a wall all week, but it does not bode well for me.

My groups are "content teams." My whole intention of using Groups was to give users additional access to select content (and content types) based on their roles within groups (editor, reviewer, approver, etc.). But this assumes that group permissions are always additive, so the existence of groups would never take away permissions granted by standard Drupal roles, whether or not a user is in a group. It also assumes these added permissions are global when it comes to content creation, so if a group gives a user permission to create basic pages then they should be able to go to /node/add/page and create a page. The group pages are useless to me and will just confuse my users. I certainly don't want them having to navigate to some obscure group node listing page in order to create some content.

Is it possible to make Group behave this way, or am I just screwed?

@jstoller not sure if this will solve all your issues, but this module, formerly a patch on group, will allow your users to add content as they currently do, from /node/add/page, by adding the group as a field on the node.

https://www.drupal.org/project/entitygroupfield

Thanks @willabby. We've already been using Entity Group Field and it certainly helps, but it doesn't give access to create nodes unless you have that permission globally outside of Group. We ended up implementing hook_entity_create_access() to let Drupal know if a user should have the right to create a particular content type due to their group membership. Here's the code, incase anyone else is in this predicament.

use Drupal\Core\Access\AccessResult;

/**
 *  Implements hook_entity_create_access();
 *
 *  @dev: Note the user's drupal role must have access to the
 *  "create new draft" workflow transition.
 */
function MY_MODULE_entity_create_access(\Drupal\Core\Session\AccountInterface $account, array $context, $entity_bundle) {
  $groups = [];
  $grp_membership_service = \Drupal::service('group.membership_loader');
  $grps = $grp_membership_service->loadByUser($account);

  foreach ($grps as $grp) {
    $loaded_group = $grp->getGroup();
    $groups[$loaded_group->id()] = $loaded_group;

    // Check group permission to create this entity and, if so, allow.
    if ($loaded_group->hasPermission('create group_node:' . $entity_bundle . ' entity', $account)) {
      return AccessResult::allowed();
    }
  }

  // By default return neutral.
  return AccessResult::neutral();
}

Should I create a new issue for the more general use case of Group informing Drupal what a user should be allowed to create, or will that be dealt with as part of the media issue here?

thanks jstoller, you save my time !
Patch works fine on drupal 9, I was not able to add media on a content type group without the administrator role.

The problem of adding media continues in 1.3 unless a global permission to add nodes is also granted.

The patch at #9 shows an error on the create/group_node page.

Notice: Undefined index: preview in Drupal\responsive_preview\ResponsivePreviewHandler::handleFormAlter() (line 64 of modules/contrib/responsive_preview/src/ResponsivePreviewHandler.php).
Drupal\responsive_preview\ResponsivePreviewHandler::handleFormAlter(Array, Object, 'node_update_form') (Line: 118)
responsive_preview_form_alter(Array, Object, 'node_update_form') (Line: 539)
Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'node_update_form') (Line: 836)
Drupal\Core\Form\FormBuilder->prepareForm('node_update_form', Array, Object) (Line: 279)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 61)
Drupal\Core\Entity\EntityFormBuilder->getForm(Object, 'default', Array) (Line: 361)
Drupal\group\Entity\Controller\GroupContentController->createForm(Object, 'group_node:update')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I seem to recall we ran into this ourselves, and it related to this for us: https://www.drupal.org/project/drupal/issues/3197416

At least that's where I was able to trace the error. Media would fail on node and it all kind of backed up to this. You could work around it as you've said, but the meat of the issue for us was.

Subscribing. I experience the exact same behavior with `8.x-1.4`

I created a patch as an improvement of #9 so that it checks the access to create group content entities using the access check service instead of permissions. This way, if we have logic in a hook_access or if we decorate the access check service, that logic will be taken into account.

Update to patch #24. Instead of calling the access check service directly, I am using the access manager to check access to the create form route.

Hello,

I tried the patch #25 and it fixed my problem.

I had a group type with nodes. The member could create the node in the group but couldn't upload media in the node.

Thank you !

For some reason when using media library with a normal user (besides drupal admin) and switching between "Grid" and "Table" widgets, the triggered routes are view.media_library.widget and view.media_library.widget_table. I am now including those routes as well in gnode_node_create_access().

Patch in the works that fixes this for us. Needs to be generalized to lookup field config for each group content type/bundle to see if it uses media library widgets, so for now it's hardcoded which is obviously not good. Just wanted to get this up here as another idea.

I uploaded the wrong code/patch! New comment incoming with corrected patch idea.

Better/correct patch. Basically, if the request path we're checking on is the media library UI, allow it.

https://www.drupal.org/files/issues/2022-06-10/media-library-access-fix-...

A thousand apologies. Had an errant closing bracket. Corrected.

I've opened #3327106: MediaLibraryFieldWidgetOpener is too opinionated to discuss the root cause.

I modified @bernardopaulino patch for the group 3 version

Thanks Sergey Gabrielyan, I'm using your patch #33 and it fixed the issue here using Group 3.x

Can we get a new release of Group that includes the patch?

Re-rolling the patch for 1.5.

I have created a module "Group Media Library" that solves some issues when using media library with groups.

Please try this patch for incorrect access.

I Took patch #36 and replaced the old (deprecated) hook gnode_field_widget_media_library_widget_form_alter with gnode_field_widget_single_element_media_library_widget_form_alter and also now using this route , entity.group_content.create_form instead of entity.group_relationship.create_form.

So, MR117 it's a reroll of #36

Hi @rigoucr .. I tried this out, but the route used in the conditional wasn't being called.

Have the same issue on Drupal 10.1.6 and Group 3.2.1
Patches are not helping.
Had the #33 patch in Drupal 9.4.x which was warking. After upgrading Drupal to 10 issue returns.

The patch #42 did the job. Thank you @lukus!

@le72, if the patch worked for you, why did you decide to delete it from the issue?

Not sure how it was happened :-( Can I delete others' files?

So, can't restore. The file from #42 works for me.

Patch #42 worked on 1.6 but didn't apply cleanly because the use statements are different. Here is a version of the patch that works with the 1.6 version of the module.

Comment #41 with patch MR117 is for the 1.x branch of groups and was what I needed. The patch from #42 seems to work on the 3.x branch, which I'll test soon on a different site.

Disregard my last comment and ignore the patch I uploaded (group-media-library-access-for-1.6.patch). It was a broken mix of the two. I'm a bit rusty on Drupal patching and not sure why my initial tests looked like it worked.

FYI for anyone reaching this issue after Upgrading to 10.2:

The patch from #42 works great on Group 3.x and fixes the access in the media library Modal when creating a new Group-Node.

I would mark it as RTBC for 3.x, but the Issue seems to be for multiple Group-Versions (with 1.x tagged) and I have not tested the 1.x patch.
But since @jilashomb has confirmed, that 1.x seems to be working with the patch from #41, I will mark it as RTBC anyway.

If this gives trouble, we could split the issue up for the different Group-Versions.

I can confirm that the patches work, but it's quite difficult to figure out when and which patch to use. Also, the patch #42 contains unnecessary changes.

So I rerolled patches for each available version: 1.x, 2.x, 3.x

The main difference between patches is the entity type name:
- 1.x and 2.x - group_content
- 3.x - group_relationship

I came across the same issue when doing the same for commerce_products with an integration with Group Commerce.
The access check is reverted to the default when trying to upload or select a Media entity. instead of checking the group permissions.

The fix (for us) was to change gnode_node_create_access() to gnode_entity_create_access(), because the code of a possible gnode_commerce_product_create_access() would basically be the same as gnode_node_create_access() in the patches above.

Beware: THE PATCHES ARE THE SAME AS THE ONES OF #52, with the difference they aren't exclusive to node entities.

Anyone have success getting this to work on the 4.x version? The 3.x versions don't appear to work.

#3439568: Ability to use gnode access checks conflicts

@socialnicheguru functionality conflicts or code conflicts inside the *group.module*?
If it's just a code conflicts, then no extra moves are needed here since there are no conflicts with the dev branches

I also tried to use the patch from #53 with version 3.3.4 - which does not work. Any update?

The problem may well lie with media_library expecting the create {bundle} block content permission. This is described in #3327106: MediaLibraryFieldWidgetOpener is too opinionated: MediaLibraryFieldWidgetOpener is too opinionated.

This shows up elsewhere, notably in layout builder (#3106315: Block content permissions required to select or upload new Media with media library when using Layout Builder). That is a very similar scenario: you are allowed to create non-reusable, inline blocks as part of a layout override, without having full on create {bundle} block content permission.

It seems strange that the result of EntityAccessControlHandlerInterface::createAccess() would vary per route. Presumably it also needs the route cache context too.

In D10, the hook hook_field_widget_WIDGET_TYPE_form_alter does not exist anymore and is replaced by hook_field_widget_single_element_WIDGET_TYPE_form_alter, so the hook gnode_field_widget_media_library_widget_form_alter must be gnode_field_widget_single_element_media_library_widget_form_alter

Change record: https://www.drupal.org/node/3180429

I cannot create an interdiff, but I based my patch on #53-1.x.

I tried to use the #53 3.x patch with 3.3.5, but it's not working (no change to permission blockers).