diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 6ee07393260..b447dd6b50f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -19,7 +19,7 @@ will warn you about unsafe migrations and has great step-by-step instructions for various scenarios. - [ ] Indexes were added if necessary. This article provides a good overview -of [indexes in Rails](https://semaphoreci.com/blog/2017/05/09/faster-rails-is-your-database-properly-indexed.html). +of [indexes in Rails](https://goo.gl/1DARYi). - [ ] Verified that the changes don't affect other apps (such as the dashboard) diff --git a/.reek b/.reek index 3276cc4e715..7a05f1eb0f0 100644 --- a/.reek +++ b/.reek @@ -92,6 +92,7 @@ TooManyInstanceVariables: - OpenidConnectRedirector - Idv::VendorResult - CloudhsmKeyGenerator + - CloudhsmKeySharer TooManyStatements: max_statements: 6 exclude: @@ -125,6 +126,7 @@ TooManyMethods: - SessionDecorator - HolidayService - PhoneDeliveryPresenter + - CloudhsmKeyGenerator UncommunicativeMethodName: exclude: - PhoneConfirmationFlow diff --git a/.rubocop.yml b/.rubocop.yml index d7d103d6832..3d06815482e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -64,6 +64,7 @@ Metrics/ClassLength: - app/services/analytics.rb - app/services/idv/session.rb - app/presenters/two_factor_auth_code/phone_delivery_presenter.rb + - lib/cloudhsm/cloudhsm_key_generator.rb Metrics/LineLength: Description: Limit lines to 100 characters. diff --git a/Gemfile.lock b/Gemfile.lock index d2cfe118289..3e1572c5a97 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -138,13 +138,13 @@ GEM arel (8.0.0) ast (2.4.0) aws-eventstream (1.0.1) - aws-partitions (1.96.0) - aws-sdk-core (3.22.1) + aws-partitions (1.97.0) + aws-sdk-core (3.24.0) aws-eventstream (~> 1.0) aws-partitions (~> 1.0) aws-sigv4 (~> 1.0) jmespath (~> 1.0) - aws-sdk-kms (1.6.0) + aws-sdk-kms (1.7.0) aws-sdk-core (~> 3) aws-sigv4 (~> 1.0) aws-sdk-s3 (1.17.0) @@ -208,7 +208,7 @@ GEM descendants_tracker (~> 0.0.1) colorize (0.8.1) concurrent-ruby (1.0.5) - connection_pool (2.2.1) + connection_pool (2.2.2) crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.4) @@ -217,6 +217,7 @@ GEM daemons (1.2.4) database_cleaner (1.7.0) debug_inspector (0.0.3) + deepl-rb (2.1.0) derailed (0.1.0) derailed_benchmarks derailed_benchmarks (1.3.4) @@ -308,7 +309,7 @@ GEM hashdiff (0.3.7) hashie (3.5.7) heapy (0.1.3) - highline (1.7.10) + highline (2.0.0) hiredis (0.6.1) htmlentities (4.3.4) http_accept_language (2.1.1) @@ -317,11 +318,12 @@ GEM httpi (2.4.3) rack socksify - i18n (1.0.1) + i18n (1.1.0) concurrent-ruby (~> 1.0) - i18n-tasks (0.9.21) + i18n-tasks (0.9.23) activesupport (>= 4.0.2) ast (>= 2.1.0) + deepl-rb (>= 2.1.0) easy_translate (>= 0.5.1) erubi highline (>= 1.7.3) @@ -334,7 +336,7 @@ GEM io-like (0.3.0) jaro_winkler (1.5.1) jmespath (1.4.0) - json (1.8.6) + json (2.1.0) json-jwt (1.9.4) activesupport aes_key_wrap @@ -390,7 +392,7 @@ GEM parser (2.5.1.2) ast (~> 2.4.0) pg (1.0.0) - phonelib (0.6.23) + phonelib (0.6.24) pkcs11 (0.2.7) powerpack (0.1.2) premailer (1.11.1) @@ -466,7 +468,7 @@ GEM readthis (2.2.0) connection_pool (~> 2.1) redis (>= 3.0, < 5.0) - recaptcha (4.8.0) + recaptcha (4.11.1) json redis (3.3.5) reek (4.8.1) @@ -512,7 +514,7 @@ GEM ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) ruby-graphviz (1.2.3) - ruby-progressbar (1.9.0) + ruby-progressbar (1.10.0) ruby-saml (1.8.0) nokogiri (>= 1.5.10) ruby_dep (1.5.0) @@ -551,9 +553,8 @@ GEM shellany (0.0.1) shoulda-matchers (3.1.2) activesupport (>= 4.0.0) - sidekiq (5.1.3) - concurrent-ruby (~> 1.0) - connection_pool (~> 2.2, >= 2.2.0) + sidekiq (5.2.1) + connection_pool (~> 2.2, >= 2.2.2) rack-protection (>= 1.5.0) redis (>= 3.3.5, < 5) simple_form (4.0.1) @@ -606,7 +607,7 @@ GEM thread_safe (0.3.6) tilt (2.0.8) timecop (0.9.1) - twilio-ruby (5.11.1) + twilio-ruby (5.12.1) faraday (~> 0.9) jwt (>= 1.5, <= 2.5) nokogiri (>= 1.6, < 2.0) diff --git a/app/controllers/account_reset/cancel_controller.rb b/app/controllers/account_reset/cancel_controller.rb index 8cbe8659e7f..44f66f8cb90 100644 --- a/app/controllers/account_reset/cancel_controller.rb +++ b/app/controllers/account_reset/cancel_controller.rb @@ -1,30 +1,20 @@ module AccountReset class CancelController < ApplicationController - def cancel - account_reset = AccountResetService.cancel_request(params[:token]) - if account_reset - handle_success(account_reset.user) - else - handle_failure - end + def create + result = AccountReset::Cancel.new(params[:token]).call + + analytics.track_event(Analytics::ACCOUNT_RESET, result.to_h) + + handle_success if result.success? + redirect_to root_url end private - def handle_success(user) - analytics.track_event(Analytics::ACCOUNT_RESET, - event: :cancel, token_valid: true, user_id: user.uuid) + def handle_success sign_out if current_user - UserMailer.account_reset_cancel(user.email).deliver_later - phone = user.phone - SmsAccountResetCancellationNotifierJob.perform_now(phone: phone) if phone.present? flash[:success] = t('devise.two_factor_authentication.account_reset.successful_cancel') end - - def handle_failure - return if params[:token].blank? - analytics.track_event(Analytics::ACCOUNT_RESET, event: :cancel, token_valid: false) - end end end diff --git a/app/controllers/account_reset/request_controller.rb b/app/controllers/account_reset/request_controller.rb index 0eb7e428faa..3b16b6d6f7f 100644 --- a/app/controllers/account_reset/request_controller.rb +++ b/app/controllers/account_reset/request_controller.rb @@ -4,6 +4,7 @@ class RequestController < ApplicationController before_action :check_account_reset_enabled before_action :confirm_two_factor_enabled + before_action :confirm_user_not_verified def show; end @@ -21,6 +22,11 @@ def check_account_reset_enabled redirect_to root_url unless FeatureManagement.account_reset_enabled? end + def confirm_user_not_verified + # IAL2 users should not be able to reset account to comply with AAL2 reqs + redirect_to account_url if decorated_user.identity_verified? + end + def reset_session_with_email email = current_user.email sign_out diff --git a/app/controllers/health/health_controller.rb b/app/controllers/health/health_controller.rb index 0fbcb5eabbd..d9fd4f5fec4 100644 --- a/app/controllers/health/health_controller.rb +++ b/app/controllers/health/health_controller.rb @@ -6,6 +6,7 @@ def health_checker checkers = { database: DatabaseHealthChecker, workers: WorkerHealthChecker, + account_reset: AccountResetHealthChecker, } # Don't run worker health checks if we're not using workers (i.e. if the # queue adapter is inline or async) diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 3d499416659..c0be8f8155c 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -62,8 +62,6 @@ def identity_needs_verification? end def build_authorize_form_from_params - user_session[:openid_auth_request] = authorization_params if user_session - @authorize_form = OpenidConnectAuthorizeForm.new(authorization_params) @authorize_decorator = OpenidConnectAuthorizeDecorator.new( diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 3372aae0914..11e7019ed04 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -38,13 +38,14 @@ def active def timeout analytics.track_event(Analytics::SESSION_TIMED_OUT) + request_id = sp_session[:request_id] sign_out flash[:notice] = t( 'session_timedout', app: APP_NAME, minutes: Figaro.env.session_timeout_in_minutes ) - redirect_to root_url + redirect_to root_url(request_id: request_id) end private diff --git a/app/decorators/identity_decorator.rb b/app/decorators/identity_decorator.rb index f652ea53921..63c65cc6eeb 100644 --- a/app/decorators/identity_decorator.rb +++ b/app/decorators/identity_decorator.rb @@ -10,6 +10,10 @@ def event_partial 'accounts/identity_item' end + def failure_to_proof_url + identity.sp_metadata[:failure_to_proof_url] + end + def return_to_sp_url identity.sp_metadata[:return_to_sp_url] end diff --git a/app/models/null_service_provider.rb b/app/models/null_service_provider.rb index 2998c226224..39e0f7a4adf 100644 --- a/app/models/null_service_provider.rb +++ b/app/models/null_service_provider.rb @@ -29,6 +29,8 @@ def logo; end def friendly_name; end + def failure_to_proof_url; end + def return_to_sp_url; end def redirect_uris diff --git a/app/presenters/two_factor_login_options_presenter.rb b/app/presenters/two_factor_login_options_presenter.rb index ae06a833f18..5e2cb40dd78 100644 --- a/app/presenters/two_factor_login_options_presenter.rb +++ b/app/presenters/two_factor_login_options_presenter.rb @@ -45,6 +45,11 @@ def options end end + def should_display_account_reset_or_cancel_link? + # IAL2 users should not be able to reset account to comply with AAL2 reqs + !current_user.decorate.identity_verified? + end + def account_reset_or_cancel_link account_reset_token_valid? ? account_reset_cancel_link : account_reset_link end diff --git a/app/services/account_reset/cancel.rb b/app/services/account_reset/cancel.rb new file mode 100644 index 00000000000..77bba914934 --- /dev/null +++ b/app/services/account_reset/cancel.rb @@ -0,0 +1,67 @@ +module AccountReset + class Cancel + include ActiveModel::Model + + validates :token, presence: { message: I18n.t('errors.account_reset.cancel_token_missing') } + validate :valid_token + + def initialize(token) + @token = token + end + + def call + @success = valid? + + if success + notify_user_via_email_of_account_reset_cancellation + notify_user_via_phone_of_account_reset_cancellation if phone.present? + update_account_reset_request + end + + FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes) + end + + private + + attr_reader :success, :token + + def valid_token + return if account_reset_request + + errors.add(:token, I18n.t('errors.account_reset.cancel_token_invalid')) if token + end + + def notify_user_via_email_of_account_reset_cancellation + UserMailer.account_reset_cancel(user.email).deliver_later + end + + def notify_user_via_phone_of_account_reset_cancellation + SmsAccountResetCancellationNotifierJob.perform_now(phone: phone) + end + + def update_account_reset_request + account_reset_request.update!(cancelled_at: Time.zone.now, + request_token: nil, + granted_token: nil) + end + + def account_reset_request + @account_reset_request ||= AccountResetRequest.find_by(request_token: token) + end + + def user + account_reset_request&.user || AnonymousUser.new + end + + def phone + user.phone + end + + def extra_analytics_attributes + { + event: 'cancel', + user_id: user.uuid, + } + end + end +end diff --git a/app/services/account_reset_health_checker.rb b/app/services/account_reset_health_checker.rb new file mode 100644 index 00000000000..4a19670e392 --- /dev/null +++ b/app/services/account_reset_health_checker.rb @@ -0,0 +1,35 @@ +module AccountResetHealthChecker + module_function + + Summary = Struct.new(:healthy, :result) do + def as_json(*args) + to_h.as_json(*args) + end + + alias_method :healthy?, :healthy + end + + # @return [Summary] + def check + rec = find_request_not_serviced_within_26_hours + Summary.new(rec.nil?, rec) + end + + # @api private + def find_request_not_serviced_within_26_hours + records = AccountResetRequest.where( + sql, tvalue: Time.zone.now - Figaro.env.account_reset_wait_period_days.to_i.days - 2.hours + ).order('requested_at ASC').limit(1) + records.first + end + + def sql + <<~SQL + cancelled_at IS NULL AND + granted_at IS NULL AND + requested_at < :tvalue AND + request_token IS NOT NULL AND + granted_token IS NULL + SQL + end +end diff --git a/app/services/account_reset_service.rb b/app/services/account_reset_service.rb index 901bdc5ee18..43e5fad94cb 100644 --- a/app/services/account_reset_service.rb +++ b/app/services/account_reset_service.rb @@ -12,15 +12,6 @@ def create_request granted_token: nil) end - def self.cancel_request(token) - account_reset = token.blank? ? nil : AccountResetRequest.find_by(request_token: token) - return false unless account_reset - account_reset.update(cancelled_at: Time.zone.now, - request_token: nil, - granted_token: nil) - account_reset - end - def self.report_fraud(token) account_reset = token.blank? ? nil : AccountResetRequest.find_by(request_token: token) return false unless account_reset @@ -67,7 +58,7 @@ def self.send_notifications_with_sql(users_sql) def self.reset_and_notify(arr) user = arr.user return false unless AccountResetService.new(user).grant_request - UserMailer.account_reset_granted(user, arr).deliver_later + UserMailer.account_reset_granted(user, arr.reload).deliver_later true end private_class_method :reset_and_notify diff --git a/app/services/populate_phone_configurations_table.rb b/app/services/populate_phone_configurations_table.rb new file mode 100644 index 00000000000..672e7aa85d2 --- /dev/null +++ b/app/services/populate_phone_configurations_table.rb @@ -0,0 +1,28 @@ +class PopulatePhoneConfigurationsTable + def call + # we don't have a uniqueness constraint in the database to let us blindly insert + # everything in a single SQL statement. So we have to load by batches and copy + # over. Much slower, but doesn't duplicate information. + User.in_batches(of: 1000) { |relation| process_batch(relation) } + end + + private + + # :reek:FeatureEnvy + def process_batch(relation) + User.transaction do + relation.each do |user| + next if user.phone_configuration.present? || user.phone.blank? + user.create_phone_configuration(phone_info_for_user(user)) + end + end + end + + def phone_info_for_user(user) + { + phone: user.phone, + confirmed_at: user.phone_confirmed_at, + delivery_preference: user.otp_delivery_preference, + } + end +end diff --git a/app/views/exception_notifier/_session.text.erb b/app/views/exception_notifier/_session.text.erb index 496e1aa9abc..3446c186bdd 100644 --- a/app/views/exception_notifier/_session.text.erb +++ b/app/views/exception_notifier/_session.text.erb @@ -16,7 +16,7 @@ Referer: <%= @request.referer %> <% session['user_return_to'] = session['user_return_to']&.split('?')&.first %> Session: <%= session %> -<% user = @kontroller.analytics_user %> +<% user = @kontroller.analytics_user || AnonymousUser.new %> User UUID: <%= user.uuid %> User's Country (based on phone): <%= Phonelib.parse(user.phone).country %> diff --git a/app/views/two_factor_authentication/options/index.html.slim b/app/views/two_factor_authentication/options/index.html.slim index 75c6c1f3ed5..a035884faae 100644 --- a/app/views/two_factor_authentication/options/index.html.slim +++ b/app/views/two_factor_authentication/options/index.html.slim @@ -23,5 +23,6 @@ p.mt-tiny.mb3 = @presenter.info = f.button :submit, t('forms.buttons.continue') br -p = @presenter.account_reset_or_cancel_link +- if @presenter.should_display_account_reset_or_cancel_link? + p = @presenter.account_reset_or_cancel_link = render 'shared/cancel', link: destroy_user_session_path diff --git a/bin/release b/bin/release index ab0ebc620ed..76ccacbb58e 100755 --- a/bin/release +++ b/bin/release @@ -98,8 +98,7 @@ def deploy_to_int Dir.chdir "#{ENV['HOME']}/login-dot-gov/identity-devops" do puts "Deploying the `stages/int` branch to the `int` environment" ENV['AWS_PROFILE'] = 'identitysandbox.gov' - run "bin/asg-recycle.sh int idp" - run "bin/asg-recycle.sh int worker" + run "bin/asg-recycle int idp" end remove_login_dot_gov_dir @@ -111,8 +110,7 @@ def deploy_to_staging Dir.chdir "#{ENV['HOME']}/login-dot-gov/identity-devops" do puts "Deploying the `stages/staging` branch to the `staging` environment" ENV['AWS_PROFILE'] = 'login.gov' - run "bin/asg-recycle.sh staging idp" - run "bin/asg-recycle.sh staging worker" + run "bin/asg-recycle staging idp" end remove_login_dot_gov_dir @@ -124,8 +122,7 @@ def deploy_to_prod Dir.chdir "#{ENV['HOME']}/login-dot-gov/identity-devops" do puts "Deploying the `stages/prod` branch to the `prod` environment" ENV['AWS_PROFILE'] = 'login.gov' - run "bin/asg-recycle.sh prod idp" - run "bin/asg-recycle.sh prod worker" + run "bin/asg-recycle prod idp" end remove_login_dot_gov_dir diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 45f350fd4ee..0ec588520c6 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -1,6 +1,9 @@ --- en: errors: + account_reset: + cancel_token_invalid: cancel token is invalid + cancel_token_missing: cancel token is missing confirm_password_incorrect: Incorrect password. invalid_authenticity_token: Oops, something went wrong. Please try again. invalid_totp: Invalid code. Please try again. diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index 70f7cd81f22..303a54eaa22 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -1,6 +1,9 @@ --- es: errors: + account_reset: + cancel_token_invalid: NOT TRANSLATED YET + cancel_token_missing: NOT TRANSLATED YET confirm_password_incorrect: La contraseña es incorrecta. invalid_authenticity_token: "¡Oops! Algo salió mal. Inténtelo de nuevo." invalid_totp: El código es inválido. Vuelva a intentarlo. diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index b6615b07d3b..038d0fd5486 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -1,6 +1,9 @@ --- fr: errors: + account_reset: + cancel_token_invalid: NOT TRANSLATED YET + cancel_token_missing: NOT TRANSLATED YET confirm_password_incorrect: Mot de passe incorrect. invalid_authenticity_token: Oups, une erreur s'est produite. Veuillez essayer de nouveau. diff --git a/config/routes.rb b/config/routes.rb index de8be2635cd..0317f73f01d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -58,7 +58,7 @@ get '/account_reset/request' => 'account_reset/request#show' post '/account_reset/request' => 'account_reset/request#create' - get '/account_reset/cancel' => 'account_reset/cancel#cancel' + get '/account_reset/cancel' => 'account_reset/cancel#create' get '/account_reset/report_fraud' => 'account_reset/report_fraud#update' get '/account_reset/confirm_request' => 'account_reset/confirm_request#show' get '/account_reset/delete_account' => 'account_reset/delete_account#show' diff --git a/config/service_providers.yml b/config/service_providers.yml index d853dc94075..d9d1a4da731 100644 --- a/config/service_providers.yml +++ b/config/service_providers.yml @@ -23,6 +23,7 @@ test: assertion_consumer_logout_service_url: 'http://example.com/test/saml/decode_slo_request' block_encryption: 'aes256-cbc' sp_initiated_login_url: 'https://example.com/auth/saml/login' + failure_to_proof_url: 'https://example.com/' friendly_name: 'Test SP' cert: 'saml_test_sp' logo: 'generic.svg' @@ -705,6 +706,7 @@ production: - 'https://fossil.energy.gov/fergas-fe/#/certify_redirect' - 'https://fossil.energy.gov/fergas-fe/#/login' - 'https://fossil.energy.gov/fergas-fe/#/login_redirect' + - 'https://fossil.energy.gov/fergas-fe/#/notice/13' - 'https://fossil.energy.gov/fergas-fe/#/notice/15' restrict_to_deploy_env: 'prod' diff --git a/db/migrate/20180728122856_add_failure_to_proof_url_to_service_provider.rb b/db/migrate/20180728122856_add_failure_to_proof_url_to_service_provider.rb new file mode 100644 index 00000000000..70b471ec641 --- /dev/null +++ b/db/migrate/20180728122856_add_failure_to_proof_url_to_service_provider.rb @@ -0,0 +1,9 @@ +class AddFailureToProofUrlToServiceProvider < ActiveRecord::Migration[5.1] + def up + add_column :service_providers, :failure_to_proof_url, :text + end + + def down + remove_column :service_providers, :failure_to_proof_url + end +end diff --git a/db/schema.rb b/db/schema.rb index c87de1d133b..32d03f67203 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180720152009) do +ActiveRecord::Schema.define(version: 20180728122856) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -179,6 +179,7 @@ t.boolean "native", default: false, null: false t.string "redirect_uris", default: [], array: true t.integer "agency_id" + t.text "failure_to_proof_url" t.index ["issuer"], name: "index_service_providers_on_issuer", unique: true end diff --git a/lib/cloudhsm/cloudhsm_key_generator.rb b/lib/cloudhsm/cloudhsm_key_generator.rb index 6f82cda8847..6040f93c2fc 100644 --- a/lib/cloudhsm/cloudhsm_key_generator.rb +++ b/lib/cloudhsm/cloudhsm_key_generator.rb @@ -1,38 +1,45 @@ require 'greenletters' require 'io/console' -# Can be run standalone (without idp or rails) or through the rake task cloudhsm -# generates saml_.key, saml_.crt, +# In prod this script is run through the cloudhsm rake task which will run on a RamDisk +# generates saml_.key, saml_.crt +# saml_.scr (cached credentials and key handle for the key_sharer script) # and saml_.txt (a transcript of the cloudhsm interaction) -# the program interactively asks for username, password, and openssl.conf location +# the program interactively asks for username, password (hidden), idp username, +# and openssl.conf location class CloudhsmKeyGenerator KEY_MGMT_UTIL = '/opt/cloudhsm/bin/key_mgmt_util'.freeze def initialize @username, @password, @openssl_conf, @timestamp = initialize_settings - output = File.open("saml_#{@timestamp}.txt", 'w') - @kmu = Greenletters::Process.new(KEY_MGMT_UTIL, transcript: output) - @kmu.start! - @kmu.wait_for(:output, /Command:/) + @idp_username = prompt_for_idp_username + @kmu = run_key_mgmt_util + wait_for_command_to_finish end def generate_saml_key saml_label = create_key_and_crt_files login_to_hsm - wrapping_key_handle = generate_symmetric_wrapping_key - import_private_key(saml_label, wrapping_key_handle) + key_handle = import_private_key(saml_label) exit_hsm - saml_label + cache_credentials_and_private_key_handle(key_handle) + [saml_label, key_handle] end - def cleanup - File.delete("saml_#{@timestamp}.crt") - File.delete("saml_#{@timestamp}.txt") - File.delete("saml_#{@timestamp}.key") + private + + def import_private_key(saml_label) + wrapping_key_handle = generate_symmetric_wrapping_key + import_wrapped_key(saml_label, wrapping_key_handle) end - private + def run_key_mgmt_util + output = File.open("saml_#{@timestamp}.txt", 'w') + kmu = Greenletters::Process.new(KEY_MGMT_UTIL, transcript: output) + kmu.start! + kmu + end def initialize_settings username, password = prompt_for_username_and_password @@ -56,13 +63,13 @@ def create_key_and_crt_files def login_to_hsm @kmu << "loginHSM -u CU -s #{@username} -p #{@password}\n" @kmu.wait_for(:output, /SUCCESS/) + wait_for_command_to_finish end def generate_symmetric_wrapping_key @kmu << "genSymKey -t 31 -s 16 -sess -l wrapping_key_for_import\n" - wrapping_key_handle = wait_for_wrapping_key_handle @kmu.wait_for(:output, /SUCCESS/) - wrapping_key_handle + wait_for_wrapping_key_handle end def wait_for_wrapping_key_handle @@ -71,16 +78,28 @@ def wait_for_wrapping_key_handle matching.matched =~ /Key Handle: (\d+)/ wrapping_key_handle = Regexp.last_match[1] end + wait_for_command_to_finish wrapping_key_handle end - def import_private_key(saml_label, wrapping_key_handle) + def import_wrapped_key(saml_label, wrapping_key_handle) @kmu << "importPrivateKey -f #{saml_label}.key -l #{saml_label} -w #{wrapping_key_handle}\n" @kmu.wait_for(:output, /SUCCESS/) + wait_for_private_key_handle + end + + def wait_for_private_key_handle + key_handle = nil + @kmu.wait_for(:output, /Key Handle: \d+/) do |_process, matching| + matching.matched =~ /Key Handle: (\d+)/ + key_handle = Regexp.last_match[1] + end + wait_for_command_to_finish + key_handle end def exit_hsm - @kmu << 'exit' + @kmu << "exit\n" Kernel.puts "Certificate written to 'saml_#{@timestamp}.crt'.\n" \ "Transcript written to 'saml_#{@timestamp}.txt'.\n" \ "Key generation complete.\n" @@ -94,16 +113,61 @@ def prompt_for_username_and_password [username, password] end + def prompt_for_idp_username + Kernel.puts 'Please enter the CloudHSM username used by the IDP' + user_input + end + def prompt_for_openssl_conf Kernel.puts 'Please enter the full path to openssl.conf' openssl_conf = user_input - raise 'openssl.conf not found' unless File.exist?(openssl_conf) + raise "openssl.conf not found at (#{openssl_conf})" unless File.exist?(openssl_conf) openssl_conf end def user_input gets.chomp end + + def wait_for_command_to_finish + @kmu.wait_for(:output, /Command:/) + end + + def cache_credentials_and_private_key_handle(key_handle) + File.open("saml_#{@timestamp}.scr", 'w') do |file| + file.puts("#{@username}:#{@password}:#{key_handle}:#{@idp_username}") + end + end end CloudhsmKeyGenerator.new.generate_saml_key if $PROGRAM_NAME == __FILE__ + +# Command: loginHSM -u CU -s username -p password + +# Cfm3LoginHSM returned: 0x00 : HSM Return: SUCCESS + +# Cluster Error Status +# Node id 1 and err state 0x00000000 : HSM Return: SUCCESS + +# Command: genSymKey -t 31 -s 16 -sess -l wrapping_key_for_import + +# Cfm3GenerateSymmetricKey returned: 0x00 : HSM Return: SUCCESS + +# Symmetric Key Created. Key Handle: 1234 + +# Cluster Error Status +# Node id 1 and err state 0x00000000 : HSM Return: SUCCESS + +# Command: importPrivateKey -f saml_20180721214537.key -l saml_20180721214537 -w 1234 +# BER encoded key length is 1218 + +# Cfm3WrapHostKey returned: 0x00 : HSM Return: SUCCESS + +# Cfm3CreateUnwrapTemplate returned: 0x00 : HSM Return: SUCCESS + +# Cfm3UnWrapKey returned: 0x00 : HSM Return: SUCCESS + +# Private Key Imported. Key Handle: 5678 + +# Cluster Error Status +# Node id 1 and err state 0x00000000 : HSM Return: SUCCESS diff --git a/lib/cloudhsm/cloudhsm_key_sharer.rb b/lib/cloudhsm/cloudhsm_key_sharer.rb new file mode 100644 index 00000000000..7e412caeb41 --- /dev/null +++ b/lib/cloudhsm/cloudhsm_key_sharer.rb @@ -0,0 +1,123 @@ +require 'greenletters' + +# In prod this script is run through the cloudhsm rake task which will run on a RamDisk +# Grants access to the key generated by cloudhsm_key_generator to the idp user +# Uses the saml key label and cached output from the generator (credentials/key handle) +# Outputs to file saml_.shr (a transcript of the cloudhsm interaction) + +class CloudhsmKeySharer + CLOUDHSM_MGMT_UTIL = \ + '/opt/cloudhsm/bin/cloudhsm_mgmt_util /opt/cloudhsm/etc/cloudhsm_mgmt_util.cfg'.freeze + + def initialize(saml_label) + @saml_label = saml_label + @username, @password, @key_handle, @idp_username = read_credentials_and_private_key_handle + @kmu = run_cloudhsm_mgmt_util + wait_for_command_to_finish + enable_encrypted + end + + def share_saml_key + login_to_hsm + idp_user_id = idp_user_id_from_list_users + raise "User #{@idp_username} not found" unless idp_user_id + share_key(@key_handle, idp_user_id) + exit_hsm + end + + private + + def read_credentials_and_private_key_handle + File.read("#{@saml_label}.scr").to_s.chomp.split(':') + end + + def run_cloudhsm_mgmt_util + output = File.open("#{@saml_label}.shr", 'w') + kmu = Greenletters::Process.new(CLOUDHSM_MGMT_UTIL, transcript: output) + kmu.start! + kmu + end + + def enable_encrypted + @kmu << "enable_e2e\n" + @kmu.wait_for(:output, /E2E enabled/) + wait_for_command_to_finish + end + + def idp_user_id_from_list_users + @kmu << "listUsers\n" + user_id = nil + @kmu.wait_for(:output, /\d+\s+CU\s+#{@idp_username}/) do |_process, matching| + user_id = matching.matched.split[0] + end + wait_for_command_to_finish + user_id + end + + def share_key(key_handle, user_id) + @kmu << "shareKey #{key_handle} #{user_id} 1\n" + @kmu.wait_for(:output, %r{Do you want to continue\(y/n\)\?}) + @kmu << "y\n" + @kmu.wait_for(:output, /success on server/) + wait_for_command_to_finish + end + + def login_to_hsm + @kmu << "loginHSM CU #{@username} #{@password}\n" + @kmu.wait_for(:output, /loginHSM success/) + wait_for_command_to_finish + end + + def exit_hsm + @kmu << "quit\n" + Kernel.puts "Key shared with the idp user successfully\n" \ + "Transcript written to '#{@saml_label}.shr'.\n" + end + + def wait_for_command_to_finish + @kmu.wait_for(:output, /aws-cloudhsm>/) + end +end + +CloudhsmKeySharer.new(ARGV[0]).share_saml_key if $PROGRAM_NAME == __FILE__ + +# cat saml_20180721031604.scr +# gen_user:password:1234:idp_user + +# /opt/cloudhsm/bin/cloudhsm_mgmt_util /opt/cloudhsm/etc/cloudhsm_mgmt_util.cfg + +# Connecting to the server(s), it may take time +# depending on the server(s) load, please wait... +# Connecting to server '127.0.0.1': hostname '127.0.0.1', port 2225... +# Connected to server '127.0.0.1': hostname '127.0.0.1', port 2225. + +# aws-cloudhsm>enable_e2e +# Server 0(127.0.0.1) is in e2e mode now... + +# aws-cloudhsm>loginHSM CU idp_generator password +# loginHSM success on server 0(127.0.0.1) + +# aws-cloudhsm>listUsers +# Users on server 0(127.0.0.1): +# Number of users found:5 + +# User Id User Type User Name MofnPubKey LoginFailureCnt 2FA +# 1 CO admin NO 0 NO +# 2 AU app_user NO 0 NO +# 3 AU jane_doe NO 0 NO +# 4 CU idp_user NO 0 NO +# 5 CU gen_user NO 0 NO + +# aws-cloudhsm>shareKey 1234 4 1 +# *************************CAUTION******************************** +# This is a CRITICAL operation, should be done on all nodes in the +# cluster. Cav server does NOT synchronize these changes with the +# nodes on which this operation is not executed or failed, please +# ensure this operation is executed on all nodes in the cluster. +# **************************************************************** + +# Do you want to continue(y/n)?y +# shareKey success on server 0(127.0.0.1) + +# aws-cloudhsm>quit +# disconnecting from servers, please wait... diff --git a/lib/tasks/cloudhsm.rake b/lib/tasks/cloudhsm.rake index e1efeb6cacd..78204b2c1c8 100644 --- a/lib/tasks/cloudhsm.rake +++ b/lib/tasks/cloudhsm.rake @@ -1,6 +1,9 @@ +# In prod this script will run on a RamDisk + namespace :cloudhsm do - desc 'Generate a new saml key' + desc 'Generate a new saml key and grant idp access to the key' task generate_saml_key: :environment do - CloudhsmKeyGenerator.new.generate_saml_key + saml_key_label, _handle = CloudhsmKeyGenerator.new.generate_saml_key + CloudhsmKeySharer.new(saml_key_label).share_saml_key end end diff --git a/lib/tasks/migrate_phone_configurations.rake b/lib/tasks/migrate_phone_configurations.rake new file mode 100644 index 00000000000..f857e0e96f2 --- /dev/null +++ b/lib/tasks/migrate_phone_configurations.rake @@ -0,0 +1,7 @@ +namespace :adhoc do + desc 'Copy phone configurations to the new table' + task populate_phone_configurations: :environment do + Rails.logger = Logger.new(STDOUT) + PopulatePhoneConfigurationsTable.new.call + end +end diff --git a/spec/controllers/account_reset/cancel_controller_spec.rb b/spec/controllers/account_reset/cancel_controller_spec.rb index 529b375fb76..a3b2a5c3777 100644 --- a/spec/controllers/account_reset/cancel_controller_spec.rb +++ b/spec/controllers/account_reset/cancel_controller_spec.rb @@ -1,67 +1,84 @@ require 'rails_helper' describe AccountReset::CancelController do - let(:user) { create(:user, :signed_up, phone: '+1 (703) 555-0000') } + include AccountResetHelper + + let(:user) { create(:user, :signed_up) } before do TwilioService::Utils.telephony_service = FakeSms end - describe '#cancel' do + describe '#create' do it 'logs a good token to the analytics' do - AccountResetService.new(user).create_request + token = create_account_reset_request_for(user) stub_analytics + analytics_hash = { + success: true, + errors: {}, + event: 'cancel', + user_id: user.uuid, + } + expect(@analytics).to receive(:track_event). - with(Analytics::ACCOUNT_RESET, - event: :cancel, token_valid: true, user_id: user.uuid) + with(Analytics::ACCOUNT_RESET, analytics_hash) - post :cancel, params: { token: AccountResetRequest.all[0].request_token } + post :create, params: { token: token } end it 'logs a bad token to the analytics' do stub_analytics + analytics_hash = { + success: false, + errors: { token: [t('errors.account_reset.cancel_token_invalid')] }, + event: 'cancel', + user_id: 'anonymous-uuid', + } + expect(@analytics).to receive(:track_event). - with(Analytics::ACCOUNT_RESET, event: :cancel, token_valid: false) + with(Analytics::ACCOUNT_RESET, analytics_hash) - post :cancel, params: { token: 'FOO' } + post :create, params: { token: 'FOO' } end - it 'redirects to the root' do - post :cancel - expect(response).to redirect_to root_url - end + it 'logs a missing token to the analytics' do + stub_analytics + analytics_hash = { + success: false, + errors: { token: [t('errors.account_reset.cancel_token_missing')] }, + event: 'cancel', + user_id: 'anonymous-uuid', + } - it 'sends an SMS if there is a phone' do - AccountResetService.new(user).create_request - allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) + expect(@analytics).to receive(:track_event). + with(Analytics::ACCOUNT_RESET, analytics_hash) - post :cancel, params: { token: AccountResetRequest.all[0].request_token } + post :create + end - expect(SmsAccountResetCancellationNotifierJob).to have_received(:perform_now).with( - phone: user.phone - ) + it 'redirects to the root without a flash message when the token is missing or invalid' do + post :create + expect(response).to redirect_to root_url end - it 'does not send an SMS if there is no phone' do - AccountResetService.new(user).create_request - allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) - user.phone = nil - user.save! - user.phone_configuration.destroy + it 'redirects to the root with a flash message when the token is valid' do + token = create_account_reset_request_for(user) - post :cancel, params: { token: AccountResetRequest.all[0].request_token } + post :create, params: { token: token } - expect(SmsAccountResetCancellationNotifierJob).to_not have_received(:perform_now) + expect(flash[:success]). + to eq t('devise.two_factor_authentication.account_reset.successful_cancel') + expect(response).to redirect_to root_url end - it 'sends an email' do - AccountResetService.new(user).create_request + it 'signs the user out if signed in and if the token is valid' do + stub_sign_in(user) + + token = create_account_reset_request_for(user) - @mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) - allow(UserMailer).to receive(:account_reset_cancel).with(user.email). - and_return(@mailer) + expect(controller).to receive(:sign_out) - post :cancel, params: { token: AccountResetRequest.all[0].request_token } + post :create, params: { token: token } end end end diff --git a/spec/controllers/health/health_controller_spec.rb b/spec/controllers/health/health_controller_spec.rb index 06b3e54de3d..9fa15add5d9 100644 --- a/spec/controllers/health/health_controller_spec.rb +++ b/spec/controllers/health/health_controller_spec.rb @@ -26,6 +26,7 @@ expect(json[:healthy]).to eq(true) expect(json[:statuses][:workers][:all_healthy]).to eq(true) expect(json[:statuses][:database][:healthy]).to eq(true) + expect(json[:statuses][:account_reset][:healthy]).to eq(true) end end diff --git a/spec/decorators/identity_decorator_spec.rb b/spec/decorators/identity_decorator_spec.rb index b075c175b16..55032322260 100644 --- a/spec/decorators/identity_decorator_spec.rb +++ b/spec/decorators/identity_decorator_spec.rb @@ -3,21 +3,21 @@ describe IdentityDecorator do include ActionView::Helpers::TagHelper - let(:user) { create(:user) } - let(:service_provider) { 'http://localhost:3000' } - let(:identity) { create(:identity, :active, user: user, service_provider: service_provider) } + describe '#return_to_sp_url' do + let(:user) { create(:user) } + let(:service_provider) { 'http://localhost:3000' } + let(:identity) { create(:identity, :active, user: user, service_provider: service_provider) } - subject { IdentityDecorator.new(identity) } + subject { IdentityDecorator.new(identity) } - describe '#return_to_sp_url' do - context 'for an sp without a return URL' do - context 'for an sp with a return URL' do - it 'returns the return url for the sp' do - return_to_sp_url = ServiceProvider.from_issuer(service_provider).return_to_sp_url - expect(subject.return_to_sp_url).to eq(return_to_sp_url) - end + context 'for an sp with a return URL' do + it 'returns the return url for the sp' do + return_to_sp_url = ServiceProvider.from_issuer(service_provider).return_to_sp_url + expect(subject.return_to_sp_url).to eq(return_to_sp_url) end + end + context 'for an sp without a return URL' do let(:service_provider) { 'https://rp2.serviceprovider.com/auth/saml/metadata' } it 'returns nil' do @@ -25,4 +25,27 @@ end end end + + describe '#failure_to_proof_url' do + let(:user) { create(:user) } + let(:service_provider) { 'https://rp1.serviceprovider.com/auth/saml/metadata' } + let(:identity) { create(:identity, :active, user: user, service_provider: service_provider) } + + subject { IdentityDecorator.new(identity) } + + context 'for an sp with a failure to proof url' do + it 'returns the failure_to_proof_url for the sp' do + failure_to_proof_url = ServiceProvider.from_issuer(service_provider).failure_to_proof_url + expect(subject.failure_to_proof_url).to eq(failure_to_proof_url) + end + end + + context 'for an sp without a failure to proof URL' do + let(:service_provider) { 'http://localhost:3000' } + + it 'returns nil' do + expect(subject.failure_to_proof_url).to eq(nil) + end + end + end end diff --git a/spec/features/account_reset/cancel_request_spec.rb b/spec/features/account_reset/cancel_request_spec.rb new file mode 100644 index 00000000000..d2524fac848 --- /dev/null +++ b/spec/features/account_reset/cancel_request_spec.rb @@ -0,0 +1,54 @@ +require 'rails_helper' + +describe 'Account Reset Request: Cancellation' do + context 'user cancels right away from the first email' do + it 'cancels the request and does not delete the user', email: true do + TwilioService::Utils.telephony_service = FakeSms + user = create(:user, :signed_up) + signin(user.email, user.password) + click_link t('two_factor_authentication.login_options_link_text') + click_link t('devise.two_factor_authentication.account_reset.link') + click_button t('account_reset.request.yes_continue') + open_last_email + click_email_link_matching(/cancel\?token/) + + expect(page).to have_current_path new_user_session_path + expect(page). + to have_content t('devise.two_factor_authentication.account_reset.successful_cancel') + + signin(user.email, user.password) + + expect(page). + to have_current_path login_two_factor_path(otp_delivery_preference: 'sms', reauthn: 'false') + end + end + + context 'user cancels from the second email after the request has been granted' do + it 'cancels the request and does not delete the user', email: true do + TwilioService::Utils.telephony_service = FakeSms + user = create(:user, :signed_up) + signin(user.email, user.password) + click_link t('two_factor_authentication.login_options_link_text') + click_link t('devise.two_factor_authentication.account_reset.link') + click_button t('account_reset.request.yes_continue') + reset_email + + Timecop.travel(Time.zone.now + 2.days) do + AccountResetService.grant_tokens_and_send_notifications + open_last_email + click_email_link_matching(/cancel\?token/) + + expect(page).to have_current_path new_user_session_path + expect(page). + to have_content t('devise.two_factor_authentication.account_reset.successful_cancel') + + signin(user.email, user.password) + + expect(page). + to have_current_path( + login_two_factor_path(otp_delivery_preference: 'sms', reauthn: 'false') + ) + end + end + end +end diff --git a/spec/features/account_reset/delete_account_spec.rb b/spec/features/account_reset/delete_account_spec.rb new file mode 100644 index 00000000000..e9635fdf13c --- /dev/null +++ b/spec/features/account_reset/delete_account_spec.rb @@ -0,0 +1,66 @@ +require 'rails_helper' + +describe 'Account Reset Request: Delete Account', email: true do + let(:user) { create(:user, :signed_up) } + + before do + TwilioService::Utils.telephony_service = FakeSms + end + + context 'as an LOA1 user' do + it 'allows the user to delete their account after 24 hours' do + signin(user.email, user.password) + click_link t('two_factor_authentication.login_options_link_text') + click_link t('devise.two_factor_authentication.account_reset.link') + click_button t('account_reset.request.yes_continue') + reset_email + + Timecop.travel(Time.zone.now + 2.days) do + AccountResetService.grant_tokens_and_send_notifications + open_last_email + click_email_link_matching(/delete_account\?token/) + + 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') + + expect(page).to have_content( + strip_tags( + t( + 'account_reset.confirm_delete_account.info', + email: user.email, + link: t('account_reset.confirm_delete_account.link_text') + ) + ) + ) + expect(page).to have_current_path(account_reset_confirm_delete_account_path) + expect(User.where(id: user.id)).to be_empty + end + end + end + + context 'as an LOA3 user' do + let(:user) do + create( + :profile, + :active, + :verified, + pii: { first_name: 'John', ssn: '111223333' } + ).user + end + + it 'does not allow the user to delete their account from 2FA screen' do + signin(user.email, user.password) + click_link t('two_factor_authentication.login_options_link_text') + + # Account reset link should not be present + expect(page).to_not have_content(t('devise.two_factor_authentication.account_reset.link')) + + # Visiting account reset directly should redirect to 2FA + visit account_reset_request_path + + expect(page.current_path).to eq(login_two_factor_path(otp_delivery_preference: :sms)) + end + end +end diff --git a/spec/features/saml/loa1_sso_spec.rb b/spec/features/saml/loa1_sso_spec.rb index 4e0b80685e7..e700001bc12 100644 --- a/spec/features/saml/loa1_sso_spec.rb +++ b/spec/features/saml/loa1_sso_spec.rb @@ -119,8 +119,10 @@ visit saml_authn_request sp_request_id = ServiceProviderRequest.last.uuid - page.set_rack_session(sp: {}) - visit new_user_session_url(request_id: sp_request_id) + + visit timeout_path + expect(current_url).to eq root_url(request_id: sp_request_id) + fill_in_credentials_and_submit(user.email, user.password) click_submit_default click_continue diff --git a/spec/lib/cloudhsm/cloudhsm_key_generator_spec.rb b/spec/lib/cloudhsm/cloudhsm_key_generator_spec.rb index e6bb7afbc59..f4280e6aa32 100644 --- a/spec/lib/cloudhsm/cloudhsm_key_generator_spec.rb +++ b/spec/lib/cloudhsm/cloudhsm_key_generator_spec.rb @@ -3,6 +3,12 @@ describe CloudhsmKeyGenerator do let(:subject) { CloudhsmKeyGenerator.new } + let(:wrapping_key) { '1234' } + let(:key_owner_name) { 'username' } + let(:key_owner_password) { 'password' } + let(:share_with_username) { 'username2' } + let(:key_handle) { '5678' } + before(:each) { mock_cloudhsm } around(:each) do |example| @@ -13,57 +19,89 @@ describe '#generate_saml_key' do it 'generates saml secret key, crt, and transcript' do - label = subject.generate_saml_key + greenletters = mock_cloudhsm + label, handle = subject.generate_saml_key saml_key = "#{label}.key" saml_crt = "#{label}.crt" transcript = "#{label}.txt" + secrets_fn = "#{label}.scr" + + expect(greenletters).to have_received(:<<). + with("loginHSM -u CU -s #{key_owner_name} -p #{key_owner_password}\n").once + expect(greenletters).to have_received(:<<). + with("genSymKey -t 31 -s 16 -sess -l wrapping_key_for_import\n").once + expect(greenletters).to have_received(:<<). + with("importPrivateKey -f #{label}.key -l #{label} -w #{wrapping_key}\n").once + expect(greenletters).to have_received(:<<).with("exit\n").once + expect(handle).to eq(key_handle) expect(File.exist?(saml_key)).to eq(true) expect(File.exist?(saml_crt)).to eq(true) expect(File.exist?(transcript)).to eq(true) - subject.cleanup + expect(File.read(secrets_fn)). + to eq("#{key_owner_name}:#{key_owner_password}:#{key_handle}:#{share_with_username}\n") + cleanup(label) end it 'raises an error if the openssl call fails' do allow_any_instance_of(CloudhsmKeyGenerator).to receive(:user_input).and_return(__FILE__) - expect { subject.generate_saml_key }.to raise_error(RuntimeError, 'Call to openssl failed') + + label = nil + expect do + label, _handle = subject.generate_saml_key + end.to raise_error(RuntimeError, 'Call to openssl failed') + + cleanup(label) end it 'raises an error if the openssl.conf is not found' do - allow_any_instance_of(CloudhsmKeyGenerator).to receive(:user_input).and_return('filenotfound') + allow_any_instance_of(CloudhsmKeyGenerator).to receive(:user_input). + and_return(key_owner_name, 'bad_filename', share_with_username) + + label = nil + expect { label, _handle = subject.generate_saml_key }. + to raise_error(RuntimeError, 'openssl.conf not found at (bad_filename)') - expect { subject.generate_saml_key }.to raise_error(RuntimeError, 'openssl.conf not found') + cleanup(label) end it 'creates a saml key label with a 12 digit timestamp' do - label = subject.generate_saml_key + label, _handle = subject.generate_saml_key expect(label =~ /\d{1,12}/).to_not be_nil - subject.cleanup + cleanup(label) end end - describe '#cleanup' do - it 'removes all the files if we request cleanup' do - label = subject.generate_saml_key - subject.cleanup + def mock_cloudhsm + allow(STDIN).to receive(:noecho).and_return(key_owner_password) - saml_key = "#{label}.key" - saml_crt = "#{label}.crt" - transcript = "#{label}.txt" - expect(File.exist?(saml_key)).to eq(false) - expect(File.exist?(saml_crt)).to eq(false) - expect(File.exist?(transcript)).to eq(false) - end - end + greenletters = instance_double(Greenletters::Process) + allow(Greenletters::Process).to receive(:new).and_return(greenletters) + allow(greenletters).to receive(:start!).and_return(true) + allow(greenletters).to receive(:wait_for).and_return(true) + allow(greenletters).to receive(:<<).and_return(true) + allow(greenletters).to receive(:wait_for).with(:output, /Key Handle: \d+/). + and_yield(nil, StringScanner.new('')) + allow_any_instance_of(StringScanner).to receive(:matched). + and_return("Key Handle: #{wrapping_key}\n", "Key Handle: #{key_handle}\n") - def mock_cloudhsm - allow(STDIN).to receive(:noecho).and_return('password') - allow_any_instance_of(Greenletters::Process).to receive(:wait_for).and_return(true) - allow_any_instance_of(Greenletters::Process).to receive(:<<).and_return(true) allow_any_instance_of(CloudhsmKeyGenerator).to receive(:user_input). - and_return('config/openssl.conf') + and_return(key_owner_name, 'config/openssl.conf', share_with_username) + + greenletters + end + + def cleanup(label) + safe_delete("#{label}.crt") + safe_delete("#{label}.txt") + safe_delete("#{label}.key") + safe_delete("#{label}.scr") + end + + def safe_delete(fn) + File.delete(fn) if File.exist?(fn) end end diff --git a/spec/lib/cloudhsm/cloudhsm_key_sharer_spec.rb b/spec/lib/cloudhsm/cloudhsm_key_sharer_spec.rb new file mode 100644 index 00000000000..a066873cf5f --- /dev/null +++ b/spec/lib/cloudhsm/cloudhsm_key_sharer_spec.rb @@ -0,0 +1,61 @@ +require 'rails_helper' +require 'cloudhsm/cloudhsm_key_sharer' + +describe CloudhsmKeySharer do + let(:saml_label) { 'saml_20180614001957' } + let(:subject) { CloudhsmKeySharer.new(saml_label) } + let(:key_handle) { '1234' } + let(:key_owner_name) { 'username' } + let(:key_owner_password) { 'password' } + let(:shared_with_username) { 'username2' } + let(:shared_with_user_id) { '4' } + let(:transcript_fn) { "#{saml_label}.shr" } + + describe '#share_saml_key' do + it 'shares saml key and generates transcript' do + greenletters = mock_cloudhsm + + subject.share_saml_key + + expect(greenletters).to have_received(:<<).with("enable_e2e\n").once + expect(greenletters).to have_received(:<<). + with("loginHSM CU #{key_owner_name} #{key_owner_password}\n").once + expect(greenletters).to have_received(:<<).with("listUsers\n").once + expect(greenletters).to have_received(:<<). + with("shareKey #{key_handle} #{shared_with_user_id} 1\n").once + expect(greenletters).to have_received(:<<).with("y\n").once + expect(greenletters).to have_received(:<<).with("quit\n").once + expect(File.exist?(transcript_fn)).to eq(true) + + File.delete(transcript_fn) + end + + it 'raises an error if the user is not found in listUsers' do + greenletters = mock_cloudhsm + allow(greenletters).to receive(:wait_for).with(:output, /\d+\s+CU\s+#{shared_with_username}/). + and_return(false) + + expect { subject.share_saml_key }. + to raise_error(RuntimeError, "User #{shared_with_username} not found") + end + end + + def mock_cloudhsm + greenletters = instance_double(Greenletters::Process) + allow(Greenletters::Process).to receive(:new).and_return(greenletters) + + allow(greenletters).to receive(:<<).and_return(true) + allow(greenletters).to receive(:start!).and_return(true) + allow(greenletters).to receive(:wait_for).and_return(true) + allow(greenletters).to receive(:wait_for).with(:output, /\d+\s+CU\s+#{shared_with_username}/). + and_yield(nil, StringScanner.new('')) + + allow_any_instance_of(StringScanner).to receive(:matched). + and_return("#{shared_with_user_id} CU #{shared_with_username} NO 0 NO\n") + + allow(File).to receive(:read).with("#{saml_label}.scr"). + and_return("#{key_owner_name}:#{key_owner_password}:#{key_handle}:#{shared_with_username}") + + greenletters + end +end diff --git a/spec/models/null_service_provider_spec.rb b/spec/models/null_service_provider_spec.rb index 7035562b026..ae5ec717123 100644 --- a/spec/models/null_service_provider_spec.rb +++ b/spec/models/null_service_provider_spec.rb @@ -58,6 +58,12 @@ end end + describe '#failure_to_proof_url' do + it 'returns nil' do + expect(subject.failure_to_proof_url).to be_nil + end + end + describe '#issuer' do it 'returns the issuer argument' do expect(subject.issuer).to eq 'foo' diff --git a/spec/services/account_reset/cancel_spec.rb b/spec/services/account_reset/cancel_spec.rb new file mode 100644 index 00000000000..f59132af926 --- /dev/null +++ b/spec/services/account_reset/cancel_spec.rb @@ -0,0 +1,112 @@ +require 'rails_helper' + +describe AccountReset::Cancel do + include AccountResetHelper + + let(:user) { create(:user, :signed_up) } + + before { TwilioService::Utils.telephony_service = FakeSms } + + it 'validates presence of token' do + request = AccountReset::Cancel.new(nil).call + + expect(request.success?).to eq false + end + + it 'validates validity of token' do + request = AccountReset::Cancel.new('foo').call + + expect(request.success?).to eq false + end + + context 'when the token is valid' do + context 'when the user has a phone enabled for SMS' do + it 'notifies the user via SMS of the account reset cancellation' do + token = create_account_reset_request_for(user) + allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) + + AccountReset::Cancel.new(token).call + + expect(SmsAccountResetCancellationNotifierJob). + to have_received(:perform_now).with(phone: user.phone) + end + end + + context 'when the user does not have a phone enabled for SMS' do + it 'does not notify the user via SMS' do + token = create_account_reset_request_for(user) + allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) + user.update!(phone: nil) + + AccountReset::Cancel.new(token).call + + expect(SmsAccountResetCancellationNotifierJob).to_not have_received(:perform_now) + end + end + + it 'notifies the user via email of the account reset cancellation' do + token = create_account_reset_request_for(user) + + @mailer = instance_double(ActionMailer::MessageDelivery, deliver_later: true) + expect(UserMailer).to receive(:account_reset_cancel).with(user.email). + and_return(@mailer) + + AccountReset::Cancel.new(token).call + end + + it 'updates the account_reset_request' do + token = create_account_reset_request_for(user) + account_reset_request = AccountResetRequest.find_by(user_id: user.id) + + AccountReset::Cancel.new(token).call + account_reset_request.reload + + expect(account_reset_request.request_token).to_not be_present + expect(account_reset_request.granted_token).to_not be_present + expect(account_reset_request.requested_at).to be_present + expect(account_reset_request.cancelled_at).to be_present + end + end + + context 'when the token is not valid' do + context 'when the user has a phone enabled for SMS' do + it 'does not notify the user via SMS of the account reset cancellation' do + allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) + + AccountReset::Cancel.new('foo').call + + expect(SmsAccountResetCancellationNotifierJob).to_not have_received(:perform_now) + end + end + + context 'when the user does not have a phone enabled for SMS' do + it 'does not notify the user via SMS' do + allow(SmsAccountResetCancellationNotifierJob).to receive(:perform_now) + user.update!(phone: nil) + + AccountReset::Cancel.new('foo').call + + expect(SmsAccountResetCancellationNotifierJob).to_not have_received(:perform_now) + end + end + + it 'does not notify the user via email of the account reset cancellation' do + expect(UserMailer).to_not receive(:account_reset_cancel) + + AccountReset::Cancel.new('foo').call + end + + it 'does not update the account_reset_request' do + create_account_reset_request_for(user) + account_reset_request = AccountResetRequest.find_by(user_id: user.id) + + AccountReset::Cancel.new('foo').call + account_reset_request.reload + + expect(account_reset_request.request_token).to be_present + expect(account_reset_request.granted_token).to_not be_present + expect(account_reset_request.requested_at).to be_present + expect(account_reset_request.cancelled_at).to_not be_present + end + end +end diff --git a/spec/services/account_reset_health_checker_spec.rb b/spec/services/account_reset_health_checker_spec.rb new file mode 100644 index 00000000000..47a4de67999 --- /dev/null +++ b/spec/services/account_reset_health_checker_spec.rb @@ -0,0 +1,58 @@ +require 'rails_helper' + +RSpec.describe AccountResetHealthChecker do + let(:wait_period) { Figaro.env.account_reset_wait_period_days.to_i.days } + let(:buffer_period) { 2.hours } # window buffer to allow servicing requests after 24 hours + describe '.check' do + subject(:summary) { AccountResetHealthChecker.check } + + context 'when there are no requests' do + it 'returns a healthy check' do + expect(summary.result).to eq(nil) + expect(summary.healthy).to eq(true) + end + end + + context 'when a request is not serviced on time' do + before do + AccountResetRequest.create(id: 1, + user_id: 2, + requested_at: Time.zone.now - wait_period - buffer_period, + request_token: 'foo') + end + + it 'returns an unhealthy check' do + expect(summary.healthy).to eq(false) + end + end + + context 'when an old request was cancelled' do + before do + AccountResetRequest.create(id: 1, + user_id: 2, + requested_at: Time.zone.now - wait_period - buffer_period, + request_token: 'foo', + cancelled_at: Time.zone.now) + end + + it 'returns an unhealthy check' do + expect(summary.healthy).to eq(true) + end + end + + context 'when all requests are serviced on time' do + before do + AccountResetRequest.create(id: 1, + user_id: 2, + requested_at: Time.zone.now - wait_period - buffer_period, + request_token: 'foo', + granted_at: Time.zone.now - buffer_period, + granted_token: 'bar') + end + + it 'returns a healthy check' do + expect(summary.healthy).to eq(true) + end + end + end +end diff --git a/spec/services/account_reset_service_spec.rb b/spec/services/account_reset_service_spec.rb index 6ad96b17490..55f63f306d2 100644 --- a/spec/services/account_reset_service_spec.rb +++ b/spec/services/account_reset_service_spec.rb @@ -1,17 +1,13 @@ require 'rails_helper' describe AccountResetService do - include Rails.application.routes.url_helpers + include AccountResetHelper let(:user) { create(:user) } let(:subject) { AccountResetService.new(user) } let(:user2) { create(:user) } let(:subject2) { AccountResetService.new(user2) } - before do - allow(Figaro.env).to receive(:account_reset_wait_period_days).and_return('1') - end - describe '#create_request' do it 'creates a new account reset request on the user' do subject.create_request @@ -34,31 +30,6 @@ end end - describe '#cancel_request' do - it 'removes tokens from a account reset request' do - subject.create_request - cancel = AccountResetService.cancel_request(user.account_reset_request.request_token) - arr = AccountResetRequest.find_by(user_id: user.id) - expect(arr.request_token).to_not be_present - expect(arr.granted_token).to_not be_present - expect(arr.requested_at).to be_present - expect(arr.cancelled_at).to be_present - expect(arr).to eq(cancel) - end - - it 'does not raise an error for a cancel request with a blank token' do - AccountResetService.cancel_request('') - end - - it 'does not raise an error for a cancel request with a nil token' do - AccountResetService.cancel_request('') - end - - it 'does not raise an error for a cancel request with a bad token' do - AccountResetService.cancel_request('ABC') - end - end - describe '#report_fraud' do it 'removes tokens from the request' do subject.create_request @@ -112,7 +83,7 @@ it 'does not send notifications when the request was cancelled' do subject.create_request - AccountResetService.cancel_request(AccountResetRequest.all[0].request_token) + cancel_request_for(user) after_waiting_the_full_wait_period do notifications_sent = AccountResetService.grant_tokens_and_send_notifications @@ -152,7 +123,7 @@ it 'does not send notifications when the request was cancelled' do subject.create_request - AccountResetService.cancel_request(AccountResetRequest.all[0].request_token) + cancel_request_for(user) notifications_sent = AccountResetService.grant_tokens_and_send_notifications expect(notifications_sent).to eq(0) diff --git a/spec/services/populate_phone_configurations_table_spec.rb b/spec/services/populate_phone_configurations_table_spec.rb new file mode 100644 index 00000000000..da4f3e9c384 --- /dev/null +++ b/spec/services/populate_phone_configurations_table_spec.rb @@ -0,0 +1,43 @@ +require 'rails_helper' + +describe PopulatePhoneConfigurationsTable do + let(:subject) { described_class.new } + + describe '#call' do + context 'a user with no phone' do + let!(:user) { create(:user) } + + it 'migrates nothing' do + subject.call + expect(user.reload.phone_configuration).to be_nil + end + end + + context 'a user with a phone' do + let!(:user) { create(:user, :with_phone) } + + context 'and no phone_configuration entry' do + before(:each) do + user.phone_configuration.delete + user.reload + end + + it 'migrates the phone' do + subject.call + configuration = user.reload.phone_configuration + expect(configuration.phone).to eq user.phone + expect(configuration.confirmed_at).to eq user.phone_confirmed_at + expect(configuration.delivery_preference).to eq user.otp_delivery_preference + end + end + + context 'and an existing phone_configuration entry' do + it 'adds no new rows' do + expect(PhoneConfiguration.where(user_id: user.id).count).to eq 1 + subject.call + expect(PhoneConfiguration.where(user_id: user.id).count).to eq 1 + end + end + end + end +end diff --git a/spec/support/account_reset_helper.rb b/spec/support/account_reset_helper.rb new file mode 100644 index 00000000000..ff32b8695ce --- /dev/null +++ b/spec/support/account_reset_helper.rb @@ -0,0 +1,12 @@ +module AccountResetHelper + def create_account_reset_request_for(user) + AccountResetService.new(user).create_request + account_reset_request = AccountResetRequest.find_by(user_id: user.id) + account_reset_request.request_token + end + + def cancel_request_for(user) + account_reset_request = AccountResetRequest.find_by(user_id: user.id) + account_reset_request.update(cancelled_at: Time.zone.now) + end +end