I'm using permissions_by_entity to control access to media and files. Some media and their files need to be restricted for a period of time and then eventually released. I was hoping to use the file_access_fix module to move the files from the private filesystem to the public filesystem when I remove the 'Staff Only' term from the media.
However, with the file_access_fix module enabled, it was automatically pushing my restricted media's files to the public filesystem. What happens is that, on a Media insert or update, it checks the permissions of the Media to see if Anonymous has access. If so, it moves the files to the public filesystem. ALL of my media were returning TRUE on this check, causing the files to be moved, even when they had the 'Staff Only' term applied to the media.
After some liberal use of logging statements, I discovered that permissions_by_entity would ALWAYS return true when checking for Anonymous user access during the hook_media_update or hook_media_presave, even if it would normally deny access to the Media and files.
To reproduce:
- Enable permissions_by_entity
- Create an 'Admin Only' term and only permit admin role access in the usual way
- Create a new Media entity with a file in the private filesystem
- View the media as an Anonymous user, it should render as expected.
- Apply the 'Admin Only' term to the media and save
- Attempt to view the Media and/or file as Anonymous user to find that access is denied (as expected).
- Create a local module 'example' with an example_media_update function to log Anon's access to the Media and enable it. E.g. :
use Drupal\user\Entity\User; function example_media_update(Drupal\Core\Entity\EntityInterface $entity) { $hasAccess = ($entity->access('view', User::getAnonymousUser())) ? 'TRUE' : 'FALSE'; \Drupal::logger('example')->debug('Media update access for Annon: '.$hasAccess); } - Change a field on the Media (e.g. the alt text for an image or the media label) and save it.
- Check the logs, you will find an entry stating "Media update access for Annon: TRUE" even though the Anonymous user still can't access the media.
This also happens when using other hooks such as hook_entity_view. I would expect that the access check when provided a specific user account would be consistent, but it appears that if the Admin user check's the Anonymous User's access, it always returns true.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | issue_3143967.patch | 568 bytes | seth.e.shaw |
Issue fork permissions_by_term-3143967
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
ytsurkDoes the patch here #3122864: Recursive referenced entity may overrule actual entities permissions help?
Comment #3
seth.e.shaw commented@ytsurk, unfortunately no. I applied the referenced patch and cleared my cache but the issue is still persisting.
Comment #4
seth.e.shaw commentedI found the offending line. In isAccessAllowedByDatabase we load the current user if the UID is GREATER THAN 0. This means if we ever what to explicitly check for Anonymous (UID 0) it will simply load the current user.
Simply changing the line to
if (is_numeric($uid) && $uid >= 0) {resolves the issue. See the attached patch.For those who know the module's internals better than I, is there a specific reason why we are loading the current user when passed UID 0 that this patch breaks?
Comment #5
seth.e.shaw commentedComment #6
smustgrave commentedLooks good to me.
Comment #9
marcoliverNeeded to do a small update to a unit test as well. Will merge in a second and tag a release today or tomorrow.
Comment #11
marcoliverComment #12
marcoliverComment #13
marcoliver