Update .rubocop.yml and fix offenses

.rubocop.yml

@@ -10,16 +10,14 @@ AllCops:
     - '**/Rakefile'
     - '**/Capfile'
   Exclude:
+    - 'bin/**/*'
+    - 'db/migrate/*'
     - 'db/schema.rb'
-    - 'node_modules/**/*'
     - 'lib/rspec/user_flow_formatter.rb'
+    - 'lib/tasks/create_test_accounts.rb'
     - 'lib/user_flow_exporter.rb'
-    - 'scripts/load_testing/*'
-    - 'spec/**/*'
+    - 'node_modules/**/*'
     - 'tmp/**/*'
-    - 'bin/**/*'
-    - 'db/migrate/*'
-    - 'lib/tasks/create_test_accounts.rb'
   TargetRubyVersion: 2.3
   TargetRailsVersion: 5.1
   UseCache: true
@@ -103,10 +101,13 @@ Metrics/ModuleLength:
 Metrics/ParameterLists:
   CountKeywordArgs: false
 
-# This is a Rails 5 feature, so it should be disabled until we upgrade
+Naming/VariableName:
+  Exclude:
+    - 'spec/services/pii/nist_encryption_spec.rb'
+
 Rails/HttpPositionalArguments:
   Description: 'Use keyword arguments instead of positional arguments in http method calls.'
-  Enabled: false
+  Enabled: true
   Include:
     - 'spec/**/*'
     - 'test/**/*'

app/controllers/application_controller.rb

@@ -184,12 +184,13 @@ def sp_session
   end
 
   def render_not_found
-    render template: 'pages/page_not_found', layout: false, status: 404, formats: :html
+    render template: 'pages/page_not_found', layout: false, status: :not_found, formats: :html
   end
 
   def render_timeout(exception)
     analytics.track_event(Analytics::RESPONSE_TIMED_OUT, analytics_exception_info(exception))
-    render template: 'pages/page_took_too_long', layout: false, status: 503, formats: :html
+    render template: 'pages/page_took_too_long',
+           layout: false, status: :service_unavailable, formats: :html
   end
 
   def analytics_exception_info(exception)

app/controllers/pages_controller.rb

@@ -4,6 +4,6 @@ class PagesController < ApplicationController
   skip_before_action :disable_caching
 
   def page_not_found
-    render layout: false, status: 404, formats: :html
+    render layout: false, status: :not_found, formats: :html
   end
 end

spec/config/initializers/active_job_logger_patch_spec.rb

@@ -5,19 +5,19 @@
 # user data from being logged.
 describe ActiveJob::Logging::LogSubscriber do
   it 'overrides the default job logger to output only specified parameters in JSON format' do
-    class FakeJob < ActiveJob::Base
+    class FakeJob < ApplicationJob
       def perform(sensitive_param:); end
     end
 
     # This list corresponds to the initializer's output
-    permitted_attributes = %w(
+    permitted_attributes = %w[
       timestamp
       event_type
       job_class
       job_queue
       job_id
       duration
-    )
+    ]
 
     # In this case, we need to assert before the action which logs, block-style to
     # match the initializer
@@ -27,7 +27,7 @@ def perform(sensitive_param:); end
       # [Sidenote: The nested assertions don't seem to be reflected in the spec
       # count--perhaps because of the uncommon block format?--but reversing them
       # will show them failing as expected.]
-      output.keys.each { |k| expect(permitted_attributes).to include(k) }
+      output.each_key { |k| expect(permitted_attributes).to include(k) }
       expect(output.keys).to_not include('sensitive_param')
     end
 

spec/controllers/idv/confirmations_controller_spec.rb

@@ -37,7 +37,7 @@ def stub_idv_session
       address2: 'Ste 456',
       city: 'Anywhere',
       state: 'KS',
-      zipcode: '66666'
+      zipcode: '66666',
     }
   end
   let(:profile) { subject.idv_session.profile }

spec/controllers/sign_up/passwords_controller_spec.rb

@@ -53,7 +53,7 @@
     render_views
     it 'instructs crawlers to not index this page' do
       token = 'foo token'
-      user = create(:user, :unconfirmed, confirmation_token: token, confirmation_sent_at: Time.zone.now)
+      create(:user, :unconfirmed, confirmation_token: token, confirmation_sent_at: Time.zone.now)
       get :new, params: { confirmation_token: token }
 
       expect(response.body).to match('<meta content="noindex,nofollow" name="robots" />')

spec/controllers/test/piv_cac_authentication_test_subject_controller_spec.rb

@@ -68,7 +68,7 @@
         uri.to_s
       end
 
-      let(:expected_token) { {'error' => 'certificate.none', 'nonce' => nonce }}
+      let(:expected_token) { { 'error' => 'certificate.none', 'nonce' => nonce } }
       let(:serialized_token) { expected_token.to_json }
       let(:nonce) { 'nonce' }
 

spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb

@@ -3,8 +3,7 @@
 describe TwoFactorAuthentication::PivCacVerificationController do
   let(:user) do
     create(:user, :signed_up, :with_piv_or_cac,
-      phone: '+1 (555) 555-0000'
-    )
+           phone: '+1 (555) 555-0000')
   end
 
   let(:nonce) { 'once' }
@@ -17,12 +16,12 @@
     allow(PivCacService).to receive(:decode_token).with('good-token').and_return(
       'uuid' => user.x509_dn_uuid,
       'dn' => x509_subject,
-      'nonce' => nonce,
+      'nonce' => nonce
     )
     allow(PivCacService).to receive(:decode_token).with('good-other-token').and_return(
       'uuid' => user.x509_dn_uuid + 'X',
       'dn' => x509_subject + 'X',
-      'nonce' => nonce,
+      'nonce' => nonce
     )
     allow(PivCacService).to receive(:decode_token).with('bad-token').and_return(
       'uuid' => 'bad-uuid',
@@ -58,7 +57,7 @@
         expect(subject.current_user).to receive(:confirm_piv_cac?).and_return(true)
         expect(subject.current_user.reload.second_factor_attempts_count).to eq 0
 
-        get :show, params: { token:  'good-token' }
+        get :show, params: { token: 'good-token' }
 
         expect(response).to redirect_to account_path
         expect(subject.user_session[:decrypted_x509]).to eq({
@@ -73,7 +72,7 @@
           attributes: { second_factor_attempts_count: 1 }
         ).call
 
-        get :show, params: { token:  'good-token' }
+        get :show, params: { token: 'good-token' }
 
         expect(subject.current_user.reload.second_factor_attempts_count).to eq 0
       end
@@ -88,7 +87,7 @@
         }
         expect(@analytics).to receive(:track_event).with(Analytics::MULTI_FACTOR_AUTH, attributes)
 
-        get :show, params: { token:  'good-token' }
+        get :show, params: { token: 'good-token' }
       end
     end
 
@@ -170,9 +169,8 @@
 
       let(:user) do
         create(:user, :signed_up, :with_piv_or_cac,
-          second_factor_locked_at: Time.zone.now - lockout_period - 1.second,
-          second_factor_attempts_count: 3
-        )
+               second_factor_locked_at: Time.zone.now - lockout_period - 1.second,
+               second_factor_attempts_count: 3)
       end
 
       describe 'when user submits an incorrect piv/cac' do

spec/controllers/users/phone_setup_controller_spec.rb

@@ -76,7 +76,6 @@
           :create,
           params: {
             user_phone_form: { phone: '703-555-0100',
-                              #  otp_delivery_preference: 'voice',
                                international_code: 'US' },
           }
         )
@@ -110,7 +109,6 @@
           :create,
           params: {
             user_phone_form: { phone: '703-555-0100',
-                              #  otp_delivery_preference: :sms,
                                international_code: 'US' },
           }
         )
@@ -143,7 +141,6 @@
           :create,
           params: {
             user_phone_form: { phone: '703-555-0100',
-                              #  otp_delivery_preference: :sms,
                                international_code: 'US' },
           }
         )

spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb

@@ -1,7 +1,6 @@
 require 'rails_helper'
 
 describe Users::PivCacAuthenticationSetupController do
-
   describe 'when not signed in' do
     describe 'GET index' do
       it 'redirects to root url' do
@@ -33,9 +32,7 @@
   describe 'when signing in' do
     before(:each) { stub_sign_in_before_2fa(user) }
     let(:user) do
-      create(:user, :signed_up, :with_piv_or_cac,
-        phone: '+1 (555) 555-0000'
-      )
+      create(:user, :signed_up, :with_piv_or_cac, phone: '+1 (555) 555-0000')
     end
 
     describe 'GET index' do
@@ -58,9 +55,7 @@
 
     context 'without associated piv/cac' do
       let(:user) do
-        create(:user, :signed_up,
-          phone: '+1 (555) 555-0000'
-        )
+        create(:user, :signed_up, phone: '+1 (555) 555-0000')
       end
 
       before(:each) do
@@ -83,7 +78,7 @@
       let(:bad_token) { 'bad-token' }
       let(:bad_token_response) do
         {
-          'error' => 'certificate.bad' ,
+          'error' => 'certificate.bad',
           'nonce' => nonce,
         }
       end
@@ -98,22 +93,24 @@
 
         context 'when redirected with a good token' do
           it 'redirects to account page' do
-            get :new, params: {token: good_token}
+            get :new, params: { token: good_token }
             expect(response).to redirect_to(account_url)
           end
 
           it 'sets the piv/cac session information' do
-            get :new, params: {token: good_token}
-            expect(subject.user_session[:decrypted_x509]).to eq ({
+            get :new, params: { token: good_token }
+            json = {
               'subject' => 'some dn',
-              'presented' => true
-            }.to_json)
+              'presented' => true,
+            }.to_json
+
+            expect(subject.user_session[:decrypted_x509]).to eq json
           end
         end
 
         context 'when redirected with an error token' do
           it 'renders the error template' do
-            get :new, params: {token: bad_token}
+            get :new, params: { token: bad_token }
             expect(response).to render_template(:error)
           end
 

spec/controllers/users/totp_setup_controller_spec.rb

@@ -6,7 +6,7 @@
       expect(subject).to have_actions(
         :before,
         :authenticate_user!,
-        [:confirm_two_factor_authenticated, if: :two_factor_enabled?],
+        [:confirm_two_factor_authenticated, if: :two_factor_enabled?]
       )
     end
   end

spec/controllers/users/two_factor_authentication_setup_controller_spec.rb

@@ -50,7 +50,7 @@
       voice_params = {
         two_factor_options_form: {
           selection: 'voice',
-        }
+        },
       }
       params = ActionController::Parameters.new(voice_params)
       response = FormResponse.new(success: true, errors: {}, extra: { selection: 'voice' })

spec/features/accessibility/idv_pages_spec.rb

@@ -43,7 +43,7 @@
     end
 
     scenario 'review page' do
-      user = sign_in_and_2fa_user
+      sign_in_and_2fa_user
       visit idv_session_path
       fill_out_idv_form_ok
       click_idv_continue
@@ -55,7 +55,7 @@
     end
 
     scenario 'personal key / confirmation page' do
-      user = sign_in_and_2fa_user
+      sign_in_and_2fa_user
       visit idv_session_path
       fill_out_idv_form_ok
       click_idv_continue

spec/features/account_history_spec.rb

@@ -32,8 +32,10 @@
   let(:identity_with_link_timestamp) { identity_with_link.decorate.happened_at_in_words }
   let(:usps_mail_sent_again_timestamp) { usps_mail_sent_again_event.decorate.happened_at_in_words }
   let(:identity_without_link_timestamp) { identity_without_link.decorate.happened_at_in_words }
-  let(:new_personal_key_event) { create(:event, event_type: :new_personal_key,
-                                        user: user, created_at: Time.zone.now - 40.days) }
+  let(:new_personal_key_event) do
+    create(:event, event_type: :new_personal_key,
+                   user: user, created_at: Time.zone.now - 40.days)
+  end
 
   before do
     sign_in_and_2fa_user(user)
@@ -42,7 +44,13 @@
   end
 
   scenario 'viewing account history' do
-    [account_created_event, usps_mail_sent_event, usps_mail_sent_again_event, new_personal_key_event].each do |event|
+    events = [
+      account_created_event,
+      usps_mail_sent_event,
+      usps_mail_sent_again_event,
+      new_personal_key_event,
+    ]
+    events.each do |event|
       decorated_event = event.decorate
       expect(page).to have_content(decorated_event.event_type)
       expect(page).to have_content(decorated_event.happened_at_in_words)

spec/features/idv/steps/jurisdiction_step_spec.rb

@@ -29,7 +29,8 @@
         select 'Alabama', from: 'jurisdiction_state'
         click_idv_continue
 
-        expect(page).to have_current_path(idv_jurisdiction_fail_path(reason: :unsupported_jurisdiction))
+        expect(page).
+          to have_current_path(idv_jurisdiction_fail_path(reason: :unsupported_jurisdiction))
         expect(page).to have_content(t('idv.titles.unsupported_jurisdiction', state: 'Alabama'))
       end
     end

spec/features/openid_connect/openid_connect_spec.rb

@@ -574,12 +574,15 @@ def enable_cloudhsm(is_enabled)
     allow(Figaro.env).to receive(:cloudhsm_enabled).and_return('true')
     SamlIdp.configure { |config| SamlIdpEncryptionConfigurator.configure(config, true) }
     allow(PKCS11).to receive(:open).and_return('true')
-    allow_any_instance_of(SamlIdp::Configurator).to receive_message_chain(:pkcs11, :active_slots, :first, :open).and_yield(MockSession)
+    allow_any_instance_of(SamlIdp::Configurator).
+      to receive_message_chain(:pkcs11, :active_slots, :first, :open).and_yield(MockSession)
     allow(MockSession).to receive(:login).and_return(true)
     allow(MockSession).to receive(:logout).and_return(true)
     allow(MockSession).to receive_message_chain(:find_objects, :first).and_return(true)
-    allow(MockSession).to receive(:sign) do |algorithm, key, input|
-      JWT::Algos::Rsa.sign(JWT::Signature::ToSign.new('RS256', input, RequestKeyManager.private_key))
+    allow(MockSession).to receive(:sign) do |_algorithm, _key, input|
+      JWT::Algos::Rsa.sign(
+        JWT::Signature::ToSign.new('RS256', input, RequestKeyManager.private_key)
+      )
     end
   end
 end

spec/features/saml/saml_spec.rb

@@ -283,7 +283,8 @@ def enable_cloudhsm(is_enabled)
     allow(Figaro.env).to receive(:cloudhsm_enabled).and_return('true')
     SamlIdp.configure { |config| SamlIdpEncryptionConfigurator.configure(config, true) }
     allow(PKCS11).to receive(:open).and_return('true')
-    allow_any_instance_of(SamlIdp::Configurator).to receive_message_chain(:pkcs11, :active_slots, :first, :open).and_yield(MockSession)
+    allow_any_instance_of(SamlIdp::Configurator).
+      to receive_message_chain(:pkcs11, :active_slots, :first, :open).and_yield(MockSession)
     allow(MockSession).to receive(:login).and_return(true)
     allow(MockSession).to receive(:logout).and_return(true)
     allow(MockSession).to receive_message_chain(:find_objects, :first).and_return(true)