From feeccb6a9e435932346d6a9ddeeb12c969362177 Mon Sep 17 00:00:00 2001 From: Dave Lawrence Date: Thu, 26 Sep 2013 11:06:28 -0400 Subject: Bug 917669 - invalid or expired authentication tokens and cookies should throw errors, not be silently ignored r/a=glob --- Bugzilla/Auth/Login/Cookie.pm | 21 +++++++++++++-------- Bugzilla/Template.pm | 5 +---- Bugzilla/Util.pm | 17 +++++++++++++++-- Bugzilla/WebService.pm | 7 +++++++ 4 files changed, 36 insertions(+), 14 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/Auth/Login/Cookie.pm b/Bugzilla/Auth/Login/Cookie.pm index 130fab8e3..3068331ea 100644 --- a/Bugzilla/Auth/Login/Cookie.pm +++ b/Bugzilla/Auth/Login/Cookie.pm @@ -14,6 +14,7 @@ use parent qw(Bugzilla::Auth::Login); use Bugzilla::Constants; use Bugzilla::Util; +use Bugzilla::Error; use List::Util qw(first); @@ -73,7 +74,9 @@ sub get_login_info { AND (ipaddr = ? OR ipaddr IS NULL)', undef, ($login_cookie, $user_id, $ip_addr)); - # If the cookie is valid, return a valid username. + # If the cookie or token is valid, return a valid username. + # If they were not valid and we are using a webservice, then + # throw an error notifying the client. if ($is_valid) { # If we logged in successfully, then update the lastused # time on the login cookie @@ -81,12 +84,16 @@ sub get_login_info { WHERE cookie = ?", undef, $login_cookie); return { user_id => $user_id }; } + elsif (i_am_webservice()) { + ThrowUserError('invalid_cookies_or_token'); + } } - # Either the he cookie is invalid, or we got no cookie. We don't want - # to ever return AUTH_LOGINFAILED, because we don't want Bugzilla to - # actually throw an error when it gets a bad cookie. It should just - # look like there was no cookie to begin with. + # Either the cookie or token is invalid and we are not authenticating + # via a webservice, or we did not receive a cookie or token. We don't + # want to ever return AUTH_LOGINFAILED, because we don't want Bugzilla to + # actually throw an error when it gets a bad cookie or token. It should just + # look like there was no cookie or token to begin with. return { failure => AUTH_NODATA }; } @@ -97,9 +104,7 @@ sub login_token { return $self->{'_login_token'} if exists $self->{'_login_token'}; - if ($usage_mode ne USAGE_MODE_XMLRPC - && $usage_mode ne USAGE_MODE_JSON - && $usage_mode ne USAGE_MODE_REST) { + if (!i_am_webservice()) { return $self->{'_login_token'} = undef; } diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index 4b227e9d2..e1f7f3b60 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -837,10 +837,7 @@ sub create { # (Wrapping the message in the WebService is unnecessary # and causes awkward things like \n's appearing in error # messages in JSON-RPC.) - unless (Bugzilla->usage_mode == USAGE_MODE_JSON - or Bugzilla->usage_mode == USAGE_MODE_XMLRPC - or Bugzilla->usage_mode == USAGE_MODE_REST) - { + unless (i_am_webservice()) { $var = wrap_comment($var, 72); } $var =~ s/\ / /g; diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index a3651182a..a9ff86c8b 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -14,8 +14,8 @@ use parent qw(Exporter); @Bugzilla::Util::EXPORT = qw(trick_taint detaint_natural detaint_signed html_quote url_quote xml_quote css_class_quote html_light_quote - i_am_cgi correct_urlbase remote_ip validate_ip - do_ssl_redirect_if_required use_attachbase + i_am_cgi i_am_webservice correct_urlbase remote_ip + validate_ip do_ssl_redirect_if_required use_attachbase diff_arrays on_main_db trim wrap_hard wrap_comment find_wrap_point format_time validate_date validate_time datetime_from @@ -237,6 +237,13 @@ sub i_am_cgi { return exists $ENV{'SERVER_SOFTWARE'} ? 1 : 0; } +sub i_am_webservice { + my $usage_mode = Bugzilla->usage_mode; + return $usage_mode == USAGE_MODE_XMLRPC + || $usage_mode == USAGE_MODE_JSON + || $usage_mode == USAGE_MODE_REST; +} + # This exists as a separate function from Bugzilla::CGI::redirect_to_https # because we don't want to create a CGI object during XML-RPC calls # (doing so can mess up XML-RPC). @@ -847,6 +854,7 @@ Bugzilla::Util - Generic utility functions for bugzilla # Functions that tell you about your environment my $is_cgi = i_am_cgi(); + my $is_webservice = i_am_webservice(); my $urlbase = correct_urlbase(); # Data manipulation @@ -974,6 +982,11 @@ Tells you whether or not you are being run as a CGI script in a web server. For example, it would return false if the caller is running in a command-line script. +=item C + +Tells you whether or not the current usage mode is WebServices related +such as JSONRPC, XMLRPC, or REST. + =item C Returns either the C or C parameter, depending on the diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm index 03548d257..92ffed659 100644 --- a/Bugzilla/WebService.pm +++ b/Bugzilla/WebService.pm @@ -181,6 +181,13 @@ For REST, you may also use the C and C variable names instead of C and C as a convenience. +=item B + +An error is now thrown if you pass invalid cookies or an invalid token. +You will need to log in again to get new cookies or a new token. Previous +releases simply ignored invalid cookies and token support was added in +Bugzilla B<5.0>. + =back =head1 STABLE, EXPERIMENTAL, and UNSTABLE -- cgit v1.2.3-24-g4f1b