From 971e0b13e49d6f8ff5007b7d9db4ab7002022768 Mon Sep 17 00:00:00 2001
From: "Michael R. Fleet" <f1337@users.noreply.github.com>
Date: Wed, 18 Aug 2021 11:35:55 -0400
Subject: [PATCH] rate-limit PPMS lookup to curb abuse (REDUX) (#7673)

* rate-limit PPMS lookup to curb abuse

Co-authored-by: Lindsey Hattamer <lindsey.hattamer@oddball.io>
Co-authored-by: Travis Hilton <travis.hilton@oddball.io>

* throttle Facility Locator by *remote_ip*, not *ip*

* test fixes for Facility Locator throttle

Co-authored-by: Lindsey Hattamer <lindsey.hattamer@oddball.io>
Co-authored-by: Riley Anderson <riley.anderson@oddball.io>

* rubocop

* pay rubocop toll twice

Co-authored-by: Lindsey Hattamer <lindsey.hattamer@oddball.io>
Co-authored-by: Travis Hilton <travis.hilton@oddball.io>
Co-authored-by: Riley Anderson <riley.anderson@oddball.io>
---
 config/initializers/rack_attack.rb  | 14 +++++++++++++
 spec/middleware/rack/attack_spec.rb | 31 +++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb
index 32c6d05bbae..63b24fb59e9 100644
--- a/config/initializers/rack_attack.rb
+++ b/config/initializers/rack_attack.rb
@@ -1,6 +1,13 @@
 # frozen_string_literal: true
 
 class Rack::Attack
+  # we're behind a load balancer and/or proxy, which is what request.ip returns
+  class Request < ::Rack::Request
+    def remote_ip
+      @remote_ip ||= (env['X-Real-Ip'] || ip).to_s
+    end
+  end
+
   # .to_h because hashes from config_for don't support non-symbol keys
   redis_options = REDIS_CONFIG[:redis].to_h
   Rack::Attack.cache.store = Rack::Attack::StoreProxy::RedisStoreProxy.new(Redis.new(redis_options))
@@ -9,6 +16,13 @@ class Rack::Attack
     req.ip if req.path == '/v0/limited'
   end
 
+  # Rate-limit PPMS lookup, in order to bore abusers.
+  # See https://github.com/department-of-veterans-affairs/va.gov-team-sensitive/blob/master/Postmortems/2021-08-16-facility-locator-possible-DOS.md
+  # for details.
+  throttle('facility_locator/ip', limit: 3, period: 1.minute) do |req|
+    req.remote_ip if req.path == '/facilities_api/v1/ccp/provider'
+  end
+
   throttle('vic_profile_photos_download/ip', limit: 8, period: 5.minutes) do |req|
     req.ip if req.path == '/v0/vic/profile_photo_attachments' && req.get?
   end
diff --git a/spec/middleware/rack/attack_spec.rb b/spec/middleware/rack/attack_spec.rb
index 6377f997455..920e16b48fc 100644
--- a/spec/middleware/rack/attack_spec.rb
+++ b/spec/middleware/rack/attack_spec.rb
@@ -94,6 +94,37 @@ def app
     end
   end
 
+  describe 'facility_locator/ip' do
+    let(:endpoint) { '/facilities_api/v1/ccp/provider' }
+    let(:headers) { { 'X-Real-Ip' => '1.2.3.4' } }
+    let(:limit) { 3 }
+
+    before do
+      limit.times do
+        get endpoint, nil, headers # rubocop:disable Rails/HttpPositionalArguments
+        expect(last_response.status).not_to eq(429)
+      end
+
+      get endpoint, nil, other_headers # rubocop:disable Rails/HttpPositionalArguments
+    end
+
+    context 'response status for repeated requests from the same IP' do
+      let(:other_headers) { headers }
+
+      it 'limits requests' do
+        expect(last_response.status).to eq(429)
+      end
+    end
+
+    context 'response status for request from different IP' do
+      let(:other_headers) { { 'X-Real-Ip' => '4.3.2.1' } }
+
+      it 'limits requests' do
+        expect(last_response.status).not_to eq(429)
+      end
+    end
+  end
+
   describe 'vic rate-limits', run_at: 'Thu, 26 Dec 2015 15:54:20 GMT' do
     before do
       limit.times do