CiviCRM Community Forums (archive)

Hi,

I have an ACL hook in place to restrict access to contacts. This works fine but when viewing the Relationships tab on contact summary, all related contacts are shown, regardless of whether the user is permitted by the ACL hook to see them. Their email, phone etc appear and the contact name links to contact view, but clicking on it gives access denied. Seems like a bug to me.

The code involved seems to be CRM_Contact_Page_View_Relationship::browse(), which calls CRM_Contact_BAO_Relationship::getRelationship, which doesn't seem to be ACL-aware.

(Edit 24 Oct 16:40 BST:)
It would be useful to have an opinion from the core team as to whether this behaviour is regarded as clearly wrong & so needing fixing in core, or whether it's intended / desired for some use cases. Obviously I'm expecting "please contribute a patch if it's important to your use case" ;-) but what I'm trying to get straight first is whether changing the behaviour in core is the correct thing to do.

Dave J

Thanks for confirming. How many hours would you estimate?

Dave J

Hi Lobo,

I've come up with one way of doing this, not sure whether it's a good way. I was struggling to see how CRM_Contact_BAO_Relationship::getRelationship can be made ACL-aware without a complete rewrite. The ACL code that I've seen for contacts uses CRM_ACL_BAO_ACL::whereClause, which IIUC assumes the query is being constructed with CRM_Contact_BAO_Query or similar, which it isn't. So what I've done is a sneaky subquery that does a simple ACL'd query to get the contact ids of contacts the user may view. Not the most elegant or, I suspect, efficient approach but, in early testing, seems to produce the right results on the relationships tab.

CRM_Contact_BAO_Relationship::getRelationship is called in over a dozen other places in the code base so this has the potential to bring ACL-flavoured good or harm to them all! Would need to review whether ACLs should be applied in each of these contexts.

Patch attached, needs much more testing and vetting for general sanity.

Dave J

Hi Lobo,

Filed http://issues.civicrm.org/jira/browse/CRM-9335 with patch. Hope the sprint's going well.

Cheers,

Dave J