Closed Bug 369648 Opened 18 years ago Closed 15 years ago

syncLDAP.pl populates database with ldap users without default email preferences

Categories

(Bugzilla :: Email Notifications, defect)

2.22.2
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: mozilla-bugs, Unassigned)

References

Details

(Keywords: regression)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1) Gecko/20070117 BonEcho/2.0.0.1
Build Identifier: Bugzilla 2.22.2 (cvs tag)

As the subject shows, contrib/syncLDAP.pl populates the profiles table with users.  What it doesn't do is populate the email_setting table with the email preferences for these users.

Which leaves it up to the user to configure the preferences.

It looks like line 272 is meant to cover that issue but my guess is that field was moved out to it's own table (email_setting) and uses a different format.

Reproducible: Always

Steps to Reproduce:
1. Configure Bugzilla to authenticate against LDAP
2. Run syncLDAP.pl
3. Attempt to add userB@domain.tld (which didn't exist prior to this run) to the CC field of a bug OR let them file a bug.  userA@domain.tld is the default assignee or QA contact.
Actual Results:  
Changes submitted for bug 24
Email sent to: userA@domain.tld
Excluding: userB@domain.tld,

Expected Results:  
Changes submitted for bug 24
Email sent to: userA@domain.tld, userB@domain.tld

The email_setting table is populated with values for each user created before the import (either db users created or those that logged in via LDAP) and after the import but not those created BY the import (syncLDAP.pl)

If you know of a quick way to replicate the settings of the first user across all of the other accounts PLEASE share!  Email me directly or post here if that is allowed.  Thanks for your time.
(continuation of "Steps to Reproduce")

Step #4:

Pick random LDAP (checked several to make sure) account imported via contrib/syncLDAP.pl and check their group permissions:

[] [] canconfirm:  Can confirm a bug.
[] [] editbugs:  Can edit all bug fields.

Step #5:

Run checksetup.pl


Step #6:

Look at those same users:

[]* []* canconfirm:  Can confirm a bug.
[]* []* editbugs:  Can edit all bug fields.

The background for those lines is gray now like DB accounts (hand created accounts via admin interface).

Those steps were executed before I filed this bug and to be absolutely clear:

The problem remains.

bug 108870, comment 68 sounds like a different issue but sounded similar enough to mention.

Also my installation of Bugzilla passed sanitycheck.cgi without issues.
Attached file workaround bash script
Since our installation of Bugzilla is pretty new we're able to wipe the email_setting table (manually) and start over with the intended defaults.

This bash script illustrates a (very crude) method for working around syncLDAP.pl not populating the email_setting table.
I forgot to clarify that after importing the output file (fix_email_setting.mysql) actions which normally would trigger an email do so now for the users imported via the syncLDAP.pl script.

After all is said and done, is it the job of checksetup.pl to detect discrepancies like this or another "sanity" script?  I know in this case it's the fault of syncLDAP.pl for importing them without any preferences at all.

Perhaps have it by default add all new users with default email preferences and add a command-line option to leave imported user email preferences blank.
I can confirm this.  Furthermore, I had to actually modify the script to work at all after I upgraded a migrated database from a previous version of bugzilla.  I've made patches to fix that but the settings still remain empty.  Also modified it to work in paged mode for servers that set limits (like Active Directory).  Oh yeah, and a fix for a quoting problem that made it die if your name had an apostrophe in it.

I'll see if we can get the settings in the right table, we're also trying to fix the issue in 337947 and then we'll put in a patch for all of it.
Created a patch the does a few things
- stops using SendSQL and uses the dbh function handles instead
- updates the @$value references see bug337947
- uses the insert_new_user function from Bugzilla::User to get the default settings
I applied the patch (made backup of original file) and it created new users (as expected).

However:

* It did not add entries to the email_setting table.
* The results mentioned in comment 1 still apply.

Before running checksetup.pl there isn't an asterisk (or star) beside the boxes and the background isn't gray.  After words there is.
Attachment #255778 - Flags: review?
(In reply to comment #7)
> I applied the patch (made backup of original file) and it created new users (as
> expected).
> 

- Did you drop the database and start from scratch on the syncLDAP?
- Did you delete all the users, email_setting, and user_group_map tables?

Not really sure what to try next, since the insert_new_user function should handle this just fine.
I didn't drop the database, but new users were added to the directory server recently and running syncLDAP found the users and created them as user ids 247 through 251.

Those users were the ones whose settings were not created properly.

So I used my (crude) bash script to create an output file of MySQL commands to update the email_setting table with the proper entries.

Regarding the user_group_map table(s) I can't say I had to modify anything there but did not check.
In Bugzilla/User.pm on 2.22.1 inside the insert_new_user function there is the following:

    foreach my $rel (RELATIONSHIPS) {
        foreach my $event (POS_EVENTS, NEG_EVENTS) {
            # These "exceptions" define the default email preferences.
            # 
            # We enable mail unless the change was made by the user, or it's
            # just a CC list addition and the user is not the reporter.
            next if ($event == EVT_CHANGED_BY_ME);
            next if (($event == EVT_CC) && ($rel != REL_REPORTER));

            $dbh->do("INSERT INTO email_setting " . 
                     "(user_id, relationship, event) " . 
                     "VALUES ($userid, $rel, $event)");
        }        
    }

    foreach my $event (GLOBAL_EVENTS) {
        $dbh->do("INSERT INTO email_setting " . 
                 "(user_id, relationship, event) " . 
                 "VALUES ($userid, " . REL_ANY . ", $event)");
    }

which sets up the email_setting table with the defaults your bash script is doing (at least on my setup). So, the conversion of the patch relies on this code to do it. I don't think syncLDAP.pl should have any other changes if the User.pm code is up to date.
Ah, I see what you're saying now.

If the problem persists, it isn't the fault of syncLDAP.pl any longer but instead somewhere else?

Does the patched syncLDAP.pl create the email settings on your 2.22.1 setup?
(In reply to comment #11)
> Does the patched syncLDAP.pl create the email settings on your 2.22.1 setup?
> 

Yes. In both scenarios of a fresh database and an incremental add of new users to the LDAP servers.
Comment on attachment 255778 [details] [diff] [review]
update to use some of the later functions in Bugzilla

Not relevant as both SendSQL to dbh and raw SQL to insert_new_user function conversions have been done in bug 303709. Value reference changes should be kept and fixed in bug 337947.
Attachment #255778 - Flags: review? → review-
This is a regression from bug 73665 and was already fixed for v3.0 by bug 303709. We might want to fix the insert_new_user part for 2.22 and possibly for 2.20 branches in this bug. I don't think we should do the SendSQL conversion for either branch.
Status: UNCONFIRMED → NEW
Depends on: emailprefs
Ever confirmed: true
Keywords: regression
OS: Linux → All
Hardware: PC → All
Version: unspecified → 2.22.2
This problem doesn't exist in Bugzilla 3.0 and newer thanks to bug 303709, as wicked said in comment 14. Bugzilla 2.22 is no longer supported, so we won't fix it there.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: