Add a U2F feature spec for multiple devices owned by the same user.17341-firefox-u2f

1. This scenario was previously tested for the registration flow, but
   not authentication.

5 files changed, 57 insertions, 21 deletions

diff --git a/app/assets/javascripts/u2f/authenticate.js.coffee b/app/assets/javascripts/u2f/authenticate.js.coffee
index be10e911c83..918c0a560fd 100644
--- a/app/assets/javascripts/u2f/authenticate.js.coffee
+++ b/app/assets/javascripts/u2f/authenticate.js.coffee
@@ -8,14 +8,17 @@ class @U2FAuthenticate
     @appId = u2fParams.app_id
     @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.
+    # 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: () =>
diff --git a/app/assets/javascripts/u2f/util.js.coffee.erb b/app/assets/javascripts/u2f/util.js.coffee
index be1d3286b01..5ef324f609d 100644
--- a/app/assets/javascripts/u2f/util.js.coffee.erb
+++ b/app/assets/javascripts/u2f/util.js.coffee
@@ -1,3 +1,3 @@
 class @U2FUtil
   @isU2FSupported: ->
-      window.u2f
+    window.u2f
diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml
index dbf4d699d01..4debd3d608f 100644
--- a/app/views/devise/sessions/two_factor.html.haml
+++ b/app/views/devise/sessions/two_factor.html.haml
@@ -1,5 +1,5 @@
-- content_for :page_specific_javascripts do
-  - if inject_u2f_api?
+- if inject_u2f_api?
+  - content_for :page_specific_javascripts do
     = page_specific_javascript_tag('u2f.js')
 
 %div
diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml
index 0e9a80a6267..355bfcf1d62 100644
--- a/app/views/profiles/two_factor_auths/show.html.haml
+++ b/app/views/profiles/two_factor_auths/show.html.haml
@@ -2,8 +2,8 @@
 - header_title "Two-Factor Authentication", profile_two_factor_auth_path
 = render 'profiles/head'
 
-- content_for :page_specific_javascripts do
-  - if inject_u2f_api?
+- if inject_u2f_api?
+  - content_for :page_specific_javascripts do
     = page_specific_javascript_tag('u2f.js')
 
 .row.prepend-top-default
diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb
index 14613754f74..9335f5bf120 100644
--- a/spec/features/u2f_spec.rb
+++ b/spec/features/u2f_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: true, js: true do
+  before { allow_any_instance_of(U2fHelper).to receive(:inject_u2f_api?).and_return(true) }
+
   def register_u2f_device(u2f_device = nil)
     u2f_device ||= FakeU2fDevice.new(page)
     u2f_device.respond_to_u2f_registration
@@ -208,21 +210,52 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         expect(page.body).to match('Authentication via U2F device failed')
       end
     end
-  end
 
-  describe "when two-factor authentication is disabled" do
-    let(:user) { create(:user) }
+    describe "when more than one device has been registered by the same user" do
+      it "allows logging in with either device" do
+        # Register first device
+        user = login_as(:user)
+        user.update_attribute(:otp_required_for_login, true)
+        visit profile_two_factor_auth_path
+        expect(page).to have_content("Your U2F device needs to be set up.")
+        first_device = register_u2f_device
+
+        # Register second device
+        visit profile_two_factor_auth_path
+        expect(page).to have_content("Your U2F device needs to be set up.")
+        second_device = register_u2f_device
+        logout
+
+        # Authenticate as both devices
+        [first_device, second_device].each do |device|
+          login_as(user)
+          device.respond_to_u2f_authentication
+          click_on "Login Via U2F Device"
+          expect(page.body).to match('We heard back from your U2F device')
+          click_on "Authenticate via U2F Device"
 
-    before do
-      login_as(user)
-      user.update_attribute(:otp_required_for_login, true)
-      visit profile_account_path
-      click_on 'Manage Two-Factor Authentication'
-      register_u2f_device
+          expect(page.body).to match('Signed in successfully')
+
+          logout
+        end
+      end
     end
 
-    it "deletes u2f registrations" do
-      expect { click_on "Disable" }.to change { U2fRegistration.count }.from(1).to(0)
+    describe "when two-factor authentication is disabled" do
+      let(:user) { create(:user) }
+
+      before do
+        user = login_as(:user)
+        user.update_attribute(:otp_required_for_login, true)
+        visit profile_account_path
+        click_on 'Manage Two-Factor Authentication'
+        expect(page).to have_content("Your U2F device needs to be set up.")
+        register_u2f_device
+      end
+
+      it "deletes u2f registrations" do
+        expect { click_on "Disable" }.to change { U2fRegistration.count }.by(-1)
+      end
     end
   end
 end