Merge branch '17333-u2f-only-after-authenticator' into 'master'

Don't allow U2F set up unless an authenticator app is set up

Closes #17333 

# TODO

- [ ]  #17333 Authenticator should be set up before enabling U2F
    - [x]  Implementation
    - [x]  Fix/add tests
    - [x]  Refactor
    - [x]  Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/964c98a3c427cac6e3de88ddc74a9f172ee9742d/builds) to pass
    - [x]  Assign to endboss for review
    - [x]  Address @DouweM's comments
        - [x]  No need for `javascript:void(0)`
        - [x]  Add screenshots
    - [ ]  Wait for merge

# Screenshots

![Screen_Shot_2016-06-15_at_8.18.03_AM](/uploads/26531fa7f6e5d7617fd11d1779021b4f/Screen_Shot_2016-06-15_at_8.18.03_AM.png)
![Screen_Shot_2016-06-15_at_8.18.37_AM](/uploads/ceaae97a987a15d3e04dd76aa8a944bd/Screen_Shot_2016-06-15_at_8.18.37_AM.png)
![Screen_Shot_2016-06-15_at_8.18.47_AM](/uploads/394224d5fcff759d5acc3bf39a138530/Screen_Shot_2016-06-15_at_8.18.47_AM.png)

See merge request !4585

4 files changed, 40 insertions, 43 deletions

diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml
index ce76cb73c9c..593be2617c1 100644
--- a/app/views/profiles/two_factor_auths/show.html.haml
+++ b/app/views/profiles/two_factor_auths/show.html.haml
@@ -51,9 +51,9 @@
     %p
       Use a hardware device to add the second factor of authentication.
     %p
-      As U2F devices are only supported by a few browsers, it's recommended that you set up a
-      two-factor authentication app as well as a U2F device so you'll always be able to log in
-      using an unsupported browser.
+      As U2F devices are only supported by a few browsers, we require that you set up a
+      two-factor authentication app before a U2F device. That way you'll always be able to
+      log in - even when you're using an unsupported browser.
   .col-lg-9
     %p
       - if @registration_key_handles.present?
diff --git a/app/views/u2f/_register.html.haml b/app/views/u2f/_register.html.haml
index 46af591fc43..cbb8dfb7829 100644
--- a/app/views/u2f/_register.html.haml
+++ b/app/views/u2f/_register.html.haml
@@ -4,11 +4,18 @@
   %p Your browser doesn't support U2F. Please use Google Chrome desktop (version 41 or newer).
 
 %script#js-register-u2f-setup{ type: "text/template" }
-  .row.append-bottom-10
-    .col-md-3
-      %a#js-setup-u2f-device.btn.btn-info{ href: 'javascript:void(0)' } Setup New U2F Device
-    .col-md-9
-      %p Your U2F device needs to be set up. Plug it in (if not already) and click the button on the left.
+  - if current_user.two_factor_otp_enabled?
+    .row.append-bottom-10
+      .col-md-3
+        %button#js-setup-u2f-device.btn.btn-info Setup New U2F Device
+      .col-md-9
+        %p Your U2F device needs to be set up. Plug it in (if not already) and click the button on the left.
+  - else
+    .row.append-bottom-10
+      .col-md-3
+        %button#js-setup-u2f-device.btn.btn-info{ disabled: true } Setup New U2F Device
+      .col-md-9
+        %p.text-warning You need to register a two-factor authentication app before you can set up a U2F device.
 
 %script#js-register-u2f-in-progress{ type: "text/template" }
   %p Trying to communicate with your device. Plug it in (if you haven't already) and press the button on the device now.
diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb
index 366a90228b1..14613754f74 100644
--- a/spec/features/u2f_spec.rb
+++ b/spec/features/u2f_spec.rb
@@ -12,39 +12,24 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
 
   describe "registration" do
     let(:user) { create(:user) }
-    before { login_as(user) }
 
-    describe 'when 2FA via OTP is disabled' do
-      it 'allows registering a new device' do
-        visit profile_account_path
-        click_on 'Enable Two-Factor Authentication'
-
-        register_u2f_device
+    before do
+      login_as(user)
+      user.update_attribute(:otp_required_for_login, true)
+    end
 
-        expect(page.body).to match('Your U2F device was registered')
-      end
+    describe 'when 2FA via OTP is disabled' do
+      before { user.update_attribute(:otp_required_for_login, false) }
 
-      it 'allows registering more than one device' do
+      it 'does not allow registering a new device' do
         visit profile_account_path
-
-        # First device
         click_on 'Enable Two-Factor Authentication'
-        register_u2f_device
-        expect(page.body).to match('Your U2F device was registered')
-
-        # Second device
-        click_on 'Manage Two-Factor Authentication'
-        register_u2f_device
-        expect(page.body).to match('Your U2F device was registered')
-        click_on 'Manage Two-Factor Authentication'
 
-        expect(page.body).to match('You have 2 U2F devices registered')
+        expect(page).to have_button('Setup New U2F Device', disabled: true)
       end
     end
 
     describe 'when 2FA via OTP is enabled' do
-      before { user.update_attributes(otp_required_for_login: true) }
-
       it 'allows registering a new device' do
         visit profile_account_path
         click_on 'Manage Two-Factor Authentication'
@@ -67,7 +52,6 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         click_on 'Manage Two-Factor Authentication'
         register_u2f_device
         expect(page.body).to match('Your U2F device was registered')
-
         click_on 'Manage Two-Factor Authentication'
         expect(page.body).to match('You have 2 U2F devices registered')
       end
@@ -76,15 +60,16 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
     it 'allows the same device to be registered for multiple users' do
       # First user
       visit profile_account_path
-      click_on 'Enable Two-Factor Authentication'
+      click_on 'Manage Two-Factor Authentication'
       u2f_device = register_u2f_device
       expect(page.body).to match('Your U2F device was registered')
       logout
 
       # Second user
-      login_as(:user)
+      user = login_as(:user)
+      user.update_attribute(:otp_required_for_login, true)
       visit profile_account_path
-      click_on 'Enable Two-Factor Authentication'
+      click_on 'Manage Two-Factor Authentication'
       register_u2f_device(u2f_device)
       expect(page.body).to match('Your U2F device was registered')
 
@@ -94,7 +79,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
     context "when there are form errors" do
       it "doesn't register the device if there are errors" do
         visit profile_account_path
-        click_on 'Enable Two-Factor Authentication'
+        click_on 'Manage Two-Factor Authentication'
 
         # Have the "u2f device" respond with bad data
         page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };")
@@ -109,7 +94,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
 
       it "allows retrying registration" do
         visit profile_account_path
-        click_on 'Enable Two-Factor Authentication'
+        click_on 'Manage Two-Factor Authentication'
 
         # Failed registration
         page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };")
@@ -133,8 +118,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
     before do
       # Register and logout
       login_as(user)
+      user.update_attribute(:otp_required_for_login, true)
       visit profile_account_path
-      click_on 'Enable Two-Factor Authentication'
+      click_on 'Manage Two-Factor Authentication'
       @u2f_device = register_u2f_device
       logout
     end
@@ -154,7 +140,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
 
     describe "when 2FA via OTP is enabled" do
       it "allows logging in with the U2F device" do
-        user.update_attributes(otp_required_for_login: true)
+        user.update_attribute(:otp_required_for_login, true)
         login_with(user)
 
         @u2f_device.respond_to_u2f_authentication
@@ -171,8 +157,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         it "does not allow logging in with that particular device" do
           # Register current user with the different U2F device
           current_user = login_as(:user)
+          current_user.update_attribute(:otp_required_for_login, true)
           visit profile_account_path
-          click_on 'Enable Two-Factor Authentication'
+          click_on 'Manage Two-Factor Authentication'
           register_u2f_device
           logout
 
@@ -191,8 +178,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         it "allows logging in with that particular device" do
           # Register current user with the same U2F device
           current_user = login_as(:user)
+          current_user.update_attribute(:otp_required_for_login, true)
           visit profile_account_path
-          click_on 'Enable Two-Factor Authentication'
+          click_on 'Manage Two-Factor Authentication'
           register_u2f_device(@u2f_device)
           logout
 
@@ -227,8 +215,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
 
     before do
       login_as(user)
+      user.update_attribute(:otp_required_for_login, true)
       visit profile_account_path
-      click_on 'Enable Two-Factor Authentication'
+      click_on 'Manage Two-Factor Authentication'
       register_u2f_device
     end
 
diff --git a/spec/javascripts/fixtures/u2f/register.html.haml b/spec/javascripts/fixtures/u2f/register.html.haml
index 393c0613fd3..5ed51be689c 100644
--- a/spec/javascripts/fixtures/u2f/register.html.haml
+++ b/spec/javascripts/fixtures/u2f/register.html.haml
@@ -1 +1,2 @@
-= render partial: "u2f/register", locals: { create_u2f_profile_two_factor_auth_path: '/profile/two_factor_auth/create_u2f' }
+- user = FactoryGirl.build(:user, :two_factor_via_otp)
+= render partial: "u2f/register", locals: { create_u2f_profile_two_factor_auth_path: '/profile/two_factor_auth/create_u2f', current_user: user }