CRM-19448 - Extend api permissions to cover entity_table/entity_id

diff --git a/CRM/Core/BAO/Note.php b/CRM/Core/BAO/Note.php
index 14432a8b9a0640d752e0ff8a791edbadbd9d11fb..c33642acd25536dab7296521783970ef5f0c455a 100644 (file)
--- 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 9f5672c8404566c53b3de01e28ce5ba366cd6b14..4d7829d778e2506c4b66840147d4d433cd34b360 100644 (file)
--- 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 04ad1cfbbb1fb9de2e7b4ec022d065f9901c33f5..c210060551cf90cc88bfe6f61493937b6cee1189 100644 (file)
--- 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 459306298e7a9e00c4ace618e1c2dd5b96a01745..d99c5892be96d330678200cf87e2a9d8bf557812 100644 (file)
--- 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 18b18d2b1f558e90021f6712889aab1235e23a50..c16e006d69190259444fa8f812559e54df8d566e 100644 (file)
--- 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']);
+    }
   }
 
   /**