From d9cbb0f0a62bba345ed26ac68364bb441f41d35d Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Fri, 12 May 2006 09:40:56 +0000 Subject: Bug 300410: Bugzilla::Auth needs to be restructured to not require a BEGIN block Patch By Max Kanat-Alexander r=LpSolit, a=myk --- Bugzilla.pm | 15 +- Bugzilla/Auth.pm | 575 ++++++++++++++------- Bugzilla/Auth/Login.pm | 125 +++++ Bugzilla/Auth/Login/CGI.pm | 73 +++ Bugzilla/Auth/Login/Cookie.pm | 83 +++ Bugzilla/Auth/Login/Env.pm | 54 ++ Bugzilla/Auth/Login/Stack.pm | 87 ++++ Bugzilla/Auth/Login/WWW.pm | 111 ---- Bugzilla/Auth/Login/WWW/CGI.pm | 275 ---------- Bugzilla/Auth/Login/WWW/CGI/Cookie.pm | 113 ---- Bugzilla/Auth/Login/WWW/Env.pm | 156 ------ Bugzilla/Auth/Persist/Cookie.pm | 153 ++++++ Bugzilla/Auth/README | 132 ----- Bugzilla/Auth/Verify.pm | 223 ++++++++ Bugzilla/Auth/Verify/DB.pm | 96 +--- Bugzilla/Auth/Verify/LDAP.pm | 248 ++++----- Bugzilla/Auth/Verify/Stack.pm | 81 +++ Bugzilla/Constants.pm | 2 + Bugzilla/User.pm | 46 +- createaccount.cgi | 2 +- query.cgi | 10 +- relogin.cgi | 5 +- show_bug.cgi | 6 +- .../en/default/account/auth/ldap-error.html.tmpl | 52 -- .../en/default/account/auth/login-small.html.tmpl | 2 +- template/en/default/account/auth/login.html.tmpl | 14 +- .../en/default/account/prefs/account.html.tmpl | 29 +- template/en/default/admin/sudo.html.tmpl | 2 +- template/en/default/global/code-error.html.tmpl | 52 +- template/en/default/global/useful-links.html.tmpl | 9 +- template/en/default/global/user-error.html.tmpl | 4 - template/en/default/index.html.tmpl | 6 +- template/en/default/sidebar.xul.tmpl | 2 +- token.cgi | 2 +- userprefs.cgi | 3 +- 35 files changed, 1499 insertions(+), 1349 deletions(-) create mode 100644 Bugzilla/Auth/Login.pm create mode 100644 Bugzilla/Auth/Login/CGI.pm create mode 100644 Bugzilla/Auth/Login/Cookie.pm create mode 100644 Bugzilla/Auth/Login/Env.pm create mode 100644 Bugzilla/Auth/Login/Stack.pm delete mode 100644 Bugzilla/Auth/Login/WWW.pm delete mode 100644 Bugzilla/Auth/Login/WWW/CGI.pm delete mode 100644 Bugzilla/Auth/Login/WWW/CGI/Cookie.pm delete mode 100644 Bugzilla/Auth/Login/WWW/Env.pm create mode 100644 Bugzilla/Auth/Persist/Cookie.pm delete mode 100644 Bugzilla/Auth/README create mode 100644 Bugzilla/Auth/Verify.pm create mode 100644 Bugzilla/Auth/Verify/Stack.pm delete mode 100644 template/en/default/account/auth/ldap-error.html.tmpl diff --git a/Bugzilla.pm b/Bugzilla.pm index eca0ebf63..b3c6ba7ce 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -26,7 +26,7 @@ package Bugzilla; use strict; use Bugzilla::Auth; -use Bugzilla::Auth::Login::WWW; +use Bugzilla::Auth::Persist::Cookie; use Bugzilla::CGI; use Bugzilla::Config; use Bugzilla::Constants; @@ -160,7 +160,13 @@ sub sudo_request { sub login { my ($class, $type) = @_; - my $authenticated_user = Bugzilla::Auth::Login::WWW->login($type); + + my $authorizer = new Bugzilla::Auth(); + $type = LOGIN_REQUIRED if Bugzilla->cgi->param('GoAheadAndLogIn'); + if (!defined $type || $type == LOGIN_NORMAL) { + $type = Param('requirelogin') ? LOGIN_REQUIRED : LOGIN_NORMAL; + } + my $authenticated_user = $authorizer->login($type); # At this point, we now know if a real person is logged in. # We must now check to see if an sudo session is in progress. @@ -200,14 +206,15 @@ sub logout { return unless user->id; $option = LOGOUT_CURRENT unless defined $option; - Bugzilla::Auth::Login::WWW->logout($_user, $option); + Bugzilla::Auth::Persist::Cookie->logout({type => $option}); + Bugzilla->logout_request() unless $option eq LOGOUT_KEEP_CURRENT; } sub logout_user { my ($class, $user) = @_; # When we're logging out another user we leave cookies alone, and # therefore avoid calling Bugzilla->logout() directly. - Bugzilla::Auth::Login::WWW->logout($user, LOGOUT_ALL); + Bugzilla::Auth::Persist::Cookie->logout({user => $user}); } # just a compatibility front-end to logout_user that gets a user by id diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 4ea3d5bd6..d650658f5 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -19,40 +19,181 @@ # # Contributor(s): Bradley Baetz # Erik Stambaugh +# Max Kanat-Alexander package Bugzilla::Auth; use strict; +use fields qw( + _info_getter + _verifier + _persister +); -use Bugzilla::Config; use Bugzilla::Constants; +use Bugzilla::Error; +use Bugzilla::Config; +use Bugzilla::Auth::Login::Stack; +use Bugzilla::Auth::Verify::Stack; +use Bugzilla::Auth::Persist::Cookie; + +use Switch; + +sub new { + my ($class, $params) = @_; + my $self = fields::new($class); + + $params ||= {}; + $params->{Login} ||= Param('user_info_class') . ',Cookie'; + $params->{Verify} ||= Param('user_verify_class'); + + $self->{_info_getter} = new Bugzilla::Auth::Login::Stack($params->{Login}); + $self->{_verifier} = new Bugzilla::Auth::Verify::Stack($params->{Verify}); + # If we ever have any other login persistence methods besides cookies, + # this could become more configurable. + $self->{_persister} = new Bugzilla::Auth::Persist::Cookie(); + + return $self; +} -# The verification method that was successfully used upon login, if any -my $current_verify_class = undef; +sub login { + my ($self, $type) = @_; + my $dbh = Bugzilla->dbh; -# 'inherit' from the main verify method -BEGIN { - for my $verifyclass (split /,\s*/, Param("user_verify_class")) { - if ($verifyclass =~ /^([A-Za-z0-9_\.\-]+)$/) { - $verifyclass = $1; - } else { - die "Badly-named user_verify_class '$verifyclass'"; + # Get login info from the cookie, form, environment variables, etc. + my $login_info = $self->{_info_getter}->get_login_info(); + + if ($login_info->{failure}) { + return $self->_handle_login_result($login_info, $type); + } + + # Now verify his username and password against the DB, LDAP, etc. + if ($self->{_info_getter}->{successful}->requires_verification) { + $login_info = $self->{_verifier}->check_credentials($login_info); + if ($login_info->{failure}) { + return $self->_handle_login_result($login_info, $type); } - require "Bugzilla/Auth/Verify/" . $verifyclass . ".pm"; + $login_info = + $self->{_verifier}->{successful}->create_or_update_user($login_info); + } + else { + $login_info = $self->{_verifier}->create_or_update_user($login_info); + } + + if ($login_info->{failure}) { + return $self->_handle_login_result($login_info, $type); + } + + # Make sure the user isn't disabled. + my $user = $login_info->{user}; + if ($user->disabledtext) { + return $self->_handle_login_result({ failure => AUTH_DISABLED, + user => $user }, $type); } + $user->set_authorizer($self); + + return $self->_handle_login_result($login_info, $type); +} + +sub can_change_password { + my ($self) = @_; + my $verifier = $self->{_verifier}->{successful}; + $verifier ||= $self->{_verifier}; + my $getter = $self->{_info_getter}->{successful}; + $getter = $self->{_info_getter} + if (!$getter || $getter->isa('Bugzilla::Auth::Login::Cookie')); + return $verifier->can_change_password && + $getter->user_can_create_account; +} + +sub can_login { + my ($self) = @_; + return $self->{_info_getter}->can_login; +} + +sub can_logout { + my ($self) = @_; + my $getter = $self->{_info_getter}->{successful}; + # If there's no successful getter, we're not logged in, so of + # course we can't log out! + return 0 unless $getter; + return $getter->can_logout; } -# PRIVATE +sub user_can_create_account { + my ($self) = @_; + my $verifier = $self->{_verifier}->{successful}; + $verifier ||= $self->{_verifier}; + my $getter = $self->{_info_getter}->{successful}; + $getter = $self->{_info_getter} + if (!$getter || $getter->isa('Bugzilla::Auth::Login::Cookie')); + return $verifier->user_can_create_account + && $getter->user_can_create_account; +} -# A number of features, like password change requests, require the DB -# verification method to be on the list. -sub has_db { - for (split (/[\s,]+/, Param("user_verify_class"))) { - if (/^DB$/) { - return 1; +sub can_change_email { + return $_[0]->user_can_create_account; +} + +sub _handle_login_result { + my ($self, $result, $login_type) = @_; + my $dbh = Bugzilla->dbh; + + my $user = $result->{user}; + my $fail_code = $result->{failure}; + + if (!$fail_code) { + if ($self->{_info_getter}->{successful}->requires_persistence) { + $self->{_persister}->persist_login($user); } } - return 0; + else { + switch ($fail_code) { + case AUTH_ERROR { + ThrowCodeError($result->{error}, $result->{details}); + } + case AUTH_NODATA { + if ($login_type == LOGIN_REQUIRED) { + # This seems like as good as time as any to get rid of + # old crufty junk in the logincookies table. Get rid + # of any entry that hasn't been used in a month. + $dbh->do("DELETE FROM logincookies WHERE " . + $dbh->sql_to_days('NOW()') . " - " . + $dbh->sql_to_days('lastused') . " > 30"); + $self->{_info_getter}->fail_nodata($self); + } + # Otherwise, we just return the "default" user. + $user = Bugzilla->user; + } + + # The username/password may be wrong + # Don't let the user know whether the username exists or whether + # the password was just wrong. (This makes it harder for a cracker + # to find account names by brute force) + case [AUTH_LOGINFAILED, AUTH_NO_SUCH_USER] { + ThrowUserError("invalid_username_or_password"); + } + + # The account may be disabled + case AUTH_DISABLED { + $self->{_persister}->logout(); + # XXX This is NOT a good way to do this, architecturally. + $self->{_persister}->clear_browser_cookies(); + # and throw a user error + ThrowUserError("account_disabled", + {'disabled_reason' => $result->{user}->disabledtext}); + } + + # If we get here, then we've run out of options, which + # shouldn't happen. + else { + ThrowCodeError("authres_unhandled", + { value => $fail_code }); + } + } + } + + return $user; } # Returns the network address for a given IP @@ -72,254 +213,300 @@ sub get_netaddr { return "0.0.0.0" if ($maskbits == 0); $addr >>= (32-$maskbits); + $addr <<= (32-$maskbits); return join(".", unpack("CCCC", pack("N", $addr))); } -# This is a replacement for the inherited authenticate function -# go through each of the available methods for each function -sub authenticate { - my $class = shift; - my @args = @_; - my @firstresult = (); - my @result = (); - my $current_verify_method; - for my $method (split /,\s*/, Param("user_verify_class")) { - $current_verify_method = $method; - $method = "Bugzilla::Auth::Verify::" . $method; - @result = $method->authenticate(@args); - @firstresult = @result unless @firstresult; - - if (($result[0] != AUTH_NODATA)&&($result[0] != AUTH_LOGINFAILED)) { - unshift @result, ($current_verify_method); - return @result; - } - } - @result = @firstresult; - # no auth match - - # see if we can set $current to the first verify method that - # will allow a new login - - my $chosen_verify_method; - for my $method (split /,\s*/, Param("user_verify_class")) { - $current_verify_method = $method; - $method = "Bugzilla::Auth::Verify::" . $method; - if ($method->can_edit('new')) { - $chosen_verify_method = $method; - } - } - - unshift @result, $chosen_verify_method; - return @result; -} - -sub can_edit { - my ($class, $type) = @_; - if ($current_verify_class) { - return $current_verify_class->can_edit($type); - } - # $current_verify_class will not be set if the user isn't logged in. That - # happens when the user is trying to create a new account, which (for now) - # is hard-coded to work with DB. - elsif (has_db) { - return Bugzilla::Auth::Verify::DB->can_edit($type); - } - return 0; -} - 1; - __END__ =head1 NAME -Bugzilla::Auth - Authentication handling for Bugzilla users +Bugzilla::Auth - An object that authenticates the login credentials for + a user. =head1 DESCRIPTION Handles authentication for Bugzilla users. Authentication from Bugzilla involves two sets of modules. One set is -used to obtain the data (from CGI, email, etc), and the other set uses -this data to authenticate against the datasource (the Bugzilla DB, LDAP, -cookies, etc). +used to obtain the username/password (from CGI, email, etc), and the +other set uses this data to authenticate against the datasource +(the Bugzilla DB, LDAP, PAM, etc.). + +Modules for obtaining the username/password are subclasses of +L, and modules for authenticating are subclasses +of L. + +=head1 AUTHENTICATION ERROR CODES + +Whenever a method in the C family fails in some way, +it will return a hashref containing at least a single key called C. +C will point to an integer error code, and depending on the error +code the hashref may contain more data. + +The error codes are explained here below. + +=head2 C + +Insufficient login data was provided by the user. This may happen in several +cases, such as cookie authentication when the cookie is not present. + +=head2 C + +An error occurred when trying to use the login mechanism. + +The hashref will also contain an C element, which is the name +of an error from C