From 028a3caf0626daf1816eabe2a08ad8fed042d97f Mon Sep 17 00:00:00 2001
From: Jay Joshua <7008757+jayjay-w@users.noreply.github.com>
Date: Tue, 23 Jan 2024 14:11:31 +0100
Subject: [PATCH] FInalize on throttling and rate limiting

- Moved limits to the configuration file
- Added tests for all limits
---
 config/config.yml.example                    |  4 +++
 config/environments/test.rb                  |  3 +-
 config/initializers/devise.rb                |  4 +--
 config/initializers/rack_attack.rb           |  9 ++++--
 test/controllers/sessions_controller_test.rb |  6 ++--
 test/lib/check_rack_attack_test.rb           | 31 ++++++++++++++++++++
 6 files changed, 49 insertions(+), 8 deletions(-)
 create mode 100644 test/lib/check_rack_attack_test.rb

diff --git a/config/config.yml.example b/config/config.yml.example
index 4d5602cbb4..35167e9f3c 100644
--- a/config/config.yml.example
+++ b/config/config.yml.example
@@ -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
diff --git a/config/environments/test.rb b/config/environments/test.rb
index 592c08505d..6e70ec23e3 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -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
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index 6e4d570a06..eea9b2b756 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -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
diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb
index 2d46ef7b52..986cc16f6e 100644
--- a/config/initializers/rack_attack.rb
+++ b/config/initializers/rack_attack.rb
@@ -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
diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb
index 50cdf22e33..74ae77bd1c 100644
--- a/test/controllers/sessions_controller_test.rb
+++ b/test/controllers/sessions_controller_test.rb
@@ -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: 'test@test.com'
     u.confirm
-    maximum_attempts = Devise.maximum_attempts
+    Devise.maximum_attempts = 2
 
-    maximum_attempts.times do
+    2.times do
       post :create, params: { api_user: { email: 'test@test.com', password: '12345679' } }
     end
 
@@ -94,7 +94,7 @@ def setup
       post :create, params: { api_user: { email: 'test@test.com', 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
diff --git a/test/lib/check_rack_attack_test.rb b/test/lib/check_rack_attack_test.rb
new file mode 100644
index 0000000000..99017b2c8f
--- /dev/null
+++ b/test/lib/check_rack_attack_test.rb
@@ -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: 'user@example.com', 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