Merge branch 'master' into stages/rc-2018-08-16

.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)
 

.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

.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.

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)

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

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

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)

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(

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

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

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

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

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

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