From 4466fcf1bc04bf6cb2eb493bd7b2d8b86eae109d Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 21 Jan 2019 13:49:08 +0800 Subject: [PATCH] FIX: Don't update `User#last_seen_at` when PG is readonly take 2. --- lib/admin_constraint.rb | 2 +- lib/auth/default_current_user_provider.rb | 3 +-- lib/homepage_constraint.rb | 2 +- lib/staff_constraint.rb | 2 +- .../default_current_user_provider_spec.rb | 22 +++++++++---------- 5 files changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/admin_constraint.rb b/lib/admin_constraint.rb index 1d0291cd7f1..c14e0aa16b8 100644 --- a/lib/admin_constraint.rb +++ b/lib/admin_constraint.rb @@ -12,7 +12,7 @@ class AdminConstraint provider.current_user && provider.current_user.admin? && custom_admin_check(request) - rescue Discourse::InvalidAccess + rescue Discourse::InvalidAccess, Discourse::ReadOnly false end diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 7f1bf243ce8..d5b59f04ddc 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -25,6 +25,7 @@ class Auth::DefaultCurrentUserProvider # our current user, return nil if none is found def current_user return @env[CURRENT_USER_KEY] if @env.key?(CURRENT_USER_KEY) + raise Discourse::ReadOnly if Discourse.pg_readonly_mode? # bypass if we have the shared session header if shared_key = @env['HTTP_X_SHARED_SESSION_KEY'] @@ -241,8 +242,6 @@ class Auth::DefaultCurrentUserProvider end def should_update_last_seen? - return false if Discourse.pg_readonly_mode? - if @request.xhr? @env["HTTP_DISCOURSE_VISIBLE".freeze] == "true".freeze elsif !!(@env[API_KEY_ENV]) || !!(@env[USER_API_KEY_ENV]) diff --git a/lib/homepage_constraint.rb b/lib/homepage_constraint.rb index 586581425c3..7520bfe560b 100644 --- a/lib/homepage_constraint.rb +++ b/lib/homepage_constraint.rb @@ -9,7 +9,7 @@ class HomePageConstraint provider = Discourse.current_user_provider.new(request.env) homepage = provider&.current_user&.user_option&.homepage || SiteSetting.anonymous_homepage homepage == @filter - rescue Discourse::InvalidAccess + rescue Discourse::InvalidAccess, Discourse::ReadOnly false end end diff --git a/lib/staff_constraint.rb b/lib/staff_constraint.rb index 7572ada16c0..4d46de3399d 100644 --- a/lib/staff_constraint.rb +++ b/lib/staff_constraint.rb @@ -7,7 +7,7 @@ class StaffConstraint provider.current_user && provider.current_user.staff? && custom_staff_check(request) - rescue Discourse::InvalidAccess + rescue Discourse::InvalidAccess, Discourse::ReadOnly false end diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb index d4ba0909af8..280b73922a0 100644 --- a/spec/components/auth/default_current_user_provider_spec.rb +++ b/spec/components/auth/default_current_user_provider_spec.rb @@ -161,20 +161,13 @@ describe Auth::DefaultCurrentUserProvider do end describe "#current_user" do - let(:unhashed_token) do - provider = provider('/') - cookies = {} - provider.log_on_user(Fabricate(:user), {}, cookies) - cookies["_t"][:value] - end - after do $redis.flushall end it "should not update last seen for suspended users" do - user = Fabricate(:user) provider = provider('/') + user = Fabricate(:user) cookies = {} provider.log_on_user(user, {}, cookies) unhashed_token = cookies["_t"][:value] @@ -201,7 +194,11 @@ describe Auth::DefaultCurrentUserProvider do end describe "when readonly mode is enabled due to postgres" do + let(:test_provider) { provider("/") } + let(:user) { Fabricate(:user) } + before do + test_provider.log_on_user(user, {}, {}) Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY) end @@ -210,10 +207,11 @@ describe Auth::DefaultCurrentUserProvider do end it "should not update last seen at" do - provider2 = provider("/", "HTTP_COOKIE" => "_t=#{unhashed_token}") - u = provider2.current_user - u.reload - expect(u.last_seen_at).to eq(nil) + expect(test_provider.current_user).to eq(user) + + expect do + provider("/?api_key=hello").current_user + end.to raise_error(Discourse::ReadOnly) end end end