SECURITY: Hide private categories in user activity export (#16276)

In some of the user's own activity export data,
we sometimes showed a secure category's name or
exposed the existence of a secure category.

app/jobs/regular/export_user_archive.rb

@@ -261,15 +261,16 @@ module Jobs
 
       CategoryUser
         .where(user_id: @current_user.id)
-        .select(:category_id, :notification_level, :last_seen_at)
+        .includes(:category)
+        .merge(Category.secured(guardian))
         .each do |cu|
-        yield [
+          yield [
-          cu.category_id,
+            cu.category_id,
-          piped_category_name(cu.category_id),
+            piped_category_name(cu.category_id, cu.category),
-          NotificationLevels.all[cu.notification_level],
+            NotificationLevels.all[cu.notification_level],
-          cu.last_seen_at
+            cu.last_seen_at
-        ]
+          ]
-      end
+        end
     end
 
     def flags_export
@@ -408,10 +409,9 @@ module Jobs
       @guardian ||= Guardian.new(@current_user)
     end
 
-    def piped_category_name(category_id)
+    def piped_category_name(category_id, category)
-      return "-" unless category_id
+      return "#{category_id}" if category_id && !category
-      category = Category.find_by(id: category_id)
+      return "-" if !guardian.can_see_category?(category)
-      return "#{category_id}" unless category
       categories = [category.name]
       while category.parent_category_id && category = category.parent_category
         categories << category.name
@@ -433,10 +433,10 @@ module Jobs
       user_archive_array = []
       topic_data = user_archive.topic
       user_archive = user_archive.as_json
-      topic_data = Topic.with_deleted.find_by(id: user_archive['topic_id']) if topic_data.nil?
+      topic_data = Topic.with_deleted.includes(:category).find_by(id: user_archive['topic_id']) if topic_data.nil?
       return user_archive_array if topic_data.nil?
 
-      categories = piped_category_name(topic_data.category_id)
+      categories = piped_category_name(topic_data.category_id, topic_data.category)
       is_pm = topic_data.archetype == "private_message" ? I18n.t("csv_export.boolean_yes") : I18n.t("csv_export.boolean_no")
       url = "#{Discourse.base_url}/t/#{topic_data.slug}/#{topic_data.id}/#{user_archive['post_number']}"
 

spec/jobs/export_user_archive_spec.rb

@@ -16,7 +16,7 @@ describe Jobs::ExportUserArchive do
   let(:component) { raise 'component not set' }
 
   fab!(:admin) { Fabricate(:admin) }
-  fab!(:category) { Fabricate(:category_with_definition) }
+  fab!(:category) { Fabricate(:category_with_definition, name: "User Archive Category") }
   fab!(:subcategory) { Fabricate(:category_with_definition, parent_category_id: category.id) }
   fab!(:topic) { Fabricate(:topic, category: category) }
   let(:post) { Fabricate(:post, user: user, topic: topic) }
@@ -169,6 +169,17 @@ describe Jobs::ExportUserArchive do
       _, csv_out = make_component_csv
       expect(csv_out).to match cat2_id.to_s
     end
+
+    it "can export a post from a secure category, obscuring the category name" do
+      cat2 = Fabricate(:private_category, group: Fabricate(:group), name: "Secret Cat")
+      topic2 = Fabricate(:topic, category: cat2, user: user, title: "This is a test secure topic")
+      _post2 = Fabricate(:post, topic: topic2, user: user)
+      data, csv_out = make_component_csv
+      expect(csv_out).not_to match "Secret Cat"
+      expect(data.length).to eq(1)
+      expect(data[0][:topic_title]).to eq("This is a test secure topic")
+      expect(data[0][:categories]).to eq("-")
+    end
   end
 
   context 'preferences' do
@@ -315,9 +326,11 @@ describe Jobs::ExportUserArchive do
   context 'category_preferences' do
     let(:component) { 'category_preferences' }
 
-    let(:subsubcategory) { Fabricate(:category_with_definition, parent_category_id: subcategory.id) }
+    let(:subsubcategory) { Fabricate(:category_with_definition, parent_category_id: subcategory.id, name: "User Archive Subcategory") }
-    let(:announcements) { Fabricate(:category_with_definition) }
+    let(:announcements) { Fabricate(:category_with_definition, name: "Announcements") }
-    let(:deleted_category) { Fabricate(:category) }
+    let(:deleted_category) { Fabricate(:category, name: "Deleted Category") }
+    let(:secure_category_group) { Fabricate(:group) }
+    let(:secure_category) { Fabricate(:private_category, group: secure_category_group, name: "Super Secret Category") }
 
     let(:reset_at) { DateTime.parse('2017-03-01 12:00') }
 
@@ -342,11 +355,12 @@ describe Jobs::ExportUserArchive do
       deleted_category.destroy!
     end
 
-    it 'correctly exports the CategoryUser table' do
+    it 'correctly exports the CategoryUser table, excluding deleted categories' do
       data, _csv_out = make_component_csv
 
-      expect(data.find { |r| r['category_id'] == category.id }).to be_nil
+      expect(data.find { |r| r['category_id'] == category.id.to_s }).to be_nil
-      expect(data.length).to eq(4)
+      expect(data.find { |r| r['category_id'] == deleted_category.id.to_s }).to be_nil
+      expect(data.length).to eq(3)
       data.sort! { |a, b| a['category_id'].to_i <=> b['category_id'].to_i }
 
       expect(data[0][:category_id]).to eq(subcategory.id.to_s)
@@ -362,8 +376,23 @@ describe Jobs::ExportUserArchive do
       expect(data[2][:category_names]).to eq(announcements.name)
       expect(data[2][:notification_level]).to eq('watching_first_post')
       expect(data[2][:dismiss_new_timestamp]).to eq('')
+    end
 
-      expect(data[3][:category_names]).to eq(data[3][:category_id])
+    it "does not include any secure categories the user does not have access to, even if the user has a CategoryUser record" do
+      CategoryUser.set_notification_level_for_category(user, NotificationLevels.all[:muted], secure_category.id)
+      data, _csv_out = make_component_csv
+
+      expect(data.any? { |r| r['category_id'] == secure_category.id.to_s }).to eq(false)
+      expect(data.length).to eq(3)
+    end
+
+    it "does include secure categories that the user has access to" do
+      CategoryUser.set_notification_level_for_category(user, NotificationLevels.all[:muted], secure_category.id)
+      GroupUser.create!(user: user, group: secure_category_group)
+      data, _csv_out = make_component_csv
+
+      expect(data.any? { |r| r['category_id'] == secure_category.id.to_s }).to eq(true)
+      expect(data.length).to eq(4)
     end
   end