diff options
4 files changed, 27 insertions, 29 deletions
diff --git a/extensions/BrowserID/Extension.pm b/extensions/BrowserID/Extension.pm index b132ea503..873cca8e3 100644 --- a/extensions/BrowserID/Extension.pm +++ b/extensions/BrowserID/Extension.pm @@ -29,7 +29,7 @@ sub auth_login_methods { my ($self, $args) = @_; my $modules = $args->{'modules'}; if (exists($modules->{'BrowserID'})) { - $modules->{'BrowserID'} = 'Bugzilla/Extension/BrowserID/Login.pm'; + $modules->{'BrowserID'} = 'Bugzilla/Extension/BrowserID/Login.pm'; } } @@ -42,9 +42,6 @@ sub config_modify_panels { grep { $_->{'name'} eq 'user_info_class' } @$auth_panel_params; if ($user_info_class) { - # XXX Bugzilla::Auth::Login::Stack doesn't let a hard failure stop the - # login process :-(( We put it in both ways round for now, for testing. - push(@{ $user_info_class->{'choices'} }, "CGI,BrowserID"); push(@{ $user_info_class->{'choices'} }, "BrowserID,CGI"); } } diff --git a/extensions/BrowserID/lib/Login.pm b/extensions/BrowserID/lib/Login.pm index eedc85d09..03d33cf6d 100644 --- a/extensions/BrowserID/lib/Login.pm +++ b/extensions/BrowserID/lib/Login.pm @@ -25,6 +25,8 @@ use base qw(Bugzilla::Auth::Login); use Bugzilla::Constants; use Bugzilla::Util; +use Bugzilla::Error; +use Bugzilla::Token; use JSON; use LWP::UserAgent; @@ -36,15 +38,20 @@ sub get_login_info { my ($self) = @_; my $cgi = Bugzilla->cgi; + my $assertion = $cgi->param("browserid_assertion"); # Avoid the assertion being copied into any 'echoes' of the current URL # in the page. $cgi->delete('browserid_assertion'); - + if (!$assertion) { return { failure => AUTH_NODATA }; } + my $token = $cgi->param("token"); + $cgi->delete('token'); + check_hash_token($token, ['login']); + my $urlbase = new URI(correct_urlbase()); my $audience = $urlbase->scheme . "://" . $urlbase->host_port; @@ -59,12 +66,9 @@ sub get_login_info { $info = decode_json($response->content()); }; - # XXX Add 120 secs because 'expires' is currently broken in deployed - # BrowserID server - it returns exact current time, so is immediately - # expired! This should be fixed soon. if ($info->{'status'} eq "okay" && $info->{'audience'} eq $audience && - (($info->{'expires'} / 1000) + 120) > time()) + ($info->{'expires'} / 1000) > time()) { my $login_data = { 'username' => $info->{'email'} @@ -76,20 +80,19 @@ sub get_login_info { my $user = $result->{'user'}; - # BrowserID logins are currently restricted to less powerful accounts - - # the most you can have is 'editbugs'. This is while the technology - # is maturing. So we need to check that the user doesn't have 'too - # many permissions' to log in this way. + # You can restrict people in a particular group from logging in using + # BrowserID by making that group a member of a group called + # "no-browser-id". # - # If a newly-created account has too many permissions, this code will + # If you have your "createemailregexp" set up in such a way that a + # newly-created account is a member of "no-browser-id", this code will # create an account for them and then fail their login. Which isn't # great, but they can still use normal-Bugzilla-login password # recovery. - my @safe_groups = ('everyone', 'canconfirm', 'editbugs'); - foreach my $group (@{ $user->groups() }) { - if (!grep { $group->name eq $_ } @safe_groups) { - return { failure => AUTH_LOGINFAILED }; - } + if ($user->in_group('no-browser-id')) { + # We use a custom error here, for greater clarity, rather than + # returning a failure code. + ThrowUserError('browserid_account_too_powerful'); } $login_data->{'user'} = $user; diff --git a/extensions/BrowserID/template/en/default/hook/account/auth/login-additional_methods.html.tmpl b/extensions/BrowserID/template/en/default/hook/account/auth/login-additional_methods.html.tmpl index 4d8d8d3f0..590efdfb3 100644 --- a/extensions/BrowserID/template/en/default/hook/account/auth/login-additional_methods.html.tmpl +++ b/extensions/BrowserID/template/en/default/hook/account/auth/login-additional_methods.html.tmpl @@ -7,10 +7,9 @@ function browserid_sign_in() { if (assertion) { // This code will be invoked once the user has successfully // selected an email address they control to sign in with. - window.location.href = "[% target FILTER none %]index.cgi?browserid_assertion=" + assertion; - } else { - // something went wrong! the user isn't logged in. - alert("Oh no! Something went wrong..."); + var token = "[% issue_hash_token(['login']) FILTER html %]"; + window.location.href = "[% login_target FILTER none %]?token=" + + token + "&browserid_assertion=" + assertion; } }); } @@ -20,4 +19,4 @@ function browserid_sign_in() { Or, you could login using BrowserID: <img src="extensions/BrowserID/web/sign_in_orange.png" onclick="browserid_sign_in()"> </p> -[% END %]
\ No newline at end of file +[% END %] diff --git a/extensions/BrowserID/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl b/extensions/BrowserID/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl index 12bff015b..9871d585d 100644 --- a/extensions/BrowserID/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl +++ b/extensions/BrowserID/template/en/default/hook/account/auth/login-small-additional_methods.html.tmpl @@ -7,14 +7,13 @@ function browserid_sign_in() { if (assertion) { // This code will be invoked once the user has successfully // selected an email address they control to sign in with. - window.location.href = "[% login_target FILTER none %]?browserid_assertion=" + assertion; - } else { - // something went wrong! the user isn't logged in. - alert("Oh no! Something went wrong..."); + var token = "[% issue_hash_token(['login']) FILTER html %]"; + window.location.href = "[% login_target FILTER none %]?token=" + + token + "&browserid_assertion=" + assertion; } }); } </script> <img src="extensions/BrowserID/web/sign_in_orange.png" onclick="browserid_sign_in()" style="margin-bottom: -8px"> or -[% END %]
\ No newline at end of file +[% END %] |