Bug 332598: Move ValidatePassword() and DBNameToIdAndCheck() from globals.pl into User.pm - Patch by Fr�d�ric Buclin <LpSolit@gmail.com> r=mkanat a=myk

2006-05-07 20:13:49 +00:00 · 2006-05-07 20:13:49 +00:00 · d9d8f6103c
--- a/webtools/bugzilla/Bugzilla/BugMail.pm
+++ b/webtools/bugzilla/Bugzilla/BugMail.pm
@ -178,16 +178,16 @@ sub ProcessOneBug {
    # At this point, we don't care if there are duplicates in these arrays.
    my $changer = $forced->{'changer'};
    if ($forced->{'owner'}) {
-        push (@assignees, &::DBNameToIdAndCheck($forced->{'owner'}));
+        push (@assignees, login_to_id($forced->{'owner'}, THROW_ERROR));
    }
    
    if ($forced->{'qacontact'}) {
-        push (@qa_contacts, &::DBNameToIdAndCheck($forced->{'qacontact'}));
+        push (@qa_contacts, login_to_id($forced->{'qacontact'}, THROW_ERROR));
    }
    
    if ($forced->{'cc'}) {
        foreach my $cc (@{$forced->{'cc'}}) {
-            push(@ccs, &::DBNameToIdAndCheck($cc));
+            push(@ccs, login_to_id($cc, THROW_ERROR));
        }
    }
    
--- a/webtools/bugzilla/Bugzilla/Constants.pm
+++ b/webtools/bugzilla/Bugzilla/Constants.pm
@ -44,6 +44,9 @@ use base qw(Exporter);
    AUTH_LOGINFAILED
    AUTH_DISABLED

+    USER_PASSWORD_MIN_LENGTH
+    USER_PASSWORD_MAX_LENGTH
+
    LOGIN_OPTIONAL
    LOGIN_NORMAL
    LOGIN_REQUIRED
@ -71,6 +74,7 @@ use base qw(Exporter);
    COMMENT_COLS

    UNLOCK_ABORT
+    THROW_ERROR
    
    RELATIONSHIPS
    REL_ASSIGNEE REL_QA REL_REPORTER REL_CC REL_VOTER 
@ -141,6 +145,10 @@ use constant AUTH_ERROR => 2;
 use constant AUTH_LOGINFAILED => 3;
 use constant AUTH_DISABLED => 4;

+# The minimum and maximum lengths a password must have.
+use constant USER_PASSWORD_MIN_LENGTH => 3;
+use constant USER_PASSWORD_MAX_LENGTH => 16;
+
 use constant LOGIN_OPTIONAL => 0;
 use constant LOGIN_NORMAL => 1;
 use constant LOGIN_REQUIRED => 2;
@ -192,6 +200,10 @@ use constant COMMENT_COLS => 80;
 # because of error
 use constant UNLOCK_ABORT => 1;

+# Determine whether a validation routine should return 0 or throw
+# an error when the validation fails.
+use constant THROW_ERROR => 1;
+
 use constant REL_ASSIGNEE           => 0;
 use constant REL_QA                 => 1;
 use constant REL_REPORTER           => 2;
--- a/webtools/bugzilla/Bugzilla/Search.pm
+++ b/webtools/bugzilla/Bugzilla/Search.pm
@ -239,7 +239,7 @@ sub init {
            foreach my $name (split(',', $email)) {
                $name = trim($name);
                if ($name) {
-                    &::DBNameToIdAndCheck($name);
+                    login_to_id($name, THROW_ERROR);
                }
            }
        }
@ -550,7 +550,7 @@ sub init {
             my $table = "longdescs_$chartid";
             push(@supptables, "INNER JOIN longdescs AS $table " .
                               "ON $table.bug_id = bugs.bug_id");
-             my $id = &::DBNameToIdAndCheck($v);
+             my $id = login_to_id($v, THROW_ERROR);
             $term = "$table.who = $id";
         },
         "^long_?desc,changedbefore" => sub {
@ -691,7 +691,7 @@ sub init {
             my $table = "longdescs_$chartid";
             push(@supptables, "INNER JOIN longdescs AS $table " .
                               "ON $table.bug_id = bugs.bug_id");
-             my $id = &::DBNameToIdAndCheck($v);
+             my $id = login_to_id($v, THROW_ERROR);
             $term = "(($table.who = $id";
             $term .= ") AND ($table.work_time <> 0))";
         },
@ -805,7 +805,7 @@ sub init {
             $f =~ m/^attachments\.(.*)$/;
             my $field = $1;
             if ($t eq "changedby") {
-                 $v = &::DBNameToIdAndCheck($v);
+                 $v = login_to_id($v, THROW_ERROR);
                 $q = &::SqlQuote($v);
                 $field = "submitter_id";
                 $t = "equals";
@ -1126,7 +1126,7 @@ sub init {
             if (!$fieldid) {
                 ThrowCodeError("invalid_field_name", {field => $f});
             }
-             my $id = &::DBNameToIdAndCheck($v);
+             my $id = login_to_id($v, THROW_ERROR);
             push(@supptables, "LEFT JOIN bugs_activity AS $table " .
                               "ON $table.bug_id = bugs.bug_id " .
                               "AND $table.fieldid = $fieldid " .
--- a/webtools/bugzilla/Bugzilla/User.pm
+++ b/webtools/bugzilla/Bugzilla/User.pm
@ -48,7 +48,7 @@ use Bugzilla::Classification;

 use base qw(Exporter);
@Bugzilla::User::EXPORT = qw(insert_new_user is_available_username
-    login_to_id
+    login_to_id validate_password
    UserInGroup
    USER_MATCH_MULTIPLE USER_MATCH_FAILED USER_MATCH_SUCCESS
    MATCH_SKIP_CONFIRM
@ -1360,7 +1360,7 @@ sub is_available_username {
 }

 sub login_to_id {
-    my ($login) = (@_);
+    my ($login, $throw_error) = @_;
    my $dbh = Bugzilla->dbh;
    # $login will only be used by the following SELECT statement, so it's safe.
    trick_taint($login);
@ -1369,11 +1369,26 @@ sub login_to_id {
                                        undef, $login);
    if ($user_id) {
        return $user_id;
+    } elsif ($throw_error) {
+        ThrowUserError('invalid_username', { name => $login });
    } else {
        return 0;
    }
 }

+sub validate_password {
+    my ($password, $matchpassword) = @_;
+
+    if (length($password) < USER_PASSWORD_MIN_LENGTH) {
+        ThrowUserError('password_too_short');
+    } elsif (length($password) > USER_PASSWORD_MAX_LENGTH) {
+        ThrowUserError('password_too_long');
+    } elsif ((defined $matchpassword) && ($password ne $matchpassword)) {
+        ThrowUserError('passwords_dont_match');
+    }
+    return 1;
+}
+
 sub UserInGroup {
    return exists Bugzilla->user->groups->{$_[0]} ? 1 : 0;
 }
@ -1774,13 +1789,15 @@ Params: $username (scalar, string) - The full login name of the username
            can change his username to $username. (That is, this function
            will return a boolean true value).

-=item C<login_to_id($login)>
+=item C<login_to_id($login, $throw_error)>

 Takes a login name of a Bugzilla user and changes that into a numeric
 ID for that user. This ID can then be passed to Bugzilla::User::new to
 create a new user.

-If no valid user exists with that login name, then the function will return 0.
+If no valid user exists with that login name, then the function returns 0.
+However, if $throw_error is set, the function will throw a user error
+instead of returning.

 This function can also be used when you want to just find out the userid
 of a user, but you don't want the full weight of Bugzilla::User.
@ -1788,6 +1805,14 @@ of a user, but you don't want the full weight of Bugzilla::User.
 However, consider using a Bugzilla::User object instead of this function
 if you need more information about the user than just their ID.

+=item C<validate_password($passwd1, $passwd2)>
+
+Returns true if a password is valid (i.e. meets Bugzilla's
+requirements for length and content), else returns false.
+
+If a second password is passed in, this function also verifies that
+the two passwords match.
+
 =item C<UserInGroup($groupname)>

 Takes a name of a group, and returns 1 if a user is in the group, 0 otherwise.
--- a/webtools/bugzilla/editusers.cgi
+++ b/webtools/bugzilla/editusers.cgi
@ -209,7 +209,7 @@ if ($action eq 'search') {
      || ThrowUserError('illegal_email_address', {addr => $login});
    is_available_username($login)
      || ThrowUserError('account_exists', {email => $login});
-    ValidatePassword($password);
+    validate_password($password);

    # Login and password are validated now, and realname and disabledtext
    # are allowed to contain anything
@ -296,7 +296,7 @@ if ($action eq 'search') {
        }
        if ($password) {
            # Validate, then trick_taint.
-            ValidatePassword($password) if $password;
+            validate_password($password) if $password;
            trick_taint($password);
            push(@changedFields, 'cryptpassword');
            push(@values, bz_crypt($password));
--- a/webtools/bugzilla/globals.pl
+++ b/webtools/bugzilla/globals.pl
@ -204,22 +204,6 @@ sub AnyDefaultGroups {
    return $::CachedAnyDefaultGroups;
 }

-sub ValidatePassword {
-    # Determines whether or not a password is valid (i.e. meets Bugzilla's
-    # requirements for length and content).    
-    # If a second password is passed in, this function also verifies that
-    # the two passwords match.
-    my ($password, $matchpassword) = @_;
-    
-    if (length($password) < 3) {
-        ThrowUserError("password_too_short");
-    } elsif (length($password) > 16) {
-        ThrowUserError("password_too_long");
-    } elsif ((defined $matchpassword) && ($password ne $matchpassword)) {
-        ThrowUserError("passwords_dont_match");
-    }
-}
-
 sub DBID_to_name {
    my ($id) = (@_);
    return "__UNKNOWN__" if !defined $id;
@ -242,16 +226,6 @@ sub DBID_to_name {
    return $::cachedNameArray{$id};
 }

-sub DBNameToIdAndCheck {
-    my ($name) = (@_);
-    my $result = login_to_id($name);
-    if ($result > 0) {
-        return $result;
-    }
-
-    ThrowUserError("invalid_username", { name => $name });
-}
-
 sub get_product_id {
    my ($prod) = @_;
    PushGlobalSQLState();
--- a/webtools/bugzilla/post_bug.cgi
+++ b/webtools/bugzilla/post_bug.cgi
@ -155,7 +155,7 @@ if (!UserInGroup("editbugs") || $cgi->param('assigned_to') eq "") {
    $cgi->param(-name => 'assigned_to', -value => $initialowner);
 } else {
    $cgi->param(-name => 'assigned_to',
-                -value => DBNameToIdAndCheck(trim($cgi->param('assigned_to'))));
+                -value => login_to_id(trim($cgi->param('assigned_to')), THROW_ERROR));
 }

 my @bug_fields = ("version", "rep_platform",
@ -182,7 +182,7 @@ if (Param("useqacontact")) {
                                                 WHERE id = ?},
                                                undef, $component_id);
    } else {
-        $qa_contact = DBNameToIdAndCheck(trim($cgi->param('qa_contact')));
+        $qa_contact = login_to_id(trim($cgi->param('qa_contact')), THROW_ERROR);
    }

    if ($qa_contact) {
@ -267,7 +267,7 @@ my %ccids;
 if (defined $cgi->param('cc')) {
    foreach my $person ($cgi->param('cc')) {
        next unless $person;
-        my $ccid = DBNameToIdAndCheck($person);
+        my $ccid = login_to_id($person, THROW_ERROR);
        if ($ccid && !$ccids{$ccid}) {
           $ccids{$ccid} = 1;
        }
--- a/webtools/bugzilla/process_bug.cgi
+++ b/webtools/bugzilla/process_bug.cgi
@ -1050,7 +1050,7 @@ if (defined $cgi->param('newcc')
    if ($cc_add) {
        $cc_add =~ s/[\s,]+/ /g; # Change all delimiters to a single space
        foreach my $person ( split(" ", $cc_add) ) {
-            my $pid = DBNameToIdAndCheck($person);
+            my $pid = login_to_id($person, THROW_ERROR);
            $cc_add{$pid} = $person;
        }
    }
@ -1060,7 +1060,7 @@ if (defined $cgi->param('newcc')
    if ($cc_remove) {
        $cc_remove =~ s/[\s,]+/ /g; # Change all delimiters to a single space
        foreach my $person ( split(" ", $cc_remove) ) {
-            my $pid = DBNameToIdAndCheck($person);
+            my $pid = login_to_id($person, THROW_ERROR);
            $cc_remove{$pid} = $person;
        }
    }
@ -1087,7 +1087,7 @@ if (defined $cgi->param('qa_contact')
    my $name = trim($cgi->param('qa_contact'));
    # The QA contact cannot be deleted from show_bug.cgi for a single bug!
    if ($name ne $cgi->param('dontchange')) {
-        $qacontact = DBNameToIdAndCheck($name) if ($name ne "");
+        $qacontact = login_to_id($name, THROW_ERROR) if ($name ne "");
        if ($qacontact && Param("strict_isolation")) {
                $usercache{$qacontact} ||= Bugzilla::User->new($qacontact);
                my $qa_user = $usercache{$qacontact};
@ -1172,7 +1172,7 @@ SWITCH: for ($cgi->param('knob')) {
        DoComma();
        if (defined $cgi->param('assigned_to')
            && trim($cgi->param('assigned_to')) ne "") { 
-            $assignee = DBNameToIdAndCheck(trim($cgi->param('assigned_to')));
+            $assignee = login_to_id(trim($cgi->param('assigned_to')), THROW_ERROR);
            if (Param("strict_isolation")) {
                $usercache{$assignee} ||= Bugzilla::User->new($assignee);
                my $assign_user = $usercache{$assignee};
--- a/webtools/bugzilla/template/en/default/global/user-error.html.tmpl
+++ b/webtools/bugzilla/template/en/default/global/user-error.html.tmpl
@ -1022,13 +1022,13 @@

  [% ELSIF error == "password_too_long" %]
    [% title = "Password Too Long" %]
-    The password is more than 16 characters long. It must be no more than
-    16 characters.
+    The password must be no more than
+    [%+ constants.USER_PASSWORD_MAX_LENGTH FILTER html %] characters long.

  [% ELSIF error == "password_too_short" %]
    [% title = "Password Too Short" %]
-    The password is less than three characters long. It must be at least
-    three characters.
+    The password must be at least
+    [%+ constants.USER_PASSWORD_MIN_LENGTH FILTER html %] characters long.

  [% ELSIF error == "patch_too_large" %]
    [% title = "File Too Large" %]
--- a/webtools/bugzilla/token.cgi
+++ b/webtools/bugzilla/token.cgi
@ -68,7 +68,7 @@ if ($cgi->param('t')) {
  
  # Make sure the token contains only valid characters in the right amount.
  # Validate password will throw an error if token is invalid
-  ValidatePassword($::token);
+  validate_password($::token);
  trick_taint($::token); # Only used in placeholders

  Bugzilla::Token::CleanTokenTable();
@ -128,7 +128,7 @@ if ( $::action eq 'chgpw' ) {
      && defined $cgi->param('matchpassword')
      || ThrowUserError("require_new_password");

-    ValidatePassword($cgi->param('password'), $cgi->param('matchpassword'));
+    validate_password($cgi->param('password'), $cgi->param('matchpassword'));
 }

 ################################################################################
--- a/webtools/bugzilla/userprefs.cgi
+++ b/webtools/bugzilla/userprefs.cgi
@ -96,7 +96,7 @@ sub SaveAccount {
        {
            $cgi->param('new_password1')
              || ThrowUserError("new_password_missing");
-            ValidatePassword($pwd1, $pwd2);
+            validate_password($pwd1, $pwd2);

            if ($cgi->param('Bugzilla_password') ne $pwd1) {
                my $cryptedpassword = bz_crypt($pwd1);
@ -313,7 +313,7 @@ sub SaveEmail {
        my @new_watch_names = split(/[,\s]+/, $cgi->param('watchedusers'));
        my %new_watch_ids;
        foreach my $username (@new_watch_names) {
-            my $watched_userid = DBNameToIdAndCheck(trim($username));
+            my $watched_userid = login_to_id(trim($username), THROW_ERROR);
            $new_watch_ids{$watched_userid} = 1;
        }
        my ($removed, $added) = diff_arrays($old_watch_ids, [keys %new_watch_ids]);
--- a/webtools/bugzilla/votes.cgi
+++ b/webtools/bugzilla/votes.cgi
@ -29,6 +29,7 @@ use lib ".";
 use Bugzilla;
 use Bugzilla::Constants;
 use Bugzilla::Bug;
+use Bugzilla::User;

 require "globals.pl";

@ -117,11 +118,11 @@ sub show_user {

    # If a bug_id is given, and we're editing, we'll add it to the votes list.
    $bug_id ||= "";
-    
+
    my $name = $cgi->param('user') || $user->login;
-    my $who = DBNameToIdAndCheck($name);
+    my $who = login_to_id($name, THROW_ERROR);
    my $userid = $user->id;
-    
+
    my $canedit = (Param('usevotes') && $userid == $who) ? 1 : 0;

    $dbh->bz_lock_tables('bugs READ', 'products READ', 'votes WRITE',