Improved the u2f flow

Added tests

app/assets/javascripts/dispatcher.js.es6

@@ -64,6 +64,17 @@
           new UsernameValidator();
           new ActiveTabMemoizer();
           break;
+        case 'sessions:create':
+          if (!gon.u2f) break;
+          window.gl.u2fAuthenticate = new gl.U2FAuthenticate(
+            $("#js-authenticate-u2f"),
+            '#js-login-u2f-form',
+            gon.u2f,
+            document.querySelector('#js-login-2fa-device'),
+            document.querySelector('.js-2fa-form'),
+          );
+          window.gl.u2fAuthenticate.start();
+          break;
         case 'projects:boards:show':
         case 'projects:boards:index':
           shortcut_handler = new ShortcutsNavigation();

app/assets/javascripts/u2f/authenticate.js.es6

@@ -8,21 +8,26 @@
 // State Flow #1: setup -> in_progress -> authenticated -> POST to server
 // State Flow #2: setup -> in_progress -> error -> setup
 (function() {
+  const global = window.gl || (window.gl = {});
+
   var bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; };
 
-  this.U2FAuthenticate = (function() {
+  global.U2FAuthenticate = (function() {
-    function U2FAuthenticate(container, u2fParams) {
+    function U2FAuthenticate(container, form, u2fParams, fallbackButton, fallbackUI) {
       this.container = container;
       this.renderNotSupported = bind(this.renderNotSupported, this);
       this.renderAuthenticated = bind(this.renderAuthenticated, this);
       this.renderError = bind(this.renderError, this);
       this.renderInProgress = bind(this.renderInProgress, this);
-      this.renderSetup = bind(this.renderSetup, this);
       this.renderTemplate = bind(this.renderTemplate, this);
       this.authenticate = bind(this.authenticate, this);
       this.start = bind(this.start, this);
       this.appId = u2fParams.app_id;
       this.challenge = u2fParams.challenge;
+      this.form = form;
+      this.fallbackButton = fallbackButton;
+      this.fallbackUI = fallbackUI;
+      if (this.fallbackButton) this.fallbackButton.addEventListener('click', this.switchToFallbackUI.bind(this));
       this.signRequests = u2fParams.sign_requests.map(function(request) {
         // The U2F Javascript API v1.1 requires a single challenge, with
         // _no challenges per-request_. The U2F Javascript API v1.0 requires a
@@ -41,7 +46,7 @@
 
     U2FAuthenticate.prototype.start = function() {
       if (U2FUtil.isU2FSupported()) {
-        return this.renderSetup();
+        return this.renderInProgress();
       } else {
         return this.renderNotSupported();
       }
@@ -77,11 +82,6 @@
       return this.container.html(template(params));
     };
 
-    U2FAuthenticate.prototype.renderSetup = function() {
-      this.renderTemplate('setup');
-      return this.container.find('#js-login-u2f-device').on('click', this.renderInProgress);
-    };
-
     U2FAuthenticate.prototype.renderInProgress = function() {
       this.renderTemplate('inProgress');
       return this.authenticate();
@@ -92,22 +92,29 @@
         error_message: error.message(),
         error_code: error.errorCode
       });
-      return this.container.find('#js-u2f-try-again').on('click', this.renderSetup);
+      return this.container.find('#js-u2f-try-again').on('click', this.renderInProgress);
     };
 
     U2FAuthenticate.prototype.renderAuthenticated = function(deviceResponse) {
       this.renderTemplate('authenticated');
-      // Prefer to do this instead of interpolating using Underscore templates
+      const container = this.container[0];
-      // because of JSON escaping issues.
+      container.querySelector('#js-device-response').value = deviceResponse;
-      return this.container.find("#js-device-response").val(deviceResponse);
+      container.querySelector(this.form).submit();
+      this.fallbackButton.classList.add('hidden');
     };
 
     U2FAuthenticate.prototype.renderNotSupported = function() {
       return this.renderTemplate('notSupported');
     };
 
+    U2FAuthenticate.prototype.switchToFallbackUI = function() {
+      this.fallbackButton.classList.add('hidden');
+      this.container[0].classList.add('hidden');
+      this.fallbackUI.classList.remove('hidden');
+    };
+
     return U2FAuthenticate;
 
   })();
 
-}).call(this);
+})();

app/views/devise/sessions/two_factor.html.haml

@@ -7,7 +7,7 @@
   .login-box
     .login-body
       - if @user.two_factor_otp_enabled?
-        = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post, html: { class: 'edit_user gl-show-field-errors' }) do |f|
+        = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post, html: { class: "edit_user gl-show-field-errors js-2fa-form #{'hidden' if @user.two_factor_u2f_enabled?}" }) do |f|
           - resource_params = params[resource_name].presence || params
           = f.hidden_field :remember_me, value: resource_params.fetch(:remember_me, 0)
           %div

app/views/u2f/_authenticate.html.haml

@@ -1,30 +1,21 @@
 #js-authenticate-u2f
+%a.btn.btn-block.btn-info#js-login-2fa-device{ href: '#' } Sign in via 2FA code
 
 %script#js-authenticate-u2f-not-supported{ type: "text/template" }
   %p Your browser doesn't support U2F. Please use Google Chrome desktop (version 41 or newer).
 
-%script#js-authenticate-u2f-setup{ type: "text/template" }
-  %div
-    %p Insert your security key (if you haven't already), and press the button below.
-    %a.btn.btn-info#js-login-u2f-device{ href: 'javascript:void(0)' } Sign in via U2F device
-
 %script#js-authenticate-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.
 
 %script#js-authenticate-u2f-error{ type: "text/template" }
   %div
     %p <%= error_message %> (error code: <%= error_code %>)
-    %a.btn.btn-warning#js-u2f-try-again Try again?
+    %a.btn.btn-block.btn-warning#js-u2f-try-again Try again?
 
 %script#js-authenticate-u2f-authenticated{ type: "text/template" }
   %div
-    %p We heard back from your U2F device. Click this button to authenticate with the GitLab server.
+    %p We heard back from your U2F device. You have been authenticated.
-    = form_tag(new_user_session_path, method: :post) do |f|
+    = form_tag(new_user_session_path, method: :post, id: 'js-login-u2f-form') do |f|
       - resource_params = params[resource_name].presence || params
       = hidden_field_tag 'user[remember_me]', resource_params.fetch(:remember_me, 0)
       = hidden_field_tag 'user[device_response]', nil, class: 'form-control', required: true, id: "js-device-response"
-      = submit_tag "Authenticate via U2F Device", class: "btn btn-success"
-
-:javascript
-  var u2fAuthenticate = new U2FAuthenticate($("#js-authenticate-u2f"), gon.u2f);
-  u2fAuthenticate.start();

spec/features/u2f_spec.rb

@@ -45,12 +45,12 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
       it 'allows registering a new device with a name' do
         visit profile_account_path
         manage_two_factor_authentication
-        expect(page.body).to match("You've already enabled two-factor authentication using mobile")
+        expect(page).to have_content("You've already enabled two-factor authentication using mobile")
 
         u2f_device = register_u2f_device
 
-        expect(page.body).to match(u2f_device.name)
+        expect(page).to have_content(u2f_device.name)
-        expect(page.body).to match('Your U2F device was registered')
+        expect(page).to have_content('Your U2F device was registered')
       end
 
       it 'allows registering more than one device' do
@@ -59,30 +59,30 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         # First device
         manage_two_factor_authentication
         first_device = register_u2f_device
-        expect(page.body).to match('Your U2F device was registered')
+        expect(page).to have_content('Your U2F device was registered')
 
         # Second device
         second_device = register_u2f_device
-        expect(page.body).to match('Your U2F device was registered')
+        expect(page).to have_content('Your U2F device was registered')
 
-        expect(page.body).to match(first_device.name)
+        expect(page).to have_content(first_device.name)
-        expect(page.body).to match(second_device.name)
+        expect(page).to have_content(second_device.name)
         expect(U2fRegistration.count).to eq(2)
       end
 
       it 'allows deleting a device' do
         visit profile_account_path
         manage_two_factor_authentication
-        expect(page.body).to match("You've already enabled two-factor authentication using mobile")
+        expect(page).to have_content("You've already enabled two-factor authentication using mobile")
 
         first_u2f_device = register_u2f_device
         second_u2f_device = register_u2f_device
 
         click_on "Delete", match: :first
 
-        expect(page.body).to match('Successfully deleted')
+        expect(page).to have_content('Successfully deleted')
         expect(page.body).not_to match(first_u2f_device.name)
-        expect(page.body).to match(second_u2f_device.name)
+        expect(page).to have_content(second_u2f_device.name)
       end
     end
 
@@ -91,7 +91,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
       visit profile_account_path
       manage_two_factor_authentication
       u2f_device = register_u2f_device
-      expect(page.body).to match('Your U2F device was registered')
+      expect(page).to have_content('Your U2F device was registered')
       logout
 
       # Second user
@@ -100,7 +100,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
       visit profile_account_path
       manage_two_factor_authentication
       register_u2f_device(u2f_device)
-      expect(page.body).to match('Your U2F device was registered')
+      expect(page).to have_content('Your U2F device was registered')
 
       expect(U2fRegistration.count).to eq(2)
     end
@@ -117,8 +117,8 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         click_on 'Register U2F Device'
 
         expect(U2fRegistration.count).to eq(0)
-        expect(page.body).to match("The form contains the following error")
+        expect(page).to have_content("The form contains the following error")
-        expect(page.body).to match("did not send a valid JSON response")
+        expect(page).to have_content("did not send a valid JSON response")
       end
 
       it "allows retrying registration" do
@@ -130,12 +130,12 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         click_on 'Setup New U2F Device'
         expect(page).to have_content('Your device was successfully set up')
         click_on 'Register U2F Device'
-        expect(page.body).to match("The form contains the following error")
+        expect(page).to have_content("The form contains the following error")
 
         # Successful registration
         register_u2f_device
 
-        expect(page.body).to match('Your U2F device was registered')
+        expect(page).to have_content('Your U2F device was registered')
         expect(U2fRegistration.count).to eq(1)
       end
     end
@@ -160,10 +160,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         login_with(user)
 
         @u2f_device.respond_to_u2f_authentication
-        click_on "Sign in via U2F device"
+
-        expect(page.body).to match('We heard back from your U2F device')
+        expect(page).to have_content('We heard back from your U2F device')
-        click_on "Authenticate via U2F Device"
+        expect(page).to have_css('.sign-out-link', visible: false)
-        expect(page.body).to match('href="/users/sign_out"')
       end
     end
 
@@ -173,11 +172,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         login_with(user)
 
         @u2f_device.respond_to_u2f_authentication
-        click_on "Sign in via U2F device"
-        expect(page.body).to match('We heard back from your U2F device')
-        click_on "Authenticate via U2F Device"
 
-        expect(page.body).to match('href="/users/sign_out"')
+        expect(page).to have_content('We heard back from your U2F device')
+        expect(page).to have_css('.sign-out-link', visible: false)
       end
     end
 
@@ -185,8 +182,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
       login_with(user, remember: true)
 
       @u2f_device.respond_to_u2f_authentication
-      click_on "Sign in via U2F device"
+      expect(page).to have_content('We heard back from your U2F device')
-      expect(page.body).to match('We heard back from your U2F device')
 
       within 'div#js-authenticate-u2f' do
         field = first('input#user_remember_me', visible: false)
@@ -208,11 +204,8 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
           # Try authenticating user with the old U2F device
           login_as(current_user)
           @u2f_device.respond_to_u2f_authentication
-          click_on "Sign in via U2F device"
+          expect(page).to have_content('We heard back from your U2F device')
-          expect(page.body).to match('We heard back from your U2F device')
+          expect(page).to have_content('Authentication via U2F device failed')
-          click_on "Authenticate via U2F Device"
-
-          expect(page.body).to match('Authentication via U2F device failed')
         end
       end
 
@@ -229,11 +222,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
           # Try authenticating user with the same U2F device
           login_as(current_user)
           @u2f_device.respond_to_u2f_authentication
-          click_on "Sign in via U2F device"
+          expect(page).to have_content('We heard back from your U2F device')
-          expect(page.body).to match('We heard back from your U2F device')
-          click_on "Authenticate via U2F Device"
 
-          expect(page.body).to match('href="/users/sign_out"')
+          expect(page).to have_css('.sign-out-link', visible: false)
         end
       end
     end
@@ -243,11 +234,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         unregistered_device = FakeU2fDevice.new(page, FFaker::Name.first_name)
         login_as(user)
         unregistered_device.respond_to_u2f_authentication
-        click_on "Sign in via U2F device"
+        expect(page).to have_content('We heard back from your U2F device')
-        expect(page.body).to match('We heard back from your U2F device')
-        click_on "Authenticate via U2F Device"
 
-        expect(page.body).to match('Authentication via U2F device failed')
+        expect(page).to have_content('Authentication via U2F device failed')
       end
     end
 
@@ -270,11 +259,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
         [first_device, second_device].each do |device|
           login_as(user)
           device.respond_to_u2f_authentication
-          click_on "Sign in via U2F device"
+          expect(page).to have_content('We heard back from your U2F device')
-          expect(page.body).to match('We heard back from your U2F device')
-          click_on "Authenticate via U2F Device"
 
-          expect(page.body).to match('href="/users/sign_out"')
+          expect(page).to have_css('.sign-out-link', visible: false)
 
           logout
         end
@@ -299,4 +286,50 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
       end
     end
   end
+
+  describe 'fallback code authentication' do
+    let(:user) { create(:user) }
+
+    def assert_fallback_ui(page)
+      expect(page).to have_button('Verify code')
+      expect(page).to have_css('#user_otp_attempt')
+      expect(page).not_to have_link('Sign in via 2FA code')
+      expect(page).not_to have_css('#js-authenticate-u2f')
+    end
+
+    before do
+      # Register and logout
+      login_as(user)
+      user.update_attribute(:otp_required_for_login, true)
+      visit profile_account_path
+    end
+
+    describe 'when no u2f device is registered' do
+      before do
+        logout
+        login_with(user)
+      end
+
+      it 'shows the fallback otp code UI' do
+        assert_fallback_ui(page)
+      end
+    end
+
+    describe 'when a u2f device is registered' do
+      before do
+        manage_two_factor_authentication
+        @u2f_device = register_u2f_device
+        logout
+        login_with(user)
+      end
+
+      it 'provides a button that shows the fallback otp code UI' do
+        expect(page).to have_link('Sign in via 2FA code')
+
+        click_link('Sign in via 2FA code')
+
+        assert_fallback_ui(page)
+      end
+    end
+  end
 end

spec/javascripts/u2f/authenticate_spec.js

@@ -14,18 +14,19 @@
     beforeEach(function() {
       this.u2fDevice = new MockU2FDevice;
       this.container = $("#js-authenticate-u2f");
-      this.component = new U2FAuthenticate(this.container, {
+      this.component = new window.gl.U2FAuthenticate(
-        sign_requests: []
+        this.container,
-      }, "token");
+        '#js-login-u2f-form',
+        {
+          sign_requests: []
+        },
+        document.querySelector('#js-login-2fa-device'),
+        document.querySelector('.js-2fa-form')
+      );
       return this.component.start();
     });
     it('allows authenticating via a U2F device', function() {
-      var authenticatedMessage, deviceResponse, inProgressMessage, setupButton, setupMessage;
+      var authenticatedMessage, deviceResponse, inProgressMessage;
-      setupButton = this.container.find("#js-login-u2f-device");
-      setupMessage = this.container.find("p");
-      expect(setupMessage.text()).toContain('Insert your security key');
-      expect(setupButton.text()).toBe('Sign in via U2F device');
-      setupButton.trigger('click');
       inProgressMessage = this.container.find("p");
       expect(inProgressMessage.text()).toContain("Trying to communicate with your device");
       this.u2fDevice.respondToAuthenticateRequest({
@@ -33,7 +34,7 @@
       });
       authenticatedMessage = this.container.find("p");
       deviceResponse = this.container.find('#js-device-response');
-      expect(authenticatedMessage.text()).toContain("Click this button to authenticate with the GitLab server");
+      expect(authenticatedMessage.text()).toContain('We heard back from your U2F device. You have been authenticated.');
       return expect(deviceResponse.val()).toBe('{"deviceData":"this is data from the device"}');
     });
     return describe("errors", function() {
@@ -62,7 +63,7 @@
           deviceData: "this is data from the device"
         });
         authenticatedMessage = this.container.find("p");
-        return expect(authenticatedMessage.text()).toContain("Click this button to authenticate with the GitLab server");
+        return expect(authenticatedMessage.text()).toContain("We heard back from your U2F device. You have been authenticated.");
       });
     });
   });

spec/support/fake_u2f_device.rb

@@ -28,6 +28,7 @@ class FakeU2fDevice
     u2f.sign = function(appId, challenges, signRequests, callback) {
       callback(#{json_response});
     };
+    window.gl.u2fAuthenticate.start();
     ")
   end