diff options
| -rw-r--r-- | Bugzilla.pm | 41 | ||||
| -rw-r--r-- | Bugzilla/Auth.pm | 108 | ||||
| -rw-r--r-- | Bugzilla/Auth/Login/CGI.pm (renamed from Bugzilla/Auth/CGI.pm) | 15 | ||||
| -rw-r--r-- | Bugzilla/Auth/Login/CGI/Cookie.pm (renamed from Bugzilla/Auth/Cookie.pm) | 8 | ||||
| -rw-r--r-- | Bugzilla/Auth/Verify/DB.pm (renamed from Bugzilla/Auth/DB.pm) | 16 | ||||
| -rw-r--r-- | Bugzilla/Auth/Verify/LDAP.pm (renamed from Bugzilla/Auth/LDAP.pm) | 16 | ||||
| -rw-r--r-- | Bugzilla/Config.pm | 7 | ||||
| -rwxr-xr-x | checksetup.pl | 11 | ||||
| -rw-r--r-- | defparams.pl | 68 | ||||
| -rwxr-xr-x | editusers.cgi | 23 | ||||
| -rw-r--r-- | t/Support/Files.pm | 2 |
11 files changed, 217 insertions, 98 deletions
diff --git a/Bugzilla.pm b/Bugzilla.pm index 5cee520c7..84cc2d721 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -18,6 +18,7 @@ # Rights Reserved. # # Contributor(s): Bradley Baetz <bbaetz@student.usyd.edu.au> +# Erik Stambaugh <erik@dasbistro.com> # package Bugzilla; @@ -52,6 +53,10 @@ sub user { return $_user; } + + +my $current_login_method = undef; + sub login { my ($class, $type) = @_; @@ -66,12 +71,18 @@ sub login { $type = LOGIN_NORMAL unless defined $type; - # For now, we can only log in from a cgi - # One day, we'll be able to log in via apache auth, an email message's - # PGP signature, and so on + # Log in using whatever methods are defined in user_info_method + + my $userid; + for my $method (split(/,\s*/, Param('user_info_method'))) { + require "Bugzilla/Auth/Login/" . $method . ".pm"; + $userid = "Bugzilla::Auth::Login::$method"->login($type); + if ($userid) { + $current_login_method = "Bugzilla::Auth::Login::$method"; + last; + } + } - use Bugzilla::Auth::CGI; - my $userid = Bugzilla::Auth::CGI->login($type); if ($userid) { $_user = new Bugzilla::User($userid); @@ -97,11 +108,14 @@ sub logout { } $option = LOGOUT_CURRENT unless defined $option; - use Bugzilla::Auth::CGI; - Bugzilla::Auth::CGI->logout($_user, $option); - if ($option != LOGOUT_KEEP_CURRENT) { - Bugzilla::Auth::CGI->clear_browser_cookies(); - logout_request(); + # $current_login_method is defined when the user's login information is + # found. If it's not defined, the user shouldn't be logged in. + if ($current_login_method) { + $current_login_method->logout($_user, $option); + if ($option != LOGOUT_KEEP_CURRENT) { + $current_login_method->clear_browser_cookies(); + logout_request(); + } } } @@ -109,8 +123,9 @@ sub logout_user { my ($class, $user) = @_; # When we're logging out another user we leave cookies alone, and # therefore avoid calling logout() directly. - use Bugzilla::Auth::CGI; - Bugzilla::Auth::CGI->logout($user, LOGOUT_ALL); + if ($current_login_method) { + $current_login_method->logout($_user, LOGOUT_ALL); + } } # just a compatibility front-end to logout_user that gets a user by id @@ -127,7 +142,7 @@ sub logout_request { # XXX clean these up eventually delete $::COOKIE{"Bugzilla_login"}; # NB - Can't delete from $cgi->cookie, so the logincookie data will - # remain there; it's only used in Bugzilla::Auth::CGI->logout anyway + # remain there; it's only used in Bugzilla::Auth::Login::CGI->logout anyway # People shouldn't rely on the cookie param for the username # - use Bugzilla->user instead! } diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index dcea8189a..e6cf27963 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -18,6 +18,7 @@ # Rights Reserved. # # Contributor(s): Bradley Baetz <bbaetz@acm.org> +# Erik Stambaugh <erik@dasbistro.com> package Bugzilla::Auth; @@ -26,19 +27,25 @@ use strict; use Bugzilla::Config; use Bugzilla::Constants; -# 'inherit' from the main loginmethod +# This is here for lack of a better place for it. I considered making it +# part of the user object, but that object doesn't necessarily point to a +# currently authenticated user. +# +# I'm willing to accept suggestions for somewhere else to put it. +my $current_verify_method = undef; + +# 'inherit' from the main verify method BEGIN { - my $loginmethod = Param("loginmethod"); - if ($loginmethod =~ /^([A-Za-z0-9_\.\-]+)$/) { - $loginmethod = $1; + for my $verifymethod (split /,\s*/, Param("user_verify_method")) { + if ($verifymethod =~ /^([A-Za-z0-9_\.\-]+)$/) { + $verifymethod = $1; } else { - die "Badly-named loginmethod '$loginmethod'"; + die "Badly-named user_verify_method '$verifymethod'"; } - require "Bugzilla/Auth/" . $loginmethod . ".pm"; + require "Bugzilla/Auth/Verify/" . $verifymethod . ".pm"; - our @ISA; - push (@ISA, "Bugzilla::Auth::" . $loginmethod); + } } # PRIVATE @@ -61,6 +68,46 @@ sub get_netaddr { 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 $self = shift; + my @args = @_; + my @firstresult = (); + my @result = (); + for my $method (split /,\s*/, Param("user_verify_method")) { + $method = "Bugzilla::Auth::Verify::" . $method; + @result = $method->authenticate(@args); + @firstresult = @result unless @firstresult; + + if (($result[0] != AUTH_NODATA)&&($result[0] != AUTH_LOGINFAILED)) { + $current_verify_method = $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 + + for my $method (split /,\s*/, Param("user_verify_method")) { + $method = "Bugzilla::Auth::Verify::" . $method; + if ($method::can_edit->{'new'}) { + $current_verify_method = $method; + } + } + + return @result; +} + +sub can_edit { + if ($current_verify_method) { + return $current_verify_method->{'can_edit'}; + } + return {}; +} + 1; __END__ @@ -78,16 +125,8 @@ 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). -The handlers for the various types of authentication -(DB/LDAP/cookies/etc) provide the actual code for each specific method -of authentication. - -The source modules (currently, only -L<Bugzilla::Auth::CGI|Bugzilla::Auth::CGI>) then use those methods to do -the authentication. - -I<Bugzilla::Auth> itself inherits from the default authentication handler, -identified by the I<loginmethod> param. +Modules for obtaining the data are located under L<Bugzilla::Auth::Login>, and +modules for authenticating are located in L<Bugzilla::Auth::Verify>. =head1 METHODS @@ -108,7 +147,9 @@ only some addresses. =head1 AUTHENTICATION Authentication modules check a user's credentials (username, password, -etc) to verify who the user is. +etc) to verify who the user is. The methods that C<Bugzilla::Auth> uses for +authentication are wrappers that check all configured modules (via the +C<Param('user_info_method')> and C<Param('user_verify_method')>) in sequence. =head2 METHODS @@ -175,19 +216,36 @@ Note that this argument is a string, not a tag. =back +=item C<current_verify_method> + +This scalar gets populated with the full name (eg., +C<Bugzilla::Auth::Verify::DB>) of the verification method being used by the +current user. If no user is logged in, it will contain the name of the first +method that allows new users, if any. Otherwise, it carries an undefined +value. + =item C<can_edit> -This determines if the user's account details can be modified. If this -method returns a C<true> value, then accounts can be created and -modified through the Bugzilla user interface. Forgotten passwords can -also be retrieved through the L<Token interface|Bugzilla::Token>. +This determines if the user's account details can be modified. It returns a +reference to a hash with the keys C<userid>, C<login_name>, and C<realname>, +which determine whether their respective profile values may be altered, and +C<new>, which determines if new accounts may be created. + +Each user verification method (chosen with C<Param('user_verify_method')> has +its own set of can_edit values. Calls to can_edit return the appropriate +values for the current user's login method. + +If a user is not logged in, C<can_edit> will contain the values of the first +verify method that allows new users to be created, if available. Otherwise it +returns an empty hash. =back =head1 LOGINS A login module can be used to try to log in a Bugzilla user in a -particular way. For example, L<Bugzilla::Auth::CGI|Bugzilla::Auth::CGI> +particular way. For example, +L<Bugzilla::Auth::Login::CGI|Bugzilla::Auth::Login::CGI> logs in users from CGI scripts, first by using form variables, and then by trying cookies as a fallback. @@ -250,5 +308,5 @@ user-performed password changes. =head1 SEE ALSO -L<Bugzilla::Auth::CGI>, L<Bugzilla::Auth::Cookie>, L<Bugzilla::Auth::DB> +L<Bugzilla::Auth::Login::CGI>, L<Bugzilla::Auth::Login::CGI::Cookie>, L<Bugzilla::Auth::Verify::DB> diff --git a/Bugzilla/Auth/CGI.pm b/Bugzilla/Auth/Login/CGI.pm index 471e538e9..2f8ca071d 100644 --- a/Bugzilla/Auth/CGI.pm +++ b/Bugzilla/Auth/Login/CGI.pm @@ -25,8 +25,9 @@ # Gervase Markham <gerv@gerv.net> # Christian Reis <kiko@async.com.br> # Bradley Baetz <bbaetz@acm.org> +# Erik Stambaugh <erik@dasbistro.com> -package Bugzilla::Auth::CGI; +package Bugzilla::Auth::Login::CGI; use strict; @@ -49,7 +50,7 @@ sub login { my $username = $cgi->param("Bugzilla_login"); my $passwd = $cgi->param("Bugzilla_password"); - my $authmethod = Param("loginmethod"); + my $authmethod = Param("user_verify_method"); my ($authres, $userid, $extra, $info) = Bugzilla::Auth->authenticate($username, $passwd); @@ -98,11 +99,11 @@ sub login { $username = $cgi->cookie("Bugzilla_login"); $passwd = $cgi->cookie("Bugzilla_logincookie"); - require Bugzilla::Auth::Cookie; + require Bugzilla::Auth::Login::CGI::Cookie; my $authmethod = "Cookie"; ($authres, $userid, $extra) = - Bugzilla::Auth::Cookie->authenticate($username, $passwd); + Bugzilla::Auth::Login::CGI::Cookie->authenticate($username, $passwd); # If the data for the cookie was incorrect, then treat that as # NODATA. This could occur if the user's IP changed, for example. @@ -143,7 +144,7 @@ sub login { { 'target' => $cgi->url(-relative=>1), 'form' => \%::FORM, 'mform' => \%::MFORM, - 'caneditaccount' => Bugzilla::Auth->can_edit, + 'caneditaccount' => Bugzilla::Auth->can_edit->{'new'}, } ) || ThrowTemplateError($template->error()); @@ -233,7 +234,7 @@ __END__ =head1 NAME -Bugzilla::Auth::CGI - CGI-based logins for Bugzilla +Bugzilla::Auth::Login::CGI - CGI-based logins for Bugzilla =head1 SUMMARY @@ -246,7 +247,7 @@ Users are first authenticated against the default authentication handler, using the CGI parameters I<Bugzilla_login> and I<Bugzilla_password>. If no data is present for that, then cookies are tried, using -L<Bugzilla::Auth::Cookie>. +L<Bugzilla::Auth::Login::CGI::Cookie>. =head1 SEE ALSO diff --git a/Bugzilla/Auth/Cookie.pm b/Bugzilla/Auth/Login/CGI/Cookie.pm index b50acbe24..9c0e2e566 100644 --- a/Bugzilla/Auth/Cookie.pm +++ b/Bugzilla/Auth/Login/CGI/Cookie.pm @@ -26,7 +26,7 @@ # Christian Reis <kiko@async.com.br> # Bradley Baetz <bbaetz@acm.org> -package Bugzilla::Auth::Cookie; +package Bugzilla::Auth::Login::CGI::Cookie; use strict; @@ -93,7 +93,7 @@ __END__ =head1 NAME -Bugzilla::Cookie - cookie authentication for Bugzilla +Bugzilla::Auth::Login::CGI::Cookie - cookie authentication for Bugzilla =head1 SUMMARY @@ -108,8 +108,8 @@ restricted to certain IP addresses as a security meaure. The exact restriction can be specified by the admin via the C<loginnetmask> parameter. This module does not ever send a cookie (It has no way of knowing when a user -is successfully logged in). Instead L<Bugzilla::Auth::CGI> handles this. +is successfully logged in). Instead L<Bugzilla::Auth::Login::CGI> handles this. =head1 SEE ALSO -L<Bugzilla::Auth>, L<Bugzilla::Auth::CGI> +L<Bugzilla::Auth>, L<Bugzilla::Auth::Login::CGI> diff --git a/Bugzilla/Auth/DB.pm b/Bugzilla/Auth/Verify/DB.pm index dee3b5db9..4db34b5cf 100644 --- a/Bugzilla/Auth/DB.pm +++ b/Bugzilla/Auth/Verify/DB.pm @@ -25,8 +25,9 @@ # Gervase Markham <gerv@gerv.net> # Christian Reis <kiko@async.com.br> # Bradley Baetz <bbaetz@acm.org> +# Erik Stambaugh <erik@dasbistro.com> -package Bugzilla::Auth::DB; +package Bugzilla::Auth::Verify::DB; use strict; @@ -34,6 +35,15 @@ use Bugzilla::Config; use Bugzilla::Constants; use Bugzilla::Util; +# can_edit is now a hash. + +my $can_edit = { + 'new' => 1, + 'userid' => 0, + 'login_name' => 1, + 'realname' => 1, +}; + sub authenticate { my ($class, $username, $passwd) = @_; @@ -61,8 +71,6 @@ sub authenticate { return (AUTH_OK, $userid); } -sub can_edit { return 1; } - sub get_id_from_username { my ($class, $username) = @_; my $dbh = Bugzilla->dbh; @@ -111,7 +119,7 @@ __END__ =head1 NAME -Bugzilla::Auth::DB - database authentication for Bugzilla +Bugzilla::Auth::Verify::DB - database authentication for Bugzilla =head1 SUMMARY diff --git a/Bugzilla/Auth/LDAP.pm b/Bugzilla/Auth/Verify/LDAP.pm index c34c3698f..737827ee0 100644 --- a/Bugzilla/Auth/LDAP.pm +++ b/Bugzilla/Auth/Verify/LDAP.pm @@ -25,8 +25,9 @@ # Gervase Markham <gerv@gerv.net> # Christian Reis <kiko@async.com.br> # Bradley Baetz <bbaetz@acm.org> +# Erik Stambaugh <erik@dasbistro.com> -package Bugzilla::Auth::LDAP; +package Bugzilla::Auth::Verify::LDAP; use strict; @@ -35,6 +36,15 @@ use Bugzilla::Constants; use Net::LDAP; +# can_edit is now a hash. + +my $can_edit = { + 'new' => 0, + 'userid' => 0, + 'login_name' => 0, + 'realname' => 0, +}; + sub authenticate { my ($class, $username, $passwd) = @_; @@ -156,15 +166,13 @@ sub authenticate { return (AUTH_OK, $userid); } -sub can_edit { return 0; } - 1; __END__ =head1 NAME -Bugzilla::Auth::LDAP - LDAP based authentication for Bugzilla +Bugzilla::Auth::Verify::LDAP - LDAP based authentication for Bugzilla This is an L<authentication module|Bugzilla::Auth/"AUTHENTICATION"> for Bugzilla, which logs the user in using an LDAP directory. diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm index b568918e3..d73f22875 100644 --- a/Bugzilla/Config.pm +++ b/Bugzilla/Config.pm @@ -25,6 +25,7 @@ # J. Paul Reed <preed@sigkill.com> # Bradley Baetz <bbaetz@student.usyd.edu.au> # Christopher Aillon <christopher@aillon.com> +# Erik Stambaugh <erik@dasbistro.com> package Bugzilla::Config; @@ -217,6 +218,12 @@ sub UpdateParams { $param{'loginmethod'} = $param{'useLDAP'} ? "LDAP" : "DB"; } + # set verify method to whatever loginmethod was + if (exists $param{'loginmethod'} && !exists $param{'user_verify_method'}) { + $param{'user_verify_method'} = $param{'loginmethod'}; + delete $param{'loginmethod'}; + } + # --- DEFAULTS FOR NEW PARAMS --- foreach my $item (@param_list) { diff --git a/checksetup.pl b/checksetup.pl index 51a1bb2f4..9961009e8 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -27,6 +27,7 @@ # Bradley Baetz <bbaetz@student.usyd.edu.au> # Tobias Burnus <burnus@net-b.de> # Gervase Markham <gerv@gerv.net> +# Erik Stambaugh <erik@dasbistro.com> # # # Direct any questions on this source code to @@ -1492,10 +1493,12 @@ END { $dbh->disconnect if $dbh } # Check for LDAP ########################################################################### -if (Param('loginmethod') eq 'LDAP') { - my $netLDAP = have_vers("Net::LDAP", 0); - if (!$netLDAP && !$silent) { - print "If you wish to use LDAP authentication, then you must install Net::LDAP\n\n"; +for my $verifymethod (split /,\s*/, Param('user_verify_method')) { + if ($verifymethod eq 'LDAP') { + my $netLDAP = have_vers("Net::LDAP", 0); + if (!$netLDAP && !$silent) { + print "If you wish to use LDAP authentication, then you must install Net::LDAP\n\n"; + } } } diff --git a/defparams.pl b/defparams.pl index 6861d0447..b067d583a 100644 --- a/defparams.pl +++ b/defparams.pl @@ -25,6 +25,7 @@ # J. Paul Reed <preed@sigkill.com> # Bradley Baetz <bbaetz@student.usyd.edu.au> # Joseph Heenan <joseph@heenan.me.uk> +# Erik Stambaugh <erik@dasbistro.com> # # This file defines all the parameters that we have a GUI to edit within @@ -127,7 +128,7 @@ sub check_netmask { return ""; } -sub check_loginmethod { +sub check_user_verify_method { # doeditparams traverses the list of params, and for each one it checks, # then updates. This means that if one param checker wants to look at # other params, it must be below that other one. So you can't have two @@ -136,18 +137,20 @@ sub check_loginmethod { # the login method as LDAP, we won't notice, but all logins will fail. # So don't do that. - my ($method, $entry) = @_; - my $res = check_multi($method, $entry); - return $res if $res; - if ($method eq 'DB') { - # No params - } elsif ($method eq 'LDAP') { - eval "require Net::LDAP"; - return "Error requiring Net::LDAP: '$@'" if $@; - return "LDAP servername is missing" unless Param("LDAPserver"); - return "LDAPBaseDN is empty" unless Param("LDAPBaseDN"); - } else { - return "Unknown loginmethod '$method' in check_loginmethod"; + my ($list, $entry) = @_; + for my $method (split /,\s*/, $list) { + my $res = check_multi($method, $entry); + return $res if $res; + if ($method eq 'DB') { + # No params + } elsif ($method eq 'LDAP') { + eval "require Net::LDAP"; + return "Error requiring Net::LDAP: '$@'" if $@; + return "LDAP servername is missing" unless Param("LDAPserver"); + return "LDAPBaseDN is empty" unless Param("LDAPBaseDN"); + } else { + return "Unknown user_verify_method '$method' in check_user_verify_method"; + } } return ""; } @@ -432,9 +435,40 @@ sub find_languages { default => '', }, + # in the future: + # + # user_verify_method and user_info_method should have choices gathered from + # whatever sits in their respective directories + # + # rather than comma-separated lists, these two should eventually become + # arrays, but that requires alterations to editparams first + + { + name => 'user_info_method', + desc => 'Methods to be used for gathering a user\'s login information. + <add> + More than one may be selected. If the first one returns nothing, + the second is tried, and so on.<br /> + The types are: + <dl> + <dt>CGI</dt> + <dd> + Asks for username and password via CGI form interface. + </dd> + </dl>', + type => 's', + choices => [ 'CGI' ], + default => 'CGI', + checker => \&check_multi + }, + { - name => 'loginmethod', - desc => 'The type of login authentication to use: + name => 'user_verify_method', + desc => 'Methods to be used for verifying (authenticating) information + gathered by user_info_method. + More than one may be selected. If the first one cannot find the + user, the second is tried, and so on.<br /> + The types are: <dl> <dt>DB</dt> <dd> @@ -450,9 +484,9 @@ sub find_languages { </dd> </dl>', type => 's', - choices => [ 'DB', 'LDAP' ], + choices => [ 'DB', 'LDAP', 'DB,LDAP', 'LDAP,DB' ], default => 'DB', - checker => \&check_loginmethod + checker => \&check_user_verify_method }, { diff --git a/editusers.cgi b/editusers.cgi index 826bb4b34..fa3efbf8f 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -23,6 +23,7 @@ # Joe Robins <jmrobins@tgix.com> # Dan Mosedale <dmose@mozilla.org> # Joel Peshkin <bugreport@peshkin.net> +# Erik Stambaugh <erik@dasbistro.com> # # Direct any questions on this source code to # @@ -114,15 +115,11 @@ sub EmitFormElements ($$$$) if ($editall) { print "</TR><TR>\n"; print " <TH ALIGN=\"right\">Password:</TH>\n"; - if(!Bugzilla::Auth->can_edit) { - print " <TD><FONT COLOR=RED>This site's authentication method does not allow password changes through Bugzilla!</FONT></TD>\n"; - } else { print qq| <TD><INPUT TYPE="PASSWORD" SIZE="16" MAXLENGTH="16" NAME="password" VALUE=""><br> (enter new password to change) </TD> |; - } print "</TR><TR>\n"; print " <TH ALIGN=\"right\">Disable text:</TH>\n"; @@ -209,7 +206,7 @@ sub EmitFormElements ($$$$) sub PutTrailer (@) { my (@links) = ("Back to the <a href=\"./\">index</a>"); - if($editall && Bugzilla::Auth->can_edit) { + if($editall) { push(@links, "<a href=\"editusers.cgi?action=add\">add</a> a new user"); } @@ -361,7 +358,7 @@ if ($action eq 'list') { } print "</TR>"; } - if ($editall && Bugzilla::Auth->can_edit) { + if ($editall) { print "<TR>\n"; my $span = $candelete ? 3 : 2; print qq{ @@ -395,12 +392,6 @@ if ($action eq 'add') { exit; } - if(!Bugzilla::Auth->can_edit) { - print "The authentication mechanism you are using does not permit accounts to be created from Bugzilla"; - PutTrailer(); - exit; - } - print "<FORM METHOD=POST ACTION=editusers.cgi>\n"; print "<TABLE BORDER=0 CELLPADDING=4 CELLSPACING=0><TR>\n"; @@ -432,12 +423,6 @@ if ($action eq 'new') { exit; } - if (!Bugzilla::Auth->can_edit) { - print "This site's authentication mechanism does not allow new users to be added."; - PutTrailer(); - exit; - } - # Cleanups and valididy checks my $realname = trim($::FORM{realname} || ''); # We don't trim the password since that could falsely lead the user @@ -814,7 +799,7 @@ if ($action eq 'update') { # Update the database with the user's new password if they changed it. - if ( Bugzilla::Auth->can_edit && $editall && $password ) { + if ( $editall && $password ) { my $passworderror = ValidatePassword($password); if ( !$passworderror ) { my $cryptpassword = SqlQuote(Crypt($password)); diff --git a/t/Support/Files.pm b/t/Support/Files.pm index ffadc562c..de5b598c5 100644 --- a/t/Support/Files.pm +++ b/t/Support/Files.pm @@ -29,7 +29,7 @@ package Support::Files; @additional_files = (); %exclude_deps = ( 'XML::Parser' => ['importxml.pl'], - 'Net::LDAP' => ['Bugzilla/Auth/LDAP.pm'], + 'Net::LDAP' => ['Bugzilla/Auth/Verify/LDAP.pm'], ); |