From: Dave Lawrence Date: Wed, 26 Feb 2014 16:33:43 +0000 (+0000) Subject: Bug 893195 - Allow token based authentication for webservices X-Git-Tag: bugzilla-4.4.3~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6ec36256c900e8a0c3e3f370814ad71e75da282d;p=thirdparty%2Fbugzilla.git Bug 893195 - Allow token based authentication for webservices r=glob,a=justdave --- diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 477dbffaa8..05238e0b3e 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -108,6 +108,15 @@ 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}; @@ -409,6 +418,14 @@ 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 290cb42ffd..f1aad4108c 100644 --- a/Bugzilla/Auth/Login.pm +++ b/Bugzilla/Auth/Login.pm @@ -8,7 +8,7 @@ package Bugzilla::Auth::Login; use strict; -use fields qw(); +use fields qw(_login_token); # 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 274e064ff1..2e31c19d57 100644 --- a/Bugzilla/Auth/Login/Cookie.pm +++ b/Bugzilla/Auth/Login/Cookie.pm @@ -17,7 +17,8 @@ use List::Util qw(first); use constant requires_persistence => 0; use constant requires_verification => 0; use constant can_login => 0; -use constant is_automatic => 1; + +sub is_automatic { return $_[0]->login_token ? 0 : 1; } # Note that Cookie never consults the Verifier, it always assumes # it has a valid DB account or it fails. @@ -25,24 +26,35 @@ sub get_login_info { my ($self) = @_; my $cgi = Bugzilla->cgi; my $dbh = Bugzilla->dbh; + my ($user_id, $login_cookie); - my $ip_addr = remote_ip(); - my $login_cookie = $cgi->cookie("Bugzilla_logincookie"); - my $user_id = $cgi->cookie("Bugzilla_login"); + if (!Bugzilla->request_cache->{auth_no_automatic_login}) { + $login_cookie = $cgi->cookie("Bugzilla_logincookie"); + $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; + # 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; + } } - unless ($user_id) { - my $cookie = first {$_->name eq 'Bugzilla_login'} - @{$cgi->{'Bugzilla_cookie_list'}}; - $user_id = $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'}); } + 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 @@ -75,4 +87,32 @@ 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 c9aaef8635..39dae147dd 100644 --- a/Bugzilla/Auth/Persist/Cookie.pm +++ b/Bugzilla/Auth/Persist/Cookie.pm @@ -13,6 +13,8 @@ use Bugzilla::Constants; use Bugzilla::Util; use Bugzilla::Token; +use Bugzilla::Auth::Login::Cookie qw(login_token); + use List::Util qw(first); sub new { @@ -84,6 +86,7 @@ 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; @@ -97,16 +100,24 @@ 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) { - $login_cookie = $cookie->value; + push(@login_cookies, $cookie->value); + } + elsif ($cookie = $cgi->cookie("Bugzilla_logincookie")) { + push(@login_cookies, $cookie); } - else { - $login_cookie = $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'}); } - trick_taint($login_cookie); + + # Make sure that @login_cookies is not empty to not break SQL statements. + push(@login_cookies, '') unless @login_cookies; # 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 @@ -115,12 +126,18 @@ 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 cookie != ? AND userid = ?", - undef, $login_cookie, $user->id); + $dbh->do("DELETE FROM logincookies WHERE " . + $dbh->sql_in('cookie', \@login_cookies, 1) . + " AND userid = ?", + undef, $user->id); } elsif ($type == LOGOUT_CURRENT) { - $dbh->do("DELETE FROM logincookies WHERE cookie = ? AND userid = ?", - undef, $login_cookie, $user->id); + $dbh->do("DELETE FROM logincookies WHERE " . + $dbh->sql_in('cookie', \@login_cookies) . + " AND userid = ?", + undef, $user->id); } else { die("Invalid type $type supplied to logout()"); } @@ -128,7 +145,6 @@ 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 804d7874ef..9f57104398 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); +use Bugzilla::WebService::Util qw(taint_data fix_credentials); use Bugzilla::Util qw(correct_urlbase trim disable_utf8); use HTTP::Message; @@ -349,6 +349,10 @@ 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 40cc6ec548..03590bd1cd 100644 --- a/Bugzilla/WebService/Server/XMLRPC.pm +++ b/Bugzilla/WebService/Server/XMLRPC.pm @@ -82,6 +82,7 @@ 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 { @@ -105,6 +106,11 @@ 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 491b4cd63d..771822e8c1 100644 --- a/Bugzilla/WebService/User.pm +++ b/Bugzilla/WebService/User.pm @@ -20,6 +20,8 @@ use Bugzilla::WebService::Util qw(filter validate translate params_to_objects); use List::Util qw(min); +use List::Util qw(first); + # Don't need auth to login use constant LOGIN_EXEMPT => { login => 1, @@ -74,14 +76,25 @@ sub login { $input_params->{'Bugzilla_password'} = $params->{password}; $input_params->{'Bugzilla_remember'} = $remember; - Bugzilla->login(); - return { id => $self->type('int', Bugzilla->user->id) }; + 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; } sub logout { my $self = shift; Bugzilla->logout; - return undef; } ################# @@ -439,10 +452,12 @@ management of cookies across sessions. =item B -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. +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. =item B diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index 6e935fe209..195de79e44 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -20,6 +20,7 @@ our @EXPORT_OK = qw( validate translate params_to_objects + fix_credentials ); sub filter ($$;$) { @@ -143,6 +144,22 @@ 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 @@ -205,3 +222,9 @@ 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.