From: Coleman Watts Date: Tue, 4 Oct 2016 23:56:04 +0000 (-0400) Subject: CRM-19448 - Extend api permissions to cover entity_table/entity_id X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=c6835264630aa9947fbe83df6b7b284cd9fe9b89;p=civicrm-core.git 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 14432a8b9a..c33642acd2 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 9f5672c840..4d7829d778 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 04ad1cfbbb..c210060551 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 459306298e..d99c5892be 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 18b18d2b1f..c16e006d69 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']); + } } /**