diff --git a/app/decorators/user_decorator.rb b/app/decorators/user_decorator.rb index e835f1adfc4..5245f618991 100644 --- a/app/decorators/user_decorator.rb +++ b/app/decorators/user_decorator.rb @@ -34,19 +34,8 @@ def visible_email_addresses end end - def lockout_time_remaining_in_words - current_time = Time.zone.now - - distance_of_time_in_words( - current_time, - current_time + lockout_time_remaining, - true, - highest_measures: 2, - ) - end - - def lockout_time_remaining - (lockout_period - (Time.zone.now - user.second_factor_locked_at)).to_i + def lockout_time_expiration + user.second_factor_locked_at + lockout_period end def active_identity_for(service_provider) @@ -150,6 +139,6 @@ def lockout_period_config end def lockout_period_expired? - (Time.zone.now - user.second_factor_locked_at) > lockout_period + lockout_time_expiration < Time.zone.now end end diff --git a/app/presenters/failure_presenter.rb b/app/presenters/failure_presenter.rb index b4bad548bbd..e028c3fe0c5 100644 --- a/app/presenters/failure_presenter.rb +++ b/app/presenters/failure_presenter.rb @@ -32,11 +32,9 @@ def title; end def header; end - def description; end + def description(_view_context); end def troubleshooting_options [] end - - def js; end end diff --git a/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb b/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb index 81cc3d63e46..c23e7fa9c4b 100644 --- a/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb +++ b/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb @@ -5,8 +5,6 @@ class MaxAttemptsReachedPresenter < FailurePresenter attr_reader :type, :decorated_user - COUNTDOWN_ID = 'countdown'.freeze - def initialize(type, decorated_user) super(:locked) @type = type @@ -21,23 +19,14 @@ def header t('titles.account_locked') end - def description - [locked_reason, please_try_again] + def description(view_context) + [locked_reason, please_try_again(view_context)] end def troubleshooting_options [read_about_two_factor_authentication, contact_support] end - def js - <<~JS - document.addEventListener('DOMContentLoaded', function() { - var test = #{decorated_user.lockout_time_remaining} * 1000; - window.LoginGov.countdownTimer(document.getElementById('#{COUNTDOWN_ID}'), test); - }); - JS - end - def locked_reason case type.to_s when 'backup_code_login_attempts' @@ -57,11 +46,12 @@ def locked_reason end end - def please_try_again + def please_try_again(view_context) t( 'two_factor_authentication.please_try_again_html', - id: COUNTDOWN_ID, - time_remaining: decorated_user.lockout_time_remaining_in_words, + countdown: view_context.render( + CountdownComponent.new(expiration: decorated_user.lockout_time_expiration), + ), ) end diff --git a/app/views/shared/_failure.html.erb b/app/views/shared/_failure.html.erb index 95d9fd804b4..61a9a298888 100644 --- a/app/views/shared/_failure.html.erb +++ b/app/views/shared/_failure.html.erb @@ -6,7 +6,7 @@ <%= render PageHeadingComponent.new.with_content(presenter.header) %> <% end %> -<% Array(presenter.description).each do |description_p| %> +<% Array(presenter.description(self)).each do |description_p| %>
<%= description_p %>
<% end %> @@ -15,7 +15,3 @@ heading: t('components.troubleshooting_options.default_heading'), options: presenter.troubleshooting_options, ) %> - -<% if presenter.js %> - <%= backwards_compatible_javascript_tag presenter.js %> -<% end %> diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index e03195fddc3..42e5cb3abe2 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -113,7 +113,7 @@ en: question: Don’t have your PIV or CAC available? piv_cac_header_text: Present your PIV/CAC piv_cac_webauthn_available: Use your security key - please_try_again_html: Please try again in %{time_remaining}. + please_try_again_html: Please try again in %{countdown}. read_about_two_factor_authentication: Read about two-factor authentication totp_fallback: question: Don’t have your authenticator app? diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 90dfe13dbe9..b564c8d0e3e 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -123,7 +123,7 @@ es: question: '¿No tienes su PIV o CAC disponibles?' piv_cac_header_text: Presenta tu PIV/CAC piv_cac_webauthn_available: Utilice su llave de seguridad - please_try_again_html: Inténtelo de nuevo en %{time_remaining}. + please_try_again_html: Inténtelo de nuevo en %{countdown}. read_about_two_factor_authentication: Conozca la autenticación de dos factores totp_fallback: question: '¿No tiene su aplicación de autenticación?' diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index ebb7b5e33aa..3c5934fb37e 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -127,8 +127,7 @@ fr: question: Votre PIV ou CAC n’est pas disponible? piv_cac_header_text: Veuillez présenter votre carte PIV/CAC piv_cac_webauthn_available: Utilisez votre clé de sécurité - please_try_again_html: Veuillez essayer de nouveau dans %{time_remaining}. + please_try_again_html: Veuillez essayer de nouveau dans %{countdown}. read_about_two_factor_authentication: En savoir plus sur l’authentification à deux facteurs totp_fallback: question: Vous n’avez pas votre application d’authentification? diff --git a/spec/decorators/user_decorator_spec.rb b/spec/decorators/user_decorator_spec.rb index 2ce781cca88..1b9925d6df9 100644 --- a/spec/decorators/user_decorator_spec.rb +++ b/spec/decorators/user_decorator_spec.rb @@ -66,27 +66,14 @@ end end - describe '#lockout_time_remaining' do - it 'returns the difference in seconds between otp drift and second_factor_locked_at' do + describe '#lockout_time_expiration' do + it 'returns the time at which lockout will expire' do freeze_time do user = build_stubbed(:user, second_factor_locked_at: Time.zone.now - 180) user_decorator = UserDecorator.new(user) allow(IdentityConfig.store).to receive(:lockout_period_in_minutes).and_return(8) - expect(user_decorator.lockout_time_remaining).to eq 300 - end - end - end - - describe '#lockout_time_remaining_in_words' do - it 'converts lockout_time_remaining to words representing minutes and seconds left' do - freeze_time do - user = build_stubbed(:user, second_factor_locked_at: Time.zone.now - 181) - user_decorator = UserDecorator.new(user) - allow(IdentityConfig.store).to receive(:lockout_period_in_minutes).and_return(8) - - expect(user_decorator.lockout_time_remaining_in_words). - to eq '4 minutes and 59 seconds' + expect(user_decorator.lockout_time_expiration).to eq Time.zone.now + 300 end end end @@ -206,6 +193,60 @@ end end + describe '#locked_out?' do + let(:locked_at) { nil } + let(:user) { User.new } + + before { allow(user).to receive(:second_factor_locked_at).and_return(locked_at) } + + around do |ex| + freeze_time { ex.run } + end + + subject(:locked_out?) { UserDecorator.new(user).locked_out? } + + it { expect(locked_out?).to eq(false) } + + context 'second factor locked out recently' do + let(:locked_at) { Time.zone.now } + + it { expect(locked_out?).to eq(true) } + end + + context 'second factor locked out a while ago' do + let(:locked_at) { Time.zone.now - UserDecorator::DEFAULT_LOCKOUT_PERIOD - 1.second } + + it { expect(locked_out?).to eq(false) } + end + end + + describe '#no_longer_locked_out?' do + let(:locked_at) { nil } + let(:user) { User.new } + + before { allow(user).to receive(:second_factor_locked_at).and_return(locked_at) } + + around do |ex| + freeze_time { ex.run } + end + + subject(:no_longer_locked_out?) { UserDecorator.new(user).no_longer_locked_out? } + + it { expect(no_longer_locked_out?).to eq(false) } + + context 'second factor locked out recently' do + let(:locked_at) { Time.zone.now } + + it { expect(no_longer_locked_out?).to eq(false) } + end + + context 'second factor locked out a while ago' do + let(:locked_at) { Time.zone.now - UserDecorator::DEFAULT_LOCKOUT_PERIOD - 1.second } + + it { expect(no_longer_locked_out?).to eq(true) } + end + end + describe '#recent_events' do let!(:user) { create(:user, :signed_up, created_at: Time.zone.now - 100.days) } let(:decorated_user) { user.decorate } diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index 0db5b161292..78d898b5c91 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -488,4 +488,18 @@ def attempt_to_bypass_2fa expect(current_url).to eq MarketingSite.security_and_privacy_practices_url end end + + describe 'second factor locked' do + before do + allow_any_instance_of(UserDecorator).to receive(:locked_out?).and_return(true) + allow_any_instance_of(UserDecorator).to receive(:lockout_time_expiration). + and_return(Time.zone.now + 10.minutes) + end + + scenario 'presents the failure screen', :js do + sign_in_user(user_with_2fa) + + expect(page).to have_content t('titles.account_locked') + end + end end diff --git a/spec/presenters/failure_presenter_spec.rb b/spec/presenters/failure_presenter_spec.rb index ba2cfa27d4f..5ee6f80dfb0 100644 --- a/spec/presenters/failure_presenter_spec.rb +++ b/spec/presenters/failure_presenter_spec.rb @@ -11,13 +11,19 @@ end context 'methods with default values of `nil`' do - %i[title header description].each do |method| + %i[title header].each do |method| describe "##{method}" do subject { presenter.send(method) } it { is_expected.to be_nil } end end + + describe '#description' do + subject { presenter.description(ActionController::Base.new.view_context) } + + it { is_expected.to be_nil } + end end describe '#troubleshooting_options' do diff --git a/spec/presenters/max_attempts_reached_presenter_spec.rb b/spec/presenters/max_attempts_reached_presenter_spec.rb index 15dcd8d12a0..6ead72deddd 100644 --- a/spec/presenters/max_attempts_reached_presenter_spec.rb +++ b/spec/presenters/max_attempts_reached_presenter_spec.rb @@ -3,8 +3,13 @@ describe TwoFactorAuthCode::MaxAttemptsReachedPresenter do let(:type) { 'otp_requests' } let(:decorated_user) { mock_decorated_user } + let(:view_context) { ActionController::Base.new.view_context } let(:presenter) { described_class.new(type, decorated_user) } + around do |ex| + freeze_time { ex.run } + end + describe 'it uses the :locked failure state' do subject { presenter.state } @@ -24,23 +29,29 @@ end context 'methods are overriden' do - %i[title header description js].each do |method| + %i[title header].each do |method| describe "##{method}" do subject { presenter.send(method) } it { is_expected.to_not be_nil } end end + + describe '#description' do + subject { presenter.description(view_context) } + + it { is_expected.to_not be_nil } + end end describe '#description' do - subject(:description) { presenter.description } + subject(:description) { presenter.description(view_context) } it 'includes failure type and time remaining' do expect(subject).to eq( [ presenter.locked_reason, - presenter.please_try_again, + presenter.please_try_again(view_context), ], ) end @@ -76,10 +87,10 @@ end describe '#please_try_again' do - subject { presenter.send(:please_try_again) } + subject { presenter.please_try_again(view_context) } it 'includes time remaining' do - expect(subject).to include('1000 years') + expect(subject).to include('1 minute') end end @@ -109,8 +120,7 @@ def mock_decorated_user decorated_user = instance_double(UserDecorator) - allow(decorated_user).to receive(:lockout_time_remaining_in_words).and_return('1000 years') - allow(decorated_user).to receive(:lockout_time_remaining).and_return(10_000) + allow(decorated_user).to receive(:lockout_time_expiration).and_return(Time.zone.now + 1.minute) decorated_user end end diff --git a/spec/views/shared/_failure.html.erb_spec.rb b/spec/views/shared/_failure.html.erb_spec.rb index d5b38f06729..3b876bcc56d 100644 --- a/spec/views/shared/_failure.html.erb_spec.rb +++ b/spec/views/shared/_failure.html.erb_spec.rb @@ -16,13 +16,11 @@ let(:header) { 'header' } let(:description) { 'description' } let(:troubleshooting_options) { [{ text: 'option', url: 'https://example.com' }] } - let(:js) { '/* */' } before do allow(presenter).to receive(:header).and_return(header) allow(presenter).to receive(:description).and_return(description) allow(presenter).to receive(:troubleshooting_options).and_return(troubleshooting_options) - allow(presenter).to receive(:js).and_return(js) end it 'renders content' do @@ -34,7 +32,6 @@ troubleshooting_options[0][:text], href: troubleshooting_options[0][:url], ) - expect(rendered).to have_css('script', visible: :all) end context 'with array description' do