FIX: Incorrect `currentUser` could be cached for requests with API key (#17279)
This happened when a middleware accessed the `currentUser` before a controller had a chance to populate the `action_dispatch.request.path_parameters` env variable. In that case Discourse would always cache `nil` as `currentUser`.
This commit is contained in:
parent
af3262d70a
commit
caa0247f5c
|
@ -1,6 +1,8 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class RouteMatcher
|
||||
PATH_PARAMETERS ||= "_DISCOURSE_REQUEST_PATH_PARAMETERS"
|
||||
|
||||
attr_reader :actions, :params, :methods, :aliases, :formats, :allowed_param_values
|
||||
|
||||
def initialize(actions: nil, params: nil, methods: nil, formats: nil, aliases: nil, allowed_param_values: nil)
|
||||
|
@ -37,11 +39,11 @@ class RouteMatcher
|
|||
|
||||
def action_allowed?(request)
|
||||
return true if actions.nil? # actions are unrestricted
|
||||
path_params = request.path_parameters
|
||||
|
||||
# message_bus is not a rails route, special handling
|
||||
return true if actions.include?("message_bus") && request.fullpath =~ /^\/message-bus\/.*\/poll/
|
||||
|
||||
path_params = path_params_from_request(request)
|
||||
actions.include? "#{path_params[:controller]}##{path_params[:action]}"
|
||||
end
|
||||
|
||||
|
@ -82,4 +84,20 @@ class RouteMatcher
|
|||
request_format = request.formats&.first&.symbol
|
||||
formats.include?(request_format)
|
||||
end
|
||||
|
||||
def path_params_from_request(request)
|
||||
if request.env[ActionDispatch::Http::Parameters::PARAMETERS_KEY].nil?
|
||||
# We need to manually recognize the path when Rails hasn't done that yet. That can happen when
|
||||
# the matcher gets called in a Middleware before the controller did its work.
|
||||
# We store the result of `recognize_path` in a custom env key, so that we don't change
|
||||
# some Rails behavior by accident.
|
||||
request.env[PATH_PARAMETERS] ||= begin
|
||||
Rails.application.routes.recognize_path(request.path_info)
|
||||
rescue ActionController::RoutingError
|
||||
{}
|
||||
end
|
||||
end
|
||||
|
||||
request.path_parameters.presence || request.env[PATH_PARAMETERS] || {}
|
||||
end
|
||||
end
|
||||
|
|
|
@ -310,6 +310,26 @@ describe Auth::DefaultCurrentUserProvider do
|
|||
expect(u.last_seen_at).to eq(nil)
|
||||
end
|
||||
end
|
||||
|
||||
it "should not cache an invalid user when Rails hasn't set `path_parameters` on the request yet" do
|
||||
SiteSetting.login_required = true
|
||||
user = Fabricate(:user)
|
||||
api_key = ApiKey.create!(user_id: user.id, created_by_id: Discourse.system_user)
|
||||
url = "/latest.rss?api_key=#{api_key.key}&api_username=#{user.username_lower}"
|
||||
env = { ActionDispatch::Http::Parameters::PARAMETERS_KEY => nil }
|
||||
|
||||
provider = provider(url, env)
|
||||
env = provider.env
|
||||
|
||||
expect(env[ActionDispatch::Http::Parameters::PARAMETERS_KEY]).to be_nil
|
||||
expect(provider.env[Auth::DefaultCurrentUserProvider::CURRENT_USER_KEY]).to be_nil
|
||||
|
||||
u = provider.current_user
|
||||
|
||||
expect(u).to eq(user)
|
||||
expect(env[ActionDispatch::Http::Parameters::PARAMETERS_KEY]).to be_blank
|
||||
expect(provider.env[Auth::DefaultCurrentUserProvider::CURRENT_USER_KEY]).to eq(u)
|
||||
end
|
||||
end
|
||||
|
||||
it "should update last seen for non ajax" do
|
||||
|
|
Loading…
Reference in New Issue