diff options
author | Luke "Jared" Bennett <lbennett@gitlab.com> | 2016-12-24 19:53:13 +0300 |
---|---|---|
committer | Luke "Jared" Bennett <lbennett@gitlab.com> | 2016-12-27 03:18:17 +0300 |
commit | b285abeccc3c466b8501d1333f7391be5d6f4334 (patch) | |
tree | d0f62bb2ebb7d054b5505e09e4a7c64217f93f86 | |
parent | 1e38f8ae7254fbe7e8608fd372a7bf3dd9e32607 (diff) |
Improved the u2f flow
Added tests
-rw-r--r-- | app/assets/javascripts/dispatcher.js.es6 | 11 | ||||
-rw-r--r-- | app/assets/javascripts/u2f/authenticate.js.es6 (renamed from app/assets/javascripts/u2f/authenticate.js) | 35 | ||||
-rw-r--r-- | app/views/devise/sessions/two_factor.html.haml | 2 | ||||
-rw-r--r-- | app/views/u2f/_authenticate.html.haml | 17 | ||||
-rw-r--r-- | spec/features/u2f_spec.rb | 119 | ||||
-rw-r--r-- | spec/javascripts/u2f/authenticate_spec.js | 23 | ||||
-rw-r--r-- | spec/support/fake_u2f_device.rb | 3 |
7 files changed, 127 insertions, 83 deletions
diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 78f68a247a2..1c1b6cd2dad 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/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(); diff --git a/app/assets/javascripts/u2f/authenticate.js b/app/assets/javascripts/u2f/authenticate.js.es6 index e407b856e10..2b992109a8c 100644 --- a/app/assets/javascripts/u2f/authenticate.js +++ b/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() { - function U2FAuthenticate(container, u2fParams) { + global.U2FAuthenticate = (function() { + 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 - // because of JSON escaping issues. - return this.container.find("#js-device-response").val(deviceResponse); + const container = this.container[0]; + container.querySelector('#js-device-response').value = 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); +})(); diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml index 2cadc424668..951f03083bf 100644 --- a/app/views/devise/sessions/two_factor.html.haml +++ b/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 diff --git a/app/views/u2f/_authenticate.html.haml b/app/views/u2f/_authenticate.html.haml index fa998c91f72..f878bece2fa 100644 --- a/app/views/u2f/_authenticate.html.haml +++ b/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. - = form_tag(new_user_session_path, method: :post) do |f| + %p We heard back from your U2F device. You have been authenticated. + = 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(); diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb index be21b403084..a8d00bb8e5a 100644 --- a/spec/features/u2f_spec.rb +++ b/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.body).to match('Your U2F device was registered') + expect(page).to have_content(u2f_device.name) + 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.body).to match(second_device.name) + expect(page).to have_content(first_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.body).to match("did not send a valid JSON response") + expect(page).to have_content("The form contains the following error") + 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') - 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 @@ -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.body).to match('We heard back from your U2F device') + expect(page).to have_content('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.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('We heard back from your U2F device') + expect(page).to have_content('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.body).to match('We heard back from your U2F device') - click_on "Authenticate via U2F Device" + expect(page).to have_content('We heard back from your 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.body).to match('We heard back from your U2F device') - click_on "Authenticate via U2F Device" + expect(page).to have_content('We heard back from your 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.body).to match('We heard back from your U2F device') - click_on "Authenticate via U2F Device" + expect(page).to have_content('We heard back from your 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 diff --git a/spec/javascripts/u2f/authenticate_spec.js b/spec/javascripts/u2f/authenticate_spec.js index a8874ab12d3..064d18519ea 100644 --- a/spec/javascripts/u2f/authenticate_spec.js +++ b/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, { - sign_requests: [] - }, "token"); + this.component = new window.gl.U2FAuthenticate( + this.container, + '#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; - 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'); + var authenticatedMessage, deviceResponse, inProgressMessage; 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."); }); }); }); diff --git a/spec/support/fake_u2f_device.rb b/spec/support/fake_u2f_device.rb index 8c407b867fe..a7605cd483a 100644 --- a/spec/support/fake_u2f_device.rb +++ b/spec/support/fake_u2f_device.rb @@ -5,7 +5,7 @@ class FakeU2fDevice @page = page @name = name end - + def respond_to_u2f_registration app_id = @page.evaluate_script('gon.u2f.app_id') challenges = @page.evaluate_script('gon.u2f.challenges') @@ -28,6 +28,7 @@ class FakeU2fDevice u2f.sign = function(appId, challenges, signRequests, callback) { callback(#{json_response}); }; + window.gl.u2fAuthenticate.start(); ") end |