From 341d8bc3f7fbe3763250af1e89020b81dad34bb8 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Mon, 11 Jul 2016 13:23:02 +0530 Subject: Add a U2F feature spec for multiple devices owned by the same user. 1. This scenario was previously tested for the registration flow, but not authentication. --- app/assets/javascripts/u2f/authenticate.js.coffee | 13 +++-- app/assets/javascripts/u2f/util.js.coffee | 3 ++ app/assets/javascripts/u2f/util.js.coffee.erb | 3 -- app/views/devise/sessions/two_factor.html.haml | 4 +- app/views/profiles/two_factor_auths/show.html.haml | 4 +- spec/features/u2f_spec.rb | 55 +++++++++++++++++----- 6 files changed, 59 insertions(+), 23 deletions(-) create mode 100644 app/assets/javascripts/u2f/util.js.coffee delete mode 100644 app/assets/javascripts/u2f/util.js.coffee.erb 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 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 be1d3286b01..00000000000 --- a/app/assets/javascripts/u2f/util.js.coffee.erb +++ /dev/null @@ -1,3 +0,0 @@ -class @U2FUtil - @isU2FSupported: -> - 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 -- cgit v1.2.1