From c6835264630aa9947fbe83df6b7b284cd9fe9b89 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Tue, 4 Oct 2016 19:56:04 -0400
Subject: [PATCH] CRM-19448 - Extend api permissions to cover
entity_table/entity_id
---
CRM/Core/BAO/Note.php | 6 +++++
CRM/Core/DAO.php | 23 +++++++++++++++--
CRM/Core/DAO/permissions.php | 19 ++------------
api/v3/EntityTag.php | 19 ++++----------
tests/phpunit/api/v3/ACLPermissionTest.php | 29 ++++++++++++++++++++++
5 files changed, 63 insertions(+), 33 deletions(-)
diff --git a/CRM/Core/BAO/Note.php b/CRM/Core/BAO/Note.php
index 14432a8b9a0..c33642acd25 100644
--- a/CRM/Core/BAO/Note.php
+++ b/CRM/Core/BAO/Note.php
@@ -143,6 +143,12 @@ class CRM_Core_BAO_Note extends CRM_Core_DAO_Note {
return CRM_Core_DAO::$_nullObject;
}
+ if ($params['entity_table'] == 'civicrm_contact' && !empty($params['check_permissions'])) {
+ if (!CRM_Contact_BAO_Contact_Permission::allow($params['entity_id'], CRM_Core_Permission::EDIT)) {
+ throw new CRM_Exception('Permission denied to modify contact record');
+ }
+ }
+
$note = new CRM_Core_BAO_Note();
if (!isset($params['modified_date'])) {
diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 9f5672c8404..4d7829d778e 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -2424,12 +2424,31 @@ SELECT contact_id
* @return array
*/
public function addSelectWhereClause() {
- // This is the default fallback, and works for contact-related entities like Email, Relationship, etc.
$clauses = array();
- foreach ($this->fields() as $fieldName => $field) {
+ $fields = $this->fields();
+ foreach ($fields as $fieldName => $field) {
+ // Clause for contact-related entities like Email, Relationship, etc.
if (strpos($fieldName, 'contact_id') === 0 && CRM_Utils_Array::value('FKClassName', $field) == 'CRM_Contact_DAO_Contact') {
$clauses[$fieldName] = CRM_Utils_SQL::mergeSubquery('Contact');
}
+ // Clause for an entity_table/entity_id combo
+ if ($fieldName == 'entity_id' && isset($fields['entity_table'])) {
+ $relatedClauses = array();
+ $relatedEntities = $this->buildOptions('entity_table', 'get');
+ foreach ((array) $relatedEntities as $table => $ent) {
+ $ent = CRM_Core_DAO_AllCoreTables::getBriefName(CRM_Core_DAO_AllCoreTables::getClassForTable($table));
+ $subquery = CRM_Utils_SQL::mergeSubquery($ent);
+ if ($subquery) {
+ $relatedClauses[] = "(entity_table = '$table' AND entity_id " . implode(' AND entity_id ', $subquery) . ")";
+ }
+ else {
+ $relatedClauses[] = "(entity_table = '$table')";
+ }
+ }
+ if ($relatedClauses) {
+ $clauses['id'] = 'IN (SELECT id FROM `' . $this->tableName() . '` WHERE (' . implode(') OR (', $relatedClauses) . '))';
+ }
+ }
}
CRM_Utils_Hook::selectWhereClause($this, $clauses);
return $clauses;
diff --git a/CRM/Core/DAO/permissions.php b/CRM/Core/DAO/permissions.php
index 04ad1cfbbb1..c210060551c 100644
--- a/CRM/Core/DAO/permissions.php
+++ b/CRM/Core/DAO/permissions.php
@@ -116,25 +116,10 @@ function _civicrm_api3_permissions($entity, $action, &$params) {
$permissions['website'] = $permissions['address'];
$permissions['im'] = $permissions['address'];
- // @todo - implement CRM_Core_BAO_EntityTag::addSelectWhereClause and remove this heavy-handed restriction
- $permissions['entity_tag'] = array(
- 'get' => array('access CiviCRM', 'view all contacts'),
- 'default' => array('access CiviCRM', 'edit all contacts'),
- );
- // @todo - ditto
+ // Also managed by ACLs - CRM-19448
+ $permissions['entity_tag'] = array('default' => array());
$permissions['note'] = $permissions['entity_tag'];
- // CRM-17350 - entity_tag ACL permissions are checked at the BAO level
- $permissions['entity_tag'] = array(
- 'get' => array(
- 'access CiviCRM',
- 'view all contacts',
- ),
- 'default' => array(
- 'access CiviCRM',
- ),
- );
-
// Allow non-admins to get and create tags to support tagset widget
// Delete is still reserved for admins
$permissions['tag'] = array(
diff --git a/api/v3/EntityTag.php b/api/v3/EntityTag.php
index 459306298e7..d99c5892be9 100644
--- a/api/v3/EntityTag.php
+++ b/api/v3/EntityTag.php
@@ -42,20 +42,7 @@
* @return array
*/
function civicrm_api3_entity_tag_get($params) {
-
- if (empty($params['entity_id'])) {
- return _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params);
- }
- else {
- //do legacy non-standard behaviour
- $values = CRM_Core_BAO_EntityTag::getTag($params['entity_id'], $params['entity_table']);
-
- $result = array();
- foreach ($values as $v) {
- $result[$v] = array('tag_id' => $v);
- }
- return civicrm_api3_create_success($result, $params, 'EntityTag');
- }
+ return _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params);
}
/**
@@ -163,5 +150,9 @@ function _civicrm_api3_entity_tag_common($params, $op = 'add') {
$values['not_removed'] += $nr;
}
}
+ if (empty($values['added']) && empty($values['removed'])) {
+ $values['is_error'] = 1;
+ $values['error_message'] = "Unable to $op tags";
+ }
return $values;
}
diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php
index 18b18d2b1f5..c16e006d691 100644
--- a/tests/phpunit/api/v3/ACLPermissionTest.php
+++ b/tests/phpunit/api/v3/ACLPermissionTest.php
@@ -66,6 +66,9 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
'civicrm_uf_match',
'civicrm_activity',
'civicrm_activity_contact',
+ 'civicrm_note',
+ 'civicrm_entity_tag',
+ 'civicrm_tag',
);
$this->quickCleanup($tablesToTruncate);
$config = CRM_Core_Config::singleton();
@@ -136,6 +139,7 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
);
// We should be prevented from getting or creating entities for a contact we don't have permission for
$this->callAPIFailure($entity, 'create', $params);
+ $this->callAPISuccess($entity, 'create', array('check_permissions' => 0) + $params);
$results = $this->callAPISuccess($entity, 'get', array('contact_id' => $disallowedContact, 'check_permissions' => 1));
$this->assertEquals(0, $results['count']);
@@ -145,6 +149,31 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
$results = $this->callAPISuccess($entity, 'get', array('contact_id' => $this->allowedContactId, 'check_permissions' => 1));
$this->assertGreaterThan(0, $results['count']);
}
+ $newTag = civicrm_api3('Tag', 'create', array(
+ 'name' => 'Foo123',
+ ));
+ $relatedEntities = array(
+ 'Note' => array('note' => 'abc'),
+ 'EntityTag' => array('tag_id' => $newTag['id']),
+ );
+ foreach ($relatedEntities as $entity => $params) {
+ $params += array(
+ 'entity_id' => $disallowedContact,
+ 'entity_table' => 'civicrm_contact',
+ 'check_permissions' => 1,
+ );
+ // We should be prevented from getting or creating entities for a contact we don't have permission for
+ $this->callAPIFailure($entity, 'create', $params);
+ $this->callAPISuccess($entity, 'create', array('check_permissions' => 0) + $params);
+ $results = $this->callAPISuccess($entity, 'get', array('entity_id' => $disallowedContact, 'entity_table' => 'civicrm_contact', 'check_permissions' => 1));
+ $this->assertEquals(0, $results['count']);
+
+ // We should be allowed to create and get for entities we do have permission on
+ $params['entity_id'] = $this->allowedContactId;
+ $this->callAPISuccess($entity, 'create', $params);
+ $results = $this->callAPISuccess($entity, 'get', array('entity_id' => $this->allowedContactId, 'entity_table' => 'civicrm_contact', 'check_permissions' => 1));
+ $this->assertGreaterThan(0, $results['count']);
+ }
}
/**
--
GitLab