Bug 226324: Move relogin.cgi code to Bugzilla::Auth::CGI. Provide a

logout() method that is proxied through Bugzilla.pm's logout(), and fix
callers to use it.  r=justdave, bbaetz, a=justdave

5 files changed, 48 insertions, 37 deletions

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 871b76a54..7e7d50004 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -99,8 +99,17 @@ sub login {
 }
 
 sub logout {
+    use Bugzilla::Auth::CGI;
+    # remove cookies and clean up database state
+    Bugzilla::Auth::CGI->logout();
+    logout_request();
+}
+
+sub logout_request {
     undef $_user;
     $::userid = 0;
+    delete $::COOKIE{"Bugzilla_login"};
+    delete $::COOKIE{"Bugzilla_logincookie"};
 }
 
 my $_dbh;
@@ -266,10 +275,13 @@ L<Bugzilla::User|Bugzilla::User>.
 
 =item C<logout>
 
-Logs out the current user. For the moment, this will just cause calls to
-C<user> to return C<undef>. Eventually this will handle deleting cookies from
-the browser and values from the database, which is currently all handled
-by C<relogin.cgi>.
+Logs out the current user.
+
+=item C<logout_request>
+
+Essentially, causes calls to C<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. 
 
 =item C<dbh>
 
diff --git a/Bugzilla/Auth/CGI.pm b/Bugzilla/Auth/CGI.pm
index e223c9fee..afbf98b27 100644
--- a/Bugzilla/Auth/CGI.pm
+++ b/Bugzilla/Auth/CGI.pm
@@ -177,6 +177,28 @@ sub login {
 
 }
 
+sub logout {
+    my ($class, $user) = @_;
+
+    if ($user) {
+        # Even though we know the userid must match, we still check it in the
+        # SQL as a sanity check, since there is no locking here, and if
+        # the user logged out from two machines simulataniously, while someone
+        # else logged in and got the same cookie, we could be logging the
+        # other user out here. Yes, this is very very very unlikely, but why
+        # take chances? - bbaetz
+        my $dbh = Bugzilla->dbh;
+        $dbh->do("DELETE FROM logincookies WHERE cookie = ? AND userid = ?",
+                 undef, $::COOKIE{"Bugzilla_logincookie"}, $user->id);
+    }
+
+    my $cgi = Bugzilla->cgi;
+    $cgi->send_cookie(-name => "Bugzilla_login",
+                      -expires => "Tue, 15-Sep-1998 21:49:00 GMT");
+    $cgi->send_cookie(-name => "Bugzilla_logincookie",
+                      -expires => "Tue, 15-Sep-1998 21:49:00 GMT");
+}
+
 1;
 
 __END__
@@ -188,7 +210,7 @@ Bugzilla::Auth::CGI - CGI-based logins for Bugzilla
 =head1 SUMMARY
 
 This is a L<login module|Bugzilla::Auth/"LOGIN"> for Bugzilla. Users connecting
-from a CGI script use this module to authenticate.
+from a CGI script use this module to authenticate. Logouts are also handled here.
 
 =head1 BEHAVIOUR
 
@@ -198,6 +220,9 @@ 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>.
 
+When a logout is performed, we take care of removing the relevant
+logincookie database entry and effectively deleting the client cookie.
+
 =head1 SEE ALSO
 
 L<Bugzilla::Auth>
diff --git a/buglist.cgi b/buglist.cgi
index 246bf64c9..b822968c3 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -95,7 +95,7 @@ if ($::FORM{'format'} && $::FORM{'format'} eq "rdf" && !$::FORM{'ctype'}) {
 # Note that if and when this call clears cookies or has other persistent 
 # effects, we'll need to do this another way instead.
 if ((exists $::FORM{'ctype'}) && ($::FORM{'ctype'} eq "js")) {
-    Bugzilla->logout();
+    Bugzilla->logout_request();
 }
 
 # Determine the format in which the user would like to receive the output.
diff --git a/createaccount.cgi b/createaccount.cgi
index cce598ac9..22b8129e9 100755
--- a/createaccount.cgi
+++ b/createaccount.cgi
@@ -47,15 +47,11 @@ unless (Bugzilla::Auth->can_edit) {
   ThrowUserError("auth_cant_create_account");
 }
 
-my $cgi = Bugzilla->cgi;
-
 # Clear out the login cookies.  Make people log in again if they create an
 # account; otherwise, they'll probably get confused.
-$cgi->send_cookie(-name => 'Bugzilla_login',
-                  -expires => 'Tue, 15-Sep-1998 21:49:00 GMT');
-$cgi->send_cookie(-name => 'Bugzilla_logincookie',
-                  -expires => 'Tue, 15-Sep-1998 21:49:00 GMT');
+Bugzilla->logout();
 
+my $cgi = Bugzilla->cgi;
 print $cgi->header();
 
 my $login = $::FORM{'login'};
diff --git a/relogin.cgi b/relogin.cgi
index 65cb07b25..b7ba4f61e 100755
--- a/relogin.cgi
+++ b/relogin.cgi
@@ -23,7 +23,6 @@
 
 use strict;
 
-use vars %::COOKIE;
 use vars qw($template $vars);
 
 use lib qw(.);
@@ -37,33 +36,12 @@ require "CGI.pl";
 ConnectToDatabase();
 quietly_check_login();
 
-my $cgi = Bugzilla->cgi;
-
-if ($::userid) {
-    # Even though we know the userid must match, we still check it in the
-    # SQL as a sanity check, since there is no locking here, and if
-    # the user logged out from two machines simulataniously, while someone
-    # else logged in and got the same cookie, we could be logging the
-    # other user out here. Yes, this is very very very unlikely, but why
-    # take chances? - bbaetz
-    SendSQL("DELETE FROM logincookies WHERE cookie = " .
-            SqlQuote($::COOKIE{"Bugzilla_logincookie"}) .
-            "AND userid = $::userid");
-}
+Bugzilla->logout();
 
-$cgi->send_cookie(-name => "Bugzilla_login",
-                  -expires => "Tue, 15-Sep-1998 21:49:00 GMT");
-$cgi->send_cookie(-name => "Bugzilla_logincookie",
-                  -expires => "Tue, 15-Sep-1998 21:49:00 GMT");
-
-delete $::COOKIE{"Bugzilla_login"};
+my $cgi = Bugzilla->cgi;
+print $cgi->header();
 
 $vars->{'message'} = "logged_out";
-
-# This entire script should eventually just become a call to Bugzilla->logout
-Bugzilla->logout;
-
-print $cgi->header();
 $template->process("global/message.html.tmpl", $vars)
   || ThrowTemplateError($template->error());