diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/controllers/concerns/authenticates_with_two_factor.rb | 1 | ||||
| -rw-r--r-- | app/views/devise/sessions/two_factor.html.haml | 3 | ||||
| -rw-r--r-- | app/views/u2f/_authenticate.html.haml | 2 | ||||
| -rw-r--r-- | spec/controllers/sessions_controller_spec.rb | 23 | ||||
| -rw-r--r-- | spec/features/u2f_spec.rb | 14 | ||||
| -rw-r--r-- | spec/javascripts/fixtures/u2f/authenticate.html.haml | 2 |
7 files changed, 43 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG index 934dabe743a..98aac94917e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.12.0 (unreleased) - Filter tags by name !6121 - 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" } |