From ea3dda093aabae970ba60270f83ba4e28944167b Mon Sep 17 00:00:00 2001 From: Matthew Bernhardt Date: Mon, 2 Nov 2026 15:28:28 -0500 Subject: [PATCH 1/2] Add support for format token in request header ** Why are these changes being introduced: We want to allow Quepid to provide the format token via a request header, rather than via the query parameters. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/use-660 ** How does this address that need: This updates the search controller to check either the params or the request header for the format token, preferring the headers first. ** Document any side effects to this change: This adds pretty minimal test coverage for the request header for now, but these tests will be re-written more heavily when we go back through and remove support for query parameters (assuming we take that step - it might be good to keep that support around for browser-based debugging) --- app/controllers/search_controller.rb | 8 ++++--- test/controllers/search_controller_test.rb | 27 +++++++++++++++++++++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 4e9de199..58e169c4 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -716,15 +716,17 @@ def authorized_request? # format_tokens_defined? confirms whether a format token is defined in both the environment and query params. # Returns a boolean. def format_tokens_defined? - return true if ENV.fetch('FORMAT_TOKEN', '').present? && params.key?(:format_token) + return false unless ENV.fetch('FORMAT_TOKEN', '').present? - false + request.headers.key?('HTTP_FORMAT_TOKEN') || params.key?(:format_token) end # valid_token? confirms whether the token received from the user matches the one defined in the environment. Returns # a boolean. def valid_token? - return true if ActiveSupport::SecurityUtils.secure_compare(params[:format_token], ENV.fetch('FORMAT_TOKEN', '')) + candidate = request.headers['HTTP_FORMAT_TOKEN'].presence || params[:format_token] + + return true if ActiveSupport::SecurityUtils.secure_compare(candidate, ENV.fetch('FORMAT_TOKEN', '')) false end diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb index 4e0efae6..0b2ba9c5 100644 --- a/test/controllers/search_controller_test.rb +++ b/test/controllers/search_controller_test.rb @@ -1888,7 +1888,7 @@ def source_filter_count(controller) 'Expected tuning parameters to affect results, but counts and title orders are identical.') end - test 'results can be returned in JSON format when env is set and valid token is provided' do + test 'results can be returned in JSON format when env is set and valid token is provided via param' do secret_value = 'sooper_sekret' quepid_ua = 'Quepid/1.0 (Web Scraper)' ClimateControl.modify FORMAT_TOKEN: secret_value do @@ -1903,6 +1903,21 @@ def source_filter_count(controller) end end + test 'results can be returned in JSON format when env is set and valid token is provided via header' do + secret_value = 'sooper_sekret' + quepid_ua = 'Quepid/1.0 (Web Scraper)' + ClimateControl.modify FORMAT_TOKEN: secret_value do + mock_timdex_search_with_hits(10) + get "/results?q=test&format=json", headers: { 'HTTP_USER_AGENT' => quepid_ua, 'HTTP_FORMAT_TOKEN' => secret_value } + assert_response :success + assert_equal 'application/json; charset=utf-8', response.content_type + response_json = JSON.parse(response.body) + assert_equal %w[results hits errors], response_json.keys + assert_instance_of Array, response_json['results'] + assert_instance_of Integer, response_json['hits'] + end + end + test 'results in JSON format will report zero hit count in the face of missing data' do SearchController.any_instance.stubs(:load_timdex_results).returns(nil) secret_value = 'sooper_sekret' @@ -1971,8 +1986,13 @@ def source_filter_count(controller) test 'requests for JSON results when env var is not set generate an unauthorized error' do secret_value = 'irrelevant' ClimateControl.modify FORMAT_TOKEN: '' do + # via param get "/results?q=test&format=json&format_token=#{secret_value}" assert_response :unauthorized + + # via headers + get '/results?q=test&format=json', headers: { 'HTTP_FORMAT_TOKEN' => secret_value } + assert_response :unauthorized end end @@ -1983,8 +2003,13 @@ def source_filter_count(controller) refute_equal secret_value, wrong_secret ClimateControl.modify FORMAT_TOKEN: secret_value do + # via param get "/results?q=test&format=json&format_token=#{wrong_secret}" assert_response :unauthorized + + # via headers + get '/results?q=test&format=json', headers: { 'HTTP_FORMAT_TOKEN' => wrong_secret } + assert_response :unauthorized end end From ef71e3017365bd5465b206caf70515b05be80cf4 Mon Sep 17 00:00:00 2001 From: Matthew Bernhardt Date: Tue, 3 Nov 2026 11:53:21 -0500 Subject: [PATCH 2/2] Respond to automated code review Changes here are: - protect against blank format_tokens - extend support for request headers to rack attack integration - add tests for both rack attack and controller to clarify precedence between the header and params (header should govern if both are supplied) --- app/controllers/search_controller.rb | 2 +- config/initializers/rack_attack.rb | 5 +- test/controllers/search_controller_test.rb | 49 ++++++++++++++++++- .../rack_attack_format_token_bypass_test.rb | 40 ++++++++++++++- 4 files changed, 90 insertions(+), 6 deletions(-) diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 58e169c4..425c792f 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -718,7 +718,7 @@ def authorized_request? def format_tokens_defined? return false unless ENV.fetch('FORMAT_TOKEN', '').present? - request.headers.key?('HTTP_FORMAT_TOKEN') || params.key?(:format_token) + request.headers['HTTP_FORMAT_TOKEN'].presence || params[:format_token].presence end # valid_token? confirms whether the token received from the user matches the one defined in the environment. Returns diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index ff964d0e..0baa7aee 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -40,12 +40,13 @@ def self.req_ip_free_path?(path) end # Returns true when the request includes a valid format token that matches the FORMAT_TOKEN environment variable. - # This allows programmatic API requests (with valid tokens) to bypass throttling. + # This supports tokens supplied via request header or via query parameter. This allows programmatic API requests (with + # valid tokens) to bypass throttling. def self.valid_format_token?(req) format_token_env = ENV.fetch('FORMAT_TOKEN', '') return false if format_token_env.blank? - format_token_param = req.params['format_token'] + format_token_param = req.env['HTTP_FORMAT_TOKEN'].presence || req.params['format_token'] return false if format_token_param.blank? ActiveSupport::SecurityUtils.secure_compare(format_token_param, format_token_env) diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb index 0b2ba9c5..cda87074 100644 --- a/test/controllers/search_controller_test.rb +++ b/test/controllers/search_controller_test.rb @@ -1942,7 +1942,7 @@ def source_filter_count(controller) end end - test 'results can be returned in JSON format when env is set and valid token is provided even with bot challenge' do + test 'results returned in JSON format when valid token is provided via querystring even with bot challenge' do secret_value = 'sooper_sekret' quepid_ua = 'Quepid/1.0 (Web Scraper)' @@ -1958,6 +1958,22 @@ def source_filter_count(controller) end end + test 'results returned in JSON format when valid token is provided via headers even with bot challenge' do + secret_value = 'sooper_sekret' + quepid_ua = 'Quepid/1.0 (Web Scraper)' + + ClimateControl.modify(FORMAT_TOKEN: secret_value, FEATURE_BOT_DETECTION: 'true') do + mock_timdex_search_with_hits(10) + get "/results?q=test&format=json", headers: { 'HTTP_FORMAT_TOKEN' => secret_value, 'HTTP_USER_AGENT' => quepid_ua } + assert_response :success + assert_equal 'application/json; charset=utf-8', response.content_type + response_json = JSON.parse(response.body) + assert_equal %w[results hits errors], response_json.keys + assert_instance_of Array, response_json['results'] + assert_instance_of Integer, response_json['hits'] + end + end + test 'results can be returned in JSON format when env is set while load-more pagination is enabled' do secret_value = 'sooper_sekret' quepid_ua = 'Quepid/1.0 (Web Scraper)' @@ -1982,6 +1998,20 @@ def source_filter_count(controller) end end + # We don't mock anything here because the error prevents any external lookups + test 'requests for JSON results with an empty token generate an unauthorized error' do + secret_value = 'sooper_sekret' + ClimateControl.modify FORMAT_TOKEN: secret_value do + get '/results?q=test&format=json', headers: { 'HTTP_FORMAT_TOKEN' => '' } + + assert_response :unauthorized + + get '/results?q=test&format=json&format_token=' + + assert_response :unauthorized + end + end + # We don't mock anything here because the error prevents any external lookups test 'requests for JSON results when env var is not set generate an unauthorized error' do secret_value = 'irrelevant' @@ -2013,6 +2043,23 @@ def source_filter_count(controller) end end + test 'requests for JSON results assign precedence to request header over query params if both present' do + secret_value = 'sooper_sekret' + wrong_secret = 'something_else' + refute_equal secret_value, wrong_secret + + ClimateControl.modify FORMAT_TOKEN: secret_value do + # wrong header, correct query params - results in an unauthorized request + get "/results?q=test&format=json&format_token=#{secret_value}", headers: { 'HTTP_FORMAT_TOKEN' => wrong_secret } + assert_response :unauthorized + + # correct header, wrong query params - results in success. Mocked response because we just test for success. + mock_timdex_search_with_hits(10) + get "/results?q=test&format=json&format_token=#{wrong_secret}", headers: { 'HTTP_FORMAT_TOKEN' => secret_value } + assert_response :success + end + end + test 'requests with unsupported format receive a 406 not-acceptable error' do mock_timdex_search_with_hits(10) get '/results?q=test&format=xml' diff --git a/test/integration/rack_attack_format_token_bypass_test.rb b/test/integration/rack_attack_format_token_bypass_test.rb index d852769b..412a9aa1 100644 --- a/test/integration/rack_attack_format_token_bypass_test.rb +++ b/test/integration/rack_attack_format_token_bypass_test.rb @@ -31,6 +31,10 @@ def rack_request_with_format_token(token_value = nil) Rack::Request.new('QUERY_STRING' => params) end + def rack_request_with_format_token_via_header(token_value = nil) + Rack::Request.new('HTTP_FORMAT_TOKEN' => token_value) + end + test 'valid_format_token? returns false when FORMAT_TOKEN env var is blank' do ClimateControl.modify(FORMAT_TOKEN: '') do refute Rack::Attack.valid_format_token?(rack_request_with_format_token('some-token')) @@ -44,19 +48,51 @@ def rack_request_with_format_token(token_value = nil) end end - test 'valid_format_token? returns false when token does not match' do + test 'valid_format_token? returns false when format_token header is missing' do + ClimateControl.modify(FORMAT_TOKEN: 'valid-token') do + refute Rack::Attack.valid_format_token?(rack_request_with_format_token_via_header) + refute Rack::Attack.valid_format_token?(rack_request_with_format_token_via_header('')) + end + end + + test 'valid_format_token? returns false when token param does not match' do ClimateControl.modify(FORMAT_TOKEN: 'valid-token') do refute Rack::Attack.valid_format_token?(rack_request_with_format_token('wrong-token')) refute Rack::Attack.valid_format_token?(rack_request_with_format_token('tampered-token')) end end - test 'valid_format_token? returns true when token matches FORMAT_TOKEN env var' do + test 'valid_format_token? returns false when token header does not match' do + ClimateControl.modify(FORMAT_TOKEN: 'valid-token') do + refute Rack::Attack.valid_format_token?(rack_request_with_format_token_via_header('wrong-token')) + refute Rack::Attack.valid_format_token?(rack_request_with_format_token_via_header('tampered-token')) + end + end + + test 'valid_format_token? returns true when querystring token matches FORMAT_TOKEN env var' do ClimateControl.modify(FORMAT_TOKEN: 'valid-token') do assert Rack::Attack.valid_format_token?(rack_request_with_format_token('valid-token')) end end + test 'valid_format_token? returns true when header token matches FORMAT_TOKEN env var' do + ClimateControl.modify(FORMAT_TOKEN: 'valid-token') do + assert Rack::Attack.valid_format_token?(rack_request_with_format_token_via_header('valid-token')) + end + end + + test 'valid_format_token? assigns precedence to request header over query parameters' do + correct = 'valid-token' + incorrect = 'wrong-token' + ClimateControl.modify(FORMAT_TOKEN: correct) do + env = { 'HTTP_FORMAT_TOKEN' => correct, 'QUERY_STRING' => "format_token=#{incorrect}" } + assert Rack::Attack.valid_format_token?(Rack::Request.new(env)) + + env = { 'HTTP_FORMAT_TOKEN' => incorrect, 'QUERY_STRING' => "format_token=#{correct}" } + refute Rack::Attack.valid_format_token?(Rack::Request.new(env)) + end + end + test 'valid_format_token? uses secure comparison to prevent timing attacks' do ClimateControl.modify(FORMAT_TOKEN: 'valid-token') do req = rack_request_with_format_token('valid-token')