Backing out bug 241900

Bugzilla.pm

@@ -18,7 +18,6 @@
 # Rights Reserved.
 #
 # Contributor(s): Bradley Baetz <bbaetz@student.usyd.edu.au>
-#                 Erik Stambaugh <erik@dasbistro.com>
 #
 
 package Bugzilla;
@@ -53,10 +52,6 @@ sub user {
     return $_user;
 }
 
-
-
-my $current_login_method = undef;
-
 sub login {
     my ($class, $type) = @_;
 
@@ -71,18 +66,12 @@ sub login {
 
     $type = LOGIN_NORMAL unless defined $type;
 
-    # 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;
-        }
-    }
+    # 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
 
+    use Bugzilla::Auth::CGI;
+    my $userid = Bugzilla::Auth::CGI->login($type);
     if ($userid) {
         $_user = new Bugzilla::User($userid);
 
@@ -108,14 +97,11 @@ sub logout {
     }
     $option = LOGOUT_CURRENT unless defined $option;
 
-    # $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();
-        }
+    use Bugzilla::Auth::CGI;
+    Bugzilla::Auth::CGI->logout($_user, $option);
+    if ($option != LOGOUT_KEEP_CURRENT) {
+        Bugzilla::Auth::CGI->clear_browser_cookies();
+        logout_request();
     }
 }
 
@@ -123,9 +109,8 @@ sub logout_user {
     my ($class, $user) = @_;
     # When we're logging out another user we leave cookies alone, and
     # therefore avoid calling logout() directly.
-    if ($current_login_method) {
-        $current_login_method->logout($_user, LOGOUT_ALL);
-    }
+    use Bugzilla::Auth::CGI;
+    Bugzilla::Auth::CGI->logout($user, LOGOUT_ALL);
 }
 
 # just a compatibility front-end to logout_user that gets a user by id
@@ -142,7 +127,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::Login::CGI->logout anyway
+    # remain there; it's only used in Bugzilla::Auth::CGI->logout anyway
     # People shouldn't rely on the cookie param for the username
     # - use Bugzilla->user instead!
 }

Bugzilla/Auth.pm

@@ -18,7 +18,6 @@
 # Rights Reserved.
 #
 # Contributor(s): Bradley Baetz <bbaetz@acm.org>
-#                 Erik Stambaugh <erik@dasbistro.com>
 
 package Bugzilla::Auth;
 
@@ -27,25 +26,19 @@ use strict;
 use Bugzilla::Config;
 use Bugzilla::Constants;
 
-# 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
+# 'inherit' from the main loginmethod
 BEGIN {
-    for my $verifymethod (split /,\s*/, Param("user_verify_method")) {
-        if ($verifymethod =~ /^([A-Za-z0-9_\.\-]+)$/) {
-            $verifymethod = $1;
+    my $loginmethod = Param("loginmethod");
+    if ($loginmethod =~ /^([A-Za-z0-9_\.\-]+)$/) {
+        $loginmethod = $1;
     }
     else {
-            die "Badly-named user_verify_method '$verifymethod'";
+        die "Badly-named loginmethod '$loginmethod'";
     }
-        require "Bugzilla/Auth/Verify/" . $verifymethod . ".pm";
+    require "Bugzilla/Auth/" . $loginmethod . ".pm";
 
-    }
+    our @ISA;
+    push (@ISA, "Bugzilla::Auth::" . $loginmethod);
 }
 
 # PRIVATE
@@ -68,46 +61,6 @@ 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__
@@ -125,8 +78,16 @@ 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).
 
-Modules for obtaining the data are located under L<Bugzilla::Auth::Login>, and
-modules for authenticating are located in L<Bugzilla::Auth::Verify>.
+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.
 
 =head1 METHODS
 
@@ -147,9 +108,7 @@ only some addresses.
 =head1 AUTHENTICATION
 
 Authentication modules check a user's credentials (username, password,
-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.
+etc) to verify who the user is.
 
 =head2 METHODS
 
@@ -216,36 +175,19 @@ 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.  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.
+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>.
 
 =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::Login::CGI|Bugzilla::Auth::Login::CGI>
+particular way. For example, L<Bugzilla::Auth::CGI|Bugzilla::Auth::CGI>
 logs in users from CGI scripts, first by using form variables, and then
 by trying cookies as a fallback.
 
@@ -308,5 +250,5 @@ user-performed password changes.
 
 =head1 SEE ALSO
 
-L<Bugzilla::Auth::Login::CGI>, L<Bugzilla::Auth::Login::CGI::Cookie>, L<Bugzilla::Auth::Verify::DB>
+L<Bugzilla::Auth::CGI>, L<Bugzilla::Auth::Cookie>, L<Bugzilla::Auth::DB>
 

Bugzilla/Auth/Login/CGI.pm → Bugzilla/Auth/CGI.pm

@@ -25,9 +25,8 @@
 #                 Gervase Markham <gerv@gerv.net>
 #                 Christian Reis <kiko@async.com.br>
 #                 Bradley Baetz <bbaetz@acm.org>
-#                 Erik Stambaugh <erik@dasbistro.com>
 
-package Bugzilla::Auth::Login::CGI;
+package Bugzilla::Auth::CGI;
 
 use strict;
 
@@ -50,7 +49,7 @@ sub login {
     my $username = $cgi->param("Bugzilla_login");
     my $passwd = $cgi->param("Bugzilla_password");
 
-    my $authmethod = Param("user_verify_method");
+    my $authmethod = Param("loginmethod");
     my ($authres, $userid, $extra, $info) =
       Bugzilla::Auth->authenticate($username, $passwd);
 
@@ -99,11 +98,11 @@ sub login {
         $username = $cgi->cookie("Bugzilla_login");
         $passwd = $cgi->cookie("Bugzilla_logincookie");
 
-        require Bugzilla::Auth::Login::CGI::Cookie;
+        require Bugzilla::Auth::Cookie;
         my $authmethod = "Cookie";
 
         ($authres, $userid, $extra) =
-          Bugzilla::Auth::Login::CGI::Cookie->authenticate($username, $passwd);
+          Bugzilla::Auth::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.
@@ -144,7 +143,7 @@ sub login {
                            { 'target' => $cgi->url(-relative=>1),
                              'form' => \%::FORM,
                              'mform' => \%::MFORM,
-                             'caneditaccount' => Bugzilla::Auth->can_edit->{'new'},
+                             'caneditaccount' => Bugzilla::Auth->can_edit,
                            }
                           )
           || ThrowTemplateError($template->error());
@@ -234,7 +233,7 @@ __END__
 
 =head1 NAME
 
-Bugzilla::Auth::Login::CGI - CGI-based logins for Bugzilla
+Bugzilla::Auth::CGI - CGI-based logins for Bugzilla
 
 =head1 SUMMARY
 
@@ -247,7 +246,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::Login::CGI::Cookie>.
+L<Bugzilla::Auth::Cookie>.
 
 =head1 SEE ALSO
 

Bugzilla/Auth/Login/CGI/Cookie.pm → Bugzilla/Auth/Cookie.pm

@@ -26,7 +26,7 @@
 #                 Christian Reis <kiko@async.com.br>
 #                 Bradley Baetz <bbaetz@acm.org>
 
-package Bugzilla::Auth::Login::CGI::Cookie;
+package Bugzilla::Auth::Cookie;
 
 use strict;
 
@@ -93,7 +93,7 @@ __END__
 
 =head1 NAME
 
-Bugzilla::Auth::Login::CGI::Cookie - cookie authentication for Bugzilla
+Bugzilla::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::Login::CGI> handles this.
+is successfully logged in). Instead L<Bugzilla::Auth::CGI> handles this.
 
 =head1 SEE ALSO
 
-L<Bugzilla::Auth>, L<Bugzilla::Auth::Login::CGI>
+L<Bugzilla::Auth>, L<Bugzilla::Auth::CGI>

Bugzilla/Auth/Verify/DB.pm → Bugzilla/Auth/DB.pm

@@ -25,9 +25,8 @@
 #                 Gervase Markham <gerv@gerv.net>
 #                 Christian Reis <kiko@async.com.br>
 #                 Bradley Baetz <bbaetz@acm.org>
-#                 Erik Stambaugh <erik@dasbistro.com>
 
-package Bugzilla::Auth::Verify::DB;
+package Bugzilla::Auth::DB;
 
 use strict;
 
@@ -35,15 +34,6 @@ 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) = @_;
 
@@ -71,6 +61,8 @@ sub authenticate {
     return (AUTH_OK, $userid);
 }
 
+sub can_edit { return 1; }
+
 sub get_id_from_username {
     my ($class, $username) = @_;
     my $dbh = Bugzilla->dbh;
@@ -119,7 +111,7 @@ __END__
 
 =head1 NAME
 
-Bugzilla::Auth::Verify::DB - database authentication for Bugzilla
+Bugzilla::Auth::DB - database authentication for Bugzilla
 
 =head1 SUMMARY
 

Bugzilla/Auth/Verify/LDAP.pm → Bugzilla/Auth/LDAP.pm

@@ -25,9 +25,8 @@
 #                 Gervase Markham <gerv@gerv.net>
 #                 Christian Reis <kiko@async.com.br>
 #                 Bradley Baetz <bbaetz@acm.org>
-#                 Erik Stambaugh <erik@dasbistro.com>
 
-package Bugzilla::Auth::Verify::LDAP;
+package Bugzilla::Auth::LDAP;
 
 use strict;
 
@@ -36,15 +35,6 @@ 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) = @_;
 
@@ -166,13 +156,15 @@ sub authenticate {
     return (AUTH_OK, $userid);
 }
 
+sub can_edit { return 0; }
+
 1;
 
 __END__
 
 =head1 NAME
 
-Bugzilla::Auth::Verify::LDAP - LDAP based authentication for Bugzilla
+Bugzilla::Auth::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.

Bugzilla/Config.pm

@@ -25,7 +25,6 @@
 #                 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;
 
@@ -218,12 +217,6 @@ 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) {

checksetup.pl

@@ -27,7 +27,6 @@
 #                 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
@@ -1493,12 +1492,10 @@ END { $dbh->disconnect if $dbh }
 # Check for LDAP
 ###########################################################################
 
-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";
-        }
+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";
     }
 }
 

defparams.pl

@@ -25,7 +25,6 @@
 #                 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
@@ -128,7 +127,7 @@ sub check_netmask {
     return "";
 }
 
-sub check_user_verify_method {
+sub check_loginmethod {
     # 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 
@@ -137,20 +136,18 @@ sub check_user_verify_method {
     # the login method as LDAP, we won't notice, but all logins will fail.
     # So don't do that.
 
-    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";
-        }
+    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";
     }
     return "";
 }
@@ -435,40 +432,9 @@ 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 => '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:
+   name => 'loginmethod',
+   desc => 'The type of login authentication to use:
             <dl>
               <dt>DB</dt>
               <dd>
@@ -484,9 +450,9 @@ sub find_languages {
               </dd>
              </dl>',
    type => 's',
-   choices => [ 'DB', 'LDAP', 'DB,LDAP', 'LDAP,DB' ],
+   choices => [ 'DB', 'LDAP' ],
    default => 'DB',
-   checker => \&check_user_verify_method
+   checker => \&check_loginmethod
   },
 
   {

editusers.cgi

@@ -23,7 +23,6 @@
 #                 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
 #
@@ -115,11 +114,15 @@ 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";
@@ -206,7 +209,7 @@ sub EmitFormElements ($$$$)
 sub PutTrailer (@)
 {
     my (@links) = ("Back to the <a href=\"./\">index</a>");
-    if($editall) {
+    if($editall && Bugzilla::Auth->can_edit) {
           push(@links,
               "<a href=\"editusers.cgi?action=add\">add</a> a new user");
     }
@@ -358,7 +361,7 @@ if ($action eq 'list') {
         }
         print "</TR>";
     }
-    if ($editall) {
+    if ($editall && Bugzilla::Auth->can_edit) {
         print "<TR>\n";
         my $span = $candelete ? 3 : 2;
         print qq{
@@ -392,6 +395,12 @@ 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";
 
@@ -423,6 +432,12 @@ 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
@@ -799,7 +814,7 @@ if ($action eq 'update') {
 
 
     # Update the database with the user's new password if they changed it.
-    if ( $editall && $password ) {
+    if ( Bugzilla::Auth->can_edit && $editall && $password ) {
         my $passworderror = ValidatePassword($password);
         if ( !$passworderror ) {
             my $cryptpassword = SqlQuote(Crypt($password));

t/Support/Files.pm

@@ -29,7 +29,7 @@ package Support::Files;
 @additional_files = ();
 %exclude_deps = (
     'XML::Parser' => ['importxml.pl'],
-    'Net::LDAP' => ['Bugzilla/Auth/Verify/LDAP.pm'],
+    'Net::LDAP' => ['Bugzilla/Auth/LDAP.pm'],
 );