From 84c83e8d4a1907f8a2972f0ab44b6402aa910c3b Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Tue, 29 Nov 2022 10:35:41 -0600 Subject: [PATCH] SECURITY: Filter tags in user notifications for visibility (#19239) --- app/mailers/user_notifications.rb | 12 +++++++----- spec/mailers/user_notifications_spec.rb | 11 ++++++++++- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 0c36b6aeea8..78bdbd722b2 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -533,11 +533,13 @@ class UserNotifications < ActionMailer::Base # tag names if opts[:show_tags_in_subject] && post.topic_id - - tags = Tag.joins(:topic_tags) - .where("topic_tags.topic_id = ?", post.topic_id) - .limit(3) - .pluck(:name) + tags = + DiscourseTagging + .visible_tags(Guardian.new(user)) + .joins(:topic_tags) + .where("topic_tags.topic_id = ?", post.topic_id) + .limit(3) + .pluck(:name) show_tags_in_subject = tags.any? ? tags.join(" ") : nil end diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 1a7c28097fd..63e4350633a 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -298,7 +298,13 @@ RSpec.describe UserNotifications do let(:category) { Fabricate(:category, name: 'India') } let(:tag1) { Fabricate(:tag, name: 'Taggo') } let(:tag2) { Fabricate(:tag, name: 'Taggie') } - let(:topic) { Fabricate(:topic, category: category, tags: [tag1, tag2], title: "Super cool topic") } + + let(:hidden_tag) { Fabricate(:tag, name: "hidden") } + let!(:hidden_tag_group) do + Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [hidden_tag.name]) + end + + let(:topic) { Fabricate(:topic, category: category, tags: [tag1, tag2, hidden_tag], title: "Super cool topic") } let(:post) { Fabricate(:post, topic: topic, raw: 'This is My super duper cool topic') } let(:response) { Fabricate(:basic_reply, topic: post.topic, user: response_by_user) } let(:user) { Fabricate(:user) } @@ -377,6 +383,9 @@ RSpec.describe UserNotifications do expect(mail_html.scan(/>Bob Marley/).count).to eq(1) expect(mail_html.scan(/>bobmarley/).count).to eq(0) + expect(mail.subject.scan(/#{tag1.name}/).count).to eq(1) + expect(mail.subject.scan(/#{hidden_tag.name}/).count).to eq(0) + SiteSetting.prioritize_username_in_ux = true mail = UserNotifications.user_replied(