Bug 327355: Email preferences are not set correctly when the user account is created by Env.pm - Patch by Frédéric Buclin <LpSolit@gmail.com> r=joel a=justdave

2 files changed, 66 insertions, 102 deletions

diff --git a/Bugzilla/Auth/Login/WWW/Env.pm b/Bugzilla/Auth/Login/WWW/Env.pm
index 64487884c..f437bf06f 100644
--- a/Bugzilla/Auth/Login/WWW/Env.pm
+++ b/Bugzilla/Auth/Login/WWW/Env.pm
@@ -26,135 +26,99 @@ use strict;
 use Bugzilla::Config;
 use Bugzilla::Error;
 use Bugzilla::Util;
+use Bugzilla::User;
 
 sub login {
     my ($class, $type) = @_;
+    my $dbh = Bugzilla->dbh;
 
     # XXX This does not currently work correctly with Param('requirelogin').
     #     Bug 253636 will hopefully see that param's needs taken care of in a
     #     parent module, but for the time being, this module does not honor
     #     the param in the way that CGI.pm does.
 
-    my $matched_userid    = '';
-    my $matched_extern_id = '';
-    my $disabledtext      = '';
-    my $new_login_name = 0;
-
-    my $dbh = Bugzilla->dbh;
-    my $sth;
+    my $matched_userid;
+    my $matched_extern_id;
+    my $disabledtext;
 
     # Gather the environment variables
-    my $env_id       = $ENV{Param("auth_env_id")};
-    my $env_email    = $ENV{Param("auth_env_email")};
-    my $env_realname = $ENV{Param("auth_env_realname")};
+    my $env_id       = $ENV{Param("auth_env_id")} || '';
+    my $env_email    = $ENV{Param("auth_env_email")} || '';
+    my $env_realname = $ENV{Param("auth_env_realname")} || '';
 
-    # allow undefined values to work with trick_taint
-    for ($env_id, $env_email, $env_realname) { $_ ||= '' };
     # make sure the email field contains only a valid email address
     my $emailregexp = Param("emailregexp");
     if ($env_email =~ /($emailregexp)/) {
         $env_email = $1;
     }
     else {
-        return undef;
+        $env_email = '';
     }
+
+    return undef unless $env_email;
+
     # untaint the remaining values
     trick_taint($env_id);
     trick_taint($env_realname);
 
-    if ($env_id || $env_email) {
-        # Look in the DB for the extern_id
-        if ($env_id) {
-
-            # Not having the email address defined but having an ID isn't
-            # allowed.
-            return undef unless $env_email;
-
-            $sth = $dbh->prepare("SELECT userid, disabledtext " .
-                                 "FROM profiles WHERE extern_id=?");
-            $sth->execute($env_id);
-            my $fetched = $sth->fetch;
-            if ($fetched) {
-                $matched_userid = $fetched->[0];
-                $disabledtext   = $fetched->[1];
-            }
-        }
+    # Look in the DB for the extern_id
+    if ($env_id) {
+        ($matched_userid, $disabledtext) =
+            $dbh->selectrow_array('SELECT userid, disabledtext
+                                   FROM profiles WHERE extern_id = ?',
+                                   undef, $env_id);
+    }
 
-        unless ($matched_userid) {
-            # There was either no match for the external ID given, or one was
-            # not present.
-            #
-            # Check to see if the email address is in there and has no
-            # external id assigned.  We test for both the login name (which we
-            # also sent), and the id, so that we have a way of telling that we
-            # got something instead of a bunch of NULLs
-            $sth = $dbh->prepare("SELECT extern_id, userid, disabledtext " .
-                                 "FROM profiles WHERE " .
-                                 $dbh->sql_istrcmp('login_name', '?'));
-            $sth->execute($env_email);
-
-            $sth->execute();
-            my $fetched = $sth->fetch();
-            if ($fetched) {
-                ($matched_extern_id, $matched_userid, $disabledtext) = @{$fetched};
+    unless ($matched_userid) {
+        # There was either no match for the external ID given, or one was
+        # not present.
+        #
+        # Check to see if the email address is in there and has no
+        # external id assigned.  We test for both the login name (which we
+        # also sent), and the id, so that we have a way of telling that we
+        # got something instead of a bunch of NULLs
+        ($matched_extern_id, $matched_userid, $disabledtext) =
+            $dbh->selectrow_array('SELECT extern_id, userid, disabledtext
+                                   FROM profiles WHERE ' .
+                                   $dbh->sql_istrcmp('login_name', '?'),
+                                   undef, $env_email);
+
+        if ($matched_userid) {
+            if ($matched_extern_id) {
+                # someone with a different external ID has that address!
+                ThrowUserError("extern_id_conflict");
             }
-            if ($matched_userid) {
-                if ($matched_extern_id) {
-                    # someone with a different external ID has that address!
-                    ThrowUserError("extern_id_conflict");
-                }
-                else
-                {
-                    # someone with no external ID used that address, time to
-                    # add the ID!
-                    $sth = $dbh->prepare("UPDATE profiles " .
-                                         "SET extern_id=? WHERE userid=?");
-                    $sth->execute($env_id, $matched_userid);
-                }
-            }
-            else
-            {
-                # Need to create a new user with that email address.  Note
-                # that cryptpassword has been filled in with '*', since the
-                # user has no DB password.
-                $sth = $dbh->prepare("INSERT INTO profiles ( " .
-                                     "login_name, cryptpassword, " .
-                                     "realname, disabledtext " .
-                                     ") VALUES ( ?, ?, ?, '' )");
-                $sth->execute($env_email, '*', $env_realname);
-                $matched_userid = $dbh->bz_last_key('profiles', 'userid');
-                $new_login_name = $matched_userid;
+            else {
+                # someone with no external ID used that address, time to
+                # add the ID!
+                $dbh->do('UPDATE profiles SET extern_id = ? WHERE userid = ?',
+                         undef,($env_id, $matched_userid));
             }
         }
-    }
-
-    # now that we hopefully have a username, we need to see if the data
-    # has to be updated
-    if ($matched_userid) {
-        $sth = $dbh->prepare("SELECT login_name, realname " .
-                             "FROM profiles " .
-                             "WHERE userid=?");
-        $sth->execute($matched_userid);
-        my $fetched = $sth->fetch;
-        my $username = $fetched->[0];
-        my $this_realname = $fetched->[1];
-        if ( ($username ne $env_email) ||
-             ($this_realname ne $env_realname) ) {
-
-            $sth = $dbh->prepare("UPDATE profiles " .
-                                 "SET login_name=?, " .
-                                 "realname=? " .
-                                 "WHERE userid=?");
-            $sth->execute($env_email,
-                          ($env_realname || $this_realname),
-                          $matched_userid);
-            $sth->execute;
-            $new_login_name = $matched_userid;
+        else {
+            # Need to create a new user with that email address.  Note
+            # that cryptpassword has been filled in with '*', since the
+            # user has no DB password.
+            insert_new_user($env_email, $env_realname, '*');
+            my $new_user = Bugzilla::User->new_from_login($env_email);
+            $matched_userid = $new_user->id;
         }
     }
 
-    # If the login name may be new, make sure the regexp groups are current
-    if ($new_login_name) {
+    # now that we hopefully have a username, we need to see if the data
+    # has to be updated. If we just created this account, then the data
+    # is already up to date.
+    my ($username, $this_realname) =
+        $dbh->selectrow_array('SELECT login_name, realname
+                               FROM profiles WHERE userid = ?',
+                               undef, $matched_userid);
+
+    if (($username ne $env_email) || ($this_realname ne $env_realname)) {
+        $dbh->do('UPDATE profiles SET login_name = ?, realname = ?
+                  WHERE userid = ?', undef,
+                  ($env_email, ($env_realname || $this_realname), $matched_userid));
+
+        # If the login name may be new, make sure the regexp groups are current
         my $userprofile = new Bugzilla::User($matched_userid);
         $userprofile->derive_regexp_groups;
     }
@@ -166,7 +130,6 @@ sub login {
     }
 
     return $matched_userid;
-
 }
 
 # This auth style does not allow the user to log out.
@@ -191,4 +154,3 @@ necessary.
 =head1 SEE ALSO
 
 L<Bugzilla::Auth>
-
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 19c45cbed..7288ab30e 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -1304,8 +1304,10 @@ sub insert_new_user {
     $disabledtext ||= '';
 
     # If not specified, generate a new random password for the user.
+    # If the password is '*', do not encrypt it; we are creating a user
+    # based on the ENV auth method.
     $password ||= generate_random_password();
-    my $cryptpassword = bz_crypt($password);
+    my $cryptpassword = ($password ne '*') ? bz_crypt($password) : $password;
 
     # XXX - These should be moved into is_available_username or validate_email_syntax
     #       At the least, they shouldn't be here. They're safe for now, though.