Permission evaluation is broadly inconsistent
Overview
(This issue/filing is still a WIP.)
The internal APIs for defining permissions use inconsistent notations. This is primarily a DX issue. (Hypothetically, confusion over security behavior can lead to security issues, but this hasn't yet been earnestly argued in this case.)
As an example, consider something like CiviReport, afform
, or dataprocessor
which allows a user to define a new end-point. It is quite likely that you would want to:
- Allow the admin-user to set permissions for the HTTP end-point
- Enforce those permissions when receiving an HTTP request for that end-point
- Allow the admin-user to add the new HTTP end-point to the nav-menu
- Enforce those permissions when generating a nav-menu
- Allow the admin-user to expose the element for access via API
- Enforce those permission when accessing via API
However, if the permission mechanics are inconsistent (i.e. differing for the router and nav-menu), then you cannot cleanly collect+enforce those permissions. You must either:
- Ask the admin-user to enter permissions in three different formats for three different permission-checkers
- Create your own (fourth) format; and map/replace/supplement the standard permission-checks in each layer
- Stick to the lowest-common-denominator of functionality - ie exactly one permission for the element.
Current behavior
- When registering a route (ie
civicrm_menu
; viahc_xmlMenu
orhc_alterMenu
), one declares the permissions needed to access the route. This is 1-2 fields,access_callback
andaccess_arguments
. Theaccess_callback
has a default (CRM_Core_Permission::checkMenu()
), and it expects thataccess_arguments
is a string likeaccess Foo,access Bar
(implied AND) oraccess Foo;access Bar
(implied OR). The ANDs/ORs cannot be mixed. - When registering a navigation item (ie
civicrm_navigation
; via SQL or API or/civicrm/admin/menu
orhc_navigationMenu
), one declares the permissions needed to see the nav-item. This is 2 fields,permission_operator
(OR/AND) andpermission
. Thepermission
looks likeaccess Foo,access Bar
. - When writing runtime logic to check permissions,
CRM_Core_Permission::check()
accepts an array of permissions. The top-level elements are AND'd, and nested elements are OR'd.- Note: The most typical usage of
check()
is to a give a string (check('access Foo')
), which is cast to 1-item array.
- Note: The most typical usage of
- When registering an API, one declares permissions (via
hc_alterAPIPermissions
orCivi\Api4\XXX::permissions()
). This appears to be the same array asPermission::check()
.
Analysis
All of the existing notations are essentially limited boolean expressions.
- For the router and nav-menu, permission expressions strictly use one operator (
A || B || C ...
;A && B && C ...
). This obviously is the more restrictive notation. In both subsystems, having a choice of operator mitigates the limit somewhat. The limit is not so onerous as to produce widespread complaints - so far, the limit seems to be acceptable. - For APIs and
check()
, they're limited to(A||B) && (C||D) && ...
. I'm guessing there's some historical perception that "AND" was more important/needful. (Interestingly, if the implied operators had been flipped --(A&&B) || (C&&D) || ...
-- then you'd have the full expressiveness of a tree of AND/OR operators, albeit a bit verbose.) - As a matter of DX, the use of
,
or;
in a boolean expression is ambiguous. The symbols&
,&&
,|
,||
,and
, andor
would be less ambiguous. OTOH,&
doesn't look nice in XML. - The routing system has a neat idea in using
access_callback
/access_arguments
-- b/c that model allows one to evolve the mechanism incrementally over time. None of the others attempt this. Alas, it's a bit hobbled by an implementation detail: when the parser reads<access_arguments>
from XML, it immediately translates to an array-format that's specific toCRM_Core_Permission::checkMenu()
. This has two effects:- For
hc_xmlMenu
, you can only usecheckMenu
or trivial functions (always true/always false). Anything more interesting/substantive would get mangled. - For
hc_alterMenu
, you may use customaccess_callback
andaccess_arguments
without concern for mangling. But if you do usecheckMenu
, thenaccess_arguments
will appear to be mangled (b/c they're in the undocumented array-notation instead of the documented string-notation).
- For
Some loose approaches:
- Embrace the mess. It's not core's problem - it's downstream's problem!
- Adopt a third-party notation (e.g. Symfony Expression Language) and patch everything (router, nav-menu, api; core, contrib, etc) to use it.
- Create an expanded/superset/backward-compatible notation -- e.g. (1) allow additional, unambiguous operators (
(
,)
,&
,|
,AND
,OR
), (2) make;
an alias forOR
(percheckMenu()
), (3) make,
a dynamic operator which defaults toAND
(percheckMenu()
) but can be changed (percivicrm_navigation.permission_operator
), (4) allow these expressions to be combined in an array-tree like API/check()
. - Take a cue from
access_callback
/permission_operator
... in all subsystems, allow an option to decide how to interpret the permission expression.
I like the last one from a LExIM perspective, but it's clear that the different checkers don't agree on a notation/format for indicating an access_callback
. But... they all agree that they can take a $perm
string... which leads to a more concrete proposal in which the $perm
string helps determine the evaluation.
Proposed behavior
Allow virtualized/delegated permissions. For example, with civicrm/admin/domain
, it's redundant to instruct both the router/xml-menu and the nav-menu to use administer CiviCRM
. Instead, the nav-menu item could declare that it inherits permissions from the route, e.g.
[
'label' => 'Organization Address and Contact Info',
'url' => 'civicrm/admin/domain?action=update&reset=1',
'permission' => '@route:civicrm/admin/domain',
]
Some possible prefixes:
-
@route:ROUTE_PATH
: Delegate to the permissions of the given route -
@api3:ENTITY:ACTION
: Delegate to the permissions of the given API entity/action -
@api4:ENTITY:ACTION
: Delegate to the permissions of the given API entity/action
The virtualized/delegated permissions can be dropped in anywhere that you can use regular permissions.
If you're creating something like afform or dataprocessor, you might register your own prefix:
function hc_config() {
CRM_Core_Permission::registerPrefix('afform', '_my_check_perm');
}
function _my_check_perm(string $prefix, string $perm) { ... }
(TODO: It may be possible to get this result to day using hc_permissionCheck
... need to test...)