diff --git a/.reek b/.reek index edce9b7387c..37bd18db79d 100644 --- a/.reek +++ b/.reek @@ -11,6 +11,7 @@ DuplicateMethodCall: - needs_to_confirm_email_change? - WorkerHealthChecker#status - FileEncryptor#encrypt + - UserFlowExporter#self.massage_assets FeatureEnvy: exclude: - track_registration @@ -22,6 +23,7 @@ FeatureEnvy: - UserDecorator#should_acknowledge_personal_key? - Pii::Attributes#[]= - OpenidConnectLogoutForm#load_identity + - Idv::ProfileMaker#pii_from_applicant InstanceVariableAssumption: exclude: - User @@ -33,6 +35,7 @@ ManualDispatch: NestedIterators: exclude: - FileEncryptor#encrypt + - UserFlowExporter#self.massage_html NilCheck: enabled: false LongParameterList: @@ -58,6 +61,9 @@ TooManyStatements: - SamlIdpAuthConcern#store_saml_request - Users::PhoneConfirmationController - FileEncryptor#encrypt + - UserFlowExporter#self.massage_assets + - UserFlowExporter#self.massage_html + - UserFlowExporter#self.run TooManyMethods: exclude: - Users::ConfirmationsController diff --git a/.rubocop.yml b/.rubocop.yml index cd7b91340b4..b2afe411f82 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -8,6 +8,7 @@ AllCops: Include: - '**/Gemfile' - '**/Rakefile' + - '**/Capfile' Exclude: - 'db/schema.rb' - 'bin/**/*' @@ -17,6 +18,7 @@ AllCops: - 'lib/rspec/user_flow_formatter.rb' - 'lib/user_flow_exporter.rb' TargetRubyVersion: 2.3 + TargetRailsVersion: 4.0 UseCache: true Rails: @@ -119,7 +121,7 @@ Rails/TimeZone: - strict - flexible -Style/AlignParameters: +Layout/AlignParameters: # Alignment of parameters in multi-line method calls. # # The `with_first_parameter` style aligns the following lines along the same @@ -160,7 +162,7 @@ Style/Documentation: - 'spec/**/*' - 'test/**/*' -Style/DotPosition: +Layout/DotPosition: Description: Checks the position of the dot in multi-line method calls. StyleGuide: https://github.com/bbatsov/ruby-style-guide#consistent-multi-line-chains EnforcedStyle: trailing @@ -179,7 +181,7 @@ Style/EmptyElse: - nil - both -Style/ExtraSpacing: +Layout/ExtraSpacing: # When true, allows most uses of extra spacing if the intent is to align # things with the previous or next line, not counting empty lines or comment # lines. @@ -200,7 +202,7 @@ Style/IfUnlessModifier: MaxLineLength: 80 # Checks the indentation of the first element in an array literal. -Style/IndentArray: +Layout/IndentArray: # The value `special_inside_parentheses` means that array literals with # brackets that have their opening bracket on the same line as a surrounding # opening round parenthesis, shall have their first element indented relative @@ -221,7 +223,7 @@ Style/IndentArray: # But it can be overridden by setting this parameter IndentationWidth: ~ -Style/MultilineOperationIndentation: +Layout/MultilineOperationIndentation: EnforcedStyle: aligned SupportedStyles: - aligned diff --git a/.slim-lint.yml b/.slim-lint.yml index aa3c71c0c74..5e6fc90f968 100644 --- a/.slim-lint.yml +++ b/.slim-lint.yml @@ -1,3 +1,5 @@ +exclude: + - 'app/views/shared/_flashes.html.slim' linters: LineLength: max: 100 @@ -6,35 +8,3 @@ linters: - 'app/views/pages/privacy_policy.html.slim' RuboCop: enabled: true - # slim-lint ignores all of these cops by default, and we need to copy them - # all over if we want to add more cops to the list. We had to do this to - # ignore the Metrics/BlockLength cop, which will hopefully be ignored by - # default if this issue is addressed: https://github.com/sds/slim-lint/issues/49 - # If it gets added to the default list of ignored cops, we can remove this - # whole RuboCop section. - ignored_cops: - - Lint/BlockAlignment - - Lint/EndAlignment - - Lint/Void - - Metrics/BlockLength - - Metrics/LineLength - - Style/AlignHash - - Style/AlignParameters - - Style/BlockNesting - - Style/FileName - - Style/FirstParameterIndentation - - Style/FrozenStringLiteralComment - - Style/IfUnlessModifier - - Style/IndentationConsistency - - Style/IndentationWidth - - Style/MultilineArrayBraceLayout - - Style/MultilineAssignmentLayout - - Style/MultilineHashBraceLayout - - Style/MultilineMethodCallBraceLayout - - Style/MultilineMethodDefinitionBraceLayout - - Style/MultilineMethodCallIndentation - - Style/MultilineOperationIndentation - - Style/Next - - Style/TrailingBlankLines - - Style/TrailingWhitespace - - Style/WhileUntilModifier diff --git a/Capfile b/Capfile index 54388cbc401..6661c0cf636 100644 --- a/Capfile +++ b/Capfile @@ -21,7 +21,7 @@ require 'new_relic/recipes' require 'whenever/capistrano' # support for git -require "capistrano/scm/git" +require 'capistrano/scm/git' install_plugin Capistrano::SCM::Git # Loads custom tasks from `lib/capistrano/tasks' if you have any defined. diff --git a/Gemfile b/Gemfile index 4348815513d..0f311402cc9 100644 --- a/Gemfile +++ b/Gemfile @@ -19,6 +19,7 @@ gem 'gibberish' gem 'gyoku' gem 'hashie' gem 'hiredis' +gem 'http_accept_language' gem 'httparty' gem 'json-jwt' gem 'lograge' diff --git a/Gemfile.lock b/Gemfile.lock index 672caac3417..1b95a8443b9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: git@github.com:18F/identity-equifax-api-client-gem.git - revision: 979a3342a2d08ee6ea2c444b846e4e15d50a0891 + revision: 4308a502baf7b65e8b463ecafc2d428d530b4349 branch: master specs: equifax (1.0.0) @@ -117,7 +117,7 @@ GEM american_date (1.1.1) arel (6.0.4) ast (2.3.0) - aws-sdk-core (2.9.20) + aws-sdk-core (2.10.1) aws-sigv4 (~> 1.0) jmespath (~> 1.0) aws-sigv4 (1.0.0) @@ -135,10 +135,10 @@ GEM coderay (>= 1.0.0) erubis (>= 2.6.6) rack (>= 0.9.0) - bindata (2.3.5) + bindata (2.4.0) binding_of_caller (0.7.2) debug_inspector (>= 0.0.1) - brakeman (3.6.1) + brakeman (3.6.2) browser (2.4.0) browserify-rails (4.2.0) addressable (>= 2.4.0) @@ -228,7 +228,7 @@ GEM rails (>= 3.1.1) diff-lcs (1.3) docile (1.1.5) - dotenv (2.2.0) + dotenv (2.2.1) dotiw (3.1.1) actionpack (>= 3) i18n @@ -293,12 +293,13 @@ GEM highline (1.7.8) hiredis (0.6.1) htmlentities (4.3.4) + http_accept_language (2.1.0) httparty (0.15.4) multi_xml (>= 0.5.2) httpi (2.4.2) rack socksify - i18n (0.8.1) + i18n (0.8.4) i18n-tasks (0.9.15) activesupport (>= 4.0.2) ast (>= 2.1.0) @@ -313,7 +314,7 @@ GEM iniparse (1.4.2) jmespath (1.3.1) json (2.1.0) - json-jwt (1.7.1) + json-jwt (1.7.2) activesupport bindata multi_json (>= 1.3) @@ -351,7 +352,7 @@ GEM mime-types (3.1) mime-types-data (~> 3.2015) mime-types-data (3.2016.0521) - mini_portile2 (2.1.0) + mini_portile2 (2.2.0) minitest (5.10.2) multi_json (1.12.1) multi_xml (0.6.0) @@ -359,9 +360,9 @@ GEM net-scp (1.2.1) net-ssh (>= 2.6.5) net-ssh (4.1.0) - newrelic_rpm (4.1.0.333) - nokogiri (1.7.2) - mini_portile2 (~> 2.1.0) + newrelic_rpm (4.2.0.334) + nokogiri (1.8.0) + mini_portile2 (~> 2.2.0) nori (2.6.0) notiffany (0.1.1) nenv (~> 0.1) @@ -370,13 +371,14 @@ GEM overcommit (0.39.1) childprocess (~> 0.6.3) iniparse (~> 1.4) + parallel (1.11.2) parser (2.4.0.0) ast (~> 2.2) - pg (0.20.0) - phony (2.15.42) - phony_rails (0.14.5) + pg (0.21.0) + phony (2.15.44) + phony_rails (0.14.6) activesupport (>= 3.0) - phony (~> 2.15) + phony (> 2.15) poltergeist (1.15.0) capybara (~> 2.1) cliver (~> 0.3.1) @@ -403,7 +405,7 @@ GEM rack-attack (5.0.1) rack rack-cors (0.4.1) - rack-mini-profiler (0.10.4) + rack-mini-profiler (0.10.5) rack (>= 1.2.0) rack-protection (1.5.3) rack @@ -429,7 +431,7 @@ GEM activesupport (>= 4.2.0.beta, < 5.0) nokogiri (~> 1.6) rails-deprecated_sanitizer (>= 1.0.1) - rails-erd (1.5.0) + rails-erd (1.5.2) activerecord (>= 3.2) activesupport (>= 3.2) choice (~> 0.2.0) @@ -453,7 +455,7 @@ GEM connection_pool (~> 2.1) redis (~> 3.0) redis (3.3.3) - reek (4.6.2) + reek (4.7.1) codeclimate-engine-rb (~> 0.4.0) parser (>= 2.4.0.0, < 2.5) rainbow (~> 2.0) @@ -486,7 +488,8 @@ GEM rspec-mocks (~> 3.5.0) rspec-support (~> 3.5.0) rspec-support (3.5.0) - rubocop (0.48.1) + rubocop (0.49.1) + parallel (~> 1.10) parser (>= 2.3.3.1, < 3.0) powerpack (~> 0.1) rainbow (>= 1.99.1, < 3.0) @@ -494,7 +497,7 @@ GEM unicode-display_width (~> 1.0, >= 1.0.1) ruby-graphviz (1.2.3) ruby-progressbar (1.8.1) - ruby-saml (1.4.2) + ruby-saml (1.4.3) nokogiri (>= 1.5.10) ruby_dep (1.5.0) ruby_parser (3.8.4) @@ -519,18 +522,18 @@ GEM wasabi (~> 3.4) scrypt (3.0.5) ffi-compiler (>= 1.0, < 2.0) - secure_headers (3.6.4) + secure_headers (3.6.5) useragent securecompare (1.0.0) sexp_processor (4.8.0) shellany (0.0.1) shoulda-matchers (3.1.1) activesupport (>= 4.0.0) - sidekiq (4.2.10) + sidekiq (5.0.3) concurrent-ruby (~> 1.0) connection_pool (~> 2.2, >= 2.2.0) rack-protection (>= 1.5.0) - redis (~> 3.2, >= 3.2.1) + redis (~> 3.3, >= 3.3.3) simple_form (3.5.0) actionpack (> 4, < 5.2) activemodel (> 4, < 5.2) @@ -546,16 +549,16 @@ GEM skinny (0.2.4) eventmachine (~> 1.0.0) thin (>= 1.5, < 1.7) - slim (3.0.7) - temple (~> 0.7.6) + slim (3.0.8) + temple (>= 0.7.6, < 0.9) tilt (>= 1.3.3, < 2.1) slim-rails (3.1.2) actionpack (>= 3.1) railties (>= 3.1) slim (~> 3.0) - slim_lint (0.12.0) + slim_lint (0.13.0) rake (>= 10, < 13) - rubocop (>= 0.48.0) + rubocop (>= 0.49.0) slim (~> 3.0) sysexits (~> 1.1) slop (3.6.0) @@ -578,7 +581,7 @@ GEM railties (>= 3.2.5, < 6) teaspoon-mocha (2.3.3) teaspoon (>= 1.0.0) - temple (0.7.7) + temple (0.8.0) terminal-table (1.8.0) unicode-display_width (~> 1.1, >= 1.1.1) test_after_commit (1.1.0) @@ -591,7 +594,7 @@ GEM thread (0.2.2) thread_safe (0.3.6) tilt (2.0.7) - timecop (0.8.1) + timecop (0.9.0) twilio-ruby (4.13.0) builder (>= 2.1.2) jwt (~> 1.0) @@ -600,7 +603,7 @@ GEM thread_safe (~> 0.1) uglifier (3.2.0) execjs (>= 0.3.0, < 3) - unicode-display_width (1.2.1) + unicode-display_width (1.3.0) uniform_notifier (1.10.0) url_safe_base64 (0.2.2) user_agent_parser (2.3.1) @@ -631,7 +634,7 @@ GEM whenever (0.9.7) chronic (>= 0.6.3) xml-simple (1.1.5) - xmlenc (0.6.8) + xmlenc (0.6.9) activemodel (>= 3.0.0) activesupport (>= 3.0.0) nokogiri (>= 1.6.0, < 2.0.0) @@ -685,6 +688,7 @@ DEPENDENCIES gyoku hashie hiredis + http_accept_language httparty i18n-tasks json-jwt @@ -746,4 +750,4 @@ RUBY VERSION ruby 2.3.3p222 BUNDLED WITH - 1.14.6 + 1.15.1 diff --git a/app/assets/images/alert/notice.svg b/app/assets/images/alert/notice.svg new file mode 100644 index 00000000000..79a9e96d604 --- /dev/null +++ b/app/assets/images/alert/notice.svg @@ -0,0 +1 @@ +warning diff --git a/app/assets/images/clock.svg b/app/assets/images/clock.svg index be085ccec15..821a9250254 100644 --- a/app/assets/images/clock.svg +++ b/app/assets/images/clock.svg @@ -1 +1,38 @@ -clock \ No newline at end of file + + + +timeout + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/app/assets/javascripts/misc/reactivate-account-modal.js b/app/assets/javascripts/misc/reactivate-account-modal.js new file mode 100644 index 00000000000..527a760cf00 --- /dev/null +++ b/app/assets/javascripts/misc/reactivate-account-modal.js @@ -0,0 +1,13 @@ +const modal = new window.LoginGov.Modal({ el: '#reactivate-account-modal' }); +const modalTrigger = document.getElementById('no-key-reactivate'); +const modalDismiss = document.getElementById('no-key-reactivate-dismiss'); + +modalTrigger.addEventListener('click', (event) => { + event.preventDefault(); + modal.show(); +}); + +modalDismiss.addEventListener('click', (event) => { + event.preventDefault(); + modal.hide(); +}); diff --git a/app/assets/stylesheets/components/_alert.scss b/app/assets/stylesheets/components/_alert.scss index 69b6475a4ca..48dd85515eb 100644 --- a/app/assets/stylesheets/components/_alert.scss +++ b/app/assets/stylesheets/components/_alert.scss @@ -2,7 +2,7 @@ $ico-size: 1rem; $ico-offset: 1rem; .alert { - background-color: $blue-light; + background-color: $blue-lighter; border-radius: $space-1; color: #5b616a; font-size: 1rem; @@ -51,3 +51,8 @@ $ico-offset: 1rem; &::before { background-image: url(image-path('alert/warning.svg')); } } + +.alert-notice { + padding-left: $space-4; + &::before { background-image: url(image-path('alert/notice.svg')); } +} diff --git a/app/assets/stylesheets/components/_card.scss b/app/assets/stylesheets/components/_card.scss index bd61f12f62a..017d0addade 100644 --- a/app/assets/stylesheets/components/_card.scss +++ b/app/assets/stylesheets/components/_card.scss @@ -11,5 +11,13 @@ @media #{$breakpoint-sm} { border-radius: 5px; + + &-wide { + margin-top: $space-4; + max-width: 100%; + padding-bottom: 0; + padding-left: 0; + padding-right: 0; + } } } diff --git a/app/assets/stylesheets/components/_container.scss b/app/assets/stylesheets/components/_container.scss index addf6d7c712..5bed3a6d2c0 100644 --- a/app/assets/stylesheets/components/_container.scss +++ b/app/assets/stylesheets/components/_container.scss @@ -1,3 +1,7 @@ .cntnr-skinny { max-width: $container-skinny-width; } .cntnr-xskinny { max-width: $container-xskinny-width; } .cntnr-xxskinny { max-width: $container-xxskinny-width; } + +@media #{$breakpoint-sm} { + .cntnr-xxskinny { max-width: $container-xskinny-width; } +} diff --git a/app/assets/stylesheets/components/_modal.scss b/app/assets/stylesheets/components/_modal.scss index 6c0425c01da..c73edfaf876 100644 --- a/app/assets/stylesheets/components/_modal.scss +++ b/app/assets/stylesheets/components/_modal.scss @@ -67,7 +67,7 @@ content: ''; display: block; height: 48px; - margin: 0 auto; + margin: $space-2 auto $space-3; position: relative; width: 48px; } @@ -96,6 +96,16 @@ width: 50px; } +.modal-timeout { + &::before { + background-image: url(image-path('clock.svg')); + } + + hr { + border-color: $teal; + } +} + .modal-warning { &::before { background-image: url(image-path('alert/warning-lg.svg')); diff --git a/app/assets/stylesheets/components/_personal-key.scss b/app/assets/stylesheets/components/_personal-key.scss index bb10d945530..2add0346dcc 100644 --- a/app/assets/stylesheets/components/_personal-key.scss +++ b/app/assets/stylesheets/components/_personal-key.scss @@ -1,3 +1,14 @@ +.key-badge::before { + background-image: url(image-path('p-key.svg')); + background-repeat: no-repeat; + content: ''; + height: 60px; + left: 45%; + position: absolute; + top: -25px; + width: 60px; +} + .separator-text > div { &::after { color: $silver; diff --git a/app/assets/stylesheets/components/_util.scss b/app/assets/stylesheets/components/_util.scss index 26b80beb1f0..e7eae1c18a6 100644 --- a/app/assets/stylesheets/components/_util.scss +++ b/app/assets/stylesheets/components/_util.scss @@ -19,11 +19,20 @@ vertical-align: middle; } - .block-center { margin: 0 auto; } +.js-fallback { + display: none; +} + +.no-js { + .js-fallback { + display: block; + } +} + .scale-down { // trigger anti-aliasing in chrome backface-visibility: hidden; diff --git a/app/assets/stylesheets/variables/_colors.scss b/app/assets/stylesheets/variables/_colors.scss index 11dd1fef9b4..84545970702 100644 --- a/app/assets/stylesheets/variables/_colors.scss +++ b/app/assets/stylesheets/variables/_colors.scss @@ -1,6 +1,7 @@ $aqua: #7fdbff !default; $blue: #0071bb !default; $blue-light: #ebf3fa !default; +$blue-lighter: #ecfcff !default; $blue-lightest: #f2f9ff !default; $navy: #112e51 !default; $teal: #00bfe7 !default; diff --git a/app/assets/stylesheets/variables/_web.scss b/app/assets/stylesheets/variables/_web.scss index b845ee43ae2..7a632869bf9 100644 --- a/app/assets/stylesheets/variables/_web.scss +++ b/app/assets/stylesheets/variables/_web.scss @@ -101,5 +101,5 @@ $breakpoint-md-lg: '(min-width: 52em) and (max-width: 64em)' !default; $container-width: 780px !default; $container-skinny-width: 620px !default; -$container-xskinny-width: 540px !default; -$container-xxskinny-width: 396px !default; +$container-xskinny-width: 416px !default; +$container-xxskinny-width: 296px !default; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 24986d7c45a..76da4d0180a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -46,7 +46,10 @@ def create_user_event(event_type, user = current_user) def decorated_session @_decorated_session ||= DecoratedSession.new( - sp: current_sp, view_context: view_context, sp_session: sp_session + sp: current_sp, + view_context: view_context, + sp_session: sp_session, + service_provider_request: service_provider_request ).call end @@ -74,11 +77,14 @@ def sp_from_sp_session end def sp_from_request_id - issuer = ServiceProviderRequest.from_uuid(params[:request_id]).issuer - sp = ServiceProvider.from_issuer(issuer) + sp = ServiceProvider.from_issuer(service_provider_request.issuer) sp if sp.is_a? ServiceProvider end + def service_provider_request + @service_provider_request ||= ServiceProviderRequest.from_uuid(params[:request_id]) + end + def after_sign_in_path_for(user) stored_location_for(user) || sp_session[:request_url] || signed_in_path end @@ -130,7 +136,9 @@ def skip_session_expiration end def set_locale - I18n.locale = params[:locale] || I18n.default_locale + I18n.locale = + http_accept_language.compatible_language_from(I18n.available_locales) || + I18n.default_locale end def sp_session diff --git a/app/controllers/concerns/account_recovery_concern.rb b/app/controllers/concerns/account_recovery_concern.rb new file mode 100644 index 00000000000..46a36b11026 --- /dev/null +++ b/app/controllers/concerns/account_recovery_concern.rb @@ -0,0 +1,15 @@ +module AccountRecoveryConcern + extend ActiveSupport::Concern + + def confirm_password_reset_profile + return if current_user.decorate.password_reset_profile + redirect_to root_url + end + + def reactivate_account_session + @_reactivate_account_session ||= ReactivateAccountSession.new( + user: current_user, + user_session: user_session + ) + end +end diff --git a/app/controllers/concerns/saml_idp_auth_concern.rb b/app/controllers/concerns/saml_idp_auth_concern.rb index 6b7616d1e82..c2b5e8f9e91 100644 --- a/app/controllers/concerns/saml_idp_auth_concern.rb +++ b/app/controllers/concerns/saml_idp_auth_concern.rb @@ -23,8 +23,6 @@ def validate_service_provider_and_authn_context end def store_saml_request - return if sp_session[:request_id] - @request_id = SecureRandom.uuid ServiceProviderRequest.find_or_create_by(uuid: @request_id) do |sp_request| sp_request.issuer = current_issuer @@ -35,15 +33,7 @@ def store_saml_request end def add_sp_metadata_to_session - return if sp_session[:request_id] - - session[:sp] = { - issuer: current_issuer, - loa3: loa3_requested?, - request_id: @request_id, - request_url: request.original_url, - requested_attributes: requested_attributes, - } + StoreSpMetadataInSession.new(session: session, request_id: @request_id).call end def requested_authn_context diff --git a/app/controllers/concerns/two_factor_authenticatable.rb b/app/controllers/concerns/two_factor_authenticatable.rb index f7a5818d6ce..e728c786c23 100644 --- a/app/controllers/concerns/two_factor_authenticatable.rb +++ b/app/controllers/concerns/two_factor_authenticatable.rb @@ -33,6 +33,16 @@ def handle_second_factor_locked_user(type) ) end + def handle_too_many_otp_sends + analytics.track_event(Analytics::MULTI_FACTOR_AUTH_MAX_SENDS) + decorator = current_user.decorate + sign_out + render( + 'two_factor_authentication/shared/max_otp_requests_reached', + locals: { decorator: decorator } + ) + end + def require_current_password redirect_to user_password_confirm_path end @@ -48,11 +58,14 @@ def check_already_authenticated end def reset_attempt_count_if_user_no_longer_locked_out - return unless decorated_user.no_longer_blocked_from_entering_2fa_code? + return unless decorated_user.no_longer_locked_out? UpdateUser.new( user: current_user, - attributes: { second_factor_attempts_count: 0, second_factor_locked_at: nil } + attributes: { + second_factor_attempts_count: 0, + second_factor_locked_at: nil, + } ).call end @@ -79,7 +92,7 @@ def handle_invalid_otp(type: 'otp') flash.now[:error] = t("devise.two_factor_authentication.invalid_#{type}") - if decorated_user.blocked_from_entering_2fa_code? + if decorated_user.locked_out? handle_second_factor_locked_user(type) else render_show_after_invalid @@ -190,7 +203,7 @@ def after_otp_action_path elsif @updating_existing_number account_path elsif decorated_user.password_reset_profile.present? - manage_reactivate_account_path + reactivate_account_path else account_path end diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 29627a123e3..662f84c4525 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -4,27 +4,19 @@ class AuthorizationController < ApplicationController include VerifyProfileConcern before_action :build_authorize_form_from_params, only: [:index] + before_action :validate_authorize_form, only: [:index] before_action :store_request, only: [:index] before_action :add_sp_metadata_to_session, only: [:index] before_action :apply_secure_headers_override, only: [:index] - # rubocop:disable Metrics/AbcSize def index return confirm_two_factor_authenticated(request_id) unless user_fully_authenticated? return redirect_to_account_or_verify_profile_url if profile_or_identity_needs_verification? - result = @authorize_form.submit(current_user, session.id) - - track_authorize_analytics(result) - - if (redirect_uri = result.extra[:redirect_uri]) - redirect_to redirect_uri - delete_branded_experience - else - render :error - end + @authorize_form.link_identity_to_service_provider(current_user, session.id) + redirect_to @authorize_form.success_redirect_uri + delete_branded_experience end - # rubocop:enable Metrics/AbcSize private @@ -70,11 +62,21 @@ def authorization_params params.permit(OpenidConnectAuthorizeForm::ATTRS) end - def store_request - return if sp_session[:request_id] + def validate_authorize_form + result = @authorize_form.submit + track_authorize_analytics(result) + return if result.success? + + if (redirect_uri = result.extra[:redirect_uri]) + redirect_to redirect_uri + else + render :error + end + end + + def store_request client_id = @authorize_form.client_id - return if client_id.blank? @request_id = SecureRandom.uuid ServiceProviderRequest.find_or_create_by(uuid: @request_id) do |sp_request| @@ -86,15 +88,7 @@ def store_request end def add_sp_metadata_to_session - return if sp_session[:request_id] - - session[:sp] = { - issuer: @authorize_form.client_id, - loa3: @authorize_form.loa3_requested?, - request_id: @request_id, - request_url: request.original_url, - requested_attributes: requested_attributes, - } + StoreSpMetadataInSession.new(session: session, request_id: @request_id).call end def requested_attributes diff --git a/app/controllers/reactivate_account_controller.rb b/app/controllers/reactivate_account_controller.rb index c8e43c8b85f..a9eff6cc5e4 100644 --- a/app/controllers/reactivate_account_controller.rb +++ b/app/controllers/reactivate_account_controller.rb @@ -1,20 +1,13 @@ class ReactivateAccountController < ApplicationController + include AccountRecoveryConcern + before_action :confirm_two_factor_authenticated before_action :confirm_password_reset_profile - def index - user_session[:acknowledge_personal_key] ||= true - end + def index; end def update - user_session.delete(:acknowledge_personal_key) + reactivate_account_session.suspend redirect_to verify_url end - - protected - - def confirm_password_reset_profile - return if current_user.decorate.password_reset_profile - redirect_to root_url - end end diff --git a/app/controllers/sign_out_controller.rb b/app/controllers/sign_out_controller.rb new file mode 100644 index 00000000000..84c24226cfe --- /dev/null +++ b/app/controllers/sign_out_controller.rb @@ -0,0 +1,13 @@ +class SignOutController < ApplicationController + include FullyAuthenticatable + + skip_before_action :handle_two_factor_authentication + + def destroy + path_after_cancellation = decorated_session.cancel_link_path + sign_out + flash[:success] = t('devise.sessions.signed_out') + redirect_to path_after_cancellation + delete_branded_experience + end +end diff --git a/app/controllers/users/reactivate_account_controller.rb b/app/controllers/users/reactivate_account_controller.rb deleted file mode 100644 index b7b4d9d492e..00000000000 --- a/app/controllers/users/reactivate_account_controller.rb +++ /dev/null @@ -1,25 +0,0 @@ -module Users - class ReactivateAccountController < ApplicationController - def index - @reactivate_account_form = ReactivateAccountForm.new(current_user) - end - - def create - @reactivate_account_form = build_reactivate_account_form - if @reactivate_account_form.submit(flash) - redirect_to account_path - else - render :index - end - end - - protected - - def build_reactivate_account_form - ReactivateAccountForm.new( - current_user, - params[:reactivate_account_form].permit(:password, :personal_key) - ) - end - end -end diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 543ce17ae9e..ae84123fdb3 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -102,7 +102,7 @@ def user_signed_in_and_not_locked_out?(user) end def user_locked_out?(user) - UserDecorator.new(user).blocked_from_entering_2fa_code? + UserDecorator.new(user).locked_out? end def store_sp_metadata_in_session diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 76e13eef3f4..90c9e42a9ea 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -35,12 +35,21 @@ def reauthn_param end def handle_valid_otp_delivery_preference(method) + otp_rate_limiter.reset_count_and_otp_last_sent_at if decorated_user.no_longer_locked_out? + + if otp_rate_limiter.exceeded_otp_send_limit? + otp_rate_limiter.lock_out_user + + return handle_too_many_otp_sends + end + send_user_otp(method) session[:code_sent] = 'true' redirect_to login_two_factor_path(otp_delivery_preference: method, reauthn: reauthn?) end def send_user_otp(method) + otp_rate_limiter.increment current_user.create_direct_otp job = "#{method.capitalize}OtpSenderJob".constantize @@ -70,5 +79,9 @@ def phone_to_deliver_to user_session[:unconfirmed_phone] end + + def otp_rate_limiter + @_otp_rate_limited ||= OtpRateLimiter.new(phone: phone_to_deliver_to, user: current_user) + end end end diff --git a/app/controllers/users/verify_password_controller.rb b/app/controllers/users/verify_password_controller.rb new file mode 100644 index 00000000000..67bea6d230f --- /dev/null +++ b/app/controllers/users/verify_password_controller.rb @@ -0,0 +1,53 @@ +module Users + class VerifyPasswordController < ApplicationController + include AccountRecoveryConcern + + before_action :confirm_two_factor_authenticated + before_action :confirm_password_reset_profile + before_action :confirm_personal_key + + def new + @verify_password_form = VerifyPasswordForm.new( + user: current_user, + password: '', + decrypted_pii: decrypted_pii + ) + end + + def update + result = verify_password_form.submit + + if result.success? + handle_success(result) + else + render :new + end + end + + private + + def confirm_personal_key + return if reactivate_account_session.personal_key? + redirect_to root_url + end + + def decrypted_pii + pii = reactivate_account_session.decrypted_pii + @_decrypted_pii ||= Pii::Attributes.new_from_json(pii) + end + + def handle_success(result) + flash[:personal_key] = result.extra[:personal_key] + reactivate_account_session.clear + redirect_to account_url + end + + def verify_password_form + VerifyPasswordForm.new( + user: current_user, + password: params.require(:user).permit(:password)[:password], + decrypted_pii: decrypted_pii + ) + end + end +end diff --git a/app/controllers/users/verify_personal_key_controller.rb b/app/controllers/users/verify_personal_key_controller.rb new file mode 100644 index 00000000000..6316fc9441b --- /dev/null +++ b/app/controllers/users/verify_personal_key_controller.rb @@ -0,0 +1,52 @@ +module Users + class VerifyPersonalKeyController < ApplicationController + include AccountRecoveryConcern + + before_action :confirm_two_factor_authenticated + before_action :confirm_password_reset_profile + before_action :init_account_recovery, only: [:new] + + def new + @personal_key_form = VerifyPersonalKeyForm.new( + user: current_user, + personal_key: '' + ) + end + + def create + result = personal_key_form.submit + + if result.success? + handle_success(result) + else + handle_failure(result) + end + end + + private + + def init_account_recovery + return if reactivate_account_session.started? + + flash.now[:notice] = t('notices.account_recovery') + reactivate_account_session.start + end + + def handle_success(result) + reactivate_account_session.store_decrypted_pii(result.extra[:decrypted_pii]) + redirect_to verify_password_url + end + + def handle_failure(result) + flash.now[:error] = result.errors[:personal_key].last + render :new + end + + def personal_key_form + VerifyPersonalKeyForm.new( + user: current_user, + personal_key: params.permit(:personal_key)[:personal_key] + ) + end + end +end diff --git a/app/controllers/verify_controller.rb b/app/controllers/verify_controller.rb index aa9452e8c13..1f62949184a 100644 --- a/app/controllers/verify_controller.rb +++ b/app/controllers/verify_controller.rb @@ -1,5 +1,6 @@ class VerifyController < ApplicationController include IdvSession + include AccountRecoveryConcern before_action :confirm_two_factor_authenticated before_action :confirm_idv_needed, only: %i[cancel fail] @@ -28,12 +29,9 @@ def fail private def profile_needs_reactivation? - return unless password_reset_profile && user_session[:acknowledge_personal_key] == true - redirect_to manage_reactivate_account_url - end - - def password_reset_profile - current_user.decorate.password_reset_profile + return unless reactivate_account_session.started? + confirm_password_reset_profile + redirect_to reactivate_account_url end def active_profile? diff --git a/app/decorators/service_provider_session_decorator.rb b/app/decorators/service_provider_session_decorator.rb index d2848e92495..d24b946301e 100644 --- a/app/decorators/service_provider_session_decorator.rb +++ b/app/decorators/service_provider_session_decorator.rb @@ -3,10 +3,11 @@ class ServiceProviderSessionDecorator DEFAULT_LOGO = 'generic.svg'.freeze - def initialize(sp:, view_context:, sp_session:) + def initialize(sp:, view_context:, sp_session:, service_provider_request:) @sp = sp @view_context = view_context @sp_session = sp_session + @service_provider_request = service_provider_request end def sp_logo @@ -71,10 +72,10 @@ def cancel_link_path private - attr_reader :sp, :view_context, :sp_session + attr_reader :sp, :view_context, :sp_session, :service_provider_request def request_url - sp_session[:request_url] + sp_session[:request_url] || service_provider_request.url end def openid_connect_redirector diff --git a/app/decorators/user_decorator.rb b/app/decorators/user_decorator.rb index 4f3a7bad068..636840c2e89 100644 --- a/app/decorators/user_decorator.rb +++ b/app/decorators/user_decorator.rb @@ -2,6 +2,7 @@ class UserDecorator MAX_RECENT_EVENTS = 5 + DEFAULT_LOCKOUT_PERIOD = 10.minutes def initialize(user) @user = user @@ -19,7 +20,7 @@ def lockout_time_remaining_in_words end def lockout_time_remaining - (Devise.direct_otp_valid_for - (Time.zone.now - user.second_factor_locked_at)).to_i + (lockout_period - (Time.zone.now - user.second_factor_locked_at)).to_i end def confirmation_period_expired_error @@ -98,12 +99,12 @@ def qrcode(otp_secret_key) qrcode.as_png(size: 280).to_data_url end - def blocked_from_entering_2fa_code? - user.second_factor_locked_at.present? && !blocked_from_2fa_period_expired? + def locked_out? + user.second_factor_locked_at.present? && !lockout_period_expired? end - def no_longer_blocked_from_entering_2fa_code? - user.second_factor_locked_at.present? && blocked_from_2fa_period_expired? + def no_longer_locked_out? + user.second_factor_locked_at.present? && lockout_period_expired? end def should_acknowledge_personal_key?(session) @@ -136,7 +137,16 @@ def masked_number(number) "***-***-#{number[-4..-1]}" end - def blocked_from_2fa_period_expired? - (Time.zone.now - user.second_factor_locked_at) > Devise.direct_otp_valid_for + def lockout_period + return DEFAULT_LOCKOUT_PERIOD if lockout_period_config.blank? + lockout_period_config.to_i.minutes + end + + def lockout_period_config + @config ||= Figaro.env.lockout_period_in_minutes + end + + def lockout_period_expired? + (Time.zone.now - user.second_factor_locked_at) > lockout_period end end diff --git a/app/forms/openid_connect_authorize_form.rb b/app/forms/openid_connect_authorize_form.rb index c5723c0eb84..eb880f33f2e 100644 --- a/app/forms/openid_connect_authorize_form.rb +++ b/app/forms/openid_connect_authorize_form.rb @@ -48,11 +48,9 @@ def initialize(params) ) end - def submit(user, rails_session_id) + def submit @success = valid? - link_identity_to_client_id(user, rails_session_id) if success - FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes) end @@ -68,6 +66,22 @@ def service_provider @_service_provider ||= ServiceProvider.from_issuer(client_id) end + def link_identity_to_service_provider(current_user, rails_session_id) + identity_linker = IdentityLinker.new(current_user, client_id) + @identity = identity_linker.link_identity( + nonce: nonce, + rails_session_id: rails_session_id, + ial: ial, + scope: scope.join(' '), + code_challenge: code_challenge + ) + end + + def success_redirect_uri + code = identity&.session_uuid + openid_connect_redirector.success_redirect_uri(code: code) if code + end + private attr_reader :identity, :success, :openid_connect_redirector, :already_linked @@ -96,17 +110,6 @@ def validate_scope errors.add(:scope, t('openid_connect.authorization.errors.no_valid_scope')) end - def link_identity_to_client_id(current_user, rails_session_id) - identity_linker = IdentityLinker.new(current_user, client_id) - @identity = identity_linker.link_identity( - nonce: nonce, - rails_session_id: rails_session_id, - ial: ial, - scope: scope.join(' '), - code_challenge: code_challenge - ) - end - def ial case acr_values.sort.max when Saml::Idp::Constants::LOA1_AUTHN_CONTEXT_CLASSREF @@ -127,10 +130,6 @@ def result_uri success ? success_redirect_uri : error_redirect_uri end - def success_redirect_uri - openid_connect_redirector.success_redirect_uri(code: identity.session_uuid) - end - def error_redirect_uri openid_connect_redirector.error_redirect_uri end diff --git a/app/forms/reactivate_account_form.rb b/app/forms/reactivate_account_form.rb deleted file mode 100644 index ccde4703e62..00000000000 --- a/app/forms/reactivate_account_form.rb +++ /dev/null @@ -1,80 +0,0 @@ -class ReactivateAccountForm - include ActiveModel::Model - include PersonalKeyValidator - - validates :personal_key, :password, presence: true - validate :validate_password_reset_profile - validate :validate_password - validate :validate_personal_key - - attr_accessor :personal_key, :password - attr_reader :user - - def initialize(user, attrs = {}) - attrs[:personal_key] ||= nil - @user = user - super attrs - - @personal_key = normalize_personal_key(personal_key) - end - - def submit(flash) - if valid? - flash[:personal_key] = reencrypt_pii - true - else - reset_sensitive_fields - false - end - end - - protected - - def password_reset_profile - @_password_reset_profile ||= user.decorate.password_reset_profile - end - - def user_access_key - @_uak ||= user.unlock_user_access_key(password) - end - - def decrypted_pii - @_pii ||= password_reset_profile.recover_pii(personal_key) - end - - def reencrypt_pii - personal_key = password_reset_profile.encrypt_pii(user_access_key, decrypted_pii) - password_reset_profile.deactivation_reason = nil - password_reset_profile.active = true - password_reset_profile.save! - personal_key - end - - def validate_password_reset_profile - errors.add :base, :no_password_reset_profile unless password_reset_profile - end - - def validate_password - return if valid_password? - errors.add :password, :password_incorrect - end - - def validate_personal_key - return check_personal_key if personal_key_decrypts? - errors.add :personal_key, :personal_key_incorrect - end - - def reset_sensitive_fields - self.password = nil - end - - def valid_password? - user.valid_password?(password) - end - - def personal_key_decrypts? - decrypted_pii.present? - rescue Pii::EncryptionError => _err - false - end -end diff --git a/app/forms/verify_account_form.rb b/app/forms/verify_account_form.rb index a5c00d86241..8de596fde01 100644 --- a/app/forms/verify_account_form.rb +++ b/app/forms/verify_account_form.rb @@ -35,12 +35,14 @@ def validate_pending_profile end def validate_otp - return if valid_otp? + return if otp.blank? || valid_otp? errors.add :otp, :confirmation_code_incorrect end def valid_otp? - ActiveSupport::SecurityUtils.secure_compare(otp, pii_attributes.otp.to_s) + otp.present? && ActiveSupport::SecurityUtils.secure_compare( + Base32::Crockford.normalize(otp), Base32::Crockford.normalize(pii_attributes.otp.to_s) + ) end def reset_sensitive_fields diff --git a/app/forms/verify_password_form.rb b/app/forms/verify_password_form.rb new file mode 100644 index 00000000000..2a0f3858637 --- /dev/null +++ b/app/forms/verify_password_form.rb @@ -0,0 +1,49 @@ +class VerifyPasswordForm + include ActiveModel::Model + + validates :password, presence: true + validate :validate_password + + attr_reader :user, :password, :decrypted_pii + + def initialize(user:, password:, decrypted_pii:) + @user = user + @password = password + @decrypted_pii = decrypted_pii + end + + def submit + success = valid? + extra = {} + + extra[:personal_key] = reencrypt_pii if success + + FormResponse.new(success: success, errors: errors, extra: extra) + end + + private + + def validate_password + return if valid_password? + errors.add :password, :password_incorrect + end + + def valid_password? + user.valid_password?(password) + end + + def reencrypt_pii + personal_key = profile.encrypt_pii(user_access_key, decrypted_pii) + profile.update(deactivation_reason: nil, active: true) + profile.save! + personal_key + end + + def profile + @_profile ||= user.decorate.password_reset_profile + end + + def user_access_key + @_uak ||= user.unlock_user_access_key(password) + end +end diff --git a/app/forms/verify_personal_key_form.rb b/app/forms/verify_personal_key_form.rb new file mode 100644 index 00000000000..c718ec1a732 --- /dev/null +++ b/app/forms/verify_personal_key_form.rb @@ -0,0 +1,57 @@ +class VerifyPersonalKeyForm + include ActiveModel::Model + include PersonalKeyValidator + + validates :personal_key, presence: true + validate :validate_personal_key + + attr_accessor :personal_key + attr_reader :user + + def initialize(user:, personal_key:) + @user = user + @personal_key = normalize_personal_key(personal_key) + end + + def submit + extra = {} + success = valid? + + if success + extra[:decrypted_pii] = decrypted_pii_json + else + reset_sensitive_fields + end + + FormResponse.new(success: valid?, errors: errors, extra: extra) + end + + private + + def decrypted_pii_json + decrypted_pii.to_json + end + + def password_reset_profile + user.decorate.password_reset_profile + end + + def decrypted_pii + @_pii ||= password_reset_profile.recover_pii(personal_key) + end + + def validate_personal_key + return check_personal_key if personal_key_decrypts? + errors.add :personal_key, :personal_key_incorrect + end + + def reset_sensitive_fields + self.personal_key = nil + end + + def personal_key_decrypts? + decrypted_pii.present? + rescue Pii::EncryptionError => _err + false + end +end diff --git a/app/inputs/array_input.rb b/app/inputs/array_input.rb deleted file mode 100644 index e08b96709de..00000000000 --- a/app/inputs/array_input.rb +++ /dev/null @@ -1,34 +0,0 @@ -class ArrayInput < SimpleForm::Inputs::StringInput - # simple_form throws a deprecation warning noting that the - # method signature should be changed to add this parameter - def input(_wrapper_options) - input_html_options[:type] ||= input_type - - existing_values = Array(object.public_send(attribute_name)).map do - build - end - - existing_values.push(build) if existing_values.empty? - - safe_join(existing_values) - end - - def input_type - :text - end - - def build - options = { - value: nil, - class: 'block col-12 field', - autocomplete: 'off', - name: "#{object_name}[#{attribute_name}][]", - } - - builder.text_field(nil, input_html_options.merge(options)) - end - - def builder - @builder ||= :builder - end -end diff --git a/app/models/null_identity.rb b/app/models/null_identity.rb index 340c8e78b56..4cf193824be 100644 --- a/app/models/null_identity.rb +++ b/app/models/null_identity.rb @@ -12,4 +12,8 @@ def deactivate def sp_metadata {} end + + def session_uuid + nil + end end diff --git a/app/models/otp_requests_tracker.rb b/app/models/otp_requests_tracker.rb new file mode 100644 index 00000000000..8ce5017e8d3 --- /dev/null +++ b/app/models/otp_requests_tracker.rb @@ -0,0 +1,13 @@ +class OtpRequestsTracker < ActiveRecord::Base + def self.find_or_create_with_phone(phone) + tries ||= 1 + phone ||= phone.strip + phone_fingerprint ||= Pii::Fingerprinter.fingerprint(phone) + + where(phone_fingerprint: phone_fingerprint). + first_or_create(otp_send_count: 0, otp_last_sent_at: Time.zone.now) + rescue ActiveRecord::RecordNotUnique + retry unless (tries -= 1).zero? + raise + end +end diff --git a/app/presenters/two_factor_auth_code/authenticator_delivery_presenter.rb b/app/presenters/two_factor_auth_code/authenticator_delivery_presenter.rb index 0f27b0959bf..fbf6a2736e3 100644 --- a/app/presenters/two_factor_auth_code/authenticator_delivery_presenter.rb +++ b/app/presenters/two_factor_auth_code/authenticator_delivery_presenter.rb @@ -22,7 +22,7 @@ def cancel_link if reauthn account_path else - destroy_user_session_path + sign_out_path end end diff --git a/app/presenters/two_factor_auth_code/phone_delivery_presenter.rb b/app/presenters/two_factor_auth_code/phone_delivery_presenter.rb index 85e805bdc34..411e52f9019 100644 --- a/app/presenters/two_factor_auth_code/phone_delivery_presenter.rb +++ b/app/presenters/two_factor_auth_code/phone_delivery_presenter.rb @@ -22,7 +22,7 @@ def cancel_link if confirmation_for_phone_change || reauthn account_path else - destroy_user_session_path + sign_out_path end end diff --git a/app/services/analytics.rb b/app/services/analytics.rb index 577e68c2b86..96811367afd 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -55,6 +55,7 @@ def uuid MULTI_FACTOR_AUTH_ENTER_OTP_VISIT = 'Multi-Factor Authentication: enter OTP visited'.freeze MULTI_FACTOR_AUTH_MAX_ATTEMPTS = 'Multi-Factor Authentication: max attempts reached'.freeze MULTI_FACTOR_AUTH_PHONE_SETUP = 'Multi-Factor Authentication: phone setup'.freeze + MULTI_FACTOR_AUTH_MAX_SENDS = 'Multi-Factor Authentication: max otp sends reached'.freeze OPENID_CONNECT_BEARER_TOKEN = 'OpenID Connect: bearer token authentication'.freeze OPENID_CONNECT_LOGOUT = 'OpenID Connect: logout'.freeze OPENID_CONNECT_REQUEST_AUTHORIZATION = 'OpenID Connect: authorization request'.freeze diff --git a/app/services/decorated_session.rb b/app/services/decorated_session.rb index ebe7829e338..5a51d68fee7 100644 --- a/app/services/decorated_session.rb +++ b/app/services/decorated_session.rb @@ -1,14 +1,18 @@ class DecoratedSession - def initialize(sp:, view_context:, sp_session:) + def initialize(sp:, view_context:, sp_session:, service_provider_request:) @sp = sp @view_context = view_context @sp_session = sp_session + @service_provider_request = service_provider_request end def call if sp.is_a? ServiceProvider ServiceProviderSessionDecorator.new( - sp: sp, view_context: view_context, sp_session: sp_session + sp: sp, + view_context: view_context, + sp_session: sp_session, + service_provider_request: service_provider_request ) else SessionDecorator.new @@ -17,5 +21,5 @@ def call private - attr_reader :sp, :view_context, :sp_session + attr_reader :sp, :view_context, :sp_session, :service_provider_request end diff --git a/app/services/idv/financials_step.rb b/app/services/idv/financials_step.rb index 63adc35a732..cffa4264dd0 100644 --- a/app/services/idv/financials_step.rb +++ b/app/services/idv/financials_step.rb @@ -10,7 +10,7 @@ def submit idv_session.financials_confirmation = false end - FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes) + FormResponse.new(success: success, errors: errors) end def form_valid_but_vendor_validation_failed? @@ -37,11 +37,5 @@ def vendor_params finance_type = idv_form.finance_type { finance_type => idv_form.idv_params[finance_type] } end - - def extra_analytics_attributes - { - vendor: { reasons: vendor_reasons }, - } - end end end diff --git a/app/services/idv/phone_step.rb b/app/services/idv/phone_step.rb index 6824b5fe1f9..252b6f86f5d 100644 --- a/app/services/idv/phone_step.rb +++ b/app/services/idv/phone_step.rb @@ -7,7 +7,7 @@ def submit idv_session.phone_confirmation = false end - FormResponse.new(success: complete?, errors: errors, extra: extra_analytics_attributes) + FormResponse.new(success: complete?, errors: errors) end def form_valid_but_vendor_validation_failed? @@ -37,9 +37,5 @@ def update_idv_session idv_session.address_verification_mechanism = :phone idv_session.params = idv_form.idv_params end - - def extra_analytics_attributes - { vendor: { reasons: vendor_reasons } } - end end end diff --git a/app/services/idv/profile_maker.rb b/app/services/idv/profile_maker.rb index 71cd2f9171b..28e2f8aaa5a 100644 --- a/app/services/idv/profile_maker.rb +++ b/app/services/idv/profile_maker.rb @@ -28,9 +28,14 @@ def pii_from_applicant(appl, norm_appl) dob: Pii::Attribute.new(raw: appl.dob, norm: norm_appl.dob), ssn: Pii::Attribute.new(raw: appl.ssn, norm: norm_appl.ssn), phone: Pii::Attribute.new(raw: appl.phone, norm: norm_appl.phone), - otp: SecureRandom.hex(5) + otp: generate_otp ) end # rubocop:enable MethodLength, AbcSize + + def generate_otp(length: 10) + # Crockford encoding is 5 bits per character + Base32::Crockford.encode(SecureRandom.random_number(2**(5 * length)), length: length) + end end end diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index 0317f404eae..188ef878467 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -56,7 +56,7 @@ def cache_encrypted_pii(password) end def vendor_params - applicant_params_ascii.merge(uuid: current_user.uuid) + applicant_params_ascii.merge('uuid' => current_user.uuid) end def profile diff --git a/app/services/marketing_site.rb b/app/services/marketing_site.rb index 992ae4da46d..ccbfa81b344 100644 --- a/app/services/marketing_site.rb +++ b/app/services/marketing_site.rb @@ -18,6 +18,6 @@ def self.help_url end def self.help_authenticator_app_url - URI.join(BASE_URL, help_url, '#what-is-an-authenticator-app').to_s + URI.join(BASE_URL, '/help/signing-in/what-is-an-authenticator-app/').to_s end end diff --git a/app/services/otp_rate_limiter.rb b/app/services/otp_rate_limiter.rb new file mode 100644 index 00000000000..1fb7bc29dcf --- /dev/null +++ b/app/services/otp_rate_limiter.rb @@ -0,0 +1,61 @@ +class OtpRateLimiter + def initialize(phone:, user:) + @phone = phone + @user = user + end + + def exceeded_otp_send_limit? + return false if entry_for_current_phone.blank? + + if rate_limit_period_expired? + reset_count_and_otp_last_sent_at + return false + end + + max_requests_reached? + end + + def max_requests_reached? + entry_for_current_phone.otp_send_count >= otp_maxretry_times + end + + def rate_limit_period_expired? + otp_last_sent_at.present? && (otp_last_sent_at + otp_findtime) < Time.zone.now + end + + def reset_count_and_otp_last_sent_at + entry_for_current_phone.update(otp_last_sent_at: Time.zone.now, otp_send_count: 0) + end + + def lock_out_user + UpdateUser.new(user: user, attributes: { second_factor_locked_at: Time.zone.now }).call + end + + def increment + now = Time.zone.now + + entry_for_current_phone.otp_send_count += 1 + entry_for_current_phone.otp_last_sent_at = now + entry_for_current_phone.save! + end + + private + + attr_reader :phone, :user + + def entry_for_current_phone + @entry ||= OtpRequestsTracker.find_or_create_with_phone(phone) + end + + def otp_last_sent_at + entry_for_current_phone.otp_last_sent_at + end + + def otp_findtime + Figaro.env.otp_delivery_blocklist_findtime.to_i.minutes + end + + def otp_maxretry_times + Figaro.env.otp_delivery_blocklist_maxretry.to_i + end +end diff --git a/app/services/personal_key_formatter.rb b/app/services/personal_key_formatter.rb index 6fec9ecb7f7..776a0b9f0c6 100644 --- a/app/services/personal_key_formatter.rb +++ b/app/services/personal_key_formatter.rb @@ -12,4 +12,8 @@ def self.regexp def self.regexp_string REGEXP end + + def self.code_length + CHAR_COUNT * WORD_COUNT + (WORD_COUNT - 1) + end end diff --git a/app/services/reactivate_account_session.rb b/app/services/reactivate_account_session.rb new file mode 100644 index 00000000000..62ed34b8085 --- /dev/null +++ b/app/services/reactivate_account_session.rb @@ -0,0 +1,55 @@ +class ReactivateAccountSession + SESSION_KEY = :reactivate_account + + def initialize(user:, user_session:) + @user = user + @session = user_session + + session[SESSION_KEY] ||= generate_session + end + + def clear + session.delete(SESSION_KEY) + end + + def start + reactivate_account_session[:active] = true + end + + def started? + reactivate_account_session[:active] + end + + def suspend + session[SESSION_KEY] = generate_session + end + + def store_decrypted_pii(pii) + reactivate_account_session[:personal_key] = true + reactivate_account_session[:pii] = pii + end + + def personal_key? + reactivate_account_session[:personal_key] + end + + def decrypted_pii + reactivate_account_session[:pii] + end + + private + + attr_reader :session + + def generate_session + { + active: false, + personal_key: false, + pii: nil, + } + end + + def reactivate_account_session + session[SESSION_KEY] + end +end diff --git a/app/services/session_encryptor.rb b/app/services/session_encryptor.rb index a51ada6ecef..ba062a76f88 100644 --- a/app/services/session_encryptor.rb +++ b/app/services/session_encryptor.rb @@ -1,6 +1,4 @@ class SessionEncryptor - MARSHAL_SIGNATURE ||= 'BAh'.freeze - def self.build_user_access_key key = Figaro.env.session_encryption_key UserAccessKey.new(password: key, salt: key) @@ -13,16 +11,7 @@ def self.build_user_access_key def self.load(value) decrypted = encryptor.decrypt(value, user_access_key) - if decrypted.start_with?(MARSHAL_SIGNATURE) - Rails.logger.info 'Marshalled session found' - # rubocop:disable Security/MarshalLoad - Marshal.load(::Base64.decode64(decrypted)).tap do |decoded_value| - dump(decoded_value) - end - # rubocop:enable Security/MarshalLoad - else - JSON.parse(decrypted, quirks_mode: true).with_indifferent_access - end + JSON.parse(decrypted, quirks_mode: true).with_indifferent_access end def self.dump(value) diff --git a/app/services/single_logout_handler.rb b/app/services/single_logout_handler.rb index 4cb2cf69952..7f9e81eee45 100644 --- a/app/services/single_logout_handler.rb +++ b/app/services/single_logout_handler.rb @@ -32,7 +32,7 @@ def failed_saml_response? end def slo_not_implemented_at_sp? - identity.sp_metadata[:assertion_consumer_logout_service_url].nil? + identity.sp_metadata[:assertion_consumer_logout_service_url].blank? end def identity diff --git a/app/services/usps_exporter.rb b/app/services/usps_exporter.rb index cf15737cc88..81cd43c2fb3 100644 --- a/app/services/usps_exporter.rb +++ b/app/services/usps_exporter.rb @@ -63,7 +63,7 @@ def make_entry_row(entry) def file_encryptor @_file_encryptor ||= FileEncryptor.new( - Rails.root.join('keys/equifax_gpg.pub.bin'), + Rails.root.join('keys', 'equifax_gpg.pub.bin'), Figaro.env.equifax_gpg_email ) end diff --git a/app/view_models/sign_up_completions_show.rb b/app/view_models/sign_up_completions_show.rb index 5e70030eb79..63addbf8516 100644 --- a/app/view_models/sign_up_completions_show.rb +++ b/app/view_models/sign_up_completions_show.rb @@ -18,6 +18,7 @@ def initialize(loa3_requested:, decorated_session:) [[:social_security_number], :social_security_number], ].freeze + # rubocop:disable Rails/OutputSafety def heading safe_join([I18n.t( 'titles.sign_up.completion_html', @@ -25,6 +26,7 @@ def heading app: APP_NAME ).html_safe]) end + # rubocop:enable Rails/OutputSafety def title I18n.t( diff --git a/app/view_models/verify/base.rb b/app/view_models/verify/base.rb index d99cf5f931f..84e4bd669ef 100644 --- a/app/view_models/verify/base.rb +++ b/app/view_models/verify/base.rb @@ -82,10 +82,12 @@ def attempts end end + # rubocop:disable Rails/OutputSafety def html_paragraph(text:, css_class: '') html = helper.safe_join([text.html_safe]) helper.content_tag(:p, html, class: css_class) end + # rubocop:enable Rails/OutputSafety def helper ActionController::Base.helpers diff --git a/app/views/accounts/_password_reset.html.slim b/app/views/accounts/_password_reset.html.slim index ab541b0f1e8..4b54b67f917 100644 --- a/app/views/accounts/_password_reset.html.slim +++ b/app/views/accounts/_password_reset.html.slim @@ -1,3 +1,3 @@ .mb4.alert.alert-warning p = t('account.index.reactivation.instructions') - p.mb0 = link_to t('account.index.reactivation.link'), manage_reactivate_account_path + p.mb0 = link_to t('account.index.reactivation.link'), reactivate_account_path diff --git a/app/views/reactivate_account/_modal.html.slim b/app/views/reactivate_account/_modal.html.slim new file mode 100644 index 00000000000..a1981763847 --- /dev/null +++ b/app/views/reactivate_account/_modal.html.slim @@ -0,0 +1,18 @@ +#reactivate-account-modal.display-none + = render layout: '/shared/modal_layout' do + .p4.cntnr-xxskinny.border-box.bg-white.rounded-xxl.modal-warning + h2.my2.fs-20p.sans-serif.regular.center + = t('instructions.account.reactivate.modal.heading') + hr.mb3.bw4.rounded + .mb5 + = t('instructions.account.reactivate.modal.copy') + .center + = form_tag reactivate_account_path, method: :put do + = button_tag t('forms.buttons.continue'), type: 'submit', + class: 'btn btn-primary col-12 mb2 p2 rounded-lg', name: :personal_key + button.btn.col-12.p2.rounded-lg.border.border-blue.blue( + type='button', id='no-key-reactivate-dismiss' + ) + = t('links.cancel') + +== javascript_include_tag 'misc/reactivate-account-modal' diff --git a/app/views/reactivate_account/index.html.slim b/app/views/reactivate_account/index.html.slim index 845f0f0b6aa..2eeb552122a 100644 --- a/app/views/reactivate_account/index.html.slim +++ b/app/views/reactivate_account/index.html.slim @@ -1,5 +1,10 @@ - title t('titles.reactivate_account') +.js-fallback + .alert.alert-warning + .bold + = t('instructions.account.reactivate.modal.copy') + h1.h3.mt0.mb2 = t('headings.account.reactivate') p.mb4 = t('instructions.account.reactivate.intro') @@ -15,8 +20,10 @@ p.mb0 = t('instructions.account.reactivate.explanation') .block-center.center.col-10 .col-12.mb2 - = link_to t('links.account.reactivate.with_key'), reactivate_account_path, + = link_to t('links.account.reactivate.with_key'), verify_personal_key_path, class: 'btn btn-primary block' - = form_tag manage_reactivate_account_path, method: :put, class: 'col-12' do + = form_tag reactivate_account_path, method: :put, class: 'col-12' do = button_tag t('links.account.reactivate.without_key'), type: 'submit', - class: 'btn btn-secondary block col-12' + class: 'btn btn-secondary block col-12', id: 'no-key-reactivate' + += render 'reactivate_account/modal' diff --git a/app/views/session_timeout/_warning.html.slim b/app/views/session_timeout/_warning.html.slim index 92b41b98b01..2d5ddb45539 100644 --- a/app/views/session_timeout/_warning.html.slim +++ b/app/views/session_timeout/_warning.html.slim @@ -1,10 +1,14 @@ #session-timeout-msg.display-none = render layout: '/shared/modal_layout' do - .p4.cntnr-skinny.border-box.bg-white.rounded.relative - = image_tag(asset_url('clock.svg'), class: 'modal-ico') - h3.mt0.mb2 = t('headings.session_timeout_warning') - p.mb3 = modal.message - = link_to modal.continue, - request.original_url, class: 'btn btn-primary' - = link_to modal.sign_out, - destroy_user_session_path, class: 'ml2 btn btn-outline' + .p3.sm-p4.cntnr-xxskinny.border-box.bg-white.rounded-xxl.modal-timeout + h2.my2.fs-20p.sans-serif.regular.center = t('headings.session_timeout_warning') + hr.mb3.bw4.rounded + .mb4 + p = modal.message + .center + = link_to modal.continue, + request.original_url, + class: 'btn btn-primary col-12 mb2 p2 rounded-lg border-box' + = link_to modal.sign_out, + destroy_user_session_path, + class: 'btn col-12 p2 rounded-lg border border-blue blue border-box' diff --git a/app/views/shared/_cancel_action_modal.html.slim b/app/views/shared/_cancel_action_modal.html.slim index 49a650dc3bd..f54e8eda37f 100644 --- a/app/views/shared/_cancel_action_modal.html.slim +++ b/app/views/shared/_cancel_action_modal.html.slim @@ -2,12 +2,10 @@ #cancel-action-modal.display-none = render layout: 'shared/modal_layout' do - .p5.cntnr-xskinny.border-box.bg-white.rounded.relative.rounded-lg.modal-warning - h3.my2.fs-20p.sans-serif.center - = t("#{app_flow}.cancel.modal_header") + .p3.sm-p4.cntnr-xxskinny.border-box.bg-white.rounded-xxl.lg.modal-warning + h3.my2.fs-20p.sans-serif.center = t("#{app_flow}.cancel.modal_header") hr.mb3.bw4.rounded - - .mb5 + .mb4 p.mb1.bold = t("#{app_flow}.cancel.warning_header") ul.px2.yellow-dots @@ -15,20 +13,20 @@ li.mb2 = point - .clearfix - button.btn.btn-primary.rounded-lg.mb2.col-12(id='loa-continue' type='button') + .center + button.btn.btn-primary.col-12.mb2.p2.rounded-lg.border-box(id='loa-continue' type='button') = t("#{app_flow}.buttons.continue") - if idv = button_to t('idv.buttons.cancel'), verify_session_path, method: :delete, - class: 'btn btn-outline blue border border-blue rounded-lg col-12 input-submit' + class: 'btn col-12 p2 rounded-lg border border-blue blue border-box' - elsif user_signing_up = button_to t('sign_up.buttons.cancel'), destroy_user_path, method: :delete, - class: 'btn btn-outline blue border border-blue rounded-lg col-12 input-submit' + class: 'btn col-12 p2 rounded-lg border border-blue blue border-box' - else .col-12 = link_to cancel_link_text, '/', id: 'cancel-action-home', - class: 'btn btn-outline blue border border-blue rounded-lg center block' + class: 'btn col-12 p2 rounded-lg border border-blue blue border-box' == javascript_include_tag 'misc/cancel-action-modal' diff --git a/app/views/shared/_modal_verification.slim b/app/views/shared/_modal_verification.slim index f10a8b44dab..258fba4b395 100644 --- a/app/views/shared/_modal_verification.slim +++ b/app/views/shared/_modal_verification.slim @@ -1,9 +1,9 @@ #verification-modal.display-none = render layout: '/shared/modal_layout' do - .p4.cntnr-xxskinny.border-box.bg-white.rounded-xxl(class="modal-#{view_model.error}") + .p3.sm-p4.cntnr-xxskinny.border-box.bg-white.rounded-xxl(class="modal-#{view_model.error}") h2.my2.fs-20p.sans-serif.regular.center = t("idv.modal.#{view_model.step_name}.heading") hr.mb3.bw4.rounded - .mb5 + .mb4 p = view_model.message = render view_model.warning_partial, count: view_model.remaining_attempts .center diff --git a/app/views/shared/_personal_key_confirmation_modal.html.slim b/app/views/shared/_personal_key_confirmation_modal.html.slim index ba53666619c..3552b5b2dc3 100644 --- a/app/views/shared/_personal_key_confirmation_modal.html.slim +++ b/app/views/shared/_personal_key_confirmation_modal.html.slim @@ -8,16 +8,7 @@ = t('users.personal_key.confirmation_error') form(id='confirm-key' method='post' action="#{update_path}" name="key-confirm") - input( - aria-label="#{t('forms.personal_key.confirmation_label')}" - maxlength="#{code.length}" - autocomplete='off' - class='col-12 mb4 border-dashed field monospace personal-key caps' - name="personal-key" - pattern="^#{PersonalKeyFormatter.regexp_string}$" - required=true - spellcheck='false' - type='text') + = render 'shared/personal_key_input', code: code = hidden_field_tag :authenticity_token, form_authenticity_token .clearfix.mxn2 diff --git a/app/views/shared/_personal_key_input.html.slim b/app/views/shared/_personal_key_input.html.slim new file mode 100644 index 00000000000..9144a329d67 --- /dev/null +++ b/app/views/shared/_personal_key_input.html.slim @@ -0,0 +1,10 @@ +input( + aria-label="#{t('forms.personal_key.confirmation_label')}" + maxlength="#{PersonalKeyFormatter.code_length}" + autocomplete='off' + class='col-12 mb4 border-dashed field monospace personal-key caps' + name="personal_key" + pattern="^#{PersonalKeyFormatter.regexp_string}$" + required=true + spellcheck='false' + type='text') diff --git a/app/views/shared/_pii_review.html.slim b/app/views/shared/_pii_review.html.slim new file mode 100644 index 00000000000..13b5ae8d0fd --- /dev/null +++ b/app/views/shared/_pii_review.html.slim @@ -0,0 +1,19 @@ +.pl5 + .h6 = t('idv.review.full_name') + .h4.bold.ico-absolute.ico-absolute-success + = "#{pii[:first_name]} #{pii[:last_name]}" + + .mt3.h6 = t('idv.review.mailing_address') + .h4.bold.ico-absolute.ico-absolute-success + = render 'shared/address', address: pii + .mt3.h6 = t('idv.review.dob') + .h4.bold.ico-absolute.ico-absolute-success #{pii[:dob].to_date.to_formatted_s(:long)} + + .mt3.h6 = t('idv.review.ssn') + .h4.bold.ico-absolute.ico-absolute-success #{pii[:ssn]} + + .h6.mt3 = t('idv.messages.phone.phone_of_record') + .h4.bold.ico-absolute.ico-absolute-success #{pii[:phone]} + + .mt3.mb3 + = link_to t('idv.messages.review.financial_info'), MarketingSite.help_url diff --git a/app/views/shared/_user_verify_password.html.slim b/app/views/shared/_user_verify_password.html.slim new file mode 100644 index 00000000000..fb16c017c4c --- /dev/null +++ b/app/views/shared/_user_verify_password.html.slim @@ -0,0 +1,4 @@ += simple_form_for(current_user, url: update_path, + html: { autocomplete: 'off', method: :put, role: 'form' }) do |f| + = f.input :password, label: t('idv.form.password'), required: true + = f.button :submit, t('forms.buttons.submit.default'), class: 'btn btn-primary btn-wide' diff --git a/app/views/two_factor_authentication/personal_key_verification/show.html.slim b/app/views/two_factor_authentication/personal_key_verification/show.html.slim index c3f6d778ce5..00dacd73161 100644 --- a/app/views/two_factor_authentication/personal_key_verification/show.html.slim +++ b/app/views/two_factor_authentication/personal_key_verification/show.html.slim @@ -8,4 +8,4 @@ p.mt-tiny.mb0 = t('devise.two_factor_authentication.personal_key_prompt') = render 'partials/personal_key/entry_fields', f: f, attribute_name: :personal_key = f.button :submit, t('forms.buttons.submit.default'), class: 'btn btn-primary' -= render 'shared/cancel', link: destroy_user_session_path += render 'shared/cancel', link: sign_out_path diff --git a/app/views/two_factor_authentication/shared/max_otp_requests_reached.html.erb b/app/views/two_factor_authentication/shared/max_otp_requests_reached.html.erb new file mode 100644 index 00000000000..c5db7d9aabe --- /dev/null +++ b/app/views/two_factor_authentication/shared/max_otp_requests_reached.html.erb @@ -0,0 +1,19 @@ +<% lockout_time_in_words = decorator.lockout_time_remaining_in_words %> +<% lockout_time_remaining = decorator.lockout_time_remaining %> +<% title t('titles.account_locked') %> + +

+ <%= t('titles.account_locked') %> +

+

+<%= t("devise.two_factor_authentication.max_otp_requests_reached") %> +

+

+ <%= t('devise.two_factor_authentication.please_try_again_html', + time_remaining: content_tag(:span, lockout_time_in_words, id: 'countdown')) %> +

+ +<%= nonced_javascript_tag do %> + var test = <%= lockout_time_remaining %> * 1000; + window.LoginGov.countdownTimer(document.getElementById('countdown'), test); +<% end %> diff --git a/app/views/users/reactivate_account/index.html.slim b/app/views/users/reactivate_account/index.html.slim deleted file mode 100644 index d56954d0e6d..00000000000 --- a/app/views/users/reactivate_account/index.html.slim +++ /dev/null @@ -1,13 +0,0 @@ -- title t('titles.reactivate_account') - -h1.h3.my0 = t('forms.reactivate_profile.title') -p.mt-tiny.mb0 = t('forms.reactivate_profile.instructions') -= simple_form_for(@reactivate_account_form, url: reactivate_account_path, - html: { autocomplete: 'off', method: :post, role: 'form' }) do |f| - = f.error :base - = render 'partials/personal_key/entry_fields', f: f, attribute_name: :personal_key - = f.input :password, required: true - = f.button :submit, t('forms.reactivate_profile.submit'), class: 'mb1' - -.mt2.pt1.border-top - = link_to t('links.cancel'), account_path diff --git a/app/views/users/verify_password/new.html.slim b/app/views/users/verify_password/new.html.slim new file mode 100644 index 00000000000..20ea38eab25 --- /dev/null +++ b/app/views/users/verify_password/new.html.slim @@ -0,0 +1,9 @@ +- title t('idv.titles.review') + +h1.h3.my0 = t('idv.titles.session.review') + += render 'shared/user_verify_password', update_path: update_verify_password_path + +.mt4 + = accordion('review-verified-info', t('idv.messages.review.intro')) do + = render 'shared/pii_review', pii: @verify_password_form.decrypted_pii diff --git a/app/views/users/verify_personal_key/new.html.slim b/app/views/users/verify_personal_key/new.html.slim new file mode 100644 index 00000000000..18162e905f2 --- /dev/null +++ b/app/views/users/verify_personal_key/new.html.slim @@ -0,0 +1,18 @@ +.relative + form(id='verify-key' method='post' action="#{create_verify_personal_key_path}" name="verify-key") + .pt4.px1.sm-px5.key-badge.border.border-dashed.border-red.rounded-xl + h3.mt0.mb2 = t('forms.personal_key.title') + p.mb3 = t('forms.personal_key.instructions') + + = render 'shared/personal_key_input', code: '' + = hidden_field_tag :authenticity_token, form_authenticity_token + + .mt3 + = button_tag t('forms.buttons.continue'), type: 'submit', class: 'btn btn-primary btn-wide' + +.mt2 + = t('forms.personal_key.alternative') + = button_to(t('links.reverify'), reactivate_account_path, method: :put, + class: 'btn btn-link ml1', form_class: 'inline-block') + += render 'shared/cancel', link: account_path diff --git a/app/views/verify/review/new.html.slim b/app/views/verify/review/new.html.slim index 7c33a27e80c..4cd7d7768d6 100644 --- a/app/views/verify/review/new.html.slim +++ b/app/views/verify/review/new.html.slim @@ -2,29 +2,8 @@ h1.h3.my0 = t('idv.titles.session.review') -= simple_form_for(current_user, url: verify_review_path, - html: { autocomplete: 'off', method: :put, role: 'form' }) do |f| - = f.input :password, label: t('idv.form.password'), required: true - = f.button :submit, t('forms.buttons.submit.default'), class: 'btn btn-primary btn-wide' += render 'shared/user_verify_password', update_path: verify_review_path .mt4 = accordion('review-verified-info', t('idv.messages.review.intro')) do - .pl5 - .h6 = t('idv.review.full_name') - .h4.bold.ico-absolute.ico-absolute-success - = "#{@idv_params[:first_name]} #{@idv_params[:last_name]}" - - .mt3.h6 = t('idv.review.mailing_address') - .h4.bold.ico-absolute.ico-absolute-success - = render 'shared/address', address: @idv_params - .mt3.h6 = t('idv.review.dob') - .h4.bold.ico-absolute.ico-absolute-success #{@idv_params[:dob].to_date.to_formatted_s(:long)} - - .mt3.h6 = t('idv.review.ssn') - .h4.bold.ico-absolute.ico-absolute-success #{@idv_params[:ssn]} - - .h6.mt3 = t('idv.messages.phone.phone_of_record').capitalize - .h4.bold.ico-absolute.ico-absolute-success #{@idv_params[:phone]} - - .mt3.mb3 - = link_to t('idv.messages.review.financial_info'), MarketingSite.help_url + = render 'shared/pii_review', pii: @idv_params diff --git a/config/application.rb b/config/application.rb index 853f6a01491..b12f84bf8a2 100644 --- a/config/application.rb +++ b/config/application.rb @@ -16,6 +16,7 @@ class Application < Rails::Application config.browserify_rails.force = true config.browserify_rails.commandline_options = '-t [ babelify --presets [ es2015 ] ]' + config.i18n.available_locales = Figaro.env.available_locales.split(' ') config.i18n.load_path += Dir[Rails.root.join('config', 'locales', '**', '*.{yml}')] routes.default_url_options[:host] = Figaro.env.domain_name diff --git a/config/application.yml.example b/config/application.yml.example index 5043f04868f..3d9d481d90c 100644 --- a/config/application.yml.example +++ b/config/application.yml.example @@ -53,6 +53,7 @@ development: attribute_cost: '4000$8$4$' # SCrypt::Engine.calibrate(max_time: 0.5) attribute_encryption_key: '2086dfbd15f5b0c584f3664422a1d3409a0d2aa6084f65b6ba57d64d4257431c124158670c7655e45cabe64194f7f7b6c7970153c285bdb8287ec0c4f7553e25' attribute_encryption_key_queue: '["old-key-one", "old-key-two"]' + available_locales: 'en es' aws_kms_key_id: 'alias/login-dot-gov-development-keymaker' aws_region: 'us-east-1' dashboard_api_token: 'test_token' @@ -72,14 +73,14 @@ development: hmac_fingerprinter_key: 'a2c813d4dca919340866ba58063e4072adc459b767a74cf2666d5c1eef3861db26708e7437abde1755eb24f4034386b0fea1850a1cb7e56bff8fae3cc6ade96c' hmac_fingerprinter_key_queue: '["old-key-one", "old-key-two"]' idp_sso_target_url: 'https://upaya-mockidp.18f.gov/saml/auth' + lockout_period_in_minutes: '10' logins_per_ip_limit: '20' logins_per_ip_period: '8' newrelic_license_key: 'xxx' newrelic_browser_key: '' newrelic_browser_app_id: '' - otp_delivery_blocklist_bantime: '30' - otp_delivery_blocklist_findtime: '15' - otp_delivery_blocklist_maxretry: '3' + otp_delivery_blocklist_findtime: '5' + otp_delivery_blocklist_maxretry: '10' otp_valid_for: '5' password_pepper: 'f22d4b2cafac9066fe2f4416f5b7a32c6942d82f7e00740c7594a095fa8de8db17c05314be7b18a5d6dd5683e73eadf6cc95aa633e5ad9a701edb95192a6a105' password_strength_enabled: 'true' @@ -107,6 +108,7 @@ production: attribute_cost: '4000$8$4$' # SCrypt::Engine.calibrate(max_time: 0.5) attribute_encryption_key: 'generate via `rake secret`' attribute_encryption_key_queue: '["old-key-one", "old-key-two"]' + available_locales: 'en' aws_kms_key_id: 'change-me' aws_region: 'change-me' disable_email_sending: 'false' @@ -127,14 +129,14 @@ production: hmac_fingerprinter_key: 'generate via `rake secret`' hmac_fingerprinter_key_queue: '["old-key-one", "old-key-two"]' idp_sso_target_url: 'example.com/saml/auth' + lockout_period_in_minutes: '10' logins_per_ip_limit: '20' logins_per_ip_period: '8' newrelic_license_key: 'xxx' newrelic_browser_key: '' newrelic_browser_app_id: '' - otp_delivery_blocklist_bantime: '30' - otp_delivery_blocklist_findtime: '15' - otp_delivery_blocklist_maxretry: '3' + otp_delivery_blocklist_findtime: '5' + otp_delivery_blocklist_maxretry: '10' otp_valid_for: '5' participate_in_dap: 'false' password_pepper: 'generate via `rake secret`' @@ -162,6 +164,7 @@ test: attribute_cost: '800$8$1$' # SCrypt::Engine.calibrate(max_time: 0.01) attribute_encryption_key: '2086dfbd15f5b0c584f3664422a1d3409a0d2aa6084f65b6ba57d64d4257431c124158670c7655e45cabe64194f7f7b6c7970153c285bdb8287ec0c4f7553e25' attribute_encryption_key_queue: '["old-key-one", "old-key-two"]' + available_locales: 'en es' aws_kms_key_id: 'alias/login-dot-gov-test-keymaker' aws_region: 'us-east-1' domain_name: 'www.example.com' @@ -181,11 +184,11 @@ test: hmac_fingerprinter_key: 'a2c813d4dca919340866ba58063e4072adc459b767a74cf2666d5c1eef3861db26708e7437abde1755eb24f4034386b0fea1850a1cb7e56bff8fae3cc6ade96c' hmac_fingerprinter_key_queue: '["old-key-one", "old-key-two"]' idp_sso_target_url: 'http://identityprovider.example.com/saml/auth' + lockout_period_in_minutes: '5' logins_per_ip_limit: '2' logins_per_ip_period: '60' max_mail_events: '2' newrelic_license_key: 'xxx' - otp_delivery_blocklist_bantime: '1' otp_delivery_blocklist_findtime: '1' otp_delivery_blocklist_maxretry: '2' otp_valid_for: '5' diff --git a/config/initializers/figaro.rb b/config/initializers/figaro.rb index 3404e34e1b9..7f1f6d617ba 100644 --- a/config/initializers/figaro.rb +++ b/config/initializers/figaro.rb @@ -15,7 +15,6 @@ 'max_mail_events', 'max_mail_events_window_in_days', 'min_password_score', - 'otp_delivery_blocklist_bantime', 'otp_delivery_blocklist_findtime', 'otp_delivery_blocklist_maxretry', 'otp_valid_for', diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index cbaaa15772f..f75a491a419 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -31,7 +31,7 @@ def remote_ip # (blacklist & throttles are skipped) unless Rails.env.production? safelist('allow from localhost') do |req| - '127.0.0.1' == req.remote_ip || '::1' == req.remote_ip + req.remote_ip == '127.0.0.1' || req.remote_ip == '::1' end end @@ -83,24 +83,6 @@ def remote_ip end end - # After maxretry OTP requests in findtime minutes, - # block all requests from that user for bantime minutes. - blocklist('OTP delivery') do |req| - # `filter` returns truthy value if request fails, or if it's to a - # previously banned phone_number so the request is blocked - phone_number = req.env['warden'].user&.phone - - Allow2Ban.filter( - "otp-#{phone_number}", - maxretry: Figaro.env.otp_delivery_blocklist_maxretry.to_i, - findtime: Figaro.env.otp_delivery_blocklist_findtime.to_i.minutes, - bantime: Figaro.env.otp_delivery_blocklist_bantime.to_i.minutes - ) do - # The count for the phone_number is incremented if the return value is truthy - req.get? && req.path == '/otp/send' - end - end - ### Custom Throttle Response ### # By default, Rack::Attack returns an HTTP 429 for throttled responses, diff --git a/config/locales/devise/en.yml b/config/locales/devise/en.yml index 60d8a4c63e6..94019d25c3f 100644 --- a/config/locales/devise/en.yml +++ b/config/locales/devise/en.yml @@ -120,6 +120,9 @@ en: max_otp_login_attempts_reached: > Your account is temporarily locked because you have entered the one-time security code incorrectly too many times. + max_otp_requests_reached: > + Your account is temporarily locked because you have requested a + security code too many times. max_personal_key_login_attempts_reached: > Your account is temporarily locked because you have entered the personal key incorrectly too many times. diff --git a/config/locales/devise/es.yml b/config/locales/devise/es.yml index 6d96b683960..acbd5a27c26 100644 --- a/config/locales/devise/es.yml +++ b/config/locales/devise/es.yml @@ -108,6 +108,7 @@ es: max_otp_login_attempts_reached: Su cuenta ha sido bloqueada temporalmente porque ha ingresado el código de acceso único de forma incorrecta demasiadas veces. + max_otp_requests_reached: NOT TRANSLATED YET max_personal_key_login_attempts_reached: Su cuenta ha sido bloqueada temporalmente porque ha ingresado el código de acceso único de forma incorrecta demasiadas veces. diff --git a/config/locales/forms/en.yml b/config/locales/forms/en.yml index fe6a908efc5..480bfc9b794 100644 --- a/config/locales/forms/en.yml +++ b/config/locales/forms/en.yml @@ -22,11 +22,8 @@ en: labels: password: New password show: Show password - reactivate_profile: - instructions: Provide the information below to reactivate your account. - submit: Reactivate profile - title: Reactivate profile personal_key: + alternative: Don't have your personal key? title: Enter your personal key instructions: Please confirm you have a copy of your personal key by entering it below. confirmation_label: Personal key diff --git a/config/locales/forms/es.yml b/config/locales/forms/es.yml index 3320f316d6c..6facc87038c 100644 --- a/config/locales/forms/es.yml +++ b/config/locales/forms/es.yml @@ -22,11 +22,8 @@ es: labels: password: Nueva contraseña show: NOT TRANSLATED YET - reactivate_profile: - instructions: Proveer la siguiente información para reactivar su cuenta. - submit: Reactivar perfil - title: Reactivar perfil personal_key: + alternative: NOT TRANSLATED YET title: NOT TRANSLATED YET instructions: NOT TRANSLATED YET confirmation_label: NOT TRANSLATED YET diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index ce747d86ecb..fd3104718a8 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -134,7 +134,7 @@ en: in_your_name: in your name or a family member's name intro: We can only send a confirmation code to a telephone number linked to your legal identity. - phone_of_record: phone of record + phone_of_record: Phone of record prepaid: on a contract, not prepaid same_as_2fa: > This phone number can be the same one you used to set up your one-time diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index 6073a393ebd..efb28f903ff 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -13,7 +13,8 @@ es: cancel: modal_header: NOT TRANSLATED YET warning_header: NOT TRANSLATED YET - warning_points: NOT TRANSLATED YET + warning_points: + - NOT TRANSLATED YET errors: bad_dob: NOT TRANSLATED YET duplicate_ssn: NOT TRANSLATED YET diff --git a/config/locales/instructions/en.yml b/config/locales/instructions/en.yml index 320ba4a18fa..b31a1308c12 100644 --- a/config/locales/instructions/en.yml +++ b/config/locales/instructions/en.yml @@ -26,6 +26,9 @@ en: intro: > We take extra steps to keep your personal information secure and private, so resetting your password takes a little extra effort. + modal: + copy: If you don't have your personal key, you will need to verify your identity again. + heading: Don't have your personal key? with_key: Do you have your personal key? forgot_password: close_window: You can close this browser window once you have reset your password. diff --git a/config/locales/instructions/es.yml b/config/locales/instructions/es.yml index 89798cec450..5e1ccb356ca 100644 --- a/config/locales/instructions/es.yml +++ b/config/locales/instructions/es.yml @@ -9,7 +9,7 @@ es: %{app}.%{tooltip} or: NOT TRANSLATED YET sms: - confirm_code_html: Lo enviamos a %{number}. ¿No recibió un código? %{link}. + confirm_code_html: Lo enviamos a %{number}. ¿No recibió un código? fallback_html: NOT TRANSLATED YET voice: confirm_code_html: NOT TRANSLATED YET @@ -20,6 +20,9 @@ es: begin: NOT TRANSLATED YET explanation: NOT TRANSLATED YET intro: NOT TRANSLATED YET + modal: + copy: NOT TRANSLATED YET + heading: NOT TRANSLATED YET with_key: NOT TRANSLATED YET forgot_password: close_window: Puede cerrar esta ventana del navegador despues que haya restablecido su contraseña. diff --git a/config/locales/links/en.yml b/config/locales/links/en.yml index 9ac5c7482cf..e70bd4206fe 100644 --- a/config/locales/links/en.yml +++ b/config/locales/links/en.yml @@ -20,6 +20,7 @@ en: privacy_policy: Privacy & security remove: Remove resend: Resend email + reverify: Please verify your identity again. sign_in: Sign in sign_out: Sign out cancel: Cancel diff --git a/config/locales/links/es.yml b/config/locales/links/es.yml index 82e7ddbfe50..da1070539d6 100644 --- a/config/locales/links/es.yml +++ b/config/locales/links/es.yml @@ -20,6 +20,7 @@ es: privacy_policy: Privacidad y seguridad remove: NOT TRANSLATED YET resend: Enviar de nuevo + reverify: NOT TRANSLATED YET sign_in: Iniciar sesión sign_out: Cerrar sesión cancel: Cancelar diff --git a/config/locales/notices/en.yml b/config/locales/notices/en.yml index e51b75ba272..98188b14bbd 100644 --- a/config/locales/notices/en.yml +++ b/config/locales/notices/en.yml @@ -1,6 +1,7 @@ --- en: notices: + account_recovery: Great! You have your personal key. dap_html: > @@ -24,11 +25,11 @@ en: continue: Keep me signed in sign_out: Sign me out message_html: > - For your security, we will sign you out in %{time_left_in_session} unless + For your security, we will sign you out in %{time_left_in_session} unless you tell us otherwise. partially_signed_in: - continue: Continue signing in - sign_out: Cancel signing in + continue: Continue sign in + sign_out: Cancel sign in message_html: > For your security, in %{time_left_in_session} we will cancel your sign in. diff --git a/config/locales/notices/es.yml b/config/locales/notices/es.yml index 9c3217130d2..46c9667b8f6 100644 --- a/config/locales/notices/es.yml +++ b/config/locales/notices/es.yml @@ -1,6 +1,7 @@ --- es: notices: + account_recovery: NOT TRANSLATED YET dap_html: NOT TRANSLATED YET forgot_password: use_diff_email: diff --git a/config/locales/sign_up/es.yml b/config/locales/sign_up/es.yml index 1d885d65fb6..48d40455ef4 100644 --- a/config/locales/sign_up/es.yml +++ b/config/locales/sign_up/es.yml @@ -8,6 +8,7 @@ modal_header: NOT TRANSLATED YET warning_header: NOT TRANSLATED YET success: NOT TRANSLATED YET - warning_points: NOT TRANSLATED YET + warning_points: + - NOT TRANSLATED YET registrations: create_account: Empezar diff --git a/config/locales/user_mailer/en.yml b/config/locales/user_mailer/en.yml index 404166dea82..c3804be8068 100644 --- a/config/locales/user_mailer/en.yml +++ b/config/locales/user_mailer/en.yml @@ -10,9 +10,9 @@ en: intro: The email address for your %{app} account has been changed. password_changed: help: > - If you did not change your password, please visit the %{app} + If you did not make this change, please visit the %{app} %{help_link} or %{contact_link}. - intro: The password for your %{app} account has been changed. + intro: You have a new password for your %{app} account. phone_changed: help: > If you did not want to change your phone number, please visit the %{app} %{help_link} diff --git a/config/routes.rb b/config/routes.rb index 9f4ab0e068a..a21e217810d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -45,10 +45,14 @@ as: :openid_connect_configuration get '/account' => 'accounts#show' - get '/account/reactivate' => 'users/reactivate_account#index', as: :reactivate_account - post '/account/reactivate' => 'users/reactivate_account#create' - get '/account/reactivate/start' => 'reactivate_account#index', as: :manage_reactivate_account + get '/account/reactivate/start' => 'reactivate_account#index', as: :reactivate_account put '/account/reactivate/start' => 'reactivate_account#update' + get '/account/reactivate/verify_password' => 'users/verify_password#new', as: :verify_password + put '/account/reactivate/verify_password' => 'users/verify_password#update', as: :update_verify_password + get '/account/reactivate/verify_personal_key' => 'users/verify_personal_key#new', + as: :verify_personal_key + post '/account/reactivate/verify_personal_key' => 'users/verify_personal_key#create', + as: :create_verify_personal_key get '/account/verify_phone' => 'users/verify_profile_phone#index', as: :verify_profile_phone post '/account/verify_phone' => 'users/verify_profile_phone#create' @@ -110,6 +114,8 @@ get '/sign_up/completed' => 'sign_up/completions#show', as: :sign_up_completed post '/sign_up/completed' => 'sign_up/completions#update' + match '/sign_out' => 'sign_out#destroy', via: %i[get post delete] + delete '/users' => 'users#destroy', as: :destroy_user if FeatureManagement.enable_identity_verification? @@ -147,5 +153,7 @@ # The line below will route all requests that aren't # defined route to the 404 page. Therefore, anything you put after this rule # will be ignored. - match '*path', via: :all, to: 'pages#page_not_found' + constraints(format: /html/) do + match '*path', via: :all, to: 'pages#page_not_found' + end end diff --git a/config/service_providers.yml b/config/service_providers.yml index 14b76e024fc..e61ac94bdde 100644 --- a/config/service_providers.yml +++ b/config/service_providers.yml @@ -52,6 +52,7 @@ test: - 'http://localhost:7654/' cert: 'saml_test_sp' friendly_name: 'Test SP' + assertion_consumer_logout_service_url: '' development: 'https://rp1.serviceprovider.com/auth/saml/metadata': @@ -358,10 +359,10 @@ production: friendly_name: 'Railroad Retirement Board' agency: 'RRB' logo: 'rrb.svg' - acs_url: 'https://onlinetest.rrb.gov/login/login/callback' - assertion_consumer_logout_service_url: 'https://onlinetest.rrb.gov/login/login/logout' - sp_initiated_login_url: 'https://onlinetest.rrb.gov/login/login/Auth' - return_to_sp_url: 'https://onlinetest.rrb.gov/login/login/RRBHome' + acs_url: 'https://onlinetest.rrb.gov/AA1/login/login/callback' + assertion_consumer_logout_service_url: 'https://onlinetest.rrb.gov/AA1/BOS/BOS/logout' + sp_initiated_login_url: 'https://onlinetest.rrb.gov/AA1/login/login/Auth' + return_to_sp_url: 'https://onlinetest.rrb.gov/AA1/Login/login/RRBHome' block_encryption: 'aes256-cbc' cert: 'rrb_bos_pre_prod' attribute_bundle: diff --git a/db/migrate/20170621202836_create_otp_requests_tracker.rb b/db/migrate/20170621202836_create_otp_requests_tracker.rb new file mode 100644 index 00000000000..22c4a3b0d41 --- /dev/null +++ b/db/migrate/20170621202836_create_otp_requests_tracker.rb @@ -0,0 +1,16 @@ +class CreateOtpRequestsTracker < ActiveRecord::Migration + def change + create_table :otp_requests_trackers do |t| + t.text :encrypted_phone + t.timestamp :otp_last_sent_at + t.integer :otp_send_count, default: 0 + t.string :attribute_cost + t.string :phone_fingerprint, default: '', null: false + + t.timestamps + + t.index :phone_fingerprint, unique: true + t.index :updated_at + end + end +end diff --git a/db/migrate/20170626205402_remove_encrypted_phone_from_otp_requests_tracker.rb b/db/migrate/20170626205402_remove_encrypted_phone_from_otp_requests_tracker.rb new file mode 100644 index 00000000000..36c575b0228 --- /dev/null +++ b/db/migrate/20170626205402_remove_encrypted_phone_from_otp_requests_tracker.rb @@ -0,0 +1,5 @@ +class RemoveEncryptedPhoneFromOtpRequestsTracker < ActiveRecord::Migration + def change + remove_column :otp_requests_trackers, :encrypted_phone, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 4895d88e652..08da593db47 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170531204549) do +ActiveRecord::Schema.define(version: 20170626205402) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -68,6 +68,18 @@ add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree add_index "identities", ["uuid"], name: "index_identities_on_uuid", unique: true, using: :btree + create_table "otp_requests_trackers", force: :cascade do |t| + t.datetime "otp_last_sent_at" + t.integer "otp_send_count", default: 0 + t.string "attribute_cost" + t.string "phone_fingerprint", default: "", null: false + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "otp_requests_trackers", ["phone_fingerprint"], name: "index_otp_requests_trackers_on_phone_fingerprint", unique: true, using: :btree + add_index "otp_requests_trackers", ["updated_at"], name: "index_otp_requests_trackers_on_updated_at", using: :btree + create_table "profiles", force: :cascade do |t| t.integer "user_id", null: false t.boolean "active", default: false, null: false diff --git a/lib/i18n_override.rb b/lib/i18n_override.rb index 0ddecbdfb80..a702f0c54e3 100644 --- a/lib/i18n_override.rb +++ b/lib/i18n_override.rb @@ -5,7 +5,7 @@ class << self def translate_with_markup(*args) i18n_text = normal_translate(*args) return i18n_text unless FeatureManagement.enable_i18n_mode? && i18n_text.is_a?(String) - return i18n_text if caller[1] =~ /flows_spec.rb|session_helper.rb/ + return i18n_text if caller(2..2).first =~ /flows_spec.rb|session_helper.rb/ key = args.first.to_s rtn = i18n_text + i18n_mode_additional_markup(key) diff --git a/lib/tasks/rotate.rake b/lib/tasks/rotate.rake index ecea5e29ec0..6a599ef7523 100644 --- a/lib/tasks/rotate.rake +++ b/lib/tasks/rotate.rake @@ -19,21 +19,6 @@ namespace :rotate do end end - desc 'encrypt plain OTP secret key' - task encrypt_otp: :environment do - num_users = User.where.not(otp_secret_key: nil).count - progress = new_progress_bar('Users', num_users) - - User.where.not(otp_secret_key: nil).find_in_batches.with_index do |users, _batch| - users.each do |user| - encrypted_attribute = EncryptedAttribute.new_from_decrypted(user.otp_secret_key).encrypted - id = user.id - execute "UPDATE users SET encrypted_otp_secret_key='#{encrypted_attribute}' WHERE id=#{id}" - progress&.increment - end - end - end - def new_progress_bar(label, num) return if ENV['PROGRESS'] == 'no' ProgressBar.create( diff --git a/lib/tasks/user_flows.rake b/lib/tasks/user_flows.rake index fc5a38477ca..af9fb3c7b89 100644 --- a/lib/tasks/user_flows.rake +++ b/lib/tasks/user_flows.rake @@ -9,7 +9,7 @@ unless Rails.env.production? end desc 'Exports user flows for the web' - task 'user_flows:web' do |t| + task 'user_flows:web' do ENV['RAILS_DISABLE_ASSET_DIGEST'] = 'true' require './lib/user_flow_exporter' Rake::Task['spec:user_flows'].invoke diff --git a/lib/user_flow_exporter.rb b/lib/user_flow_exporter.rb index b1b089b1e65..0bc8af45368 100644 --- a/lib/user_flow_exporter.rb +++ b/lib/user_flow_exporter.rb @@ -1,11 +1,11 @@ # This module is part of the User Flows toolchest -# +# # UserFlowExporter.run - scrapes user flows for use on the web -# +# # Dependencies: # - Must be running the application locally eg (localhost:3000) # - Must have wget installed and available on your PATH -# +# # Executing: # Start the application with: # $ make run @@ -35,7 +35,7 @@ def self.run output_dir = "public#{FEDERALIST_PATH}" # -r = recursively mirrors site - # -H = span hosts (e.g. include assets from other domains) + # -H = span hosts (e.g. include assets from other domains) # -p = download all assets associated with the page # --no-host-directories = removes domain prefix from output path # -P = output prefix (a.k.a the directory to dump the assets) @@ -56,8 +56,9 @@ def self.run def self.massage_html(dir) Dir.glob("#{dir}/**/*.html") do |html| - File.open(html) do |f| - contents = File.read(f.path) + File.open(html) do |file| + path = file.path + contents = File.read(path) contents.gsub!("http://#{ASSET_HOST}/", "#{FEDERALIST_PATH}/") contents.gsub!('.css?body=1', '.css') contents.gsub!('.js?body=1', '.js') @@ -67,8 +68,8 @@ def self.massage_html(dir) contents.gsub!("", "") - File.open(f.path, "w") {|file| file.puts contents } - Kernel.puts "Updated #{f.path} references" + File.open(path, "w") {|file| file.puts contents } + Kernel.puts "Updated #{path} references" end end end diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index 98f20b72581..0315fa4c4d9 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -130,6 +130,25 @@ end context 'user is not signed in' do + context 'without valid acr_values' do + before { params.delete(:acr_values) } + + it 'handles the error and does not blow up' do + action + + expect(response).to redirect_to(/^#{params[:redirect_uri]}/) + end + end + + context 'with a bad redirect_uri' do + before { params[:redirect_uri] = '!!!' } + + it 'renders the error page' do + action + expect(controller).to render_template('openid_connect/authorization/error') + end + end + it 'redirects to SP landing page with the request_id in the params' do action sp_request_id = ServiceProviderRequest.last.uuid diff --git a/spec/controllers/reactivate_account_controller_spec.rb b/spec/controllers/reactivate_account_controller_spec.rb index b88d9593cb2..b094e36c7ff 100644 --- a/spec/controllers/reactivate_account_controller_spec.rb +++ b/spec/controllers/reactivate_account_controller_spec.rb @@ -23,12 +23,6 @@ expect(subject).to render_template(:index) end - - it 'sets a key on the user session for future redirect guidance' do - get :index - - expect(subject.user_session[:acknowledge_personal_key]).to eq true - end end context 'wthout a password reset profile' do diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index a08b2c16d98..f6fe4d29e6d 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -292,11 +292,11 @@ sp_request_id = ServiceProviderRequest.last.uuid expect(session[:sp]).to eq( - loa3: false, issuer: saml_settings.issuer, - request_id: sp_request_id, + loa3: false, request_url: @saml_request.request.original_url, - requested_attributes: [:email] + request_id: sp_request_id, + requested_attributes: ['email'] ) end diff --git a/spec/controllers/sign_out_controller_spec.rb b/spec/controllers/sign_out_controller_spec.rb new file mode 100644 index 00000000000..a080e73cf6c --- /dev/null +++ b/spec/controllers/sign_out_controller_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +describe SignOutController do + describe '#destroy' do + it 'redirects to decorated_session.cancel_link_path with flash message' do + stub_sign_in_before_2fa + allow(controller.decorated_session).to receive(:cancel_link_path).and_return('foo') + + get :destroy + + expect(response).to redirect_to 'foo' + expect(flash[:success]).to eq t('devise.sessions.signed_out') + end + + it 'calls #sign_out and #delete_branded_experience' do + expect(controller).to receive(:sign_out).and_call_original + expect(controller).to receive(:delete_branded_experience) + + get :destroy + end + end +end diff --git a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb index 7a1a427e8b1..f7c3b1afcf2 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -158,8 +158,9 @@ context 'when the user lockout period expires' do before do sign_in_before_2fa + lockout_period = Figaro.env.lockout_period_in_minutes.to_i.minutes subject.current_user.update( - second_factor_locked_at: Time.zone.now - Devise.direct_otp_valid_for - 1.second, + second_factor_locked_at: Time.zone.now - lockout_period - 1.second, second_factor_attempts_count: 3 ) end diff --git a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb index 66152e2f37e..d55bb7d82bf 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -87,10 +87,11 @@ context 'when the user lockout period expires' do before do + lockout_period = Figaro.env.lockout_period_in_minutes.to_i.minutes user = create( :user, :signed_up, - second_factor_locked_at: Time.zone.now - Devise.direct_otp_valid_for - 1.second, + second_factor_locked_at: Time.zone.now - lockout_period - 1.second, second_factor_attempts_count: 3 ) sign_in_before_2fa(user) diff --git a/spec/controllers/users/reactivate_account_controller_spec.rb b/spec/controllers/users/reactivate_account_controller_spec.rb deleted file mode 100644 index 38337b04a9c..00000000000 --- a/spec/controllers/users/reactivate_account_controller_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'rails_helper' - -RSpec.describe Users::ReactivateAccountController do - include Features::LocalizationHelper - - describe '#create' do - subject(:action) do - post( - :create, - reactivate_account_form: { - password: 'password', - personal_key: 'personal_key', - } - ) - end - - before do - stub_sign_in - - form = instance_double('ReactivateAccountForm', submit: success) - expect(controller).to receive(:build_reactivate_account_form).and_return(form) - end - - context 'with a valid form' do - let(:success) { true } - - it 'redirects to the profile page' do - action - - expect(response).to redirect_to(account_path) - end - end - - context 'with an invalid form' do - let(:success) { false } - - it 'renders the index page to show errors' do - action - - expect(response).to render_template('users/reactivate_account/index') - end - end - end -end diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index 02214bcae8c..26b9618fa99 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -111,7 +111,8 @@ def index describe '#send_code' do context 'when selecting SMS OTP delivery' do before do - sign_in_before_2fa + @user = create(:user, :with_phone) + sign_in_before_2fa(@user) @old_otp = subject.current_user.direct_otp allow(SmsOtpSenderJob).to receive(:perform_later) end @@ -147,6 +148,27 @@ def index get :send_code, otp_delivery_selection_form: { otp_delivery_preference: 'sms' } end + + it 'calls OtpRateLimiter#exceeded_otp_send_limit? and #increment' do + otp_rate_limiter = instance_double(OtpRateLimiter) + allow(OtpRateLimiter).to receive(:new).with(phone: @user.phone, user: @user). + and_return(otp_rate_limiter) + + expect(otp_rate_limiter).to receive(:exceeded_otp_send_limit?) + expect(otp_rate_limiter).to receive(:increment) + + get :send_code, otp_delivery_selection_form: { otp_delivery_preference: 'sms' } + end + + it 'marks the user as locked out after too many attempts' do + expect(@user.second_factor_locked_at).to be_nil + + (Figaro.env.otp_delivery_blocklist_maxretry.to_i + 1).times do + get :send_code, otp_delivery_selection_form: { otp_delivery_preference: 'sms' } + end + + expect(@user.reload.second_factor_locked_at.to_f).to be_within(0.1).of(Time.zone.now.to_f) + end end context 'when selecting voice OTP delivery' do diff --git a/spec/controllers/users/verify_password_controller_spec.rb b/spec/controllers/users/verify_password_controller_spec.rb new file mode 100644 index 00000000000..540ce2967a7 --- /dev/null +++ b/spec/controllers/users/verify_password_controller_spec.rb @@ -0,0 +1,92 @@ +require 'rails_helper' + +describe Users::VerifyPasswordController do + let(:user) { create(:user, profiles: profiles, personal_key: personal_key) } + let(:profiles) { [] } + let(:recovery_hash) { { personal_key: personal_key } } + let(:personal_key) { 'key' } + + before do + stub_sign_in(user) + end + + context 'without password_reset_profile' do + describe '#new' do + it 'redirects user to the home page' do + get :new + + expect(response).to redirect_to(root_url) + end + end + end + + context 'without personal key flag set' do + let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] } + + describe '#new' do + it 'redirects to the root url' do + get :new + expect(response).to redirect_to(root_url) + end + end + + describe '#update' do + it 'redirects to the root url' do + get :new + expect(response).to redirect_to(root_url) + end + end + end + + context 'with password reset profile' do + let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] } + let(:response_ok) { FormResponse.new(success: true, errors: {}, extra: { personal_key: key }) } + let(:response_bad) { FormResponse.new(success: false, errors: {}) } + let(:key) { 'key' } + + before do + allow(subject.reactivate_account_session).to receive(:personal_key?).and_return(personal_key) + end + + describe '#new' do + it 'renders the `new` template' do + get :new + + expect(response).to render_template(:new) + end + end + + describe '#update' do + let(:form) { instance_double(VerifyPasswordForm) } + + before do + expect(controller).to receive(:verify_password_form).and_return(form) + end + + context 'with valid password' do + before do + allow(form).to receive(:submit).and_return(response_ok) + put :update, user: { password: user.password } + end + + it 'redirects to the account page' do + expect(response).to redirect_to(account_url) + end + + it 'sets a new personal key as a flash message' do + expect(flash[:personal_key]).to eq(key) + end + end + + context 'without valid password' do + it 'renders the new template' do + allow(form).to receive(:submit).and_return(response_bad) + + put :update, user: { password: user.password } + + expect(response).to render_template(:new) + end + end + end + end +end diff --git a/spec/controllers/users/verify_personal_key_controller_spec.rb b/spec/controllers/users/verify_personal_key_controller_spec.rb new file mode 100644 index 00000000000..0e16e56d7ca --- /dev/null +++ b/spec/controllers/users/verify_personal_key_controller_spec.rb @@ -0,0 +1,90 @@ +require 'rails_helper' + +describe Users::VerifyPersonalKeyController do + let(:user) { create(:user, profiles: profiles, personal_key: personal_key) } + let(:profiles) { [] } + let(:personal_key) { 'key' } + + before { stub_sign_in(user) } + + describe 'before actions' do + it 'only allows 2fa users through' do + expect(subject).to have_actions(:confirm_two_factor_authenticated) + end + end + + describe '#new' do + context 'without password_reset_profile' do + it 'redirects user to the home page' do + get :new + expect(response).to redirect_to(root_url) + end + end + + context 'with password reset profile' do + let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] } + + it 'renders the `new` template' do + get :new + + expect(response).to render_template(:new) + end + + it 'displays a flash message to the user' do + get :new + + expect(subject.flash[:notice]).to eq(t('notices.account_recovery')) + end + end + end + + describe '#create' do + let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] } + let(:form) { instance_double(VerifyPersonalKeyForm) } + let(:error_text) { 'bad_key' } + let(:personal_key_error) { { personal_key: [error_text] } } + let(:response_ok) { FormResponse.new(success: true, errors: {}) } + let(:response_bad) { FormResponse.new(success: false, errors: personal_key_error, extra: {}) } + + context 'wth a valid form' do + before do + allow(VerifyPersonalKeyForm).to receive(:new). + with(user: subject.current_user, personal_key: personal_key). + and_return(form) + allow(form).to receive(:submit).and_return(response_ok) + end + + it 'redirects to the next step of the account recovery flow' do + post :create, personal_key: personal_key + + expect(response).to redirect_to(verify_password_url) + end + + it 'stores that the personal key was entered in the user session' do + post :create, personal_key: personal_key + + expect(subject.reactivate_account_session.personal_key?).to eq(true) + end + end + + context 'with an invalid form' do + let(:bad_key) { 'baaad' } + + before do + allow(VerifyPersonalKeyForm).to receive(:new). + with(user: subject.current_user, personal_key: bad_key). + and_return(form) + allow(form).to receive(:submit).and_return(response_bad) + post :create, personal_key: bad_key + end + + it 'sets an error in the flash' do + expect(flash[:error]).to eq(error_text) + end + + it 'renders the new template' do + expect(response).to render_template(:new) + end + end + end +end diff --git a/spec/controllers/verify/finance_controller_spec.rb b/spec/controllers/verify/finance_controller_spec.rb index d897a3da771..4ae70d9091c 100644 --- a/spec/controllers/verify/finance_controller_spec.rb +++ b/spec/controllers/verify/finance_controller_spec.rb @@ -138,7 +138,6 @@ result = { success: true, errors: {}, - vendor: { reasons: ['Good number'] }, } expect(@analytics).to have_received(:track_event).with( @@ -154,7 +153,6 @@ result = { success: false, errors: { ccn: ['The ccn could not be verified.'] }, - vendor: { reasons: ['Bad number'] }, } expect(@analytics).to have_received(:track_event). @@ -171,7 +169,6 @@ result = { success: false, errors: { ccn: ['Credit card number should be only last 8 digits.'] }, - vendor: { reasons: nil }, } expect(@analytics).to have_received(:track_event). diff --git a/spec/controllers/verify/phone_controller_spec.rb b/spec/controllers/verify/phone_controller_spec.rb index b084db44bdc..5f20d7df012 100644 --- a/spec/controllers/verify/phone_controller_spec.rb +++ b/spec/controllers/verify/phone_controller_spec.rb @@ -66,7 +66,6 @@ errors: { phone: [invalid_phone_message], }, - vendor: { reasons: nil }, } expect(@analytics).to have_received(:track_event).with( @@ -89,7 +88,7 @@ put :create, idv_phone_form: { phone: good_phone } - result = { success: true, errors: {}, vendor: { reasons: ['Good number'] } } + result = { success: true, errors: {} } expect(@analytics).to have_received(:track_event).with( Analytics::IDV_PHONE_CONFIRMATION, result @@ -107,7 +106,6 @@ errors: { phone: ['The phone number could not be verified.'], }, - vendor: { reasons: ['Bad number'] }, } expect(flash[:warning]).to match t('idv.modal.phone.heading') diff --git a/spec/controllers/verify/sessions_controller_spec.rb b/spec/controllers/verify/sessions_controller_spec.rb index 21ff0ee7d3b..b0e90722b48 100644 --- a/spec/controllers/verify/sessions_controller_spec.rb +++ b/spec/controllers/verify/sessions_controller_spec.rb @@ -94,7 +94,7 @@ it 'assigned user UUID to applicant' do post :create, profile: user_attrs - expect(subject.idv_session.applicant[:uuid]).to eq subject.current_user.uuid + expect(subject.idv_session.applicant['uuid']).to eq subject.current_user.uuid end end diff --git a/spec/controllers/verify_controller_spec.rb b/spec/controllers/verify_controller_spec.rb index ca51bf85787..dad1b899d63 100644 --- a/spec/controllers/verify_controller_spec.rb +++ b/spec/controllers/verify_controller_spec.rb @@ -38,11 +38,11 @@ it 'redirects to account recovery if user has a password reset profile' do profile = create(:profile, deactivation_reason: :password_reset) stub_sign_in(profile.user) - allow(subject).to receive(:user_session).and_return(acknowledge_personal_key: true) + allow(subject.reactivate_account_session).to receive(:started?).and_return(true) get :index - expect(response).to redirect_to manage_reactivate_account_url + expect(response).to redirect_to reactivate_account_url end end diff --git a/spec/decorators/service_provider_session_decorator_spec.rb b/spec/decorators/service_provider_session_decorator_spec.rb index 3f1ed39ff0c..56bd0ab121a 100644 --- a/spec/decorators/service_provider_session_decorator_spec.rb +++ b/spec/decorators/service_provider_session_decorator_spec.rb @@ -3,7 +3,12 @@ RSpec.describe ServiceProviderSessionDecorator do let(:view_context) { ActionController::Base.new.view_context } subject do - ServiceProviderSessionDecorator.new(sp: sp, view_context: view_context, sp_session: {}) + ServiceProviderSessionDecorator.new( + sp: sp, + view_context: view_context, + sp_session: {}, + service_provider_request: ServiceProviderRequest.new + ) end let(:sp) { build_stubbed(:service_provider) } let(:sp_name) { subject.sp_name } @@ -59,7 +64,10 @@ it 'returns the agency name if friendly name is not present' do sp = build_stubbed(:service_provider, friendly_name: nil) subject = ServiceProviderSessionDecorator.new( - sp: sp, view_context: view_context, sp_session: {} + sp: sp, + view_context: view_context, + sp_session: {}, + service_provider_request: ServiceProviderRequest.new ) expect(subject.sp_name).to eq sp.agency expect(subject.sp_name).to_not be_nil @@ -73,7 +81,10 @@ sp = build_stubbed(:service_provider, logo: sp_logo) subject = ServiceProviderSessionDecorator.new( - sp: sp, view_context: view_context, sp_session: {} + sp: sp, + view_context: view_context, + sp_session: {}, + service_provider_request: ServiceProviderRequest.new ) expect(subject.sp_logo).to eq sp_logo @@ -85,7 +96,10 @@ sp = build_stubbed(:service_provider, logo: nil) subject = ServiceProviderSessionDecorator.new( - sp: sp, view_context: view_context, sp_session: {} + sp: sp, + view_context: view_context, + sp_session: {}, + service_provider_request: ServiceProviderRequest.new ) expect(subject.sp_logo).to eq 'generic.svg' @@ -96,7 +110,10 @@ describe '#cancel_link_path' do it 'returns sign_up_start_url with the request_id as a param' do subject = ServiceProviderSessionDecorator.new( - sp: sp, view_context: view_context, sp_session: { request_id: 'foo' } + sp: sp, + view_context: view_context, + sp_session: { request_id: 'foo' }, + service_provider_request: ServiceProviderRequest.new ) expect(subject.cancel_link_path). diff --git a/spec/decorators/user_decorator_spec.rb b/spec/decorators/user_decorator_spec.rb index 994e203fd49..5cea4336e82 100644 --- a/spec/decorators/user_decorator_spec.rb +++ b/spec/decorators/user_decorator_spec.rb @@ -6,9 +6,9 @@ Timecop.freeze(Time.zone.now) do user = build_stubbed(:user, second_factor_locked_at: Time.zone.now - 180) user_decorator = UserDecorator.new(user) - allow(Devise).to receive(:direct_otp_valid_for).and_return(535) + allow(Figaro.env).to receive(:lockout_period_in_minutes).and_return('8') - expect(user_decorator.lockout_time_remaining).to eq 355 + expect(user_decorator.lockout_time_remaining).to eq 300 end end end @@ -18,10 +18,10 @@ Timecop.freeze(Time.zone.now) do user = build_stubbed(:user, second_factor_locked_at: Time.zone.now - 180) user_decorator = UserDecorator.new(user) - allow(Devise).to receive(:direct_otp_valid_for).and_return(535) + allow(Figaro.env).to receive(:lockout_period_in_minutes).and_return('8') expect(user_decorator.lockout_time_remaining_in_words). - to eq '5 minutes and 55 seconds' + to eq '5 minutes' end end end diff --git a/spec/features/idv/flow_spec.rb b/spec/features/idv/flow_spec.rb index 3530bb75ce9..4ebe49ed1dd 100644 --- a/spec/features/idv/flow_spec.rb +++ b/spec/features/idv/flow_spec.rb @@ -384,6 +384,8 @@ end scenario 'continue phone OTP verification after cancel' do + allow(Figaro.env).to receive(:otp_delivery_blocklist_maxretry).and_return('4') + different_phone = '555-555-9876' user = sign_in_live_with_2fa visit verify_session_path diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index c700d626f14..c0367043c44 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -102,7 +102,6 @@ prompt: 'select_account' ) - sp_request_id = ServiceProviderRequest.last.uuid allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) sign_in_user(user) @@ -112,8 +111,6 @@ click_submit_default expect(current_url).to start_with('http://localhost:7654/auth/result') - expect(ServiceProviderRequest.from_uuid(sp_request_id)). - to be_a NullServiceProviderRequest expect(page.get_rack_session.keys).to_not include('sp') end @@ -134,7 +131,6 @@ prompt: 'select_account' ) - sp_request_id = ServiceProviderRequest.last.uuid allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) sign_in_user(user) @@ -150,8 +146,6 @@ click_submit_default expect(current_url).to start_with('http://localhost:7654/auth/result') - expect(ServiceProviderRequest.from_uuid(sp_request_id)). - to be_a NullServiceProviderRequest expect(page.get_rack_session.keys).to_not include('sp') end end @@ -238,16 +232,12 @@ sign_up_user_from_sp_without_confirming_email(email) end - sp_request_id = ServiceProviderRequest.last.uuid - perform_in_browser(:two) do confirm_email_in_a_different_browser(email) click_button t('forms.buttons.continue') redirect_uri = URI(current_url) expect(redirect_uri.to_s).to start_with('gov.gsa.openidconnect.test://result') - expect(ServiceProviderRequest.from_uuid(sp_request_id)). - to be_a NullServiceProviderRequest expect(page.get_rack_session.keys).to_not include('sp') end end @@ -423,15 +413,14 @@ end context 'visiting IdP via SP, then going back to SP and visiting IdP again' do - it 'maintains the request_id in the params' do + it 'displays the branded page' do visit_idp_from_sp_with_loa1 - sp_request_id = ServiceProviderRequest.last.uuid - expect(current_url).to eq sign_up_start_url(request_id: sp_request_id) + expect(current_url).to match(%r{http://www.example.com/sign_up/start\?request_id=.+}) visit_idp_from_sp_with_loa1 - expect(current_url).to eq sign_up_start_url(request_id: sp_request_id) + expect(current_url).to match(%r{http://www.example.com/sign_up/start\?request_id=.+}) end end @@ -472,6 +461,65 @@ end end + context 'canceling sign in with active identities present' do + it 'signs the user out and returns to the home page' do + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + + user = create(:user, :signed_up) + + visit_idp_from_sp_with_loa1 + click_link t('links.sign_in') + fill_in_credentials_and_submit(user.email, user.password) + click_submit_default + visit destroy_user_session_url + + visit_idp_from_sp_with_loa1 + click_link t('links.sign_in') + fill_in_credentials_and_submit(user.email, user.password) + sp_request_id = ServiceProviderRequest.last.uuid + sp = ServiceProvider.from_issuer('urn:gov:gsa:openidconnect:sp:server') + click_link t('links.cancel') + + expect(current_url).to eq sign_up_start_url(request_id: sp_request_id) + expect(page).to have_content t('links.back_to_sp', sp: sp.friendly_name) + end + end + + context 'creating two accounts during the same session' do + it 'allows the second account creation process to complete fully', email: true do + first_email = 'test1@test.com' + second_email = 'test2@test.com' + + perform_in_browser(:one) do + visit_idp_from_sp_with_loa1 + sign_up_user_from_sp_without_confirming_email(first_email) + end + + perform_in_browser(:two) do + confirm_email_in_a_different_browser(first_email) + click_button t('forms.buttons.continue') + redirect_uri = URI(current_url) + + expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') + expect(page.get_rack_session.keys).to_not include('sp') + end + + perform_in_browser(:one) do + visit_idp_from_sp_with_loa1 + sign_up_user_from_sp_without_confirming_email(second_email) + end + + perform_in_browser(:two) do + confirm_email_in_a_different_browser(second_email) + click_button t('forms.buttons.continue') + redirect_uri = URI(current_url) + + expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') + expect(page.get_rack_session.keys).to_not include('sp') + end + end + end + def visit_idp_from_sp_with_loa1(state: SecureRandom.hex) client_id = 'urn:gov:gsa:openidconnect:sp:server' nonce = SecureRandom.hex diff --git a/spec/features/saml/loa1_sso_spec.rb b/spec/features/saml/loa1_sso_spec.rb index 93d17e37cdd..dc974e046c7 100644 --- a/spec/features/saml/loa1_sso_spec.rb +++ b/spec/features/saml/loa1_sso_spec.rb @@ -13,8 +13,6 @@ sign_up_user_from_sp_without_confirming_email(email) end - sp_request_id = ServiceProviderRequest.last.uuid - perform_in_browser(:two) do confirm_email_in_a_different_browser(email) @@ -32,8 +30,6 @@ click_on t('forms.buttons.continue') expect(current_url).to eq authn_request - expect(ServiceProviderRequest.from_uuid(sp_request_id)). - to be_a NullServiceProviderRequest expect(page.get_rack_session.keys).to_not include('sp') end end @@ -43,12 +39,9 @@ saml_authn_request = auth_request.create(saml_settings) visit saml_authn_request - sp_request_id = ServiceProviderRequest.last.uuid sign_in_live_with_2fa(user) expect(current_url).to eq saml_authn_request - expect(ServiceProviderRequest.from_uuid(sp_request_id)). - to be_a NullServiceProviderRequest expect(page.get_rack_session.keys).to_not include('sp') visit root_path @@ -139,16 +132,66 @@ end context 'visiting IdP via SP, then going back to SP and visiting IdP again' do - it 'maintains the request_id in the params' do + it 'displays the branded page' do authn_request = auth_request.create(saml_settings) visit authn_request - sp_request_id = ServiceProviderRequest.last.uuid - expect(current_url).to eq sign_up_start_url(request_id: sp_request_id) + expect(current_url).to match(%r{http://www.example.com/sign_up/start\?request_id=.+}) + + visit authn_request + + expect(current_url).to match(%r{http://www.example.com/sign_up/start\?request_id=.+}) + end + end + + context 'canceling sign in after email and password' do + it 'returns to the branded landing page' do + user = create(:user, :signed_up) + authn_request = auth_request.create(saml_settings) visit authn_request + click_link t('links.sign_in') + fill_in_credentials_and_submit(user.email, user.password) + sp_request_id = ServiceProviderRequest.last.uuid + sp = ServiceProvider.from_issuer('http://localhost:3000') + click_link t('links.cancel') expect(current_url).to eq sign_up_start_url(request_id: sp_request_id) + expect(page).to have_content t('links.back_to_sp', sp: sp.friendly_name) + end + end + + context 'creating two accounts during the same session' do + it 'allows the second account creation process to complete fully', email: true do + first_email = 'test1@test.com' + second_email = 'test2@test.com' + authn_request = auth_request.create(saml_settings) + + perform_in_browser(:one) do + visit authn_request + sign_up_user_from_sp_without_confirming_email(first_email) + end + + perform_in_browser(:two) do + confirm_email_in_a_different_browser(first_email) + click_button t('forms.buttons.continue') + + expect(current_url).to eq authn_request + expect(page.get_rack_session.keys).to_not include('sp') + end + + perform_in_browser(:one) do + visit authn_request + sign_up_user_from_sp_without_confirming_email(second_email) + end + + perform_in_browser(:two) do + confirm_email_in_a_different_browser(second_email) + click_button t('forms.buttons.continue') + + expect(current_url).to eq authn_request + expect(page.get_rack_session.keys).to_not include('sp') + end end end @@ -174,6 +217,6 @@ def stub_personal_key(user:, code:) end def enter_personal_key_words_on_modal(code) - fill_in 'personal-key', with: code + fill_in 'personal_key', with: code end end diff --git a/spec/features/two_factor_authentication/change_factor_spec.rb b/spec/features/two_factor_authentication/change_factor_spec.rb index 76dbb1a4c8f..49880e3891c 100644 --- a/spec/features/two_factor_authentication/change_factor_spec.rb +++ b/spec/features/two_factor_authentication/change_factor_spec.rb @@ -15,6 +15,8 @@ end scenario 'editing phone number' do + allow(Figaro.env).to receive(:otp_delivery_blocklist_maxretry).and_return('4') + mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) allow(UserMailer).to receive(:phone_changed).with(user).and_return(mailer) diff --git a/spec/features/two_factor_authentication/sign_in_spec.rb b/spec/features/two_factor_authentication/sign_in_spec.rb index 80d33721990..b3e116f5427 100644 --- a/spec/features/two_factor_authentication/sign_in_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_spec.rb @@ -115,6 +115,7 @@ def submit_prefilled_otp_code it 'locks the user out and leaves user on the page during entire lockout period' do allow(Figaro.env).to receive(:session_check_frequency).and_return('0') allow(Figaro.env).to receive(:session_check_delay).and_return('0') + lockout_period = Figaro.env.lockout_period_in_minutes.to_i.minutes five_minute_countdown_regex = /4:5\d/ user = create(:user, :signed_up) @@ -132,7 +133,7 @@ def submit_prefilled_otp_code UpdateUser.new( user: user, attributes: { - second_factor_locked_at: Time.zone.now - (Devise.direct_otp_valid_for + 1.second), + second_factor_locked_at: Time.zone.now - (lockout_period + 1.second), } ).call @@ -143,6 +144,158 @@ def submit_prefilled_otp_code end end + context 'user requests an OTP too many times within `findtime` minutes', js: true do + it 'locks the user out and leaves user on the page during entire lockout period' do + allow(Figaro.env).to receive(:session_check_frequency).and_return('0') + allow(Figaro.env).to receive(:session_check_delay).and_return('0') + lockout_period = Figaro.env.lockout_period_in_minutes.to_i.minutes + five_minute_countdown_regex = /4:5\d/ + + user = create(:user, :signed_up) + sign_in_before_2fa(user) + + Figaro.env.otp_delivery_blocklist_maxretry.to_i.times do + click_link t('links.two_factor_authentication.resend_code.sms') + end + + expect(page).to have_content t('titles.account_locked') + expect(page).to have_content(five_minute_countdown_regex) + expect(page).to have_content t('devise.two_factor_authentication.max_otp_requests_reached') + + visit root_path + signin(user.email, user.password) + + expect(page).to have_content t('titles.account_locked') + expect(page).to have_content(five_minute_countdown_regex) + expect(page). + to have_content t('devise.two_factor_authentication.max_generic_login_attempts_reached') + + # let lockout period expire + Timecop.travel(lockout_period) do + signin(user.email, user.password) + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') + end + end + end + + context 'findtime period is greater than lockout period' do + it 'does not lock the user' do + allow(Figaro.env).to receive(:otp_delivery_blocklist_findtime).and_return('10') + lockout_period = Figaro.env.lockout_period_in_minutes.to_i.minutes + user = create(:user, :signed_up) + + sign_in_before_2fa(user) + + Figaro.env.otp_delivery_blocklist_maxretry.to_i.times do + click_link t('links.two_factor_authentication.resend_code.sms') + end + + expect(page).to have_content t('titles.account_locked') + + # let lockout period expire + Timecop.travel(lockout_period) do + signin(user.email, user.password) + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') + end + end + end + + context 'user requests OTP several times but spaced out far apart' do + it 'does not lock the user out' do + max_attempts = Figaro.env.otp_delivery_blocklist_maxretry.to_i + findtime = Figaro.env.otp_delivery_blocklist_findtime.to_i.minutes + user = create(:user, :signed_up) + + sign_in_before_2fa(user) + (max_attempts - 1).times do + click_link t('links.two_factor_authentication.resend_code.sms') + end + click_submit_default + + expect(current_path).to eq account_path + + phone_fingerprint = Pii::Fingerprinter.fingerprint(user.phone) + rate_limited_phone = OtpRequestsTracker.find_by(phone_fingerprint: phone_fingerprint) + + # let findtime period expire + rate_limited_phone.update(otp_last_sent_at: Time.zone.now - (findtime + 1)) + + visit destroy_user_session_url + sign_in_before_2fa(user) + + expect(rate_limited_phone.reload.otp_send_count).to eq 1 + + click_link t('links.two_factor_authentication.resend_code.sms') + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') + + click_submit_default + + expect(current_path).to eq account_path + end + end + + context '2 users with same phone number request OTP too many times within findtime' do + it 'locks both users out' do + allow(Figaro.env).to receive(:otp_delivery_blocklist_maxretry).and_return('3') + first_user = create(:user, :signed_up, phone: '+1 703-555-1212') + second_user = create(:user, :signed_up, phone: '+1 703-555-1212') + max_attempts = Figaro.env.otp_delivery_blocklist_maxretry.to_i + + sign_in_before_2fa(first_user) + click_link t('links.two_factor_authentication.resend_code.sms') + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') + + visit destroy_user_session_url + + sign_in_before_2fa(second_user) + click_link t('links.two_factor_authentication.resend_code.sms') + phone_fingerprint = Pii::Fingerprinter.fingerprint(first_user.phone) + rate_limited_phone = OtpRequestsTracker.find_by(phone_fingerprint: phone_fingerprint) + + expect(current_path).to eq otp_send_path + expect(rate_limited_phone.otp_send_count).to eq max_attempts + + visit account_path + + expect(current_path).to eq root_path + + visit destroy_user_session_url + + signin(first_user.email, first_user.password) + + expect(page).to have_content t('devise.two_factor_authentication.max_otp_requests_reached') + + visit account_path + expect(current_path).to eq root_path + end + end + + context 'When setting up 2FA for the first time' do + it 'enforces rate limiting only for current phone' do + second_user = create(:user, :signed_up, phone: '+1 202-555-1212') + + sign_in_before_2fa + max_attempts = Figaro.env.otp_delivery_blocklist_maxretry.to_i + + submit_2fa_setup_form_with_valid_phone_and_choose_phone_call_delivery + + max_attempts.times do + click_link t('links.two_factor_authentication.resend_code.voice') + end + + expect(page).to have_content t('titles.account_locked') + + visit root_path + signin(second_user.email, second_user.password) + + expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms') + end + end + context 'user signs in while locked out' do it 'signs the user out and lets them know they are locked out' do user = create(:user, :signed_up, second_factor_locked_at: Time.zone.now - 1.minute) diff --git a/spec/features/users/password_recovery_via_recovery_code_spec.rb b/spec/features/users/password_recovery_via_recovery_code_spec.rb index b6f33f6adb6..e3be9e3c66f 100644 --- a/spec/features/users/password_recovery_via_recovery_code_spec.rb +++ b/spec/features/users/password_recovery_via_recovery_code_spec.rb @@ -2,23 +2,21 @@ feature 'Password recovery via personal key' do include PersonalKeyHelper + include IdvHelper let(:user) { create(:user, :signed_up) } let(:new_password) { 'some really awesome new password' } let(:pii) { { ssn: '666-66-1234', dob: '1920-01-01', first_name: 'alice' } } - scenario 'resets password and reactivates profile with personal key', email: true do + scenario 'resets password and reactivates profile with personal key', email: true, js: true do + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + personal_key = personal_key_from_pii(user, pii) trigger_reset_password_and_click_email_link(user.email) reset_password_and_sign_back_in(user, new_password) click_submit_default - enter_correct_otp_code_for_user(user) - - expect(current_path).to eq manage_reactivate_account_path - - click_on t('links.account.reactivate.with_key') expect(current_path).to eq reactivate_account_path @@ -28,14 +26,39 @@ expect(page).to have_content t('headings.account.verified_account') end + scenario 'resets password and reactivates profile with no personal key', email: true, js: true do + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + personal_key_from_pii(user, pii) + trigger_reset_password_and_click_email_link(user.email) + reset_password_and_sign_back_in(user, new_password) + click_submit_default + + expect(current_path).to eq(reactivate_account_path) + + visit account_path + + expect(page).not_to have_content(t('headings.account.verified_account')) + + visit reactivate_account_path + click_on t('links.account.reactivate.without_key') + click_on t('forms.buttons.continue') + click_idv_begin + complete_idv_profile_ok(user, new_password) + acknowledge_and_confirm_personal_key + + expect(current_path).to eq(account_path) + expect(page).to have_content(t('headings.account.verified_account')) + end + scenario 'resets password, makes personal key, attempts reactivate profile', email: true do + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + _personal_key = personal_key_from_pii(user, pii) trigger_reset_password_and_click_email_link(user.email) reset_password_and_sign_back_in(user, new_password) click_submit_default - enter_correct_otp_code_for_user(user) visit manage_personal_key_path @@ -44,7 +67,9 @@ expect(current_path).to eq reactivate_account_path - reactivate_profile(new_password, new_personal_key) + click_on t('links.account.reactivate.with_key') + fill_in 'personal_key', with: new_personal_key + click_on t('forms.buttons.continue') expect(page).to have_content t('errors.messages.personal_key_incorrect') end @@ -76,6 +101,30 @@ expect(page).to have_content t('idv.messages.personal_key') end + context 'account recovery alternative paths' do + before do + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + personal_key_from_pii(user, pii) + trigger_reset_password_and_click_email_link(user.email) + reset_password_and_sign_back_in(user, new_password) + click_submit_default + end + + scenario 'resets password, chooses to reverify on personal key entry page', email: true do + click_on t('links.account.reactivate.with_key') + click_on t('links.reverify') + + expect(current_path).to eq(verify_path) + end + + scenario 'resets password, view modal and close it', email: true, js: true do + click_on t('links.account.reactivate.without_key') + click_on t('links.cancel') + + expect(page).not_to have_content('[id="reactivate-account-modal"]') + end + end + def scrape_personal_key new_personal_key_words = [] page.all(:css, '[data-personal-key]').each do |node| @@ -85,8 +134,16 @@ def scrape_personal_key end def reactivate_profile(password, personal_key) + click_on t('links.account.reactivate.with_key') + + expect(current_path).to eq verify_personal_key_path + + fill_in 'personal_key', with: personal_key + click_on t('forms.buttons.continue') + + expect(current_path).to eq verify_password_path + fill_in 'Password', with: password - enter_personal_key(personal_key: personal_key) - click_button t('forms.reactivate_profile.submit') + click_on t('forms.buttons.submit.default') end end diff --git a/spec/features/users/regenerate_personal_key_spec.rb b/spec/features/users/regenerate_personal_key_spec.rb index 26927907574..dcd88ffc7e1 100644 --- a/spec/features/users/regenerate_personal_key_spec.rb +++ b/spec/features/users/regenerate_personal_key_spec.rb @@ -119,7 +119,7 @@ def expect_accordion_content_to_become_visible def expect_confirmation_modal_to_appear_with_first_code_field_in_focus expect(page).not_to have_xpath("//div[@id='personal-key-confirm'][@class='display-none']") expect(page).not_to have_xpath("//#{invisible_selector}[@id='personal-key']") - expect(page.evaluate_script('document.activeElement.name')).to eq 'personal-key' + expect(page.evaluate_script('document.activeElement.name')).to eq 'personal_key' end def press_shift_tab diff --git a/spec/features/visitors/i18n_spec.rb b/spec/features/visitors/i18n_spec.rb new file mode 100644 index 00000000000..e86cf30231c --- /dev/null +++ b/spec/features/visitors/i18n_spec.rb @@ -0,0 +1,26 @@ +require 'rails_helper' + +feature 'Internationalization' do + context 'visit homepage' do + before do + page.driver.header 'Accept-Language', locale + visit root_path + end + + context 'when the user has set their locale to :en' do + let(:locale) { :en } + + it 'displays a translated header to the user' do + expect(page).to have_content t('headings.sign_in_without_sp', locale: 'en') + end + end + + context 'when the user has set their locale to :es' do + let(:locale) { :es } + + it 'displays a translated header to the user' do + expect(page).to have_content t('headings.sign_in_without_sp', locale: 'es') + end + end + end +end diff --git a/spec/forms/openid_connect_authorize_form_spec.rb b/spec/forms/openid_connect_authorize_form_spec.rb index 187160c5d9a..5d1d0d49aa3 100644 --- a/spec/forms/openid_connect_authorize_form_spec.rb +++ b/spec/forms/openid_connect_authorize_form_spec.rb @@ -34,64 +34,22 @@ let(:code_challenge_method) { nil } describe '#submit' do - let(:user) { create(:user) } - let(:rails_session_id) { SecureRandom.hex } - subject(:result) { form.submit(user, rails_session_id) } + subject(:result) { form.submit } context 'with valid params' do it 'is successful' do allow(FormResponse).to receive(:new) - form.submit(user, rails_session_id) - - identity = user.identities.where(service_provider: client_id).first + form.submit extra_attributes = { client_id: client_id, - redirect_uri: "#{redirect_uri}?code=#{identity.session_uuid}&state=#{state}", + redirect_uri: nil, } expect(FormResponse).to have_received(:new). with(success: true, errors: {}, extra: extra_attributes) end - - it 'links an identity for this client with the code as the session_uuid' do - redirect_uri = URI(result.to_h[:redirect_uri]) - code = URIService.params(redirect_uri)[:code] - - identity = user.identities.where(service_provider: client_id).first - expect(identity.session_uuid).to eq(code) - expect(identity.nonce).to eq(nonce) - end - - it 'sets the max ial/loa from the request on the identity' do - result - - identity = user.identities.where(service_provider: client_id).first - expect(identity.ial).to eq(3) - end - - context 'with PKCE' do - let(:code_challenge) { 'abcdef' } - let(:code_challenge_method) { 'S256' } - - it 'records the code_challenge on the identity' do - allow(FormResponse).to receive(:new) - - form.submit(user, rails_session_id) - - identity = user.identities.where(service_provider: client_id).first - - extra_attributes = { - client_id: client_id, - redirect_uri: "#{redirect_uri}?code=#{identity.session_uuid}&state=#{state}", - } - - expect(identity.code_challenge).to eq(code_challenge) - expect(FormResponse).to have_received(:new). - with(success: true, errors: {}, extra: extra_attributes) - end - end end context 'with invalid params' do @@ -291,4 +249,45 @@ expect(form.client_id).to eq 'foobar' end end + + describe '#link_identity_to_service_provider' do + let(:user) { create(:user) } + let(:rails_session_id) { SecureRandom.hex } + + context 'with PKCE' do + let(:code_challenge) { 'abcdef' } + let(:code_challenge_method) { 'S256' } + + it 'records the code_challenge on the identity' do + form.link_identity_to_service_provider(user, rails_session_id) + + identity = user.identities.where(service_provider: client_id).first + + expect(identity.code_challenge).to eq(code_challenge) + expect(identity.nonce).to eq(nonce) + expect(identity.ial).to eq(3) + end + end + end + + describe '#success_redirect_uri' do + let(:user) { create(:user) } + let(:rails_session_id) { SecureRandom.hex } + + context 'when the identity has been linked' do + it 'returns a redirect URI with the code from the identity session_uuid' do + form.link_identity_to_service_provider(user, rails_session_id) + identity = user.identities.where(service_provider: client_id).first + + expect(form.success_redirect_uri). + to eq "#{redirect_uri}?code=#{identity.session_uuid}&state=#{state}" + end + end + + context 'when the identity has not been linked' do + it 'returns nil' do + expect(form.success_redirect_uri).to be_nil + end + end + end end diff --git a/spec/forms/reactivate_account_form_spec.rb b/spec/forms/reactivate_account_form_spec.rb deleted file mode 100644 index 3fd7ba23d57..00000000000 --- a/spec/forms/reactivate_account_form_spec.rb +++ /dev/null @@ -1,99 +0,0 @@ -require 'rails_helper' - -describe ReactivateAccountForm do - subject(:form) do - ReactivateAccountForm.new(user, - password: password) - end - - let(:user) { build_stubbed(:user) } - let(:personal_key) { nil } - let(:password) { nil } - - describe '#valid?' do - let(:password) { 'asd' } - let(:personal_key) { 'foo' } - let(:valid_personal_key?) { true } - let(:valid_password?) { true } - let(:personal_key_decrypts?) { true } - - before do - allow(form).to receive(:valid_password?).and_return(valid_password?) - allow(form).to receive(:valid_personal_key?).and_return(valid_personal_key?) - allow(form).to receive(:personal_key_decrypts?).and_return(personal_key_decrypts?) - end - - context 'when required attributes are not present' do - let(:password) { nil } - let(:personal_key) { nil } - - it 'is invalid' do - expect(subject).to_not be_valid - expect(subject.errors[:personal_key]).to include t('errors.messages.blank') - expect(subject.errors[:password]).to eq [t('errors.messages.blank')] - end - end - - context 'when there is no profile that has had its password reset' do - let(:password_reset_profile) { nil } - - it 'is invalid' do - expect(subject).to_not be_valid - expect(subject.errors[:base]).to eq [t('errors.messages.no_password_reset_profile')] - end - end - - context 'when personal key does not match' do - subject(:form) do - ReactivateAccountForm.new(user, - personal_key: personal_key, - password: password) - end - - let(:valid_personal_key?) { false } - - it 'is invalid' do - expect(subject).to_not be_valid - expect(subject.errors[:personal_key]).to eq [t('errors.messages.personal_key_incorrect')] - end - end - - context 'when password does not match' do - let(:valid_password?) { false } - - it 'is invalid' do - expect(subject).to_not be_valid - expect(subject.errors[:password]).to eq [t('errors.messages.password_incorrect')] - end - end - end - - describe '#submit' do - before { expect(form).to receive(:valid?).and_return(valid?) } - - let(:flash) { {} } - - context 'with a valid form' do - let(:valid?) { true } - - it 're-encrypts the PII and sets the personal key in the flash' do - personal_key = '555' - expect(form).to receive(:reencrypt_pii).and_return(personal_key) - - form.submit(flash) - - expect(flash[:personal_key]).to eq(personal_key) - end - end - - context 'with an invalid form' do - let(:valid?) { false } - - it 'clears the password' do - form.submit(flash) - - expect(form.password).to be_nil - end - end - end -end diff --git a/spec/forms/verify_account_form_spec.rb b/spec/forms/verify_account_form_spec.rb index 87c17b8b5fa..422bff0bef1 100644 --- a/spec/forms/verify_account_form_spec.rb +++ b/spec/forms/verify_account_form_spec.rb @@ -2,23 +2,18 @@ describe VerifyAccountForm do subject(:form) do - VerifyAccountForm.new(user: user, otp: otp, pii_attributes: pii_attributes) + VerifyAccountForm.new(user: user, otp: entered_otp, pii_attributes: pii_attributes) end let(:user) { pending_profile.user } + let(:entered_otp) { otp } let(:otp) { 'abc123' } let(:pii_attributes) { Pii::Attributes.new_from_hash(otp: otp) } let(:pending_profile) { create(:profile, deactivation_reason: :verification_pending) } describe '#valid?' do - let(:valid_otp?) { true } - - before do - allow(form).to receive(:valid_otp?).and_return(valid_otp?) - end - context 'when required attributes are not present' do - let(:otp) { nil } + let(:entered_otp) { nil } it 'is invalid' do expect(subject).to_not be_valid @@ -36,8 +31,28 @@ end end + context 'OTP crockford normalizing' do + context 'when the entered OTP has lowercase' do + let(:entered_otp) { 'abcdef12345' } + let(:otp) { 'ABCDEF12345' } + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when the entered OTP has ohs instead of zeroes' do + let(:entered_otp) { 'oOoOoOoOoO' } + let(:otp) { '0000000000' } + + it 'is valid' do + expect(subject).to be_valid + end + end + end + context 'when OTP does not match' do - let(:valid_otp?) { false } + let(:entered_otp) { 'wrong' } it 'is invalid' do expect(subject).to_not be_valid @@ -47,12 +62,6 @@ end describe '#submit' do - let(:valid_otp?) { true } - - before do - allow(form).to receive(:valid_otp?).and_return(valid_otp?) - end - context 'correct OTP' do it 'returns true' do expect(subject.submit).to eq true @@ -68,7 +77,7 @@ end context 'incorrect OTP' do - let(:valid_otp?) { false } + let(:entered_otp) { 'wrong' } it 'clears form' do subject.submit diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 24a50f0c01c..1ef54850cd0 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -38,6 +38,10 @@ Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new end + config.before(:each) do + I18n.locale = :en + end + config.before(:each) do allow(ValidateEmail).to receive(:mx_valid?).and_return(true) Rack::Attack.cache.store.clear diff --git a/spec/requests/page_not_found_spec.rb b/spec/requests/page_not_found_spec.rb new file mode 100644 index 00000000000..9e3a62f9586 --- /dev/null +++ b/spec/requests/page_not_found_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +RSpec.describe 'Missing pages and assets', type: :request do + describe 'missing page' do + it 'responds with 404' do + get '/nonexistent-page' + + expect(response.status).to eq 404 + end + end + + describe 'missing PNG' do + it 'responds with 404' do + get '/mobile-icon.png' + + expect(response.status).to eq 404 + end + end + + describe 'missing CSS' do + it 'responds with 404' do + get '/application-random-hash.css' + + expect(response.status).to eq 404 + end + end + + describe 'missing JS' do + it 'responds with 404' do + get '/application-random-hash.js' + + expect(response.status).to eq 404 + end + end +end diff --git a/spec/services/file_encryptor_spec.rb b/spec/services/file_encryptor_spec.rb index 83956fee579..aeba9fd5621 100644 --- a/spec/services/file_encryptor_spec.rb +++ b/spec/services/file_encryptor_spec.rb @@ -5,7 +5,7 @@ subject(:file_encryptor) do FileEncryptor.new( - Rails.root.join('keys/equifax_gpg.pub.bin'), + Rails.root.join('keys', 'equifax_gpg.pub.bin'), email ) end diff --git a/spec/services/idv/financials_step_spec.rb b/spec/services/idv/financials_step_spec.rb index 0e1f57828a3..4e2407e82cb 100644 --- a/spec/services/idv/financials_step_spec.rb +++ b/spec/services/idv/financials_step_spec.rb @@ -20,7 +20,6 @@ def build_step(params) describe '#submit' do it 'returns FormResponse with success: false for invalid params' do step = build_step(finance_type: :ccn, ccn: '1234') - extra = { vendor: { reasons: nil } } errors = { ccn: [t('idv.errors.invalid_ccn')] } response = instance_double(FormResponse) @@ -29,13 +28,12 @@ def build_step(params) expect(submission).to eq response expect(FormResponse).to have_received(:new). - with(success: false, errors: errors, extra: extra) + with(success: false, errors: errors) expect(idv_session.financials_confirmation).to eq false end it 'returns FormResponse with success: true for mock-happy CCN' do step = build_step(finance_type: :ccn, ccn: '12345678') - extra = { vendor: { reasons: ['Good number'] } } response = instance_double(FormResponse) allow(FormResponse).to receive(:new).and_return(response) @@ -44,7 +42,7 @@ def build_step(params) expect(submission).to eq response expect(FormResponse).to have_received(:new). - with(success: true, errors: {}, extra: extra) + with(success: true, errors: {}) expect(idv_session.financials_confirmation).to eq true expect(idv_session.params).to eq idv_finance_form.idv_params end @@ -52,7 +50,6 @@ def build_step(params) it 'returns FormResponse with success: false for mock-sad CCN' do step = build_step(finance_type: :ccn, ccn: '00000000') - extra = { vendor: { reasons: ['Bad number'] } } errors = { ccn: ['The ccn could not be verified.'] } response = instance_double(FormResponse) @@ -61,7 +58,7 @@ def build_step(params) expect(submission).to eq response expect(FormResponse).to have_received(:new). - with(success: false, errors: errors, extra: extra) + with(success: false, errors: errors) expect(idv_session.financials_confirmation).to eq false end end diff --git a/spec/services/idv/phone_step_spec.rb b/spec/services/idv/phone_step_spec.rb index ad216931aaf..3a609f56829 100644 --- a/spec/services/idv/phone_step_spec.rb +++ b/spec/services/idv/phone_step_spec.rb @@ -25,12 +25,11 @@ def build_step(params) step = build_step(phone: '555') errors = { phone: [invalid_phone_message] } - extra = { vendor: { reasons: nil } } result = instance_double(FormResponse) expect(FormResponse).to receive(:new). - with(success: false, errors: errors, extra: extra).and_return(result) + with(success: false, errors: errors).and_return(result) expect(step.submit).to eq result expect(idv_session.phone_confirmation).to eq false end @@ -39,9 +38,8 @@ def build_step(params) step = build_step(phone: '555-555-0000') result = instance_double(FormResponse) - extra = { vendor: { reasons: ['Good number'] } } - expect(FormResponse).to receive(:new).with(success: true, errors: {}, extra: extra). + expect(FormResponse).to receive(:new).with(success: true, errors: {}). and_return(result) expect(step.submit).to eq result expect(idv_session.phone_confirmation).to eq true @@ -52,12 +50,11 @@ def build_step(params) step = build_step(phone: '555-555-5555') errors = { phone: ['The phone number could not be verified.'] } - extra = { vendor: { reasons: ['Bad number'] } } result = instance_double(FormResponse) expect(FormResponse).to receive(:new). - with(success: false, errors: errors, extra: extra).and_return(result) + with(success: false, errors: errors).and_return(result) expect(step.submit).to eq result expect(idv_session.phone_confirmation).to eq false end diff --git a/spec/services/idv/profile_maker_spec.rb b/spec/services/idv/profile_maker_spec.rb index aa3b8e8bc50..32961b4a65a 100644 --- a/spec/services/idv/profile_maker_spec.rb +++ b/spec/services/idv/profile_maker_spec.rb @@ -27,6 +27,10 @@ expect(pii).to be_a Pii::Attributes expect(pii.first_name.raw).to eq 'Some' expect(pii.first_name.norm).to eq 'Somebody' + + otp = pii.otp.raw + expect(otp.length).to eq(10) + expect(otp).to eq(Base32::Crockford.normalize(otp)) end end end diff --git a/spec/services/idv/profile_step_spec.rb b/spec/services/idv/profile_step_spec.rb index ffc71ea173d..e51f6f48c25 100644 --- a/spec/services/idv/profile_step_spec.rb +++ b/spec/services/idv/profile_step_spec.rb @@ -142,7 +142,7 @@ def build_step(params) step.submit expect(idv_session.params).to eq user_attrs - expect(idv_session.applicant).to eq user_attrs.merge(uuid: user.uuid) + expect(idv_session.applicant).to eq user_attrs.merge('uuid' => user.uuid) end end diff --git a/spec/services/marketing_site_spec.rb b/spec/services/marketing_site_spec.rb index 10cd420c379..bd72aca8384 100644 --- a/spec/services/marketing_site_spec.rb +++ b/spec/services/marketing_site_spec.rb @@ -28,7 +28,7 @@ describe '.help_authenticator_app_url' do it 'points to the authenticator app section of the help page' do expect(MarketingSite.help_authenticator_app_url).to eq( - 'https://www.login.gov/help#what-is-an-authenticator-app' + 'https://www.login.gov/help/signing-in/what-is-an-authenticator-app/' ) end end diff --git a/spec/services/otp_rate_limiter_spec.rb b/spec/services/otp_rate_limiter_spec.rb new file mode 100644 index 00000000000..bdf838dcac8 --- /dev/null +++ b/spec/services/otp_rate_limiter_spec.rb @@ -0,0 +1,56 @@ +require 'rails_helper' + +RSpec.describe OtpRateLimiter do + let(:current_user) { build(:user, :with_phone) } + subject(:otp_rate_limiter) { OtpRateLimiter.new(phone: current_user.phone, user: current_user) } + let(:phone_fingerprint) { Pii::Fingerprinter.fingerprint(current_user.phone) } + let(:rate_limited_phone) { OtpRequestsTracker.find_by(phone_fingerprint: phone_fingerprint) } + + describe '#exceeded_otp_send_limit?' do + it 'is false by default' do + expect(otp_rate_limiter.exceeded_otp_send_limit?).to eq(false) + end + + it 'is true after maxretry_times attemps in findtime minutes' do + expect(otp_rate_limiter.exceeded_otp_send_limit?).to eq(false) + + Figaro.env.otp_delivery_blocklist_maxretry.to_i.times do + otp_rate_limiter.increment + end + + expect(otp_rate_limiter.exceeded_otp_send_limit?).to eq(true) + end + end + + describe '#increment' do + it 'sets the otp_last_sent_at' do + now = Time.zone.now + otp_rate_limiter.increment + + expect(rate_limited_phone.otp_last_sent_at.to_i).to eq(now.to_i) + end + + it 'increments the otp_send_count' do + otp_rate_limiter.increment + + expect { otp_rate_limiter.increment }. + to change { rate_limited_phone.reload.otp_send_count }.from(1).to(2) + end + end + + describe '#lock_out_user' do + before do + otp_rate_limiter.increment + rate_limited_phone.otp_last_sent_at = 5.minutes.ago + rate_limited_phone.otp_send_count = 0 + end + + it 'sets the second_factor_locked_at' do + expect(current_user.second_factor_locked_at).to be_nil + + otp_rate_limiter.lock_out_user + + expect(current_user.second_factor_locked_at.to_i).to eq(Time.zone.now.to_i) + end + end +end diff --git a/spec/services/reactivate_account_session_spec.rb b/spec/services/reactivate_account_session_spec.rb new file mode 100644 index 00000000000..af72c9161ab --- /dev/null +++ b/spec/services/reactivate_account_session_spec.rb @@ -0,0 +1,94 @@ +require 'rails_helper' + +describe ReactivateAccountSession do + let(:user) { build(:user) } + let(:user_session) { {} } + + before do + @reactivate_account_session = ReactivateAccountSession.new( + user: user, + user_session: user_session + ) + end + + describe '#clear' do + it 'deletes the reactivate account session object from user_session' do + expect(user_session).to have_key(:reactivate_account) + + @reactivate_account_session.clear + + expect(user_session).to be_empty + end + end + + describe '#start' do + it 'sets the session object `active` flag to true' do + @reactivate_account_session.start + expect(user_session[:reactivate_account][:active]).to be(true) + end + end + + describe '#started?' do + it 'initializes set to false' do + expect(@reactivate_account_session.started?).to be(false) + end + + it 'returns a boolean if the account reactivate flow has started or not' do + @reactivate_account_session.start + expect(@reactivate_account_session.started?).to be(true) + end + end + + describe '#suspend' do + it 'sets the reactivate account object back to its defaults' do + pii = {} + + @reactivate_account_session.start + @reactivate_account_session.store_decrypted_pii(pii) + + expect(@reactivate_account_session.started?).to be(true) + expect(@reactivate_account_session.personal_key?).to be(true) + expect(@reactivate_account_session.decrypted_pii).to be(pii) + + @reactivate_account_session.suspend + + expect(@reactivate_account_session.started?).to be(false) + expect(@reactivate_account_session.personal_key?).to be(false) + expect(@reactivate_account_session.decrypted_pii).to eq(nil) + end + end + + describe '#store_decrypted_pii' do + it 'stores the supplied object in the session and toggles `personal_key` flag' do + pii = {} + @reactivate_account_session.store_decrypted_pii(pii) + account_reactivation_obj = user_session[:reactivate_account] + expect(account_reactivation_obj[:personal_key]).to be(true) + expect(account_reactivation_obj[:pii]).to eq(pii) + end + end + + describe '#personal_key?' do + it 'defaults to false' do + expect(@reactivate_account_session.personal_key?).to be(false) + end + + it 'returns a boolean indicating if the user hsa validated their personal key' do + @reactivate_account_session.store_decrypted_pii({}) + expect(@reactivate_account_session.personal_key?).to be(true) + end + end + + describe '#decrypted_pii' do + it 'returns nil as a default' do + expect(@reactivate_account_session.decrypted_pii).to eq(nil) + end + + it 'returns the pii stored in the session' do + pii = {} + @reactivate_account_session.store_decrypted_pii(pii) + + expect(@reactivate_account_session.decrypted_pii).to eq(pii) + end + end +end diff --git a/spec/services/session_encryptor_spec.rb b/spec/services/session_encryptor_spec.rb index 9b22b772a45..2e8b7026718 100644 --- a/spec/services/session_encryptor_spec.rb +++ b/spec/services/session_encryptor_spec.rb @@ -7,25 +7,6 @@ expect(SessionEncryptor.load(session)).to eq('foo' => 'bar') end - - # rubocop:disable Security/MarshalLoad - context 'value previously serialized with Marshal and Base64 encoded' do - it 'decrypts the session and then calls .dump' do - plain = ::Base64.encode64(Marshal.dump(foo: 'bar')) - user_access_key = SessionEncryptor.user_access_key - session = SessionEncryptor.encryptor.encrypt(plain, user_access_key) - decrypted = SessionEncryptor.encryptor.decrypt(session, user_access_key) - decoded_session = Marshal.load(::Base64.decode64(decrypted)) - - allow(Rails.logger).to receive(:info) - allow(SessionEncryptor).to receive(:dump) - - expect(SessionEncryptor.load(session)).to eq(foo: 'bar') - expect(Rails.logger).to have_received(:info).with('Marshalled session found') - expect(SessionEncryptor).to have_received(:dump).with(decoded_session) - end - end - # rubocop:enable Security/MarshalLoad end describe '#dump' do diff --git a/spec/services/usps_exporter_spec.rb b/spec/services/usps_exporter_spec.rb index ccb2af3630b..2e4eba2144c 100644 --- a/spec/services/usps_exporter_spec.rb +++ b/spec/services/usps_exporter_spec.rb @@ -39,7 +39,7 @@ end let(:file_encryptor) do FileEncryptor.new( - Rails.root.join('keys/equifax_gpg.pub.bin'), + Rails.root.join('keys', 'equifax_gpg.pub.bin'), Figaro.env.equifax_gpg_email ) end diff --git a/spec/support/features/idv_helper.rb b/spec/support/features/idv_helper.rb index a7a82cbe21c..a228756ac77 100644 --- a/spec/support/features/idv_helper.rb +++ b/spec/support/features/idv_helper.rb @@ -85,7 +85,7 @@ def click_idv_cancel click_on t('idv.buttons.cancel') end - def complete_idv_profile_ok(user) + def complete_idv_profile_ok(user, password = user_password) fill_out_idv_form_ok click_idv_continue fill_out_financial_form_ok @@ -93,7 +93,7 @@ def complete_idv_profile_ok(user) click_idv_address_choose_phone fill_out_phone_form_ok(user.phone) click_idv_continue - fill_in 'Password', with: user_password + fill_in 'Password', with: password click_submit_default end end diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 4174dbce389..f5831cf230b 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -162,7 +162,7 @@ def acknowledge_and_confirm_personal_key click_on button_text, class: 'personal-key-continue' - fill_in 'personal-key', with: code_words.join('-').downcase + extra_characters_get_ignored + fill_in 'personal_key', with: code_words.join('-').downcase + extra_characters_get_ignored click_on button_text, class: 'personal-key-confirm' end @@ -243,7 +243,7 @@ def sign_up_user_from_sp_without_confirming_email(email) expect(current_url).to eq sign_up_email_url(request_id: sp_request_id) expect(page).to have_css('img[src*=sp-logos]') - submit_form_with_valid_email + submit_form_with_valid_email(email) expect(current_url).to eq sign_up_verify_email_url(request_id: sp_request_id) expect(last_email.html_part.body).to have_content "?_request_id=#{sp_request_id}" diff --git a/spec/support/shared_examples_for_otp_forms.rb b/spec/support/shared_examples_for_otp_forms.rb index 7e8723de173..38a798dbe38 100644 --- a/spec/support/shared_examples_for_otp_forms.rb +++ b/spec/support/shared_examples_for_otp_forms.rb @@ -8,7 +8,7 @@ describe 'tertiary form actions' do it 'allows the user to cancel out of the sign in process' do render - expect(rendered).to have_link(t('links.cancel'), href: destroy_user_session_path) + expect(rendered).to have_link(t('links.cancel'), href: sign_out_path) end end end diff --git a/spec/views/accounts/show.html.slim_spec.rb b/spec/views/accounts/show.html.slim_spec.rb index 4d6c9e1c826..73851ae246c 100644 --- a/spec/views/accounts/show.html.slim_spec.rb +++ b/spec/views/accounts/show.html.slim_spec.rb @@ -93,7 +93,7 @@ render expect(rendered).to have_link(t('account.index.reactivation.link'), - href: manage_reactivate_account_path) + href: reactivate_account_path) end end diff --git a/spec/views/devise/passwords/new.html.slim_spec.rb b/spec/views/devise/passwords/new.html.slim_spec.rb index 2cf85198c02..5cee8700ed1 100644 --- a/spec/views/devise/passwords/new.html.slim_spec.rb +++ b/spec/views/devise/passwords/new.html.slim_spec.rb @@ -5,8 +5,20 @@ before do @password_reset_email_form = PasswordResetEmailForm.new('') - + sp = build_stubbed( + :service_provider, + friendly_name: 'Awesome Application!', + return_to_sp_url: 'www.awesomeness.com' + ) + view_context = ActionController::Base.new.view_context + @decorated_session = DecoratedSession.new( + sp: sp, + view_context: view_context, + sp_session: {}, + service_provider_request: ServiceProviderRequest.new + ).call allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:decorated_session).and_return(@decorated_session) end it 'has a localized title' do diff --git a/spec/views/devise/sessions/new.html.slim_spec.rb b/spec/views/devise/sessions/new.html.slim_spec.rb index a5f5855b860..4fce885fbcd 100644 --- a/spec/views/devise/sessions/new.html.slim_spec.rb +++ b/spec/views/devise/sessions/new.html.slim_spec.rb @@ -54,7 +54,10 @@ ) view_context = ActionController::Base.new.view_context @decorated_session = DecoratedSession.new( - sp: sp, view_context: view_context, sp_session: {} + sp: sp, + view_context: view_context, + sp_session: {}, + service_provider_request: ServiceProviderRequest.new ).call allow(view).to receive(:decorated_session).and_return(@decorated_session) end diff --git a/spec/views/layouts/application.html.slim_spec.rb b/spec/views/layouts/application.html.slim_spec.rb index d52f33d2766..ed2129717fa 100644 --- a/spec/views/layouts/application.html.slim_spec.rb +++ b/spec/views/layouts/application.html.slim_spec.rb @@ -6,7 +6,12 @@ before do allow(view).to receive(:user_fully_authenticated?).and_return(true) allow(view).to receive(:decorated_session).and_return( - DecoratedSession.new(sp: nil, view_context: nil, sp_session: {}).call + DecoratedSession.new( + sp: nil, + view_context: nil, + sp_session: {}, + service_provider_request: ServiceProviderRequest.new + ).call ) allow(view.request).to receive(:original_url).and_return('http://test.host/foobar') allow(view).to receive(:current_user).and_return(User.new) @@ -76,7 +81,12 @@ allow(view).to receive(:current_user).and_return(nil) allow(view).to receive(:user_fully_authenticated?).and_return(false) allow(view).to receive(:decorated_session).and_return( - DecoratedSession.new(sp: nil, view_context: nil, sp_session: {}).call + DecoratedSession.new( + sp: nil, + view_context: nil, + sp_session: {}, + service_provider_request: nil + ).call ) allow(Figaro.env).to receive(:participate_in_dap).and_return('true') diff --git a/spec/views/reactivate_account/index.html.slim_spec.rb b/spec/views/reactivate_account/index.html.slim_spec.rb new file mode 100644 index 00000000000..ab66e8e8d04 --- /dev/null +++ b/spec/views/reactivate_account/index.html.slim_spec.rb @@ -0,0 +1,9 @@ +require 'rails_helper' + +describe 'reactivate_account/index.html.slim' do + it 'displays a fallback warning alert when js is off' do + render + + expect(rendered).to have_content(t('instructions.account.reactivate.modal.copy')) + end +end diff --git a/spec/views/shared/_nav_branded.html.slim_spec.rb b/spec/views/shared/_nav_branded.html.slim_spec.rb index 99a28197491..4a296cb6605 100644 --- a/spec/views/shared/_nav_branded.html.slim_spec.rb +++ b/spec/views/shared/_nav_branded.html.slim_spec.rb @@ -9,7 +9,10 @@ :service_provider, logo: 'generic.svg', friendly_name: 'Best SP ever' ) decorated_session = ServiceProviderSessionDecorator.new( - sp: sp_with_logo, view_context: view_context, sp_session: {} + sp: sp_with_logo, + view_context: view_context, + sp_session: {}, + service_provider_request: nil ) allow(view).to receive(:decorated_session).and_return(decorated_session) render @@ -24,7 +27,10 @@ before do sp_without_logo = build_stubbed(:service_provider, friendly_name: 'No logo no problem') decorated_session = ServiceProviderSessionDecorator.new( - sp: sp_without_logo, view_context: view_context, sp_session: {} + sp: sp_without_logo, + view_context: view_context, + sp_session: {}, + service_provider_request: nil ) allow(view).to receive(:decorated_session).and_return(decorated_session) render diff --git a/spec/views/sign_up/registrations/new.html.slim_spec.rb b/spec/views/sign_up/registrations/new.html.slim_spec.rb index 5d13ea5fbe1..54828b53f32 100644 --- a/spec/views/sign_up/registrations/new.html.slim_spec.rb +++ b/spec/views/sign_up/registrations/new.html.slim_spec.rb @@ -9,7 +9,7 @@ view_context = ActionController::Base.new.view_context @decorated_session = DecoratedSession.new( - sp: nil, view_context: view_context, sp_session: {} + sp: nil, view_context: view_context, sp_session: {}, service_provider_request: nil ).call allow(view).to receive(:decorated_session).and_return(@decorated_session) end diff --git a/spec/views/sign_up/registrations/show.html.slim_spec.rb b/spec/views/sign_up/registrations/show.html.slim_spec.rb index a64fdea7183..ec5c3bd8eaf 100644 --- a/spec/views/sign_up/registrations/show.html.slim_spec.rb +++ b/spec/views/sign_up/registrations/show.html.slim_spec.rb @@ -41,7 +41,7 @@ ) view_context = ActionController::Base.new.view_context @decorated_session = DecoratedSession.new( - sp: @sp, view_context: view_context, sp_session: {} + sp: @sp, view_context: view_context, sp_session: {}, service_provider_request: nil ).call allow(view).to receive(:decorated_session).and_return(@decorated_session) end diff --git a/spec/views/verify/fail.html.slim_spec.rb b/spec/views/verify/fail.html.slim_spec.rb index a2b80771bc3..404a7ea0d0d 100644 --- a/spec/views/verify/fail.html.slim_spec.rb +++ b/spec/views/verify/fail.html.slim_spec.rb @@ -7,7 +7,7 @@ before do sp = build_stubbed(:service_provider, friendly_name: 'Awesome Application!') @decorated_session = ServiceProviderSessionDecorator.new( - sp: sp, view_context: view_context, sp_session: {} + sp: sp, view_context: view_context, sp_session: {}, service_provider_request: nil ) allow(view).to receive(:decorated_session).and_return(@decorated_session) end