Fix for bug 226764: Move InvalidateLogins into Bugzilla::Auth::CGI.

Consolidates the logout code into Bugzilla::Auth::CGI, and provides
simple front-end wrappers in Bugzilla.pm for use in the CGIs we have.
r=bbaetz, joel; a=justdave.

Adds a set of constants to the logout() API which allow specifying "how
much" we should log out -- all sessions, the current session, or all
sessions but the current one.

Fixes callsites to use this new API; cleans and documents things a
bit while we're at it. Part I in the great COOKIE apocalypse.

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 7e7d50004f88a99b13481ad85cb449f4a2486a02..5cee520c7f2987d083207babb1bdbb8020a3703e 100644 (file)
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -83,33 +83,53 @@ sub login {
         # so it needs to be set.
         $::COOKIE{'Bugzilla_login'} = $_user->login;
     } else {
-        # Old compat stuff
-
-        undef $_user;
-        $::userid = 0;
-        delete $::COOKIE{'Bugzilla_login'};
-        delete $::COOKIE{'Bugzilla_logincookie'};
-        # NB - Can't delete from $cgi->cookie, so the cookie data will
-        # remain there
-        # People shouldn't rely on the cookie param for the username
-        # - use Bugzilla->user instead!
+        logout_request();
     }
 
     return $_user;
 }
 
 sub logout {
+    my ($class, $option) = @_;
+    if (! $_user) {
+        # If we're not logged in, go away
+        return;
+    }
+    $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();
+    }
+}
+
+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;
-    # remove cookies and clean up database state
-    Bugzilla::Auth::CGI->logout();
-    logout_request();
+    Bugzilla::Auth::CGI->logout($user, LOGOUT_ALL);
 }
 
+# just a compatibility front-end to logout_user that gets a user by id
+sub logout_user_by_id {
+    my ($class, $id) = @_;
+    my $user = new Bugzilla::User($id);
+    $class->logout_user($user);
+}
+
+# hack that invalidates credentials for a single request
 sub logout_request {
     undef $_user;
     $::userid = 0;
+    # XXX clean these up eventually
     delete $::COOKIE{"Bugzilla_login"};
-    delete $::COOKIE{"Bugzilla_logincookie"};
+    # NB - Can't delete from $cgi->cookie, so the logincookie data will
+    # 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!
 }
 
 my $_dbh;
@@ -264,7 +284,7 @@ method for those scripts/templates which are only use via CGI, though.
 
 =item C<user>
 
-The current L<Bugzilla::User>. C<undef> if there is no currently logged in user
+The current C<Bugzilla::User>. C<undef> if there is no currently logged in user
 or if the login code has not yet been run.
 
 =item C<login>
@@ -273,15 +293,29 @@ Logs in a user, returning a C<Bugzilla::User> object, or C<undef> if there is
 no logged in user. See L<Bugzilla::Auth|Bugzilla::Auth> and
 L<Bugzilla::User|Bugzilla::User>.
 
-=item C<logout>
+=item C<logout($option)>
+
+Logs out the current user, which involves invalidating user sessions and
+cookies. Three options are available from
+L<Bugzilla::Constants|Bugzilla::Constants>: LOGOUT_CURRENT (the
+default), LOGOUT_ALL or LOGOUT_KEEP_CURRENT.
+
+=item C<logout_user($user)>
+
+Logs out the specified user (invalidating all his sessions), taking a
+Bugzilla::User instance.
+
+=item C<logout_by_id($id)>
 
-Logs out the current user.
+Logs out the user with the id specified. This is a compatibility
+function to be used in callsites where there is only a userid and no
+Bugzilla::User instance.
 
 =item C<logout_request>
 
-Essentially, causes calls to C<user> to return C<undef>. This has the
+Essentially, causes calls to C<Bugzilla->user> to return C<undef>. This has the
 effect of logging out a user for the current request only; cookies and
-database state are left intact. 
+database sessions are left intact. 
 
 =item C<dbh>
 

diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm
index 22c2fc56161ad72db0eaf595f1393d151ae48add..dcea8189a087ad4c47b7fe52f274e4ce21b37a8d 100644 (file)
--- a/Bugzilla/Auth.pm
+++ b/Bugzilla/Auth.pm
@@ -73,15 +73,18 @@ Bugzilla::Auth - Authentication handling for Bugzilla users
 
 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).
+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).
 
-The handlers for the various types of authentication (DB/LDAP/cookies/etc)
-provide the actual code for each specific method of authentication.
+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.
+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.
@@ -96,16 +99,16 @@ authentication or login modules.
 =item C<Bugzilla::Auth::get_netaddr($ipaddr)>
 
 Given an ip address, this returns the associated network address, using
-C<Param('loginnetmask')> at the netmask. This can be used to obtain data in
-order to restrict weak authentication methods (such as cookies) to only some
-addresses.
+C<Param('loginnetmask')> as the netmask. This can be used to obtain data
+in order to restrict weak authentication methods (such as cookies) to
+only some addresses.
 
 =back
 
 =head1 AUTHENTICATION
 
-Authentication modules check a users's credentials (username, password, etc) to
-verify who the user is.
+Authentication modules check a user's credentials (username, password,
+etc) to verify who the user is.
 
 =head2 METHODS
 
@@ -113,19 +116,21 @@ verify who the user is.
 
 =item C<authenticate($username, $pass)>
 
-This method is passed a username and a password, and returns a list containing
-up to four return values, depending on the results of the authentication.
+This method is passed a username and a password, and returns a list
+containing up to four return values, depending on the results of the
+authentication.
 
 The first return value is one of the status codes defined in
-L<Bugzilla::Constants|Bugzilla::Constants> and described below.  The rest of
-the return values are status code-specific and are explained in the status
-code descriptions.
+L<Bugzilla::Constants|Bugzilla::Constants> and described below.  The
+rest of the return values are status code-specific and are explained in
+the status code descriptions.
 
 =over 4
 
 =item C<AUTH_OK>
 
-Authentication succeeded. The second variable is the userid of the new user.
+Authentication succeeded. The second variable is the userid of the new
+user.
 
 =item C<AUTH_NODATA>
 
@@ -162,41 +167,52 @@ information.
 
 =item C<AUTH_DISABLED>
 
-The user successfully logged in, but their account has been disabled. The
-second argument in the returned array is the userid, and the third is some
-text explaining why the account was disabled. This text would typically come
-from the C<disabledtext> field in the C<profiles> table. Note that this 
-argument is a string, not a tag.
+The user successfully logged in, but their account has been disabled.
+The second argument in the returned array is the userid, and the third
+is some text explaining why the account was disabled. This text would
+typically come from the C<disabledtext> field in the C<profiles> table.
+Note that this argument is a string, not a tag.
 
 =back
 
 =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>.
+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::CGI|Bugzilla::Auth::CGI> logs in users
-from CGI scripts, first by trying database authentication against the
-Bugzilla C<profiles> table, and then by trying cookies as a fallback.
+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>
+logs in users from CGI scripts, first by using form variables, and then
+by trying cookies as a fallback.
 
-A login module consists of a single method, C<login>, which takes a C<$type>
-argument, using constants found in C<Bugzilla::Constants>.
+The login interface consists of the following methods:
+
+=item C<login>, which takes a C<$type> argument, using constants found in
+C<Bugzilla::Constants>.
+
+The login method may use various authentication modules (described
+above) to try to authenticate a user, and should return the userid on
+success, or C<undef> on failure.
+
+When a login is required, but data is not present, it is the job of the
+login method to prompt the user for this data.
+
+The constants accepted by C<login> include the following:
 
 =over 4
 
 =item C<LOGIN_OPTIONAL>
 
-A login is never required to access this data. Attempting to login is still 
-useful, because this allows the page to be personalised. Note that an 
-incorrect login will still trigger an error, even though the lack of a login 
-will be OK.
+A login is never required to access this data. Attempting to login is
+still useful, because this allows the page to be personalised. Note that
+an incorrect login will still trigger an error, even though the lack of
+a login will be OK.
 
 =item C<LOGIN_NORMAL>
 
@@ -209,12 +225,30 @@ A login is always required to access this data.
 
 =back
 
-The login module uses various authentication modules to try to authenticate
-a user, and returns the userid on success, or C<undef> on failure.
+=item C<logout>, which takes a C<Bugzilla::User> argument for the user
+being logged out, and an C<$option> argument. Possible values for
+C<$option> include:
+
+=over 4
+
+=item C<LOGOUT_CURRENT>
 
-When a login is required, but data is not present, it is the job of the login
-module to prompt the user for this data.
+Log out the user and invalidate his currently registered session.
+
+=item C<LOGOUT_ALL>
+
+Log out the user, and invalidate all sessions the user has registered in
+Bugzilla.
+
+=item C<LOGOUT_KEEP_CURRENT>
+
+Invalidate all sessions the user has registered excluding his current
+session; this option should leave the user logged in. This is useful for
+user-performed password changes.
+
+=back
 
 =head1 SEE ALSO
 
 L<Bugzilla::Auth::CGI>, L<Bugzilla::Auth::Cookie>, L<Bugzilla::Auth::DB>
+

diff --git a/editusers.cgi b/editusers.cgi
index 411704dce5504a1599a523c2b262ff36d81d0f63..e7ef0e7d30fbfda908458b24fa4c7ca4023e24b2 100755 (executable)
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -34,6 +34,7 @@ use lib ".";
 require "CGI.pl";
 require "globals.pl";
 
+use Bugzilla;
 use Bugzilla::User;
 
 # Shut up misguided -w warnings about "used only once".  "use vars" just
@@ -660,8 +661,7 @@ if ($action eq 'delete') {
 
     SendSQL("DELETE FROM profiles
              WHERE login_name=" . SqlQuote($user));
-    SendSQL("DELETE FROM logincookies
-             WHERE userid=" . $userid);
+    Bugzilla->logout_user_by_id($userid);
     print "User deleted.<BR>\n";
 
     PutTrailer($localtrailer);
@@ -818,7 +818,7 @@ if ($action eq 'update') {
                      FROM profiles
                      WHERE login_name=" . SqlQuote($userold));
             my $userid = FetchOneColumn();
-            InvalidateLogins($userid);
+            Bugzilla->logout_user_by_id($userid);
             print "Updated password.<BR>\n";
         } else {
             print "Did not update password: $passworderror<br>\n";
@@ -838,7 +838,7 @@ if ($action eq 'update') {
                  FROM profiles
                  WHERE login_name=" . SqlQuote($userold));
         my $userid = FetchOneColumn();
-        InvalidateLogins($userid);
+        Bugzilla->logout_user_by_id($userid);
         print "Updated disabled text.<BR>\n";
     }
     if ($editall && $user ne $userold) {

diff --git a/globals.pl b/globals.pl
index aef84f2a66072f1c384ab7a54d04968edfac65cc..2b68b2b41e778586f1d0fd4b79d4bed295f885a2 100644 (file)
--- a/globals.pl
+++ b/globals.pl
@@ -425,19 +425,6 @@ sub InsertNewUser {
     return $password;
 }
 
-# Removes all entries from logincookies for $userid, except for the
-# optional $keep, which refers the logincookies.cookie primary key.
-# (This is useful so that a user changing their password stays logged in)
-sub InvalidateLogins {
-    my ($userid, $keep) = @_;
-
-    my $remove = "DELETE FROM logincookies WHERE userid = $userid";
-    if (defined $keep) {
-        $remove .= " AND cookie != " . SqlQuote($keep);
-    }
-    SendSQL($remove);
-}
-
 sub GenerateRandomPassword {
     my ($size) = @_;
 

diff --git a/token.cgi b/token.cgi
index c297495510b9d4d24b558f68cd1cbc950c4b3fd6..697da39b18a557a4a66d644112af195111c9637f 100755 (executable)
--- a/token.cgi
+++ b/token.cgi
@@ -201,7 +201,7 @@ sub changePassword {
     SendSQL("DELETE FROM tokens WHERE token = $::quotedtoken");
     SendSQL("UNLOCK TABLES");
 
-    InvalidateLogins($userid);
+    Bugzilla->logout_user_by_id($userid);
 
     $vars->{'message'} = "password_changed";
 

diff --git a/userprefs.cgi b/userprefs.cgi
index 5466c80cc4e1b625fee3dc2b1dc7e47ff715e66d..15afdb21c196de8fdc689490b9525b543a522473 100755 (executable)
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -26,6 +26,7 @@ use strict;
 use lib qw(.);
 
 use Bugzilla;
+use Bugzilla::Constants;
 
 require "CGI.pl";
 
@@ -108,8 +109,9 @@ sub SaveAccount {
             SendSQL("UPDATE profiles 
                      SET    cryptpassword = $cryptedpassword 
                      WHERE  userid = $userid");
+
             # Invalidate all logins except for the current one
-            InvalidateLogins($userid, $cgi->cookie("Bugzilla_logincookie"));
+            Bugzilla->logout(LOGOUT_KEEP_CURRENT);
         }
     }