Fix for bug 226754: Move InvalidateLogins into Bugzilla::Auth::CGI. Conso...

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/Auth/CGI.pm b/Bugzilla/Auth/CGI.pm
index c453f2dcd3b3d5e0d7bd0c1acb9ec496ff3b559d..471e538e944a54970e9d5735de47256fca36d83d 100644 (file)
--- a/Bugzilla/Auth/CGI.pm
+++ b/Bugzilla/Auth/CGI.pm
@@ -92,10 +92,6 @@ sub login {
                               -value => $logincookie);
 
         }
-
-        # compat code. The cookie value is used for logouts, and that
-        # isn't generic yet.
-        $::COOKIE{'Bugzilla_logincookie'} = $logincookie;
     }
     elsif ($authres == AUTH_NODATA) {
         # No data from the form, so try to login via cookies
@@ -184,29 +180,46 @@ sub login {
     }
 
     # If we get here, then we've run out of options, which shouldn't happen
-    ThrowCodeError("authres_unhandled",
-                   { authres => $authres,
-                     type => $type,
-                   }
-                  );
-
+    ThrowCodeError("authres_unhandled", { authres => $authres, 
+                                          type => $type, });
 }
 
+# Logs user out, according to the option provided; this consists of
+# removing entries from logincookies for the specified $user.
 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 ($class, $user, $option) = @_;
+    my $dbh = Bugzilla->dbh;
+    $option = LOGOUT_ALL unless defined $option;
+
+    if ($option == LOGOUT_ALL) {
+            $dbh->do("DELETE FROM logincookies WHERE userid = ?",
+                     undef, $user->id);
+            return;
     }
 
+    # The LOGOUT_*_CURRENT options require a cookie 
+    my $cookie = Bugzilla->cgi->cookie("Bugzilla_logincookie");
+    detaint_natural($cookie);
+
+    # These queries use both the cookie ID and the user ID as keys. 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 simultaneously, 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
+    if ($option == LOGOUT_KEEP_CURRENT) {
+        $dbh->do("DELETE FROM logincookies WHERE cookie != ? AND userid = ?",
+                 undef, $cookie, $user->id);
+    } elsif ($option == LOGOUT_CURRENT) {
+        $dbh->do("DELETE FROM logincookies WHERE cookie = ? AND userid = ?",
+                 undef, $cookie, $user->id);
+    } else {
+        die("Invalid option $option supplied to logout()");
+  }
+}
+
+sub clear_browser_cookies {
     my $cgi = Bugzilla->cgi;
     $cgi->send_cookie(-name => "Bugzilla_login",
                       -expires => "Tue, 15-Sep-1998 21:49:00 GMT");
@@ -235,9 +248,6 @@ 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/Bugzilla/Auth/Cookie.pm b/Bugzilla/Auth/Cookie.pm
index 7dd2967fb7a95bea3d841f343edfff03f04c79a9..b50acbe242f68668d8b9d16246b90ba8e93655e4 100644 (file)
--- a/Bugzilla/Auth/Cookie.pm
+++ b/Bugzilla/Auth/Cookie.pm
@@ -80,10 +80,6 @@ sub authenticate {
                  undef,
                  $login_cookie);
 
-        # compat code. The cookie value is used for logouts, and that
-        # isn't generic yet. Detaint it so that its usable
-        detaint_natural($::COOKIE{'Bugzilla_logincookie'});
-
         return (AUTH_OK, $userid);
     }
 

diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index a61cb46206c84a6fe370be211e57ab383a35d88e..1ccde6b990cabfced778ed228e0b96faf31e6b6a 100644 (file)
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -46,6 +46,10 @@ use base qw(Exporter);
     LOGIN_OPTIONAL
     LOGIN_NORMAL
     LOGIN_REQUIRED
+
+    LOGOUT_ALL
+    LOGOUT_CURRENT
+    LOGOUT_KEEP_CURRENT
 );
 
 @Bugzilla::Constants::EXPORT_OK = qw(contenttypes);
@@ -83,7 +87,7 @@ use constant CONTROLMAPSHOWN => 1;
 use constant CONTROLMAPDEFAULT => 2;
 use constant CONTROLMAPMANDATORY => 3;
 
-# See Bugzilla::Auth for docs for these
+# See Bugzilla::Auth for docs on AUTH_*, LOGIN_* and LOGOUT_*
 
 use constant AUTH_OK => 0;
 use constant AUTH_NODATA => 1;
@@ -95,6 +99,10 @@ use constant LOGIN_OPTIONAL => 0;
 use constant LOGIN_NORMAL => 1;
 use constant LOGIN_REQUIRED => 2;
 
+use constant LOGOUT_ALL => 0;
+use constant LOGOUT_CURRENT => 1;
+use constant LOGOUT_KEEP_CURRENT => 2;
+
 use constant contenttypes =>
   {
    "html" => "text/html" ,