Bug 134022: PERFORMANCE: deleting old login cookies locks login checks

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=mkanat

diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm
index 74678afa8b77ba2c86ee6f9daece445569f69cbc..8e18f8699892667e85ad12c9748c4bc52a38672c 100644 (file)
--- a/Bugzilla/Auth.pm
+++ b/Bugzilla/Auth.pm
@@ -151,23 +151,17 @@ sub _handle_login_result {
         ThrowCodeError($result->{error}, $result->{details});
     }
     elsif ($fail_code == AUTH_NODATA) {
-        if ($login_type == LOGIN_REQUIRED) {
-            # This seems like as good as time as any to get rid of
-            # old crufty junk in the logincookies table.  Get rid
-            # of any entry that hasn't been used in a month.
-            $dbh->do("DELETE FROM logincookies WHERE " .
-                     $dbh->sql_to_days('NOW()') . " - " .
-                     $dbh->sql_to_days('lastused') . " > 30");
-            $self->{_info_getter}->fail_nodata($self);
-        }
-        # Otherwise, we just return the "default" user.
+        $self->{_info_getter}->fail_nodata($self) 
+            if $login_type == LOGIN_REQUIRED;
+
+        # If we're not LOGIN_REQUIRED, we just return the default user.
         $user = Bugzilla->user;
     }
     # The username/password may be wrong
     # Don't let the user know whether the username exists or whether
     # the password was just wrong. (This makes it harder for a cracker
     # to find account names by brute force)
-    elsif (($fail_code == AUTH_LOGINFAILED) || ($fail_code == AUTH_NO_SUCH_USER)) {
+    elsif ($fail_code == AUTH_LOGINFAILED or $fail_code == AUTH_NO_SUCH_USER) {
         ThrowUserError("invalid_username_or_password");
     }
     # The account may be disabled

diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm
index 9098f8989b70dc5ed65f116eeddd54a71956da01..420bad16b0683a014e6e44081a24471c0c168745 100644 (file)
--- a/Bugzilla/Auth/Persist/Cookie.pm
+++ b/Bugzilla/Auth/Persist/Cookie.pm
@@ -60,6 +60,8 @@ sub persist_login {
     # subsequent login
     trick_taint($ip_addr);
 
+    $dbh->bz_start_transaction();
+
     my $login_cookie = 
         Bugzilla::Token::GenerateUniqueToken('logincookies', 'cookie');
 
@@ -67,6 +69,13 @@ sub persist_login {
               VALUES (?, ?, ?, NOW())",
               undef, $login_cookie, $user->id, $ip_addr);
 
+    # Issuing a new cookie is a good time to clean up the old
+    # cookies.
+    $dbh->do("DELETE FROM logincookies WHERE lastused < LOCALTIMESTAMP(0) - "
+             . $dbh->sql_interval(MAX_LOGINCOOKIE_AGE, 'DAY'));
+
+    $dbh->bz_commit_transaction();
+
     # Prevent JavaScript from accessing login cookies.
     my %cookieargs = ('-httponly' => 1);
 

diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 1a54b5a13dcb33609e2e7f341ef5f46fc7305105..318b2c53b141fc87f378bc5857f70f539e3dc3b8 100644 (file)
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -140,6 +140,7 @@ use File::Basename;
     ON_WINDOWS
 
     MAX_TOKEN_AGE
+    MAX_LOGINCOOKIE_AGE
 
     SAFE_PROTOCOLS
 
@@ -353,6 +354,8 @@ use constant FIELD_TYPE_DATETIME  => 5;
 
 # The maximum number of days a token will remain valid.
 use constant MAX_TOKEN_AGE => 3;
+# How many days a logincookie will remain valid if not used.
+use constant MAX_LOGINCOOKIE_AGE => 30;
 
 # Protocols which are considered as safe.
 use constant SAFE_PROTOCOLS => ('afs', 'cid', 'ftp', 'gopher', 'http', 'https',