From 5c76819f088805d6a3b483b00c34850eb766025a Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Tue, 20 Jan 2009 20:09:46 +0000 Subject: Bug 134022: PERFORMANCE: deleting old login cookies locks login checks Patch By Max Kanat-Alexander r=LpSolit, a=mkanat --- Bugzilla/Auth.pm | 16 +++++----------- Bugzilla/Auth/Persist/Cookie.pm | 9 +++++++++ Bugzilla/Constants.pm | 3 +++ 3 files changed, 17 insertions(+), 11 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 74678afa8..8e18f8699 100644 --- 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 9098f8989..420bad16b 100644 --- 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 921c03275..d93f91271 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -142,6 +142,7 @@ use File::Basename; ON_WINDOWS MAX_TOKEN_AGE + MAX_LOGINCOOKIE_AGE SAFE_PROTOCOLS @@ -363,6 +364,8 @@ use constant FIELD_TYPE_BUG_ID => 6; # 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', -- cgit v1.2.3-24-g4f1b