merged with bugzilla/extensions/browserid

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 %]