diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 0f8e7444c4..d85a74dbb8 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -1231,7 +1231,7 @@ class TopicsController < ApplicationController respond_to do |format| format.html do - @tags = SiteSetting.tagging_enabled ? @topic_view.topic.tags : [] + @tags = SiteSetting.tagging_enabled ? @topic_view.topic.tags.visible(guardian) : [] @breadcrumbs = helpers.categories_breadcrumb(@topic_view.topic) || [] @description_meta = @topic_view.topic.excerpt.present? ? @topic_view.topic.excerpt : @topic_view.summary diff --git a/app/models/tag.rb b/app/models/tag.rb index 71435ef046..f3bdedc41a 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -38,6 +38,7 @@ class Tag < ActiveRecord::Base ->(guardian) { where("tags.#{Tag.topic_count_column(guardian)} > 0") } scope :base_tags, -> { where(target_tag_id: nil) } + scope :visible, ->(guardian = nil) { merge(DiscourseTagging.visible_tags(guardian)) } has_many :tag_users, dependent: :destroy # notification settings diff --git a/app/views/topics/show.html.erb b/app/views/topics/show.html.erb index f9bb80008f..60670a3cd5 100644 --- a/app/views/topics/show.html.erb +++ b/app/views/topics/show.html.erb @@ -129,7 +129,7 @@ <% content_for :head do %> <%= auto_discovery_link_tag(@topic_view, {action: :feed, slug: @topic_view.topic.slug, topic_id: @topic_view.topic.id}, rel: 'alternate nofollow', title: t('rss_posts_in_topic', topic: @topic_view.title), type: 'application/rss+xml') %> - <%= raw crawlable_meta_data(title: @topic_view.title, description: @topic_view.summary(strip_images: true), image: @topic_view.image_url, read_time: @topic_view.read_time, like_count: @topic_view.like_count, ignore_canonical: true, published_time: @topic_view.published_time, breadcrumbs: @breadcrumbs, tags: @topic_view.tags) %> + <%= raw crawlable_meta_data(title: @topic_view.title, description: @topic_view.summary(strip_images: true), image: @topic_view.image_url, read_time: @topic_view.read_time, like_count: @topic_view.like_count, ignore_canonical: true, published_time: @topic_view.published_time, breadcrumbs: @breadcrumbs, tags: @tags.map(&:name)) %> <% if @topic_view.prev_page || @topic_view.next_page %> <% if @topic_view.prev_page %> diff --git a/lib/topic_view.rb b/lib/topic_view.rb index b73e93a258..854306235d 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -244,9 +244,9 @@ class TopicView if @topic.category_id != SiteSetting.uncategorized_category_id && @topic.category_id && @topic.category title += " - #{@topic.category.name}" - elsif SiteSetting.tagging_enabled && @topic.tags.exists? + elsif SiteSetting.tagging_enabled && visible_tags.exists? title += - " - #{@topic.tags.order("tags.#{Tag.topic_count_column(@guardian)} DESC").first.name}" + " - #{visible_tags.order("tags.#{Tag.topic_count_column(@guardian)} DESC").first.name}" end end title @@ -713,10 +713,6 @@ class TopicView end end - def tags - @topic.tags.map(&:name) - end - protected def read_posts_set @@ -820,7 +816,7 @@ class TopicView def find_topic(topic_or_topic_id) return topic_or_topic_id if topic_or_topic_id.is_a?(Topic) # with_deleted covered in #check_and_raise_exceptions - Topic.with_deleted.includes(:category, :tags).find_by(id: topic_or_topic_id) + Topic.with_deleted.includes(:category).find_by(id: topic_or_topic_id) end def unfiltered_posts @@ -990,4 +986,8 @@ class TopicView end end end + + def visible_tags + @visible_tags ||= topic.tags.visible(guardian) + end end diff --git a/spec/lib/topic_view_spec.rb b/spec/lib/topic_view_spec.rb index 5032846488..79b6bc9563 100644 --- a/spec/lib/topic_view_spec.rb +++ b/spec/lib/topic_view_spec.rb @@ -825,6 +825,25 @@ RSpec.describe TopicView do it { is_expected.not_to include(tag1.name) } it { is_expected.not_to include(tag2.name) } end + + context "with restricted tags" do + let(:tag_group) { Fabricate.build(:tag_group) } + let(:tag_group_permission) do + Fabricate.build(:tag_group_permission, tag_group: tag_group) + end + + before do + SiteSetting.tagging_enabled = true + # avoid triggering a `before_create` callback in `TagGroup` which + # messes with permissions + tag_group.tag_group_permissions << tag_group_permission + tag_group.save! + tag_group_permission.tag_group.tags << tag2 + end + + it { is_expected.not_to include(tag2.name) } + it { is_expected.to include(tag1.name) } + end end end end @@ -1072,17 +1091,4 @@ RSpec.describe TopicView do end end end - - describe "#tags" do - subject(:topic_view_tags) { topic_view.tags } - - let(:topic_view) { described_class.new(topic, user) } - let(:topic) { Fabricate.build(:topic, tags: tags) } - let(:tags) { Fabricate.build_times(2, :tag) } - let(:user) { Fabricate(:user) } - - it "returns the tags names" do - expect(topic_view_tags).to match tags.map(&:name) - end - end end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 5f9c62c03b..6ddfff1af7 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2507,6 +2507,28 @@ RSpec.describe TopicsController do expect(body).to have_tag(:script, src: "/assets/discourse.js") expect(body).to have_tag(:meta, with: { name: "fragment" }) end + + context "with restricted tags" do + let(:tag_group) { Fabricate.build(:tag_group) } + let(:tag_group_permission) { Fabricate.build(:tag_group_permission, tag_group: tag_group) } + let(:restricted_tag) { Fabricate(:tag) } + let(:public_tag) { Fabricate(:tag) } + + before do + # avoid triggering a `before_create` callback in `TagGroup` which + # messes with permissions + tag_group.tag_group_permissions << tag_group_permission + tag_group.save! + tag_group_permission.tag_group.tags << restricted_tag + topic.tags << [public_tag, restricted_tag] + end + + it "doesn’t expose restricted tags" do + get "/t/#{topic.slug}/#{topic.id}/print", headers: { HTTP_USER_AGENT: "Rails Testing" } + expect(response.body).to match(public_tag.name) + expect(response.body).not_to match(restricted_tag.name) + end + end end it "records redirects" do diff --git a/spec/views/topics/show.html.erb_spec.rb b/spec/views/topics/show.html.erb_spec.rb index dda660961d..e229354441 100644 --- a/spec/views/topics/show.html.erb_spec.rb +++ b/spec/views/topics/show.html.erb_spec.rb @@ -12,6 +12,8 @@ RSpec.describe "topics/show.html.erb" do view.stubs(:crawler_layout?).returns(false) view.stubs(:url_for).returns("https://www.example.com/test.rss") view.instance_variable_set("@topic_view", topic_view) + assign(:tags, []) + render template: "topics/show", formats: [:html] expect(view.content_for(:head)).to match(