summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-06-15 10:12:55 +0000
committerDouwe Maan <douwe@gitlab.com>2016-06-15 10:12:55 +0000
commit3a857e0e6c9460692f4b5d61e634205da9dee63a (patch)
treed452f5872abba0c27fa416f4d73dd0977bd964db
parent8249fdcd47b33c71e5f9908c0ac877487070e39d (diff)
parentd8a531687c8aaef67d6b7586916273cf59d4b5a3 (diff)
downloadgitlab-ce-3a857e0e6c9460692f4b5d61e634205da9dee63a.tar.gz
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
-rw-r--r--app/views/profiles/two_factor_auths/show.html.haml6
-rw-r--r--app/views/u2f/_register.html.haml17
-rw-r--r--spec/features/u2f_spec.rb57
-rw-r--r--spec/javascripts/fixtures/u2f/register.html.haml3
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 }