diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 41a3ee3e1c8ab5055aa91156e4b1ac01bd4d9841..ccc1c3e6c855c3b34072b5eaa9f6d87e7a4c2579 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -16,8 +16,7 @@ module AuthenticatesWithTwoFactor # # Returns nil def prompt_for_two_factor(user) - # Set @user for Devise views - @user = user # rubocop:disable Gitlab/ModuleWithInstanceVariables + @user = user # rubocop:disable Gitlab/ModuleWithInstanceVariables -- Set @user for Devise views return handle_locked_user(user) unless user.can?(:log_in) diff --git a/app/controllers/concerns/authenticates_with_two_factor_for_admin_mode.rb b/app/controllers/concerns/authenticates_with_two_factor_for_admin_mode.rb index 045ccf1e5b880856f277d826d031ef13796246e0..0f36e78dab330acbf785d84762212bc7abd44344 100644 --- a/app/controllers/concerns/authenticates_with_two_factor_for_admin_mode.rb +++ b/app/controllers/concerns/authenticates_with_two_factor_for_admin_mode.rb @@ -8,6 +8,8 @@ module AuthenticatesWithTwoFactorForAdminMode end def admin_mode_prompt_for_two_factor(user) + @user = user # rubocop:disable Gitlab/ModuleWithInstanceVariables -- Set @user for Admin views + return handle_locked_user(user) unless user.can?(:log_in) session[:otp_user_id] = user.id diff --git a/app/views/admin/sessions/_two_factor_otp.html.haml b/app/views/admin/sessions/_two_factor_otp.html.haml deleted file mode 100644 index 5978290c66cb89d758cfdef7bb584ca3482a2ff1..0000000000000000000000000000000000000000 --- a/app/views/admin/sessions/_two_factor_otp.html.haml +++ /dev/null @@ -1,10 +0,0 @@ -= form_tag(admin_session_path, { method: :post, class: "edit_user gl-show-field-errors js-2fa-form #{'hidden' if current_user.two_factor_webauthn_enabled?}" }) do - .form-group - = label_tag :user_otp_attempt, _('Enter verification code') - = text_field_tag 'user[otp_attempt]', nil, class: 'form-control', required: true, autofocus: true, autocomplete: 'off', inputmode: 'numeric', title: _('This field is required.') - %p.form-text.text-muted.hint - = _("Enter the code from your two-factor authenticator app. If you've lost your device, you can enter one of your recovery codes.") - - .submit-container - = render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm, block: true) do - = _("Verify code") diff --git a/app/views/admin/sessions/two_factor.html.haml b/app/views/admin/sessions/two_factor.html.haml index 802470be71fc75f1065854386be39cc82a07960b..37a33a11a70159ea99b28ab96879d5a84f5f6c60 100644 --- a/app/views/admin/sessions/two_factor.html.haml +++ b/app/views/admin/sessions/two_factor.html.haml @@ -5,7 +5,4 @@ .col-md-5 .login-page .login-box.gl-p-5 - - if current_user.two_factor_enabled? - = render 'admin/sessions/two_factor_otp' - - if current_user.two_factor_webauthn_enabled? - = render 'authentication/authenticate', render_remember_me: false, target_path: admin_session_path + = render 'devise/shared/totp_recovery_code_or_webauthn', admin_mode: true diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml index c39e6230f69352d7923a7efc6c933f90ae7ad3cb..d305560b097b50dcbe25d008ecaa4e357f19be9e 100644 --- a/app/views/devise/sessions/two_factor.html.haml +++ b/app/views/devise/sessions/two_factor.html.haml @@ -1,18 +1,2 @@ .login-box.gl-p-5 - - if @user.two_factor_enabled? - = gitlab_ui_form_for(resource, as: resource_name, url: session_path(resource_name), method: :post, html: { class: "gl-show-field-errors js-2fa-form #{'hidden' if @user.two_factor_webauthn_enabled?}", aria: { live: 'assertive' }}) do |f| - .form-group - = f.label :otp_attempt, _('Enter verification code') - = f.text_field :otp_attempt, class: 'form-control gl-form-input', required: true, autofocus: true, autocomplete: 'off', inputmode: 'numeric', title: _('This field is required.'), data: { testid: 'two-fa-code-field' } - %p.form-text.text-muted.hint - = _("Enter the code from your two-factor authenticator app. If you've lost your device, you can enter one of your recovery codes.") - - - if remember_me_enabled? - - resource_params = params[resource_name].presence || params - = f.hidden_field :remember_me, value: resource_params.fetch(:remember_me, 0) - - = render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm, block: true, button_options: { data: { testid: 'verify-code-button' } }) do - = _("Verify code") - - - if @user.two_factor_webauthn_enabled? - = render "authentication/authenticate", params: params, resource: resource, resource_name: resource_name, render_remember_me: true, target_path: new_user_session_path + = render 'devise/shared/totp_recovery_code_or_webauthn', admin_mode: false diff --git a/app/views/devise/shared/_totp_recovery_code_or_webauthn.html.haml b/app/views/devise/shared/_totp_recovery_code_or_webauthn.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..52c0ff0d436a960f00a49317876adb70bcc83c21 --- /dev/null +++ b/app/views/devise/shared/_totp_recovery_code_or_webauthn.html.haml @@ -0,0 +1,20 @@ +- target_path = admin_mode ? admin_session_path : user_session_path +- render_remember_me = admin_mode ? false : remember_me_enabled? + += gitlab_ui_form_for(:user, url: target_path, method: :post, html: { class: "gl-show-field-errors js-2fa-form #{'hidden' if @user.two_factor_webauthn_enabled?}", aria: { live: 'assertive' }}) do |f| + .form-group + = f.label :otp_attempt, _('Enter verification code') + -# Note: we use inputmode="numeric", because TOTP. However, recovery codes are alphanumeric. + = f.text_field :otp_attempt, class: 'form-control gl-form-input', required: true, autofocus: true, autocomplete: 'off', inputmode: 'numeric', title: _('This field is required.'), data: { testid: 'two-fa-code-field' } + %p.form-text.text-muted.hint + = _("Enter the code from your two-factor authenticator app. If you've lost your device, you can enter one of your recovery codes.") + + - if render_remember_me + - resource_params = params[:user].presence || params + = f.hidden_field :remember_me, value: resource_params.fetch(:remember_me, 0) + + = render Pajamas::ButtonComponent.new(type: :submit, variant: :confirm, block: true, button_options: { data: { testid: 'verify-code-button' } }) do + = _("Verify code") + +- if @user.two_factor_webauthn_enabled? + = render 'authentication/authenticate', target_path: target_path, render_remember_me: render_remember_me diff --git a/qa/qa/page/main/two_factor_auth.rb b/qa/qa/page/main/two_factor_auth.rb index 186027900cac703a112a66eddaabeaa499714dc4..2da0acf36aa89ce6415a246a6dadbc239d29766f 100644 --- a/qa/qa/page/main/two_factor_auth.rb +++ b/qa/qa/page/main/two_factor_auth.rb @@ -4,7 +4,7 @@ module QA module Page module Main class TwoFactorAuth < Page::Base - view 'app/views/devise/sessions/two_factor.html.haml' do + view 'app/views/devise/shared/_totp_recovery_code_or_webauthn.html.haml' do element 'verify-code-button' element 'two-fa-code-field' end diff --git a/spec/views/admin/sessions/two_factor.html.haml_spec.rb b/spec/views/admin/sessions/two_factor.html.haml_spec.rb index 9ac9356b91a6db6f7eef720118e29d66ef6a9f22..334a31f8247d4b70a8df741b827e0a2d20e7f1e2 100644 --- a/spec/views/admin/sessions/two_factor.html.haml_spec.rb +++ b/spec/views/admin/sessions/two_factor.html.haml_spec.rb @@ -2,20 +2,9 @@ require 'spec_helper' -RSpec.describe 'admin/sessions/two_factor.html.haml' do +RSpec.describe 'admin/sessions/two_factor.html.haml', feature_category: :system_access do before do - allow(view).to receive(:current_user).and_return(user) - end - - context 'user has no two factor auth' do - let(:user) { create(:admin) } - - it 'shows tab' do - render - - expect(rendered).to have_no_field('user[otp_attempt]') - expect(rendered).to have_no_field('user[device_response]') - end + assign(:user, user) end context 'user has otp active' do