Bug 95732 - remove logincookies.cryptpassword, and invalidate cookies from

CGI.pl

@@ -689,8 +689,7 @@ sub quietly_check_login() {
                 "profiles.login_name, " .
                 "profiles.login_name = " .
                 SqlQuote($::COOKIE{"Bugzilla_login"}) .
-                " AND profiles.cryptpassword = logincookies.cryptpassword " .
-                "AND logincookies.hostname = " .
+                " AND logincookies.hostname = " .
                 SqlQuote($ENV{"REMOTE_HOST"}) .
                 ", profiles.disabledtext " .
                 " FROM profiles, logincookies WHERE logincookies.cookie = " .
@@ -979,7 +978,7 @@ sub confirm_login {
        if (!defined $ENV{'REMOTE_HOST'}) {
          $ENV{'REMOTE_HOST'} = $ENV{'REMOTE_ADDR'};
        }
-       SendSQL("insert into logincookies (userid,cryptpassword,hostname) values (@{[DBNameToIdAndCheck($enteredlogin)]}, @{[SqlQuote($realcryptpwd)]}, @{[SqlQuote($ENV{'REMOTE_HOST'})]})");
+       SendSQL("insert into logincookies (userid,hostname) values (@{[DBNameToIdAndCheck($enteredlogin)]}, @{[SqlQuote($ENV{'REMOTE_HOST'})]})");
        SendSQL("select LAST_INSERT_ID()");
        my $logincookie = FetchOneColumn();
 

checksetup.pl

@@ -1082,7 +1082,6 @@ $table{groups} =
 $table{logincookies} =
    'cookie mediumint not null auto_increment primary key,
     userid mediumint not null,
-    cryptpassword varchar(34),
     hostname varchar(128),
     lastused timestamp,
 
@@ -2596,6 +2595,29 @@ AddField("bugs", "cclist_accessible", "tinyint not null default 1");
 # using the attachment manager can record changes to attachments.
 AddField("bugs_activity", "attach_id", "mediumint null");
 
+# 2001-01-17 bbaetz@student.usyd.edu.au bug 95732
+# Remove logincookies.cryptpassword, and delete entries which become
+# invalid
+if (GetFieldDef("logincookies", "cryptpassword")) {
+    # We need to delete any cookies which are invalid, before dropping the
+    # column
+
+    print "Removing invalid login cookies...\n";
+
+    # mysql doesn't support DELETE with multi-table queries, so we have
+    # to iterate
+    my $sth = $dbh->prepare("SELECT cookie FROM logincookies, profiles " .
+                            "WHERE logincookies.cryptpassword != " .
+                            "profiles.cryptpassword AND " .
+                            "logincookies.userid = profiles.userid");
+    $sth->execute();
+    while (my ($cookie) = $sth->fetchrow_array()) {
+        $dbh->do("DELETE FROM logincookies WHERE cookie = $cookie");
+    }
+
+    DropField("logincookies", "cryptpassword");
+}
+
 # If you had to change the --TABLE-- definition in any way, then add your
 # differential change code *** A B O V E *** this comment.
 #

editusers.cgi

@@ -808,6 +808,11 @@ if ($action eq 'update') {
             SendSQL("UPDATE  profiles
                      SET     cryptpassword = $cryptpassword
                      WHERE   login_name = $loginname");
+            SendSQL("SELECT userid
+                     FROM profiles
+                     WHERE login_name=" . SqlQuote($userold));
+            my $userid = FetchOneColumn();
+            InvalidateLogins($userid);
             print "Updated password.<BR>\n";
         } else {
             print "Did not update password: $passworderror<br>\n";
@@ -827,8 +832,7 @@ if ($action eq 'update') {
                  FROM profiles
                  WHERE login_name=" . SqlQuote($userold));
         my $userid = FetchOneColumn();
-        SendSQL("DELETE FROM logincookies
-                 WHERE userid=" . $userid);
+        InvalidateLogins($userid);
         print "Updated disabled text.<BR>\n";
     }
     if ($editall && $user ne $userold) {

globals.pl

@@ -706,6 +706,19 @@ 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) = @_;
 

relogin.cgi

@@ -29,6 +29,25 @@ use lib qw(.);
 
 require "CGI.pl";
 
+# We don't want to remove a random logincookie from the db, so
+# call quietly_check_login. If we're logged in after this, then
+# the logincookie must be correct
+
+ConnectToDatabase();
+quietly_check_login();
+
+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");
+}
+
 my $cookiepath = Param("cookiepath");
 print "Set-Cookie: Bugzilla_login= ; path=$cookiepath; expires=Sun, 30-Jun-80 00:00:00 GMT
 Set-Cookie: Bugzilla_logincookie= ; path=$cookiepath; expires=Sun, 30-Jun-80 00:00:00 GMT

token.cgi

@@ -227,6 +227,8 @@ sub changePassword {
     SendSQL("DELETE FROM tokens WHERE token = $::quotedtoken");
     SendSQL("UNLOCK TABLES");
 
+    InvalidateLogins($userid);
+
     # Return HTTP response headers.
     print "Content-Type: text/html\n\n";
 

userprefs.cgi

@@ -171,6 +171,8 @@ sub SaveAccount {
         SendSQL("UPDATE  profiles 
                  SET     cryptpassword = $cryptedpassword 
                  WHERE   userid = $userid");
+        # Invalidate all logins except for the current one
+        InvalidateLogins($userid, $::COOKIE{"Bugzilla_logincookie"});
     }
     SendSQL("UPDATE profiles SET " .
             "realname = " . SqlQuote(trim($::FORM{'realname'})) .