From e4541c561c2dcbc6e5ba955c5798c5f6207da6d8 Mon Sep 17 00:00:00 2001 From: systopia Date: Wed, 12 Oct 2016 22:51:26 +0100 Subject: [PATCH] obeying master jenkins obeying master jenkins --- CRM/Contact/BAO/Contact/Permission.php | 33 ++++++++--------- CRM/Contact/Selector.php | 1 - tests/phpunit/CRM/ACL/ListTest.php | 51 ++++++++++++-------------- 3 files changed, 39 insertions(+), 46 deletions(-) diff --git a/CRM/Contact/BAO/Contact/Permission.php b/CRM/Contact/BAO/Contact/Permission.php index 44ba8e7d37..9cdcee8c0b 100644 --- a/CRM/Contact/BAO/Contact/Permission.php +++ b/CRM/Contact/BAO/Contact/Permission.php @@ -33,7 +33,7 @@ class CRM_Contact_BAO_Contact_Permission { /** - * Check which of the given contact IDs the logged in user + * Check which of the given contact IDs the logged in user * has permissions for the operation type according * * Caution: general permissions (like 'edit all contacts') @@ -56,7 +56,7 @@ class CRM_Contact_BAO_Contact_Permission { $contact_id_list = implode(',', $contact_ids); // make sure the the general permissions are given - if ( CRM_Core_Permission::check('edit all contacts') + if (CRM_Core_Permission::check('edit all contacts') || $type == CRM_Core_Permission::VIEW && CRM_Core_Permission::check('view all contacts') ) { @@ -64,8 +64,8 @@ class CRM_Contact_BAO_Contact_Permission { if (CRM_Core_Permission::check('access deleted contacts')) { // if user can access delted contacts -> fine return $contact_ids; - - } else { + } + else { // if the user CANNOT access deleted contacts, these need to be filtered $filter_query = "SELECT DISTINCT(id) FROM civicrm_contact WHERE id IN ($contact_id_list) AND is_deleted = 0"; $query = CRM_Core_DAO::executeQuery($filter_query); @@ -93,7 +93,7 @@ class CRM_Contact_BAO_Contact_Permission { $LEFT_JOIN_DELETED = $CAN_ACCESS_DELETED = ''; if (!CRM_Core_Permission::check('access deleted contacts')) { $LEFT_JOIN_DELETED = "LEFT JOIN civicrm_contact ON civicrm_contact.id = contact_id"; - $AND_CAN_ACCESS_DELETED = "AND civicrm_contact.is_deleted = 0"; + $AND_CAN_ACCESS_DELETED = "AND civicrm_contact.is_deleted = 0"; } // RUN the query @@ -110,7 +110,7 @@ WHERE contact_id IN ({$contact_id_list}) $result_set[(int) $result->contact_id] = TRUE; } - // if some have been rejected, double check for permissions inherited by relationship + // if some have been rejected, double check for permissions inherited by relationship if (count($result_set) < count($contact_ids)) { $rejected_contacts = array_diff_key($contact_ids, $result_set); $allowed_by_relationship = self::relationshipList($rejected_contacts); @@ -138,8 +138,8 @@ WHERE contact_id IN ({$contact_id_list}) $contactID = (int) $session->get('userID'); // first: check if contact is trying to view own contact - if ( $type == CRM_Core_Permission::VIEW && CRM_Core_Permission::check('view my contact') - || $type == CRM_Core_Permission::EDIT && CRM_Core_Permission::check('edit my contact') + if ($type == CRM_Core_Permission::VIEW && CRM_Core_Permission::check('view my contact') + || $type == CRM_Core_Permission::EDIT && CRM_Core_Permission::check('edit my contact') ) { return TRUE; } @@ -188,8 +188,9 @@ WHERE contact_a.id = %1 AND $permission"; * Should we force a recompute. */ public static function cache($userID, $type = CRM_Core_Permission::VIEW, $force = FALSE) { - static $_processed = array( CRM_Core_Permission::VIEW => array(), - CRM_Core_Permission::EDIT => array()); + static $_processed = array( + CRM_Core_Permission::VIEW => array(), + CRM_Core_Permission::EDIT => array()); if ($type == CRM_Core_Permission::VIEW) { $operationClause = " operation IN ( 'Edit', 'View' ) "; @@ -437,7 +438,6 @@ WHERE (( contact_id_a = %1 AND contact_id_b = %2 AND is_permission_a_b = 1 ) OR } - /** * Filter a list of contact_ids by the ones that the * currently active user as a permissioned relationship with @@ -450,7 +450,7 @@ WHERE (( contact_id_a = %1 AND contact_id_b = %2 AND is_permission_a_b = 1 ) OR */ public static function relationshipList($contact_ids) { $result_set = array(); - + // no processing empty lists (avoid SQL errors as well) if (empty($contact_ids)) { return array(); @@ -480,7 +480,7 @@ WHERE (( contact_id_a = %1 AND contact_id_b = %2 AND is_permission_a_b = 1 ) OR $LEFT_JOIN_DELETED = $AND_CAN_ACCESS_DELETED = ''; if (!CRM_Core_Permission::check('access deleted contacts')) { $LEFT_JOIN_DELETED = "LEFT JOIN civicrm_contact ON civicrm_contact.id = {$contact_id_column} "; - $AND_CAN_ACCESS_DELETED = "AND civicrm_contact.is_deleted = 0"; + $AND_CAN_ACCESS_DELETED = "AND civicrm_contact.is_deleted = 0"; } $queries[] = " @@ -492,8 +492,7 @@ SELECT DISTINCT(civicrm_relationship.{$contact_id_column}) AS contact_id AND civicrm_relationship.is_active = 1 AND civicrm_relationship.is_permission_{$direction['from']}_{$direction['to']} = 1 $AND_CAN_ACCESS_DELETED"; - } - + } if ($config->secondDegRelPermissions) { // ADD SECOND DEGREE RELATIONSHIPS @@ -501,7 +500,7 @@ SELECT DISTINCT(civicrm_relationship.{$contact_id_column}) AS contact_id foreach ($directions as $first_direction) { foreach ($directions as $second_direction) { // add clause for deleted contacts, if the user doesn't have the permission to access them - $LEFT_JOIN_DELETED = $AND_CAN_ACCESS_DELETED = ''; + $LEFT_JOIN_DELETED = $AND_CAN_ACCESS_DELETED = ''; if (!CRM_Core_Permission::check('access deleted contacts')) { $LEFT_JOIN_DELETED = "LEFT JOIN civicrm_contact first_degree_contact ON first_degree_contact.id = second_degree_relationship.contact_id_{$second_direction['from']}\n"; $LEFT_JOIN_DELETED .= "LEFT JOIN civicrm_contact second_degree_contact ON second_degree_contact.id = second_degree_relationship.contact_id_{$second_direction['to']} "; @@ -537,8 +536,6 @@ SELECT DISTINCT(civicrm_relationship.{$contact_id_column}) AS contact_id } - - /** * @param int $contactID * @param CRM_Core_Form $form diff --git a/CRM/Contact/Selector.php b/CRM/Contact/Selector.php index d77437d904..a8e1aa30fc 100644 --- a/CRM/Contact/Selector.php +++ b/CRM/Contact/Selector.php @@ -933,7 +933,6 @@ class CRM_Contact_Selector extends CRM_Core_Selector_Base implements CRM_Core_Se $links_template = self::links($this->_context, $this->_contextMenu, $this->_key); - foreach ($rows as $id => & $row) { $links = $links_template; diff --git a/tests/phpunit/CRM/ACL/ListTest.php b/tests/phpunit/CRM/ACL/ListTest.php index 85365751c4..0b5b650806 100644 --- a/tests/phpunit/CRM/ACL/ListTest.php +++ b/tests/phpunit/CRM/ACL/ListTest.php @@ -24,7 +24,7 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { * general test for the 'view all contacts' permission */ public function testViewAllPermission() { - // create test contacts + // create test contacts $contacts = $this->createScenarioPlain(); // test WITH all permissions @@ -33,14 +33,12 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { sort($result); $this->assertEquals($result, $contacts, "Contacts should be viewable when 'view all contacts'"); - // test WITH explicit permission CRM_Core_Config::singleton()->userPermissionClass->permissions = array('view all contacts'); $result = CRM_Contact_BAO_Contact_Permission::allowList($contacts, CRM_Core_Permission::VIEW); sort($result); $this->assertEquals($result, $contacts, "Contacts should be viewable when 'view all contacts'"); - // test WITHOUT permission CRM_Core_Config::singleton()->userPermissionClass->permissions = array(); $result = CRM_Contact_BAO_Contact_Permission::allowList($contacts); @@ -62,7 +60,6 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { sort($result); $this->assertEquals($result, $contacts, "Contacts should be viewable when 'edit all contacts'"); - // test WITHOUT permission CRM_Core_Config::singleton()->userPermissionClass->permissions = array(); $result = CRM_Contact_BAO_Contact_Permission::allowList($contacts); @@ -89,14 +86,13 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { $result = CRM_Contact_BAO_Contact_Permission::allowList($contacts, CRM_Core_Permission::EDIT); sort($result); $this->assertNotContains($deleted_contact_id, $result, "Deleted contacts should be excluded"); - $this->assertEquals(count($result), count($contacts)-1, "Only deleted contacts should be excluded"); - + $this->assertEquals(count($result), count($contacts) - 1, "Only deleted contacts should be excluded"); } /** * Test access based on relations - * + * * There should be the following permission-relationship * contact[0] -> contact[1] -> contact[2] */ @@ -116,14 +112,13 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { $result = CRM_Contact_BAO_Contact_Permission::allowList($contacts, $permission); sort($result); - $this->assertNotContains($contacts[0], $result, "User[0] should NOT have $permission_label permission on contact[0]."); - $this->assertContains( $contacts[1], $result, "User[0] should have $permission_label permission on contact[1]."); + $this->assertContains($contacts[1], $result, "User[0] should have $permission_label permission on contact[1]."); $this->assertNotContains($contacts[2], $result, "User[0] should NOT have $permission_label permission on contact[2]."); $this->assertNotContains($contacts[3], $result, "User[0] should NOT have $permission_label permission on contact[3]."); $this->assertNotContains($contacts[4], $result, "User[0] should NOT have $permission_label permission on contact[4]."); } - + // run this for SECOND DEGREE relations $config->secondDegRelPermissions = TRUE; $this->assertTrue($config->secondDegRelPermissions); @@ -132,8 +127,8 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { sort($result); $this->assertNotContains($contacts[0], $result, "User[0] should NOT have $permission_label permission on contact[0]."); - $this->assertContains( $contacts[1], $result, "User[0] should have $permission_label permission on contact[1]."); - $this->assertContains( $contacts[2], $result, "User[0] should have second degree $permission_label permission on contact[2]."); + $this->assertContains($contacts[1], $result, "User[0] should have $permission_label permission on contact[1]."); + $this->assertContains($contacts[2], $result, "User[0] should have second degree $permission_label permission on contact[2]."); $this->assertNotContains($contacts[3], $result, "User[0] should NOT have $permission_label permission on contact[3]."); $this->assertNotContains($contacts[4], $result, "User[0] should NOT have $permission_label permission on contact[4]."); } @@ -157,11 +152,11 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { $result = CRM_Contact_BAO_Contact_Permission::allowList($contacts); sort($result); - $this->assertContains( $contacts[0], $result, "User[0] should NOT have an ACL permission on contact[0]."); - $this->assertContains( $contacts[1], $result, "User[0] should have an ACL permission on contact[1]."); + $this->assertContains($contacts[0], $result, "User[0] should NOT have an ACL permission on contact[0]."); + $this->assertContains($contacts[1], $result, "User[0] should have an ACL permission on contact[1]."); $this->assertNotContains($contacts[2], $result, "User[0] should NOT have an ACL permission on contact[2]."); $this->assertNotContains($contacts[3], $result, "User[0] should NOT have an RELATION permission on contact[3]."); - $this->assertContains( $contacts[4], $result, "User[0] should NOT have an ACL permission on contact[4]."); + $this->assertContains($contacts[4], $result, "User[0] should NOT have an ACL permission on contact[4]."); } @@ -183,11 +178,11 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { $result = CRM_Contact_BAO_Contact_Permission::allowList($contacts); sort($result); - $this->assertContains( $contacts[0], $result, "User[0] should have an ACL permission on contact[0]."); - $this->assertContains( $contacts[1], $result, "User[0] should have an ACL permission on contact[1]."); - $this->assertContains( $contacts[2], $result, "User[0] should have second degree an relation permission on contact[2]."); + $this->assertContains($contacts[0], $result, "User[0] should have an ACL permission on contact[0]."); + $this->assertContains($contacts[1], $result, "User[0] should have an ACL permission on contact[1]."); + $this->assertContains($contacts[2], $result, "User[0] should have second degree an relation permission on contact[2]."); $this->assertNotContains($contacts[3], $result, "User[0] should NOT have an ACL permission on contact[3]."); - $this->assertContains( $contacts[4], $result, "User[0] should have an ACL permission on contact[4]."); + $this->assertContains($contacts[4], $result, "User[0] should have an ACL permission on contact[4]."); } /** @@ -213,7 +208,7 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { foreach ($user_permission_options as $user_permissions) { // select the contact range $contact_range = $contacts; - if (is_array($user_permissions) && count($user_permissions)==0) { + if (is_array($user_permissions) && count($user_permissions) == 0) { // slight (explainable) deviation on the own contact unset($contact_range[0]); } @@ -236,7 +231,8 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { // listPermission reports PERMISSION GRANTED $this->assertTrue($individual_result, "Permission::allow denies {$permission_label} access for contact[{$contact_index[$contact_id]}], while Permission::allowList grants it. User permission: '{$user_permissions_label}'"); - } else { + } + else { // listPermission reports PERMISSION DENIED $this->assertFalse($individual_result, "Permission::allow grantes {$permission_label} access for contact[{$contact_index[$contact_id]}], while Permission::allowList denies it. User permission: '{$user_permissions_label}'"); @@ -278,7 +274,7 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { // create some relationships $this->callAPISuccess('Relationship', 'create', array( - 'relationship_type_id' => 1, // CHILD OF + 'relationship_type_id' => 1, // CHILD OF 'contact_id_a' => $contacts[1], 'contact_id_b' => $contacts[0], 'is_permission_b_a' => 1, @@ -286,7 +282,7 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { )); $this->callAPISuccess('Relationship', 'create', array( - 'relationship_type_id' => 1, // CHILD OF + 'relationship_type_id' => 1, // CHILD OF 'contact_id_a' => $contacts[2], 'contact_id_b' => $contacts[1], 'is_permission_b_a' => 1, @@ -295,7 +291,7 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { // create some relationships $this->callAPISuccess('Relationship', 'create', array( - 'relationship_type_id' => 1, // CHILD OF + 'relationship_type_id' => 1, // CHILD OF 'contact_id_a' => $contacts[4], 'contact_id_b' => $contacts[2], 'is_permission_b_a' => 1, @@ -305,13 +301,14 @@ class CRM_ACL_ListTest extends CiviUnitTestCase { return $contacts; } - /** - * ACL HOOK implementation for various tests + /** + * ACL HOOK implementation for various tests */ - public function hook_civicrm_aclWhereClause($type, &$tables, &$whereTables, &$contactID, &$where ) { + public function hook_civicrm_aclWhereClause($type, &$tables, &$whereTables, &$contactID, &$where) { if (!empty($this->allowedContactsACL)) { $contact_id_list = implode(',', $this->allowedContactsACL); $where = " contact_a.id IN ($contact_id_list)"; } } + } -- 2.25.1