Skip to content

Commit

Permalink
FInalize on throttling and rate limiting
Browse files Browse the repository at this point in the history
- Moved limits to the configuration file
- Added tests for all limits
  • Loading branch information
jayjay-w committed Jan 23, 2024
1 parent 0b658cd commit ae61d9a
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 10 deletions.
4 changes: 4 additions & 0 deletions config/config.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ development: &default
#
tipline_user_max_messages_per_day: 1500

devise_maximum_attempts: 5
devise_unlock_accounts_after: 1
login_rate_limit: 10
api_rate_limit: 100
test:
<<: *default
checkdesk_base_url_private: http://api:3000
Expand Down
2 changes: 0 additions & 2 deletions config/environments/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,4 @@
else
puts '[WARNING] config.hosts not provided. Only requests from localhost are allowed. To change, update `whitelisted_hosts` in config.yml'
end

config.hosts << "api.check.orb.local"
end
3 changes: 2 additions & 1 deletion config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@

# Show full error reports and disable caching.
config.consider_all_requests_local = true
config.action_controller.perform_caching = false
config.action_controller.perform_caching = true
config.cache_store = :memory_store

# Raise exceptions instead of rendering exception templates.
config.action_dispatch.show_exceptions = false
Expand Down
4 changes: 2 additions & 2 deletions config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ def http_auth_body
config.lock_strategy = :failed_attempts
config.unlock_strategy = :both
config.unlock_keys = [ :time ]
config.maximum_attempts = 5
config.unlock_in = 1.hour
config.maximum_attempts = CheckConfig.get('devise_maximum_attempts', 5).to_i
config.unlock_in = CheckConfig.get('devise_unlock_accounts_after', 1).to_i.hour
end

AuthTrail.geocode = false
Expand Down
9 changes: 7 additions & 2 deletions config/initializers/rack_attack.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
class Rack::Attack
# Throttle login attempts by IP address
throttle('logins/ip', limit: 10, period: 60.seconds) do |req|
throttle('logins/ip', limit: ChheckConfig.get('login_rate_limit', 10).to_i, period: 60.seconds) do |req|
if req.path == '/api/users/sign_in' && req.post?
req.ip
end
end

# Throttle login attempts by email address
throttle('logins/email', limit: 10, period: 60.seconds) do |req|
throttle('logins/email', limit: ChheckConfig.get('login_rate_limit', 10).to_i, period: 60.seconds) do |req|
if req.path == '/api/users/sign_in' && req.post?
# Return the email if present, nil otherwise
req.params['user']['email'].presence if req.params['user']
end
end

# Throttle all graphql requests by IP address
throttle('api/graphql', limit: ChheckConfig.get('api_rate_limit', 100).to_i, period: 60.seconds) do |req|
req.ip if req.path == '/api/graphql'
end
end
6 changes: 3 additions & 3 deletions test/controllers/sessions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ def setup
test "should lock user after excessive login requests" do
u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: '[email protected]'
u.confirm
maximum_attempts = Devise.maximum_attempts
Devise.maximum_attempts = 2

maximum_attempts.times do
2.times do
post :create, params: { api_user: { email: '[email protected]', password: '12345679' } }
end

Expand All @@ -94,7 +94,7 @@ def setup
post :create, params: { api_user: { email: '[email protected]', password: '12345679' } }
end

travel_to 2.hours.from_now do
travel_to CheckConfig.get('devise_unlock_accounts_after', 5).to_i + 1.hours.from_now do
u.reload
assert !u.access_locked?
assert_nil u.locked_at
Expand Down
31 changes: 31 additions & 0 deletions test/lib/check_rack_attack_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
require 'test_helper'

class ThrottlingTest < ActionDispatch::IntegrationTest
setup do
Rails.cache.clear
end

test "should throttle excessive requests to /api/graphql" do
limit = 100

limit.times do
post api_graphql_path
assert_response :unauthorized
end

get api_graphql_path
assert_response :too_many_requests
end

test "should throttle excessive requests to /api/users/sign_in" do
limit = 10
user_params = { api_user: { email: '[email protected]', password: 'password' } }

limit.times do
post api_user_session_path, params: user_params, as: :json
end

post api_user_session_path, params: user_params, as: :json
assert_response :too_many_requests
end
end

0 comments on commit ae61d9a

Please sign in to comment.