Merge pull request #2153 from 18F/mb-ensure-uuid-lg-244

LG-244 Exclude events with invalid tokens

app/views/exception_notifier/_data.text.erb

@@ -1,4 +1,4 @@
-<% job = @data[:sidekiq][:job] %>
+<% job = @data.dig(:sidekiq, :job) || {} %>
 
 Job class: <%= job['wrapped'] %>
 Queue: <%= job['queue'] %>

config/initializers/ahoy.rb

@@ -21,7 +21,11 @@ def track_event(data)
     def exclude?
       return if FeatureManagement.enable_load_testing_mode?
       return if FeatureManagement.use_dashboard_service_providers?
-      super
+      # Ahoy visitor_token and visit_token are read from request headers
+      # and cookies, and pentesters often send cURL requests with
+      # bogus token values. We want to exclude events where tokens are
+      # invalid UUIDs.
+      super || invalid_uuid?(ahoy.visitor_token) || invalid_uuid?(ahoy.visit_token)
     end
 
     protected
@@ -41,5 +45,15 @@ def visit_logger
     def event_logger
       @event_logger ||= ActiveSupport::Logger.new(Rails.root.join('log', 'events.log'))
     end
+
+    def invalid_uuid?(token)
+      # The match? method does not exist for the Regexp class in Ruby < 2.4
+      # Here, it comes from Active Support. Once we upgrade to Ruby 2.5,
+      # we probably want to ignore the Rails definition and use Ruby's.
+      # To do that, we'll need to set `config.active_support.bare = true`,
+      # and then only require the extensions we use.
+      uuid_regex = /\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/
+      !uuid_regex.match?(token)
+    end
   end
 end

spec/config/initializers/ahoy_spec.rb

@@ -0,0 +1,57 @@
+require 'rails_helper'
+
+describe Ahoy::Store do
+  context 'visit_token is an invalid UUID' do
+    it 'excludes the event' do
+      MockAhoy = Struct.new(:visit_token, :visitor_token)
+      mock_ahoy = MockAhoy.new('foo', '1056d484-194c-4b8c-978d-0c0f57958f04')
+      store = Ahoy::Store.new(ahoy: mock_ahoy)
+
+
+      expect(store.exclude?).to eq true
+    end
+  end
+
+  context 'visitor_token is an invalid UUID' do
+    it 'excludes the event' do
+      MockAhoy = Struct.new(:visit_token, :visitor_token)
+      mock_ahoy = MockAhoy.new('1056d484-194c-4b8c-978d-0c0f57958f04', 'foo')
+      store = Ahoy::Store.new(ahoy: mock_ahoy)
+
+
+      expect(store.exclude?).to eq true
+    end
+  end
+
+  context 'both visitor_token and visit_token are a valid UUID' do
+    it 'does not exclude the event' do
+      MockAhoy = Struct.new(:visit_token, :visitor_token)
+      mock_ahoy = MockAhoy.new(
+        '1056d484-194c-4b8c-978d-0c0f57958f04', '1056d484-194c-4b8c-978d-0c0f57958f04'
+      )
+      store = Ahoy::Store.new(ahoy: mock_ahoy)
+
+
+      expect(store.exclude?).to eq false
+    end
+  end
+
+  context 'FeatureManagement.enable_load_testing_mode? is true' do
+    it 'does not exclude the event' do
+      allow(FeatureManagement).to receive(:enable_load_testing_mode?).and_return(true)
+      store = Ahoy::Store.new({})
+
+      expect(store.exclude?).to be_nil
+    end
+  end
+
+  context 'FeatureManagement.use_dashboard_service_providers? is true' do
+    it 'does not exclude the event' do
+      allow(FeatureManagement).to receive(:use_dashboard_service_providers?).
+        and_return(true)
+      store = Ahoy::Store.new({})
+
+      expect(store.exclude?).to be_nil
+    end
+  end
+end