Contrib.social

Typo in openerAccess method causing faulty access results

Open on Drupal.org →
Created on 14 June 2024, 5 months ago

Problem/Motivation

I have a project in which group members have permissions to create media but no permission to create a node (page). They do however have permission to edit a page and thus to add new media on that page. But when those group members tried to edit a page and opened the media library modal to upload a new image and create a new media item the popup had a ajax error after uploading the new image file.

I also noticed a difference between a media field directly added to the content type (like a teaser for example) and a media field added to a paragraph type which is referenced by a field on the content type. The problem does not happen on the teaser field but does happen on the paragraph.

After searching for a few days I discovered that after selecting a file to upload it got an access result for the entity type node:page instead of the entity type media:image. Which was of course denied since the permission to create nodes was turned off. After searching and testing some more I found out this module was using $group_state->getGroupRelationPluginId() in the openerAccess method instead of $group_state->getGroupMediaRelationPluginId() like it does in the addFormAcceess method just above it.

So I created a patch and tested the change which now works as expected.

The only problem is I'm just not sure if I'm causing unwanted side effects by making this change. So a code review is more than welcome.

Steps to reproduce

Create content type with media teaser field and paragraphs with paragraph type that references a media type.
Create a group type with permissions to edit a content type (but no permission to add new nodes).
Add permissions to add/edit media items.
As an admin: create a node and add it to the group.
Log in as a user who's member of that group and try to edit the node. See the difference when adding a new media item when editing the teaser field or when adding a new paragraph.

🐛 Bug report
Status

Active

Version

2.0

Component

Code

Created by

🇧🇪Belgium aimevp Belgium

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @aimevp
  • Comment 5 months ago →
    🇧🇪Belgium aimevp Belgium

    I'm trying to making a merge request but can't figure out why I keep getting access denied in git. So for now here's a patch file...

  • Status changed to Postponed: needs info 4 months ago
  • Comment 4 months ago →
    🇧🇪Belgium msnassar

    Sorry for late answer...

    This is not a typo... It is intended to check the access to create/edit the root host entity (in your case the page content type). I recommend to look into this code snippet. Usually, if you have the access to edit the root host entity, you should be able to add/upload new media to it. Please, let me know your finding...

    I am hiding your patch as it has a security vulnerability...

  • Status changed to Active 4 months ago
  • Comment 4 months ago →
    🇧🇪Belgium aimevp Belgium

    I'm a bit confused what to do now because clearly the code wasn't working as intended in my case. People who had node edit permission (without create permission) within a group were unable to create new media. Has this been tested in that manner? Or can you think of a reason (bad configuration for example) why this isn't working in my project?

  • Comment 4 months ago →
    🇧🇪Belgium msnassar

    I did quick debug.... I confirm the issue... It seems this is due to the fact that we check for entity create access but not update. See here

    I believe, in case the entity is exist, we instead have to check the access to update. You can get the entity and its type from MediaLibraryState e.g. MediaLibraryState::fromRequest($this->requestStack->getCurrentRequest())
    Then to check for the access to update, we use entityAccessfrom Drupal\group\Plugin\Group\RelationHandler.

    As soon as I have the time, I will look deeply into it. However, patch is very welcome :)

  • Status changed to Needs work about 1 month ago
  • Comment about 1 month ago →
    🇺🇸United States SocialNicheGuru
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024