]> git.ipfire.org Git - thirdparty/bugzilla.git/commitdiff
Backout Bug 893195 - Allow token based authentication for webservices
authorDave Lawrence <dlawrence@mozilla.com>
Tue, 25 Feb 2014 23:21:14 +0000 (23:21 +0000)
committerDave Lawrence <dlawrence@mozilla.com>
Tue, 25 Feb 2014 23:21:14 +0000 (23:21 +0000)
Bugzilla/Auth.pm
Bugzilla/Auth/Login.pm
Bugzilla/Auth/Login/Cookie.pm
Bugzilla/Auth/Persist/Cookie.pm
Bugzilla/WebService/Server/JSONRPC.pm
Bugzilla/WebService/Server/XMLRPC.pm
Bugzilla/WebService/User.pm
Bugzilla/WebService/Util.pm

index 05238e0b3e9df58a8e373733b3777c8e452b4575..477dbffaa83adf231a721fc2fd1f0801aee4107b 100644 (file)
@@ -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<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
index f1aad4108cd9a158a6fe7a46ad5eeb229665e10b..290cb42ffd23ece6a7d990b876d2d2e7e74a5e93 100644 (file)
@@ -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
index 2e31c19d57d82f604fce860a97e5be82b6560167..274e064ff196f459f4d73acb064b611a6eab6e8d 100644 (file)
@@ -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;
index 42d4d6028c0a8c12d29b71ebcaa121d58be26ac7..c9aaef8635a54eed1da8433a04c1e496ccc618c3 100644 (file)
@@ -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 {
index 9f57104398201cab6463425b19cb49db7b623441..804d7874efc6df4fe0f9df22a5bc5a9b7a3b6b3e 100644 (file)
@@ -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') {
index 03590bd1cd3698f921fb48f36ab2119cabc1e116..40cc6ec5489eaecf34dc18ab5cb736179efc95c7 100644 (file)
@@ -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;
 }
index 238ab4dc5497f90d1dc043b9134cc4e8ea2164b7..491b4cd63d9084b02d34b1fc1a440453ae1c13af 100644 (file)
@@ -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<Returns>
 
-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.
+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.
 
 =item B<Errors>
 
index 195de79e44406bb795c15d43d34b9a9026d1896f..6e935fe2091b2c6e3261986456fda3137ad554c5 100644 (file)
@@ -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.