Commit 1876e376 authored by eileen's avatar eileen

Add test to demonstrate fatal error when accessing permitted users that are...

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
parent 2e4c3928
......@@ -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']);
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment