Merge branch 'security-33689-post-filter-search-results-ce-12-2' into '12-2-stable'

Filter out search results based on permissions to avoid bugs leaking data

See merge request gitlab/gitlabhq!3494

9 files changed, 44 insertions, 9 deletions

diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 0d066d0d99f..b8525f7b135 100644
--- a/app/models/discussion.rb
+++ b/app/models/discussion.rb
@@ -16,6 +16,7 @@ class Discussion
             :commit_id,
             :for_commit?,
             :for_merge_request?,
+            :noteable_ability_name,
             :to_ability_name,
             :editable?,
             :visible_for?,
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 2ad2838111e..012e72ece5a 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -254,6 +254,10 @@ class Milestone < ApplicationRecord
     group || project
   end
 
+  def to_ability_name
+    model_name.singular
+  end
+
   def group_milestone?
     group_id.present?
   end
diff --git a/app/models/note.rb b/app/models/note.rb
index 3956ec192b1..307f409de09 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -353,6 +353,10 @@ class Note < ApplicationRecord
   end
 
   def to_ability_name
+    model_name.singular
+  end
+
+  def noteable_ability_name
     for_snippet? ? noteable.class.name.underscore : noteable_type.demodulize.underscore
   end
 
diff --git a/app/models/project.rb b/app/models/project.rb
index a1bd5edaba9..02ccd4542f7 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1223,6 +1223,10 @@ class Project < ApplicationRecord
     end
   end
 
+  def to_ability_name
+    model_name.singular
+  end
+
   # rubocop: disable CodeReuse/ServiceClass
   def execute_hooks(data, hooks_scope = :push_hooks)
     run_after_commit_or_now do
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index b2af6c874c7..dcde8cefa0d 100644
--- a/app/policies/note_policy.rb
+++ b/app/policies/note_policy.rb
@@ -9,7 +9,7 @@ class NotePolicy < BasePolicy
 
   condition(:editable, scope: :subject) { @subject.editable? }
 
-  condition(:can_read_noteable) { can?(:"read_#{@subject.to_ability_name}") }
+  condition(:can_read_noteable) { can?(:"read_#{@subject.noteable_ability_name}") }
 
   condition(:is_visible) { @subject.visible_for?(@user) }
 
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 83710ffce2f..be213d8ceba 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -281,7 +281,7 @@ class NotificationService
   end
 
   def send_new_note_notifications(note)
-    notify_method = "note_#{note.to_ability_name}_email".to_sym
+    notify_method = "note_#{note.noteable_ability_name}_email".to_sym
 
     recipients = NotificationRecipientService.build_new_note_recipients(note)
     recipients.each do |recipient|
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 3704a2d468d..d7c522247e8 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -206,6 +206,14 @@ describe Milestone do
     end
   end
 
+  describe '#to_ability_name' do
+    it 'returns milestone' do
+      milestone = build(:milestone)
+
+      expect(milestone.to_ability_name).to eq('milestone')
+    end
+  end
+
   describe '.search' do
     let(:milestone) { create(:milestone, title: 'foo', description: 'bar') }
 
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 927fbdb93d8..5128564eacc 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -539,24 +539,30 @@ describe Note do
   end
 
   describe '#to_ability_name' do
-    it 'returns snippet for a project snippet note' do
-      expect(build(:note_on_project_snippet).to_ability_name).to eq('project_snippet')
+    it 'returns note' do
+      expect(build(:note).to_ability_name).to eq('note')
+    end
+  end
+
+  describe '#noteable_ability_name' do
+    it 'returns project_snippet for a project snippet note' do
+      expect(build(:note_on_project_snippet).noteable_ability_name).to eq('project_snippet')
     end
 
     it 'returns personal_snippet for a personal snippet note' do
-      expect(build(:note_on_personal_snippet).to_ability_name).to eq('personal_snippet')
+      expect(build(:note_on_personal_snippet).noteable_ability_name).to eq('personal_snippet')
     end
 
     it 'returns merge_request for an MR note' do
-      expect(build(:note_on_merge_request).to_ability_name).to eq('merge_request')
+      expect(build(:note_on_merge_request).noteable_ability_name).to eq('merge_request')
     end
 
     it 'returns issue for an issue note' do
-      expect(build(:note_on_issue).to_ability_name).to eq('issue')
+      expect(build(:note_on_issue).noteable_ability_name).to eq('issue')
     end
 
-    it 'returns issue for a commit note' do
-      expect(build(:note_on_commit).to_ability_name).to eq('commit')
+    it 'returns commit for a commit note' do
+      expect(build(:note_on_commit).noteable_ability_name).to eq('commit')
     end
   end
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index de5fe9ee8a8..980110073ad 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -4334,6 +4334,14 @@ describe Project do
     end
   end
 
+  describe '#to_ability_name' do
+    it 'returns project' do
+      project = build(:project_empty_repo)
+
+      expect(project.to_ability_name).to eq('project')
+    end
+  end
+
   describe '#execute_hooks' do
     let(:data) { { ref: 'refs/heads/master', data: 'data' } }
     it 'executes active projects hooks with the specified scope' do