Merge branch '17341-firefox-u2f' into 'master'

Allow U2F devices to be used in Firefox

- Adds U2F support for Firefox
- Improve U2F feature detection logic
- Have authentication flow be closer to the spec (single challenge instead of a challenge for each `signRequest`)

- Closes #17341 
- Related to #15337 

See merge request !5177

10 files changed, 36 insertions, 31 deletions

diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee
index c98763d6271..eceff6d91d5 100644
--- a/app/assets/javascripts/application.js.coffee
+++ b/app/assets/javascripts/application.js.coffee
@@ -53,7 +53,6 @@
 #= require_directory ./u2f
 #= require_directory .
 #= require fuzzaldrin-plus
-#= require u2f
 
 window.slugify = (text) ->
   text.replace(/[^-a-zA-Z0-9]+/g, '_').toLowerCase()
diff --git a/app/assets/javascripts/u2f/authenticate.js.coffee b/app/assets/javascripts/u2f/authenticate.js.coffee
index 6deb902c8de..918c0a560fd 100644
--- a/app/assets/javascripts/u2f/authenticate.js.coffee
+++ b/app/assets/javascripts/u2f/authenticate.js.coffee
@@ -6,8 +6,20 @@
 class @U2FAuthenticate
   constructor: (@container, u2fParams) ->
     @appId = u2fParams.app_id
-    @challenges = u2fParams.challenges
-    @signRequests = u2fParams.sign_requests
+    @challenge = u2fParams.challenge
+
+    # The U2F Javascript API v1.1 requires a single challenge, with
+    # _no challenges per-request_. The U2F Javascript API v1.0 requires a
+    # challenge per-request, which is done by copying the single challenge
+    # into every request.
+    #
+    # In either case, we don't need the per-request challenges that the server
+    # has generated, so we can remove them.
+    #
+    # Note: The server library fixes this behaviour in (unreleased) version 1.0.0.
+    # This can be removed once we upgrade.
+    # https://github.com/castle/ruby-u2f/commit/103f428071a81cd3d5f80c2e77d522d5029946a4
+    @signRequests = u2fParams.sign_requests.map (request) -> _(request).omit('challenge')
 
   start: () =>
     if U2FUtil.isU2FSupported()
@@ -16,7 +28,7 @@ class @U2FAuthenticate
       @renderNotSupported()
 
   authenticate: () =>
-    u2f.sign(@appId, @challenges, @signRequests, (response) =>
+    u2f.sign(@appId, @challenge, @signRequests, (response) =>
       if response.errorCode
         error = new U2FError(response.errorCode)
         @renderError(error);
diff --git a/app/assets/javascripts/u2f/util.js.coffee b/app/assets/javascripts/u2f/util.js.coffee
new file mode 100644
index 00000000000..5ef324f609d
--- /dev/null
+++ b/app/assets/javascripts/u2f/util.js.coffee
@@ -0,0 +1,3 @@
+class @U2FUtil
+  @isU2FSupported: ->
+    window.u2f
diff --git a/app/assets/javascripts/u2f/util.js.coffee.erb b/app/assets/javascripts/u2f/util.js.coffee.erb
deleted file mode 100644
index d59341c38b9..00000000000
--- a/app/assets/javascripts/u2f/util.js.coffee.erb
+++ /dev/null
@@ -1,15 +0,0 @@
-# Helper class for U2F (universal 2nd factor) device registration and authentication.
-
-class @U2FUtil
-  @isU2FSupported: ->
-    if @testMode
-      true
-    else
-      gon.u2f.browser_supports_u2f
-
-  @enableTestMode: ->
-    @testMode = true
-
-<% if Rails.env.test? %>
-U2FUtil.enableTestMode();
-<% end %>
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 9cc31620d9f..a1004d9bcea 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -344,10 +344,6 @@ class ApplicationController < ActionController::Base
     session[:skip_tfa] && session[:skip_tfa] > Time.current
   end
 
-  def browser_supports_u2f?
-    browser.chrome? && browser.version.to_i >= 41 && !browser.device.mobile?
-  end
-
   def redirect_to_home_page_url?
     # If user is not signed-in and tries to access root_path - redirect him to landing page
     # Don't redirect to the default URL to prevent endless redirections
diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index 998b8adc411..ba07cea569c 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -57,7 +57,7 @@ module AuthenticatesWithTwoFactor
 
   # Authenticate using the response from a U2F (universal 2nd factor) device
   def authenticate_with_two_factor_via_u2f(user)
-    if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenges])
+    if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge])
       # Remove any lingering user data from login
       session.delete(:otp_user_id)
       session.delete(:challenges)
@@ -77,11 +77,9 @@ module AuthenticatesWithTwoFactor
 
     if key_handles.present?
       sign_requests = u2f.authentication_requests(key_handles)
-      challenges = sign_requests.map(&:challenge)
-      session[:challenges] = challenges
-      gon.push(u2f: { challenges: challenges, app_id: u2f_app_id,
-                      sign_requests: sign_requests,
-                      browser_supports_u2f: browser_supports_u2f? })
+      session[:challenge] ||= u2f.challenge
+      gon.push(u2f: { challenge: session[:challenge], app_id: u2f_app_id,
+                      sign_requests: sign_requests })
     end
   end
 end
diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb
index 6a358fdcc05..e37e9e136db 100644
--- a/app/controllers/profiles/two_factor_auths_controller.rb
+++ b/app/controllers/profiles/two_factor_auths_controller.rb
@@ -100,7 +100,6 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
 
     gon.push(u2f: { challenges: session[:challenges], app_id: u2f_app_id,
                     register_requests: registration_requests,
-                    sign_requests: sign_requests,
-                    browser_supports_u2f: browser_supports_u2f? })
+                    sign_requests: sign_requests })
   end
 end
diff --git a/app/helpers/u2f_helper.rb b/app/helpers/u2f_helper.rb
new file mode 100644
index 00000000000..143b4ca6b51
--- /dev/null
+++ b/app/helpers/u2f_helper.rb
@@ -0,0 +1,5 @@
+module U2fHelper
+  def inject_u2f_api?
+    browser.chrome? && browser.version.to_i >= 41 && !browser.device.mobile?
+  end
+end
diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml
index a373f61bd3c..4debd3d608f 100644
--- a/app/views/devise/sessions/two_factor.html.haml
+++ b/app/views/devise/sessions/two_factor.html.haml
@@ -1,3 +1,7 @@
+- if inject_u2f_api?
+  - content_for :page_specific_javascripts do
+    = page_specific_javascript_tag('u2f.js')
+
 %div
   .login-box
     .login-heading
diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml
index 8780da1dec4..366f1fed35b 100644
--- a/app/views/profiles/two_factor_auths/show.html.haml
+++ b/app/views/profiles/two_factor_auths/show.html.haml
@@ -2,6 +2,10 @@
 - header_title "Two-Factor Authentication", profile_two_factor_auth_path
 = render 'profiles/head'
 
+- if inject_u2f_api?
+  - content_for :page_specific_javascripts do
+    = page_specific_javascript_tag('u2f.js')
+
 .row.prepend-top-default
   .col-lg-3
     %h4.prepend-top-0