FIX: Ensure anon-cached values are never returned for API requests (#20021)

Under some situations, we would inadvertently return a public (unauthenticated) result to an authenticated API request. This commit adds the `Api-Key` header to our anonymous cache bypass logic.
This commit is contained in:
David Taylor 2023-01-26 13:26:29 +00:00 committed by GitHub
parent e717529d80
commit 798b4bb604
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 19 additions and 1 deletions

View File

@ -66,6 +66,7 @@ module Middleware
!@request.path.ends_with?("srv/status") &&
@request[Auth::DefaultCurrentUserProvider::API_KEY].nil? &&
@env[Auth::DefaultCurrentUserProvider::USER_API_KEY].nil? &&
@env[Auth::DefaultCurrentUserProvider::HEADER_API_KEY].nil? &&
CrawlerDetection.is_blocked_crawler?(@env[USER_AGENT])
end
@ -184,11 +185,13 @@ module Middleware
request = Rack::Request.new(@env)
request.cookies["_bypass_cache"].nil? && (request.path != "/srv/status") &&
request[Auth::DefaultCurrentUserProvider::API_KEY].nil? &&
@env[Auth::DefaultCurrentUserProvider::HEADER_API_KEY].nil? &&
@env[Auth::DefaultCurrentUserProvider::USER_API_KEY].nil?
end
def force_anonymous!
@env[Auth::DefaultCurrentUserProvider::USER_API_KEY] = nil
@env[Auth::DefaultCurrentUserProvider::HEADER_API_KEY] = nil
@env["HTTP_COOKIE"] = nil
@env["HTTP_DISCOURSE_LOGGED_IN"] = nil
@env["rack.request.cookie.hash"] = {}

View File

@ -4,7 +4,7 @@ RSpec.describe Middleware::AnonymousCache do
let(:middleware) { Middleware::AnonymousCache.new(lambda { |_| [200, {}, []] }) }
def env(opts = {})
create_request_env(path: "http://test.com/path?bla=1").merge(opts)
create_request_env(path: opts.delete(:path) || "http://test.com/path?bla=1").merge(opts)
end
describe Middleware::AnonymousCache::Helper do
@ -38,6 +38,18 @@ RSpec.describe Middleware::AnonymousCache do
it "is false for srv/status routes" do
expect(new_helper("PATH_INFO" => "/srv/status").cacheable?).to eq(false)
end
it "is false for API requests using header" do
expect(new_helper("HTTP_API_KEY" => "abcde").cacheable?).to eq(false)
end
it "is false for API requests using parameter" do
expect(new_helper(path: "/path?api_key=abc").cacheable?).to eq(false)
end
it "is false for User API requests using header" do
expect(new_helper("HTTP_USER_API_KEY" => "abcde").cacheable?).to eq(false)
end
end
describe "per theme cache" do
@ -322,6 +334,9 @@ RSpec.describe Middleware::AnonymousCache do
"QUERY_STRING" => "api_key=#{api_key.key}&api_username=system",
}
expect(@status).to eq(200)
get "/latest", headers: { "HTTP_API_KEY" => api_key.key, "HTTP_API_USERNAME" => "system" }
expect(@status).to eq(200)
end
it "applies blocked_crawler_user_agents correctly" do