Bug 280994 : Move ValidateNewUser out of globals.pl

Patch by Max Kanat-Alexander <mkanat@kerio.com>  r=vladd  a=justdave

7 files changed, 55 insertions, 40 deletions

diff --git a/Bugzilla/Auth/Verify/LDAP.pm b/Bugzilla/Auth/Verify/LDAP.pm
index cda67fb80..551a70f45 100644
--- a/Bugzilla/Auth/Verify/LDAP.pm
+++ b/Bugzilla/Auth/Verify/LDAP.pm
@@ -33,7 +33,7 @@ use strict;
 
 use Bugzilla::Config;
 use Bugzilla::Constants;
-use Bugzilla::User qw(insert_new_user);
+use Bugzilla::User;
 
 use Net::LDAP;
 
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 05ef77e32..8f5f6a762 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -40,7 +40,7 @@ use Bugzilla::Constants;
 use Bugzilla::Auth;
 
 use base qw(Exporter);
-@Bugzilla::User::EXPORT_OK = qw(insert_new_user);
+@Bugzilla::User::EXPORT = qw(insert_new_user is_available_username);
 
 ################################################################################
 # Functions
@@ -958,6 +958,40 @@ sub insert_new_user ($$) {
     return $password;
 }
 
+sub is_available_username ($;$) {
+    my ($username, $old_username) = @_;
+
+    if(&::DBname_to_id($username) != 0) {
+        return 0;
+    }
+
+    my $dbh = Bugzilla->dbh;
+    # $username is safe because it is only used in SELECT placeholders.
+    trick_taint($username);
+    # Reject if the new login is part of an email change which is
+    # still in progress
+    #
+    # substring/locate stuff: bug 165221; this used to use regexes, but that
+    # was unsafe and required weird escaping; using substring to pull out
+    # the new/old email addresses and locate() to find the delimeter (':')
+    # is cleaner/safer
+    my $sth = $dbh->prepare(
+        "SELECT eventdata FROM tokens WHERE tokentype = 'emailold'
+        AND SUBSTRING(eventdata, 1, (LOCATE(':', eventdata) - 1)) = ?
+        OR SUBSTRING(eventdata, (LOCATE(':', eventdata) + 1)) = ?");
+    $sth->execute($username, $username);
+
+    if (my ($eventdata) = $sth->fetchrow_array()) {
+        # Allow thru owner of token
+        if($old_username && ($eventdata eq "$old_username:$username")) {
+            return 1;
+        }
+        return 0;
+    }
+
+    return 1;
+}
+
 1;
 
 __END__
@@ -1183,6 +1217,19 @@ Params: $username (scalar, string) - The login name for the new user.
 
 Returns: The password that we randomly generated for this user, in plain text.
 
+=item C<is_available_username>
+
+Returns a boolean indicating whether or not the supplied username is
+already taken in Bugzilla.
+
+Params: $username (scalar, string) - The full login name of the username 
+            that you are checking.
+        $old_username (scalar, string) - If you are checking an email-change
+            token, insert the "old" username that the user is changing from,
+            here. Then, as long as it's the right user for that token, he 
+            can change his username to $username. (That is, this function
+            will return a boolean true value).
+
 =back
 
 =head1 SEE ALSO
diff --git a/createaccount.cgi b/createaccount.cgi
index 6867ea3c4..60a180623 100755
--- a/createaccount.cgi
+++ b/createaccount.cgi
@@ -30,7 +30,7 @@ use lib qw(.);
 
 require "CGI.pl";
 
-use Bugzilla::User qw(insert_new_user);
+use Bugzilla::User;
 
 # Shut up misguided -w warnings about "used only once":
 use vars qw(
@@ -61,7 +61,7 @@ if (defined($login)) {
     CheckEmailSyntax($login);
     $vars->{'login'} = $login;
     
-    if (!ValidateNewUser($login)) {
+    if (!is_available_username($login)) {
         # Account already exists        
         $template->process("account/exists.html.tmpl", $vars)
           || ThrowTemplateError($template->error());
diff --git a/editusers.cgi b/editusers.cgi
index a1eccd956..8cd53efd0 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -434,7 +434,7 @@ if ($action eq 'new') {
         PutTrailer($localtrailer);
         exit;
     }
-    if (!ValidateNewUser($user)) {
+    if (!is_available_username($user)) {
         print "The user '$user' does already exist. Please press\n";
         print "<b>Back</b> and try again.\n";
         PutTrailer($localtrailer);
diff --git a/globals.pl b/globals.pl
index f4a11e72f..d793a4659 100644
--- a/globals.pl
+++ b/globals.pl
@@ -375,39 +375,6 @@ sub GetVersionTable {
     $::VersionTableLoaded = 1;
 }
 
-# Validates a given username as a new username
-# returns 1 if valid, 0 if invalid
-sub ValidateNewUser {
-    my ($username, $old_username) = @_;
-
-    if(DBname_to_id($username) != 0) {
-        return 0;
-    }
-
-    my $sqluname = SqlQuote($username);
-
-    # Reject if the new login is part of an email change which is 
-    # still in progress
-    #
-    # substring/locate stuff: bug 165221; this used to use regexes, but that
-    # was unsafe and required weird escaping; using substring to pull out
-    # the new/old email addresses and locate() to find the delimeter (':')
-    # is cleaner/safer
-    SendSQL("SELECT eventdata FROM tokens WHERE tokentype = 'emailold' 
-     AND SUBSTRING(eventdata, 1, (LOCATE(':', eventdata) - 1)) = $sqluname 
-     OR SUBSTRING(eventdata, (LOCATE(':', eventdata) + 1)) = $sqluname");
-
-    if (my ($eventdata) = FetchSQLData()) {
-        # Allow thru owner of token
-        if($old_username && ($eventdata eq "$old_username:$username")) {
-            return 1;
-        }
-        return 0;
-    }
-
-    return 1;
-}
-
 sub GenerateRandomPassword {
     my $size = (shift or 10); # default to 10 chars if nothing specified
     return join("", map{ ('0'..'9','a'..'z','A'..'Z')[rand 62] } (1..$size));
diff --git a/token.cgi b/token.cgi
index 8b4636a79..bf810834f 100755
--- a/token.cgi
+++ b/token.cgi
@@ -243,7 +243,7 @@ sub changeEmail {
     }
     # The new email address should be available as this was 
     # confirmed initially so cancel token if it is not still available
-    if (! ValidateNewUser($new_email,$old_email)) {
+    if (! is_available_username($new_email,$old_email)) {
         $vars->{'email'} = $new_email; # Needed for Bugzilla::Token::Cancel's mail
         Bugzilla::Token::Cancel($::token,"account_exists");
         ThrowUserError("account_exists", { email => $new_email } );
diff --git a/userprefs.cgi b/userprefs.cgi
index 6950fea88..f62f02500 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -29,6 +29,7 @@ use Bugzilla;
 use Bugzilla::Constants;
 use Bugzilla::Search;
 use Bugzilla::Auth;
+use Bugzilla::User;
 
 require "CGI.pl";
 
@@ -122,7 +123,7 @@ sub SaveAccount {
             # Before changing an email address, confirm one does not exist.
             CheckEmailSyntax($new_login_name);
             trick_taint($new_login_name);
-            ValidateNewUser($new_login_name)
+            is_available_username($new_login_name)
               || ThrowUserError("account_exists", {email => $new_login_name});
 
             Bugzilla::Token::IssueEmailChangeToken($userid,$old_login_name,