isAccessAllowed() method always returns allowed due to recursive entity checks (permissions_by_entity)

Created on 9 May 2025, 3 months ago

Problem/Motivation

The Permissions by Entity module's isAccessAllowed() method can incorrectly return TRUE during recursive entity checks. This happens when referenced entities contain taxonomy term reference fields that are not restricted by the configured target_bundles in the Permissions by Term module. As a result, access may be unintentionally granted.

Steps to reproduce

* Add a entity_reference field to a media item that references one of the target_bundles configured.
* Add another entity_reference field that references a vocabulary not configured as a permissions_by_term target_bundle.
* add an access control term to a media item, and view the media item as a user who is not supposed to have access to that term.

Proposed resolution

* During recursive access checks, validate that entity_reference fields being evaluated match the configured target_bundles from permissions_by_term.settings.
* Skip the parent field on taxonomy term entities during recursive checks to prevent unnecessary evaluation and potential circular references.

🐛 Bug report

Status

Active

Version

3.1

Component

Code

Created by

🇦🇺Australia carlopogus

Live updates comments and jobs are added and updated live.

Merge Requests

!59isAccessAllowed() method always returns allowed due to recursive entity checks (permissions_by_entity)
Open
🇦🇺Australia carlopogus
updated 3 months ago

Comments & Activities

Issue created by @carlopogus
Merge request !59issues #3523498 Improve access control logic for entity reference fields in isAccessAllowed() method → (Open) created by carlopogus
Comment 3 months ago →
🇦🇺Australia carlopogus
Pipeline finished with Failed
3 months ago
Total: 205s
#492819
Comment 3 months ago →
🇦🇺Australia carlopogus
Im attaching another patch to support view handler.
Pipeline finished with Failed
3 months ago
Total: 197s
#492868
Comment 3 months ago →
🇩🇪Germany Peter Majmesku 🇩🇪Düsseldorf
Hi @carlopogus,

thanks for working on this. I've added some comments to your merge request.

Please check the testing pipelines. They are currently failing. This code modifies the deep logics of the module. For this we need tests. Please add a test which reproduces the issue and proves the fix.

I'll happily review this again.

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024