SearchKit - Allow super admins to disable Search Display access checks #20607
Conversation
|
(Standard links)
|
…ter CiviCRM data' By default, the permission was previously 'administer CiviCRM'. The new permission is a subset for data administrators.
The new column determines whether permissions will be checked when running a display
colemanw
force-pushed
the
searchDisplayAccessBypass
branch
from
caeeada to
c18ff7d
Compare
… GROUP_CONCAT Ensures that e.g. an array of integer fields will be returned as integers and not an array of strings
colemanw
force-pushed
the
searchDisplayAccessBypass
branch
from
c18ff7d to
3944657
Compare
colemanw
changed the title
This allows users with 'all CiviCRM permissions and ACLs' to configure a search display to bypass permission checks and display all records to the user. Once a display is set to bypass ACLs, it can only be edited by a super-admin, ordinary admin users will not be able to edit the display or the saved search. Such a display will not automatically appear on its own page; it must be embedded in an Afform, and the Afform will act as gatekeeper for users to view the display.
colemanw
force-pushed
the
searchDisplayAccessBypass
branch
from
3944657 to
5623bf2
Compare
| * Upgrade 1005 - add acl_bypass column. | ||
| * @return bool | ||
| */ | ||
| public function upgrade_1005() { |
There was a problem hiding this comment.
So this feels right but just making a point that these Extension upgrades are only run via the Extension Upgrade process not the main upgrade process but it should flag that they need to be done anyway.
There was a problem hiding this comment.
IMO we ought to run extension upgrades as part of the main upgrader.
|
So @colemanw one question on this is that does this still provide any field level permission handling or does that get completely by-passed as well? |
|
@colemanw I tested this on the PR test site and it seems like it works well but want to do more r-run testing and one thing I just considered is that if a search display cannot be used at all if permissions are disabled without embedding on an afform, should the ability to disable permission only show if afform is enabled? |
ACL will be by-passed, BUT the search display will never show fields that are not configured to be part of the search, and it will never allow filters that are not explicitly part of the select clause or a filter field on the afform. So those security checks will always be performed. |
Well, maybe. I didn't want to rule out the possibility of a developer embedding a search display on some other (non-afform) Angular screen tho. |
|
I think my concerns are satisfied here and it worked in my testing merging. |
Overview
This allows users with 'all CiviCRM permissions and ACLs' to configure a search display to bypass permission checks and display all records to the user.
Technical Details
Once a display is set to bypass ACLs, it can only be edited by a super-admin, ordinary admin users will not be able to edit the display or the saved search.
Such a display will not automatically appear on its own page; it must be embedded in an Afform, and the Afform will act as gatekeeper for users to view the display.
Comments
This builds upon the groundwork done in #19797 and #20533