From 3d1bbf7446ac83957c26f8e374fb269d6304f297 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Mon, 4 Jul 2022 16:33:59 +0200 Subject: [PATCH] FIX: Logout could fail due to cached user Logging out failed when the current user was cached by an instance of `Auth::DefaultCurrentUserProvider` and `#log_off_user` was called on a different instance of that class. Co-authored-by: Sam --- lib/auth/default_current_user_provider.rb | 6 ++++-- .../auth/default_current_user_provider_spec.rb | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 519caeae93..d3ff23325c 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -25,6 +25,7 @@ require_relative '../route_matcher' class Auth::DefaultCurrentUserProvider CURRENT_USER_KEY ||= "_DISCOURSE_CURRENT_USER" + USER_TOKEN_KEY ||= "_DISCOURSE_USER_TOKEN" API_KEY ||= "api_key" API_USERNAME ||= "api_username" HEADER_API_KEY ||= "HTTP_API_KEY" @@ -99,6 +100,7 @@ class Auth::DefaultCurrentUserProvider def initialize(env) @env = env @request = Rack::Request.new(env) + @user_token = env[USER_TOKEN_KEY] end # our current user, return nil if none is found @@ -136,7 +138,7 @@ class Auth::DefaultCurrentUserProvider limiter = RateLimiter.new(nil, "cookie_auth_#{request.ip}", COOKIE_ATTEMPTS_PER_MIN , 60) if limiter.can_perform? - @user_token = begin + @env[USER_TOKEN_KEY] = @user_token = begin UserAuthToken.lookup( auth_token, seen: true, @@ -255,7 +257,7 @@ class Auth::DefaultCurrentUserProvider end def log_on_user(user, session, cookie_jar, opts = {}) - @user_token = UserAuthToken.generate!( + @env[USER_TOKEN_KEY] = @user_token = UserAuthToken.generate!( user_id: user.id, user_agent: @env['HTTP_USER_AGENT'], path: @env['REQUEST_PATH'], diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb index f3687be647..d1a4323744 100644 --- a/spec/components/auth/default_current_user_provider_spec.rb +++ b/spec/components/auth/default_current_user_provider_spec.rb @@ -760,4 +760,21 @@ describe Auth::DefaultCurrentUserProvider do env = { "HTTP_COOKIE" => "_t=#{cookie}", "REMOTE_ADDR" => ip } expect(provider('/', env).current_user).to eq(nil) end + + describe "#log_off_user" do + it "should work when the current user was cached by a different provider instance" do + user_provider = provider('/') + user_provider.log_on_user(user, {}, user_provider.cookie_jar) + cookie = user_provider.cookie_jar["_t"] + env = create_request_env(path: "/").merge({ method: "GET", "HTTP_COOKIE" => "_t=#{cookie}" }) + + user_provider = TestProvider.new(env) + expect(user_provider.current_user).to eq(user) + expect(UserAuthToken.find_by(user_id: user.id)).to be_present + + user_provider = TestProvider.new(env) + user_provider.log_off_user({}, user_provider.cookie_jar) + expect(UserAuthToken.find_by(user_id: user.id)).to be_nil + end + end end