[Regression] Users without permission can edit recurring contributions
Overview
Users without the permission "edit contributions" can edit recurring contributions of any contact (might be mitigated by specific scenarios, see below).
Reproduction steps
- Create a user with a role that has only the following permissions:
- CiviCRM: access CiviCRM backend and API
- CiviContribute (Contributions): access CiviContribute but explicitly not the "CiviContribute (Contributions): Edit Contributions" permission
- Configure an ACL for them to view contacts in a specific group and their contributions. - not sure this is necessary
- With that user logged in, go to any visible contact's Contributions tab and the Recurring Contributions sub tab
Current behaviour
The user is presented links to edit recurring contributions and clicking it also shows the form. Saving the form is also allowed.
Expected behaviour
Editing should not be allowed, i.e. there should be no Edit link at all and opening the forms should result in a 404.
Environment information
- CiviCRM: 5.24.5
- CMS: Drupal 7.30
Comments
This seems to have been introduced as a regression with https://github.com/civicrm/civicrm-core/pull/13237 which originated in #571 (closed).
The compared contact ID is retrieved using $this->getContactID()
which calls CRM_Core_Form::setContactID()
which states the following in its docblock:
/**
* Get contact if for a form object. Prioritise
* - cid in URL if 0 (on behalf on someoneelse)
* (@todo consider setting a variable if onbehalf for clarity of downstream 'if's
* - logged in user id if it matches the one in the cid in the URL
* - contact id validated from a checksum from a checksum
* - cid from the url if the caller has ACL permission to view
* - fallback is logged in user (or ? NULL if no logged in user) (@todo wouldn't 0 be more intuitive?)
*
* @return NULL|int
*/
The relevant part seems to be: cid from the url if the caller has ACL permission to view
, but I couldn't verify yet, since it's a production system this is appearing on.
Background: The scenario is a regional department user (being restricted to viewing contacts of a specific regional group only - which is done using ACLs), who should be allowed to view contributions, but not edit them at all.
Since the aforementioned issue tried to bypass access checks for editing one's own recurring contributions, I'm not sure this is a correct assumption at all. There should be a separate permission for controlling whether editing own recurring contributions is allowed.