SearchKit - Bypass ACLs for entity displays and require super-admin permission#35629
Conversation
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
…ermission Entity displays are tables/views built from a query. They should always return the same results regardless of the logged-in user, therefore those user ACLs should not be part of the query. This switches such displays to always bypass permission checks. And like any other display with `acl_bypass`, they now require the super-admin permission to create or edit such displays.
colemanw
force-pushed
the
entityDisplayPerms
branch
from
6e5fd58 to
0cf2cd9
Compare
| this.displayTypes = Object.fromEntries(CRM.crmSearchAdmin.displayTypes.map(type => [type.id, type])); | ||
|
|
||
| // Only super admins are allowed to create entity displays | ||
| if (!CRM.checkPerm('all CiviCRM permissions and ACLs')) { |
There was a problem hiding this comment.
what if superadmin creates the entity display, then a non-super admin tries to edit the same saved search?
There was a problem hiding this comment.
@ufundo non-super-admins won't be allowed to edit the search or the display.
Can a non-super admin edit the Saved Search, and thereby sneak something else out?
A bit tangential, but do we block using |
|
@ufundo good questions.
No, they can't. See https://github.com/civicrm/civicrm-core/pull/35629/changes#diff-1ee3649dbd3ff87b7a414c8c76dbdee26b62185742b3301587b74cd0d6dd8e72R84
Yes, once #35613 is merged we do. |
|
Thanks @colemanw I've |
ufundo
added
the
merge ready
3 participants
Overview
Prevents a crash when rebuilding a SearchKit entity display from a cron job or when run by a less-permissioned user. Ensures consistent results of entity displays regardless of logged-in user.
Technical Details
Entity displays are tables/views built from a query. They should always return the same results regardless of the logged-in user, therefore those user ACLs should not be part of the query.
This switches such displays to always bypass permission checks. And like any other display with
acl_bypass, they now require the super-admin permission to create or edit such displays. This prevents any unwanted access by less-permissioned users.