Multi-Org segregation failures: CiviMail sends to contacts from other sites

[davej]

Hi,

A big segregation failure with CiviMail, on 3.1.3 + latest multi-org patches. A mailing was sent from L2 site A. The recipients selected were:

+ Members of smart group "Is a member of A", defined as: Membership Type = any of the available types. Note that as mentioned in http://forum.civicrm.org/index.php/topic,13326.0.html , the search form shows membership types from other domains that should not be there. The user had selected all of these membership types for the smart group.
Members: 361.

+ Members of group "A Newsletter"
Members: 302.

However the mailing was sent to 2544 contacts.

Total number of contacts in site A = 978.

Total number of memberships of all types across all domains = 363.

So I really don't know where the 2544 came from. However it's very close to:

Code: [Select]

select count(distinct contact_id) from civicrm_email;
+----------------------------+
| count(distinct contact_id) |
+----------------------------+
|                       2548 |
+----------------------------+

I.e. the mailing has gone to all but 4 contacts with email addresses across the whole multi-org db. The user who sent the mailing does not have view or edit all contact permission or administer multiple orgs.

Lobo has suggested that the cause is the CiviMail cron script running with full privileges, due to the way scripts authenticate - see IRC transcripts below. However even if this is true, I can't see where the number 2544 came from. If I log into the site as uid 1 with full multi-org privileges and view group members of the two groups to which the mailing was sent, they have 361 and 302 members, just as they do when viewed by a normal user. Exactly the same if I log into the L1 (grandparent) site as uid 1. So I don't see how a permissioning problem explains the number of recipients being 2544.

The client has confirmed that when they created the mailing, the recipient count shown was 358.

Dave J

-- Recording IRC discussion 20 Apr 2010 --

[14:28] <davej_> Hi dlobo, any thoughts on http://forum.civicrm.org/index.php/topic,13351.0.html other than aargh?
[14:28] <dlobo> we'll need to deal with it next week davej_
[14:29] <dlobo> davej_: what user was runnng the cron job
[14:29] <dlobo> i bet its access control with regard to that
[14:29] <dlobo> wanna do a test (log all email)
[14:29] <dlobo> and send it via q=civicrm/mailing/queue&reset=1
[14:29] <dlobo> with a permissioned used
[14:29] <dlobo> user
[14:29] <dlobo> davej_: above 4 sentences were for u
...
[17:51] <davej_> dlobo: cron runs as a user with no Drupal roles, so just auth user and in no groups apart from correct site parent group. Haven't managed to replicate problem on dev site yet.

-- Recording IRC discussion 21 Apr 2010 --

[14:15] <davej_> Hi dlobo, no joy with CiviMail issue. Cron runs as user with no roles, just auth user, no Civi perms except register/view eventy & subscribe. In correct site parent group and no other groups.
[14:16] <davej_> Haven't been able to replicate on dev using CIVICRM_MAIL_LOG.
[14:16] <dlobo> davej_: i think when cron runs, the code is structured to give "all" permissions
[14:16] <dlobo> but there is an issue (already fied i think) that we bootstrap drupal
[14:17] <dlobo> in which case we'll inherit permissions etc
[14:17] <dlobo> can this wait till friday
[14:17] <dlobo> we are  bit slammed with drupalcon
[14:18] <dlobo> and civicon planning
[14:18] <dlobo> sorry for the response, and i hope u'll understand
[14:22] <davej_> dlobo. Sure. Just had call ... pressure to do another mailing on Monday. Any pointers welcome as I've drawn a blank so far. I was assuming the recipient list gets complied & stored at the point when the mailing is created rather than at send time.
[14:22] <dlobo> davej_: the easy guaranteed way (and u should test it)
[14:22] <dlobo> with the current code base is:
[14:22] <dlobo> 1. disable cron job
[14:22] <dlobo> 2. create a mailing
[14:23] <dlobo> 3. schedule it for now
[14:23] <dlobo> 4. run q=civicrm/mailing/queue&reset=1 (as the user with selective permission)
[14:23] <dlobo> this thing creates the stuff
[14:23] <dlobo> but you had a good idea of creating the list when we schedule it, that we should consider
[14:24] <davej_> I tried 1-4 on dev site, also tried sending via cron & both worked correctly, can't replicate the problem.
[14:25] <dlobo> davej_: i'm prety sure the cron stuff is a bug
[14:25] <dlobo> and i can replicate it
[14:25] <dlobo> i know the way it happens
[14:27] <davej_> dlobo: client thinks mailings were working OK until recently. The one that went wrong uses a smart group, wonder if that's relevant. Anyway, better let you get on.
[14:27] <dlobo> yeah
[14:27] <dlobo> this only affects smart groups primarily
[14:28] <dlobo> (or static groups where u have contacts from different orgs there, but this is not very likely)
[14:28] <dlobo> any place where we need to put permissioning
[14:29] <davej_> So fix will be next week at the earliest I guess - or something I might be able to do?
[14:32] <dlobo> davej_: if u do steps 1 to 4 with smart groups it will work
[14:32] <dlobo> (please test)
[14:34] <dlobo> since at the point, we are running in a drupal environment
[14:34] <dlobo> and know the acl's and permissions
[14:35] <davej_> dlobo: Thanks. I just wish I could replicate the problem so that I can be confident we have isolated it & found a reliable workaround. However it refuses to break, whether I send via cron or as proper user. But thanks for this!

[Eileen]

Hi, Just an extra note to this. The mailings don't seem to be domain specific so if two sites run the cron the first one to run will try to send the mailing. (or in the case we had if they run more or less at the same time they both will)

3.1.4

[Donald Lobo]

eileen:

Since you'll are also quite involved with the multi-site bandwagon, would be great if u'll could spend some developer hours in figuring out how best to address the below issue and file a patch for that

we are quite slammed with various 3.x issues and multi-site seems to be slipping a wee bit any help would be really great and much appreciated

lobo

[Eileen]

Superficially I'm not sure how civi determines which site a mailing belongs to as there is no domain id on the civicrm_mailing table and it seems that the cron for each site will try to send all the mailings

[davej]

Hi Eileen,

Superficially I'm not sure how civi determines which site a mailing belongs to as there is no domain id on the civicrm_mailing table and it seems that the cron for each site will try to send all the mailings

civicrm_mailing has created_id, which is the other mechanism used to determine which site an entity belongs to: if the creating contact belongs to site A's site parent group then show the entity in site A. I believe this is what's used to filter the list of mailings shown at civicrm/mailing&reset=1 . I expect the civimail cron job could and should do the same.

Dave J

[davej]

Recording further IRC discussions...

26 Apr

[17:18] <davej_> dlobo: did you see the updated (on Weds) details of the CiviMail issue at http://forum.civicrm.org/index.php/topic,13351.0.html ?
[17:19] <dlobo> deepaks: can u please check
[17:19] <dlobo> lets see if we can try to reproduce
[17:19] <dlobo> i think i have an idea / steps
[17:19] <dlobo> davej_: if u have a test harness, before u send out the email, can u truncate group_contact_cache
[17:19] <dlobo> and see if that extracts all records
[17:20] <davej_> dlobo: Will try.
[17:20] <dlobo> deepaks: can u read the forum posts and start thinking about it
[17:20] <dlobo> we'll chat at next break
[17:20] <deepaks> dlobo: ok
[17:32] <davej_> dlobo: I tried truncating civicrm_group_contact_cache before sending mailing, still can't replicate issue, it sent to correct number. Count of civicrm_group_contact_cache remained zero after sending.
[17:50] <davej_> dlobo: I'm off shortly, anything else to try quickly?
[17:50] <dlobo> davej_: did u try the truncate and see if it reproduced?
[17:50] <davej_> dlobo: Yes, see above, didn't replicate problem.
[17:51] <dlobo> and u used smart groups, davej_
[17:52] <dlobo> davej_: deepak and i will take a shot at it
[17:52] <davej_> dlobo: yes, used copy of the actual mailing that misbehaved.

28 Apr

[15:37] <davej_> dlobo: Is there any way the Outbound Email setting (SMTP vs Sendmail) could be implicated in our CiviMail problem? Clutching at straws but it seems this setting was changed to sendmail not long before the mailing, because the client was getting errors from the SMTP library when sending test civimails.
[15:37] <dlobo> argh
[15:37] <dlobo> i dont think its tht
[15:38] <dlobo> but we do recommend smtp
[15:38] <dlobo> do u want to the client to send out one more mailing
[15:38] <dlobo> exact same way
[15:38] <dlobo> but this time (switch to smtp to avoid bugs)
[15:38] <dlobo> and enable logging
[15:38] <dlobo> insread of mail going out?
[15:40] <davej_> dlobo: Can try; but in trying to replicate the problem, I"ve been using SMTP + CIVICRM_MAIL_LOG and can't replicate. And I can't do a comparison text using sendmail, as CIVICRM_MAIL_LOG is then ignored.
[15:40] <davej_> text = test
[15:41] <davej_> Until we can replicate the problem, I'm nervous about letting them do a real mailout.
[15:41] <dlobo> it is real but not really real
[15:41] <dlobo> since u r logging the email
[15:41] <dlobo> and not sending it
[15:44] <davej_> We'll give it a go on live, but doing the same test on dev using a copy of the live db & same code base, can't replicate the error.
[16:01] <davej_> dlobo: I tried the above test on live, same behaviour as dev, i.e. couldn't replicate problem, correct no. of recipients.
[16:01] <dlobo> davej_: ok, we'll do a code review
[16:06] <davej_> dlobo: OK. I'm recommending to Olly that they don't do any more CiviMailings until we've replicated the issue & have a fix or workaround or know what scenario triggers the problem, but he's being pressured. Are you confident that it's specific to smart groups? Difficult to tell until we can replcate!

30 Apr

[15:57] <davej_> Hi dlobo deepaks, OK to chat about CiviMail issue?
[15:57] <dlobo> deepaks:
[15:57] <deepaks> davej_: ready
[15:57] <dlobo> wanana give an update
[15:58] <davej_> Nothing new really, can't replicate.
[15:58] <deepaks> dlobo did some tests yesterday using smart groups and came out that even for normal user in multi-site setup, for civimail the recipient count covers contacts from all domains, and same for cron
[15:59] <deepaks> and suspect that could be the problem
[15:59] <davej_> Then we may have 2 issues, as our recipient count can't be explained by ACL failure.
[16:00] <davej_> Looking at smart group members as uid 1 on grandparent site, it's still far fewer than no of recipients.
[16:01] <davej_> - as described on the forum post.
[16:02] <deepaks> davej_: when u logged in normal user and try to create mailing using that smart group, whats the count u see from second step onwards
[16:03] <davej_> COunt shows expected number - and client confirmed that they also saw expected number.
[16:03] <deepaks> davej_: if thats different from search listing count, the same user see
[16:06] <deepaks> davej_: i have a simple setup ready which demonstrate this.
[16:07] <deepaks> not sure what other problem could be
[16:07] <dlobo> davej_: u need to replicate as civimail.cronjob.php
[16:07] <dlobo> which basically acts like a user WITHOUT the drupal framework
[16:07] <dlobo> in 3.1
[16:07] <dlobo> and hence the whole permissioning stuff break
[16:08] <dlobo> i.e. the cronjob user is basically admin
[16:08] <dlobo> i think the best solution is to port the 3.2 patch where we bootstrap drupal in the cronjobs to 3.1
[16:09] <davej_> deepaks: If we assume 2 problems: (1) civimail cron script not correctly ACL'd; (2) a mailing was sent to a number of recipients far greater than what could be explained by ACL failure; then your setup demonstrates (1), is that right?
[16:09] <davej_> I have been testing with civimail.cronjob.php throughout.
[16:10] <davej_> - and can't replicate problem (2).
[16:11] <davej_> However at least a fix for (1) would limit the damage that (2) could do!
[16:12] <dlobo> davej_: hang on
[16:12] <dlobo> some more discoveries
[16:12] <dlobo> looking at code
[16:12] <davej_> Ooh...
[16:12] <dlobo> seems like getRecipients does not use permissioning code at ALL
[16:12] <dlobo> (its some old code that we have not looked at with regard to permissioning)
[16:13] <davej_> So (1) happens even if run by correct user through queue URL
[16:13] <davej_> ?
[16:14] <davej_> I.e. /civicrm/mailing/queue&reset=1 ?
[16:19] <davej_> I haven't really tested problem (1) because the groups the mailing was sent to show the same number of members whether viewed as normal user or uid 1.
[16:20] <dlobo> yes
[16:21] <davej_> One approach we discussed was for civimail to compile the recipient list when the mail is scheduled.
[16:22] <davej_> This would allow a definitive count to be shown, so would reveal if problem (2) occurred before actually sending.
[16:22] <davej_> And allows correct ACL to be used.
[16:25] <davej_> - Otherwise how would cron script know which user's permissions to apply?
[16:29] <dlobo> davej_: cron script is sent a name / password
[16:29] <dlobo> so it should apply permissions based on that name/password
[16:30] <dlobo> deepaks: is looking at applying an ACL patch
[16:30] <dlobo> for the mailing stuf
[16:30] <dlobo> which should result in correct behavior
[16:31] <davej_> dlobo: yes, just wondering about the case - not necessarily in multi-org - where the user composing the mail only has access to a subset of contacts and expects the mail only to go to contacts within that subset (cos they're not aware of others).
[16:32] <davej_> dlobo: I'm still concerned that problem (2) remains a complete mystery.
[16:32] <davej_> And problem (2) is what we've encountered.
[16:33] <dlobo> davej_: depends on what the smart group was
[16:33] <dlobo> if the smart group was something like
[16:33] <dlobo> all contacts from bristol
[16:34] <dlobo> basically civimail would send a mailing to ALL the contacts in the db across multiple sites from the cronjob
[16:34] <davej_> Please see http://forum.civicrm.org/index.php/topic,13351.0.html - "If I log into the site as uid 1 with full multi-org privileges and view group members of the two groups to which the mailing was sent, they have 361 and 302 members, just as they do when viewed by a normal user. Exactly the same if I log into the L1 (grandparent) site as uid 1. So I don't see how a permissioning problem explains the number of recipients being 2544."
[16:35] <dlobo> we have the mailing in the DB right?
[16:35] <dlobo> can we get a copu of it
[16:36] <dlobo> so within the framework, we use a lot of smart group caching
[16:36] <davej_> I have a db dump from a few hours after the mailout
[16:36] <dlobo> civimail code basically recomputes as admin user and does not use the cache
[16:36] <dlobo> do u have a test setup with that db dump
[16:36] <dlobo> that we can ssh into
[16:37] <dlobo> or if u can mail it to deepak we can investigate
[16:37] <dlobo> and explain 2544
[16:38] <davej_> I"ll get something set up & mail the details.


There was email discussion too, edited highlights:

From:    deepak
Date:    1 May 2010 05:01:40 BST

DS> Filed an issue to fix CiviMail permissioning - http://issues.civicrm.org/jira/browse/CRM-6177.
DS> Patch - http://fisheye2.atlassian.com/changelog/CiviCRM?cs=27333.

DS> With regard to permissioning from cron , the fix from http://issues.civicrm.org/jira/browse/CRM-4542 could be backported to 3.1.
DS> I tested it locally on 3.1 and it works


From:    deepak
Date:    2 May 2010 04:49:25 BST

DJ> The fix for CRM-6177 should ensure that ACLs are applied when sending a mailing using civicrm/mailing/queue&reset=1 DJ> when logged in as a suitable user, is that right?

DS> right.


DJ> And the backported fix for CRM-4542 would fix this for cron. Do you have a patch for this for 3.1?
DS> Right.

DS> From - http://fisheye2.atlassian.com/changelog/CiviCRM?cs=27094, only apply patches to these files :
DS> CRM/Utils/System.php
DS> CRM/Utils/System/Drupal.php
DS> bin/civimail.cronjob.php

DS> Plus apply this one (1 file) - http://fisheye2.atlassian.com/changelog/CiviCRM?cs=27131

DJ> So I think this is where we're up to in terms of the 2 issues I mentioned yesterday on IRC:
DJ> (1) ACL failure
DJ> - should be fixed by the above;
DJ> (2) mailing sent to 2544 recipients when ACL failures cannot explain this
DJ> - remains a mystery. However the fixes for (1) should mean that at worst, a malfunctioning mailing would go to all contacts within a site rather than all contacts across all sites.

DS> Hopefully we can figure (2) out with your db.

[Eileen]

Hi Dave

civicrm_mailing has created_id, which is the other mechanism used to determine which site an entity belongs to: if the creating contact belongs to site A's site parent group then show the entity in site A. I believe this is what's used to filter the list of mailings shown at civicrm/mailing&reset=1 . I expect the civimail cron job could and should do the same.

So, I guess the senders we have been using are members of both the head site group and the branch site group and it seems that because they belong to both both sites are trying to send the mailing - would that be right?

[davej]

Hi,

So, I guess the senders we have been using are members of both the head site group and the branch site group and it seems that because they belong to both both sites are trying to send the mailing - would that be right?

What I meant was: I think you're right in your original description of the problem and I was suggesting a way of fixing it: we don't have domain_id to tell us which site a mailing belongs to but we do have created_id, which can serve the same purpose.

I've done some testing, which confirms that running cron on one site, A, in a multi-site installation causes all scheduled mailings across all sites to be run. Which is doubly bad because they're all run as a user on site A.

Deepak has just fixed some ACL problems with CiviMail, as described above. Before these fixes, ACLs were not applied at all when sending mailings - so e.g. if a mailing is sending to a smart group, it would be sent to contacts across all sites who match the smart group criteria. The first part of the fix addresses this for mails sent using /civicrm/mailing/queue&reset=1 ; the second part fixes it for cron.

That's great, if a site A mailing is sent by a cron job calling site A. But if a site B cron job catches it first, then the mailing will be sent with the permissions of the site B user specified in the cron job.

So we need to restrict civimail.cronjob.php so that it only runs mailings belonging to the site that it's called on. I think it's possible to do this using created_id in civicrm_mailing.

(Edit) The same applies when sending a mailing using /civicrm/mailing/queue&reset=1 : calling this URL on site A causes all scheduled mailings across all sites to be run. This also needs fixing. Both this route and the cron script call CRM_Mailing_BAO_Job::runJobs() so I think it's a matter of modifying $query there to return only mailings belonging to the site.

(Edit) I've come up with a patch for this against 3.1.4, which I've attached to http://issues.civicrm.org/jira/browse/CRM-6177 (civimail-restrict-mailing-jobs). Eileen, would you be able to test this on your setup? You'll need to have Deepak's patches in place too to make the ACL work correctly, as described towards the end of http://forum.civicrm.org/index.php/topic,13351.msg58204.html#msg58204 .

Dave J

[Eileen]

Hi,

I'll try the patch but - would it be worth having a domain_id field on ALL tables so that as multisite functionality gets developed the standard for it is to use the domain id?

[davej]

I'll try the patch

Deepak has come up with a different patch; mine works (based on who created the mailing) but apparently the view mailings screen uses a different algorithm to decide which mailings to show, based on whether they're sent to groups that the user is allowed to view. Makes sense to use the same algorithm for both.

Deepak's patch:

Code: [Select]

Index: CRM/Mailing/BAO/Job.php
===================================================================
--- CRM/Mailing/BAO/Job.php (revision 27349)
+++ CRM/Mailing/BAO/Job.php (working copy)
@@ -59,7 +59,7 @@
    public static function runJobs($testParams = null) {
        $job =& new CRM_Mailing_BAO_Job();

-        $mailing =& new CRM_Mailing_DAO_Mailing();
+        $mailing =& new CRM_Mailing_BAO_Mailing();

        $config =& CRM_Core_Config::singleton();
        $jobTable     = CRM_Mailing_DAO_Job::getTableName();
@@ -73,7 +73,8 @@
            $job->query($query);
        } else {
            $currentTime = date( 'YmdHis' );
-           
+            $mailingACL  = CRM_Mailing_BAO_Mailing::mailingACL( 'm' );
+
            /* FIXME: we might want to go to a progress table.. */
            $query = "
SELECT   j.*
@@ -86,6 +87,7 @@
   AND       j.status = 'Scheduled' )
    OR     ( j.status = 'Running'
   AND       j.end_date IS null ) )
+   AND   {$mailingACL}
ORDER BY j.scheduled_date,
         j.start_date";

Index: CRM/Mailing/BAO/Mailing.php
===================================================================
--- CRM/Mailing/BAO/Mailing.php (revision 27349)
+++ CRM/Mailing/BAO/Mailing.php (working copy)
@@ -1684,13 +1684,13 @@
        return;
    }

-    static function mailingACL( ) {
+    static function mailingACL( $alias = null ) {
        $mailingACL = " ( 0 ) ";

        $mailingIDs =& self::mailingACLIDs( );
        if ( ! empty( $mailingIDs ) ) {
            $mailingIDs = implode( ',', $mailingIDs );
-            $tableName  = self::getTableName( );
+            $tableName  = !$alias ? self::getTableName( ) : $alias;
            $mailingACL = " $tableName.id IN ( $mailingIDs ) ";
        }
        return $mailingACL;


but - would it be worth having a domain_id field on ALL tables so that as multisite functionality gets developed the standard for it is to use the domain id?

There used to be but it was removed in 2.1.

Dave J

[Donald Lobo]

I'll try the patch but - would it be worth having a domain_id field on ALL tables so that as multisite functionality gets developed the standard for it is to use the domain id?

having the domain_id field in ALL tables does not allow you to model the hierarchy nicely without major changes to the code (i.e. L1, L2 etc)

i do think the current model is significantly better and allowing ACL's to decide how to partition the data is way more flexible and can handle multiple configurations

lobo

[Eileen]

Hi,

Is testing these patches holding up 3.1.5? I can see

http://issues.civicrm.org/jira/browse/CRM-6177 is still open for 3.1.5

Am I right in thinking this is a pre-requisite for it:

http://issues.civicrm.org/jira/browse/CRM-4542

And that the patch pasted '2 posts ago' by davej supercedes the one in 6177.

[yogibear]

Hi all,

Have setup a second org and now want to do a mailout from that second org.

Should I be concerened about segregation issues and all contacts from both orgs getting the mailout or have I misunderstood the above?

Thanks.

[Eileen]

I suggest you test it. The main issue we have been seeing is that the domain in the click throughs is often 'wrong'. If someone in an L2 groups sends from the main site. then the embedded URLS will reflect the L2 group they are in.