Bug 893195 - Allow token based authentication for webservices

Bugzilla/Auth.pm

@@ -109,6 +109,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};
@@ -410,6 +419,14 @@ Params:      None
 Returns:     C<true> if users can change their own email address,
              C<false> otherwise.
 
+=item C<login_token>
+
+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<login_token> and C<user_id>.
+
 =back
 
 =head1 STRUCTURE

Bugzilla/Auth/Login.pm

@@ -9,7 +9,7 @@ package Bugzilla::Auth::Login;
 
 use 5.10.1;
 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

Bugzilla/Auth/Login/Cookie.pm

@@ -20,7 +20,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.
@@ -28,10 +29,11 @@ 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.
@@ -45,6 +47,16 @@ sub get_login_info {
                                 @{$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
@@ -78,4 +90,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
+        && $usage_mode ne USAGE_MODE_REST) {
+        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;

Bugzilla/Auth/Persist/Cookie.pm

@@ -15,6 +15,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 {
@@ -86,6 +88,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;
@@ -99,16 +102,23 @@ 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);
     }
     else {
-        $login_cookie = $cgi->cookie("Bugzilla_logincookie");
+        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'});
     }
-    trick_taint($login_cookie);
+
+    return if !@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
@@ -117,12 +127,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()");
     }

Bugzilla/WebService/Server/JSONRPC.pm

@@ -25,7 +25,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;
 
 use HTTP::Message;
@@ -373,6 +373,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') {

Bugzilla/WebService/Server/REST.pm

@@ -16,7 +16,7 @@ use Bugzilla;
 use Bugzilla::Constants;
 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 html_quote);
 
 # Load resource modules
@@ -69,7 +69,7 @@ sub handle {
 
     my $params = $self->_retrieve_json_params;
 
-    $self->_fix_credentials($params);
+    fix_credentials($params);
 
     # Fix includes/excludes for each call
     rest_include_exclude($params);
@@ -131,7 +131,7 @@ sub response {
 
     # If accessing through web browser, then display in readable format
     if ($self->content_type eq 'text/html') {
-        $result = $self->json->pretty->canonical->encode($result);
+        $result = $self->json->pretty->canonical->allow_nonref->encode($result);
 
         my $template = Bugzilla->template;
         $content = "";
@@ -162,8 +162,15 @@ sub handle_login {
     # explicitly gives that site their username and password. (This is
     # particularly important for JSONP, which would allow a remote site
     # to use private data without the user's knowledge, unless we had this
-    # protection in place.)
-    if (!grep($_ eq $self->request->method, ('POST', 'PUT'))) {
+    # protection in place.) We do allow this for GET /login as we need to
+    # for Bugzilla::Auth::Persist::Cookie to create a login cookie that we
+    # can also use for Bugzilla_token support. This is OK as it requires
+    # a login and password to be supplied and will fail if they are not
+    # valid for the user.
+    if (!grep($_ eq $self->request->method, ('POST', 'PUT'))
+        && !($self->bz_class_name eq 'Bugzilla::WebService::User'
+            && $self->bz_method_name eq 'login'))
+    {
         # XXX There's no particularly good way for us to get a parameter
         # to Bugzilla->login at this point, so we pass this information
         # around using request_cache, which is a bit of a hack. The
@@ -424,15 +431,6 @@ sub _find_resource {
     return $handler_found;
 }
 
-sub _fix_credentials {
-    my ($self, $params) = @_;
-    # Allow user to pass in &username=foo&password=bar
-    if (exists $params->{'username'} && exists $params->{'password'}) {
-        $params->{'Bugzilla_login'}    = delete $params->{'username'};
-        $params->{'Bugzilla_password'} = delete $params->{'password'};
-    }
-}
-
 sub _best_content_type {
     my ($self, @types) = @_;
     return ($self->_simple_content_negotiation(@types))[0] || '*/*';
@@ -545,15 +543,23 @@ if you have a Bugzilla account by providing your login credentials.
 
 =over
 
-=item Username and password
+=item Login name and password
 
 Pass in as query parameters of any request:
 
-username=fred@bedrock.com&password=ilovewilma
+login=fred@example.com&password=ilovecheese
 
 Remember to URL encode any special characters, which are often seen in passwords and to
 also enable SSL support.
 
+=item Login token
+
+By calling GET /login?login=fred@example.com&password=ilovecheese, you get back
+a C<token> value which can then be passed to each subsequent call as
+authentication. This is useful for third party clients that cannot use cookies
+and do not want to store a user's login and password in the client. You can also
+pass in "token" as a convenience.
+
 =back
 
 =head1 ERRORS

Bugzilla/WebService/Server/REST/Resources/User.pm

@@ -19,6 +19,16 @@ BEGIN {
 
 sub _rest_resources {
     my $rest_resources = [
+        qr{^/login$}, {
+            GET => {
+                method => 'login'
+            }
+        },
+        qr{^/logout$}, {
+            GET => {
+                method => 'logout'
+            }
+        },
         qr{^/valid_login$}, {
             GET => {
                 method => 'valid_login'

Bugzilla/WebService/User.pm

@@ -19,6 +19,8 @@ use Bugzilla::User;
 use Bugzilla::Util qw(trim);
 use Bugzilla::WebService::Util qw(filter validate translate params_to_objects);
 
+use List::Util qw(first);
+
 # Don't need auth to login
 use constant LOGIN_EXEMPT => {
     login => 1,
@@ -73,14 +75,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;
 }
 
 sub valid_login {
@@ -448,10 +461,12 @@ management of cookies across sessions.
 
 =item B<Returns>
 
-On success, a hash containing one item, C<id>, 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<id>, the numeric id of the
+user that was logged in, and a C<token> 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<Errors>
 

Bugzilla/WebService/Util.pm

@@ -23,6 +23,7 @@ our @EXPORT_OK = qw(
     validate
     translate
     params_to_objects
+    fix_credentials
 );
 
 sub filter ($$;$) {
@@ -146,6 +147,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 GET /login. We also do not delete them as
+    # GET /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
@@ -209,6 +226,12 @@ 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.
+
 =head1 B<Methods in need of POD>
 
 =over