Skip to content
Permalink
Browse files Browse the repository at this point in the history
SECURITY: Update to exclude tag topic filter (#20006)
Ignores tags specified in exclude_tag topics param that a user does not
have access to.

Co-authored-by: Blake Erickson <o.blakeerickson@gmail.com>
  • Loading branch information
nbianca and oblakeerickson committed Jan 25, 2023
1 parent 105fee9 commit f55e0fe
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
19 changes: 11 additions & 8 deletions lib/topic_query.rb
Expand Up @@ -735,14 +735,17 @@ def default_results(options = {})
result = result.where.not(id: TopicTag.distinct.pluck(:topic_id))
end

result = result.where(<<~SQL, name: @options[:exclude_tag]) if @options[:exclude_tag].present?
topics.id NOT IN (
SELECT topic_tags.topic_id
FROM topic_tags
INNER JOIN tags ON tags.id = topic_tags.tag_id
WHERE tags.name = :name
)
SQL
if @options[:exclude_tag].present? &&
!DiscourseTagging.hidden_tag_names(@guardian).include?(@options[:exclude_tag])
result = result.where(<<~SQL, name: @options[:exclude_tag])
topics.id NOT IN (
SELECT topic_tags.topic_id
FROM topic_tags
INNER JOIN tags ON tags.id = topic_tags.tag_id
WHERE tags.name = :name
)
SQL
end
end

result = apply_ordering(result, options)
Expand Down
8 changes: 8 additions & 0 deletions spec/lib/topic_query_spec.rb
Expand Up @@ -409,13 +409,21 @@
fab!(:tagged_topic3) { Fabricate(:topic, tags: [tag, other_tag]) }
fab!(:tagged_topic4) { Fabricate(:topic, tags: [uppercase_tag]) }
fab!(:no_tags_topic) { Fabricate(:topic) }
fab!(:tag_group) do
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [other_tag.name])
end
let(:synonym) { Fabricate(:tag, target_tag: tag, name: "synonym") }

it "excludes a tag if desired" do
topics = TopicQuery.new(moderator, exclude_tag: tag.name).list_latest.topics
expect(topics.any? { |t| t.tags.include?(tag) }).to eq(false)
end

it "does not exclude a tagged topic without permission" do
topics = TopicQuery.new(user, exclude_tag: other_tag.name).list_latest.topics
expect(topics.map(&:id)).to include(tagged_topic2.id)
end

it "returns topics with the tag when filtered to it" do
expect(TopicQuery.new(moderator, tags: tag.name).list_latest.topics).to contain_exactly(
tagged_topic1,
Expand Down

0 comments on commit f55e0fe

Please sign in to comment.