From: Dave Lawrence Date: Tue, 25 Feb 2014 23:21:14 +0000 (+0000) Subject: Backout Bug 893195 - Allow token based authentication for webservices X-Git-Tag: bugzilla-4.4.3~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=87064aeff3770518cb68518ac9786ef95742e933;p=thirdparty%2Fbugzilla.git Backout Bug 893195 - Allow token based authentication for webservices --- diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 05238e0b3e..477dbffaa8 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -108,15 +108,6 @@ sub can_logout { return $getter->can_logout; } -sub login_token { - my ($self) = @_; - my $getter = $self->{_info_getter}->{successful}; - if ($getter && $getter->isa('Bugzilla::Auth::Login::Cookie')) { - return $getter->login_token; - } - return undef; -} - sub user_can_create_account { my ($self) = @_; my $verifier = $self->{_verifier}->{successful}; @@ -418,14 +409,6 @@ Params: None Returns: C if users can change their own email address, C otherwise. -=item C - -Description: If a login token was used instead of a cookie then this - will return the current login token data such as user id - and the token itself. -Params: None -Returns: A hash containing C and C. - =back =head1 STRUCTURE diff --git a/Bugzilla/Auth/Login.pm b/Bugzilla/Auth/Login.pm index f1aad4108c..290cb42ffd 100644 --- a/Bugzilla/Auth/Login.pm +++ b/Bugzilla/Auth/Login.pm @@ -8,7 +8,7 @@ package Bugzilla::Auth::Login; use strict; -use fields qw(_login_token); +use fields qw(); # Determines whether or not a user can logout. It's really a subroutine, # but we implement it here as a constant. Override it in subclasses if diff --git a/Bugzilla/Auth/Login/Cookie.pm b/Bugzilla/Auth/Login/Cookie.pm index 2e31c19d57..274e064ff1 100644 --- a/Bugzilla/Auth/Login/Cookie.pm +++ b/Bugzilla/Auth/Login/Cookie.pm @@ -17,8 +17,7 @@ use List::Util qw(first); use constant requires_persistence => 0; use constant requires_verification => 0; use constant can_login => 0; - -sub is_automatic { return $_[0]->login_token ? 0 : 1; } +use constant is_automatic => 1; # Note that Cookie never consults the Verifier, it always assumes # it has a valid DB account or it fails. @@ -26,35 +25,24 @@ sub get_login_info { my ($self) = @_; my $cgi = Bugzilla->cgi; my $dbh = Bugzilla->dbh; - my ($user_id, $login_cookie); - if (!Bugzilla->request_cache->{auth_no_automatic_login}) { - $login_cookie = $cgi->cookie("Bugzilla_logincookie"); - $user_id = $cgi->cookie("Bugzilla_login"); + my $ip_addr = remote_ip(); + my $login_cookie = $cgi->cookie("Bugzilla_logincookie"); + my $user_id = $cgi->cookie("Bugzilla_login"); - # If cookies cannot be found, this could mean that they haven't - # been made available yet. In this case, look at Bugzilla_cookie_list. - unless ($login_cookie) { - my $cookie = first {$_->name eq 'Bugzilla_logincookie'} - @{$cgi->{'Bugzilla_cookie_list'}}; - $login_cookie = $cookie->value if $cookie; - } - unless ($user_id) { - my $cookie = first {$_->name eq 'Bugzilla_login'} - @{$cgi->{'Bugzilla_cookie_list'}}; - $user_id = $cookie->value if $cookie; - } + # If cookies cannot be found, this could mean that they haven't + # been made available yet. In this case, look at Bugzilla_cookie_list. + unless ($login_cookie) { + my $cookie = first {$_->name eq 'Bugzilla_logincookie'} + @{$cgi->{'Bugzilla_cookie_list'}}; + $login_cookie = $cookie->value if $cookie; } - - # If no cookies were provided, we also look for a login token - # passed in the parameters of a webservice - my $token = $self->login_token; - if ($token && (!$login_cookie || !$user_id)) { - ($user_id, $login_cookie) = ($token->{'user_id'}, $token->{'login_token'}); + unless ($user_id) { + my $cookie = first {$_->name eq 'Bugzilla_login'} + @{$cgi->{'Bugzilla_cookie_list'}}; + $user_id = $cookie->value if $cookie; } - my $ip_addr = remote_ip(); - if ($login_cookie && $user_id) { # Anything goes for these params - they're just strings which # we're going to verify against the db @@ -87,32 +75,4 @@ sub get_login_info { return { failure => AUTH_NODATA }; } -sub login_token { - my ($self) = @_; - my $input = Bugzilla->input_params; - my $usage_mode = Bugzilla->usage_mode; - - return $self->{'_login_token'} if exists $self->{'_login_token'}; - - if ($usage_mode ne USAGE_MODE_XMLRPC - && $usage_mode ne USAGE_MODE_JSON) - { - return $self->{'_login_token'} = undef; - } - - # Check if a token was passed in via requests for WebServices - my $token = trim(delete $input->{'Bugzilla_token'}); - return $self->{'_login_token'} = undef if !$token; - - my ($user_id, $login_token) = split('-', $token, 2); - if (!detaint_natural($user_id) || !$login_token) { - return $self->{'_login_token'} = undef; - } - - return $self->{'_login_token'} = { - user_id => $user_id, - login_token => $login_token - }; -} - 1; diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm index 42d4d6028c..c9aaef8635 100644 --- a/Bugzilla/Auth/Persist/Cookie.pm +++ b/Bugzilla/Auth/Persist/Cookie.pm @@ -13,8 +13,6 @@ use Bugzilla::Constants; use Bugzilla::Util; use Bugzilla::Token; -use Bugzilla::Auth::Login::Cookie qw(login_token); - use List::Util qw(first); sub new { @@ -86,7 +84,6 @@ sub logout { my $dbh = Bugzilla->dbh; my $cgi = Bugzilla->cgi; - my $input = Bugzilla->input_params; $param = {} unless $param; my $user = $param->{user} || Bugzilla->user; my $type = $param->{type} || LOGOUT_ALL; @@ -100,23 +97,16 @@ sub logout { # The LOGOUT_*_CURRENT options require the current login cookie. # If a new cookie has been issued during this run, that's the current one. # If not, it's the one we've received. - my @login_cookies; my $cookie = first {$_->name eq 'Bugzilla_logincookie'} @{$cgi->{'Bugzilla_cookie_list'}}; + my $login_cookie; if ($cookie) { - push(@login_cookies, $cookie->value); + $login_cookie = $cookie->value; } else { - push(@login_cookies, $cgi->cookie("Bugzilla_logincookie")); - } - - # If we are a webservice using a token instead of cookie - # then add that as well to the login cookies to delete - if (my $login_token = $user->authorizer->login_token) { - push(@login_cookies, $login_token->{'login_token'}); + $login_cookie = $cgi->cookie("Bugzilla_logincookie") || ''; } - - return if !@login_cookies; + trick_taint($login_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 @@ -125,18 +115,12 @@ sub logout { # 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 - map { trick_taint($_) } @login_cookies; - @login_cookies = map { $dbh->quote($_) } @login_cookies; if ($type == LOGOUT_KEEP_CURRENT) { - $dbh->do("DELETE FROM logincookies WHERE " . - $dbh->sql_in('cookie', \@login_cookies, 1) . - " AND userid = ?", - undef, $user->id); + $dbh->do("DELETE FROM logincookies WHERE cookie != ? AND userid = ?", + undef, $login_cookie, $user->id); } elsif ($type == LOGOUT_CURRENT) { - $dbh->do("DELETE FROM logincookies WHERE " . - $dbh->sql_in('cookie', \@login_cookies) . - " AND userid = ?", - undef, $user->id); + $dbh->do("DELETE FROM logincookies WHERE cookie = ? AND userid = ?", + undef, $login_cookie, $user->id); } else { die("Invalid type $type supplied to logout()"); } @@ -144,6 +128,7 @@ sub logout { if ($type != LOGOUT_KEEP_CURRENT) { clear_browser_cookies(); } + } sub clear_browser_cookies { diff --git a/Bugzilla/WebService/Server/JSONRPC.pm b/Bugzilla/WebService/Server/JSONRPC.pm index 9f57104398..804d7874ef 100644 --- a/Bugzilla/WebService/Server/JSONRPC.pm +++ b/Bugzilla/WebService/Server/JSONRPC.pm @@ -23,7 +23,7 @@ BEGIN { use Bugzilla::Error; use Bugzilla::WebService::Constants; -use Bugzilla::WebService::Util qw(taint_data fix_credentials); +use Bugzilla::WebService::Util qw(taint_data); use Bugzilla::Util qw(correct_urlbase trim disable_utf8); use HTTP::Message; @@ -349,10 +349,6 @@ sub _argument_type_check { } } - # Update the params to allow for several convenience key/values - # use for authentication - fix_credentials($params); - Bugzilla->input_params($params); if ($self->request->method eq 'POST') { diff --git a/Bugzilla/WebService/Server/XMLRPC.pm b/Bugzilla/WebService/Server/XMLRPC.pm index 03590bd1cd..40cc6ec548 100644 --- a/Bugzilla/WebService/Server/XMLRPC.pm +++ b/Bugzilla/WebService/Server/XMLRPC.pm @@ -82,7 +82,6 @@ our @ISA = qw(XMLRPC::Deserializer); use Bugzilla::Error; use Bugzilla::WebService::Constants qw(XMLRPC_CONTENT_TYPE_WHITELIST); -use Bugzilla::WebService::Util qw(fix_credentials); use Scalar::Util qw(tainted); sub deserialize { @@ -106,11 +105,6 @@ sub deserialize { my $params = $som->paramsin; # This allows positional parameters for Testopia. $params = {} if ref $params ne 'HASH'; - - # Update the params to allow for several convenience key/values - # use for authentication - fix_credentials($params); - Bugzilla->input_params($params); return $som; } diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm index 238ab4dc54..491b4cd63d 100644 --- a/Bugzilla/WebService/User.pm +++ b/Bugzilla/WebService/User.pm @@ -18,7 +18,7 @@ use Bugzilla::User; use Bugzilla::Util qw(trim detaint_natural); use Bugzilla::WebService::Util qw(filter validate translate params_to_objects); -use List::Util qw(first min); +use List::Util qw(min); # Don't need auth to login use constant LOGIN_EXEMPT => { @@ -74,25 +74,14 @@ sub login { $input_params->{'Bugzilla_password'} = $params->{password}; $input_params->{'Bugzilla_remember'} = $remember; - my $user = Bugzilla->login(); - - my $result = { id => $self->type('int', $user->id) }; - - # We will use the stored cookie value combined with the user id - # to create a token that can be used with future requests in the - # query parameters - my $login_cookie = first { $_->name eq 'Bugzilla_logincookie' } - @{ Bugzilla->cgi->{'Bugzilla_cookie_list'} }; - if ($login_cookie) { - $result->{'token'} = $user->id . "-" . $login_cookie->value; - } - - return $result; + Bugzilla->login(); + return { id => $self->type('int', Bugzilla->user->id) }; } sub logout { my $self = shift; Bugzilla->logout; + return undef; } ################# @@ -450,12 +439,10 @@ management of cookies across sessions. =item B -On success, a hash containing two items, C, the numeric id of the -user that was logged in, and a C which can be passed in -the parameters as authentication in other calls. A set of http cookies -is also sent with the response. These cookies *or* the token can be sent -along with any future requests to the webservice, for the duration of the -session. +On success, a hash containing one item, C, the numeric id of the +user that was logged in. A set of http cookies is also sent with the +response. These cookies must be sent along with any future requests +to the webservice, for the duration of the session. =item B diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index 195de79e44..6e935fe209 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -20,7 +20,6 @@ our @EXPORT_OK = qw( validate translate params_to_objects - fix_credentials ); sub filter ($$;$) { @@ -144,22 +143,6 @@ sub params_to_objects { return \@objects; } -sub fix_credentials { - my ($params) = @_; - # Allow user to pass in login=foo&password=bar as a convenience - # even if not calling User.login. We also do not delete them as - # User.login requires "login" and "password". - if (exists $params->{'login'} && exists $params->{'password'}) { - $params->{'Bugzilla_login'} = $params->{'login'}; - $params->{'Bugzilla_password'} = $params->{'password'}; - } - # Allow user to pass token=12345678 as a convenience which becomes - # "Bugzilla_token" which is what the auth code looks for. - if (exists $params->{'token'}) { - $params->{'Bugzilla_token'} = $params->{'token'}; - } -} - __END__ =head1 NAME @@ -222,9 +205,3 @@ parameters passed to a WebService method (the first parameter to this function). Helps make life simpler for WebService methods that internally create objects via both "ids" and "names" fields. Also de-duplicates objects that were loaded by both "ids" and "names". Returns an arrayref of objects. - -=head2 fix_credentials - -Allows for certain parameters related to authentication such as Bugzilla_login, -Bugzilla_password, and Bugzilla_token to have shorter named equivalents passed in. -This function converts the shorter versions to their respective internal names.