Commit 516100c5 authored by Robert Speicher's avatar Robert Speicher

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
parents 27983533 9224f031
......@@ -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)
......
......@@ -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.'
......
......@@ -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 }
......@@ -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"
......
......@@ -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)
......
......@@ -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
......
= 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" }
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