From 317103abfb7f5ed64c27aa2350cd115027638359 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 12 Jul 2021 19:45:35 -0400 Subject: [PATCH] Distinguish custom field VIEW from EDIT permissions Before: CustomGroup ACLs made no distinction between VIEW and EDIT After: User with VIEW can see a custom group but not edit it, both in the UI and the API. --- CRM/Activity/Form/Activity.php | 2 +- CRM/Case/Form/CaseView.php | 7 +- CRM/Contact/BAO/Contact.php | 4 +- CRM/Contact/BAO/Relationship.php | 2 +- CRM/Contact/Form/CustomData.php | 2 +- .../Form/Search/Custom/ActivitySearch.php | 24 +- .../Form/Search/Custom/MultipleValues.php | 9 +- CRM/Contact/Page/AJAX.php | 5 + CRM/Contact/Page/Inline/CustomData.php | 15 +- CRM/Contact/Page/View/CustomData.php | 21 +- CRM/Contact/Page/View/Relationship.php | 3 +- CRM/Contact/Page/View/Summary.php | 15 +- CRM/Contribute/Form/ContributionView.php | 3 +- CRM/Contribute/Page/ContributionRecur.php | 3 +- CRM/Core/BAO/Address.php | 3 +- CRM/Core/BAO/CustomField.php | 31 ++- CRM/Core/BAO/CustomGroup.php | 39 ++- CRM/Core/BAO/CustomValue.php | 7 +- CRM/Core/BAO/UFGroup.php | 16 +- CRM/Core/Permission.php | 18 +- CRM/Custom/Form/CustomData.php | 2 +- CRM/Custom/Page/AJAX.php | 10 +- CRM/Dedupe/Merger.php | 6 +- CRM/Event/Form/ParticipantView.php | 13 +- CRM/Event/Page/EventInfo.php | 3 +- CRM/Grant/Form/GrantView.php | 3 +- CRM/Member/Form/MembershipView.php | 3 +- CRM/Pledge/Form/PledgeView.php | 3 +- .../Page/MultipleRecordFieldsListing.php | 14 +- Civi/Api4/CustomValue.php | 12 +- .../Generic/Traits/CustomValueActionTrait.php | 19 ++ Civi/Api4/Generic/Traits/DAOActionTrait.php | 6 +- Civi/Api4/Service/Schema/Joiner.php | 8 + api/v3/CustomValue.php | 3 +- api/v3/utils.php | 6 +- templates/CRM/ACL/Form/ACL.tpl | 1 - .../Contact/Page/View/CustomDataFieldView.tpl | 6 +- templates/CRM/Custom/Page/CustomDataView.tpl | 4 +- .../Page/MultipleRecordFieldsListing.tpl | 2 +- tests/phpunit/api/v3/ACLPermissionTest.php | 4 +- .../api/v4/Action/CustomGroupACLTest.php | 249 ++++++++++++++++++ 41 files changed, 496 insertions(+), 110 deletions(-) create mode 100644 tests/phpunit/api/v4/Action/CustomGroupACLTest.php diff --git a/CRM/Activity/Form/Activity.php b/CRM/Activity/Form/Activity.php index 482937f3b1..bd7e7b10ee 100644 --- a/CRM/Activity/Form/Activity.php +++ b/CRM/Activity/Form/Activity.php @@ -360,7 +360,7 @@ class CRM_Activity_Form_Activity extends CRM_Contact_Form_Task { if ($this->_action & CRM_Core_Action::VIEW) { // Get the tree of custom fields. $this->_groupTree = CRM_Core_BAO_CustomGroup::getTree('Activity', NULL, - $this->_activityId, 0, $this->_activityTypeId + $this->_activityId, 0, $this->_activityTypeId, NULL, TRUE, NULL, FALSE, CRM_Core_Permission::VIEW ); } diff --git a/CRM/Case/Form/CaseView.php b/CRM/Case/Form/CaseView.php index 45a659fcd0..5a8106a56e 100644 --- a/CRM/Case/Form/CaseView.php +++ b/CRM/Case/Form/CaseView.php @@ -149,7 +149,12 @@ class CRM_Case_Form_CaseView extends CRM_Core_Form { NULL, $this->_caseID, NULL, - $entitySubType + $entitySubType, + NULL, + TRUE, + NULL, + FALSE, + CRM_Core_Permission::VIEW ); CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $this->_caseID); } diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index c76c288836..7e2ff0b74e 100644 --- a/CRM/Contact/BAO/Contact.php +++ b/CRM/Contact/BAO/Contact.php @@ -381,7 +381,7 @@ class CRM_Contact_BAO_Contact extends CRM_Contact_DAO_Contact { if (!empty($params['custom']) && is_array($params['custom']) ) { - CRM_Core_BAO_CustomValueTable::store($params['custom'], 'civicrm_contact', $contact->id); + CRM_Core_BAO_CustomValueTable::store($params['custom'], 'civicrm_contact', $contact->id, $isEdit ? 'edit' : 'create'); } $transaction->commit(); @@ -1614,7 +1614,7 @@ WHERE civicrm_contact.id = " . CRM_Utils_Type::escape($id, 'Integer'); else { foreach (CRM_Contact_BAO_ContactType::basicTypes() as $type) { $fields = array_merge($fields, - CRM_Core_BAO_CustomField::getFieldsForImport($type, FALSE, FALSE, $search, $checkPermissions, $withMultiRecord) + CRM_Core_BAO_CustomField::getFieldsForImport($type, FALSE, FALSE, $search, $checkPermissions ? CRM_Core_Permission::VIEW : FALSE, $withMultiRecord) ); } } diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php index 76e61e8f65..cacd788ac9 100644 --- a/CRM/Contact/BAO/Relationship.php +++ b/CRM/Contact/BAO/Relationship.php @@ -1011,7 +1011,7 @@ WHERE relationship_type_id = " . CRM_Utils_Type::escape($type, 'Integer'); $existingValues = CRM_Core_BAO_CustomValueTable::getEntityValues($relationshipId, 'Relationship'); // Create a similar array for the new relationship. $newValues = []; - if (array_key_exists('custom', $params)) { + if (isset($params['custom']) && is_array($params['custom'])) { // $params['custom'] seems to be an array. Each value is again an array. // This array contains one value (key -1), and this value seems to be // an array with the information about the custom value. diff --git a/CRM/Contact/Form/CustomData.php b/CRM/Contact/Form/CustomData.php index 00b771fb7e..05e3bd1ff1 100644 --- a/CRM/Contact/Form/CustomData.php +++ b/CRM/Contact/Form/CustomData.php @@ -224,7 +224,7 @@ class CRM_Contact_Form_CustomData extends CRM_Core_Form { TRUE, NULL, FALSE, - TRUE, + CRM_Core_Permission::EDIT, $this->_copyValueId ); $valueIdDefaults = []; diff --git a/CRM/Contact/Form/Search/Custom/ActivitySearch.php b/CRM/Contact/Form/Search/Custom/ActivitySearch.php index cd1f08c685..3cc5e6d4e7 100644 --- a/CRM/Contact/Form/Search/Custom/ActivitySearch.php +++ b/CRM/Contact/Form/Search/Custom/ActivitySearch.php @@ -49,7 +49,17 @@ class CRM_Contact_Form_Search_Custom_ActivitySearch extends CRM_Contact_Form_Sea ]; //Add custom fields to columns array for inclusion in export - $groupTree = CRM_Core_BAO_CustomGroup::getTree('Activity'); + $groupTree = CRM_Core_BAO_CustomGroup::getTree('Activity', + [], + NULL, + NULL, + [], + NULL, + TRUE, + NULL, + FALSE, + CRM_Core_Permission::VIEW + ); //use simplified formatted groupTree $groupTree = CRM_Core_BAO_CustomGroup::formatGroupTree($groupTree); @@ -180,7 +190,17 @@ class CRM_Contact_Form_Search_Custom_ActivitySearch extends CRM_Contact_Form_Sea } // add custom group fields to SELECT and FROM clause - $groupTree = CRM_Core_BAO_CustomGroup::getTree('Activity'); + $groupTree = CRM_Core_BAO_CustomGroup::getTree('Activity', + [], + NULL, + NULL, + [], + NULL, + TRUE, + NULL, + FALSE, + CRM_Core_Permission::VIEW + ); foreach ($groupTree as $key) { if (!empty($key['extends']) && $key['extends'] === 'Activity') { diff --git a/CRM/Contact/Form/Search/Custom/MultipleValues.php b/CRM/Contact/Form/Search/Custom/MultipleValues.php index bd000d85e7..fe5d5374ce 100644 --- a/CRM/Contact/Form/Search/Custom/MultipleValues.php +++ b/CRM/Contact/Form/Search/Custom/MultipleValues.php @@ -31,7 +31,14 @@ class CRM_Contact_Form_Search_Custom_MultipleValues extends CRM_Contact_Form_Sea public function __construct(&$formValues) { parent::__construct($formValues); - $this->_groupTree = CRM_Core_BAO_CustomGroup::getTree("'Contact', 'Individual', 'Organization', 'Household'", NULL, NULL, -1); + $this->_groupTree = CRM_Core_BAO_CustomGroup::getTree("'Contact', 'Individual', 'Organization', 'Household'", NULL, NULL, -1, + [], + NULL, + TRUE, + NULL, + FALSE, + CRM_Core_Permission::VIEW + ); $this->_group = $this->_formValues['group'] ?? NULL; diff --git a/CRM/Contact/Page/AJAX.php b/CRM/Contact/Page/AJAX.php index a542ef0239..d2de1806a4 100644 --- a/CRM/Contact/Page/AJAX.php +++ b/CRM/Contact/Page/AJAX.php @@ -293,6 +293,11 @@ class CRM_Contact_Page_AJAX { $customValueID = CRM_Utils_Type::escape($_REQUEST['valueID'], 'Positive'); $customGroupID = CRM_Utils_Type::escape($_REQUEST['groupID'], 'Positive'); $contactId = CRM_Utils_Request::retrieve('contactId', 'Positive'); + if (!CRM_Core_BAO_CustomGroup::checkGroupAccess($customGroupID, CRM_Core_Permission::EDIT) || + !CRM_Contact_BAO_Contact_Permission::allow($contactId, CRM_Core_Permission::EDIT) + ) { + CRM_Utils_System::permissionDenied(); + } CRM_Core_BAO_CustomValue::deleteCustomValue($customValueID, $customGroupID); if ($contactId) { echo CRM_Contact_BAO_Contact::getCountComponent('custom_' . $customGroupID, $contactId); diff --git a/CRM/Contact/Page/Inline/CustomData.php b/CRM/Contact/Page/Inline/CustomData.php index 8dd50fd786..fc00e5d072 100644 --- a/CRM/Contact/Page/Inline/CustomData.php +++ b/CRM/Contact/Page/Inline/CustomData.php @@ -35,10 +35,19 @@ class CRM_Contact_Page_Inline_CustomData extends CRM_Core_Page { //custom groups Inline $entityType = CRM_Contact_BAO_Contact::getContactType($contactId); $entitySubType = CRM_Contact_BAO_Contact::getContactSubType($contactId); - $groupTree = CRM_Core_BAO_CustomGroup::getTree($entityType, NULL, $contactId, - $cgId, $entitySubType + // Custom group with VIEW permission + $visibleGroups = CRM_Core_BAO_CustomGroup::getTree($entityType, + NULL, + $contactId, + NULL, + $entitySubType, + NULL, + TRUE, + NULL, + FALSE, + CRM_Core_Permission::VIEW ); - $details = CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $contactId); + $details = CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $visibleGroups, FALSE, NULL, NULL, NULL, $contactId, CRM_Core_Permission::EDIT); //get the fields of single custom group record if ($customRecId == 1) { $fields = reset($details[$cgId]); diff --git a/CRM/Contact/Page/View/CustomData.php b/CRM/Contact/Page/View/CustomData.php index 8e78b9064d..3000627b0b 100644 --- a/CRM/Contact/Page/View/CustomData.php +++ b/CRM/Contact/Page/View/CustomData.php @@ -83,18 +83,9 @@ class CRM_Contact_Page_View_CustomData extends CRM_Core_Page { $session = CRM_Core_Session::singleton(); $session->pushUserContext(CRM_Utils_System::url($doneURL, 'action=browse&selectedChild=custom_' . $this->_groupId), FALSE); - // Get permission detail - view or edit. - // use a contact id specific function which gives us much better granularity - // CRM-12646 - $editCustomData = CRM_Contact_BAO_Contact_Permission::allow($this->_contactId, CRM_Core_Permission::EDIT); - $this->assign('editCustomData', $editCustomData); - - // Allow to edit own custom data CRM-5518. - $editOwnCustomData = FALSE; - if ($session->get('userID') == $this->_contactId) { - $editOwnCustomData = TRUE; - } - $this->assign('editOwnCustomData', $editOwnCustomData); + // Check permission to edit this contact + $editPermission = CRM_Contact_BAO_Contact_Permission::allow($this->_contactId, CRM_Core_Permission::EDIT); + $this->assign('editPermission', $editPermission); if ($this->_action == CRM_Core_Action::BROWSE) { @@ -137,7 +128,7 @@ class CRM_Contact_Page_View_CustomData extends CRM_Core_Page { $groupTitle = CRM_Core_BAO_CustomGroup::getTitle($this->_groupId); CRM_Utils_System::setTitle(ts('View %1 Record', [1 => $groupTitle])); $groupTree = CRM_Core_BAO_CustomGroup::getTree($entityType, NULL, $this->_contactId, - $this->_groupId, $entitySubType, NULL, TRUE, NULL, FALSE, TRUE, $this->_cgcount + $this->_groupId, $entitySubType, NULL, TRUE, NULL, FALSE, CRM_Core_Permission::VIEW, $this->_cgcount ); $recId = $this->_recId; @@ -146,10 +137,10 @@ class CRM_Contact_Page_View_CustomData extends CRM_Core_Page { } else { $groupTree = CRM_Core_BAO_CustomGroup::getTree($entityType, NULL, $this->_contactId, - $this->_groupId, $entitySubType + $this->_groupId, $entitySubType, NULL, TRUE, NULL, FALSE, CRM_Core_Permission::VIEW ); } - CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, $recId, $this->_contactId); + CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, $recId, $this->_contactId, TRUE); } } else { diff --git a/CRM/Contact/Page/View/Relationship.php b/CRM/Contact/Page/View/Relationship.php index c1d88ae79c..26cb59f04c 100644 --- a/CRM/Contact/Page/View/Relationship.php +++ b/CRM/Contact/Page/View/Relationship.php @@ -94,7 +94,8 @@ class CRM_Contact_Page_View_Relationship extends CRM_Core_Page { $viewNote = CRM_Core_BAO_Note::getNote($this->getEntityId()); $this->assign('viewNote', $viewNote); - $groupTree = CRM_Core_BAO_CustomGroup::getTree('Relationship', NULL, $this->getEntityId(), 0, $relType); + $groupTree = CRM_Core_BAO_CustomGroup::getTree('Relationship', NULL, $this->getEntityId(), 0, $relType, + NULL, TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $this->getEntityId()); $rType = CRM_Utils_Array::value('rtype', $viewRelationship[$this->getEntityId()]); diff --git a/CRM/Contact/Page/View/Summary.php b/CRM/Contact/Page/View/Summary.php index 4e04ef4a59..1cfd39f9db 100644 --- a/CRM/Contact/Page/View/Summary.php +++ b/CRM/Contact/Page/View/Summary.php @@ -40,14 +40,20 @@ class CRM_Contact_Page_View_Summary extends CRM_Contact_Page_View { trim($entitySubType, CRM_Core_DAO::VALUE_SEPARATOR) ); } - $groupTree = CRM_Core_BAO_CustomGroup::getTree($entityType, + // Custom groups with VIEW permission + $visibleGroups = CRM_Core_BAO_CustomGroup::getTree($entityType, NULL, $this->_contactId, NULL, - $entitySubType + $entitySubType, + NULL, + TRUE, + NULL, + FALSE, + CRM_Core_Permission::VIEW ); - CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $this->_contactId); + CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $visibleGroups, FALSE, NULL, NULL, NULL, $this->_contactId, CRM_Core_Permission::EDIT); // also create the form element for the activity links box $controller = new CRM_Core_Controller_Simple( @@ -166,7 +172,8 @@ class CRM_Contact_Page_View_Summary extends CRM_Contact_Page_View { $idValue = $blockVal['master_id']; } } - $groupTree = CRM_Core_BAO_CustomGroup::getTree(ucfirst($key), NULL, $idValue); + $groupTree = CRM_Core_BAO_CustomGroup::getTree(ucfirst($key), NULL, $idValue, NULL, [], + NULL, TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); // we setting the prefix to dnc_ below so that we don't overwrite smarty's grouptree var. $defaults[$key][$blockId]['custom'] = CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, "dnc_"); } diff --git a/CRM/Contribute/Form/ContributionView.php b/CRM/Contribute/Form/ContributionView.php index f0cbfbe943..2e3d48bb3e 100644 --- a/CRM/Contribute/Form/ContributionView.php +++ b/CRM/Contribute/Form/ContributionView.php @@ -79,7 +79,8 @@ class CRM_Contribute_Form_ContributionView extends CRM_Core_Form { } } - $groupTree = CRM_Core_BAO_CustomGroup::getTree('Contribution', NULL, $id, 0, CRM_Utils_Array::value('financial_type_id', $values)); + $groupTree = CRM_Core_BAO_CustomGroup::getTree('Contribution', NULL, $id, 0, $values['financial_type_id'] ?? NULL, + NULL, TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $id); $premiumId = NULL; diff --git a/CRM/Contribute/Page/ContributionRecur.php b/CRM/Contribute/Page/ContributionRecur.php index 65a1453eed..b87fa360b9 100644 --- a/CRM/Contribute/Page/ContributionRecur.php +++ b/CRM/Contribute/Page/ContributionRecur.php @@ -70,7 +70,8 @@ class CRM_Contribute_Page_ContributionRecur extends CRM_Core_Page { $contributionRecur['membership_name'] = $membershipDetails['membership_name']; } - $groupTree = CRM_Core_BAO_CustomGroup::getTree('ContributionRecur', NULL, $contributionRecur['id']); + $groupTree = CRM_Core_BAO_CustomGroup::getTree('ContributionRecur', NULL, $contributionRecur['id'], NULL, [], + NULL, TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $contributionRecur['id']); $this->assign('recur', $contributionRecur); diff --git a/CRM/Core/BAO/Address.php b/CRM/Core/BAO/Address.php index 636294b426..e1623b2faa 100644 --- a/CRM/Core/BAO/Address.php +++ b/CRM/Core/BAO/Address.php @@ -85,7 +85,8 @@ class CRM_Core_BAO_Address extends CRM_Core_DAO_Address { $addressCustom = $params['custom']; } else { - $customFields = CRM_Core_BAO_CustomField::getFields('Address', FALSE, TRUE, NULL, NULL, FALSE, FALSE, $checkPermissions); + $customFields = CRM_Core_BAO_CustomField::getFields('Address', FALSE, TRUE, NULL, NULL, + FALSE, FALSE, $checkPermissions ? CRM_Core_Permission::EDIT : FALSE); if (!empty($customFields)) { $addressCustom = CRM_Core_BAO_CustomField::postProcess($params, diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php index ba06f71973..944f49878f 100644 --- a/CRM/Core/BAO/CustomField.php +++ b/CRM/Core/BAO/CustomField.php @@ -309,8 +309,8 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { * Return only top level custom data, for eg, only Participant and ignore subname and subtype. * @param bool $onlySubType * Return only custom data for subtype. - * @param bool $checkPermission - * If false, do not include permissioning clause. + * @param bool|int $checkPermission + * Either a CRM_Core_Permission constant or FALSE to disable checks * * @return array * an array of active custom fields. @@ -324,8 +324,12 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { $customDataSubName = NULL, $onlyParent = FALSE, $onlySubType = FALSE, - $checkPermission = TRUE + $checkPermission = CRM_Core_Permission::EDIT ) { + if ($checkPermission === TRUE) { + CRM_Core_Error::deprecatedWarning('Unexpected TRUE passed to CustomField::getFields $checkPermission param.'); + $checkPermission = CRM_Core_Permission::EDIT; + } if (empty($customDataType)) { $customDataType = ['Contact', 'Individual', 'Organization', 'Household']; } @@ -366,14 +370,14 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { $cacheKey .= $inline ? '_1_' : '_0_'; $cacheKey .= $onlyParent ? '_1_' : '_0_'; $cacheKey .= $onlySubType ? '_1_' : '_0_'; - $cacheKey .= $checkPermission ? '_1_' . CRM_Core_Session::getLoggedInContactID() . '_' : '_0_0_'; + $cacheKey .= $checkPermission ? $checkPermission . CRM_Core_Session::getLoggedInContactID() . '_' : '_0_0_'; $cacheKey .= '_' . CRM_Core_Config::domainID() . '_'; $cgTable = CRM_Core_DAO_CustomGroup::getTableName(); // also get the permission stuff here if ($checkPermission) { - $permissionClause = CRM_Core_Permission::customGroupClause(CRM_Core_Permission::VIEW, + $permissionClause = CRM_Core_Permission::customGroupClause($checkPermission, "{$cgTable}." ); } @@ -488,7 +492,7 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { // also get the permission stuff here if ($checkPermission) { - $permissionClause = CRM_Core_Permission::customGroupClause(CRM_Core_Permission::VIEW, + $permissionClause = CRM_Core_Permission::customGroupClause($checkPermission, "{$cgTable}.", TRUE ); } @@ -555,7 +559,10 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { } /** - * Return the field ids and names (with groups) for import purpose. + * Return field ids and names (with groups). + * + * NOTE: Despite this function's name, it is used both for IMPORT and EXPORT. + * The $checkPermission variable should be set to VIEW for export and EDIT for import. * * @param int|string $contactType Contact type * @param bool $showAll @@ -564,8 +571,8 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { * Return fields ONLY related to basic types. * @param bool $search * When called from search and multiple records need to be returned. - * @param bool $checkPermission - * If false, do not include permissioning clause. + * @param bool|int $checkPermission + * Either a CRM_Core_Permission constant or FALSE to disable checks * * @param bool $withMultiple * @@ -579,6 +586,10 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { $checkPermission = TRUE, $withMultiple = FALSE ) { + if ($checkPermission === TRUE) { + // TODO: Trigger deprecation notice for passing TRUE + $checkPermission = CRM_Core_Permission::EDIT; + } // Note: there are situations when we want getFieldsForImport() return fields related // ONLY to basic contact types, but NOT subtypes. And thats where $onlyParent is helpful $fields = &self::getFields($contactType, @@ -1443,7 +1454,7 @@ class CRM_Core_BAO_CustomField extends CRM_Core_DAO_CustomField { NULL, FALSE, FALSE, - $checkPermission + $checkPermission ? CRM_Core_Permission::EDIT : FALSE ); if (!array_key_exists($customFieldId, $customFields)) { diff --git a/CRM/Core/BAO/CustomGroup.php b/CRM/Core/BAO/CustomGroup.php index 8dda74008d..2fa0d47b5f 100644 --- a/CRM/Core/BAO/CustomGroup.php +++ b/CRM/Core/BAO/CustomGroup.php @@ -337,8 +337,8 @@ class CRM_Core_BAO_CustomGroup extends CRM_Core_DAO_CustomGroup { * @param bool $returnAll * Do not restrict by subtype at all. (The parameter feels a bit cludgey but is only used from the * api - through which it is properly tested - so can be refactored with some comfort.) - * - * @param bool $checkPermission + * @param bool|int $checkPermission + * Either a CRM_Core_Permission constant or FALSE to disable checks * @param string|int $singleRecord * holds 'new' or id if view/edit/copy form for a single record is being loaded. * @param bool $showPublicOnly @@ -367,10 +367,14 @@ class CRM_Core_BAO_CustomGroup extends CRM_Core_DAO_CustomGroup { $fromCache = TRUE, $onlySubType = NULL, $returnAll = FALSE, - $checkPermission = TRUE, + $checkPermission = CRM_Core_Permission::EDIT, $singleRecord = NULL, $showPublicOnly = FALSE ) { + if ($checkPermission === TRUE) { + CRM_Core_Error::deprecatedWarning('Unexpected TRUE passed to CustomGroup::getTree $checkPermission param.'); + $checkPermission = CRM_Core_Permission::EDIT; + } if ($entityID) { $entityID = CRM_Utils_Type::escape($entityID, 'Integer'); } @@ -529,7 +533,7 @@ WHERE civicrm_custom_group.is_active = 1 if ($checkPermission) { // ensure that the user has access to these custom groups $strWhere .= " AND " . - CRM_Core_Permission::customGroupClause(CRM_Core_Permission::VIEW, + CRM_Core_Permission::customGroupClause($checkPermission, 'civicrm_custom_group.' ); } @@ -1879,16 +1883,23 @@ SELECT IF( EXISTS(SELECT name FROM civicrm_contact_type WHERE name like %1), 1, * @param null $prefix * @param int $customValueId * @param int $entityId + * @param bool $checkEditPermission * * @return array|int * @throws \CRM_Core_Exception */ - public static function buildCustomDataView(&$form, &$groupTree, $returnCount = FALSE, $gID = NULL, $prefix = NULL, $customValueId = NULL, $entityId = NULL) { + public static function buildCustomDataView(&$form, $groupTree, $returnCount = FALSE, $gID = NULL, $prefix = NULL, $customValueId = NULL, $entityId = NULL, $checkEditPermission = FALSE) { + // Filter out pesky extra info + unset($groupTree['info']); + $details = []; + + $editableGroups = []; + if ($checkEditPermission) { + $editableGroups = \CRM_Core_Permission::customGroup(CRM_Core_Permission::EDIT); + } + foreach ($groupTree as $key => $group) { - if ($key === 'info') { - continue; - } foreach ($group['fields'] as $k => $properties) { $groupID = $group['id']; @@ -1915,7 +1926,7 @@ SELECT IF( EXISTS(SELECT name FROM civicrm_contact_type WHERE name like %1), 1, if (!isset($details[$groupID][$values['id']]['editable'])) { $details[$groupID][$values['id']]['editable'] = FALSE; } - if (empty($properties['is_view'])) { + if (empty($properties['is_view']) && in_array($key, $editableGroups)) { $details[$groupID][$values['id']]['editable'] = TRUE; } // also return contact reference contact id if user has view all or edit all contacts perm @@ -2265,4 +2276,14 @@ SELECT civicrm_custom_group.id as groupID, civicrm_custom_group.title as groupT return CRM_Core_OptionGroup::values('custom_data_type', FALSE, FALSE, FALSE, NULL, 'name')[$extendsEntityColumn]; } + /** + * @param int $groupId + * @param int $operation + * @param int|null $userId + */ + public static function checkGroupAccess($groupId, $operation = CRM_Core_Permission::EDIT, $userId = NULL): bool { + $allowedGroups = CRM_Core_Permission::customGroup($operation, FALSE, $userId); + return in_array($groupId, $allowedGroups); + } + } diff --git a/CRM/Core/BAO/CustomValue.php b/CRM/Core/BAO/CustomValue.php index ca4d0ea388..44b3d296aa 100644 --- a/CRM/Core/BAO/CustomValue.php +++ b/CRM/Core/BAO/CustomValue.php @@ -248,11 +248,8 @@ class CRM_Core_BAO_CustomValue extends CRM_Core_DAO { throw new CRM_Core_Exception('Received invalid group-name in CustomValue::checkAccess'); } - $customGroups = [$id => $id]; - $defaultGroups = CRM_Core_Permission::customGroupAdmin() ? [$id] : []; - // FIXME: Per current onscreen help (Admin=>ACLs=>Add ACLs), CustomGroup ACLs treat VIEW and EDIT as the same. Skimming code, it appears that existing checks use VIEW. - $accessList = CRM_ACL_API::group(CRM_Core_Permission::VIEW, $userID, 'civicrm_custom_group', $customGroups, $defaultGroups); - if (empty($accessList)) { + $actionType = $action === 'get' ? CRM_Core_Permission::VIEW : CRM_Core_Permission::EDIT; + if (!\CRM_Core_BAO_CustomGroup::checkGroupAccess($id, $actionType, $userID)) { return FALSE; } diff --git a/CRM/Core/BAO/UFGroup.php b/CRM/Core/BAO/UFGroup.php index 99e5ffc186..3b7df791c6 100644 --- a/CRM/Core/BAO/UFGroup.php +++ b/CRM/Core/BAO/UFGroup.php @@ -340,7 +340,7 @@ class CRM_Core_BAO_UFGroup extends CRM_Core_DAO_UFGroup { $field = CRM_Core_DAO::executeQuery($query); $importableFields = self::getProfileFieldMetadata($showAll); - list($customFields, $addressCustomFields) = self::getCustomFields($ctype); + list($customFields, $addressCustomFields) = self::getCustomFields($ctype, $skipPermission ? FALSE : $permissionType); while ($field->fetch()) { list($name, $formattedField) = self::formatUFField($group, $field, $customFields, $addressCustomFields, $importableFields, $permissionType); @@ -400,7 +400,7 @@ class CRM_Core_BAO_UFGroup extends CRM_Core_DAO_UFGroup { $profileType = CRM_Core_BAO_UFField::calculateProfileType(implode(',', $ufGroupType)); $contactActivityProfile = CRM_Core_BAO_UFField::checkContactActivityProfileTypeByGroupType(implode(',', $ufGroupType)); $importableFields = self::getImportableFields($showAll, $profileType, $contactActivityProfile); - list($customFields, $addressCustomFields) = self::getCustomFields($ctype); + list($customFields, $addressCustomFields) = self::getCustomFields($ctype, $permissionType); $formattedFields = []; foreach ($fieldArrs as $fieldArr) { @@ -722,13 +722,17 @@ class CRM_Core_BAO_UFGroup extends CRM_Core_DAO_UFGroup { /** * @param $ctype - * + * @param int|bool $checkPermission * @return mixed */ - protected static function getCustomFields($ctype) { - $cacheKey = 'uf_group_custom_fields_' . $ctype; + protected static function getCustomFields($ctype, $checkPermission = CRM_Core_Permission::VIEW) { + // Only Edit and View is supported in ACL for custom field. + if ($checkPermission == CRM_Core_Permission::CREATE) { + $checkPermission = CRM_Core_Permission::EDIT; + } + $cacheKey = 'uf_group_custom_fields_' . $ctype . '_' . (int) $checkPermission; if (!Civi::cache('metadata')->has($cacheKey)) { - $customFields = CRM_Core_BAO_CustomField::getFieldsForImport($ctype, FALSE, FALSE, FALSE, TRUE, TRUE); + $customFields = CRM_Core_BAO_CustomField::getFieldsForImport($ctype, FALSE, FALSE, FALSE, $checkPermission, TRUE); // hack to add custom data for components $components = ['Contribution', 'Participant', 'Membership', 'Activity', 'Case']; diff --git a/CRM/Core/Permission.php b/CRM/Core/Permission.php index ca50dc4ea6..6e167ea48a 100644 --- a/CRM/Core/Permission.php +++ b/CRM/Core/Permission.php @@ -208,23 +208,24 @@ class CRM_Core_Permission { } /** + * @param int $userId * @return bool */ - public static function customGroupAdmin() { + public static function customGroupAdmin($userId = NULL) { // check if user has all powerful permission // or administer civicrm permission (CRM-1905) - if (self::check('access all custom data')) { + if (self::check('access all custom data', $userId)) { return TRUE; } if ( - self::check('administer Multiple Organizations') && + self::check('administer Multiple Organizations', $userId) && self::isMultisiteEnabled() ) { return TRUE; } - if (self::check('administer CiviCRM data')) { + if (self::check('administer CiviCRM data', $userId)) { return TRUE; } @@ -234,21 +235,22 @@ class CRM_Core_Permission { /** * @param int $type * @param bool $reset + * @param int $userId * * @return array */ - public static function customGroup($type = CRM_Core_Permission::VIEW, $reset = FALSE) { + public static function customGroup($type = CRM_Core_Permission::VIEW, $reset = FALSE, $userId = NULL) { $customGroups = CRM_Core_PseudoConstant::get('CRM_Core_DAO_CustomField', 'custom_group_id', ['fresh' => $reset]); $defaultGroups = []; // check if user has all powerful permission // or administer civicrm permission (CRM-1905) - if (self::customGroupAdmin()) { - $defaultGroups = array_keys($customGroups); + if (self::customGroupAdmin($userId)) { + return array_keys($customGroups); } - return CRM_ACL_API::group($type, NULL, 'civicrm_custom_group', $customGroups, $defaultGroups); + return CRM_ACL_API::group($type, $userId, 'civicrm_custom_group', $customGroups, $defaultGroups); } /** diff --git a/CRM/Custom/Form/CustomData.php b/CRM/Custom/Form/CustomData.php index 50492eab5c..8d2803cce8 100644 --- a/CRM/Custom/Form/CustomData.php +++ b/CRM/Custom/Form/CustomData.php @@ -203,7 +203,7 @@ class CRM_Custom_Form_CustomData { $getCachedTree, $onlySubType, FALSE, - TRUE, + CRM_Core_Permission::EDIT, $singleRecord ); diff --git a/CRM/Custom/Page/AJAX.php b/CRM/Custom/Page/AJAX.php index 78e2addf77..5ddbb19ccd 100644 --- a/CRM/Custom/Page/AJAX.php +++ b/CRM/Custom/Page/AJAX.php @@ -99,6 +99,12 @@ class CRM_Custom_Page_AJAX { $params['cid'] = CRM_Utils_Type::escape($_GET['cid'], 'Integer'); $params['cgid'] = CRM_Utils_Type::escape($_GET['cgid'], 'Integer'); + if (!CRM_Core_BAO_CustomGroup::checkGroupAccess($params['cgid'], CRM_Core_Permission::VIEW) || + !CRM_Contact_BAO_Contact_Permission::allow($params['cid'], CRM_Core_Permission::VIEW) + ) { + CRM_Utils_System::permissionDenied(); + } + $contactType = CRM_Contact_BAO_Contact::getContactType($params['cid']); $obj = new CRM_Profile_Page_MultipleRecordFieldsListing(); @@ -117,7 +123,6 @@ class CRM_Custom_Page_AJAX { // format params and add class attributes $fieldList = []; foreach ($fields as $id => $value) { - $field = []; foreach ($value as $fieldId => &$fieldName) { if (!empty($attributes[$fieldId][$id]['class'])) { $fieldName = ['data' => $fieldName, 'cellClass' => $attributes[$fieldId][$id]['class']]; @@ -127,8 +132,7 @@ class CRM_Custom_Page_AJAX { CRM_Utils_Array::crmReplaceKey($value, $fieldId, $fName); } } - $field = $value; - array_push($fieldList, $field); + array_push($fieldList, $value); } $totalRecords = !empty($obj->_total) ? $obj->_total : 0; diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index aa3daad5aa..a34962d8d1 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -1282,10 +1282,12 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m // handle custom fields $mainTree = CRM_Core_BAO_CustomGroup::getTree($main['contact_type'], NULL, $mainId, -1, - CRM_Utils_Array::value('contact_sub_type', $main), NULL, TRUE, NULL, TRUE, $checkPermissions + CRM_Utils_Array::value('contact_sub_type', $main), NULL, TRUE, NULL, TRUE, + $checkPermissions ? CRM_Core_Permission::EDIT : FALSE ); $otherTree = CRM_Core_BAO_CustomGroup::getTree($main['contact_type'], NULL, $otherId, -1, - CRM_Utils_Array::value('contact_sub_type', $other), NULL, TRUE, NULL, TRUE, $checkPermissions + CRM_Utils_Array::value('contact_sub_type', $other), NULL, TRUE, NULL, TRUE, + $checkPermissions ? CRM_Core_Permission::EDIT : FALSE ); foreach ($otherTree as $gid => $group) { diff --git a/CRM/Event/Form/ParticipantView.php b/CRM/Event/Form/ParticipantView.php index 4345047ff4..57e4af7ecd 100644 --- a/CRM/Event/Form/ParticipantView.php +++ b/CRM/Event/Form/ParticipantView.php @@ -138,15 +138,20 @@ class CRM_Event_Form_ParticipantView extends CRM_Core_Form { $finalTree = []; foreach ($allRoleIDs as $k => $v) { - $roleGroupTree = CRM_Core_BAO_CustomGroup::getTree('Participant', NULL, $participantID, NULL, $v, $roleCustomDataTypeID); + $roleGroupTree = CRM_Core_BAO_CustomGroup::getTree('Participant', NULL, $participantID, NULL, $v, $roleCustomDataTypeID, + TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); $eventGroupTree = CRM_Core_BAO_CustomGroup::getTree('Participant', NULL, $participantID, NULL, - $values[$participantID]['event_id'], $eventNameCustomDataTypeID + $values[$participantID]['event_id'], $eventNameCustomDataTypeID, + TRUE, NULL, FALSE, CRM_Core_Permission::VIEW ); $eventTypeID = CRM_Core_DAO::getFieldValue("CRM_Event_DAO_Event", $values[$participantID]['event_id'], 'event_type_id', 'id'); - $eventTypeGroupTree = CRM_Core_BAO_CustomGroup::getTree('Participant', NULL, $participantID, NULL, $eventTypeID, $eventTypeCustomDataTypeID); + $eventTypeGroupTree = CRM_Core_BAO_CustomGroup::getTree('Participant', NULL, $participantID, NULL, $eventTypeID, $eventTypeCustomDataTypeID, + TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); + $participantGroupTree = CRM_Core_BAO_CustomGroup::getTree('Participant', NULL, $participantID, NULL, [], NULL, + TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); $groupTree = CRM_Utils_Array::crmArrayMerge($roleGroupTree, $eventGroupTree); $groupTree = CRM_Utils_Array::crmArrayMerge($groupTree, $eventTypeGroupTree); - $groupTree = CRM_Utils_Array::crmArrayMerge($groupTree, CRM_Core_BAO_CustomGroup::getTree('Participant', NULL, $participantID)); + $groupTree = CRM_Utils_Array::crmArrayMerge($groupTree, $participantGroupTree); foreach ($groupTree as $treeId => $trees) { $finalTree[$treeId] = $trees; } diff --git a/CRM/Event/Page/EventInfo.php b/CRM/Event/Page/EventInfo.php index 83671166ab..fe358a051c 100644 --- a/CRM/Event/Page/EventInfo.php +++ b/CRM/Event/Page/EventInfo.php @@ -172,7 +172,8 @@ class CRM_Event_Page_EventInfo extends CRM_Core_Page { } //retrieve custom field information - $groupTree = CRM_Core_BAO_CustomGroup::getTree('Event', NULL, $this->_id, 0, $values['event']['event_type_id'], NULL, TRUE, NULL, FALSE, TRUE, NULL, TRUE); + $groupTree = CRM_Core_BAO_CustomGroup::getTree('Event', NULL, $this->_id, 0, $values['event']['event_type_id'], NULL, + TRUE, NULL, FALSE, CRM_Core_Permission::VIEW, NULL, TRUE); CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $this->_id); $this->assign('action', CRM_Core_Action::VIEW); //To show the event location on maps directly on event info page diff --git a/CRM/Grant/Form/GrantView.php b/CRM/Grant/Form/GrantView.php index 672861bcf0..b6e4cfa4f5 100644 --- a/CRM/Grant/Form/GrantView.php +++ b/CRM/Grant/Form/GrantView.php @@ -99,7 +99,8 @@ class CRM_Grant_Form_GrantView extends CRM_Core_Form { $this->assign('attachment', $attachment); $grantType = CRM_Core_DAO::getFieldValue("CRM_Grant_DAO_Grant", $this->_id, "grant_type_id"); - $groupTree = CRM_Core_BAO_CustomGroup::getTree("Grant", NULL, $this->_id, 0, $grantType); + $groupTree = CRM_Core_BAO_CustomGroup::getTree("Grant", NULL, $this->_id, 0, $grantType, NULL, + TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $this->_id); $this->assign('id', $this->_id); diff --git a/CRM/Member/Form/MembershipView.php b/CRM/Member/Form/MembershipView.php index 348ea4b1eb..e376a87824 100644 --- a/CRM/Member/Form/MembershipView.php +++ b/CRM/Member/Form/MembershipView.php @@ -373,7 +373,8 @@ SELECT r.id, c.id as cid, c.display_name as name, c.job_title as comment, $memType = CRM_Core_DAO::getFieldValue("CRM_Member_DAO_Membership", $this->membershipID, "membership_type_id"); - $groupTree = CRM_Core_BAO_CustomGroup::getTree('Membership', NULL, $this->membershipID, 0, $memType); + $groupTree = CRM_Core_BAO_CustomGroup::getTree('Membership', NULL, $this->membershipID, 0, $memType, NULL, + TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $this->membershipID); $isRecur = CRM_Core_DAO::getFieldValue('CRM_Member_DAO_Membership', $this->membershipID, 'contribution_recur_id'); diff --git a/CRM/Pledge/Form/PledgeView.php b/CRM/Pledge/Form/PledgeView.php index c8785c39a3..e2095af55e 100644 --- a/CRM/Pledge/Form/PledgeView.php +++ b/CRM/Pledge/Form/PledgeView.php @@ -47,7 +47,8 @@ class CRM_Pledge_Form_PledgeView extends CRM_Core_Form { } // handle custom data. - $groupTree = CRM_Core_BAO_CustomGroup::getTree('Pledge', NULL, $params['id']); + $groupTree = CRM_Core_BAO_CustomGroup::getTree('Pledge', NULL, $params['id'], NULL, [], NULL, + TRUE, NULL, FALSE, CRM_Core_Permission::VIEW); CRM_Core_BAO_CustomGroup::buildCustomDataView($this, $groupTree, FALSE, NULL, NULL, NULL, $params['id']); if (!empty($values['contribution_page_id'])) { diff --git a/CRM/Profile/Page/MultipleRecordFieldsListing.php b/CRM/Profile/Page/MultipleRecordFieldsListing.php index a6c92afc62..87486b1174 100644 --- a/CRM/Profile/Page/MultipleRecordFieldsListing.php +++ b/CRM/Profile/Page/MultipleRecordFieldsListing.php @@ -241,7 +241,15 @@ class CRM_Profile_Page_MultipleRecordFieldsListing extends CRM_Core_Page_Basic { } $linkAction = array_sum(array_keys($this->links())); } - + // Check permissions to edit the contact and the custom fields + $editPermission = FALSE; + if ($this->_contactId) { + $editPermission = CRM_Core_BAO_CustomGroup::checkGroupAccess($customGroupId, CRM_Core_Permission::EDIT) && + CRM_Contact_BAO_Contact_Permission::allow($this->_contactId, CRM_Core_Permission::EDIT); + if (!$editPermission) { + $linkAction -= (CRM_Core_Action::COPY + CRM_Core_Action::UPDATE + CRM_Core_Action::DELETE); + } + } if (!empty($fieldIDs) && $this->_contactId) { $DTparams = !empty($this->_DTparams) ? $this->_DTparams : NULL; // commonly used for both views i.e profile listing view (profileDataView) and custom data listing view (customDataView) @@ -267,6 +275,9 @@ class CRM_Profile_Page_MultipleRecordFieldsListing extends CRM_Core_Page_Basic { } $customGroupInfo = CRM_Core_BAO_CustomGroup::getGroupTitles($fieldInput); $this->_customGroupTitle = $customGroupInfo[$fieldIdInput]['groupTitle']; + } + elseif ($this->_pageViewType == 'customDataView') { + } // $cgcount is defined before 'if' condition as entity may have no record // and $cgcount is used to build new record url @@ -430,6 +441,7 @@ class CRM_Profile_Page_MultipleRecordFieldsListing extends CRM_Core_Page_Basic { } } } + $this->assign('editPermission', $editPermission); $this->assign('dateFields', $dateFields); $this->assign('dateFieldsVals', $dateFieldsVals); $this->assign('cgcount', $cgcount); diff --git a/Civi/Api4/CustomValue.php b/Civi/Api4/CustomValue.php index 7e7496099d..a4a7d935a8 100644 --- a/Civi/Api4/CustomValue.php +++ b/Civi/Api4/CustomValue.php @@ -126,11 +126,13 @@ class CustomValue { * @return array */ public static function permissions() { - $entity = 'contact'; - $permissions = \CRM_Core_Permission::getEntityActionPermissions(); - - // Merge permissions for this entity with the defaults - return \CRM_Utils_Array::value($entity, $permissions, []) + $permissions['default']; + // Permissions are managed by ACLs + return [ + 'create' => [], + 'update' => [], + 'delete' => [], + 'get' => [], + ]; } /** diff --git a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php index 066167fb2c..d70c29f7fc 100644 --- a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php +++ b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php @@ -41,6 +41,25 @@ trait CustomValueActionTrait { return 'Custom_' . $this->getCustomGroup(); } + /** + * Is this api call permitted? + * + * This function is called if checkPermissions is set to true. + * + * @return bool + */ + public function isAuthorized(): bool { + if ($this->getActionName() !== 'getFields') { + // Check access to custom group + $permissionToCheck = $this->getActionName() == 'get' ? \CRM_Core_Permission::VIEW : \CRM_Core_Permission::EDIT; + $groupId = \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $this->getCustomGroup(), 'id', 'name'); + if (!\CRM_Core_BAO_CustomGroup::checkGroupAccess($groupId, $permissionToCheck)) { + return FALSE; + } + } + return parent::isAuthorized(); + } + /** * @inheritDoc */ diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php index 2c266c4028..31c889f965 100644 --- a/Civi/Api4/Generic/Traits/DAOActionTrait.php +++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php @@ -244,15 +244,13 @@ trait DAOActionTrait { NULL, $entityId, FALSE, - FALSE, + $this->getCheckPermissions(), TRUE ); } } - if ($customParams) { - $params['custom'] = $customParams; - } + $params['custom'] = $customParams ?: NULL; } /** diff --git a/Civi/Api4/Service/Schema/Joiner.php b/Civi/Api4/Service/Schema/Joiner.php index 353aed300b..b09aaaef32 100644 --- a/Civi/Api4/Service/Schema/Joiner.php +++ b/Civi/Api4/Service/Schema/Joiner.php @@ -14,6 +14,7 @@ namespace Civi\Api4\Service\Schema; use Civi\API\Exception\UnauthorizedException; use Civi\Api4\Query\Api4SelectQuery; +use Civi\Api4\Service\Schema\Joinable\CustomGroupJoinable; use Civi\Api4\Utils\CoreUtil; class Joiner { @@ -70,6 +71,13 @@ class Joiner { if ($joinEntity && !$query->checkEntityAccess($joinEntity)) { throw new UnauthorizedException('Cannot join to ' . $joinEntity); } + if ($query->getCheckPermissions() && is_a($link, CustomGroupJoinable::class)) { + // Check access to custom group + $groupId = \CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $link->getTargetTable(), 'id', 'table_name'); + if (!\CRM_Core_BAO_CustomGroup::checkGroupAccess($groupId, \CRM_Core_Permission::VIEW)) { + throw new UnauthorizedException('Cannot join to ' . $link->getAlias()); + } + } if ($link->isDeprecated()) { \CRM_Core_Error::deprecatedWarning("Deprecated join alias '$alias' used in APIv4 get. Should be changed to '{$alias}_id'"); } diff --git a/api/v3/CustomValue.php b/api/v3/CustomValue.php index 7c2a5a790b..72ef10ab59 100644 --- a/api/v3/CustomValue.php +++ b/api/v3/CustomValue.php @@ -340,7 +340,8 @@ function civicrm_api3_custom_value_gettree($params) { } } } - $tree = CRM_Core_BAO_CustomGroup::getTree($treeParams['entityType'], $toReturn, $params['entity_id'], NULL, $treeParams['subTypes'], $treeParams['subName'], TRUE, NULL, FALSE, CRM_Utils_Array::value('check_permissions', $params, TRUE)); + $permission = empty($params['check_permissions']) ? FALSE : CRM_Core_Permission::VIEW; + $tree = CRM_Core_BAO_CustomGroup::getTree($treeParams['entityType'], $toReturn, $params['entity_id'], NULL, $treeParams['subTypes'], $treeParams['subName'], TRUE, NULL, FALSE, $permission); unset($tree['info']); $result = []; foreach ($tree as $group) { diff --git a/api/v3/utils.php b/api/v3/utils.php index 0c8f319020..cc2ee37cad 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -1088,7 +1088,7 @@ function _civicrm_api3_object_to_array_unique_fields(&$dao, &$values) { * ID of entity per $extends. */ function _civicrm_api3_custom_format_params($params, &$values, $extends, $entityId = NULL) { - if (!empty($params['custom'])) { + if (!empty($params['custom']) && empty($params['check_permissions'])) { // The Import class does the formatting first - ideally it wouldn't but this early return // provides transitional support. return; @@ -1115,7 +1115,7 @@ function _civicrm_api3_custom_format_params($params, &$values, $extends, $entity } CRM_Core_BAO_CustomField::formatCustomField($customFieldID, $values['custom'], - $value, $extends, $customValueID, $entityId, FALSE, FALSE, TRUE + $value, $extends, $customValueID, $entityId, FALSE, !empty($params['check_permissions']), TRUE ); } } @@ -1432,7 +1432,7 @@ function _civicrm_api3_custom_data_get(&$returnArray, $checkPermission, $entity, TRUE, NULL, TRUE, - $checkPermission + $checkPermission ? CRM_Core_Permission::VIEW : FALSE ); $groupTree = CRM_Core_BAO_CustomGroup::formatGroupTree($groupTree, 1); $customValues = []; diff --git a/templates/CRM/ACL/Form/ACL.tpl b/templates/CRM/ACL/Form/ACL.tpl index 5c8bf2cc6d..f27f8974bd 100644 --- a/templates/CRM/ACL/Form/ACL.tpl +++ b/templates/CRM/ACL/Form/ACL.tpl @@ -76,7 +76,6 @@ -
{ts}NOTE: For Custom Data ACLs, the 'View' and 'Edit' operations currently do the same thing. Either option grants the right to view AND / OR edit custom data fields (in all groups, or in a specific custom data group). Neither option grants access to administration of custom data fields.{/ts}
diff --git a/templates/CRM/Contact/Page/View/CustomDataFieldView.tpl b/templates/CRM/Contact/Page/View/CustomDataFieldView.tpl index 5a02515989..44a7c62f66 100644 --- a/templates/CRM/Contact/Page/View/CustomDataFieldView.tpl +++ b/templates/CRM/Contact/Page/View/CustomDataFieldView.tpl @@ -7,9 +7,9 @@ | and copyright information, see https://civicrm.org/licensing | +--------------------------------------------------------------------+ *} -
-
- {if $permission EQ 'edit'} +
+
+ {if $permission EQ 'edit' && !empty($cd_edit.editable)}
{ts}Edit{/ts}
diff --git a/templates/CRM/Custom/Page/CustomDataView.tpl b/templates/CRM/Custom/Page/CustomDataView.tpl index df241801a4..cd6360d8a4 100644 --- a/templates/CRM/Custom/Page/CustomDataView.tpl +++ b/templates/CRM/Custom/Page/CustomDataView.tpl @@ -19,7 +19,7 @@ {if $multiRecordDisplay neq 'single'}
{assign var='index' value=$groupId|cat:"_$cvID"} - {if ($showEdit && $cd_edit.editable && $groupId) && ($editOwnCustomData or $editCustomData)} + {if $showEdit && $cd_edit.editable && $groupId && $editPermission}
{/if}
- {if $groupId and $cvID and $editCustomData and $cd_edit.editable} + {if $groupId and $cvID and $editPermission and $cd_edit.editable}
{/if} - {if !$reachedMax} + {if empty($reachedMax) && !empty($editPermission)}
{if $pageViewType eq 'customDataView'}
addRecord(['entity_id' => $c2, $textField => '2']) ->execute(); - $this->setPermissions(['access CiviCRM', 'view debug output']); + $this->setPermissions(['access CiviCRM', 'view debug output', 'access all custom data']); $this->hookClass->setHook('civicrm_aclWhereClause', [$this, 'aclWhereHookAllResults']); // Without "access deleted contacts" we won't see C2 @@ -1131,7 +1131,7 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase { $this->assertCount(1, $vals); $this->assertEquals($c1, $vals[0]['entity_id']); - $this->setPermissions(['access CiviCRM', 'access deleted contacts', 'view debug output']); + $this->setPermissions(['access CiviCRM', 'access deleted contacts', 'view debug output', 'access all custom data']); $this->hookClass->setHook('civicrm_aclWhereClause', [$this, 'aclWhereHookAllResults']); $this->cleanupCachedPermissions(); diff --git a/tests/phpunit/api/v4/Action/CustomGroupACLTest.php b/tests/phpunit/api/v4/Action/CustomGroupACLTest.php new file mode 100644 index 0000000000..ec75d2affa --- /dev/null +++ b/tests/phpunit/api/v4/Action/CustomGroupACLTest.php @@ -0,0 +1,249 @@ +addWhere('id', '>', 0)->execute(); + unset(\Civi::$statics['CRM_Contact_BAO_Contact_Permission']); + \CRM_Core_DAO::executeQuery('TRUNCATE civicrm_acl_contact_cache'); + } + + public function testViewEditCustomGroupACLs() { + $groups = ['readWrite' => 'Edit', 'readOnly' => 'View', 'superSecret' => NULL]; + $v3 = []; + + foreach ($groups as $name => $access) { + $singleGroup = CustomGroup::create(FALSE) + ->addValue('name', 'My' . ucfirst($name) . 'Single') + ->addValue('extends', 'Individual') + ->addChain('field', CustomField::create() + ->addValue('label', 'MyField') + ->addValue('html_type', 'Text') + ->addValue('custom_group_id', '$id'), 0) + ->execute()->single(); + $v3['single'][$name] = 'custom_' . $singleGroup['field']['id']; + $multiGroup = CustomGroup::create(FALSE) + ->addValue('name', 'My' . ucfirst($name) . 'Multi') + ->addValue('extends', 'Individual') + ->addValue('is_multiple', TRUE) + ->addChain('field', CustomField::create() + ->addValue('label', 'MyField') + ->addValue('html_type', 'Text') + ->addValue('custom_group_id', '$id'), 0) + ->execute()->single(); + $v3['multi'][$name] = 'custom_' . $multiGroup['field']['id']; + if ($access) { + ACL::create(FALSE)->setValues([ + 'name' => $name . 'Single', + 'entity_id' => 0, + 'operation' => $access, + 'object_table' => 'civicrm_custom_group', + 'object_id' => $singleGroup['id'], + ])->execute(); + ACL::create(FALSE)->setValues([ + 'name' => $name . 'Multi', + 'entity_id' => 0, + 'operation' => $access, + 'object_table' => 'civicrm_custom_group', + 'object_id' => $multiGroup['id'], + ])->execute(); + } + } + + $this->createLoggedInUser(); + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access all custom data', 'access CiviCRM', 'add contacts']; + + $cid = Contact::create()->setValues([ + 'contact_type' => 'Individual', + 'first_name' => 'test123', + 'MyReadWriteSingle.MyField' => '123', + 'MyReadOnlySingle.MyField' => '456', + 'MySuperSecretSingle.MyField' => '789', + ])->execute()->first()['id']; + + // TEST SINGLE-VALUE CUSTOM GROUPS + + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM', 'view all contacts', 'edit all contacts']; + + // Ensure ACLs apply to APIv4 get + $result = Contact::get() + ->addWhere('id', '=', $cid) + ->addSelect('custom.*') + ->execute()->single(); + $this->assertEquals('123', $result['MyReadWriteSingle.MyField']); + $this->assertEquals('456', $result['MyReadOnlySingle.MyField']); + $this->assertArrayNotHasKey('MySuperSecretSingle.MyField', $result); + + // Ensure ACLs apply to APIv3 get + $result = civicrm_api3('Contact', 'get', [ + 'id' => $cid, + 'check_permissions' => 1, + 'return' => [$v3['single']['readWrite'], $v3['single']['readOnly'], $v3['single']['superSecret']], + ])['values'][$cid]; + $this->assertEquals('123', $result[$v3['single']['readWrite']]); + $this->assertArrayNotHasKey($v3['single']['superSecret'], $result); + $this->assertEquals('456', $result[$v3['single']['readOnly']]); + + // Try to update all fields - ACLs will restrict based on write access + Contact::update()->setValues([ + 'id' => $cid, + 'first_name' => 'test1234', + 'MyReadWriteSingle.MyField' => '1234', + 'MyReadOnlySingle.MyField' => '4567', + 'MySuperSecretSingle.MyField' => '7890', + ])->execute(); + + // Verify only first name & readWrite field were altered by APIv4 + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access all custom data', 'access CiviCRM', 'view all contacts', 'edit all contacts']; + $result = Contact::get() + ->addWhere('id', '=', $cid) + ->addSelect('first_name', 'custom.*') + ->execute()->single(); + $this->assertEquals('test1234', $result['first_name']); + $this->assertEquals('1234', $result['MyReadWriteSingle.MyField']); + $this->assertEquals('456', $result['MyReadOnlySingle.MyField']); + $this->assertEquals('789', $result['MySuperSecretSingle.MyField']); + + // Try updating all fields with APIv3 - ACLs will restrict based on write access + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM', 'view all contacts', 'edit all contacts']; + civicrm_api3('Contact', 'create', [ + 'check_permissions' => 1, + 'id' => $cid, + 'first_name' => 'test12345', + $v3['single']['readWrite'] => '12345', + $v3['single']['readOnly'] => '45678', + $v3['single']['superSecret'] => '7890!', + ]); + + // Verify only first name & readWrite field were altered by APIv3 + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access all custom data', 'access CiviCRM', 'view all contacts', 'edit all contacts']; + $result = Contact::get() + ->addWhere('id', '=', $cid) + ->addSelect('first_name', 'custom.*') + ->execute()->single(); + $this->assertEquals('test12345', $result['first_name']); + $this->assertEquals('12345', $result['MyReadWriteSingle.MyField']); + $this->assertEquals('456', $result['MyReadOnlySingle.MyField']); + $this->assertEquals('789', $result['MySuperSecretSingle.MyField']); + + // TEST MULTI-VALUE CUSTOM GROUPS + + $multiValues = [ + 'MyReadWriteMulti' => ['red', 'blue'], + 'MyReadOnlyMulti' => ['purple', 'orange'], + 'MySuperSecretMulti' => ['brown', 'black'], + ]; + foreach ($multiValues as $groupName => $values) { + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access all custom data', 'access CiviCRM', 'view all contacts', 'edit all contacts']; + foreach ($values as $value) { + CustomValue::create($groupName) + ->addValue('MyField', $value) + ->addValue('entity_id', $cid) + ->execute(); + } + // Check that all but SuperSecret values can be read + try { + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM', 'view all contacts', 'edit all contacts']; + $result = CustomValue::get($groupName) + ->addWhere('entity_id', '=', $cid) + ->addOrderBy('id') + ->execute(); + if ($groupName === 'MySuperSecretMulti') { + $this->fail('API call should have failed'); + } + $this->assertEquals($values, $result->column('MyField')); + } + catch (\API_Exception $e) { + if ($groupName !== 'MySuperSecretMulti') { + $this->fail('API get should have succeeded'); + } + } + // Check that it works via join also + $result = Contact::get() + ->addWhere('id', '=', $cid) + ->addJoin("Custom_$groupName AS customGroup") + ->addSelect("customGroup.MyField") + ->execute(); + if ($groupName !== 'MySuperSecretMulti') { + $this->assertEquals($values, $result->column("customGroup.MyField")); + } + else { + foreach ($result as $row) { + $this->assertArrayNotHasKey("customGroup.MyField", $row); + } + } + try { + CustomValue::create($groupName) + ->addValue('MyField', 'new') + ->addValue('entity_id', $cid) + ->execute(); + if ($groupName !== 'MyReadWriteMulti') { + $this->fail('API call should have failed'); + } + } + catch (\API_Exception $e) { + if ($groupName === 'MyReadWriteMulti') { + $this->fail('API create should have succeeded'); + } + } + } + // Try updating with APIv3 + civicrm_api3('Contact', 'create', [ + 'check_permissions' => 1, + 'id' => $cid, + 'first_name' => 'test12345', + // Should update the first record in the "readWrite" group + $v3['multi']['readWrite'] . '_1' => 'changed1', + // These 2 updates should fail due to ACLs + $v3['multi']['readOnly'] . '_1' => 'changed2', + $v3['multi']['superSecret'] . '_1' => 'changed3', + ]); + + // Ensure only readWrite group has been modified + \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access all custom data', 'access CiviCRM', 'view all contacts', 'edit all contacts']; + $expectedValues = [ + 'MyReadWriteMulti' => ['changed1', 'blue', 'new'], + 'MyReadOnlyMulti' => ['purple', 'orange'], + 'MySuperSecretMulti' => ['brown', 'black'], + ]; + foreach ($expectedValues as $groupName => $expected) { + $result = CustomValue::get($groupName) + ->addWhere('entity_id', '=', $cid) + ->addOrderBy('id') + ->execute(); + $this->assertEquals($expected, $result->column('MyField')); + } + + } + +} -- 2.25.1