AuthenticatedControllerAction sets Raven current_user for APIs (#13825)

We were not capturing the current user on IDT API requests for Sentry reporting.

This refactor makes the `set_raven_user` method a controller concern we can share in any controller that needs to capture `current_user` for error reporting.

See https://dsva.slack.com/archives/CD5DAQNCU/p1585581882005900

.security.yml

@@ -1,2 +1,3 @@
 CVES:
-  CVE-2020-5267: 2020-03-29
+# placeholder to make non-nil array
+  CVE-no-such-number: 2020-01-01

Gemfile

@@ -47,7 +47,7 @@ gem "puma", "~> 3.12.4"
 # rack versions before 2.0.6 are affected by CVE-2018-16470 and CVE-2018-16471.
 # Explicitly define rack version here to avoid that.
 gem "rack", "~> 2.0.6"
-gem "rails", "5.2.4.1"
+gem "rails", "5.2.4.2"
 # Used to colorize output for rake tasks
 gem "rainbow"
 # React

Gemfile.lock

@@ -74,48 +74,48 @@ GEM
   remote: https://rubygems.org/
   specs:
     aasm (4.11.0)
-    actioncable (5.2.4.1)
-      actionpack (= 5.2.4.1)
+    actioncable (5.2.4.2)
+      actionpack (= 5.2.4.2)
       nio4r (~> 2.0)
       websocket-driver (>= 0.6.1)
-    actionmailer (5.2.4.1)
-      actionpack (= 5.2.4.1)
-      actionview (= 5.2.4.1)
-      activejob (= 5.2.4.1)
+    actionmailer (5.2.4.2)
+      actionpack (= 5.2.4.2)
+      actionview (= 5.2.4.2)
+      activejob (= 5.2.4.2)
       mail (~> 2.5, >= 2.5.4)
       rails-dom-testing (~> 2.0)
-    actionpack (5.2.4.1)
-      actionview (= 5.2.4.1)
-      activesupport (= 5.2.4.1)
+    actionpack (5.2.4.2)
+      actionview (= 5.2.4.2)
+      activesupport (= 5.2.4.2)
       rack (~> 2.0, >= 2.0.8)
       rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.2)
-    actionview (5.2.4.1)
-      activesupport (= 5.2.4.1)
+    actionview (5.2.4.2)
+      activesupport (= 5.2.4.2)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.3)
-    activejob (5.2.4.1)
-      activesupport (= 5.2.4.1)
+    activejob (5.2.4.2)
+      activesupport (= 5.2.4.2)
       globalid (>= 0.3.6)
-    activemodel (5.2.4.1)
-      activesupport (= 5.2.4.1)
-    activerecord (5.2.4.1)
-      activemodel (= 5.2.4.1)
-      activesupport (= 5.2.4.1)
+    activemodel (5.2.4.2)
+      activesupport (= 5.2.4.2)
+    activerecord (5.2.4.2)
+      activemodel (= 5.2.4.2)
+      activesupport (= 5.2.4.2)
       arel (>= 9.0)
     activerecord-import (1.0.2)
       activerecord (>= 3.2)
     activerecord-oracle_enhanced-adapter (5.2.8)
       activerecord (~> 5.2.0)
       ruby-plsql (>= 0.6.0)
-    activestorage (5.2.4.1)
-      actionpack (= 5.2.4.1)
-      activerecord (= 5.2.4.1)
+    activestorage (5.2.4.2)
+      actionpack (= 5.2.4.2)
+      activerecord (= 5.2.4.2)
       marcel (~> 0.3.1)
-    activesupport (5.2.4.1)
+    activesupport (5.2.4.2)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 0.7, < 2)
       minitest (~> 5.1)
@@ -328,7 +328,7 @@ GEM
     mime-types (3.3)
       mime-types-data (~> 3.2015)
     mime-types-data (3.2019.1009)
-    mimemagic (0.3.3)
+    mimemagic (0.3.4)
     mini_mime (1.0.2)
     mini_portile2 (2.4.0)
     minitest (5.14.0)
@@ -379,23 +379,23 @@ GEM
     psych (3.1.0)
     public_suffix (3.1.1)
     puma (3.12.4)
-    rack (2.0.8)
+    rack (2.0.9)
     rack-contrib (2.1.0)
       rack (~> 2.0)
     rack-test (1.1.0)
       rack (>= 1.0, < 3)
-    rails (5.2.4.1)
-      actioncable (= 5.2.4.1)
-      actionmailer (= 5.2.4.1)
-      actionpack (= 5.2.4.1)
-      actionview (= 5.2.4.1)
-      activejob (= 5.2.4.1)
-      activemodel (= 5.2.4.1)
-      activerecord (= 5.2.4.1)
-      activestorage (= 5.2.4.1)
-      activesupport (= 5.2.4.1)
+    rails (5.2.4.2)
+      actioncable (= 5.2.4.2)
+      actionmailer (= 5.2.4.2)
+      actionpack (= 5.2.4.2)
+      actionview (= 5.2.4.2)
+      activejob (= 5.2.4.2)
+      activemodel (= 5.2.4.2)
+      activerecord (= 5.2.4.2)
+      activestorage (= 5.2.4.2)
+      activesupport (= 5.2.4.2)
       bundler (>= 1.3.0)
-      railties (= 5.2.4.1)
+      railties (= 5.2.4.2)
       sprockets-rails (>= 2.0.0)
     rails-dom-testing (2.0.3)
       activesupport (>= 4.2.0)
@@ -407,9 +407,9 @@ GEM
       ruby-graphviz (~> 1.2)
     rails-html-sanitizer (1.3.0)
       loofah (~> 2.3)
-    railties (5.2.4.1)
-      actionpack (= 5.2.4.1)
-      activesupport (= 5.2.4.1)
+    railties (5.2.4.2)
+      actionpack (= 5.2.4.2)
+      activesupport (= 5.2.4.2)
       method_source
       rake (>= 0.8.7)
       thor (>= 0.19.0, < 2.0)
@@ -643,7 +643,7 @@ DEPENDENCIES
   pry-byebug
   puma (~> 3.12.4)
   rack (~> 2.0.6)
-  rails (= 5.2.4.1)
+  rails (= 5.2.4.2)
   rails-erd
   rainbow
   rb-readline

app/controllers/application_controller.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class ApplicationController < ApplicationBaseController
+  include AuthenticatedControllerAction
+
   before_action :set_application
   around_action :set_timezone
   before_action :setup_fakes
@@ -203,18 +205,6 @@ def required_parameters_missing(array_of_keys)
     }, status: :bad_request
   end
 
-  def set_raven_user
-    if current_user && ENV["SENTRY_DSN"]
-      # Raven sends error info to Sentry.
-      Raven.user_context(
-        email: current_user.email,
-        css_id: current_user.css_id,
-        station_id: current_user.station_id,
-        regional_office: current_user.regional_office
-      )
-    end
-  end
-
   def set_timezone
     old_time_zone = Time.zone
     Time.zone = session[:timezone] || current_user&.timezone

app/controllers/concerns/authenticated_controller_action.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module AuthenticatedControllerAction
+  extend ActiveSupport::Concern
+
+  def set_raven_user
+    return unless sentry_reporting_is_live?
+
+    if current_user
+      # Raven sends error info to Sentry.
+      Raven.user_context(
+        email: current_user.email,
+        css_id: current_user.css_id,
+        station_id: current_user.station_id,
+        regional_office: current_user.regional_office
+      )
+    end
+  end
+
+  def sentry_reporting_is_live?
+    ENV["SENTRY_DSN"]
+  end
+end

app/controllers/idt/api/v1/base_controller.rb

@@ -1,9 +1,12 @@
 # frozen_string_literal: true
 
 class Idt::Api::V1::BaseController < ActionController::Base
+  include AuthenticatedControllerAction
+
   protect_from_forgery with: :exception
   before_action :validate_token
   before_action :set_application
+  before_action :set_raven_user
 
   # :nocov:
   rescue_from StandardError do |error|
@@ -47,6 +50,9 @@ def user
     end
   end
 
+  # set_raven_user via AuthenticatedControllerAction expects a current_user
+  alias current_user user
+
   def set_application
     RequestStore.store[:application] = "idt"
   end

spec/controllers/idt/api/appeals_controller_spec.rb

@@ -623,6 +623,13 @@
     end
 
     context "when appeal has already been outcoded" do
+      before do
+        allow(controller).to receive(:sentry_reporting_is_live?) { true }
+        allow(Raven).to receive(:user_context) do |args|
+          @raven_user = args
+        end
+      end
+
       it "throws an error" do
         BvaDispatchTask.create_from_root_task(root_task)
         post :outcode, params: params
@@ -636,6 +643,7 @@
                         "Cannot outcode the same appeal and task combination more than once"
 
         expect(response_detail).to eq error_message
+        expect(@raven_user[:css_id]).to eq(user.css_id)
       end
     end
   end