Cache is handled by the Permission by Entity's AccessResultCache, which only was made for nodes. When using entities these tags mix-up, as they only respect the $entity->id() and $account->id().
Use cache tags which do respect the entity type id and do not pollute the database with node entries.
PbE has a whole different access check system (not using cache at all), which also should be ported to PbT.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | permissions_by_term-tag_and_invalidate_cached_access_results-3238759.patch | 7.75 KB | fskreuz |
| #7 | interdiff.txt | 1.95 KB | fskreuz |
| #4 | 3238759-PbE_respect_entity_type_id-4.patch | 6.21 KB | ytsurk |
Comments
Comment #2
ytsurkHere's a first workaround, a replicated AccessResultCache service in PbE.
Comment #3
ambient.impactThis is a thing that caught me by surprise when we enabled Permissions by Entity on Omnipedia, as it started giving users access to unpublished nodes with those terms, even though they didn't have permission to view unpublished nodes. With only Permissions by Term enabled, they would only be granted access when the nodes were published. As you can imagine, this isn't the kind of surprise you'd want in something users pay for and thus real money is involved.
Comment #4
ytsurkHere a next patch, leaving nodes alone.
Comment #5
fskreuz commentedPatch works for me (PbT 3.1.17, D 9.2.8). Works on Media and direct access to the media's files.
Comment #6
fskreuz commentedUpdated patch to include hooks to invalidate tags when an entity is modified.
Comment #7
fskreuz commentedAdded additional check to only invalidate on content entities.
Comment #8
fskreuz commentedComment #9
jepster_Hi,
sorry, I cannot reproduce this. I have created a media document, attached a private file to it and the access was always restricted. First request after cache clear and the next request. I have tested it on an environment with Drupal, Permissions by Term and Permissions by Entity installed only. Maybe your issue is somehow tied to any other dependency. It can help to install an new Drupal installation with a limited amount of modules for isolating the issue.
Comment #10
ytsurkThere are clear steps on how to reproduce this?
Also, by only looking at the code, the problem is visible. Please read the issue description again.
PbE really should be reworked/alinged with PbTs access check logic. Keep in mind, that PbE does have to deal with different entity types, and not "just" nodes.
According to your comment
Comment #11
ytsurkThis is a serious security threat!
Comment #12
fskreuz commented@Peter I have steps to reproduce over at https://www.drupal.org/project/permissions_by_term/issues/3256980 (which I closed when I discovered this issue is similar and already had a working patch).
@ytsurk Indeed it is a security issue, would fall under the information disclosure category.
Comment #13
ytsurkComment #14
ytsurkPutting this on the map again.
Peter, you need a node and media entity with the same id. Please follow my instructions in the issue description!
Comment #15
jaymcgraw commentedFrom the experience of someone testing this module out on a new dev site starting with no content, I was about ready to give up on this module because it didn't work as expected. Then I dove in to the code, discovered this issue and realized it wasn't working because I was testing with node id 1 and paragraph id 1 and media id 1. I agree it's a security issue, and it's also a poor experience to someone new to the module. I'm glad to see there's an issue opened with a patch for this.
Comment #16
smustgrave commentedThis patch worked for me.
I'm 100% all for getting permissions_by_entity working better as users want to use this with media but appears to be lacking.
Comment #17
julianvppw commentedWith permissions_by_entity being a sub-module of permissions_by_term, I wasn't sure where to apply this patch:
permissions_by_term-tag_and_invalidate_cached_access_results-3238759.patchFor every location I tried (web/modules and web/modules/contrib/permissions_by_term/modules) files were not being updated or created.
So I applied the patch manually. But still, Media Entities are being cached in the
/jsonapi/media/private_documentendpoint.This means when I clear cache, first request works but it becomes a snapshot for subsequent requests.
Comment #18
ambient.impact@julianvppw Patches are relative to the project, not a sub-module. That means it should be applied to
web/modules/contrib/permissions_by_termComment #19
jepster_Thanks for the patch. Released via version 3.1.18.