diff --git a/.rubocop.yml b/.rubocop.yml index 11d3b732a5e..e962e136b39 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -36,6 +36,7 @@ Metrics/BlockLength: Exclude: - 'Rakefile' - '**/*.rake' + - 'app/decorators/identity_decorator.rb' - 'app/decorators/user_decorator.rb' - 'app/services/omniauth_authorizer.rb' - 'app/services/single_logout_handler.rb' diff --git a/Gemfile b/Gemfile index b113cf1ea60..a5bace60a8e 100644 --- a/Gemfile +++ b/Gemfile @@ -25,6 +25,7 @@ gem 'http_accept_language' gem 'httparty' gem 'identity-hostdata', github: '18F/identity-hostdata', branch: 'master' gem 'json-jwt' +gem 'local_time' gem 'lograge' gem 'net-sftp' gem 'newrelic_rpm' diff --git a/Gemfile.lock b/Gemfile.lock index 34a3ec3d899..92e177232b4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -117,20 +117,20 @@ GEM arel (8.0.0) ast (2.4.0) aws-eventstream (1.0.1) - aws-partitions (1.103.0) - aws-sdk-core (3.27.0) + aws-partitions (1.111.0) + aws-sdk-core (3.38.0) aws-eventstream (~> 1.0) aws-partitions (~> 1.0) aws-sigv4 (~> 1.0) jmespath (~> 1.0) - aws-sdk-kms (1.9.0) + aws-sdk-kms (1.11.0) aws-sdk-core (~> 3, >= 3.26.0) aws-sigv4 (~> 1.0) aws-sdk-s3 (1.17.0) aws-sdk-core (~> 3, >= 3.21.2) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.0) - aws-sdk-ses (1.10.0) + aws-sdk-ses (1.13.0) aws-sdk-core (~> 3, >= 3.26.0) aws-sigv4 (~> 1.0) aws-sigv4 (1.0.3) @@ -154,21 +154,22 @@ GEM brakeman (4.3.1) browser (2.5.3) builder (3.2.3) - bullet (5.7.6) + bullet (5.9.0) activesupport (>= 3.0.0) - uniform_notifier (~> 1.11.0) + uniform_notifier (~> 1.11) bummr (0.3.2) rainbow thor byebug (10.0.2) - capybara (3.7.2) + capybara (3.10.1) addressable mini_mime (>= 0.1.3) nokogiri (~> 1.8) rack (>= 1.6.0) rack-test (>= 0.6.3) - xpath (~> 3.1) - capybara-screenshot (1.0.21) + regexp_parser (~> 1.2) + xpath (~> 3.2) + capybara-screenshot (1.0.22) capybara (>= 1.0, < 4) launchy capybara-selenium (0.0.6) @@ -178,19 +179,19 @@ GEM childprocess (0.9.0) ffi (~> 1.0, >= 1.0.11) choice (0.2.0) - chromedriver-helper (1.2.0) + chromedriver-helper (2.1.0) archive-zip (~> 0.10) nokogiri (~> 1.8) chunky_png (1.3.10) codeclimate-engine-rb (0.4.1) virtus (~> 1.0) - codeclimate-test-reporter (1.0.8) + codeclimate-test-reporter (1.0.9) simplecov (<= 0.13) coderay (1.1.2) coercible (1.0.0) descendants_tracker (~> 0.0.1) colorize (0.8.1) - concurrent-ruby (1.0.5) + concurrent-ruby (1.1.3) connection_pool (2.2.2) cose (0.1.0) cbor (~> 0.5.9.2) @@ -243,15 +244,15 @@ GEM actionmailer (>= 4.0, < 6) activesupport (>= 4.0, < 6) execjs (2.7.0) - factory_bot (4.11.0) + factory_bot (4.11.1) activesupport (>= 3.0.0) - factory_bot_rails (4.11.0) - factory_bot (~> 4.11.0) + factory_bot_rails (4.11.1) + factory_bot (~> 4.11.1) railties (>= 3.0.0) fakefs (0.18.0) faker (1.9.1) i18n (>= 0.7) - faraday (0.15.2) + faraday (0.15.3) multipart-post (>= 1.2, < 3) fasterer (0.4.1) colorize (~> 0.7) @@ -291,23 +292,25 @@ GEM hashie (3.6.0) heapy (0.1.3) highline (2.0.0) - hiredis (0.6.1) + hiredis (0.6.3) htmlentities (4.3.4) http_accept_language (2.1.1) - httparty (0.16.2) + httparty (0.16.3) + mime-types (~> 3.0) multi_xml (>= 0.5.2) httpi (2.4.3) rack socksify - i18n (1.1.0) + i18n (1.1.1) concurrent-ruby (~> 1.0) - i18n-tasks (0.9.24) + i18n-tasks (0.9.28) activesupport (>= 4.0.2) ast (>= 2.1.0) erubi highline (>= 2.0.0) i18n parser (>= 2.2.3.0) + rails-i18n rainbow (>= 2.2.2, < 4.0) terminal-table (>= 1.5.1) ice_nine (0.11.2) @@ -330,48 +333,53 @@ GEM rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) ruby_dep (~> 1.2) + local_time (2.1.0) lograge (0.10.0) actionpack (>= 4) activesupport (>= 4) railties (>= 4) request_store (~> 1.0) - loofah (2.2.2) + loofah (2.2.3) crass (~> 1.0.2) nokogiri (>= 1.5.9) lumberjack (1.0.13) macaddr (1.7.1) systemu (~> 2.6.2) - mail (2.7.0) + mail (2.7.1) mini_mime (>= 0.1.1) memory_profiler (0.9.11) - method_source (0.9.0) + method_source (0.9.2) + mime-types (3.2.2) + mime-types-data (~> 3.2015) + mime-types-data (3.2018.0812) mini_mime (1.0.1) mini_portile2 (2.3.0) minitest (5.11.3) multi_xml (0.6.0) multipart-post (2.0.0) - mustermann (1.0.2) + mustermann (1.0.3) nenv (0.3.0) net-sftp (2.1.2) net-ssh (>= 2.6.5) net-ssh (4.1.0) - newrelic_rpm (5.4.0.347) + newrelic_rpm (5.5.0.348) nio4r (2.3.1) - nokogiri (1.8.4) + nokogiri (1.8.5) mini_portile2 (~> 2.3.0) nori (2.6.0) notiffany (0.1.1) nenv (~> 0.1) shellany (~> 0.0) + openssl (2.1.2) orm_adapter (0.5.0) overcommit (0.46.0) childprocess (~> 0.6, >= 0.6.3) iniparse (~> 1.4) parallel (1.12.1) - parser (2.5.1.2) + parser (2.5.3.0) ast (~> 2.4.0) pg (1.1.3) - phonelib (0.6.24) + phonelib (0.6.27) pkcs11 (0.2.7) powerpack (0.1.2) premailer (1.11.1) @@ -389,14 +397,14 @@ GEM pry (~> 0.10) public_suffix (3.0.3) puma (3.12.0) - rack (2.0.5) - rack-attack (5.4.0) + rack (2.0.6) + rack-attack (5.4.2) rack (>= 1.0, < 3) rack-cors (1.0.2) rack-headers_filter (0.0.1) rack-mini-profiler (1.0.0) rack (>= 1.2.0) - rack-protection (2.0.3) + rack-protection (2.0.4) rack rack-proxy (0.6.4) rack @@ -432,6 +440,9 @@ GEM ruby-graphviz (~> 1.2) rails-html-sanitizer (1.0.4) loofah (~> 2.2, >= 2.2.2) + rails-i18n (5.1.2) + i18n (>= 0.7, < 2) + railties (>= 5.0, < 6) railties (5.1.6) actionpack (= 5.1.6) activesupport (= 5.1.6) @@ -448,15 +459,16 @@ GEM readthis (2.2.0) connection_pool (~> 2.1) redis (>= 3.0, < 5.0) - recaptcha (4.12.0) + recaptcha (4.13.0) json redis (3.3.5) - reek (5.0.2) + reek (5.2.0) codeclimate-engine-rb (~> 0.4.0) kwalify (~> 0.7.0) parser (>= 2.5.0.0, < 2.6, != 2.5.1.1) rainbow (>= 2.0, < 4.0) referer-parser (0.3.0) + regexp_parser (1.2.0) request_store (1.4.1) rack (>= 1.4) responders (2.4.0) @@ -471,13 +483,13 @@ GEM rspec-mocks (~> 3.8.0) rspec-core (3.8.0) rspec-support (~> 3.8.0) - rspec-expectations (3.8.1) + rspec-expectations (3.8.2) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.8.0) rspec-mocks (3.8.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.8.0) - rspec-rails (3.8.0) + rspec-rails (3.8.1) actionpack (>= 3.0) activesupport (>= 3.0) railties (>= 3.0) @@ -524,7 +536,7 @@ GEM nokogiri (>= 1.8.1) nori (~> 2.4) wasabi (~> 3.4) - scrypt (3.0.5) + scrypt (3.0.6) ffi-compiler (>= 1.0, < 2.0) secure_headers (6.0.0) selenium-webdriver (3.14.0) @@ -541,20 +553,20 @@ GEM docile (~> 1.1.0) json (>= 1.8, < 3) simplecov-html (~> 0.10.0) - simplecov-html (0.10.0) - sinatra (2.0.3) + simplecov-html (0.10.2) + sinatra (2.0.4) mustermann (~> 1.0) rack (~> 2.0) - rack-protection (= 2.0.3) + rack-protection (= 2.0.4) tilt (~> 2.0) - slim (3.0.9) + slim (4.0.1) temple (>= 0.7.6, < 0.9) - tilt (>= 1.3.3, < 2.1) - slim-rails (3.1.3) + tilt (>= 2.0.6, < 2.1) + slim-rails (3.2.0) actionpack (>= 3.1) railties (>= 3.1) - slim (~> 3.0) - slim_lint (0.16.0) + slim (>= 3.0, < 5.0) + slim_lint (0.16.1) rake (>= 10, < 13) rubocop (>= 0.50.0) slim (>= 3.0, < 5.0) @@ -567,8 +579,8 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - stringex (2.8.4) - strong_migrations (0.2.3) + stringex (2.8.5) + strong_migrations (0.3.1) activerecord (>= 3.2.0) sysexits (1.2.0) systemu (2.6.5) @@ -579,11 +591,11 @@ GEM daemons (~> 1.0, >= 1.0.9) eventmachine (~> 1.0, >= 1.0.4) rack (>= 1, < 3) - thor (0.20.0) + thor (0.20.3) thread_safe (0.3.6) tilt (2.0.8) timecop (0.9.1) - twilio-ruby (5.12.4) + twilio-ruby (5.15.2) faraday (~> 0.9) jwt (>= 1.5, <= 2.5) nokogiri (>= 1.6, < 2.0) @@ -593,18 +605,18 @@ GEM rails (>= 3.1.1) randexp rotp (>= 3.2.0) - typhoeus (1.3.0) + typhoeus (1.3.1) ethon (>= 0.9.0) tzinfo (1.2.5) thread_safe (~> 0.1) uglifier (3.2.0) execjs (>= 0.3.0, < 3) unicode-display_width (1.4.0) - uniform_notifier (1.11.0) + uniform_notifier (1.12.1) user_agent_parser (2.4.1) uuid (2.3.9) macaddr (~> 1.0) - valid_email (0.1.0) + valid_email (0.1.2) activemodel mail (>= 2.6.1) virtus (1.0.5) @@ -617,9 +629,11 @@ GEM wasabi (3.5.0) httpi (~> 2.0) nokogiri (>= 1.4.2) - webauthn (1.3.0) + webauthn (1.7.0) cbor (~> 0.5.9.2) cose (~> 0.1.0) + jwt (>= 1.5, < 3.0) + openssl (~> 2.0) webmock (3.4.2) addressable (>= 2.3.6) crack (>= 0.3.2) @@ -641,7 +655,7 @@ GEM xmlmapper (>= 0.7.3) xmlmapper (0.7.3) nokogiri (~> 1.5) - xpath (3.1.0) + xpath (3.2.0) nokogiri (~> 1.8) zonebie (0.6.1) zxcvbn-js (4.4.3) @@ -694,6 +708,7 @@ DEPENDENCIES json-jwt knapsack lexisnexis! + local_time lograge net-sftp newrelic_rpm @@ -756,4 +771,4 @@ RUBY VERSION ruby 2.5.1p57 BUNDLED WITH - 1.16.5 + 1.16.6 diff --git a/app/assets/images/security-key.svg b/app/assets/images/security-key.svg new file mode 100644 index 00000000000..0e609bd4a54 --- /dev/null +++ b/app/assets/images/security-key.svg @@ -0,0 +1 @@ +security-key \ No newline at end of file diff --git a/app/assets/stylesheets/variables/_app.scss b/app/assets/stylesheets/variables/_app.scss index 2125b2ec0fb..8d19a7fa99c 100644 --- a/app/assets/stylesheets/variables/_app.scss +++ b/app/assets/stylesheets/variables/_app.scss @@ -22,15 +22,15 @@ $line-height-2: 1.3 !default; $line-height-3: 1.75 !default; $line-height-4: 2 !default; -$h1: 1.75rem !default; -$h2: 1.5rem !default; -$h3: 1.125rem !default; +$h1: 1.625rem !default; +$h2: 1.25rem !default; +$h3: 1rem !default; $h4: .875rem !default; -$h5: .75rem !default; -$h6: .75rem !default; +$h5: .8125rem !default; +$h6: .625rem !default; -$sm-h1: 2.5rem !default; -$sm-h2: 2rem !default; +$sm-h1: 1.3125rem !default; +$sm-h2: 1.125rem !default; $sm-h3: 1.5rem !default; $sm-h4: 1.125rem !default; $sm-h5: .875rem !default; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 462c8f3e7df..a36efcf8e2f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,4 +1,7 @@ +require 'core_extensions/string/permit' + class ApplicationController < ActionController::Base + String.include CoreExtensions::String::Permit include UserSessionContext include VerifyProfileConcern include LocaleHelper diff --git a/app/controllers/concerns/phone_confirmation.rb b/app/controllers/concerns/phone_confirmation.rb index ebf8df79f07..266a59df7f6 100644 --- a/app/controllers/concerns/phone_confirmation.rb +++ b/app/controllers/concerns/phone_confirmation.rb @@ -1,21 +1,21 @@ module PhoneConfirmation - def prompt_to_confirm_phone(phone:, selected_delivery_method: nil) + def prompt_to_confirm_phone(id:, phone:, selected_delivery_method: nil) user_session[:unconfirmed_phone] = phone user_session[:context] = 'confirmation' redirect_to otp_send_url( otp_delivery_selection_form: { - otp_delivery_preference: otp_delivery_method(phone, selected_delivery_method), + otp_delivery_preference: otp_delivery_method(id, phone, selected_delivery_method), } ) end private - def otp_delivery_method(phone, selected_delivery_method) + def otp_delivery_method(id, phone, selected_delivery_method) return :sms if PhoneNumberCapabilities.new(phone).sms_only? return selected_delivery_method if selected_delivery_method.present? - MfaContext.new(current_user).phone_configurations.first&.delivery_preference || + MfaContext.new(current_user).phone_configuration(id)&.delivery_preference || current_user.otp_delivery_preference end end diff --git a/app/controllers/concerns/two_factor_authenticatable.rb b/app/controllers/concerns/two_factor_authenticatable.rb index 52df8a0072f..6478ebfa0ce 100644 --- a/app/controllers/concerns/two_factor_authenticatable.rb +++ b/app/controllers/concerns/two_factor_authenticatable.rb @@ -131,7 +131,7 @@ def handle_valid_otp_for_authentication_context end def assign_phone - @updating_existing_number = old_phone.present? + @updating_existing_number = user_session[:phone_id].present? if @updating_existing_number && confirmation_context? phone_changed @@ -142,10 +142,6 @@ def assign_phone update_phone_attributes end - def old_phone - MfaContext.new(current_user).phone_configurations.first&.phone - end - def phone_changed create_user_event(:phone_changed) current_user.confirmed_email_addresses.each do |email_address| @@ -160,7 +156,8 @@ def phone_confirmed def update_phone_attributes UpdateUser.new( user: current_user, - attributes: { phone: user_session[:unconfirmed_phone], phone_confirmed_at: Time.zone.now } + attributes: { phone_id: user_session[:phone_id], phone: user_session[:unconfirmed_phone], + phone_confirmed_at: Time.zone.now } ).call end @@ -253,7 +250,7 @@ def generic_data def display_phone_to_deliver_to if authentication_context? - decorated_user.masked_two_factor_phone_number + masked_number(phone_configuration.phone) else user_session[:unconfirmed_phone] end @@ -261,7 +258,7 @@ def display_phone_to_deliver_to def voice_otp_delivery_unsupported? phone_number = if authentication_context? - MfaContext.new(current_user).phone_configurations.first&.phone + phone_configuration&.phone else user_session[:unconfirmed_phone] end @@ -297,4 +294,13 @@ def presenter_for_two_factor_authentication_method view: view_context ) end + + def phone_configuration + MfaContext.new(current_user).phone_configuration(user_session[:phone_id]) + end + + def masked_number(number) + return '' if number.blank? + "***-***-#{number[-4..-1]}" + end end diff --git a/app/controllers/sign_up/email_resend_controller.rb b/app/controllers/sign_up/email_resend_controller.rb index 5a46b03636d..2ee962a5af0 100644 --- a/app/controllers/sign_up/email_resend_controller.rb +++ b/app/controllers/sign_up/email_resend_controller.rb @@ -1,7 +1,5 @@ module SignUp class EmailResendController < ApplicationController - include UnconfirmedUserConcern - def new @user = User.new @resend_email_confirmation_form = ResendEmailConfirmationForm.new diff --git a/app/controllers/two_factor_authentication/options_controller.rb b/app/controllers/two_factor_authentication/options_controller.rb index 60a5c9080c9..bba4b0fc10b 100644 --- a/app/controllers/two_factor_authentication/options_controller.rb +++ b/app/controllers/two_factor_authentication/options_controller.rb @@ -55,8 +55,13 @@ def mfa_redirect_url options = EXTRA_URL_OPTIONS[selection] || {} configuration_id = @two_factor_options_form.configuration_id - options[:id] = configuration_id if configuration_id.present? + user_session[:phone_id] = configuration_id if configuration_id.present? + options[:id] = user_session[:phone_id] + build_url(selection, options) + end + + def build_url(selection, options) method = FACTOR_TO_URL_METHOD[selection] public_send(method, options) if method.present? end diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 0de0813df51..e248d9c117c 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -58,7 +58,7 @@ def confirm_voice_capability end def phone - MfaContext.new(current_user).phone_configurations.first&.phone || + MfaContext.new(current_user).phone_configuration(user_session[:phone_id])&.phone || user_session[:unconfirmed_phone] end diff --git a/app/controllers/users/personal_keys_controller.rb b/app/controllers/users/personal_keys_controller.rb index 72f2b13d462..4dfccebce70 100644 --- a/app/controllers/users/personal_keys_controller.rb +++ b/app/controllers/users/personal_keys_controller.rb @@ -27,6 +27,7 @@ def create user_session[:personal_key] = create_new_code analytics.track_event(Analytics::PROFILE_PERSONAL_KEY_CREATE) Event.create(user_id: current_user.id, event_type: :new_personal_key) + send_new_personal_key_notification redirect_to manage_personal_key_url end @@ -45,5 +46,14 @@ def next_step def user_has_not_visited_any_sp_yet? current_user.identities.pluck(:last_authenticated_at).compact.empty? end + + def send_new_personal_key_notification + current_user.confirmed_email_addresses.each do |email_address| + UserMailer.personal_key_regenerated(email_address.email).deliver_now + end + MfaContext.new(current_user).phone_configurations.each do |phone_configuration| + SmsPersonalKeyRegenerationNotifierJob.perform_now(phone: phone_configuration.phone) + end + end end end diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb index fdd95c94adf..574852e5ccd 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -21,7 +21,7 @@ def create analytics.track_event(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result.to_h) if result.success? - prompt_to_confirm_phone(phone: @user_phone_form.phone) + prompt_to_confirm_phone(id: nil, phone: @user_phone_form.phone) else render :index end diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index 0d33f537179..fa434197a11 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -4,7 +4,25 @@ class PhonesController < ReauthnRequiredController before_action :confirm_two_factor_authenticated + def add + user_session[:phone_id] = nil + @user_phone_form = UserPhoneForm.new(current_user, nil) + @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + end + + def create + @user_phone_form = UserPhoneForm.new(current_user, nil) + @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + if @user_phone_form.submit(user_params).success? + confirm_phone + bypass_sign_in current_user + else + render :add + end + end + def edit + set_phone_id @user_phone_form = UserPhoneForm.new(current_user, phone_configuration) @presenter = PhoneSetupPresenter.new(delivery_preference) end @@ -26,7 +44,7 @@ def delete ).submit analytics.track_event(Analytics::PHONE_DELETION_REQUESTED, result.to_h) if result.success? - flash[:success] = t('two_factor_authentication.phone.delete.success') + handle_successful_delete else flash[:error] = t('two_factor_authentication.phone.delete.failure') end @@ -40,7 +58,7 @@ def delete # doing away with this controller. Once we move to multiple phones, we'll allow # adding and deleting, but not editing. def phone_configuration - MfaContext.new(current_user).phone_configurations.first + MfaContext.new(current_user).phone_configuration(user_session[:phone_id]) end def user_params @@ -55,13 +73,26 @@ def process_updates form = @user_phone_form if form.phone_changed? analytics.track_event(Analytics::PHONE_CHANGE_REQUESTED) - flash[:notice] = t('devise.registrations.phone_update_needs_confirmation') - prompt_to_confirm_phone( - phone: form.phone, selected_delivery_method: form.otp_delivery_preference - ) + confirm_phone else redirect_to account_url end end + + def confirm_phone + flash[:notice] = t('devise.registrations.phone_update_needs_confirmation') + prompt_to_confirm_phone(id: user_session[:phone_id], phone: @user_phone_form.phone, + selected_delivery_method: @user_phone_form.otp_delivery_preference) + end + + def handle_successful_delete + flash[:success] = t('two_factor_authentication.phone.delete.success') + create_user_event(:phone_removed) + end + + def set_phone_id + phone_id = params[:id] + user_session[:phone_id] = phone_id if phone_id.present? + end end end diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 89f31c2a2eb..988331434f8 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -31,7 +31,7 @@ def phone_enabled? end def phone_configuration - MfaContext.new(current_user).phone_configurations.first + MfaContext.new(current_user).phone_configuration(user_session[:phone_id]) end def validate_otp_delivery_preference_and_send_code @@ -51,10 +51,11 @@ def delivery_preference end def update_otp_delivery_preference_if_needed + return unless user_signed_in? OtpDeliveryPreferenceUpdater.new( user: current_user, preference: delivery_params[:otp_delivery_preference], - context: otp_delivery_selection_form.context + phone_id: user_session[:phone_id] ).call end diff --git a/app/decorators/event_decorator.rb b/app/decorators/event_decorator.rb index 255747154e6..3937b08c368 100644 --- a/app/decorators/event_decorator.rb +++ b/app/decorators/event_decorator.rb @@ -8,7 +8,7 @@ def event_type end def happened_at - event.created_at + event.created_at.utc end def happened_at_in_words diff --git a/app/decorators/identity_decorator.rb b/app/decorators/identity_decorator.rb index 63c65cc6eeb..15d9544be6a 100644 --- a/app/decorators/identity_decorator.rb +++ b/app/decorators/identity_decorator.rb @@ -19,11 +19,15 @@ def return_to_sp_url end def created_at_in_words - UtcTimePresenter.new(identity.created_at).to_s + UtcTimePresenter.new(created_at).to_s + end + + def created_at + identity.created_at.utc end def happened_at - identity.last_authenticated_at + identity.last_authenticated_at.utc end def happened_at_in_words diff --git a/app/decorators/mfa_context.rb b/app/decorators/mfa_context.rb index 4f1f9042559..4749a42a093 100644 --- a/app/decorators/mfa_context.rb +++ b/app/decorators/mfa_context.rb @@ -13,6 +13,11 @@ def phone_configurations end end + def phone_configuration(id = nil) + return phone_configurations.first if id.blank? + phone_configurations.find { |cfg| cfg.id.to_s == id.to_s } + end + def webauthn_configurations if user.present? user.webauthn_configurations diff --git a/app/forms/two_factor_login_options_form.rb b/app/forms/two_factor_login_options_form.rb index c3d324b404a..3002a759a8d 100644 --- a/app/forms/two_factor_login_options_form.rb +++ b/app/forms/two_factor_login_options_form.rb @@ -11,11 +11,7 @@ def initialize(user) end def submit(params) - selection = params[:selection] - (selection, configuration_id) = selection.split(':', 2) if selection.present? - - self.selection = selection - self.configuration_id = configuration_id + self.selection, self.configuration_id = selection_and_configuration_id(params) success = valid? @@ -28,6 +24,16 @@ def submit(params) attr_writer :selection attr_writer :configuration_id + def selection_and_configuration_id(params) + selection = params[:selection] + configuration_id = nil + if selection =~ /(.+)[:_](\d+)/ + selection = Regexp.last_match(1) + configuration_id = Regexp.last_match(2) + end + [selection, configuration_id] + end + def extra_analytics_attributes { selection: selection, diff --git a/app/forms/two_factor_options_form.rb b/app/forms/two_factor_options_form.rb index 07be228f3e0..4c7e2b5bd9f 100644 --- a/app/forms/two_factor_options_form.rb +++ b/app/forms/two_factor_options_form.rb @@ -37,7 +37,7 @@ def extra_analytics_attributes def user_needs_updating? %w[voice sms].include?(selection) && - selection != MfaContext.new(user).phone_configurations.first&.delivery_preference && + selection != MfaContext.new(user).phone_configuration&.delivery_preference && selection != user.otp_delivery_preference end diff --git a/app/javascript/app/local-time.js b/app/javascript/app/local-time.js new file mode 100644 index 00000000000..f3689498693 --- /dev/null +++ b/app/javascript/app/local-time.js @@ -0,0 +1,108 @@ +// modifications marked with "login.gov" original here: https://github.com/basecamp/local_time/blob/master/app/assets/javascripts/local-time.js + +(function(){var t=this;(function(){(function(){var t=[].slice; + +// login.gov +window.LocalTime={ + + config:{},run:function(){return this.getController().processElements()},process:function(){var e,n,r,a;for(n=1<=arguments.length?t.call(arguments,0):[],r=0,a=n.length;r11?"pm":"am")).toUpperCase();case"P":return i("time."+(c>11?"pm":"am"));case"S":return n(h,m);case"w":return u;case"y":return n(f%100,m);case"Y":return f;case"Z":return r(e)}})},n=function(t,e){switch(e){case"-":return t;default:return("0"+t).slice(-2)}},r=function(t){var e,n,r,a,i;return i=t.toString(),(e=null!=(n=i.match(/\(([\w\s]+)\)$/))?n[1]:void 0)?/\s/.test(e)?e.match(/\b(\w)/g).join(""):e:(e=null!=(r=i.match(/(\w{3,4})\s\d{4}$/))?r[1]:void 0)?e:(e=null!=(a=i.match(/(UTC[\+\-]\d+)/))?a[1]:void 0)?e:""}}.call(this),function(){e.CalendarDate=function(){function t(t,e,n){this.date=new Date(Date.UTC(t,e-1)),this.date.setUTCDate(n),this.year=this.date.getUTCFullYear(),this.month=this.date.getUTCMonth()+1,this.day=this.date.getUTCDate(),this.value=this.date.getTime()}return t.fromDate=function(t){return new this(t.getFullYear(),t.getMonth()+1,t.getDate())},t.today=function(){return this.fromDate(new Date)},t.prototype.equals=function(t){return(null!=t?t.value:void 0)===this.value},t.prototype.is=function(t){return this.equals(t)},t.prototype.isToday=function(){return this.is(this.constructor.today())},t.prototype.occursOnSameYearAs=function(t){return this.year===(null!=t?t.year:void 0)},t.prototype.occursThisYear=function(){return this.occursOnSameYearAs(this.constructor.today())},t.prototype.daysSince=function(t){if(t)return(this.date-t.date)/864e5},t.prototype.daysPassed=function(){return this.constructor.today().daysSince(this)},t}()}.call(this),function(){var t,n,r;n=e.strftime,r=e.translate,t=e.getI18nValue,e.RelativeTime=function(){function a(t){this.date=t,this.calendarDate=e.CalendarDate.fromDate(this.date)}return a.prototype.toString=function(){var t,e;return(e=this.toTimeElapsedString())?r("time.elapsed",{time:e}):(t=this.toWeekdayString())?(e=this.toTimeString(),r("datetime.at",{date:t,time:e})):r("date.on",{date:this.toDateString()})},a.prototype.toTimeOrDateString=function(){return this.calendarDate.isToday()?this.toTimeString():this.toDateString()},a.prototype.toTimeElapsedString=function(){var t,e,n,a,i;return n=(new Date).getTime()-this.date.getTime(),a=Math.round(n/1e3),e=Math.round(a/60),t=Math.round(e/60),n<0?null:a<10?(i=r("time.second"),r("time.singular",{time:i})):a<45?a+" "+r("time.seconds"):a<90?(i=r("time.minute"),r("time.singular",{time:i})):e<45?e+" "+r("time.minutes"):e<90?(i=r("time.hour"),r("time.singularAn",{time:i})):t<24?t+" "+r("time.hours"):""},a.prototype.toWeekdayString=function(){switch(this.calendarDate.daysPassed()){case 0:return r("date.today");case 1:return r("date.yesterday");case-1:return r("date.tomorrow");case 2:case 3:case 4:case 5:case 6:return n(this.date,"%A");default:return""}},a.prototype.toDateString=function(){var e;return e=t(this.calendarDate.occursThisYear()?"date.formats.thisYear":"date.formats.default"),n(this.date,e)},a.prototype.toTimeString=function(){return n(this.date,t("time.formats.default"))},a}()}.call(this),function(){var t,n=function(t,e){return function(){return t.apply(e,arguments)}};t=e.elementMatchesSelector,e.PageObserver=function(){function e(t,e){this.selector=t,this.callback=e,this.processInsertion=n(this.processInsertion,this),this.processMutations=n(this.processMutations,this)}return e.prototype.start=function(){if(!this.started)return this.observeWithMutationObserver()||this.observeWithMutationEvent(),this.started=!0},e.prototype.observeWithMutationObserver=function(){var t;if("undefined"!=typeof MutationObserver&&null!==MutationObserver)return t=new MutationObserver(this.processMutations),t.observe(document.documentElement,{childList:!0,subtree:!0}),!0},e.prototype.observeWithMutationEvent=function(){return addEventListener("DOMNodeInserted",this.processInsertion,!1),!0},e.prototype.findSignificantElements=function(e){var n;return n=[],(null!=e?e.nodeType:void 0)===Node.ELEMENT_NODE&&(t(e,this.selector)&&n.push(e),n.push.apply(n,e.querySelectorAll(this.selector))),n},e.prototype.processMutations=function(t){var e,n,r,a,i,o,s,u;for(e=[],n=0,a=t.length;n lookup(s)).join('')}`; } -function disableSubmit(submitEl, score = 0) { +function disableSubmit(submitEl, length = 0, score = 0) { if (!submitEl) return; - if (score < 3) { + if (score < 3 || length < 12) { submitEl.setAttribute('disabled', true); } else { submitEl.removeAttribute('disabled'); @@ -73,7 +73,7 @@ function analyzePw() { pwStrength.innerHTML = strength; pwFeedback.innerHTML = feedback; - disableSubmit(submit, z.score); + disableSubmit(submit, z.password.length, z.score); } if (/(msie 9)/i.test(userAgent)) { diff --git a/app/javascript/packs/webauthn-authenticate.js b/app/javascript/packs/webauthn-authenticate.js index 7be4c2ee672..3b271d95009 100644 --- a/app/javascript/packs/webauthn-authenticate.js +++ b/app/javascript/packs/webauthn-authenticate.js @@ -17,6 +17,10 @@ function webauthn() { } return window.btoa(binary); }; + // If webauthn is not supported redirect back to the 2fa options list + if (!(navigator && navigator.credentials && navigator.credentials.create)) { + window.location.href = '/login/two_factor/options'; + } const challengeBytes = new Uint8Array(JSON.parse(document.getElementById('user_challenge').value)); let credentialIds = document.getElementById('credential_ids').value; credentialIds = credentialIds.split(','); diff --git a/app/javascript/packs/webauthn-unhide.js b/app/javascript/packs/webauthn-unhide.js new file mode 100644 index 00000000000..872f4220070 --- /dev/null +++ b/app/javascript/packs/webauthn-unhide.js @@ -0,0 +1,17 @@ +function unhideWebauthn() { + if (navigator && navigator.credentials && navigator.credentials.create) { + const elem = document.getElementById('select_webauthn'); + if (elem) { + elem.classList.remove('hidden'); + } + } else { + const checkboxes = document.querySelectorAll('input[name="two_factor_options_form[selection]"]'); + for (let i = 0, len = checkboxes.length; i < len; i += 1) { + if (!checkboxes[i].classList.contains('hidden')) { + checkboxes[i].checked = true; + break; + } + } + } +} +document.addEventListener('DOMContentLoaded', unhideWebauthn); diff --git a/app/jobs/sms_personal_key_regeneration_notifier_job.rb b/app/jobs/sms_personal_key_regeneration_notifier_job.rb new file mode 100644 index 00000000000..3ac8db56757 --- /dev/null +++ b/app/jobs/sms_personal_key_regeneration_notifier_job.rb @@ -0,0 +1,14 @@ +class SmsPersonalKeyRegenerationNotifierJob < ApplicationJob + queue_as :sms + + # :reek:UtilityFunction + def perform(phone:) + TwilioService::Utils.new.send_sms( + to: phone, + body: I18n.t( + 'jobs.sms_personal_key_regeneration_notifier_job.message', + app: APP_NAME + ) + ) + end +end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index dcf0044e6a0..f6461808faa 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -31,6 +31,10 @@ def personal_key_sign_in(email) mail(to: email, subject: t('user_mailer.personal_key_sign_in.subject')) end + def personal_key_regenerated(email) + mail(to: email, subject: t('user_mailer.personal_key_regenerated.subject')) + end + def account_reset_request(email_address, account_reset) @token = account_reset&.request_token mail(to: email_address.email, subject: t('user_mailer.account_reset_request.subject')) diff --git a/app/models/event.rb b/app/models/event.rb index 13d468e6fc7..9683ca8ecdc 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -17,6 +17,7 @@ class Event < ApplicationRecord personal_key_used: 13, webauthn_key_added: 14, webauthn_key_removed: 15, + phone_removed: 16, } validates :event_type, presence: true diff --git a/app/presenters/two_factor_authentication/selection_presenter.rb b/app/presenters/two_factor_authentication/selection_presenter.rb index 9cc06235fe1..6e6b383168d 100644 --- a/app/presenters/two_factor_authentication/selection_presenter.rb +++ b/app/presenters/two_factor_authentication/selection_presenter.rb @@ -21,6 +21,10 @@ def info t("two_factor_authentication.#{option_mode}.#{method}_info") end + def html_class + '' + end + private def option_mode diff --git a/app/presenters/two_factor_authentication/webauthn_selection_presenter.rb b/app/presenters/two_factor_authentication/webauthn_selection_presenter.rb index 53d9ddff59f..8a697032cd3 100644 --- a/app/presenters/two_factor_authentication/webauthn_selection_presenter.rb +++ b/app/presenters/two_factor_authentication/webauthn_selection_presenter.rb @@ -3,5 +3,9 @@ class WebauthnSelectionPresenter < SelectionPresenter def method :webauthn end + + def html_class + 'hidden' + end end end diff --git a/app/presenters/utc_time_presenter.rb b/app/presenters/utc_time_presenter.rb index 83456a95b7f..a454f01c955 100644 --- a/app/presenters/utc_time_presenter.rb +++ b/app/presenters/utc_time_presenter.rb @@ -5,7 +5,7 @@ def initialize(timestamp) def to_s # i18n-tasks-use t('date.month_names') - I18n.l(timestamp, format: :event_timestamp_utc) + I18n.l(timestamp, format: :event_timestamp) end private diff --git a/app/services/otp_delivery_preference_updater.rb b/app/services/otp_delivery_preference_updater.rb index ce1592a504d..bead12dc960 100644 --- a/app/services/otp_delivery_preference_updater.rb +++ b/app/services/otp_delivery_preference_updater.rb @@ -1,8 +1,8 @@ class OtpDeliveryPreferenceUpdater - def initialize(user:, preference:, context:) + def initialize(user:, preference:, phone_id:) @user = user @preference = preference - @context = context + @phone_id = phone_id end def call @@ -12,7 +12,7 @@ def call private - attr_reader :user, :preference, :context + attr_reader :user, :preference, :phone_id def should_update_user? return false unless user @@ -21,7 +21,7 @@ def should_update_user? def otp_delivery_preference_changed? return true if preference != user.otp_delivery_preference - phone_configuration = MfaContext.new(user).phone_configurations.first + phone_configuration = MfaContext.new(user).phone_configuration(phone_id) phone_configuration.present? && preference != phone_configuration.delivery_preference end end diff --git a/app/services/update_user.rb b/app/services/update_user.rb index 014fa8e8277..a1cef214fae 100644 --- a/app/services/update_user.rb +++ b/app/services/update_user.rb @@ -5,7 +5,7 @@ def initialize(user:, attributes:) end def call - result = user.update!(attributes.except(:phone, :phone_confirmed_at)) + result = user.update!(attributes.except(:phone_id, :phone, :phone_confirmed_at)) manage_phone_configuration result end @@ -15,7 +15,7 @@ def call attr_reader :user, :attributes def manage_phone_configuration - if MfaContext.new(user).phone_configurations.any? + if attributes[:phone_id].present? update_phone_configuration else create_phone_configuration @@ -23,14 +23,18 @@ def manage_phone_configuration end def update_phone_configuration - MfaContext.new(user).phone_configurations.first.update!(phone_attributes) + MfaContext.new(user).phone_configuration(attributes[:phone_id]).update!(phone_attributes) end def create_phone_configuration - return if phone_attributes[:phone].blank? + return if phone_attributes[:phone].blank? || duplicate_phone? MfaContext.new(user).phone_configurations.create(phone_attributes) end + def duplicate_phone? + MfaContext.new(user).phone_configurations.map(&:phone).index(phone_attributes[:phone]) + end + def phone_attributes @phone_attributes ||= { phone: attributes[:phone], diff --git a/app/views/account_reset/delete_account/show.html.slim b/app/views/account_reset/delete_account/show.html.slim index 0ad05055252..f74626f8f24 100644 --- a/app/views/account_reset/delete_account/show.html.slim +++ b/app/views/account_reset/delete_account/show.html.slim @@ -6,10 +6,8 @@ p.mt-tiny.mb0 br h4.my2 = t('account_reset.delete_account.are_you_sure') -= button_to t('account_reset.delete_account.delete_button'), \ - account_reset_delete_account_path, method: :delete, \ - class: 'btn btn-red col-6 p2 rounded-lg border bw2 bg-lightest-red border-red border-box' -br -br -hr -= link_to t('account_reset.delete_account.cancel'), root_url += button_to t('account_reset.request.no_cancel'), root_url, method: :get, + class: 'btn btn-primary btn-wide mb1 personal-key-continue', + 'data-toggle': 'modal' += button_to t('account_reset.request.yes_continue'), account_reset_delete_account_path, \ + method: :delete, class: 'btn btn-link' diff --git a/app/views/accounts/_connected_app.html.slim b/app/views/accounts/_connected_app.html.slim index a03c55bf0dc..8d821d913f8 100644 --- a/app/views/accounts/_connected_app.html.slim +++ b/app/views/accounts/_connected_app.html.slim @@ -7,4 +7,5 @@ - else = identity.display_name .sm-col.sm-col-6.px1.sm-right-align - = t('account.connected_apps.associated') + ' ' + identity.created_at_in_words + == t('account.connected_apps.associated', + timestamp: local_time(identity.created_at, t('time.formats.event_timestamp'))) diff --git a/app/views/accounts/_event_item.html.slim b/app/views/accounts/_event_item.html.slim index 0de2fd45045..cad7a923951 100644 --- a/app/views/accounts/_event_item.html.slim +++ b/app/views/accounts/_event_item.html.slim @@ -3,4 +3,4 @@ .sm-col.sm-col-6.px1 .bold.truncate = event.event_type .sm-col.sm-col-6.px1.sm-right-align - = event.happened_at_in_words + = local_time(event.happened_at, t('time.formats.event_timestamp')) diff --git a/app/views/accounts/_identity_item.html.slim b/app/views/accounts/_identity_item.html.slim index 25d0d5988ec..3d2b23cd4df 100644 --- a/app/views/accounts/_identity_item.html.slim +++ b/app/views/accounts/_identity_item.html.slim @@ -8,4 +8,4 @@ - else = t('event_types.authenticated_at', service_provider: event.display_name) .sm-col.sm-col-6.px1.sm-right-align - = event.happened_at_in_words + = local_time(event.happened_at, t('time.formats.event_timestamp')) diff --git a/app/views/accounts/_phone.html.slim b/app/views/accounts/_phone.html.slim index a159b29c771..3a8c23ead90 100644 --- a/app/views/accounts/_phone.html.slim +++ b/app/views/accounts/_phone.html.slim @@ -3,14 +3,13 @@ .col.col-6.bold = t('account.index.phone') .right-align.col.col-6 - - if MfaContext.new(current_user).phone_configurations.empty? - .btn.btn-account-action.rounded-lg.bg-light-blue - = link_to t('account.index.phone_add'), manage_phone_path + .btn.btn-account-action.rounded-lg.bg-light-blue + = link_to t('account.index.phone_add'), add_phone_path - MfaContext.new(current_user).phone_configurations.each do |phone_configuration| .p2.col.col-12.border-top.border-blue-light.account-list-item .col.col-8.sm-6.truncate = phone_configuration.phone .col.col-4.sm-6.right-align = render @view_model.manage_action_partial, - path: manage_phone_url, + path: manage_phone_url(id: phone_configuration.id), name: t('account.index.phone') diff --git a/app/views/two_factor_authentication/options/index.html.slim b/app/views/two_factor_authentication/options/index.html.slim index 4ad5242d9af..5b51075e6fc 100644 --- a/app/views/two_factor_authentication/options/index.html.slim +++ b/app/views/two_factor_authentication/options/index.html.slim @@ -11,14 +11,16 @@ p.mt-tiny.mb3 = @presenter.info fieldset.m0.p0.border-none. legend.mb2.serif.bold = @presenter.label - @presenter.options.each_with_index do |option, index| - label.btn-border.col-12.mb2 for="two_factor_options_form_selection_#{option.type}" - .radio - = radio_button_tag('two_factor_options_form[selection]', + span id="select_#{option.type}" class="#{option.html_class}" + label.btn-border.col-12.mb2 for="two_factor_options_form_selection_#{option.type}" + .radio + = radio_button_tag('two_factor_options_form[selection]', option.type, - index.zero?) - span.indicator.mt-tiny - span.blue.bold.fs-20p = option.label - .regular.gray-dark.fs-10p.mb-tiny = option.info + index.zero?, + class: option.html_class.to_s) + span.indicator.mt-tiny + span.blue.bold.fs-20p = option.label + .regular.gray-dark.fs-10p.mb-tiny = option.info = f.button :submit, t('forms.buttons.continue') @@ -26,3 +28,5 @@ br - if @presenter.should_display_account_reset_or_cancel_link? p = @presenter.account_reset_or_cancel_link = render 'shared/cancel', link: destroy_user_session_path + +== javascript_pack_tag 'webauthn-unhide' diff --git a/app/views/user_mailer/personal_key_regenerated.html.slim b/app/views/user_mailer/personal_key_regenerated.html.slim new file mode 100644 index 00000000000..9ae7e2becc0 --- /dev/null +++ b/app/views/user_mailer/personal_key_regenerated.html.slim @@ -0,0 +1,16 @@ +p.lead + strong == t('.intro') + +table.spacer + tbody + tr + td.s10 height="10px" + |   +table.hr + tr + th + |   + +p == t('.help_html', + reset_password_url: forgot_password_url, + account_url: account_url) diff --git a/app/views/users/phones/add.html.slim b/app/views/users/phones/add.html.slim new file mode 100644 index 00000000000..6ad88eb65ea --- /dev/null +++ b/app/views/users/phones/add.html.slim @@ -0,0 +1,23 @@ +- title t('titles.add_info.phone') + +h1.h3.my0 = t('headings.add_info.phone') += simple_form_for(@user_phone_form, + html: { autocomplete: 'off', method: :post, role: 'form' }, + data: { international_phone_form: true }, + url: add_phone_path) do |f| + .sm-col-8.js-intl-tel-code-select + = f.input :international_code, + collection: international_phone_codes, + include_blank: false, + input_html: { class: 'international-code' } + .sm-col-8.mb3 + = f.label :phone + strong.left = @presenter.label + = f.input :phone, as: :tel, label: false, required: true, + input_html: { class: 'phone col-8 mb4' } + = render 'users/shared/otp_delivery_preference_selection' + = f.button :submit, t('forms.buttons.continue'), class: 'btn-wide' += render 'shared/cancel', link: account_path + += stylesheet_link_tag 'intl-tel-number/intlTelInput' += javascript_pack_tag 'intl-tel-input' diff --git a/app/views/users/phones/edit.html.slim b/app/views/users/phones/edit.html.slim index 58e1ca307b5..63793bab03a 100644 --- a/app/views/users/phones/edit.html.slim +++ b/app/views/users/phones/edit.html.slim @@ -20,7 +20,7 @@ h1.h3.my0 = t('headings.edit_info.phone') - if MfaPolicy.new(current_user).multiple_factors_enabled? && @user_phone_form.phone.present? br .sm-col-8.mb3 - = button_to t('forms.phone.buttons.delete'), manage_phone_url, \ + = button_to t('forms.phone.buttons.delete'), manage_phone_url(id: params[:id]), \ class: 'btn btn-danger btn-wide', \ method: :delete = render 'shared/cancel', link: account_path diff --git a/app/views/users/webauthn_setup/new.html.slim b/app/views/users/webauthn_setup/new.html.slim index e3b7d09b62f..7c736094c04 100644 --- a/app/views/users/webauthn_setup/new.html.slim +++ b/app/views/users/webauthn_setup/new.html.slim @@ -2,6 +2,7 @@ - help_link = link_to t('links.what_is_webauthn'), MarketingSite.help_hardware_security_key_url, target: :_blank += image_tag asset_url('security-key.svg'), width: '90', class: 'ml1' h1.h3.my0 = t('headings.webauthn_setup.new') p.mt-tiny.mb3 = t('forms.webauthn_setup.intro_html', link: help_link) div diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 71fdb22d099..e9eb1f52038 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -63,6 +63,7 @@ search: - app/assets/images - app/assets/fonts - app/views/shared/newrelic/_browser_instrumentation.html.erb + - app/javascript/app/local-time.js ## Alternatively, the only files or `File.fnmatch patterns` to search in `paths`: ## If specified, this settings takes priority over `exclude`, but `exclude` still applies. diff --git a/config/locales/account/en.yml b/config/locales/account/en.yml index e777badba1d..da29c9b4249 100644 --- a/config/locales/account/en.yml +++ b/config/locales/account/en.yml @@ -2,7 +2,7 @@ en: account: connected_apps: - associated: Associated + associated: Associated %{timestamp} index: address: Current address auth_app_disabled: not enabled diff --git a/config/locales/account/es.yml b/config/locales/account/es.yml index 2f9cf0e47f9..3407a9bc55e 100644 --- a/config/locales/account/es.yml +++ b/config/locales/account/es.yml @@ -2,7 +2,7 @@ es: account: connected_apps: - associated: Asociado + associated: Asociado %{timestamp} index: address: Dirección actual auth_app_disabled: no permitido diff --git a/config/locales/account/fr.yml b/config/locales/account/fr.yml index 8e1965b4350..fff3e00bca9 100644 --- a/config/locales/account/fr.yml +++ b/config/locales/account/fr.yml @@ -2,7 +2,7 @@ fr: account: connected_apps: - associated: Associé + associated: Associé %{timestamp} index: address: Adresse actuelle auth_app_disabled: non activée diff --git a/config/locales/account_reset/en.yml b/config/locales/account_reset/en.yml index aa5ca0313a3..45d5f44a951 100644 --- a/config/locales/account_reset/en.yml +++ b/config/locales/account_reset/en.yml @@ -21,8 +21,6 @@ en: phone number. delete_account: are_you_sure: Are you sure you want to delete your account? - cancel: Cancel - delete_button: Delete account info: Deleting your account should be your last resort if you are locked out of your account. You will not be able to recover any information linked to your account. Once your account is deleted, you can create a new one using diff --git a/config/locales/account_reset/es.yml b/config/locales/account_reset/es.yml index ec756dff785..216599f027d 100644 --- a/config/locales/account_reset/es.yml +++ b/config/locales/account_reset/es.yml @@ -22,8 +22,6 @@ es: a su registro número de teléfono. delete_account: are_you_sure: "¿Seguro que quieres eliminar tu cuenta?" - cancel: Cancelar - delete_button: Borrar cuenta info: Eliminar su cuenta debe ser su último recurso si está bloqueado          de tu cuenta No podrá recuperar ninguna información vinculada a su cuenta. Una vez que se elimine su cuenta, puede crear una nueva usando la misma dirección diff --git a/config/locales/account_reset/fr.yml b/config/locales/account_reset/fr.yml index bbd0758b6d5..12bf48555c0 100644 --- a/config/locales/account_reset/fr.yml +++ b/config/locales/account_reset/fr.yml @@ -23,8 +23,6 @@ fr: votre numéro de téléphone enregistré. delete_account: are_you_sure: Êtes-vous sûr de vouloir supprimer votre compte? - cancel: Annuler - delete_button: Supprimer le compte info: La suppression de votre compte devrait être votre dernier recours si vous êtes en lock-out de votre compte Vous ne pourrez pas récupérer les informations liées à ton compte. Une fois votre compte supprimé, vous pouvez en créer un diff --git a/config/locales/event_types/en.yml b/config/locales/event_types/en.yml index 6dc6d1abcca..1bd9c6ad802 100644 --- a/config/locales/event_types/en.yml +++ b/config/locales/event_types/en.yml @@ -14,6 +14,7 @@ en: personal_key_used: Personal key used to sign in phone_changed: Phone number changed phone_confirmed: Phone confirmed + phone_removed: Phone number removed piv_cac_disabled: PIV/CAC card unassociated piv_cac_enabled: PIV/CAC card associated usps_mail_sent: Letter sent diff --git a/config/locales/event_types/es.yml b/config/locales/event_types/es.yml index e6d3666452d..fd997966804 100644 --- a/config/locales/event_types/es.yml +++ b/config/locales/event_types/es.yml @@ -14,6 +14,7 @@ es: personal_key_used: Clave personal utilizada para iniciar sesión phone_changed: Número de teléfono cambiado phone_confirmed: Teléfono confirmado + phone_removed: Teléfono eliminado piv_cac_disabled: Tarjeta PIV/CAC no asociada piv_cac_enabled: Tarjeta PIV/CAC asociada usps_mail_sent: Carta enviada diff --git a/config/locales/event_types/fr.yml b/config/locales/event_types/fr.yml index 4d9fc103f25..fc313f97567 100644 --- a/config/locales/event_types/fr.yml +++ b/config/locales/event_types/fr.yml @@ -14,6 +14,7 @@ fr: personal_key_used: Clé personnelle utilisée pour la connexion phone_changed: Numéro de téléphone modifié phone_confirmed: Numéro de téléphone confirmé + phone_removed: Numéro de téléphone supprimé piv_cac_disabled: Carte PIV/CAC non associée piv_cac_enabled: Carte PIV/CAC associée usps_mail_sent: Lettre envoyée diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index bb563307eaa..9f733dfd293 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -12,6 +12,8 @@ en: verified_account: Verified Account account_recovery_setup: piv_cac_linked: Your PIV/CAC card is linked to your account + add_info: + phone: Add a phone number cancellations: confirmation: You have cancelled verifying your identity with login.gov prompt: Are you sure you want to cancel? diff --git a/config/locales/headings/es.yml b/config/locales/headings/es.yml index 58bfd4defc4..92a0f466cfd 100644 --- a/config/locales/headings/es.yml +++ b/config/locales/headings/es.yml @@ -12,6 +12,8 @@ es: verified_account: Cuenta verificada account_recovery_setup: piv_cac_linked: Tu tarjeta PIV/CAC está vinculada a tu cuenta + add_info: + phone: Agregar un número de teléfono cancellations: confirmation: Ha cancelado la verificación de su identidad con login.gov prompt: "¿Estas seguro que quieres cancelar?" diff --git a/config/locales/headings/fr.yml b/config/locales/headings/fr.yml index 9377c7c3812..ad9ae7794ce 100644 --- a/config/locales/headings/fr.yml +++ b/config/locales/headings/fr.yml @@ -12,6 +12,8 @@ fr: verified_account: Compte vérifié account_recovery_setup: piv_cac_linked: Votre carte PIV/CAC est liée à votre compte + add_info: + phone: Ajouter un numéro de téléphone cancellations: confirmation: Vous avez annulé la vérification de votre identité avec login.gov prompt: Es-tu sûre de vouloir annuler? diff --git a/config/locales/jobs/en.yml b/config/locales/jobs/en.yml index 864b89c864a..f74ba0a79ba 100644 --- a/config/locales/jobs/en.yml +++ b/config/locales/jobs/en.yml @@ -12,6 +12,9 @@ en: in to your account. This code will expire in %{expiration} minutes." verify_message: "%{code} is your %{app} confirmation code. Use this to confirm your phone number. This code will expire in %{expiration} minutes." + sms_personal_key_regeneration_notifier_job: + message: A new personal key has been issued for your %{app} account. If this + wasn’t you, reset your password and contact us at security@login.gov. sms_personal_key_sign_in_notifier_job: - message: Your personal key was just used to sign into your login.gov account. - If this wasn’t you, reset your password and contact us at security@login.gov. + message: Your personal key was just used to sign into your %{app} account. If + this wasn’t you, reset your password and contact us at security@login.gov. diff --git a/config/locales/jobs/es.yml b/config/locales/jobs/es.yml index d2aec114d8d..8d29586838d 100644 --- a/config/locales/jobs/es.yml +++ b/config/locales/jobs/es.yml @@ -12,6 +12,9 @@ es: ingresando a su cuenta. Este código caducará en %{expiration} minutos." verify_message: "%{code} es tu código de confirmación de %{app}. Use esto para confirmar su número de teléfono. Este código caducará en %{expiration} minutos." + sms_personal_key_regeneration_notifier_job: + message: Se ha emitido una nueva clave personal para tu cuenta %{app}. Si no + eres tú, restablece tu contraseña y ponte en contacto con nosotros en security@login.gov. sms_personal_key_sign_in_notifier_job: message: Su clave personal solo se utilizó para iniciar sesión en su cuenta - login.gov. Si no fue así, reinicie su contraseña y contáctenos en security@login.gov. + %{app}. Si no fue así, reinicie su contraseña y contáctenos en security@login.gov. diff --git a/config/locales/jobs/fr.yml b/config/locales/jobs/fr.yml index a2aa952d6ab..f4302b8bc74 100644 --- a/config/locales/jobs/fr.yml +++ b/config/locales/jobs/fr.yml @@ -14,7 +14,11 @@ fr: verify_message: "%{code} est votre code de confirmation %{app}. Utilisez-le pour confirmer votre numéro de téléphone. Ce code expirera dans %{expiration} minutes." + sms_personal_key_regeneration_notifier_job: + message: Une nouvelle clé personnelle a été émise pour votre compte %{app}. + Si vous ne l'avez pas demandée, réinitialisez votre mot de passe et contactez-nous + à security@login.gov. sms_personal_key_sign_in_notifier_job: message: Votre clé personnelle a été utilisée pour vous connecter à votre compte - login.gov. Si ce n’était pas vous, changez votre mot de passe et contactez-nous + %{app}. Si ce n’était pas vous, changez votre mot de passe et contactez-nous à security@login.gov. diff --git a/config/locales/titles/en.yml b/config/locales/titles/en.yml index 6e5379493ed..6da95bf39e4 100644 --- a/config/locales/titles/en.yml +++ b/config/locales/titles/en.yml @@ -4,6 +4,8 @@ en: account: Account account_locked: Account temporarily locked account_recovery_setup: Account Recovery Setup + add_info: + phone: Add a phone number confirmations: new: Resend confirmation instructions for your account show: Choose a password diff --git a/config/locales/titles/es.yml b/config/locales/titles/es.yml index 470668e9336..fec5104b40c 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -4,6 +4,8 @@ es: account: Cuenta account_locked: Cuenta bloqueada temporalmente account_recovery_setup: Ajustes de recuperación de cuenta + add_info: + phone: Agregar un número de teléfono confirmations: new: Reenviar instrucciones de confirmación de su cuenta show: Elija una contraseña diff --git a/config/locales/titles/fr.yml b/config/locales/titles/fr.yml index 53c783c4a37..c05dfdf1fdd 100644 --- a/config/locales/titles/fr.yml +++ b/config/locales/titles/fr.yml @@ -4,6 +4,8 @@ fr: account: Compte account_locked: Compte temporairement verrouillé account_recovery_setup: Configuration de la récupération du compte + add_info: + phone: Ajouter un numéro de téléphone confirmations: new: Envoyer les instructions de confirmation pour votre compte show: Choisissez un mot de passe diff --git a/config/locales/user_mailer/en.yml b/config/locales/user_mailer/en.yml index aaf2a4491e0..55f0e59a478 100644 --- a/config/locales/user_mailer/en.yml +++ b/config/locales/user_mailer/en.yml @@ -44,6 +44,24 @@ en: help: If you did not make this change, please visit the %{app} %{help_link} or %{contact_link}. intro: You have a new password for your %{app} account. + personal_key_regenerated: + help_html:

Your login.gov account was just issued a new 16-character personal + key. You're getting this email to make sure it was you.

If you just + signed in and regenerated your personal key, great! There's nothing you need + to do.

If you did not just regenerate your personal key, or if you're + not sure, please immediately take these steps to secure your account:

  1. Change your password. Choose a password + that you haven't already used with this account.
  2. Sign + in to your login.gov account and make sure you recognize all + of the information on your account page, including the methods you use for + two-factor authentication, such as phone number, authentication app, or security + key.
  3. On your login.gov account page, + request a new personal key. Remember, never share it unless you are + using it to sign into a trusted website that uses login.gov.

You + should then contact us by calling 844-875-6446 or emailing security@login.gov.


Thanks,
The + login.gov team + intro: New personal key issued + subject: Account Security Alert personal_key_sign_in: help_html:

Your login.gov account was just signed into using your 16-character personal key. You're getting this email to make sure it was you.

If @@ -55,10 +73,10 @@ en: in to your login.gov account and make sure you recognize all of the information on your account page, including the methods you use for two-factor authentication, such as phone number, authentication app, or security - key.

  • On your login.gov account page, request a new personal - key. Remember, never share it unless you are using it to sign into - a trusted website that uses login.gov.
  • You should then contact - us by calling 844-875-6446 or emailing security@login.gov.

    + key.
  • On your login.gov account page, + request a new personal key. Remember, never share it unless you are + using it to sign into a trusted website that uses login.gov.
  • You + should then contact us by calling 844-875-6446 or emailing security@login.gov.


    Thanks,
    The login.gov team intro: Personal key used to sign in subject: Account Security Alert diff --git a/config/locales/user_mailer/es.yml b/config/locales/user_mailer/es.yml index eb4eb6a58dc..5cc4f6ba901 100644 --- a/config/locales/user_mailer/es.yml +++ b/config/locales/user_mailer/es.yml @@ -43,6 +43,27 @@ es: password_changed: help: Si no realizó este cambio, visite el %{app} %{help_link} o el %{contact_link}. intro: Tiene una contraseña nueva para su cuenta de %{app}. + personal_key_regenerated: + help_html:

    Tu cuenta de login.gov acaba de emitir una nueva clave personal + de 16 caracteres. Estás recibiendo este correo electrónico para verificar + que eras tú.

    Si acabas de iniciar sesión y has regenerado tu clave + personal, ¡fantástico! No es necesario que hagas nada.

    Si no acabas + de regenerar tu clave personal, o si no estás seguro, sigue estos pasos para + proteger tu cuenta:

    1. Cambia + tu contraseña. Elige una contraseña que aún no hayas utilizado + con esta cuenta.
    2. Inicia sesión en + tu cuenta de login.gov y asegúrate de que reconoces toda la información + de la página de tu cuenta, como los métodos que utilizas para la autenticación + de dos factores, como el número de teléfono, la aplicación de autenticación + o la clave de seguridad.
    3. En la página + de tu cuenta de login.gov, solicita una nueva clave personal. + Recuerda no compartirla nunca a menos que la estés usando para acceder a un + sitio web de confianza que utilice login.gov.

    Deberías ponerte + en contacto con nosotros llamando al 844-875-6446 o enviando un correo electrónico + a security@login.gov.


    Gracias,
    El + equipo de login.gov + intro: Nueva clave personal emitida + subject: Alerta de seguridad de la cuenta personal_key_sign_in: help_html:

    Su cuenta login.gov acaba de iniciar sesión con su clave personal de 16 caracteres. Usted está recibiendo este e-mail para asegurarse de que diff --git a/config/locales/user_mailer/fr.yml b/config/locales/user_mailer/fr.yml index 1c386e50dba..5e6e73b709c 100644 --- a/config/locales/user_mailer/fr.yml +++ b/config/locales/user_mailer/fr.yml @@ -45,6 +45,27 @@ fr: help: Si vous n'avez pas changé votre mot de passe, veuillez visiter le %{help_link} de %{app} ou %{contact_link}. intro: Le mot de passe de votre compte %{app} a été changé. + personal_key_regenerated: + help_html:

    Votre compte login.gov vient de recevoir une nouvelle clé personnelle + de 16 caractères. Le but de cet e-mail est de s'assurer que c'est bien vous + qui en êtes à l'origine.

    Si vous venez de vous connecter et de régénérer + votre clé personnelle, c'est parfait ! Vous n'avez rien à faire.

    Si + vous ne venez pas de régénérer votre clé personnelle, ou en cas de doute, + effectuez immédiatement les actions suivantes pour sécuriser votre compte :

    1. Modifiez votre mot de passe. Choisissez + un mot de passe que vous n'avez pas encore utilisé avec ce compte.
    2. Connectez-vous à votre compte login.gov + et vérifiez bien que toutes les informations sur la page de votre compte sont + correctes, y compris les méthodes que vous utilisez pour l’authentification + à deux facteurs, dont le numéro de téléphone, l’application d’authentification + ou la clé de sécurité.
    3. Sur votre page + de compte login.gov, demandez une nouvelle clé personnelle. N'oubliez + pas de ne jamais la partager, sauf si vous l'utilisez pour vous connecter + à un site de confiance qui utilise login.gov.

    Veuillez ensuite + nous contacter en appelant le 844-875-6446 ou par e-mail à security@login.gov.


    Merci,
    L'équipe + login.gov + intro: Nouvelle clé personnelle émise + subject: Alerte de sécurité du compte personal_key_sign_in: help_html:

    Votre compte login.gov a été connecté à l'aide de votre clé personnelle. Vous recevez cet email pour vous assurer que c'était bien vous.

    Si vous diff --git a/config/routes.rb b/config/routes.rb index fac1e4c448b..d08c590d510 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -137,6 +137,8 @@ match '/manage/email' => 'users/emails#update', via: %i[patch put] get '/manage/password' => 'users/passwords#edit' patch '/manage/password' => 'users/passwords#update' + get '/add/phone' => 'users/phones#add' + post '/add/phone' => 'users/phones#create' get '/manage/phone' => 'users/phones#edit' match '/manage/phone' => 'users/phones#update', via: %i[patch put] delete '/manage/phone' => 'users/phones#delete' diff --git a/lib/core_extensions/string/permit.rb b/lib/core_extensions/string/permit.rb new file mode 100644 index 00000000000..37fb341ebd4 --- /dev/null +++ b/lib/core_extensions/string/permit.rb @@ -0,0 +1,18 @@ +# When a controller uses Strong Parameters such as: +# params.require(:user).permit(:email), the `user` param is assumed to +# be a Hash, but it's easy for a pentester (for example) to set the +# `user` param to a String instead, which by default would raise a 500 +# error because the String class doesn't respond to `permit`. To get +# around that, we monkey patched the Ruby String class to raise an +# instance of ActionController::ParameterMissing, which will return +# a 400 error. 500 errors can potentially page people in the middle of +# the night, whereas 400 errors don't. +module CoreExtensions + module String + module Permit + def permit(*) + raise ActionController::ParameterMissing, '#permit called on String' + end + 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 8f745eba529..2ea0bf80d33 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -281,6 +281,8 @@ context 'user has an existing phone number' do context 'user enters a valid code' do before do + controller.user_session[:phone_id] = \ + MfaContext.new(subject.current_user).phone_configurations.last.id post( :create, params: { diff --git a/spec/controllers/users/phones_controller_spec.rb b/spec/controllers/users/phones_controller_spec.rb index 4f2cbb5b602..3ddb880700a 100644 --- a/spec/controllers/users/phones_controller_spec.rb +++ b/spec/controllers/users/phones_controller_spec.rb @@ -216,4 +216,35 @@ end end end + + context 'user adds phone' do + let(:user) { create(:user, :signed_up, with: { phone: '+1 (202) 555-1234' }) } + let(:new_phone) { '202-555-4321' } + before do + stub_sign_in(user) + + stub_analytics + allow(@analytics).to receive(:track_event) + end + + it 'gives the user a form to enter a new phone number' do + get :add + expect(response).to render_template(:add) + end + + it 'lets user know they need to confirm their new phone' do + put :create, params: { + user_phone_form: { phone: new_phone, + international_code: 'US', + otp_delivery_preference: 'sms' }, + } + expect(flash[:notice]).to eq t('devise.registrations.phone_update_needs_confirmation') + expect( + MfaContext.new(user).phone_configurations.reload.first.phone + ).to_not eq '+1 202-555-4321' + expect(response).to redirect_to(otp_send_path(otp_delivery_selection_form: + { otp_delivery_preference: 'sms' })) + expect(subject.user_session[:context]).to eq 'confirmation' + end + end end diff --git a/spec/features/account_connected_apps_spec.rb b/spec/features/account_connected_apps_spec.rb index 35a11c064cd..46faf6e274e 100644 --- a/spec/features/account_connected_apps_spec.rb +++ b/spec/features/account_connected_apps_spec.rb @@ -44,8 +44,7 @@ identity_with_link.display_name, href: 'http://localhost:3000' ) - expect(t('account.connected_apps.associated') + ' ' + identity_without_link_timestamp). - to appear_before(t('account.connected_apps.associated') + ' ' + identity_with_link_timestamp) + expect(identity_without_link_timestamp).to appear_before(identity_with_link_timestamp) end def build_account_connected_apps diff --git a/spec/features/account_reset/delete_account_spec.rb b/spec/features/account_reset/delete_account_spec.rb index ef4d0461664..fbe44556a09 100644 --- a/spec/features/account_reset/delete_account_spec.rb +++ b/spec/features/account_reset/delete_account_spec.rb @@ -32,7 +32,7 @@ expect(page).to have_content(t('account_reset.delete_account.title')) expect(page).to have_current_path(account_reset_delete_account_path) - click_on t('account_reset.delete_account.delete_button') + click_button t('account_reset.request.yes_continue') expect(page).to have_content( strip_tags( diff --git a/spec/features/two_factor_authentication/change_factor_spec.rb b/spec/features/two_factor_authentication/change_factor_spec.rb index b9d03bf7324..311f5fdfa3d 100644 --- a/spec/features/two_factor_authentication/change_factor_spec.rb +++ b/spec/features/two_factor_authentication/change_factor_spec.rb @@ -26,7 +26,7 @@ MfaContext.new(user).phone_configurations.reload.first.confirmed_at new_phone = '+1 703-555-0100' - visit manage_phone_path + visit manage_phone_path(id: user.phone_configurations.first.id) expect(page).to have_content t('help_text.change_factor', factor: 'phone') diff --git a/spec/features/users/regenerate_personal_key_spec.rb b/spec/features/users/regenerate_personal_key_spec.rb index 88f9887669c..1bceac55cd6 100644 --- a/spec/features/users/regenerate_personal_key_spec.rb +++ b/spec/features/users/regenerate_personal_key_spec.rb @@ -5,12 +5,18 @@ include PersonalKeyHelper include SamlAuthHelper + before { stub_twilio_service } + context 'during sign up' do - scenario 'user refreshes personal key page' do + scenario 'refreshing personal key page displays the same key and does not notify the user' do sign_up_and_view_personal_key personal_key = scrape_personal_key + # The user should not receive an SMS and an email + expect(UserMailer).to_not receive(:personal_key_regenerated) + expect(SmsPersonalKeyRegenerationNotifierJob).to_not receive(:perform_now) + visit sign_up_personal_key_path expect(current_path).to eq(sign_up_personal_key_path) @@ -24,10 +30,19 @@ context 'after sign up' do context 'regenerating personal key' do - scenario 'displays new code' do + scenario 'displays new code and notifies the user' do user = sign_in_and_2fa_user old_digest = user.encrypted_recovery_code_digest + # The user should receive an SMS and an email + personal_key_sign_in_mail = double + expect(personal_key_sign_in_mail).to receive(:deliver_now) + expect(UserMailer).to receive(:personal_key_regenerated). + with(user.email). + and_return(personal_key_sign_in_mail) + expect(SmsPersonalKeyRegenerationNotifierJob).to receive(:perform_now). + with(phone: user.phone_configurations.first.phone) + click_button t('account.links.regenerate_personal_key') expect(user.reload.encrypted_recovery_code_digest).to_not eq old_digest diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 5f36a1173a9..31a6dd3a34e 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -396,7 +396,7 @@ expect(VoiceOtpSenderJob).to_not have_received(:perform_later) expect(SmsOtpSenderJob).to have_received(:perform_later).exactly(:once) expect(page). - to have_current_path(login_two_factor_path(otp_delivery_preference: 'sms')) + to have_current_path(login_two_factor_path(otp_delivery_preference: 'sms', reauthn: false)) expect(page).to have_content t( 'two_factor_authentication.otp_delivery_preference.phone_unsupported', location: 'India' diff --git a/spec/features/users/user_edit_spec.rb b/spec/features/users/user_edit_spec.rb index cc8a4d59f78..0942096977b 100644 --- a/spec/features/users/user_edit_spec.rb +++ b/spec/features/users/user_edit_spec.rb @@ -89,6 +89,7 @@ click_button t('forms.phone.buttons.delete') expect(page).to have_current_path(account_path) expect(MfaPolicy.new(user.reload).multiple_factors_enabled?).to eq false + expect(page).to have_content t('event_types.phone_removed') end end end diff --git a/spec/jobs/sms_personal_key_regeneration_notifier_job_spec.rb b/spec/jobs/sms_personal_key_regeneration_notifier_job_spec.rb new file mode 100644 index 00000000000..68a1763527b --- /dev/null +++ b/spec/jobs/sms_personal_key_regeneration_notifier_job_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +describe SmsPersonalKeyRegenerationNotifierJob do + include Features::ActiveJobHelper + + before do + reset_job_queues + TwilioService::Utils.telephony_service = FakeSms + FakeSms.messages = [] + end + + describe '.perform' do + it 'sends a message about the personal key sign in to the user' do + allow(Figaro.env).to receive(:twilio_messaging_service_sid).and_return('fake_sid') + + described_class.perform_now(phone: '+1 (202) 345-6789') + + messages = FakeSms.messages + + expect(messages.size).to eq(1) + + msg = messages.first + + expect(msg.messaging_service_sid).to eq('fake_sid') + expect(msg.to).to eq('+1 (202) 345-6789') + expect(msg.body). + to eq(I18n.t('jobs.sms_personal_key_regeneration_notifier_job.message', app: APP_NAME)) + end + end +end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 2ca18021e43..91a91a4fb4d 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -66,6 +66,26 @@ end end + describe 'personal_key_regenerated' do + let(:mail) { UserMailer.personal_key_regenerated(user.email) } + + it_behaves_like 'a system email' + + it 'sends to the current email' do + expect(mail.to).to eq [user.email] + end + + it 'renders the subject' do + expect(mail.subject).to eq t('user_mailer.personal_key_regenerated.subject') + end + + it 'renders the body' do + expect(mail.html_part.body).to have_content( + t('user_mailer.personal_key_regenerated.intro') + ) + end + end + describe 'signup_with_your_email' do let(:mail) { UserMailer.signup_with_your_email(user.email) } diff --git a/spec/presenters/two_factor_authentication/webauthn_selection_presenter_spec.rb b/spec/presenters/two_factor_authentication/webauthn_selection_presenter_spec.rb index 74457bdb301..1743d1e9e4d 100644 --- a/spec/presenters/two_factor_authentication/webauthn_selection_presenter_spec.rb +++ b/spec/presenters/two_factor_authentication/webauthn_selection_presenter_spec.rb @@ -9,4 +9,10 @@ expect(subject.type).to eq 'webauthn' end end + + describe '#html_class' do + it 'returns hidden' do + expect(subject.html_class).to eq 'hidden' + end + end end diff --git a/spec/presenters/utc_time_presenter_spec.rb b/spec/presenters/utc_time_presenter_spec.rb index 9eea502daeb..9aa80e5ca09 100644 --- a/spec/presenters/utc_time_presenter_spec.rb +++ b/spec/presenters/utc_time_presenter_spec.rb @@ -10,7 +10,7 @@ Time.zone = current_timezone expect(UtcTimePresenter.new(timestamp).to_s).to eq( - 'April 12, 2017 at 6:19 PM UTC' + 'April 12, 2017 at 6:19 PM' ) end end diff --git a/spec/requests/invalid_sign_in_params_spec.rb b/spec/requests/invalid_sign_in_params_spec.rb index c4721221143..b42a0196ea2 100644 --- a/spec/requests/invalid_sign_in_params_spec.rb +++ b/spec/requests/invalid_sign_in_params_spec.rb @@ -1,8 +1,12 @@ require 'rails_helper' describe 'visiting sign in page with invalid user params' do - it 'does not raise an exception' do - get new_user_session_path, params: { user: 'test@test.com' } + it 'raises ActionController::ParameterMissing' do + params = { user: 'test@test.com' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { get new_user_session_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) end end diff --git a/spec/requests/params_is_string_instead_of_hash_spec.rb b/spec/requests/params_is_string_instead_of_hash_spec.rb new file mode 100644 index 00000000000..8c265907ce8 --- /dev/null +++ b/spec/requests/params_is_string_instead_of_hash_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +# When a controller uses Strong Parameters such as: +# params.require(:user).permit(:email), the `user` param is assumed to +# be a Hash, but it's easy for a pentester (for example) to set the +# `user` param to a String instead, which by default would raise a 500 +# error because the String class doesn't respond to `permit`. To get +# around that, we monkey patched the Ruby String class to raise an +# instance of ActionController::ParameterMissing, which will return +# a 400 error. 500 errors can potentially page people in the middle of +# the night, whereas 400 errors don't. +describe 'submitting email resend form with required param as String' do + it 'raises ActionController::ParameterMissing' do + params = { resend_email_confirmation_form: 'abcdef' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { post sign_up_create_email_resend_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) + end +end + +describe 'submitting email registration form with required param as String' do + it 'raises ActionController::ParameterMissing' do + params = { user: 'abcdef' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { post sign_up_register_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) + end +end diff --git a/spec/services/otp_delivery_preference_updater_spec.rb b/spec/services/otp_delivery_preference_updater_spec.rb index aa48a0e1e1b..e21f9bb44ec 100644 --- a/spec/services/otp_delivery_preference_updater_spec.rb +++ b/spec/services/otp_delivery_preference_updater_spec.rb @@ -5,7 +5,7 @@ OtpDeliveryPreferenceUpdater.new( user: build_stubbed(:user, otp_delivery_preference: 'sms'), preference: 'sms', - context: 'authentication' + phone_id: 1 ) end @@ -25,7 +25,7 @@ updater = OtpDeliveryPreferenceUpdater.new( user: user, preference: 'sms', - context: 'authentication' + phone_id: 1 ) attributes = { otp_delivery_preference: 'sms' } @@ -45,7 +45,7 @@ updater = OtpDeliveryPreferenceUpdater.new( user: nil, preference: 'sms', - context: 'idv' + phone_id: 1 ) expect(UpdateUser).to_not receive(:new) diff --git a/spec/services/phone_verification_spec.rb b/spec/services/phone_verification_spec.rb index b771ba20d05..a67a0ec43d7 100644 --- a/spec/services/phone_verification_spec.rb +++ b/spec/services/phone_verification_spec.rb @@ -43,8 +43,14 @@ PhoneVerification.adapter = Faraday.new(url: PhoneVerification::AUTHY_HOST) locale = 'fr' - body = "code_length=6&country_code=1&custom_code=#{code}&locale=#{locale}&" \ - "phone_number=7035551212&via=sms" + body = { + code_length: '6', + country_code: '1', + custom_code: code, + locale: locale, + phone_number: '7035551212', + via: 'sms', + } stub_request(:post, 'https://api.authy.com/protected/json/phones/verification/start'). with( @@ -53,7 +59,7 @@ 'Accept' => '*/*', 'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Content-Type' => 'application/x-www-form-urlencoded', - 'User-Agent' => 'Faraday v0.15.2', + 'User-Agent' => 'Faraday v0.15.3', 'X-Authy-Api-Key' => Figaro.env.twilio_verify_api_key, } ). diff --git a/spec/services/update_user_spec.rb b/spec/services/update_user_spec.rb index cd55785b766..59776123cdf 100644 --- a/spec/services/update_user_spec.rb +++ b/spec/services/update_user_spec.rb @@ -16,21 +16,6 @@ context 'with a phone already configured' do let(:user) { create(:user, :with_phone) } - it 'updates the phone configuration' do - confirmed_at = 1.day.ago.change(usec: 0) - attributes = { - otp_delivery_preference: 'voice', - phone: '+1 222 333-4444', - phone_confirmed_at: confirmed_at, - } - updater = UpdateUser.new(user: user, attributes: attributes) - updater.call - phone_configuration = user.phone_configurations.reload.first - expect(phone_configuration.delivery_preference).to eq 'voice' - expect(phone_configuration.confirmed_at).to eq confirmed_at - expect(phone_configuration.phone).to eq '+1 222 333-4444' - end - it 'does not delete the phone configuration' do attributes = { phone: nil } updater = UpdateUser.new(user: user, attributes: attributes) diff --git a/spec/views/account_reset/delete_account/show.html.slim_spec.rb b/spec/views/account_reset/delete_account/show.html.slim_spec.rb index bd5573e0aba..eb22b80c76b 100644 --- a/spec/views/account_reset/delete_account/show.html.slim_spec.rb +++ b/spec/views/account_reset/delete_account/show.html.slim_spec.rb @@ -13,6 +13,6 @@ it 'has button to delete' do render - expect(rendered).to have_button t('account_reset.delete_account.delete_button') + expect(rendered).to have_button t('account_reset.request.yes_continue') end end diff --git a/spec/views/accounts/show.html.slim_spec.rb b/spec/views/accounts/show.html.slim_spec.rb index 8d598c732df..be4c5c10cde 100644 --- a/spec/views/accounts/show.html.slim_spec.rb +++ b/spec/views/accounts/show.html.slim_spec.rb @@ -166,18 +166,18 @@ render expect(rendered).to have_link( - t('account.index.phone_add'), href: manage_phone_path + t('account.index.phone_add'), href: add_phone_path ) end end context 'user has a phone' do - it 'shows no add phone link' do + it 'shows add phone link' do render - expect(rendered).to_not have_content t('account.index.phone_add') - expect(rendered).to_not have_link( - t('account.index.phone_add'), href: manage_phone_path + expect(rendered).to have_content t('account.index.phone_add') + expect(rendered).to have_link( + t('account.index.phone_add'), href: add_phone_path ) end @@ -185,7 +185,7 @@ render expect(rendered).to have_link( - t('account.index.phone'), href: manage_phone_url + t('account.index.phone'), href: manage_phone_url(id: user.phone_configurations.first.id) ) end end