From 1876e376e3b63322c73d5dffce39324f3533b94b Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Tue, 15 Jan 2019 12:44:50 +1300
Subject: [PATCH] Add test to demonstrate fatal error when accessing permitted
 users that are cached using the acl_cache.

This has arisen during investigation of a possible regression - it turns out that if you give the 'everyone' group
access to a contact using ACLs (or hooks I believe) they get a fatal error on any attempt at event or other registration.

The issue is that when attempting to check for duplicates the call is made using check_permission. This in itself is a possible
regression as the CRM_Dedupe_Finder::dupesByParams function now drops the check_permission key when it is equal to 0 from

https://github.com/civicrm/civicrm-core/commit/4f33e78b901fb7cdb38a3026f88b59a2f9fd2c68

So we have an issue that
1) we are now applying check_permission when doing a dupe_check from front end forms - this probably is resulting in 5.9
sites getting too many duplicates are they would always be null for anon users
2) if we DO do a permissions check when an acl or hook has been used to give anon users permission to access
contacts then they will get a fatal error. This is because it sets contact_id to 0 and attempts to insert it into the
acl_contact_cache.

I think we need to either remove the array_filter line that we think we may not need per code comments
or add specific handling for the check_permission flag

AND drop the foreign key constraint on the civicm_acl_contact_cache table. This means they will
no longer be removed when a contact is deleted but this is a clean up issue rather than one with
functionaly implications & we *should* have some form of cleanup in play on that table. In addition,
removing the constraint will reduce write contention
---
 tests/phpunit/api/v3/ACLPermissionTest.php | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php
index da3c1ce092..73ffd354ef 100644
--- a/tests/phpunit/api/v3/ACLPermissionTest.php
+++ b/tests/phpunit/api/v3/ACLPermissionTest.php
@@ -678,6 +678,36 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
       'id' => $this->scenarioIDs['Contact']['non_permitted_contact'],
       'check_permissions' => 1,
     ], 0);
+
+    // Also check that we can access ACLs through a path that uses the acl_contact_cache table.
+    // historically this has caused errors due to the key_constraint on that table.
+    // This is a bit of an artificial check as we have to amp up permissions to access this api.
+    // However, the lower level function is more directly accessed through the Contribution & Event & Profile
+    $dupes = $this->callAPISuccess('Contact', 'duplicatecheck', [
+      'match' => [
+        'first_name' => 'Anthony',
+        'last_name' => 'Anderson',
+        'contact_type' => 'Individual',
+        'email' => 'anthony_anderson@civicrm.org',
+      ],
+      'check_permissions' => 0,
+    ]);
+    // Actually this should be 2 but there is a line of array_filter in dupesByParams that causes
+    // check_permissions to be dropped at that point. I am working aginst rc now - that should possibly be removed against master.
+    $this->assertEquals(1, $dupes['count']);
+    CRM_Core_Config::singleton()->userPermissionClass->permissions = ['administer CiviCRM'];
+
+    $dupes = $this->callAPISuccess('Contact', 'duplicatecheck', [
+      'match' => [
+        'first_name' => 'Anthony',
+        'last_name' => 'Anderson',
+        'contact_type' => 'Individual',
+        'email' => 'anthony_anderson@civicrm.org',
+      ],
+      'check_permissions' => 1,
+    ]);
+    $this->assertEquals(1, $dupes['count']);
+
   }
 
 }
-- 
2.25.1