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

4 files changed, 49 insertions, 12 deletions

diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index d7be12a1a..3919c0ec6 100644
--- a/Bugzilla/BugMail.pm
+++ b/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));
         }
     }
     
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 0b612cbba..8e245d0b6 100644
--- a/Bugzilla/Constants.pm
+++ b/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;
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 960ff336d..352147331 100644
--- a/Bugzilla/Search.pm
+++ b/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 " .
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 3ce346812..4fb41d852 100644
--- a/Bugzilla/User.pm
+++ b/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.