Merge branch '326061-hide-confidential-comments-from-atom-feed' into 'master'

Hide confidential comments from atom feed

See merge request gitlab-org/gitlab!60676

app/controllers/projects_controller.rb

@@ -162,6 +162,7 @@ class ProjectsController < Projects::ApplicationController
 
       format.atom do
         load_events
+        @events = @events.select { |event| event.visible_to_user?(current_user) }
         render layout: 'xml.atom'
       end
     end

app/models/event.rb

@@ -390,16 +390,15 @@ class Event < ApplicationRecord
       read_snippet: %i[personal_snippet_note? project_snippet_note?],
       read_milestone: %i[milestone?],
       read_wiki: %i[wiki_page?],
-      read_design: %i[design_note? design?]
+      read_design: %i[design_note? design?],
+      read_note: %i[note?]
     }
   end
 
   private
 
   def permission_object
-    if note?
-      note_target
-    elsif target_id.present?
+    if target_id.present?
       target
     else
       project

app/models/event_collection.rb

@@ -59,6 +59,7 @@ class EventCollection
     parents_for_lateral = parents.select(:id).to_sql
 
     lateral = filtered_events
+      # Applying the limit here (before we filter (permissions) means we may get less than limit)
       .limit(limit_for_join_lateral)
       .where("events.#{parent_column} = parents_for_lateral.id") # rubocop:disable GitlabSecurity/SqlInjection
       .to_sql

ee/spec/models/ee/event_spec.rb

@@ -10,7 +10,6 @@ RSpec.describe Event do
     let_it_be(:reporter) { create(:user) }
     let_it_be(:author) { create(:author) }
     let_it_be(:admin) { create(:admin) }
-    let_it_be(:project) { create(:project) }
 
     let(:users) { [non_member, member, reporter, guest, author, admin] }
 
@@ -21,10 +20,6 @@ RSpec.describe Event do
     before do
       stub_licensed_features(epics: true)
 
-      project.add_developer(member)
-      project.add_guest(guest)
-      project.add_reporter(reporter)
-
       if defined?(group)
         group.add_developer(member)
         group.add_guest(guest)

spec/controllers/projects_controller_spec.rb

@@ -119,11 +119,6 @@ RSpec.describe ProjectsController do
         get :activity, params: { namespace_id: project.namespace, id: project, format: :json }
 
         expect(json_response['html']).to eq("\n")
-      end
-
-      it 'filters out invisible event when calculating the count' do
-        get :activity, params: { namespace_id: project.namespace, id: project, format: :json }
-
         expect(json_response['count']).to eq(0)
       end
     end
@@ -1484,6 +1479,30 @@ RSpec.describe ProjectsController do
     end
   end
 
+  context 'GET show.atom' do
+    let_it_be(:public_project) { create(:project, :public) }
+    let_it_be(:event) { create(:event, :commented, project: public_project, target: create(:note, project: public_project)) }
+    let_it_be(:invisible_event) { create(:event, :commented, project: public_project, target: create(:note, :confidential, project: public_project)) }
+
+    it 'filters by calling event.visible_to_user?' do
+      expect(EventCollection).to receive_message_chain(:new, :to_a).and_return([event, invisible_event])
+      expect(event).to receive(:visible_to_user?).and_return(true)
+      expect(invisible_event).to receive(:visible_to_user?).and_return(false)
+
+      get :show, format: :atom, params: { id: public_project, namespace_id: public_project.namespace }
+
+      expect(response).to render_template('xml.atom')
+      expect(assigns(:events)).to eq([event])
+    end
+
+    it 'filters by calling event.visible_to_user?' do
+      get :show, format: :atom, params: { id: public_project, namespace_id: public_project.namespace }
+
+      expect(response).to render_template('xml.atom')
+      expect(assigns(:events)).to eq([event])
+    end
+  end
+
   describe 'GET resolve' do
     shared_examples 'resolvable endpoint' do
       it 'redirects to the project page' do

spec/models/event_spec.rb

@@ -268,6 +268,7 @@ RSpec.describe Event do
     let(:design) { create(:design, issue: issue, project: project) }
     let(:note_on_commit) { create(:note_on_commit, project: project) }
     let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) }
+    let(:confidential_note) { create(:note, noteable: issue, project: project, confidential: true) }
     let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) }
     let(:note_on_project_snippet) { create(:note_on_project_snippet, author: author, noteable: project_snippet, project: project) }
     let(:note_on_personal_snippet) { create(:note_on_personal_snippet, author: author, noteable: personal_snippet, project: nil) }
@@ -399,6 +400,16 @@ RSpec.describe Event do
         include_examples 'visible to assignee and author', true
       end
 
+      context 'confidential note' do
+        let(:target) { confidential_note }
+
+        include_examples 'visibility examples' do
+          let(:visibility) { visible_to_none_except(:member) }
+        end
+
+        include_examples 'visible to author', true
+      end
+
       context 'private project' do
         let(:project) { private_project }
         let(:target) { note_on_issue }