diff options
| -rw-r--r-- | Bugzilla/Auth.pm | 2 | ||||
| -rw-r--r-- | Bugzilla/Auth/Login/CGI.pm | 41 | ||||
| -rw-r--r-- | Bugzilla/Auth/Persist/Cookie.pm | 4 | ||||
| -rw-r--r-- | Bugzilla/CGI.pm | 15 | ||||
| -rw-r--r-- | Bugzilla/Template.pm | 5 | ||||
| -rw-r--r-- | Bugzilla/WebService.pm | 38 | ||||
| -rw-r--r-- | Bugzilla/WebService/User.pm | 52 | ||||
| -rwxr-xr-x | relogin.cgi | 13 | ||||
| -rw-r--r-- | template/en/default/account/auth/login-small.html.tmpl | 4 | ||||
| -rw-r--r-- | template/en/default/account/auth/login.html.tmpl | 4 | ||||
| -rw-r--r-- | template/en/default/admin/sudo.html.tmpl | 5 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 9 |
12 files changed, 130 insertions, 62 deletions
diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index 6b291d8ae..42e4ee784 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -153,7 +153,7 @@ sub _handle_login_result { if ($self->{_info_getter}->{successful}->requires_persistence and !Bugzilla->request_cache->{auth_no_automatic_login}) { - $self->{_persister}->persist_login($user); + $user->{_login_token} = $self->{_persister}->persist_login($user); } } elsif ($fail_code == AUTH_ERROR) { diff --git a/Bugzilla/Auth/Login/CGI.pm b/Bugzilla/Auth/Login/CGI.pm index a55275a54..f9e3bbf18 100644 --- a/Bugzilla/Auth/Login/CGI.pm +++ b/Bugzilla/Auth/Login/CGI.pm @@ -17,19 +17,52 @@ use Bugzilla::Constants; use Bugzilla::WebService::Constants; use Bugzilla::Util; use Bugzilla::Error; +use Bugzilla::Token; sub get_login_info { my ($self) = @_; my $params = Bugzilla->input_params; + my $cgi = Bugzilla->cgi; + + my $login = trim(delete $params->{'Bugzilla_login'}); + my $password = delete $params->{'Bugzilla_password'}; + # The token must match the cookie to authenticate the request. + my $login_token = delete $params->{'Bugzilla_login_token'}; + my $login_cookie = $cgi->cookie('Bugzilla_login_request_cookie'); - my $username = trim(delete $params->{"Bugzilla_login"}); - my $password = delete $params->{"Bugzilla_password"}; + my $valid = 0; + # If the web browser accepts cookies, use them. + if ($login_token && $login_cookie) { + my ($time, undef) = split(/-/, $login_token); + # Regenerate the token based on the information we have. + my $expected_token = issue_hash_token(['login_request', $login_cookie], $time); + $valid = 1 if $expected_token eq $login_token; + $cgi->remove_cookie('Bugzilla_login_request_cookie'); + } + # WebServices and other local scripts can bypass this check. + # This is safe because we won't store a login cookie in this case. + elsif (Bugzilla->usage_mode != USAGE_MODE_BROWSER) { + $valid = 1; + } + # Else falls back to the Referer header and accept local URLs. + # Attachments are served from a separate host (ideally), and so + # an evil attachment cannot abuse this check with a redirect. + elsif (my $referer = $cgi->referer) { + my $urlbase = correct_urlbase(); + $valid = 1 if $referer =~ /^\Q$urlbase\E/; + } + # If the web browser doesn't accept cookies and the Referer header + # is missing, we have no way to make sure that the authentication + # request comes from the user. + elsif ($login && $password) { + ThrowUserError('auth_untrusted_request', { login => $login }); + } - if (!defined $username || !defined $password) { + if (!$login || !$password || !$valid) { return { failure => AUTH_NODATA }; } - return { username => $username, password => $password }; + return { username => $login, password => $password }; } sub fail_nodata { diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm index 5a9857cba..6f4eac96d 100644 --- a/Bugzilla/Auth/Persist/Cookie.pm +++ b/Bugzilla/Auth/Persist/Cookie.pm @@ -54,6 +54,10 @@ sub persist_login { $dbh->bz_commit_transaction(); + # We do not want WebServices to generate login cookies. + # All we need is the login token for User.login. + return $login_cookie if i_am_webservice(); + # Prevent JavaScript from accessing login cookies. my %cookieargs = ('-httponly' => 1); diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index d7e81d793..48b4fb0bf 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -291,7 +291,8 @@ sub header { my $self = shift; my %headers; - + my $user = Bugzilla->user; + # If there's only one parameter, then it's a Content-Type. if (scalar(@_) == 1) { %headers = ('-type' => shift(@_)); @@ -304,6 +305,18 @@ sub header { $headers{'-content_disposition'} = $self->{'_content_disp'}; } + if (!$user->id && $user->authorizer->can_login + && !$self->cookie('Bugzilla_login_request_cookie')) + { + my %args; + $args{'-secure'} = 1 if Bugzilla->params->{ssl_redirect}; + + $self->send_cookie(-name => 'Bugzilla_login_request_cookie', + -value => generate_random_password(), + -httponly => 1, + %args); + } + # Add the cookies in if we have any if (scalar(@{$self->{Bugzilla_cookie_list}})) { $headers{'-cookie'} = $self->{Bugzilla_cookie_list}; diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index b8fcae107..56d31dd2d 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -920,6 +920,11 @@ sub create { # Allow templates to generate a token themselves. 'issue_hash_token' => \&Bugzilla::Token::issue_hash_token, + 'get_login_request_token' => sub { + my $cookie = Bugzilla->cgi->cookie('Bugzilla_login_request_cookie'); + return $cookie ? issue_hash_token(['login_request', $cookie]) : ''; + }, + # A way for all templates to get at Field data, cached. 'bug_fields' => sub { my $cache = Bugzilla->request_cache; diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm index 9638d1132..ebad7930a 100644 --- a/Bugzilla/WebService.pm +++ b/Bugzilla/WebService.pm @@ -141,9 +141,7 @@ There are various ways to log in: =item C<User.login> You can use L<Bugzilla::WebService::User/login> to log in as a Bugzilla -user. This issues standard HTTP cookies that you must then use in future -calls, so your client must be capable of receiving and transmitting -cookies. +user. This issues a token that you must then use in future calls. =item C<Bugzilla_login> and C<Bugzilla_password> @@ -163,30 +161,28 @@ WebService method to perform a login: =item C<Bugzilla_restrictlogin> (boolean) - Optional. If true, then your login will only be valid for your IP address. -=item C<Bugzilla_rememberlogin> (boolean) - Optional. If true, -then the cookie sent back to you with the method response will -not expire. - =back -The C<Bugzilla_restrictlogin> and C<Bugzilla_rememberlogin> options -are only used when you have also specified C<Bugzilla_login> and -C<Bugzilla_password>. - -Note that Bugzilla will return HTTP cookies along with the method -response when you use these arguments (just like the C<User.login> method -above). +The C<Bugzilla_restrictlogin> option is only used when you have also +specified C<Bugzilla_login> and C<Bugzilla_password>. -For REST, you may also use the C<username> and C<password> variable +For REST, you may also use the C<login> and C<password> variable names instead of C<Bugzilla_login> and C<Bugzilla_password> as a -convenience. +convenience. You may also use C<token> instead of C<Bugzilla_token>. + +=item C<Bugzilla_token> + +B<Added in Bugzilla 5.0> + +You can specify C<Bugzilla_token> as argument to any WebService method, +and you will be logged in as that user if the token is correct. This is +the token returned when calling C<User.login> mentioned above. -=item B<Added in Bugzilla 5.0> +An error is thrown if you pass an invalid token and you will need to log +in again to get a new token. -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>. +Token support was added in Bugzilla B<5.0> and support for login cookies +has been dropped for security reasons. =back diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm index f69ae8ed4..f8358f78d 100644 --- a/Bugzilla/WebService/User.pm +++ b/Bugzilla/WebService/User.pm @@ -51,7 +51,6 @@ use constant MAPPED_RETURNS => { sub login { my ($self, $params) = @_; - my $remember = $params->{remember}; # Username and password params are required foreach my $param ("login", "password") { @@ -59,33 +58,18 @@ sub login { || ThrowCodeError('param_required', { param => $param }); } - # Convert $remember from a boolean 0/1 value to a CGI-compatible one. - if (defined($remember)) { - $remember = $remember? 'on': ''; - } - else { - # Use Bugzilla's default if $remember is not supplied. - $remember = - Bugzilla->params->{'rememberlogin'} eq 'defaulton'? 'on': ''; - } - # Make sure the CGI user info class works if necessary. my $input_params = Bugzilla->input_params; $input_params->{'Bugzilla_login'} = $params->{login}; $input_params->{'Bugzilla_password'} = $params->{password}; - $input_params->{'Bugzilla_remember'} = $remember; + $input_params->{'Bugzilla_restrictlogin'} = $params->{restrict_login}; 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; + if ($user->{_login_token}) { + $result->{'token'} = $user->id . "-" . $user->{_login_token}; } return $result; @@ -464,13 +448,9 @@ etc. This method logs in an user. =item C<password> (string) - The user's password. -=item C<remember> (bool) B<Optional> - if the cookies returned by the -call to login should expire with the session or not. In order for -this option to have effect the Bugzilla server must be configured to -allow the user to set this option - the Bugzilla parameter -I<rememberlogin> must be set to "defaulton" or -"defaultoff". Addionally, the client application must implement -management of cookies across sessions. +=item C<restrict_login> (bool) B<Optional> - If set to a true value, +the token returned by this method will only be valid from the IP address +which called this method. =back @@ -478,12 +458,9 @@ management of cookies across sessions. 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 +the parameters as authentication in other calls. The token can be sent along with any future requests to the webservice, for the duration of the -session. Note that cookies are not accepted for GET requests for JSONRPC -and REST for security reasons. You may, however, use the token or valid -login parameters for those requests. +session, i.e. till L<User.logout|/logout> is called. =item B<Errors> @@ -509,6 +486,19 @@ A login or password parameter was not provided. =back +=item B<History> + +=over + +=item C<remember> was removed in Bugzilla B<5.0> as this method no longer +creates a login cookie. + +=item C<restrict_login> was added in Bugzilla B<5.0>. + +=item C<token> was added in Bugzilla B<5.0>. + +=back + =back =head2 logout diff --git a/relogin.cgi b/relogin.cgi index 52944a811..0c1cb9ad6 100755 --- a/relogin.cgi +++ b/relogin.cgi @@ -52,6 +52,19 @@ elsif ($action eq 'prepare-sudo') { # Keep a temporary record of the user visiting this page $vars->{'token'} = issue_session_token('sudo_prepared'); + if ($user->authorizer->can_login) { + my $value = generate_random_password(); + my %args; + $args{'-secure'} = 1 if Bugzilla->params->{ssl_redirect}; + + $cgi->send_cookie(-name => 'Bugzilla_login_request_cookie', + -value => $value, + -httponly => 1, + %args); + + $vars->{'login_request_token'} = issue_hash_token(['login_request', $value]); + } + # Show the sudo page $vars->{'target_login_default'} = $cgi->param('target_login'); $vars->{'reason_default'} = $cgi->param('reason'); diff --git a/template/en/default/account/auth/login-small.html.tmpl b/template/en/default/account/auth/login-small.html.tmpl index 32dbe431b..5868b8671 100644 --- a/template/en/default/account/auth/login-small.html.tmpl +++ b/template/en/default/account/auth/login-small.html.tmpl @@ -46,7 +46,9 @@ [%+ "checked" IF Param('rememberlogin') == "defaulton" %]> <label for="Bugzilla_remember[% qs_suffix %]">Remember</label> [% END %] - <input type="submit" name="GoAheadAndLogIn" value="Log in" + <input type="hidden" name="Bugzilla_login_token" + value="[% get_login_request_token() FILTER html %]"> + <input type="submit" name="GoAheadAndLogIn" value="Log in" id="log_in[% qs_suffix %]"> <a href="#" onclick="return hide_mini_login_form('[% qs_suffix %]')">[x]</a> </form> diff --git a/template/en/default/account/auth/login.html.tmpl b/template/en/default/account/auth/login.html.tmpl index bf20edb8b..b6da535cc 100644 --- a/template/en/default/account/auth/login.html.tmpl +++ b/template/en/default/account/auth/login.html.tmpl @@ -76,8 +76,10 @@ [% PROCESS "global/hidden-fields.html.tmpl" exclude="^Bugzilla_(login|password|restrictlogin)$" %] + <input type="hidden" name="Bugzilla_login_token" + value="[% get_login_request_token() FILTER html %]"> <input type="submit" name="GoAheadAndLogIn" value="Log in" id="log_in"> - + <p> (Note: you should make sure cookies are enabled for this site. Otherwise, you will be required to log in frequently.) diff --git a/template/en/default/admin/sudo.html.tmpl b/template/en/default/admin/sudo.html.tmpl index d4faf4ea7..bf0cd7b6f 100644 --- a/template/en/default/admin/sudo.html.tmpl +++ b/template/en/default/admin/sudo.html.tmpl @@ -68,9 +68,10 @@ <p> Finally, enter <label for="Bugzilla_password">your [% terms.Bugzilla %] password</label>: - <input type="hidden" name="Bugzilla_login" value=" - [%- user.login FILTER html %]"> + <input type="hidden" name="Bugzilla_login" value="[% user.login FILTER html %]"> <input type="password" id="Bugzilla_password" name="Bugzilla_password" size="20" required> + <input type="hidden" name="Bugzilla_login_token" + value="[% login_request_token FILTER html %]"> <br> This is done for two reasons. First of all, it is done to reduce the chances of someone doing large amounts of damage using your diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 11d4f8ad1..0b5d66764 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -219,6 +219,15 @@ [% Hook.process("auth_failure") %] + [% ELSIF error == "auth_untrusted_request" %] + [% title = "Untrusted Authentication Request" %] + You tried to log in using the <em>[% login FILTER html %]</em> account, + but [% terms.Bugzilla %] is unable to trust your request. Make sure + your web browser accepts cookies and that you haven't been redirected + here from an external web site. + <a href="index.cgi?GoAheadAndLogIn=1">Click here</a> if you really want + to log in. + [% ELSIF error == "attachment_deletion_disabled" %] [% title = "Attachment Deletion Disabled" %] Attachment deletion is disabled on this installation. |