Skip to content

Commit

Permalink
rate-limit PPMS lookup to curb abuse (REDUX) (#7673)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
4 people authored Aug 18, 2021
1 parent f89182f commit 971e0b1
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
14 changes: 14 additions & 0 deletions config/initializers/rack_attack.rb
Original file line number Diff line number Diff line change
@@ -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))
Expand All @@ -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
Expand Down
31 changes: 31 additions & 0 deletions spec/middleware/rack/attack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 971e0b1

Please sign in to comment.