From 0b80f0b48cf1a66acf155efe4bdee18325ebcaaa Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 14 Jan 2016 23:53:55 -0500 Subject: [PATCH] CRM-17645 - Better integration of hook_civicrm_selectWhereClause --- CRM/Case/BAO/Case.php | 8 ++++-- CRM/Case/BAO/CaseContact.php | 25 ++++------------- CRM/Contact/BAO/Contact.php | 5 +++- CRM/Contact/BAO/Contact/Permission.php | 14 +++------- CRM/Contact/BAO/Relationship.php | 13 --------- CRM/Core/DAO.php | 38 +++++++++++++++++++++++--- api/v3/utils.php | 2 +- 7 files changed, 53 insertions(+), 52 deletions(-) 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. -- 2.25.1