SECURITY: Only show tags to users with permission (#15148)

app/models/tag_user.rb

@@ -4,6 +4,20 @@ class TagUser < ActiveRecord::Base
   belongs_to :tag
   belongs_to :user
 
+  scope :notification_level_visible, -> (notification_levels = TagUser.notification_levels.values) {
+    select("tag_users.*")
+      .distinct
+      .joins("LEFT OUTER JOIN tag_group_memberships ON tag_users.tag_id = tag_group_memberships.tag_id")
+      .joins("LEFT OUTER JOIN tag_group_permissions ON tag_group_memberships.tag_group_id = tag_group_permissions.tag_group_id")
+      .joins("LEFT OUTER JOIN group_users on group_users.user_id = tag_users.user_id")
+      .where("(tag_group_permissions.group_id IS NULL
+               OR tag_group_permissions.group_id = group_users.group_id
+               OR group_users.group_id = :staff_group_id)
+              AND tag_users.notification_level IN (:notification_levels)",
+             staff_group_id: Group::AUTO_GROUPS[:staff],
+             notification_levels: notification_levels)
+  }
+
   def self.notification_levels
     NotificationLevels.all
   end
@@ -208,7 +222,10 @@ class TagUser < ActiveRecord::Base
         [name, self.notification_levels[:muted]]
       end
     else
-      notification_levels = TagUser.where(user: user).joins(:tag).pluck("tags.name", :notification_level)
+      notification_levels = TagUser
+        .notification_level_visible
+        .where(user: user)
+        .joins(:tag).pluck("tags.name", :notification_level)
     end
 
     Hash[*notification_levels.flatten]

app/services/post_alerter.rb

@@ -185,9 +185,10 @@ class PostAlerter
   end
 
   def tag_watchers(topic)
-    topic.tag_users
-      .where(notification_level: TagUser.notification_levels[:watching_first_post])
-      .pluck(:user_id)
+    topic
+      .tag_users
+      .notification_level_visible([TagUser.notification_levels[:watching_first_post]])
+      .distinct(:user_id).pluck(:user_id)
   end
 
   def category_watchers(topic)
@@ -783,9 +784,13 @@ class PostAlerter
           FROM tag_users
      LEFT JOIN topic_users tu ON tu.user_id = tag_users.user_id
                              AND tu.topic_id = :topic_id
-         WHERE tag_users.notification_level = :watching
+     LEFT JOIN tag_group_memberships tgm ON tag_users.tag_id = tgm.tag_id
+     LEFT JOIN tag_group_permissions tgp ON tgm.tag_group_id = tgp.tag_group_id
+     LEFT JOIN group_users gu ON gu.user_id = tag_users.user_id
+         WHERE (tgp.group_id IS NULL OR tgp.group_id = gu.group_id OR gu.group_id = :staff_group_id)
+               AND (tag_users.notification_level = :watching
                     AND tag_users.tag_id IN (:tag_ids)
-           AND (tu.user_id IS NULL OR tu.notification_level = :watching)
+                    AND (tu.user_id IS NULL OR tu.notification_level = :watching))
       SQL
     end
 
@@ -793,7 +798,8 @@ class PostAlerter
       watching: TopicUser.notification_levels[:watching],
       topic_id: post.topic_id,
       category_id: post.topic.category_id,
-      tag_ids: tag_ids
+      tag_ids: tag_ids,
+      staff_group_id: Group::AUTO_GROUPS[:staff]
     )
 
     if group_ids.present?

spec/fabricators/tag_group_permission_fabricator.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+Fabricator(:tag_group_permission) do
+  tag_group
+  group
+  permission_type TagGroupPermission.permission_types[:readonly]
+end

spec/models/tag_user_spec.rb

@@ -22,6 +22,58 @@ describe TagUser do
     TagUser.notification_levels[:watching]
   end
 
+  context "notification_level_visible" do
+    let!(:tag1) { Fabricate(:tag) }
+    let!(:tag2) { Fabricate(:tag) }
+    let!(:tag3) { Fabricate(:tag) }
+    let!(:tag4) { Fabricate(:tag) }
+    fab!(:user1) { Fabricate(:user) }
+    fab!(:user2) { Fabricate(:user) }
+    let!(:tag_user1) { TagUser.create(user: user1, tag: tag1, notification_level: TagUser.notification_levels[:watching]) }
+    let!(:tag_user2) { TagUser.create(user: user1, tag: tag2, notification_level: TagUser.notification_levels[:tracking]) }
+    let!(:tag_user3) { TagUser.create(user: user2, tag: tag3, notification_level: TagUser.notification_levels[:watching_first_post]) }
+    let!(:tag_user4) { TagUser.create(user: user2, tag: tag4, notification_level: TagUser.notification_levels[:muted]) }
+
+    it "scopes to notification levels visible due to absence of tag group" do
+      expect(TagUser.notification_level_visible.length).to be(4)
+    end
+
+    it "scopes to notification levels visible by tag group permission" do
+      group1 = Fabricate(:group)
+      tag_group1 = Fabricate(:tag_group, tags: [tag1])
+      Fabricate(:tag_group_permission, tag_group: tag_group1, group: group1)
+
+      group2 = Fabricate(:group)
+      tag_group2 = Fabricate(:tag_group, tags: [tag2])
+      Fabricate(:tag_group_permission, tag_group: tag_group2, group: group2)
+
+      Fabricate(:group_user, group: group1, user: user1)
+
+      expect(TagUser.notification_level_visible.pluck(:id)).to match_array([
+        tag_user1.id, tag_user3.id, tag_user4.id
+      ])
+    end
+
+    it "scopes to notification levels visible because user is staff" do
+      group2 = Fabricate(:group)
+      tag_group2 = Fabricate(:tag_group, tags: [tag2])
+      Fabricate(:tag_group_permission, tag_group: tag_group2, group: group2)
+
+      staff_group = Group.find(Group::AUTO_GROUPS[:staff])
+      Fabricate(:group_user, group: staff_group, user: user1)
+
+      expect(TagUser.notification_level_visible.length).to be(4)
+    end
+
+    it "scopes to notification levels visible by specified notification level" do
+      expect(TagUser.notification_level_visible([TagUser.notification_levels[:watching]]).length).to be(1)
+      expect(TagUser.notification_level_visible(
+        [TagUser.notification_levels[:watching],
+         TagUser.notification_levels[:tracking]]
+      ).length).to be(2)
+    end
+  end
+
   context "change" do
     it "watches or tracks on change" do
       user = Fabricate(:user)
@@ -264,6 +316,7 @@ describe TagUser do
         TagUser.create(user: user, tag: tag3, notification_level: TagUser.notification_levels[:watching_first_post])
         TagUser.create(user: user, tag: tag4, notification_level: TagUser.notification_levels[:muted])
       end
+
       it "gets the tag_user notification levels for all tags the user is tracking and does not
       include tags the user is not tracking at all" do
         tag5 = Fabricate(:tag)
@@ -274,6 +327,14 @@ describe TagUser do
         expect(levels[tag4.name]).to eq(TagUser.notification_levels[:muted])
         expect(levels.key?(tag5.name)).to eq(false)
       end
+
+      it "does not show a tag is tracked if the user does not belong to the tag group with permissions" do
+        group = Fabricate(:group)
+        tag_group = Fabricate(:tag_group, tags: [tag2])
+        Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
+
+        expect(TagUser.notification_levels_for(user).keys).to match_array([tag1.name, tag3.name, tag4.name])
+      end
     end
   end
 end

spec/services/post_alerter_spec.rb

@@ -1285,6 +1285,33 @@ describe PostAlerter do
         notification_data = JSON.parse(notification.data)
         expect(notification_data["display_username"]).to eq(I18n.t("embed.replies", count: 2))
       end
+
+      it "does not add notification if user does not belong to tag group with permissions" do
+        tag = Fabricate(:tag)
+        topic = Fabricate(:topic, tags: [tag])
+        post = Fabricate(:post, topic: topic)
+        tag_group = Fabricate(:tag_group, tags: [tag])
+        group = Fabricate(:group)
+        Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
+
+        TagUser.change(user.id, tag.id, TagUser.notification_levels[:watching])
+
+        expect { PostAlerter.post_created(post) }.not_to change { Notification.count }
+      end
+
+      it "adds notification if user belongs to tag group with permissions" do
+        tag = Fabricate(:tag)
+        topic = Fabricate(:topic, tags: [tag])
+        post = Fabricate(:post, topic: topic)
+        tag_group = Fabricate(:tag_group, tags: [tag])
+        group = Fabricate(:group)
+        Fabricate(:group_user, group: group, user: user)
+        Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
+
+        TagUser.change(user.id, tag.id, TagUser.notification_levels[:watching])
+
+        expect { PostAlerter.post_created(post) }.to change { Notification.count }.by(1)
+      end
     end
 
     context "on change" do
@@ -1351,6 +1378,64 @@ describe PostAlerter do
         expect(Notification.where(user_id: admin.id).count).to eq(1)
       end
     end
+
+    context "with tag groups" do
+      fab!(:tag)  { Fabricate(:tag) }
+      fab!(:group) { Fabricate(:group) }
+      fab!(:user) { Fabricate(:user) }
+      fab!(:topic) { Fabricate(:topic, tags: [tag]) }
+      fab!(:post) { Fabricate(:post, topic: topic) }
+
+      shared_examples "tag user with notification level" do |notification_level, notification_type|
+        it "notifies a user who is watching a tag that does not belong to a tag group" do
+          TagUser.change(user.id, tag.id, TagUser.notification_levels[notification_level])
+          PostAlerter.post_created(post)
+          expect(user.notifications.where(notification_type: Notification.types[notification_type]).count).to eq(1)
+        end
+
+        it "does not notify a user watching a tag with tag group permissions that he does not belong to" do
+          tag_group = Fabricate(:tag_group, tags: [tag])
+          Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
+
+          TagUser.change(user.id, tag.id, TagUser.notification_levels[notification_level])
+
+          PostAlerter.post_created(post)
+
+          expect(user.notifications.where(notification_type: Notification.types[notification_type]).count).to eq(0)
+        end
+
+        it "notifies a user watching a tag with tag group permissions that he belongs to" do
+          Fabricate(:group_user, group: group, user: user)
+
+          TagUser.change(user.id, tag.id, TagUser.notification_levels[notification_level])
+
+          PostAlerter.post_created(post)
+
+          expect(user.notifications.where(notification_type: Notification.types[notification_type]).count).to eq(1)
+        end
+
+        it "notifies a staff watching a tag with tag group permissions that he does not belong to" do
+          tag_group = Fabricate(:tag_group, tags: [tag])
+          Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
+          staff_group = Group.find(Group::AUTO_GROUPS[:staff])
+          Fabricate(:group_user, group: staff_group, user: user)
+
+          TagUser.change(user.id, tag.id, TagUser.notification_levels[notification_level])
+
+          PostAlerter.post_created(post)
+
+          expect(user.notifications.where(notification_type: Notification.types[notification_type]).count).to eq(1)
+        end
+      end
+
+      context "with :watching notification level" do
+        include_examples "tag user with notification level", :watching, :posted
+      end
+
+      context "with :watching_first_post notification level" do
+        include_examples "tag user with notification level", :watching_first_post, :watching_first_post
+      end
+    end
   end
 
   describe '#extract_linked_users' do