diff --git a/Gemfile.lock b/Gemfile.lock index b48605158..b985f123e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -20,7 +20,7 @@ GIT GIT remote: https://github.com/baarkerlounger/two_factor_authentication.git - revision: a7522becd7222f1aa4ddf73d7caf19f05bdb4dac + revision: 1fa214d18d311e019a343f836f2c591c0fa3d308 specs: two_factor_authentication (2.2.0) devise @@ -116,17 +116,17 @@ GEM public_suffix (>= 2.0.2, < 5.0) ast (2.4.2) aws-eventstream (1.2.0) - aws-partitions (1.551.0) - aws-sdk-core (3.125.5) + aws-partitions (1.552.0) + aws-sdk-core (3.126.0) aws-eventstream (~> 1, >= 1.0.2) aws-partitions (~> 1, >= 1.525.0) aws-sigv4 (~> 1.1) jmespath (~> 1.0) - aws-sdk-kms (1.53.0) - aws-sdk-core (~> 3, >= 3.125.0) + aws-sdk-kms (1.54.0) + aws-sdk-core (~> 3, >= 3.126.0) aws-sigv4 (~> 1.1) - aws-sdk-s3 (1.111.3) - aws-sdk-core (~> 3, >= 3.125.0) + aws-sdk-s3 (1.112.0) + aws-sdk-core (~> 3, >= 3.126.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.4) aws-sigv4 (1.4.0) @@ -170,7 +170,7 @@ GEM railties (>= 3.2) encryptor (3.0.0) erubi (1.10.0) - excon (0.90.0) + excon (0.91.0) factory_bot (6.2.0) activesupport (>= 5.0.0) factory_bot_rails (6.2.0) diff --git a/app/controllers/auth/two_factor_authentication_controller.rb b/app/controllers/auth/two_factor_authentication_controller.rb index d107ef15f..7d820f9af 100644 --- a/app/controllers/auth/two_factor_authentication_controller.rb +++ b/app/controllers/auth/two_factor_authentication_controller.rb @@ -2,4 +2,24 @@ class Auth::TwoFactorAuthenticationController < Devise::TwoFactorAuthenticationC def show_resend render "devise/two_factor_authentication/resend" end + + def update + resource.errors.add :base, I18n.t("devise.two_factor_authentication.code_required") if resource && params_code.empty? + super + end + +private + + def after_two_factor_fail_for(resource) + resource.second_factor_attempts_count += 1 + resource.save! + + if resource.max_login_attempts? + sign_out(resource) + render :max_login_attempts_reached, status: :unprocessable_entity + else + resource.errors.add :base, I18n.t("devise.two_factor_authentication.code_incorrect") if resource + render :show, status: :unprocessable_entity + end + end end diff --git a/app/views/devise/two_factor_authentication/show.html.erb b/app/views/devise/two_factor_authentication/show.html.erb index a742056fe..1eea10171 100644 --- a/app/views/devise/two_factor_authentication/show.html.erb +++ b/app/views/devise/two_factor_authentication/show.html.erb @@ -1,8 +1,9 @@ <% content_for :title, "Check your phone" %> -<%= form_with(url: "/admin/two-factor-authentication", html: { method: :put }) do |f| %> +<%= form_with(model: resource, url: "/admin/two-factor-authentication", html: { method: :put }) do |f| %>
+ <%= f.govuk_error_summary %>

<%= content_for(:title) %> @@ -11,10 +12,10 @@

We’ve sent you a text message with a security code.

<%= f.govuk_number_field :code, - label: { text: "Security code" }, - width: 5, - autocomplete: 'one-time-code', - autofocus: true + label: { text: "Security code" }, + width: 5, + autocomplete: 'one-time-code', + autofocus: true %> <%= f.govuk_submit "Submit" %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 4055cda6c..5b2994020 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -1,7 +1,7 @@ - <%= browser_title(yield(:title), @user, @organisation, @case_log, @resource) %> + <%= browser_title(yield(:title), @admin_user, @user, @organisation, @case_log, @resource) %> <%= csrf_meta_tags %> <%= csp_meta_tag %> <%= tag :meta, name: 'viewport', content: 'width=device-width, initial-scale=1' %> diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 0d9dcbaf7..de41a7b75 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -313,11 +313,11 @@ Devise.setup do |config| # 2FA config.max_login_attempts = 3 # Maximum second factor attempts count. config.allowed_otp_drift_seconds = 30 # Allowed TOTP time drift between client and server. - config.otp_length = 6 # TOTP code length - config.direct_otp_valid_for = 5.minutes # Time before direct OTP becomes invalid - config.direct_otp_length = 6 # Direct OTP code length + config.otp_length = 5 # TOTP code length + config.direct_otp_valid_for = 15.minutes # Time before direct OTP becomes invalid + config.direct_otp_length = 5 # Direct OTP code length config.remember_otp_session_for_seconds = 1.day # Time before browser has to perform 2fA again. Default is 0. config.otp_secret_encryption_key = ENV["OTP_SECRET_ENCRYPTION_KEY"] config.second_factor_resource_id = "id" # Field or method name used to set value for 2fA remember cookie - config.delete_cookie_on_logout = false # Delete cookie when user signs out, to force 2fA again on login + config.delete_cookie_on_logout = true # Delete cookie when user signs out, to force 2fA again on login end diff --git a/spec/features/admin_panel_spec.rb b/spec/features/admin_panel_spec.rb index 45998ac0c..93a0e0a1f 100644 --- a/spec/features/admin_panel_spec.rb +++ b/spec/features/admin_panel_spec.rb @@ -30,13 +30,16 @@ RSpec.describe "Admin Panel" do expect(page).to have_content("Two factor authentication successful.") end - context "but it is more than 5 minutes old" do + context "but it is more than 15 minutes old" do it "does not authenticate successfully" do click_button("Login") - admin.update!(direct_otp_sent_at: 10.minutes.ago) + admin.update!(direct_otp_sent_at: 16.minutes.ago) fill_in("code", with: otp) click_button("Submit") expect(page).to have_content("Check your phone") + expect(page).to have_http_status(:unprocessable_entity) + expect(page).to have_title("Error") + expect(page).to have_selector("#error-summary-title") end end end @@ -50,6 +53,9 @@ RSpec.describe "Admin Panel" do fill_in("code", with: otp) click_button("Submit") expect(page).to have_content("Check your phone") + expect(page).to have_http_status(:unprocessable_entity) + expect(page).to have_title("Error") + expect(page).to have_selector("#error-summary-title") end end @@ -72,4 +78,24 @@ RSpec.describe "Admin Panel" do expect(page).to have_current_path("/admin/two-factor-authentication") end end + + context "when logging out and in again" do + before do + allow(SecureRandom).to receive(:random_number).and_return(otp) + end + + it "requires the 2FA code on each login" do + visit("/admin") + fill_in("admin_user[email]", with: admin.email) + fill_in("admin_user[password]", with: admin.password) + click_button("Login") + fill_in("code", with: otp) + click_button("Submit") + click_link("Logout") + fill_in("admin_user[email]", with: admin.email) + fill_in("admin_user[password]", with: admin.password) + click_button("Login") + expect(page).to have_content("Check your phone") + end + end end