summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2016-09-09 18:42:11 +0000
committerRobert Speicher <robert@gitlab.com>2016-09-09 18:42:11 +0000
commit516100c55b589e69198feca602a2f71e4e66e280 (patch)
tree636e240a98511e980050d103815b4bc92b5bd0b8
parent2798353321d31092577d71b02ecf95b607e9d902 (diff)
parent9224f031cfcb8283566f9d1dcc336ae644faf062 (diff)
downloadgitlab-ce-516100c55b589e69198feca602a2f71e4e66e280.tar.gz
Merge branch 'add-remember-option-u2f' into 'master'
Pass the remember_me option into the u2f form and support it while authenticating ## What does this MR do? Adds remember me support in the u2f authentication, and makes sure the flag gets passed from the login form to the u2f form. Based on the changes for the same thing done for regular 2fa: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4369 ## Why was this MR needed? The remember me option wasn't working for u2f devices (yubikey) ## What are the relevant issue numbers? Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/18103 See merge request !5918
-rw-r--r--CHANGELOG1
-rw-r--r--app/controllers/concerns/authenticates_with_two_factor.rb1
-rw-r--r--app/views/devise/sessions/two_factor.html.haml3
-rw-r--r--app/views/u2f/_authenticate.html.haml2
-rw-r--r--spec/controllers/sessions_controller_spec.rb23
-rw-r--r--spec/features/u2f_spec.rb14
-rw-r--r--spec/javascripts/fixtures/u2f/authenticate.html.haml2
7 files changed, 43 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG
index d77187635d7..d10f880b0ac 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -10,6 +10,7 @@ v 8.12.0 (unreleased)
- Give project selection dropdowns responsive width, make non-wrapping.
- Make push events have equal vertical spacing.
- Add two-factor recovery endpoint to internal API !5510
+ - Pass the "Remember me" value to the U2F authentication form
- Remove vendor prefixes for linear-gradient CSS (ClemMakesApps)
- Add font color contrast to external label in admin area (ClemMakesApps)
- Change logo animation to CSS (ClemMakesApps)
diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index ba07cea569c..d5a8a962662 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -62,6 +62,7 @@ module AuthenticatesWithTwoFactor
session.delete(:otp_user_id)
session.delete(:challenges)
+ remember_me(user) if user_params[:remember_me] == '1'
sign_in(user)
else
flash.now[:alert] = 'Authentication via U2F device failed.'
diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml
index 4debd3d608f..e623f7cff88 100644
--- a/app/views/devise/sessions/two_factor.html.haml
+++ b/app/views/devise/sessions/two_factor.html.haml
@@ -18,6 +18,5 @@
= f.submit "Verify code", class: "btn btn-save"
- if @user.two_factor_u2f_enabled?
-
%hr
- = render "u2f/authenticate"
+ = render "u2f/authenticate", locals: { params: params, resource: resource, resource_name: resource_name }
diff --git a/app/views/u2f/_authenticate.html.haml b/app/views/u2f/_authenticate.html.haml
index 75fb0e303ad..9657101ace5 100644
--- a/app/views/u2f/_authenticate.html.haml
+++ b/app/views/u2f/_authenticate.html.haml
@@ -20,6 +20,8 @@
%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|
+ - 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"
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 4e9bfb0c69b..8f27e616c3e 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -136,6 +136,29 @@ describe SessionsController do
post(:create, { user: user_params }, { otp_user_id: user.id })
end
+ context 'remember_me field' do
+ it 'sets a remember_user_token cookie when enabled' do
+ allow(U2fRegistration).to receive(:authenticate).and_return(true)
+ allow(controller).to receive(:find_user).and_return(user)
+ expect(controller).
+ to receive(:remember_me).with(user).and_call_original
+
+ authenticate_2fa_u2f(remember_me: '1', login: user.username, device_response: "{}")
+
+ expect(response.cookies['remember_user_token']).to be_present
+ end
+
+ it 'does nothing when disabled' do
+ allow(U2fRegistration).to receive(:authenticate).and_return(true)
+ allow(controller).to receive(:find_user).and_return(user)
+ expect(controller).not_to receive(:remember_me)
+
+ authenticate_2fa_u2f(remember_me: '0', login: user.username, device_response: "{}")
+
+ expect(response.cookies['remember_user_token']).to be_nil
+ end
+ end
+
it "creates an audit log record" do
allow(U2fRegistration).to receive(:authenticate).and_return(true)
expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { SecurityEvent.count }.by(1)
diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb
index a46e48c76ed..ff6933dc8d9 100644
--- a/spec/features/u2f_spec.rb
+++ b/spec/features/u2f_spec.rb
@@ -156,6 +156,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
describe "when 2FA via OTP is disabled" do
it "allows logging in with the U2F device" do
+ user.update_attribute(:otp_required_for_login, false)
login_with(user)
@u2f_device.respond_to_u2f_authentication
@@ -181,6 +182,19 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature:
end
end
+ it 'persists remember_me value via hidden field' do
+ login_with(user, remember: true)
+
+ @u2f_device.respond_to_u2f_authentication
+ click_on "Login Via 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)
+ expect(field.value).to eq '1'
+ end
+ end
+
describe "when a given U2F device has already been registered by another user" do
describe "but not the current user" do
it "does not allow logging in with that particular device" do
diff --git a/spec/javascripts/fixtures/u2f/authenticate.html.haml b/spec/javascripts/fixtures/u2f/authenticate.html.haml
index 859e79a6c9e..779d6429a5f 100644
--- a/spec/javascripts/fixtures/u2f/authenticate.html.haml
+++ b/spec/javascripts/fixtures/u2f/authenticate.html.haml
@@ -1 +1 @@
-= render partial: "u2f/authenticate", locals: { new_user_session_path: "/users/sign_in" }
+= render partial: "u2f/authenticate", locals: { new_user_session_path: "/users/sign_in", params: {}, resource_name: "user" }