Commit 3a857e0e authored by Douwe Maan's avatar Douwe Maan

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
parents 8249fdcd d8a53168
...@@ -51,9 +51,9 @@ ...@@ -51,9 +51,9 @@
%p %p
Use a hardware device to add the second factor of authentication. Use a hardware device to add the second factor of authentication.
%p %p
As U2F devices are only supported by a few browsers, it's recommended that you set up a As U2F devices are only supported by a few browsers, we require that you set up a
two-factor authentication app as well as a U2F device so you'll always be able to log in two-factor authentication app before a U2F device. That way you'll always be able to
using an unsupported browser. log in - even when you're using an unsupported browser.
.col-lg-9 .col-lg-9
%p %p
- if @registration_key_handles.present? - if @registration_key_handles.present?
......
...@@ -4,11 +4,18 @@ ...@@ -4,11 +4,18 @@
%p Your browser doesn't support U2F. Please use Google Chrome desktop (version 41 or newer). %p Your browser doesn't support U2F. Please use Google Chrome desktop (version 41 or newer).
%script#js-register-u2f-setup{ type: "text/template" } %script#js-register-u2f-setup{ type: "text/template" }
.row.append-bottom-10 - if current_user.two_factor_otp_enabled?
.col-md-3 .row.append-bottom-10
%a#js-setup-u2f-device.btn.btn-info{ href: 'javascript:void(0)' } Setup New U2F Device .col-md-3
.col-md-9 %button#js-setup-u2f-device.btn.btn-info Setup New U2F Device
%p Your U2F device needs to be set up. Plug it in (if not already) and click the button on the left. .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" } %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. %p Trying to communicate with your device. Plug it in (if you haven't already) and press the button on the device now.
......
...@@ -12,39 +12,24 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -12,39 +12,24 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
describe "registration" do describe "registration" do
let(:user) { create(:user) } let(:user) { create(:user) }
before { login_as(user) }
describe 'when 2FA via OTP is disabled' do before do
it 'allows registering a new device' do login_as(user)
visit profile_account_path user.update_attribute(:otp_required_for_login, true)
click_on 'Enable Two-Factor Authentication' end
register_u2f_device
expect(page.body).to match('Your U2F device was registered') describe 'when 2FA via OTP is disabled' do
end 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 visit profile_account_path
# First device
click_on 'Enable Two-Factor Authentication' 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
end end
describe 'when 2FA via OTP is enabled' do describe 'when 2FA via OTP is enabled' do
before { user.update_attributes(otp_required_for_login: true) }
it 'allows registering a new device' do it 'allows registering a new device' do
visit profile_account_path visit profile_account_path
click_on 'Manage Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
...@@ -67,7 +52,6 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -67,7 +52,6 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
click_on 'Manage Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
register_u2f_device register_u2f_device
expect(page.body).to match('Your U2F device was registered') expect(page.body).to match('Your U2F device was registered')
click_on 'Manage Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
expect(page.body).to match('You have 2 U2F devices registered') expect(page.body).to match('You have 2 U2F devices registered')
end end
...@@ -76,15 +60,16 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -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 it 'allows the same device to be registered for multiple users' do
# First user # First user
visit profile_account_path visit profile_account_path
click_on 'Enable Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
u2f_device = register_u2f_device u2f_device = register_u2f_device
expect(page.body).to match('Your U2F device was registered') expect(page.body).to match('Your U2F device was registered')
logout logout
# Second user # Second user
login_as(:user) user = login_as(:user)
user.update_attribute(:otp_required_for_login, true)
visit profile_account_path visit profile_account_path
click_on 'Enable Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
register_u2f_device(u2f_device) register_u2f_device(u2f_device)
expect(page.body).to match('Your U2F device was registered') expect(page.body).to match('Your U2F device was registered')
...@@ -94,7 +79,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -94,7 +79,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
context "when there are form errors" do context "when there are form errors" do
it "doesn't register the device if there are errors" do it "doesn't register the device if there are errors" do
visit profile_account_path visit profile_account_path
click_on 'Enable Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
# Have the "u2f device" respond with bad data # Have the "u2f device" respond with bad data
page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };") page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };")
...@@ -109,7 +94,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -109,7 +94,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
it "allows retrying registration" do it "allows retrying registration" do
visit profile_account_path visit profile_account_path
click_on 'Enable Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
# Failed registration # Failed registration
page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };") page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };")
...@@ -133,8 +118,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -133,8 +118,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
before do before do
# Register and logout # Register and logout
login_as(user) login_as(user)
user.update_attribute(:otp_required_for_login, true)
visit profile_account_path visit profile_account_path
click_on 'Enable Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
@u2f_device = register_u2f_device @u2f_device = register_u2f_device
logout logout
end end
...@@ -154,7 +140,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -154,7 +140,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
describe "when 2FA via OTP is enabled" do describe "when 2FA via OTP is enabled" do
it "allows logging in with the U2F device" 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) login_with(user)
@u2f_device.respond_to_u2f_authentication @u2f_device.respond_to_u2f_authentication
...@@ -171,8 +157,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -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 it "does not allow logging in with that particular device" do
# Register current user with the different U2F device # Register current user with the different U2F device
current_user = login_as(:user) current_user = login_as(:user)
current_user.update_attribute(:otp_required_for_login, true)
visit profile_account_path visit profile_account_path
click_on 'Enable Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
register_u2f_device register_u2f_device
logout logout
...@@ -191,8 +178,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -191,8 +178,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
it "allows logging in with that particular device" do it "allows logging in with that particular device" do
# Register current user with the same U2F device # Register current user with the same U2F device
current_user = login_as(:user) current_user = login_as(:user)
current_user.update_attribute(:otp_required_for_login, true)
visit profile_account_path visit profile_account_path
click_on 'Enable Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
register_u2f_device(@u2f_device) register_u2f_device(@u2f_device)
logout logout
...@@ -227,8 +215,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: ...@@ -227,8 +215,9 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
before do before do
login_as(user) login_as(user)
user.update_attribute(:otp_required_for_login, true)
visit profile_account_path visit profile_account_path
click_on 'Enable Two-Factor Authentication' click_on 'Manage Two-Factor Authentication'
register_u2f_device register_u2f_device
end end
......
= 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 }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment