CRM-20441 Fix issue where multiple activity Ids were not supported wh…

[seamuslee001]

…en check permissions was used

[seamuslee001]

@davejenx @eileenmcnaughton @monishdeb Hey Folks i'm 99% sure this will fix the breakage Dave found. Dave are you able to test this?

[eileenmcnaughton]

Looks good - thanks!

[seamuslee001]

just pasting here from Mattermost: @JohnFF "@seamuslee looks good"

[davejenx]

@seamuslee001 Just tested the current 4.7.19-rc branch and I now get a different fatal error on contact view as an ACL'd user with just "access CiviCRM" + ACL view for one group.
"Sorry but we are not able to provide this at the moment.
You do not have permission to view this activity"
That error is happening on /civicrm/contact/view?reset=1&cid=X where X is the id of any contact that the user is permitted to view and that has activities. I'm not expecting to see this message when trying to view contact summary.

Backtrace:
#0 .../dmaster/sites/all/modules/civicrm/CRM/Core/Error.php(456): CRM_Core_Error::backtrace("backTrace", TRUE)
#1 .../dmaster/sites/all/modules/civicrm/CRM/Core/Invoke.php(55): CRM_Core_Error::handleUnhandledException(Object(CiviCRM_API3_Exception))
#2 .../dmaster/sites/all/modules/civicrm/drupal/civicrm.module(448): CRM_Core_Invoke::invoke((Array:3))
#3 internal function: civicrm_invoke("contact", "view")
#4 .../dmaster/includes/menu.inc(527): call_user_func_array("civicrm_invoke", (Array:2))
#5 .../dmaster/index.php(21): menu_execute_active_handler()
#6 {main}

[seamuslee001]

@davejenx

So first off that makes some sort of sense to a degree as its probably trying to get a count of the number of activities

2. From what i have seen to view an activity you need access to all 2 / 3 contacts involved (source + target if no assignee or if there is one you need to have access to the assignee as well)

In that respect i believe that this is a new issue rather than the same issue as CRM-20441 as CRM-20441 was dealing with the problem where the check was failing as it couldn't interpret multiple ids. Here it is handling that and failing This is the function which is killing things for you https://github.com/civicrm/civicrm-core/blob/master/CRM/Activity/BAO/Activity.php#L2084

[seamuslee001]

@colemanw Coleman Can you please comment on Dave's latest error as to what we should do, It appears it is correctly checking permissions now but that check is failing

[colemanw]

This could result in permission escalation if the operator is not "IN". E.g. passing 'id' => array('NOT IN' => 123) would let you see all activities if you have access to view 123.

[seamuslee001]

@colemanw should we limit it to just IN for the moment or include Like as well?

[colemanw]

I think let's play it safe and stick with IN.

…

On 04/24/2017 07:08 PM, Seamus Lee wrote: ***@***.**** commented on this pull request. ------------------------------------------------------------------------ In api/v3/Activity.php <#10212 (comment)>: > @@ -304,11 +304,23 @@ function civicrm_api3_activity_get($params) { "Cannot access activities. Required permission: 'view all activities''" ); } - - if (!CRM_Activity_BAO_Activity::checkPermission($params['id'], CRM_Core_Action::VIEW)) { - throw new \Civi\API\Exception\UnauthorizedException( - 'You do not have permission to view this activity' - ); + $ids = array(); + if (is_array($params['id'])) { + foreach ($params['id'] as $operator => $values) { + if (in_array($operator, CRM_Core_DAO::acceptedSQLOperators())) { @colemanw <https://github.com/colemanw> should we limit it to just IN for the moment or include Like as well? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10212 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACveIKW-lby7Zs-HjAyOtnEr0iyrizmOks5rzStWgaJpZM4NE47U>.