diff options
| -rw-r--r-- | Bugzilla/BugMail.pm | 6 | ||||
| -rw-r--r-- | Bugzilla/Constants.pm | 12 | ||||
| -rw-r--r-- | Bugzilla/Search.pm | 10 | ||||
| -rw-r--r-- | Bugzilla/User.pm | 33 | ||||
| -rwxr-xr-x | editusers.cgi | 4 | ||||
| -rw-r--r-- | globals.pl | 26 | ||||
| -rwxr-xr-x | post_bug.cgi | 6 | ||||
| -rwxr-xr-x | process_bug.cgi | 8 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 8 | ||||
| -rwxr-xr-x | token.cgi | 4 | ||||
| -rwxr-xr-x | userprefs.cgi | 4 | ||||
| -rwxr-xr-x | votes.cgi | 7 |
12 files changed, 70 insertions, 58 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. diff --git a/editusers.cgi b/editusers.cgi index c5e67e28b..df31c8a4f 100755 --- a/editusers.cgi +++ b/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)); diff --git a/globals.pl b/globals.pl index ac95974d2..ad6cfd761 100644 --- a/globals.pl +++ b/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(); diff --git a/post_bug.cgi b/post_bug.cgi index 14763151a..273117c6b 100755 --- a/post_bug.cgi +++ b/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; } diff --git a/process_bug.cgi b/process_bug.cgi index 8dfa4018d..9ef459bec 100755 --- a/process_bug.cgi +++ b/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}; diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index db3bb682f..1af63b179 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/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" %] @@ -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')); } ################################################################################ diff --git a/userprefs.cgi b/userprefs.cgi index 3dc68121e..7eb78af42 100755 --- a/userprefs.cgi +++ b/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]); @@ -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', |