Merge branch 'jej/mattermost-notification-confidentiality' into 'security-10-4'

Prevent notes on confidential issues from being sent to chat

See merge request gitlab/gitlabhq!2248

9 files changed, 39 insertions, 8 deletions

diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 85d35900c71..3e4235dd4f8 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -70,6 +70,7 @@ class Projects::HooksController < Projects::ApplicationController
       :confidential_issues_events,
       :merge_requests_events,
       :note_events,
+      :confidential_note_events,
       :push_events,
       :tag_push_events,
       :token,
diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb
index 240783bc7fd..f872990122e 100644
--- a/app/helpers/services_helper.rb
+++ b/app/helpers/services_helper.rb
@@ -7,9 +7,11 @@ module ServicesHelper
       "Event will be triggered when a new tag is pushed to the repository"
     when "note", "note_events"
       "Event will be triggered when someone adds a comment"
+    when "confidential_note", "confidential_note_events"
+      "Event will be triggered when someone adds a comment on a confidential issue"
     when "issue", "issue_events"
       "Event will be triggered when an issue is created/updated/closed"
-    when "confidential_issue", "confidential_issue_events"
+    when "confidential_issue", "confidential_issues_events"
       "Event will be triggered when a confidential issue is created/updated/closed"
     when "merge_request", "merge_request_events"
       "Event will be triggered when a merge request is created/updated/merged"
diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index a8c424a6614..4914468b84e 100644
--- a/app/models/hooks/project_hook.rb
+++ b/app/models/hooks/project_hook.rb
@@ -5,6 +5,7 @@ class ProjectHook < WebHook
     issue_hooks:              :issues_events,
     confidential_issue_hooks: :confidential_issues_events,
     note_hooks:               :note_events,
+    confidential_note_hooks:  :confidential_note_events,
     merge_request_hooks:      :merge_requests_events,
     job_hooks:                :job_events,
     pipeline_hooks:           :pipeline_events,
diff --git a/app/models/note.rb b/app/models/note.rb
index 184fbd5f5ae..bed1e64dd72 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -267,6 +267,10 @@ class Note < ActiveRecord::Base
     self.special_role = Note::SpecialRole::FIRST_TIME_CONTRIBUTOR
   end
 
+  def confidential?
+    noteable.try(:confidential?)
+  end
+
   def editable?
     !system?
   end
diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb
index 818cfb01b14..39f0c7641cd 100644
--- a/app/models/project_services/chat_notification_service.rb
+++ b/app/models/project_services/chat_notification_service.rb
@@ -21,8 +21,16 @@ class ChatNotificationService < Service
     end
   end
 
+  def confidential_issue_channel
+    properties['confidential_issue_channel'].presence || properties['issue_channel']
+  end
+
+  def confidential_note_channel
+    properties['confidential_note_channel'].presence || properties['note_channel']
+  end
+
   def self.supported_events
-    %w[push issue confidential_issue merge_request note tag_push
+    %w[push issue confidential_issue merge_request note confidential_note tag_push
        pipeline wiki_page]
   end
 
@@ -55,7 +63,9 @@ class ChatNotificationService < Service
 
     return false unless message
 
-    channel_name = get_channel_field(object_kind).presence || channel
+    event_type = data[:event_type] || object_kind
+
+    channel_name = get_channel_field(event_type).presence || channel
 
     opts = {}
     opts[:channel] = channel_name if channel_name
diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb
index 768f0a7472e..64947fc855a 100644
--- a/app/models/project_services/hipchat_service.rb
+++ b/app/models/project_services/hipchat_service.rb
@@ -46,7 +46,7 @@ class HipchatService < Service
   end
 
   def self.supported_events
-    %w(push issue confidential_issue merge_request note tag_push pipeline)
+    %w(push issue confidential_issue merge_request note confidential_note tag_push pipeline)
   end
 
   def execute(data)
diff --git a/app/models/service.rb b/app/models/service.rb
index df49b945cd6..d8c49d43995 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -14,6 +14,7 @@ class Service < ActiveRecord::Base
   default_value_for :merge_requests_events, true
   default_value_for :tag_push_events, true
   default_value_for :note_events, true
+  default_value_for :confidential_note_events, true
   default_value_for :job_events, true
   default_value_for :pipeline_events, true
   default_value_for :wiki_page_events, true
@@ -42,6 +43,7 @@ class Service < ActiveRecord::Base
   scope :confidential_issue_hooks, -> { where(confidential_issues_events: true, active: true) }
   scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) }
   scope :note_hooks, -> { where(note_events: true, active: true) }
+  scope :confidential_note_hooks, -> { where(confidential_note_events: true, active: true) }
   scope :job_hooks, -> { where(job_events: true, active: true) }
   scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) }
   scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) }
@@ -162,8 +164,10 @@ class Service < ActiveRecord::Base
   def self.prop_accessor(*args)
     args.each do |arg|
       class_eval %{
-        def #{arg}
-          properties['#{arg}']
+        unless method_defined?(arg)
+          def #{arg}
+            properties['#{arg}']
+          end
         end
 
         def #{arg}=(value)
diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb
index 6a10e172483..4b9e7bb695b 100644
--- a/app/services/notes/post_process_service.rb
+++ b/app/services/notes/post_process_service.rb
@@ -24,8 +24,10 @@ module Notes
 
     def execute_note_hooks
       note_data = hook_data
-      @note.project.execute_hooks(note_data, :note_hooks)
-      @note.project.execute_services(note_data, :note_hooks)
+      hooks_scope = @note.confidential? ? :confidential_note_hooks : :note_hooks
+
+      @note.project.execute_hooks(note_data, hooks_scope)
+      @note.project.execute_services(note_data, hooks_scope)
     end
   end
 end
diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml
index 1f0e7629fb4..3dba9c099c4 100644
--- a/app/views/shared/web_hooks/_form.html.haml
+++ b/app/views/shared/web_hooks/_form.html.haml
@@ -33,6 +33,13 @@
         %p.light
           This URL will be triggered when someone adds a comment
     %li
+      = form.check_box :confidential_note_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :confidential_note_events, class: 'list-label' do
+          %strong Confidential Comments
+        %p.light
+          This URL will be triggered when someone adds a comment on a confidential issue
+    %li
       = form.check_box :issues_events, class: 'pull-left'
       .prepend-left-20
         = form.label :issues_events, class: 'list-label' do