From: Coleman Watts <coleman@civicrm.org>
Date: Fri, 15 Jan 2016 04:53:55 +0000 (-0500)
Subject: CRM-17645 - Better integration of hook_civicrm_selectWhereClause
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=0b80f0b48cf1a66acf155efe4bdee18325ebcaaa;p=civicrm-core.git

CRM-17645 - Better integration of hook_civicrm_selectWhereClause
---

diff --git a/CRM/Case/BAO/Case.php b/CRM/Case/BAO/Case.php
index b3e77b25ab..d4d19f8bb9 100644
--- a/CRM/Case/BAO/Case.php
+++ b/CRM/Case/BAO/Case.php
@@ -3122,18 +3122,20 @@ WHERE id IN (' . implode(',', $copiedActivityIds) . ')';
    * @inheritDoc
    */
   public function addSelectWhereClause() {
+    // We always return an array with these keys, even if they are empty,
+    // because this tells the query builder that we have considered these fields for acls
     $clauses = array(
       'id' => array(),
       // Only case admins can view deleted cases
-      'is_deleted' => CRM_Core_Permission::check('administer CiviCase') ? NULL : "= 0",
+      'is_deleted' => CRM_Core_Permission::check('administer CiviCase') ? array() : array("= 0"),
     );
     // Ensure the user has permission to view the case client
-    $contactClause = CRM_Contact_BAO_Contact_Permission::cacheSubquery();
+    $contactClause = CRM_Core_DAO::mergeSubquery('Contact');
     if ($contactClause) {
       $contactClause = implode(' AND contact_id ', $contactClause);
       $clauses['id'][] = "IN (SELECT case_id FROM civicrm_case_contact WHERE contact_id $contactClause)";
     }
-    // The api gatekeeper ensures the user has at least "access all cases and activities"
+    // The api gatekeeper ensures the user has at least "access my cases and activities"
     // so if they do not have permission to see all cases we'll assume they can only access their own
     if (!CRM_Core_Permission::check('access all cases and activities')) {
       $user = (int) CRM_Core_Session::getLoggedInContactID();
diff --git a/CRM/Case/BAO/CaseContact.php b/CRM/Case/BAO/CaseContact.php
index 08751a224c..4e693b42b3 100644
--- a/CRM/Case/BAO/CaseContact.php
+++ b/CRM/Case/BAO/CaseContact.php
@@ -81,28 +81,13 @@ class CRM_Case_BAO_CaseContact extends CRM_Case_DAO_CaseContact {
    * @inheritDoc
    */
   public function addSelectWhereClause() {
-    // In order to make things easier for downstream developers, we reuse and adapt case acls here.
-    // This doesn't yield the most straightforward query, but hopefully the sql engine will sort it out.
-    $clauses = array(
+    return array(
+      // Reuse case acls
+      'case_id' => CRM_Core_DAO::mergeSubquery('Case'),
       // Case acls already check for contact access so we can just mark contact_id as handled
-      'contact_id' => NULL,
-      'case_id' => array(),
+      'contact_id' => array(),
     );
-    $caseSubclauses = array();
-    $caseBao = new CRM_Case_BAO_Case();
-    foreach ($caseBao->addSelectWhereClause() as $field => $fieldClauses) {
-      if ($field == 'id' && $fieldClauses) {
-        $clauses['case_id'] = array_merge($clauses['case_id'], (array) $fieldClauses);
-      }
-      elseif ($fieldClauses) {
-        $caseSubclauses[] = "$field " . implode(" AND $field ", (array) $fieldClauses);
-      }
-    }
-    if ($caseSubclauses) {
-      $clauses['case_id'][] = 'IN (SELECT id FROM civicrm_case WHERE ' . implode(' AND ', $caseSubclauses) . ')';
-    }
-    CRM_Utils_Hook::selectWhereClause($this, $clauses);
-    return $clauses;
+    // Don't call hook selectWhereClause, the case query already did
   }
 
 }
diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
index a8813dc283..d8b00f9a44 100644
--- a/CRM/Contact/BAO/Contact.php
+++ b/CRM/Contact/BAO/Contact.php
@@ -3429,8 +3429,11 @@ LEFT JOIN civicrm_address add2 ON ( add1.master_id = add2.id )
    * @inheritDoc
    */
   public function addSelectWhereClause() {
+    // We always return an array with these keys, even if they are empty,
+    // because this tells the query builder that we have considered these fields for acls
     $clauses = array(
-      'id' => CRM_Contact_BAO_Contact_Permission::cacheSubquery(),
+      'id' => (array) CRM_Contact_BAO_Contact_Permission::cacheSubquery(),
+      'is_deleted' => CRM_Core_Permission::check('access deleted contacts') ? array() : array('!= 1'),
     );
     CRM_Utils_Hook::selectWhereClause($this, $clauses);
     return $clauses;
diff --git a/CRM/Contact/BAO/Contact/Permission.php b/CRM/Contact/BAO/Contact/Permission.php
index 72b53585ca..91935931b4 100644
--- a/CRM/Contact/BAO/Contact/Permission.php
+++ b/CRM/Contact/BAO/Contact/Permission.php
@@ -225,23 +225,17 @@ AND    $operationClause LIMIT 1";
   }
 
   /**
-   * Generate acl subqueries that can be placed in the WHERE clause of a query or the ON clause of a JOIN
+   * Generate acl subquery that can be placed in the WHERE clause of a query or the ON clause of a JOIN
    *
-   * Returns an array of clauses that can each be placed after the name of the contact_id field in the query.
-   *
-   * @return array
+   * @return string|null
    */
   public static function cacheSubquery() {
-    $clauses = array();
     if (!CRM_Core_Permission::check(array(array('view all contacts', 'edit all contacts')))) {
       $contactID = (int) CRM_Core_Session::getLoggedInContactID();
       self::cache($contactID);
-      $clauses[] = "IN (SELECT contact_id FROM civicrm_acl_contact_cache WHERE user_id = $contactID)";
-    }
-    if (!CRM_Core_Permission::check('access deleted contacts')) {
-      $clauses[] = "NOT IN (SELECT id FROM civicrm_contact WHERE is_deleted = 1)";
+      return "IN (SELECT contact_id FROM civicrm_acl_contact_cache WHERE user_id = $contactID)";
     }
-    return $clauses;
+    return NULL;
   }
 
   /**
diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php
index 58ed491d0f..8f4c19f378 100644
--- a/CRM/Contact/BAO/Relationship.php
+++ b/CRM/Contact/BAO/Relationship.php
@@ -2101,17 +2101,4 @@ AND cc.sort_name LIKE '%$name%'";
     return $relationshipsDT;
   }
 
-  /**
-   * @inheritDoc
-   */
-  public function addSelectWhereClause() {
-    // Generate an acl clause for both contacts in the relationship
-    $clauses = array(
-      'contact_id_a' => CRM_Contact_BAO_Contact_Permission::cacheSubquery(),
-      'contact_id_b' => CRM_Contact_BAO_Contact_Permission::cacheSubquery(),
-    );
-    CRM_Utils_Hook::selectWhereClause($this, $clauses);
-    return $clauses;
-  }
-
 }
diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index 1fed8486c7..f256fde225 100644
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -2467,17 +2467,22 @@ 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();
-    $fields = $this->fields();
-    $cidField = CRM_Utils_Array::value('contact_id', $fields);
-    if (CRM_Utils_Array::value('FKClassName', $cidField) == 'CRM_Contact_DAO_Contact') {
-      $clauses['contact_id'] = CRM_Contact_BAO_Contact_Permission::cacheSubquery();
+    foreach ($this->fields() as $fieldName => $field) {
+      if (strpos($fieldName, 'contact_id') === 0 && CRM_Utils_Array::value('FKClassName', $field) == 'CRM_Contact_DAO_Contact') {
+        $clauses[$fieldName] = self::mergeSubquery('Contact');
+      }
     }
     CRM_Utils_Hook::selectWhereClause($this, $clauses);
     return $clauses;
   }
 
   /**
+   * This returns the final permissioned query string for this entity
+   *
+   * With acls from related entities + additional clauses from hook_civicrm_selectWhereClause
+   *
    * @param string $tableAlias
    * @return array
    */
@@ -2496,4 +2501,29 @@ SELECT contact_id
     return $clauses;
   }
 
+  /**
+   * Helper function for adding the permissioned subquery from one entity onto another
+   *
+   * @param string $entity
+   * @param string $joinColumn
+   * @return array
+   */
+  public static function mergeSubquery($entity, $joinColumn = 'id') {
+    $baoName = _civicrm_api3_get_BAO($entity);
+    $bao = new $baoName();
+    $clauses = $subclauses = array();
+    foreach ((array) $bao->addSelectWhereClause() as $field => $vals) {
+      if ($vals && $field == $joinColumn) {
+        $clauses = array_merge($clauses, (array) $vals);
+      }
+      elseif ($vals) {
+        $subclauses[] = "$field " . implode(" AND $field ", (array) $vals);
+      }
+    }
+    if ($subclauses) {
+      $clauses[] = "IN (SELECT `$joinColumn` FROM `" . $bao->tableName() . "` WHERE " . implode(' AND ', $subclauses) . ")";
+    }
+    return $clauses;
+  }
+
 }
diff --git a/api/v3/utils.php b/api/v3/utils.php
index a63c15f560..ecae06f1bb 100644
--- a/api/v3/utils.php
+++ b/api/v3/utils.php
@@ -373,7 +373,7 @@ function _civicrm_api3_get_DAO($name) {
 }
 
 /**
- * Return the DAO of the function or Entity.
+ * Return the BAO name of the function or Entity.
  *
  * @param string $name
  *   Is either a function of the api (civicrm_{entity}_create or the entity name.