From b9a669ba320945a538a36a33daa2a5a7cdf4b350 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Sun, 25 Feb 2018 22:01:51 +0530 Subject: [PATCH] FIX: do not log personal message view if user can't see the message --- lib/topic_view.rb | 10 +++++++--- spec/components/topic_view_spec.rb | 5 +++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 041f52fe3ef..9153a9880c0 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -489,11 +489,15 @@ class TopicView raise Discourse::NotFound if @topic.blank? # Special case: If the topic is private and the user isn't logged in, ask them # to log in! - if @topic.present? && @topic.private_message? - raise Discourse::NotLoggedIn.new if @user.blank? - StaffActionLogger.new(@user).log_check_personal_message(@topic) if SiteSetting.log_personal_messages_views && @topic.all_allowed_users.where(id: @user.id).blank? + if @topic.present? && @topic.private_message? && @user.blank? + raise Discourse::NotLoggedIn.new end + # can user see this topic? raise Discourse::InvalidAccess.new("can't see #{@topic}", @topic) unless @guardian.can_see?(@topic) + # log personal message views + if SiteSetting.log_personal_messages_views && @topic.present? && @topic.private_message? && @topic.all_allowed_users.where(id: @user.id).blank? + StaffActionLogger.new(@user).log_check_personal_message(@topic) + end end def get_minmax_ids(post_number) diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb index 29ee51328e3..06615bda10f 100644 --- a/spec/components/topic_view_spec.rb +++ b/spec/components/topic_view_spec.rb @@ -138,6 +138,11 @@ describe TopicView do TopicView.new(private_message.id, evil_trout) expect(UserHistory.where(action: UserHistory.actions[:check_personal_message]).count).to eq(0) end + + it "does not log personal message view if user can't see the message" do + expect { TopicView.new(private_message.id, Fabricate(:user)) }.to raise_error(Discourse::InvalidAccess) + expect(UserHistory.where(action: UserHistory.actions[:check_personal_message]).count).to eq(0) + end end it "provides an absolute url" do