Bug 285695: [PostgreSQL] Username checks for login, etc. need to be case insensitive

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=justdave

7 files changed, 67 insertions, 24 deletions

diff --git a/Bugzilla/Auth/Login/WWW/Env.pm b/Bugzilla/Auth/Login/WWW/Env.pm
index 2f29d570f..985393206 100644
--- a/Bugzilla/Auth/Login/WWW/Env.pm
+++ b/Bugzilla/Auth/Login/WWW/Env.pm
@@ -84,7 +84,8 @@ sub login {
             # 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 login_name=?");
+                                 "FROM profiles WHERE " .
+                                 $dbh->sql_istrcmp('login_name', '?'));
             $sth->execute($env_email);
 
             $sth->execute();
diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm
index 1d5c6850c..4a45e81e7 100644
--- a/Bugzilla/Auth/Verify/DB.pm
+++ b/Bugzilla/Auth/Verify/DB.pm
@@ -34,6 +34,10 @@ use strict;
 use Bugzilla::Config;
 use Bugzilla::Constants;
 use Bugzilla::Util;
+# Because of the screwy way that Auth works, it thinks
+# that we're redefining subroutines if we "use" anything
+# that "uses" Bugzilla::Auth.
+require Bugzilla::User;
 
 my $edit_options = {
     'new' => 1,
@@ -52,11 +56,8 @@ sub authenticate {
 
     return (AUTH_NODATA) unless defined $username && defined $passwd;
 
-    # We're just testing against the db: any value is ok
-    trick_taint($username);
-
-    my $userid = $class->get_id_from_username($username);
-    return (AUTH_LOGINFAILED) unless defined $userid;
+    my $userid = Bugzilla::User::login_to_id($username);
+    return (AUTH_LOGINFAILED) unless $userid;
 
     return (AUTH_LOGINFAILED, $userid) 
         unless $class->check_password($userid, $passwd);
@@ -74,15 +75,6 @@ sub authenticate {
     return (AUTH_OK, $userid);
 }
 
-sub get_id_from_username {
-    my ($class, $username) = @_;
-    my $dbh = Bugzilla->dbh;
-    my $sth = $dbh->prepare_cached("SELECT userid FROM profiles " .
-                                   "WHERE login_name=?");
-    my ($userid) = $dbh->selectrow_array($sth, undef, $username);
-    return $userid;
-}
-
 sub get_disabled {
     my ($class, $userid) = @_;
     my $dbh = Bugzilla->dbh;
diff --git a/Bugzilla/Auth/Verify/LDAP.pm b/Bugzilla/Auth/Verify/LDAP.pm
index 551a70f45..ee58f9d7e 100644
--- a/Bugzilla/Auth/Verify/LDAP.pm
+++ b/Bugzilla/Auth/Verify/LDAP.pm
@@ -137,7 +137,8 @@ sub authenticate {
     my $dbh = Bugzilla->dbh;
     my $sth = $dbh->prepare_cached("SELECT userid, disabledtext " .
                                    "FROM profiles " .
-                                   "WHERE login_name=?");
+                                   "WHERE " .
+                                   $dbh->sql_istrcmp('login_name', '?'));
     my ($userid, $disabledtext) =
       $dbh->selectrow_array($sth,
                             undef,
diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index e11d52592..76e090d6c 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -218,6 +218,19 @@ sub import {
     $Exporter::ExportLevel-- if $is_exporter;
 }
 
+sub sql_istrcmp {
+    my ($self, $left, $right, $op) = @_;
+    $op ||= "=";
+
+    return $self->sql_istring($left) . " $op " . $self->sql_istring($right);
+}
+
+sub sql_istring {
+    my ($self, $string) = @_;
+
+    return "LOWER($string)";
+}
+
 sub sql_position {
     my ($self, $fragment, $text) = @_;
 
@@ -1153,6 +1166,33 @@ formatted SQL command have prefix C<sql_>. All other methods have prefix C<bz_>.
               $text = text to search for (scalar)
  Returns:     formatted SQL for for full text search
 
+=item C<sql_istrcmp>
+
+ Description: Returns SQL for a case-insensitive string comparison.
+ Params:      $left - What should be on the left-hand-side of the
+                      operation.
+              $right - What should be on the right-hand-side of the
+                       operation.
+              $op (optional) - What the operation is. Should be a 
+                  valid ANSI SQL comparison operator, like "=", "<", 
+                  "LIKE", etc. Defaults to "=" if not specified.
+ Returns:     A SQL statement that will run the comparison in 
+              a case-insensitive fashion.
+ Note:        Uses sql_istring, so it has the same performance concerns.
+              Try to avoid using this function unless absolutely necessary.
+              Subclass Implementors: Override sql_istring instead of this
+              function, most of the time (this function uses sql_istring).
+
+=item C<sql_istring>
+
+ Description: Returns SQL syntax "preparing" a string or text column for
+              case-insensitive comparison. 
+ Params:      $string - string to convert (scalar)
+ Returns:     formatted SQL making the string case insensitive
+ Note:        The default implementation simply calls LOWER on the parameter.
+              If this is used to search on a text column with index, the index
+              will not be usually used unless it was created as LOWER(column).
+
 =item C<bz_lock_tables>
 
  Description: Performs a table lock operation on specified tables.
diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm
index 3472a351d..983cb3b06 100644
--- a/Bugzilla/DB/Mysql.pm
+++ b/Bugzilla/DB/Mysql.pm
@@ -111,6 +111,12 @@ sub sql_fulltext_search {
     return "MATCH($column) AGAINST($text)";
 }
 
+sub sql_istring {
+    my ($self, $string) = @_;
+    
+    return $string;
+}
+
 sub sql_to_days {
     my ($self, $date) = @_;
 
diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm
index 2262e3e73..fe72915a3 100644
--- a/Bugzilla/Token.pm
+++ b/Bugzilla/Token.pm
@@ -104,7 +104,7 @@ sub IssuePasswordToken {
                     AND tokens.tokentype = 'password'
                     AND tokens.issuedate > NOW() - " .
                     $dbh->sql_interval('10 MINUTE') . "
-                    WHERE login_name = $quotedloginname");
+                 WHERE " . $dbh->sql_istrcmp('login_name', $quotedloginname));
     my ($userid, $toosoon) = &::FetchSQLData();
 
     if ($toosoon) {
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index ff3d38721..0c2de0f4c 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -84,7 +84,8 @@ sub new {
 # in the id its already had to validate (or the User.pm object, of course)
 sub new_from_login {
     my $invocant = shift;
-    return $invocant->_create("login_name=?", @_);
+    my $dbh = Bugzilla->dbh;
+    return $invocant->_create($dbh->sql_istrcmp('login_name', '?'), @_);
 }
 
 # Internal helper for the above |new| methods
@@ -631,14 +632,15 @@ sub match {
 
         # Build the query.
         my $sqlstr = &::SqlQuote($wildstr);
-        my $query  = "SELECT DISTINCT userid, realname, login_name, " .

+        my $query  = "SELECT DISTINCT userid, realname, login_name, " .
                      "LENGTH(login_name) AS namelength " .
                      "FROM profiles ";
         if (&::Param('usevisibilitygroups')) {
             $query .= ", user_group_map ";
         }
-        $query    .= "WHERE (login_name LIKE $sqlstr " .
-                     "OR realname LIKE $sqlstr) ";
+        $query .= "WHERE ("  
+            . $dbh->sql_istrcmp('login_name', $sqlstr, "LIKE") . " OR " .
+              $dbh->sql_istrcmp('realname', $sqlstr, "LIKE") . ") ";
         if (&::Param('usevisibilitygroups')) {
             $query .= "AND user_group_map.user_id = userid " .
                       "AND isbless = 0 " .
@@ -664,7 +666,7 @@ sub match {
         my $sqlstr = &::SqlQuote($str);
         my $query  = "SELECT userid, realname, login_name " .
                      "FROM profiles " .
-                     "WHERE login_name = $sqlstr ";
+                     "WHERE " . $dbh->sql_istrcmp('login_name', $sqlstr);
         # Exact matches don't care if a user is disabled.
 
         &::PushGlobalSQLState();
@@ -1213,8 +1215,9 @@ sub login_to_id ($) {
     my $dbh = Bugzilla->dbh;
     # $login will only be used by the following SELECT statement, so it's safe.
     trick_taint($login);
-    my $user_id = $dbh->selectrow_array(
-        "SELECT userid FROM profiles WHERE login_name = ?", undef, $login);
+    my $user_id = $dbh->selectrow_array("SELECT userid FROM profiles WHERE " .
+                                        $dbh->sql_istrcmp('login_name', '?'),
+                                        undef, $login);
     if ($user_id) {
         return $user_id;
     } else {