From 78122df37e588b4bb1ad16ce3475e3164dff4d90 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:30:24 +1100 Subject: [PATCH 1/2] feat: throttle access token requests separately --- README.md | 2 + config/application.rb | 4 ++ config/initializers/rack_attack.rb | 29 +++++++++++-- test/api/auth_test.rb | 68 ++++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 23006732c..e11253894 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,8 @@ for `DF_SECRET_KEY_BASE`, `DF_SECRET_KEY_ATTR`, `DF_SECRET_KEY_DEVISE`, | `ONTRACK_AUTH_RATE_LIMIT_ENABLED` | Enable auth throttling; accepts true/false or 1/0. | `true` | | `ONTRACK_AUTH_RATE_LIMIT` | Maximum authentication requests per IP in one window. | `5` | | `ONTRACK_AUTH_RATE_PERIOD_SECONDS` | Authentication throttle window in seconds. | `20` | +| `ONTRACK_AUTH_REFRESH_RATE_LIMIT` | Maximum refresh requests per refresh cookie in one window. | `30` | +| `ONTRACK_AUTH_REFRESH_RATE_PERIOD_SECONDS` | Refresh throttle window in seconds. | `60` | | `DF_SECRET_KEY_BASE` | Rails secret key base fallback. | Required in production | | `DF_SECRET_KEY_ATTR` | Legacy encrypted-attribute key fallback. | Required in production | | `DF_SECRET_KEY_DEVISE` | Devise secret fallback. | Required in production | diff --git a/config/application.rb b/config/application.rb index 7a0d450d9..70b73dea4 100644 --- a/config/application.rb +++ b/config/application.rb @@ -45,8 +45,12 @@ class Application < Rails::Application config.auth_rate_limit_enabled = %w[true 1].include?(auth_rate_limit_enabled) config.auth_rate_limit = Integer(ENV.fetch('ONTRACK_AUTH_RATE_LIMIT', 5)) config.auth_rate_period_seconds = Integer(ENV.fetch('ONTRACK_AUTH_RATE_PERIOD_SECONDS', 20)) + config.auth_refresh_rate_limit = Integer(ENV.fetch('ONTRACK_AUTH_REFRESH_RATE_LIMIT', 30)) + config.auth_refresh_rate_period_seconds = Integer(ENV.fetch('ONTRACK_AUTH_REFRESH_RATE_PERIOD_SECONDS', 60)) raise ArgumentError, 'ONTRACK_AUTH_RATE_LIMIT must be positive' unless config.auth_rate_limit.positive? raise ArgumentError, 'ONTRACK_AUTH_RATE_PERIOD_SECONDS must be positive' unless config.auth_rate_period_seconds.positive? + raise ArgumentError, 'ONTRACK_AUTH_REFRESH_RATE_LIMIT must be positive' unless config.auth_refresh_rate_limit.positive? + raise ArgumentError, 'ONTRACK_AUTH_REFRESH_RATE_PERIOD_SECONDS must be positive' unless config.auth_refresh_rate_period_seconds.positive? # ==> Student work directory # File server location for storing student's work. Defaults to `student_work` diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 9ea4caa6f..86daaca53 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -1,5 +1,9 @@ +require 'digest' + module Rack class Attack + AUTH_REFRESH_PATHS = %w[/api/auth/access-token /api/auth/access-token.json].freeze + # Production uses the configured Rails cache: Redis when available, # otherwise an in-process memory store. Rack::Attack.cache.store = if Rails.env.production? @@ -8,10 +12,24 @@ class Attack ActiveSupport::Cache::MemoryStore.new end - # Limit repeated login attempts against the API auth endpoint. + # Limit repeated login attempts without counting routine access-token refreshes. throttle('auth/ip', limit: Doubtfire::Application.config.auth_rate_limit, period: Doubtfire::Application.config.auth_rate_period_seconds.seconds) do |req| - req.ip if Doubtfire::Application.config.auth_rate_limit_enabled && req.path.start_with?('/api/auth') && req.post? + next unless Doubtfire::Application.config.auth_rate_limit_enabled && req.post? && req.path.start_with?('/api/auth') + next if AUTH_REFRESH_PATHS.include?(req.path) + + req.ip + end + + # Limit repeated use of one refresh cookie without affecting other users of a shared IP. + throttle('auth/refresh/token', limit: Doubtfire::Application.config.auth_refresh_rate_limit, + period: Doubtfire::Application.config.auth_refresh_rate_period_seconds.seconds) do |req| + next unless Doubtfire::Application.config.auth_rate_limit_enabled && req.post? && AUTH_REFRESH_PATHS.include?(req.path) + + refresh_token = req.cookies['refresh_token'] + next if refresh_token.blank? || refresh_token.bytesize > 256 + + Digest::SHA256.hexdigest(refresh_token) end # Return a consistent API-friendly throttling response. @@ -29,7 +47,12 @@ class Attack now = match_data[:epoch_time] || Time.now.to_i retry_after = match_data[:period].to_i - (now % match_data[:period].to_i) if match_data[:period].to_i.positive? - body = { error: 'Too many login attempts. Please try again shortly.' }.to_json + message = if AUTH_REFRESH_PATHS.include?(rack_env['PATH_INFO']) + 'Too many token refresh attempts. Please try again shortly.' + else + 'Too many login attempts. Please try again shortly.' + end + body = { error: message }.to_json headers = { 'Content-Type' => 'application/json' } headers['Retry-After'] = retry_after.to_s if retry_after diff --git a/test/api/auth_test.rb b/test/api/auth_test.rb index b107a20e2..7b3357422 100644 --- a/test/api/auth_test.rb +++ b/test/api/auth_test.rb @@ -49,6 +49,74 @@ def test_auth_post_is_rate_limited Rack::Attack.cache.store.clear end + def test_access_token_refresh_does_not_use_login_rate_limit + original_setting = Doubtfire::Application.config.auth_rate_limit_enabled + Doubtfire::Application.config.auth_rate_limit_enabled = true + Rack::Attack.cache.store.clear + Rack::Attack.enabled = true + + limit = Doubtfire::Application.config.auth_rate_limit + user = FactoryBot.create(:user) + token = user.generate_authentication_token!(token_type: :refresh_token) + set_cookie "username=#{user.username}" + set_cookie "refresh_token=#{token.authentication_token}" + + (limit + 1).times do |index| + path = index.even? ? '/api/auth/access-token' : '/api/auth/access-token.json' + post path + assert_equal 201, last_response.status + end + + credentials = { username: 'aadmin123', password: 'password' } + limit.times do + post_json '/api/auth.json', credentials + assert_equal 401, last_response.status + end + + post_json '/api/auth.json', credentials + assert_equal 429, last_response.status + ensure + Doubtfire::Application.config.auth_rate_limit_enabled = original_setting + Rack::Attack.cache.store.clear + end + + def test_access_token_refresh_has_its_own_rate_limit + original_setting = Doubtfire::Application.config.auth_rate_limit_enabled + Doubtfire::Application.config.auth_rate_limit_enabled = true + Rack::Attack.cache.store.clear + Rack::Attack.enabled = true + + travel_to Time.zone.parse('2026-07-21 11:00:00 UTC') do + user = FactoryBot.create(:user) + token = user.generate_authentication_token!(token_type: :refresh_token) + set_cookie "username=#{user.username}" + set_cookie "refresh_token=#{token.authentication_token}" + + Doubtfire::Application.config.auth_refresh_rate_limit.times do + post '/api/auth/access-token' + assert_equal 201, last_response.status + end + + post '/api/auth/access-token' + assert_equal 429, last_response.status + assert_equal 'Too many token refresh attempts. Please try again shortly.', last_response_body['error'] + assert_operator last_response.headers['Retry-After'].to_i, :>, 0 + + other_user = FactoryBot.create(:user) + other_token = other_user.generate_authentication_token!(token_type: :refresh_token) + set_cookie "username=#{other_user.username}" + set_cookie "refresh_token=#{other_token.authentication_token}" + post '/api/auth/access-token' + assert_equal 201, last_response.status + + post_json '/api/auth.json', username: 'aadmin123', password: 'password' + assert_equal 401, last_response.status + end + ensure + Doubtfire::Application.config.auth_rate_limit_enabled = original_setting + Rack::Attack.cache.store.clear + end + def test_auth_post_is_not_rate_limited_when_disabled original_setting = Doubtfire::Application.config.auth_rate_limit_enabled Doubtfire::Application.config.auth_rate_limit_enabled = false From 009e9e5f7cc3c6ad9f7c6e6556c76142c3f50ec8 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:19:43 +1100 Subject: [PATCH 2/2] fix: throttle password logins per username and IP --- README.md | 2 +- config/initializers/rack_attack.rb | 20 ++++++++++++------ test/api/auth_test.rb | 34 ++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index e11253894..863f0cf22 100644 --- a/README.md +++ b/README.md @@ -90,7 +90,7 @@ for `DF_SECRET_KEY_BASE`, `DF_SECRET_KEY_ATTR`, `DF_SECRET_KEY_DEVISE`, | `DF_ACCESS_TOKEN_EXPIRY_SECONDS` | Access-token lifetime in seconds. | `7200` | | `DF_REFRESH_TOKEN_EXPIRY_SECONDS` | Refresh-token lifetime in seconds. | `604800` | | `ONTRACK_AUTH_RATE_LIMIT_ENABLED` | Enable auth throttling; accepts true/false or 1/0. | `true` | -| `ONTRACK_AUTH_RATE_LIMIT` | Maximum authentication requests per IP in one window. | `5` | +| `ONTRACK_AUTH_RATE_LIMIT` | Maximum password logins per username and IP in one window. | `5` | | `ONTRACK_AUTH_RATE_PERIOD_SECONDS` | Authentication throttle window in seconds. | `20` | | `ONTRACK_AUTH_REFRESH_RATE_LIMIT` | Maximum refresh requests per refresh cookie in one window. | `30` | | `ONTRACK_AUTH_REFRESH_RATE_PERIOD_SECONDS` | Refresh throttle window in seconds. | `60` | diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 86daaca53..f7b5c70bd 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -3,6 +3,7 @@ module Rack class Attack AUTH_REFRESH_PATHS = %w[/api/auth/access-token /api/auth/access-token.json].freeze + AUTH_LOGIN_PATHS = %w[/api/auth /api/auth.json].freeze # Production uses the configured Rails cache: Redis when available, # otherwise an in-process memory store. @@ -12,13 +13,20 @@ class Attack ActiveSupport::Cache::MemoryStore.new end - # Limit repeated login attempts without counting routine access-token refreshes. - throttle('auth/ip', limit: Doubtfire::Application.config.auth_rate_limit, - period: Doubtfire::Application.config.auth_rate_period_seconds.seconds) do |req| - next unless Doubtfire::Application.config.auth_rate_limit_enabled && req.post? && req.path.start_with?('/api/auth') - next if AUTH_REFRESH_PATHS.include?(req.path) + # Limit password guesses per username and IP, so users sharing a campus IP never block each other. + throttle('auth/username_ip', limit: Doubtfire::Application.config.auth_rate_limit, + period: Doubtfire::Application.config.auth_rate_period_seconds.seconds) do |req| + next unless Doubtfire::Application.config.auth_rate_limit_enabled && req.post? && AUTH_LOGIN_PATHS.include?(req.path) - req.ip + params = begin + ActionDispatch::Request.new(req.env).params + rescue ActionController::BadRequest, ActionDispatch::Http::Parameters::ParseError + {} + end + username = params['username'] + next unless params['password'].present? && username.is_a?(String) && username.present? + + "#{req.ip}:#{username.strip.downcase}" end # Limit repeated use of one refresh cookie without affecting other users of a shared IP. diff --git a/test/api/auth_test.rb b/test/api/auth_test.rb index 7b3357422..9ecb4728c 100644 --- a/test/api/auth_test.rb +++ b/test/api/auth_test.rb @@ -49,6 +49,40 @@ def test_auth_post_is_rate_limited Rack::Attack.cache.store.clear end + def test_auth_rate_limit_is_per_username_on_shared_ip + original_setting = Doubtfire::Application.config.auth_rate_limit_enabled + Doubtfire::Application.config.auth_rate_limit_enabled = true + Rack::Attack.cache.store.clear + Rack::Attack.enabled = true + + limit = Doubtfire::Application.config.auth_rate_limit + (limit + 1).times do + post_json '/api/auth.json', username: 'aadmin123', password: 'password' + end + assert_equal 429, last_response.status + + post_json '/api/auth.json', username: 'aadmin', password: 'password' + assert_equal 201, last_response.status + ensure + Doubtfire::Application.config.auth_rate_limit_enabled = original_setting + Rack::Attack.cache.store.clear + end + + def test_auth_token_exchange_is_not_rate_limited + original_setting = Doubtfire::Application.config.auth_rate_limit_enabled + Doubtfire::Application.config.auth_rate_limit_enabled = true + Rack::Attack.cache.store.clear + Rack::Attack.enabled = true + + (Doubtfire::Application.config.auth_rate_limit + 1).times do + post_json '/api/auth.json', username: 'aadmin', auth_token: 'invalid' + assert_equal 419, last_response.status + end + ensure + Doubtfire::Application.config.auth_rate_limit_enabled = original_setting + Rack::Attack.cache.store.clear + end + def test_access_token_refresh_does_not_use_login_rate_limit original_setting = Doubtfire::Application.config.auth_rate_limit_enabled Doubtfire::Application.config.auth_rate_limit_enabled = true