From: eileenmcnaughton <eileen@fuzion.co.nz>
Date: Mon, 25 Jan 2016 03:13:02 +0000 (+0000)
Subject: CRM-17350 fix previous entity acl fix to make check_permissions optional
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=424616b83a89e4a75bec88438abddd469dd5ec30;p=civicrm-core.git

CRM-17350 fix previous entity acl fix to make check_permissions optional
---

diff --git a/CRM/Activity/Form/Task/AddToTag.php b/CRM/Activity/Form/Task/AddToTag.php
index d975cb4a06..155702b894 100644
--- a/CRM/Activity/Form/Task/AddToTag.php
+++ b/CRM/Activity/Form/Task/AddToTag.php
@@ -135,7 +135,8 @@ class CRM_Activity_Form_Task_AddToTag extends CRM_Activity_Form_Task {
     foreach ($allTags as $key => $dnc) {
       $this->_name[] = $this->_tags[$key];
 
-      list($total, $added, $notAdded) = CRM_Core_BAO_EntityTag::addEntitiesToTag($this->_activityHolderIds, $key, 'civicrm_activity');
+      list($total, $added, $notAdded) = CRM_Core_BAO_EntityTag::addEntitiesToTag($this->_activityHolderIds, $key,
+        'civicrm_activity', FALSE);
 
       $status = array(ts('Activity tagged', array('count' => $added, 'plural' => '%count activities tagged')));
       if ($notAdded) {
diff --git a/CRM/Activity/Form/Task/RemoveFromTag.php b/CRM/Activity/Form/Task/RemoveFromTag.php
index 27a3c064aa..3c3c305b85 100644
--- a/CRM/Activity/Form/Task/RemoveFromTag.php
+++ b/CRM/Activity/Form/Task/RemoveFromTag.php
@@ -124,7 +124,8 @@ class CRM_Activity_Form_Task_RemoveFromTag extends CRM_Activity_Form_Task {
     foreach ($allTags as $key => $dnc) {
       $this->_name[] = $this->_tags[$key];
 
-      list($total, $removed, $notRemoved) = CRM_Core_BAO_EntityTag::removeEntitiesFromTag($this->_activityHolderIds, $key, 'civicrm_activity');
+      list($total, $removed, $notRemoved) = CRM_Core_BAO_EntityTag::removeEntitiesFromTag($this->_activityHolderIds,
+        $key, 'civicrm_activity', FALSE);
 
       $status = array(
         ts('%count activity un-tagged', array(
diff --git a/CRM/Contact/Form/Task/AddToTag.php b/CRM/Contact/Form/Task/AddToTag.php
index 99437277a6..16b2e16264 100644
--- a/CRM/Contact/Form/Task/AddToTag.php
+++ b/CRM/Contact/Form/Task/AddToTag.php
@@ -136,7 +136,8 @@ class CRM_Contact_Form_Task_AddToTag extends CRM_Contact_Form_Task {
     foreach ($allTags as $key => $dnc) {
       $this->_name[] = $this->_tags[$key];
 
-      list($total, $added, $notAdded) = CRM_Core_BAO_EntityTag::addEntitiesToTag($this->_contactIds, $key);
+      list($total, $added, $notAdded) = CRM_Core_BAO_EntityTag::addEntitiesToTag($this->_contactIds, $key,
+        'civicrm_contact', FALSE);
 
       $status = array(ts('%count contact tagged', array('count' => $added, 'plural' => '%count contacts tagged')));
       if ($notAdded) {
diff --git a/CRM/Contact/Form/Task/RemoveFromTag.php b/CRM/Contact/Form/Task/RemoveFromTag.php
index d2d4820e03..4e5d5c6b74 100644
--- a/CRM/Contact/Form/Task/RemoveFromTag.php
+++ b/CRM/Contact/Form/Task/RemoveFromTag.php
@@ -124,7 +124,8 @@ class CRM_Contact_Form_Task_RemoveFromTag extends CRM_Contact_Form_Task {
     foreach ($allTags as $key => $dnc) {
       $this->_name[] = $this->_tags[$key];
 
-      list($total, $removed, $notRemoved) = CRM_Core_BAO_EntityTag::removeEntitiesFromTag($this->_contactIds, $key);
+      list($total, $removed, $notRemoved) = CRM_Core_BAO_EntityTag::removeEntitiesFromTag($this->_contactIds, $key,
+        'civicrm_contact', FALSE);
 
       $status = array(
         ts('%count contact un-tagged', array(
diff --git a/CRM/Contact/Import/ImportJob.php b/CRM/Contact/Import/ImportJob.php
index c09d128837..b0ec85c527 100644
--- a/CRM/Contact/Import/ImportJob.php
+++ b/CRM/Contact/Import/ImportJob.php
@@ -424,7 +424,7 @@ class CRM_Contact_Import_ImportJob {
     if (is_array($this->_tag)) {
       $tagAdditions = array();
       foreach ($this->_tag as $tagId => $val) {
-        $addTagCount = CRM_Core_BAO_EntityTag::addEntitiesToTag($contactIds, $tagId);
+        $addTagCount = CRM_Core_BAO_EntityTag::addEntitiesToTag($contactIds, $tagId, 'civicrm_contact', FALSE);
         $totalTagCount = $addTagCount[1];
         if (isset($addedTag) && $tagId == $addedTag->id) {
           $tagName = $newTagName;
diff --git a/CRM/Core/BAO/EntityTag.php b/CRM/Core/BAO/EntityTag.php
index 5ceed31b9e..449bbb6779 100644
--- a/CRM/Core/BAO/EntityTag.php
+++ b/CRM/Core/BAO/EntityTag.php
@@ -129,11 +129,13 @@ class CRM_Core_BAO_EntityTag extends CRM_Core_DAO_EntityTag {
    *   The id of the tag.
    * @param string $entityTable
    *   Name of entity table default:civicrm_contact.
+   * @param bool $applyPermissions
+   *   Should permissions be applied in this function.
    *
    * @return array
-   *   (total, added, notAdded) count of enities added to tag
+   *   (total, added, notAdded) count of entities added to tag
    */
-  public static function addEntitiesToTag(&$entityIds, $tagId, $entityTable = 'civicrm_contact') {
+  public static function addEntitiesToTag(&$entityIds, $tagId, $entityTable, $applyPermissions) {
     $numEntitiesAdded = 0;
     $numEntitiesNotAdded = 0;
     $entityIdsAdded = array();
@@ -141,7 +143,7 @@ class CRM_Core_BAO_EntityTag extends CRM_Core_DAO_EntityTag {
     foreach ($entityIds as $entityId) {
       // CRM-17350 - check if we have permission to edit the contact
       // that this tag belongs to.
-      if (!CRM_Contact_BAO_Contact_Permission::allow($entityId, CRM_Core_Permission::EDIT)) {
+      if ($applyPermissions && !self::checkPermissionOnEntityTag($entityId, $entityTable)) {
         $numEntitiesNotAdded++;
         continue;
       }
@@ -172,7 +174,30 @@ class CRM_Core_BAO_EntityTag extends CRM_Core_DAO_EntityTag {
   }
 
   /**
-   * Given an array of entity ids and entity table, remove entity(s) tags
+   * Basic check for ACL permission on editing/creating/removing a tag.
+   *
+   * In the absence of something better contacts get a proper check and other entities
+   * default to 'edit all contacts'. This is currently only accessed from the api which previously
+   * applied edit all contacts to all - so while still too restrictive it represents a loosening.
+   *
+   * Current possible entities are attachments, activities, cases & contacts.
+   *
+   * @param int $entityID
+   * @param string $entityTable
+   *
+   * @return bool
+   */
+  public static function checkPermissionOnEntityTag($entityID, $entityTable) {
+    if ($entityTable == 'civicrm_contact') {
+      return CRM_Contact_BAO_Contact_Permission::allow($entityID, CRM_Core_Permission::EDIT);
+    }
+    else {
+      return CRM_Core_Permission::check('edit all contacts');
+    }
+  }
+
+  /**
+   * Given an array of entity ids and entity table, remove entity(s)tags.
    *
    * @param array $entityIds
    *   (reference ) the array of entity ids to be removed.
@@ -180,11 +205,13 @@ class CRM_Core_BAO_EntityTag extends CRM_Core_DAO_EntityTag {
    *   The id of the tag.
    * @param string $entityTable
    *   Name of entity table default:civicrm_contact.
+   * @param bool $applyPermissions
+   *   Should permissions be applied in this function.
    *
    * @return array
    *   (total, removed, notRemoved) count of entities removed from tags
    */
-  public static function removeEntitiesFromTag(&$entityIds, $tagId, $entityTable = 'civicrm_contact') {
+  public static function removeEntitiesFromTag(&$entityIds, $tagId, $entityTable, $applyPermissions) {
     $numEntitiesRemoved = 0;
     $numEntitiesNotRemoved = 0;
     $entityIdsRemoved = array();
@@ -192,8 +219,8 @@ class CRM_Core_BAO_EntityTag extends CRM_Core_DAO_EntityTag {
     foreach ($entityIds as $entityId) {
       // CRM-17350 - check if we have permission to edit the contact
       // that this tag belongs to.
-      if (!CRM_Contact_BAO_Contact_Permission::allow($entityId, CRM_Core_Permission::EDIT)) {
-        $numEntitiesNotAdded++;
+      if ($applyPermissions && !self::checkPermissionOnEntityTag($entityId, $entityTable)) {
+        $numEntitiesNotRemoved++;
         continue;
       }
       $tag = new CRM_Core_DAO_EntityTag();
diff --git a/api/v3/EntityTag.php b/api/v3/EntityTag.php
index f46af28d25..9d7c0bf395 100644
--- a/api/v3/EntityTag.php
+++ b/api/v3/EntityTag.php
@@ -49,6 +49,7 @@ function civicrm_api3_entity_tag_get($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);
@@ -128,6 +129,7 @@ function _civicrm_api3_entity_tag_common($params, $op = 'add') {
       }
     }
   }
+
   if (empty($entityIDs)) {
     return civicrm_api3_create_error('contact_id is a required field');
   }
@@ -145,7 +147,8 @@ function _civicrm_api3_entity_tag_common($params, $op = 'add') {
   if ($op == 'add') {
     $values['total_count'] = $values['added'] = $values['not_added'] = 0;
     foreach ($tagIDs as $tagID) {
-      list($te, $a, $na) = CRM_Core_BAO_EntityTag::addEntitiesToTag($entityIDs, $tagID, $entityTable);
+      list($te, $a, $na) = CRM_Core_BAO_EntityTag::addEntitiesToTag($entityIDs, $tagID, $entityTable,
+        CRM_Utils_Array::value('check_permissions', $params));
       $values['total_count'] += $te;
       $values['added'] += $a;
       $values['not_added'] += $na;
@@ -154,7 +157,7 @@ function _civicrm_api3_entity_tag_common($params, $op = 'add') {
   else {
     $values['total_count'] = $values['removed'] = $values['not_removed'] = 0;
     foreach ($tagIDs as $tagID) {
-      list($te, $r, $nr) = CRM_Core_BAO_EntityTag::removeEntitiesFromTag($entityIDs, $tagID, $entityTable);
+      list($te, $r, $nr) = CRM_Core_BAO_EntityTag::removeEntitiesFromTag($entityIDs, $tagID, $entityTable, CRM_Utils_Array::value('check_permissions', $params));
       $values['total_count'] += $te;
       $values['removed'] += $r;
       $values['not_removed'] += $nr;