From 8a06f991edf359f9ffeb53cc57173023c44d9644 Mon Sep 17 00:00:00 2001 From: "kiko%async.com.br" <> Date: Sat, 27 Mar 2004 09:28:29 +0000 Subject: 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. --- Bugzilla.pm | 72 +++++++++++++++++++++++++---------- Bugzilla/Auth.pm | 114 ++++++++++++++++++++++++++++++++++++------------------- editusers.cgi | 8 ++-- globals.pl | 13 ------- token.cgi | 2 +- userprefs.cgi | 4 +- 6 files changed, 135 insertions(+), 78 deletions(-) diff --git a/Bugzilla.pm b/Bugzilla.pm index 7e7d50004..5cee520c7 100644 --- 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 -The current L. C if there is no currently logged in user +The current C. C if there is no currently logged in user or if the login code has not yet been run. =item C @@ -273,15 +293,29 @@ Logs in a user, returning a C object, or C if there is no logged in user. See L and L. -=item C +=item C + +Logs out the current user, which involves invalidating user sessions and +cookies. Three options are available from +L: LOGOUT_CURRENT (the +default), LOGOUT_ALL or LOGOUT_KEEP_CURRENT. + +=item C + +Logs out the specified user (invalidating all his sessions), taking a +Bugzilla::User instance. + +=item C -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 -Essentially, causes calls to C to return C. This has the +Essentially, causes calls to Cuser> to return C. 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 diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 22c2fc561..dcea8189a 100644 --- 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 -then use those methods to do the authentication. +The source modules (currently, only +L) then use those methods to do +the authentication. I itself inherits from the default authentication handler, identified by the I param. @@ -96,16 +99,16 @@ authentication or login modules. =item C Given an ip address, this returns the associated network address, using -C 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 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 -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 and described below. The rest of -the return values are status code-specific and are explained in the status -code descriptions. +L 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 -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 @@ -162,41 +167,52 @@ information. =item C -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 field in the C 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 field in the C table. +Note that this argument is a string, not a tag. =back =item C This determines if the user's account details can be modified. If this -method returns a C value, then accounts can be created and modified -through the Bugzilla user interface. Forgotten passwords can also be -retrieved through the L. +method returns a C value, then accounts can be created and +modified through the Bugzilla user interface. Forgotten passwords can +also be retrieved through the L. =back =head1 LOGINS -A login module can be used to try to log in a Bugzilla user in a particular -way. For example, L logs in users -from CGI scripts, first by trying database authentication against the -Bugzilla C 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 +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, which takes a C<$type> -argument, using constants found in C. +The login interface consists of the following methods: + +=item C, which takes a C<$type> argument, using constants found in +C. + +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 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 include the following: =over 4 =item C -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 @@ -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 on failure. +=item C, which takes a C argument for the user +being logged out, and an C<$option> argument. Possible values for +C<$option> include: + +=over 4 + +=item C -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 + +Log out the user, and invalidate all sessions the user has registered in +Bugzilla. + +=item C + +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, L, L + diff --git a/editusers.cgi b/editusers.cgi index 411704dce..e7ef0e7d3 100755 --- 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.
\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.
\n"; } else { print "Did not update password: $passworderror
\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.
\n"; } if ($editall && $user ne $userold) { diff --git a/globals.pl b/globals.pl index aef84f2a6..2b68b2b41 100644 --- 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 c29749551..697da39b1 100755 --- 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 5466c80cc..15afdb21c 100755 --- 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); } } -- cgit v1.2.3-24-g4f1b