From 75b22bf924d17ebc81925283c0ffc055bddf866d Mon Sep 17 00:00:00 2001 From: Jeremy Prevost Date: Thu, 16 Jul 2026 09:28:13 -0400 Subject: [PATCH] Turnstile verified users bypass req/ip throttling Why are these changes being introduced: * Actual users were being blocked when switching tabs rapidly * We don't want to increase the throttle limit for everyone, so we will bypass the throttle for verified users Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/USE-647 How does this address that need: * Abstracts list of throttles that allow turnstile bypass * Adds a check for turnstile verified users to bypass the req/ip and previous throttles they could bypass updated to use new abstraction * Adjusts which routes bypass the req/ip throttle using new abstraction (static pages don't need throttling) Document any side effects to this change: * Bots or absusive users that can pass the turnstile check now have more leeway to make requests --- config/initializers/rack_attack.rb | 83 +++++++++++-------- .../rack_attack_cookie_bypass_test.rb | 73 ++++++++++++++-- .../rack_attack_throttle_429_test.rb | 17 ++-- 3 files changed, 123 insertions(+), 50 deletions(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index ff1cd51f..8336f64a 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -1,4 +1,34 @@ class Rack::Attack + # List of throttles that honor the Turnstile grace cookie. These throttles will redirect to the Turnstile challenge + # page instead of returning 429, allowing verified users to continue during the grace period without increasing + # limits for unverified traffic. + TURNSTILE_CHALLENGE_THROTTLES = %w[results/global req/ip/results req/ip].freeze + + # List of paths that are cheap/free to serve and should not count against the per-IP throttle limit. + REQ_IP_FREE_PATHS = %w[/ /assets /turnstile /about /about-natural-language-search].freeze + + # Returns true when the request includes the plain Turnstile grace cookie set after + # a successful challenge, and that cookie has a valid Rails signature with a future + # expiration timestamp. Rack::Attack runs before Rails controller/session handling, + # so recoverable throttles use this middleware-readable proof to let verified users + # continue during the grace period without increasing limits for unverified traffic. + def self.turnstile_grace_cookie_valid?(req) + cookie_value = req.cookies['turnstile_verified_at'] + return false if cookie_value.blank? + + expiration_timestamp = Rails.application.message_verifier(:turnstile_grace).verify(cookie_value) + expiration_timestamp > Time.current.to_i + rescue ActiveSupport::MessageVerifier::InvalidSignature + false + end + + # Returns true if the request path is in the list of cheap/free paths that should not count against the per-IP + # throttle limit. + def self.req_ip_free_path?(path) + REQ_IP_FREE_PATHS.any? do |free_path| + path == free_path || (free_path != '/' && path.start_with?("#{free_path}/")) + end + end ### Configure Cache ### @@ -62,21 +92,12 @@ class Rack::Attack throttle('results/global', limit: (ENV.fetch('RESULTS_GLOBAL_LIMIT_PER_SEC', 30)).to_i, period: 1.second) do |req| + # Only apply to /results and /record endpoints next nil unless req.path.start_with?('/results') || req.path.start_with?('/record') - # Skip throttling if this IP recently passed Turnstile verification. - # Grace period is stored in a signed cookie that survives Redis eviction. - # The signature prevents clients from forging a far-future timestamp to bypass throttles. - cookie_value = req.cookies['turnstile_verified_at'] - if cookie_value.present? - begin - expiration_timestamp = Rails.application.message_verifier(:turnstile_grace).verify(cookie_value) - next nil if expiration_timestamp > Time.current.to_i - rescue ActiveSupport::MessageVerifier::InvalidSignature - # Tampered or invalid cookie — proceed with throttling - end - end + # Skip throttling if the user has a valid Turnstile grace cookie + next nil if Rack::Attack.turnstile_grace_cookie_valid?(req) # Use a constant key so this is a true global limit, not per-IP 'results' @@ -99,31 +120,25 @@ class Rack::Attack # Only apply to /results and /record endpoints next nil unless req.path.start_with?('/results') || req.path.start_with?('/record') - # Skip throttling if this IP recently passed Turnstile verification. - # Grace period is stored in a signed cookie that survives Redis eviction. - # The signature prevents clients from forging a far-future timestamp to bypass throttles. - cookie_value = req.cookies['turnstile_verified_at'] - if cookie_value.present? - begin - expiration_timestamp = Rails.application.message_verifier(:turnstile_grace).verify(cookie_value) - next nil if expiration_timestamp > Time.current.to_i - rescue ActiveSupport::MessageVerifier::InvalidSignature - # Tampered or invalid cookie — proceed with throttling - end - end + # Skip throttling if the user has a valid Turnstile grace cookie + next nil if Rack::Attack.turnstile_grace_cookie_valid?(req) req.ip end - # Throttle all requests by IP (default is 100 requests per 10 minutes) - # Excludes /assets and /turnstile paths (users need to access Turnstile to verify and bypass throttles) + # Throttle all requests by IP (default is 100 requests per 10 minutes). + # Excludes cheap/free paths, and skips users with a valid Turnstile grace cookie. # # Key: "rack::attack:#{Time.now.to_i/:period}:req/ip:#{req.ip}" throttle('req/ip', limit: (ENV.fetch('REQUESTS_PER_PERIOD', 100)).to_i, period: (ENV.fetch('REQUEST_PERIOD', 10)).to_i.minutes) do |req| - # don't include assets or turnstile verification paths as requests - req.ip unless req.path.start_with?('/assets') || req.path.start_with?('/turnstile') + + # Skip throttling if the user has a valid Turnstile grace cookie + next nil if Rack::Attack.turnstile_grace_cookie_valid?(req) + + # don't include cheap static pages or turnstile verification paths as requests + req.ip unless Rack::Attack.req_ip_free_path?(req.path) end # Throttle redirects by IP (default is 5 per 10 minutes) @@ -170,14 +185,14 @@ class Rack::Attack ### Custom Throttle Response ### - # Redirect /results and /record throttles to Turnstile challenge instead of 429. + # Redirect user-recoverable throttles to Turnstile challenge instead of 429. # This allows real users to solve a CAPTCHA and continue, rather than getting # hard-blocked. This is more user-friendly for tuning since we can't perfectly # distinguish bots from heavy legitimate usage. # - # IMPORTANT: Only redirect if the matched throttle is one that has a grace period cache check - # (results/global or req/ip/results). Other throttles (req/ip, etc.) don't have grace period - # exemptions, so redirecting would create an infinite loop. + # IMPORTANT: Only redirect throttles that honor the Turnstile grace cookie. Other throttles + # return 429 because passing Turnstile would not bypass them. Turnstile throttles are set + # via TURNSTILE_CHALLENGE_THROTTLES constant at the top of this file. # # For throttles without grace period support, return 429 instead. self.throttled_responder = lambda do |env| @@ -189,9 +204,7 @@ class Rack::Attack # Log all throttled requests to understand traffic patterns Rails.logger.warn("THROTTLED_REQUEST: UA=#{request.user_agent.inspect} | IP=#{request.ip} | Path=#{request.path.inspect} | Throttle=#{matched_throttle.inspect}") - # Only redirect to Turnstile for /results and /record if it's a throttle with grace period support - if (request.path.start_with?('/results') || request.path.start_with?('/record')) && - (matched_throttle == 'results/global' || matched_throttle == 'req/ip/results') + if TURNSTILE_CHALLENGE_THROTTLES.include?(matched_throttle) # Redirect to Turnstile challenge return_to = "#{request.path_info}?#{request.query_string}".gsub(/\?$/, '') [ 302, diff --git a/test/integration/rack_attack_cookie_bypass_test.rb b/test/integration/rack_attack_cookie_bypass_test.rb index 9afcf948..093a67ae 100644 --- a/test/integration/rack_attack_cookie_bypass_test.rb +++ b/test/integration/rack_attack_cookie_bypass_test.rb @@ -69,14 +69,43 @@ def trigger_throttle_via_requests(endpoint_path, count, cookie_value = nil, para end end + def turnstile_cookie(expiration_time) + Rails.application.message_verifier(:turnstile_grace).generate(expiration_time.to_i) + end + + def rack_request_with_turnstile_cookie(cookie_value = nil) + cookie_header = cookie_value ? "turnstile_verified_at=#{cookie_value}" : nil + Rack::Request.new('HTTP_COOKIE' => cookie_header) + end + + test 'turnstile_grace_cookie_valid? validates signed future expiration cookie' do + refute Rack::Attack.turnstile_grace_cookie_valid?(rack_request_with_turnstile_cookie) + refute Rack::Attack.turnstile_grace_cookie_valid?(rack_request_with_turnstile_cookie('tampered-value')) + refute Rack::Attack.turnstile_grace_cookie_valid?(rack_request_with_turnstile_cookie(turnstile_cookie(Time.current - 1.minute))) + + assert Rack::Attack.turnstile_grace_cookie_valid?(rack_request_with_turnstile_cookie(turnstile_cookie(Time.current + 15.minutes))) + end + + test 'req_ip_free_path? identifies cheap paths excluded from general request throttle' do + assert Rack::Attack.req_ip_free_path?('/') + assert Rack::Attack.req_ip_free_path?('/assets/application.css') + assert Rack::Attack.req_ip_free_path?('/turnstile/verify') + assert Rack::Attack.req_ip_free_path?('/about') + assert Rack::Attack.req_ip_free_path?('/about-natural-language-search') + + refute Rack::Attack.req_ip_free_path?('/results') + refute Rack::Attack.req_ip_free_path?('/record/test-id') + refute Rack::Attack.req_ip_free_path?('/style-guide') + refute Rack::Attack.req_ip_free_path?('/aboutness') + end + test 'valid grace period cookie bypasses throttle when throttle is active' do # Get the throttle limit from environment or use default limit = ENV.fetch('RESULTS_GLOBAL_LIMIT_PER_SEC', 30).to_i # Create a valid, non-expired cookie future_timestamp = (Time.current + 15.minutes).to_i - verifier = Rails.application.message_verifier(:turnstile_grace) - valid_cookie = verifier.generate(future_timestamp) + valid_cookie = turnstile_cookie(future_timestamp) # Make requests WITHOUT cookie to build up throttle counter trigger_throttle_via_requests('/results', limit + 5, params: { q: 'test', tab: 'primo' }) @@ -113,8 +142,7 @@ def trigger_throttle_via_requests(endpoint_path, count, cookie_value = nil, para # Create a validly-signed but expired cookie past_timestamp = (Time.current - 1.minute).to_i - verifier = Rails.application.message_verifier(:turnstile_grace) - expired_cookie = verifier.generate(past_timestamp) + expired_cookie = turnstile_cookie(past_timestamp) # Build up throttle counter trigger_throttle_via_requests('/results', limit + 5, params: { q: 'test', tab: 'primo' }) @@ -166,8 +194,7 @@ def trigger_throttle_via_requests(endpoint_path, count, cookie_value = nil, para limit = ENV.fetch('RESULTS_THROTTLE_LIMIT', 10).to_i future_timestamp = (Time.current + 15.minutes).to_i - verifier = Rails.application.message_verifier(:turnstile_grace) - valid_cookie = verifier.generate(future_timestamp) + valid_cookie = turnstile_cookie(future_timestamp) # Build up throttle counter for /record endpoint trigger_throttle_via_requests('/record/test-id', limit + 5, params: {}) @@ -178,4 +205,38 @@ def trigger_throttle_via_requests(endpoint_path, count, cookie_value = nil, para # Should NOT be throttled assert_response :success, 'Valid grace period cookie should bypass throttle on /record' end + + test 'valid cookie bypasses general req/ip throttle' do + limit = ENV.fetch('REQUESTS_PER_PERIOD', 100).to_i + valid_cookie = turnstile_cookie(Time.current + 15.minutes) + + trigger_throttle_via_requests('/style-guide', limit + 5, params: {}) + + get '/style-guide', headers: { 'HTTP_COOKIE' => "turnstile_verified_at=#{valid_cookie}" } + + assert_response :success, 'Valid grace period cookie should bypass the general req/ip throttle' + end + + test 'invalid cookie does not bypass general req/ip throttle and redirects to Turnstile' do + limit = ENV.fetch('REQUESTS_PER_PERIOD', 100).to_i + + trigger_throttle_via_requests('/style-guide', limit + 5, params: {}) + + get '/style-guide', headers: { 'HTTP_COOKIE' => 'turnstile_verified_at=tampered-value' } + + assert_equal 302, status, 'Invalid cookie should not bypass the general req/ip throttle' + assert response.location.include?('/turnstile'), 'Should redirect to Turnstile on throttle' + end + + test 'free paths do not count against general req/ip throttle' do + limit = ENV.fetch('REQUESTS_PER_PERIOD', 100).to_i + + [ '/', '/about', '/about-natural-language-search' ].each do |path| + trigger_throttle_via_requests(path, limit + 5, params: {}) + end + + get '/style-guide' + + assert_response :success, 'Cheap/free paths should not increment the general req/ip throttle counter' + end end diff --git a/test/integration/rack_attack_throttle_429_test.rb b/test/integration/rack_attack_throttle_429_test.rb index ac7afb98..3366d4ba 100644 --- a/test/integration/rack_attack_throttle_429_test.rb +++ b/test/integration/rack_attack_throttle_429_test.rb @@ -1,8 +1,8 @@ require 'test_helper' class RackAttackThrottle429Test < ActionDispatch::IntegrationTest - # Test that throttles without grace period support (req/ip, req/ip/redirects) - # return a 429 response instead of redirecting to Turnstile. + # Test that throttles with grace period support redirect to Turnstile, while + # throttles without grace period support return a 429 response. # # Note: These tests use mocking to simulate Rack::Attack throttle conditions # because env vars are read at class load time, not request time. We test the @@ -21,15 +21,14 @@ def build_mock_env(path: '/results', query_string: '', matched_throttle: 'req/ip } end - test 'req/ip throttle returns 429 (not Turnstile redirect)' do - # The req/ip throttle has no grace period, so it should return 429 + test 'req/ip throttle redirects to Turnstile' do env = build_mock_env(path: '/about', matched_throttle: 'req/ip') - status, headers, body = Rack::Attack.throttled_responder.call(env) + status, headers, _body = Rack::Attack.throttled_responder.call(env) - assert_equal 429, status - assert_equal 'text/plain', headers['Content-Type'] - assert_equal ['Too Many Requests'], body + assert_equal 302, status + assert headers['Location'].include?('/turnstile') + assert headers['Location'].include?('return_to=') end test 'req/ip/redirects throttle returns 429 (not Turnstile redirect)' do @@ -84,7 +83,7 @@ def build_mock_env(path: '/results', query_string: '', matched_throttle: 'req/ip test '429 response includes plain text content type and body' do # Verify the exact response format for 429 errors - env = build_mock_env(path: '/api/endpoint', matched_throttle: 'req/ip') + env = build_mock_env(path: '/api/endpoint', matched_throttle: 'unknown/throttle') status, headers, body = Rack::Attack.throttled_responder.call(env)