Merge pull request #9346 from 18F/stages/rc-2023-10-10-patch-1

Deploy RC 322 to prod
18F · Oct 10, 2023 · c1e4c4d · c1e4c4d
2 parents b66e676 + a3e45a7
commit c1e4c4d
Show file tree

Hide file tree

Showing 111 changed files with 1,944 additions and 2,712 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -6,6 +6,7 @@
 require:
   - rubocop-rails
   - rubocop-performance
+  - ./lib/linters/analytics_event_name_linter.rb
   - ./lib/linters/localized_validation_message_linter.rb
   - ./lib/linters/image_size_linter.rb
   - ./lib/linters/mail_later_linter.rb
@@ -43,6 +44,11 @@ Bundler/InsecureProtocolSource:
 Gemspec/DuplicatedAssignment:
   Enabled: true
 
+IdentityIdp/AnalyticsEventNameLinter:
+  Enabled: true
+  Include:
+    - app/services/analytics_events.rb
+
 IdentityIdp/LocalizedValidationMessageLinter:
   Enabled: true
 

diff --git a/app/controllers/concerns/idv/document_capture_concern.rb b/app/controllers/concerns/idv/document_capture_concern.rb
@@ -30,7 +30,6 @@ def failure(message, extra = nil)
     end
 
     # @param [DocAuth::Response,
-    #   DocumentCaptureSessionAsyncResult,
     #   DocumentCaptureSessionResult] response
     def extract_pii_from_doc(user, response, store_in_session: false)
       pii_from_doc = response.pii_from_doc.merge(

diff --git a/app/controllers/concerns/idv/verify_info_concern.rb b/app/controllers/concerns/idv/verify_info_concern.rb
@@ -33,17 +33,19 @@ def shared_update
         user_id: current_user.id,
         threatmetrix_session_id: idv_session.threatmetrix_session_id,
         request_ip: request.remote_ip,
-        double_address_verification: capture_secondary_id_enabled,
+        double_address_verification: double_address_verification,
       )
 
       return true
     end
 
     private
 
-    def capture_secondary_id_enabled
-      current_user.establishing_in_person_enrollment&.
-          capture_secondary_id_enabled || false
+    def double_address_verification
+      # If in person return true else return false. This is temporary until we add a feature flag
+      # to track enrollment was created in the in person flow.
+      # todo LG-11235 update value based on new feature flag
+      current_user.has_in_person_enrollment?
     end
 
     def should_use_aamva?(pii)
@@ -163,7 +165,7 @@ def process_async_state(current_async_state)
         return
       end
 
-      return if confirm_not_rate_limited
+      return if confirm_not_rate_limited_after_doc_auth
 
       if current_async_state.none?
         idv_session.invalidate_verify_info_step!

diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb
@@ -9,7 +9,6 @@ module IdvStepConcern
   included do
     before_action :confirm_two_factor_authenticated
     before_action :confirm_idv_needed
-    before_action :confirm_not_rate_limited
     before_action :confirm_no_pending_gpo_profile
     before_action :confirm_no_pending_in_person_enrollment
     before_action :handle_fraud

diff --git a/app/controllers/concerns/rate_limit_concern.rb b/app/controllers/concerns/rate_limit_concern.rb
@@ -1,9 +1,11 @@
 module RateLimitConcern
   extend ActiveSupport::Concern
 
-  def confirm_not_rate_limited
+  ALL_IDV_RATE_LIMITTERS = [:idv_resolution, :idv_doc_auth, :proof_address, :proof_ssn].freeze
+
+  def confirm_not_rate_limited(rate_limiters = ALL_IDV_RATE_LIMITTERS)
     rate_limited = false
-    %i[idv_resolution idv_doc_auth proof_address proof_ssn].each do |rate_limit_type|
+    rate_limiters.each do |rate_limit_type|
       if rate_limit_redirect!(rate_limit_type)
         rate_limited = true
         break
@@ -12,6 +14,16 @@ def confirm_not_rate_limited
     rate_limited
   end
 
+  def confirm_not_rate_limited_after_doc_auth
+    rate_limitters = [:idv_resolution, :proof_ssn, :proof_address]
+    confirm_not_rate_limited(rate_limitters)
+  end
+
+  def confirm_not_rate_limited_after_idv_resolution
+    rate_limitters = [:proof_address]
+    confirm_not_rate_limited(rate_limitters)
+  end
+
   def rate_limit_redirect!(rate_limit_type)
     if idv_attempter_rate_limited?(rate_limit_type)
       track_rate_limited_event(rate_limit_type)

diff --git a/app/controllers/idv/address_controller.rb b/app/controllers/idv/address_controller.rb
@@ -2,6 +2,7 @@ module Idv
   class AddressController < ApplicationController
     include IdvStepConcern
 
+    before_action :confirm_not_rate_limited_after_doc_auth
     before_action :confirm_document_capture_complete
 
     def new

diff --git a/app/controllers/idv/agreement_controller.rb b/app/controllers/idv/agreement_controller.rb
@@ -3,6 +3,7 @@ class AgreementController < ApplicationController
     include IdvStepConcern
     include StepIndicatorConcern
 
+    before_action :confirm_not_rate_limited
     before_action :confirm_welcome_step_complete
     before_action :confirm_agreement_needed
 

diff --git a/app/controllers/idv/capture_doc_status_controller.rb b/app/controllers/idv/capture_doc_status_controller.rb
@@ -43,8 +43,7 @@ def redirect_url
 
     def session_result
       return @session_result if defined?(@session_result)
-      @session_result = document_capture_session.load_result ||
-                        document_capture_session.load_doc_auth_async_result
+      @session_result = document_capture_session.load_result
     end
 
     def document_capture_session

diff --git a/app/controllers/idv/document_capture_controller.rb b/app/controllers/idv/document_capture_controller.rb
@@ -5,6 +5,7 @@ class DocumentCaptureController < ApplicationController
     include IdvStepConcern
     include StepIndicatorConcern
 
+    before_action :confirm_not_rate_limited
     before_action :confirm_hybrid_handoff_complete
     before_action :confirm_document_capture_needed
     before_action :override_csp_to_allow_acuant

diff --git a/app/controllers/idv/getting_started_controller.rb b/app/controllers/idv/getting_started_controller.rb
@@ -2,6 +2,7 @@ module Idv
   class GettingStartedController < ApplicationController
     include IdvStepConcern
 
+    before_action :confirm_not_rate_limited
     before_action :confirm_agreement_needed
 
     def show

diff --git a/app/controllers/idv/hybrid_handoff_controller.rb b/app/controllers/idv/hybrid_handoff_controller.rb
@@ -4,6 +4,7 @@ class HybridHandoffController < ApplicationController
     include IdvStepConcern
     include StepIndicatorConcern
 
+    before_action :confirm_not_rate_limited
     before_action :confirm_verify_info_step_needed
     before_action :confirm_agreement_step_complete
     before_action :confirm_hybrid_handoff_needed, only: :show

diff --git a/app/controllers/idv/in_person/ssn_controller.rb b/app/controllers/idv/in_person/ssn_controller.rb
@@ -6,6 +6,7 @@ class SsnController < ApplicationController
       include Steps::ThreatMetrixStepHelper
       include ThreatMetrixConcern
 
+      before_action :confirm_not_rate_limited_after_doc_auth
       before_action :confirm_verify_info_step_needed
       before_action :confirm_in_person_address_step_complete
       before_action :confirm_repeat_ssn, only: :show

diff --git a/app/controllers/idv/in_person/verify_info_controller.rb b/app/controllers/idv/in_person/verify_info_controller.rb
@@ -6,14 +6,13 @@ class VerifyInfoController < ApplicationController
       include Steps::ThreatMetrixStepHelper
       include VerifyInfoConcern
 
+      before_action :confirm_not_rate_limited_after_doc_auth, except: [:show]
       before_action :confirm_ssn_step_complete
       before_action :confirm_verify_info_step_needed
-      skip_before_action :confirm_not_rate_limited, only: :show
 
       def show
         @step_indicator_steps = step_indicator_steps
         @ssn = idv_session.ssn
-        @capture_secondary_id_enabled = capture_secondary_id_enabled
 
         analytics.idv_doc_auth_verify_visited(**analytics_arguments)
         Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]).

diff --git a/app/controllers/idv/link_sent_controller.rb b/app/controllers/idv/link_sent_controller.rb
@@ -4,6 +4,7 @@ class LinkSentController < ApplicationController
     include IdvStepConcern
     include StepIndicatorConcern
 
+    before_action :confirm_not_rate_limited
     before_action :confirm_hybrid_handoff_complete
     before_action :confirm_document_capture_needed
 
@@ -84,10 +85,7 @@ def take_photo_with_phone_successful?
     end
 
     def document_capture_session_result
-      @document_capture_session_result ||= begin
-        document_capture_session&.load_result ||
-          document_capture_session&.load_doc_auth_async_result
-      end
+      @document_capture_session_result ||= document_capture_session&.load_result
     end
   end
 end
diff --git a/app/controllers/idv/phone_controller.rb b/app/controllers/idv/phone_controller.rb
@@ -7,10 +7,10 @@ class PhoneController < ApplicationController
 
     attr_reader :idv_form
 
+    before_action :confirm_not_rate_limited_after_idv_resolution, except: [:new]
     before_action :confirm_verify_info_step_complete
     before_action :confirm_step_needed
     before_action :set_idv_form
-    skip_before_action :confirm_not_rate_limited, only: :new
 
     def new
       flash.keep(:success) if should_keep_flash_success?
@@ -24,7 +24,7 @@ def new
 
       render 'shared/wait' and return if async_state.in_progress?
 
-      return if confirm_not_rate_limited
+      return if confirm_not_rate_limited_after_idv_resolution
 
       if async_state.none?
         Funnel::DocAuth::RegisterStep.new(current_user.id, current_sp&.issuer).

diff --git a/app/controllers/idv/phone_question_controller.rb b/app/controllers/idv/phone_question_controller.rb
@@ -4,6 +4,7 @@ class PhoneQuestionController < ApplicationController
     include IdvStepConcern
     include StepIndicatorConcern
 
+    before_action :confirm_not_rate_limited
     before_action :confirm_verify_info_step_needed
     before_action :confirm_agreement_step_complete
     before_action :confirm_hybrid_handoff_needed, only: :show

diff --git a/app/controllers/idv/review_controller.rb b/app/controllers/idv/review_controller.rb
@@ -10,7 +10,6 @@ class ReviewController < ApplicationController
     before_action :confirm_verify_info_step_complete
     before_action :confirm_address_step_complete
     before_action :confirm_current_password, only: [:create]
-    skip_before_action :confirm_not_rate_limited
 
     helper_method :step_indicator_step
 

diff --git a/app/controllers/idv/ssn_controller.rb b/app/controllers/idv/ssn_controller.rb
@@ -5,6 +5,7 @@ class SsnController < ApplicationController
     include Steps::ThreatMetrixStepHelper
     include ThreatMetrixConcern
 
+    before_action :confirm_not_rate_limited_after_doc_auth
     before_action :confirm_verify_info_step_needed
     before_action :confirm_document_capture_complete
     before_action :confirm_repeat_ssn, only: :show

diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb
@@ -5,9 +5,9 @@ class VerifyInfoController < ApplicationController
     include VerifyInfoConcern
     include Steps::ThreatMetrixStepHelper
 
+    before_action :confirm_not_rate_limited_after_doc_auth, except: [:show]
     before_action :confirm_ssn_step_complete
     before_action :confirm_verify_info_step_needed
-    skip_before_action :confirm_not_rate_limited, only: :show
 
     def show
       @step_indicator_steps = step_indicator_steps

diff --git a/app/controllers/idv/welcome_controller.rb b/app/controllers/idv/welcome_controller.rb
@@ -4,6 +4,7 @@ class WelcomeController < ApplicationController
     include StepIndicatorConcern
     include GettingStartedAbTestConcern
 
+    before_action :confirm_not_rate_limited
     before_action :confirm_welcome_needed
     before_action :maybe_redirect_for_getting_started_ab_test
 

diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb
@@ -3,7 +3,6 @@ class LogoutController < ApplicationController
     include SecureHeadersConcern
     include FullyAuthenticatable
 
-    before_action :apply_secure_headers_override, only: [:index, :delete]
     before_action :confirm_two_factor_authenticated, only: [:delete]
 
     def index
@@ -41,6 +40,17 @@ def delete
 
     private
 
+    def apply_logout_secure_headers_override(redirect_uri, service_provider)
+      return if service_provider.nil? || redirect_uri.nil?
+
+      uris = SecureHeadersAllowList.csp_with_sp_redirect_uris(
+        redirect_uri,
+        service_provider.redirect_uris,
+      )
+
+      override_form_action_csp(uris)
+    end
+
     def require_logout_confirmation?
       (logout_params[:id_token_hint].nil? || IdentityConfig.store.reject_id_token_hint_in_logout) &&
         logout_params[:client_id] &&
@@ -55,6 +65,7 @@ def build_logout_form
     end
 
     def handle_successful_logout_request(result, redirect_uri)
+      apply_logout_secure_headers_override(redirect_uri, @logout_form.service_provider)
       if require_logout_confirmation?
         analytics.oidc_logout_visited(**result.to_h.except(:redirect_uri))
         @params = {

diff --git a/app/forms/idv/in_person/address_form.rb b/app/forms/idv/in_person/address_form.rb
@@ -8,10 +8,6 @@ class AddressForm
 
       attr_accessor(*ATTRIBUTES)
 
-      def initialize(capture_secondary_id_enabled:)
-        @capture_secondary_id_enabled = capture_secondary_id_enabled
-      end
-
       def self.model_name
         ActiveModel::Name.new(self, nil, 'InPersonAddress')
       end
@@ -33,9 +29,6 @@ def submit(params)
 
       private
 
-      attr_reader :capture_secondary_id_enabled
-      alias_method :capture_secondary_id_enabled?, :capture_secondary_id_enabled
-
       def consume_params(params)
         params.each do |key, value|
           raise_invalid_address_parameter_error(key) unless ATTRIBUTES.include?(key.to_sym)

diff --git a/app/forms/idv/state_id_form.rb b/app/forms/idv/state_id_form.rb
@@ -13,9 +13,8 @@ def self.model_name
       ActiveModel::Name.new(self, nil, 'StateId')
     end
 
-    def initialize(pii, capture_secondary_id_enabled:)
+    def initialize(pii)
       @pii = pii
-      @capture_secondary_id_enabled = capture_secondary_id_enabled
     end
 
     def submit(params)
@@ -36,9 +35,6 @@ def submit(params)
 
     private
 
-    attr_reader :capture_secondary_id_enabled
-    alias_method :capture_secondary_id_enabled?, :capture_secondary_id_enabled
-
     def consume_params(params)
       params.each do |key, value|
         raise_invalid_state_id_parameter_error(key) unless ATTRIBUTES.include?(key.to_sym)

diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb
@@ -120,9 +120,8 @@ def check_enrollment(enrollment)
     enrollment.update(status_check_attempted_at: status_check_attempted_at)
   end
 
-  def passed_with_unsupported_secondary_id_type?(enrollment, response)
-    return enrollment.capture_secondary_id_enabled &&
-           response['secondaryIdType'].present? &&
+  def passed_with_unsupported_secondary_id_type?(response)
+    return response['secondaryIdType'].present? &&
            SUPPORTED_SECONDARY_ID_TYPES.exclude?(response['secondaryIdType'])
   end
 
@@ -414,7 +413,7 @@ def process_enrollment_response(enrollment, response)
 
     case response['status']
     when IPP_STATUS_PASSED
-      if passed_with_unsupported_secondary_id_type?(enrollment, response)
+      if passed_with_unsupported_secondary_id_type?(response)
         handle_unsupported_secondary_id(enrollment, response)
       elsif SUPPORTED_ID_TYPES.include?(response['primaryIdType'])
         handle_successful_status_update(enrollment, response)

diff --git a/app/jobs/reports/monthly_key_metrics_report.rb b/app/jobs/reports/monthly_key_metrics_report.rb
@@ -11,9 +11,11 @@ def perform(date = Time.zone.today)
 
       account_reuse_table = account_reuse_report.account_reuse_report
       total_profiles_table = account_reuse_report.total_identities_report
+      account_deletion_rate_table = account_deletion_rate_report.account_deletion_report
 
       upload_to_s3(account_reuse_table, report_name: 'account_reuse')
       upload_to_s3(total_profiles_table, report_name: 'total_profiles')
+      upload_to_s3(account_deletion_rate_table, report_name: 'account_deletion_rate')
 
       email_tables = [
         [
@@ -28,6 +30,14 @@ def perform(date = Time.zone.today)
           { title: 'Total proofed identities' },
           *total_profiles_table,
         ],
+        [
+          {
+            title: 'Account deletion rate (last 30 days)',
+            float_as_percent: true,
+            precision: 4,
+          },
+          *account_deletion_rate_table,
+        ],
       ]
 
       email_message = "Report: #{REPORT_NAME} #{date}"
@@ -57,6 +67,10 @@ def account_reuse_report
       @account_reuse_report ||= Reporting::AccountReuseAndTotalIdentitiesReport.new(report_date)
     end
 
+    def account_deletion_rate_report
+      @account_deletion_rate_report ||= Reporting::AccountDeletionRateReport.new(report_date)
+    end
+
     def upload_to_s3(report_body, report_name: nil)
       _latest, path = generate_s3_paths(REPORT_NAME, 'csv', subname: report_name, now: report_date)