SECURITY: Use different anon cache keys for XHR requests

XHR requests are handled differently by the application and the
responses do not have any preloaded data so the cache key needs to
differntiate between those requests.
This commit is contained in:
Bianca Nenciu 2024-09-27 12:55:03 +03:00 committed by Alan Guo Xiang Tan
parent 1da97de7f0
commit e081cc14fb
No known key found for this signature in database
GPG Key ID: 286D2AB58F8C86B6
2 changed files with 42 additions and 1 deletions

View File

@ -156,8 +156,14 @@ module Middleware
def cache_key def cache_key
return @cache_key if defined?(@cache_key) return @cache_key if defined?(@cache_key)
# Rack `xhr?` performs a case sensitive comparison, but Rails `xhr?`
# performs a case insensitive comparison. We use the latter everywhere
# else in the application, so we should use it here as well.
is_xhr = @env["HTTP_X_REQUESTED_WITH"]&.casecmp("XMLHttpRequest") == 0 ? "t" : "f"
@cache_key = @cache_key =
+"ANON_CACHE_#{@env["HTTP_ACCEPT"]}_#{@env[Rack::RACK_URL_SCHEME]}_#{@env["HTTP_HOST"]}#{@env["REQUEST_URI"]}" +"ANON_CACHE_#{is_xhr}_#{@env["HTTP_ACCEPT"]}_#{@env[Rack::RACK_URL_SCHEME]}_#{@env["HTTP_HOST"]}#{@env["REQUEST_URI"]}"
@cache_key << AnonymousCache.build_cache_key(self) @cache_key << AnonymousCache.build_cache_key(self)
@cache_key @cache_key
end end

View File

@ -1256,6 +1256,41 @@ RSpec.describe ApplicationController do
get "/", headers: { "HTTP_USER_AGENT" => "iam badcrawler" } get "/", headers: { "HTTP_USER_AGENT" => "iam badcrawler" }
expect(response.status).to eq(429) expect(response.status).to eq(429)
end end
context "with XHR requests" do
before { global_setting :anon_cache_store_threshold, 1 }
def preloaded_data
response_html = Nokogiri::HTML5.fragment(response.body)
JSON.parse(response_html.css("#data-preloaded")[0]["data-preloaded"])
end
it "does not return the same preloaded data for XHR and non-XHR requests" do
# Request is stored in cache
get "/", headers: { "X-Requested-With" => "XMLHTTPrequest" }
expect(response.status).to eq(200)
expect(response.headers["X-Discourse-Cached"]).to eq("store")
expect(preloaded_data).not_to have_key("site")
# Request is served from cache
get "/", headers: { "X-Requested-With" => "xmlhttprequest" }
expect(response.status).to eq(200)
expect(response.headers["X-Discourse-Cached"]).to eq("true")
expect(preloaded_data).not_to have_key("site")
# Request is not served from cache because of different headers, but is stored
get "/"
expect(response.status).to eq(200)
expect(response.headers["X-Discourse-Cached"]).to eq("store")
expect(preloaded_data).to have_key("site")
# Request is served from cache
get "/", headers: { "X-Requested-With" => "xmlhttprequest" }
expect(response.status).to eq(200)
expect(response.headers["X-Discourse-Cached"]).to eq("true")
expect(preloaded_data).not_to have_key("site")
end
end
end end
end end