Unverified Commit 52967b10 authored by Douwe Maan's avatar Douwe Maan Committed by James Lopez

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

[10.6] Prevent notes on confidential issues from being sent to chat

See merge request gitlab/gitlabhq!2366
# Conflicts:
#	app/helpers/services_helper.rb
parent 98106ec5
module ServicesHelper
def service_event_description(event)
case event
when "push", "push_events"
"Event will be triggered by a push to the repository"
when "tag_push", "tag_push_events"
"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_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"
when "pipeline", "pipeline_events"
"Event will be triggered when a pipeline status changes"
when "wiki_page", "wiki_page_events"
"Event will be triggered when a wiki page is created/updated"
when "commit", "commit_events"
"Event will be triggered when a commit is created/updated"
end
end
def service_event_field_name(event)
event = event.pluralize if %w[merge_request issue confidential_issue].include?(event)
"#{event}_events"
......
......@@ -7,6 +7,7 @@ class ProjectHook < WebHook
:issue_hooks,
:confidential_issue_hooks,
:note_hooks,
:confidential_note_hooks,
:merge_request_hooks,
:job_hooks,
:pipeline_hooks,
......
......@@ -268,6 +268,10 @@ class Note < ActiveRecord::Base
self.special_role = Note::SpecialRole::FIRST_TIME_CONTRIBUTOR
end
def confidential?
noteable.try(:confidential?)
end
def editable?
!system?
end
......
......@@ -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
......
......@@ -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)
......
......@@ -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) }
......@@ -168,8 +170,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)
......
......@@ -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
......@@ -32,6 +32,13 @@
%strong Comments
%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
......
---
title: Adds confidential notes channel for Slack/Mattermost
merge_request:
author:
type: security
class AddConfidentialNoteEventsToWebHooks < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column :web_hooks, :confidential_note_events, :boolean
end
def down
remove_column :web_hooks, :confidential_note_events
end
end
class AddConfidentialNoteEventsToServices < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column :services, :confidential_note_events, :boolean
change_column_default :services, :confidential_note_events, true
end
def down
remove_column :services, :confidential_note_events
end
end
class ScheduleSetConfidentialNoteEventsOnWebhooks < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
BATCH_SIZE = 1_000
INTERVAL = 5.minutes
disable_ddl_transaction!
def up
migration = Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnWebhooks
migration_name = migration.to_s.demodulize
relation = migration::WebHook.hooks_to_update
queue_background_migration_jobs_by_range_at_intervals(relation,
migration_name,
INTERVAL,
batch_size: BATCH_SIZE)
end
def down
end
end
class ScheduleSetConfidentialNoteEventsOnServices < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
BATCH_SIZE = 1_000
INTERVAL = 20.minutes
disable_ddl_transaction!
def up
migration = Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnServices
migration_name = migration.to_s.demodulize
relation = migration::Service.services_to_update
queue_background_migration_jobs_by_range_at_intervals(relation,
migration_name,
INTERVAL,
batch_size: BATCH_SIZE)
end
def down
end
end
......@@ -1684,6 +1684,7 @@ ActiveRecord::Schema.define(version: 20180327101207) do
t.boolean "confidential_issues_events", default: true, null: false
t.boolean "commit_events", default: true, null: false
t.boolean "job_events", default: false, null: false
t.boolean "confidential_note_events", default: true
end
add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree
......@@ -2022,6 +2023,7 @@ ActiveRecord::Schema.define(version: 20180327101207) do
t.boolean "confidential_issues_events", default: false, null: false
t.boolean "repository_update_events", default: false, null: false
t.boolean "job_events", default: false, null: false
t.boolean "confidential_note_events"
end
add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
......
......@@ -72,7 +72,7 @@ module API
class ProjectHook < Hook
expose :project_id, :issues_events, :confidential_issues_events
expose :note_events, :pipeline_events, :wiki_page_events
expose :note_events, :confidential_note_events, :pipeline_events, :wiki_page_events
expose :job_events
end
......@@ -794,7 +794,7 @@ module API
expose :id, :title, :created_at, :updated_at, :active
expose :push_events, :issues_events, :confidential_issues_events
expose :merge_requests_events, :tag_push_events, :note_events
expose :pipeline_events, :wiki_page_events
expose :confidential_note_events, :pipeline_events, :wiki_page_events
expose :job_events
# Expose serialized properties
expose :properties do |service, options|
......
......@@ -14,6 +14,7 @@ module API
optional :merge_requests_events, type: Boolean, desc: "Trigger hook on merge request events"
optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events"
optional :note_events, type: Boolean, desc: "Trigger hook on note(comment) events"
optional :confidential_note_events, type: Boolean, desc: "Trigger hook on confidential note(comment) events"
optional :job_events, type: Boolean, desc: "Trigger hook on job events"
optional :pipeline_events, type: Boolean, desc: "Trigger hook on pipeline events"
optional :wiki_page_events, type: Boolean, desc: "Trigger hook on wiki events"
......
# frozen_string_literal: true
# rubocop:disable Style/Documentation
module Gitlab
module BackgroundMigration
# Ensures services which previously recieved all notes events continue
# to recieve confidential ones.
class SetConfidentialNoteEventsOnServices
class Service < ActiveRecord::Base
self.table_name = 'services'
include ::EachBatch
def self.services_to_update
where(confidential_note_events: nil, note_events: true)
end
end
def perform(start_id, stop_id)
Service.services_to_update
.where(id: start_id..stop_id)
.update_all(confidential_note_events: true)
end
end
end
end
# frozen_string_literal: true
# rubocop:disable Style/Documentation
module Gitlab
module BackgroundMigration
# Ensures hooks which previously recieved all notes events continue
# to recieve confidential ones.
class SetConfidentialNoteEventsOnWebhooks
class WebHook < ActiveRecord::Base
self.table_name = 'web_hooks'
include ::EachBatch
def self.hooks_to_update
where(confidential_note_events: nil, note_events: true)
end
end
def perform(start_id, stop_id)
WebHook.hooks_to_update
.where(id: start_id..stop_id)
.update_all(confidential_note_events: true)
end
end
end
end
......@@ -9,6 +9,7 @@ module Gitlab
#
# data = {
# object_kind: "note",
# event_type: "confidential_note",
# user: {
# name: String,
# username: String,
......@@ -51,8 +52,11 @@ module Gitlab
end
def build_base_data(project, user, note)
event_type = note.confidential? ? 'confidential_note' : 'note'
base_data = {
object_kind: "note",
event_type: event_type,
user: user.hook_attrs,
project_id: project.id,
project: project.hook_attrs,
......
......@@ -860,7 +860,7 @@ into similar problems in the future (e.g. when new tables are created).
# Each job is scheduled with a `delay_interval` in between.
# If you use a small interval, then some jobs may run at the same time.
#
# model_class - The table being iterated over
# model_class - The table or relation being iterated over
# job_class_name - The background migration job class as a string
# delay_interval - The duration between each job's scheduled time (must respond to `to_f`)
# batch_size - The maximum number of rows per job
......
......@@ -11,7 +11,8 @@ module Gitlab
def build(user: nil, changes: {})
hook_data = {
object_kind: issuable.class.name.underscore,
object_kind: object_kind,
event_type: event_type,
user: user.hook_attrs,
project: issuable.project.hook_attrs,
object_attributes: issuable.hook_attrs,
......@@ -36,6 +37,18 @@ module Gitlab
private
def object_kind
issuable.class.name.underscore
end
def event_type
if issuable.try(:confidential?)
"confidential_#{object_kind}"
else
object_kind
end
end
def issuable_builder
case issuable
when Issue
......
......@@ -15,6 +15,7 @@ FactoryBot.define do
issues_events true
confidential_issues_events true
note_events true
confidential_note_events true
job_events true
pipeline_events true
wiki_page_events true
......
require 'spec_helper'
describe Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnServices, :migration, schema: 20180122154930 do
let(:services) { table(:services) }
describe '#perform' do
it 'migrates services where note_events is true' do
service = services.create(confidential_note_events: nil, note_events: true)
subject.perform(service.id, service.id)
expect(service.reload.confidential_note_events).to eq(true)
end
it 'ignores services where note_events is false' do
service = services.create(confidential_note_events: nil, note_events: false)
subject.perform(service.id, service.id)
expect(service.reload.confidential_note_events).to eq(nil)
end
it 'ignores services where confidential_note_events has already been set' do
service = services.create(confidential_note_events: false, note_events: true)
subject.perform(service.id, service.id)
expect(service.reload.confidential_note_events).to eq(false)
end
end
end
require 'spec_helper'
describe Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnWebhooks, :migration, schema: 20180104131052 do
let(:web_hooks) { table(:web_hooks) }
describe '#perform' do
it 'migrates hooks where note_events is true' do
hook = web_hooks.create(confidential_note_events: nil, note_events: true)
subject.perform(hook.id, hook.id)
expect(hook.reload.confidential_note_events).to eq(true)
end
it 'ignores hooks where note_events is false' do
hook = web_hooks.create(confidential_note_events: nil, note_events: false)
subject.perform(hook.id, hook.id)
expect(hook.reload.confidential_note_events).to eq(nil)
end
it 'ignores hooks where confidential_note_events has already been set' do
hook = web_hooks.create(confidential_note_events: false, note_events: true)
subject.perform(hook.id, hook.id)
expect(hook.reload.confidential_note_events).to eq(false)
end
end
end
......@@ -55,6 +55,14 @@ describe Gitlab::DataBuilder::Note do
.to be > issue.hook_attrs['updated_at']
end
context 'with confidential issue' do
let(:issue) { create(:issue, project: project, confidential: true) }
it 'sets event_type to confidential_note' do
expect(data[:event_type]).to eq('confidential_note')
end
end
include_examples 'project hook data'
include_examples 'deprecated repository hook data'
end
......
......@@ -390,6 +390,7 @@ Service:
- default
- wiki_page_events
- confidential_issues_events
- confidential_note_events
ProjectHook:
- id
- url
......@@ -410,6 +411,7 @@ ProjectHook:
- token
- group_id
- confidential_issues_events
- confidential_note_events
- repository_update_events
ProtectedBranch:
- id
......
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20180122154930_schedule_set_confidential_note_events_on_services.rb')
describe ScheduleSetConfidentialNoteEventsOnServices, :migration, :sidekiq do
let(:services_table) { table(:services) }
let(:migration_class) { Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnServices }
let(:migration_name) { migration_class.to_s.demodulize }
let!(:service_1) { services_table.create!(confidential_note_events: nil, note_events: true) }
let!(:service_2) { services_table.create!(confidential_note_events: nil, note_events: true) }
let!(:service_migrated) { services_table.create!(confidential_note_events: true, note_events: true) }
let!(:service_skip) { services_table.create!(confidential_note_events: nil, note_events: false) }
let!(:service_new) { services_table.create!(confidential_note_events: false, note_events: true) }
let!(:service_4) { services_table.create!(confidential_note_events: nil, note_events: true) }
before do
stub_const("#{described_class}::BATCH_SIZE", 1)
end
it 'schedules background migrations at correct time' do
Sidekiq::Testing.fake! do
Timecop.freeze do
migrate!
expect(migration_name).to be_scheduled_delayed_migration(20.minutes, service_1.id, service_1.id)
expect(migration_name).to be_scheduled_delayed_migration(40.minutes, service_2.id, service_2.id)
expect(migration_name).to be_scheduled_delayed_migration(60.minutes, service_4.id, service_4.id)
expect(BackgroundMigrationWorker.jobs.size).to eq 3
end
end
end
it 'correctly processes services' do
Sidekiq::Testing.inline! do
expect(services_table.where(confidential_note_events: nil).count).to eq 4
expect(services_table.where(confidential_note_events: true).count).to eq 1
migrate!
expect(services_table.where(confidential_note_events: nil).count).to eq 1
expect(services_table.where(confidential_note_events: true).count).to eq 4
end
end
end
......@@ -4,8 +4,24 @@ require Rails.root.join('db', 'post_migrate', '20171013104327_migrate_gcp_cluste
describe MigrateGcpClustersToNewClustersArchitectures, :migration do
let(:projects) { table(:projects) }
let(:project) { projects.create }
let(:user) { create(:user) }
let(:service) { create(:kubernetes_service, project_id: project.id) }
let(:users) { table(:users) }
let(:user) { users.create! }
let(:service) { GcpMigrationSpec::KubernetesService.create!(project_id: project.id) }
module GcpMigrationSpec
class KubernetesService < ActiveRecord::Base
self.table_name = 'services'
serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize
default_value_for :active, true
default_value_for :type, 'KubernetesService'
default_value_for :properties, {
api_url: 'https://kubernetes.example.com',
token: 'a' * 40
}
end
end
context 'when cluster is being created' do
let(:project_id) { project.id }
......
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20180104131052_schedule_set_confidential_note_events_on_webhooks.rb')
describe ScheduleSetConfidentialNoteEventsOnWebhooks, :migration, :sidekiq do
let(:web_hooks_table) { table(:web_hooks) }
let(:migration_class) { Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnWebhooks }
let(:migration_name) { migration_class.to_s.demodulize }
let!(:web_hook_1) { web_hooks_table.create!(confidential_note_events: nil, note_events: true) }
let!(:web_hook_2) { web_hooks_table.create!(confidential_note_events: nil, note_events: true) }
let!(:web_hook_migrated) { web_hooks_table.create!(confidential_note_events: true, note_events: true) }
let!(:web_hook_skip) { web_hooks_table.create!(confidential_note_events: nil, note_events: false) }
let!(:web_hook_new) { web_hooks_table.create!(confidential_note_events: false, note_events: true) }
let!(:web_hook_4) { web_hooks_table.create!(confidential_note_events: nil, note_events: true) }
before do
stub_const("#{described_class}::BATCH_SIZE", 1)
end
it 'schedules background migrations at correct time' do
Sidekiq::Testing.fake! do
Timecop.freeze do
migrate!
expect(migration_name).to be_scheduled_delayed_migration(5.minutes, web_hook_1.id, web_hook_1.id)
expect(migration_name).to be_scheduled_delayed_migration(10.minutes, web_hook_2.id, web_hook_2.id)
expect(migration_name).to be_scheduled_delayed_migration(15.minutes, web_hook_4.id, web_hook_4.id)
expect(BackgroundMigrationWorker.jobs.size).to eq 3
end
end
end
it 'correctly processes web hooks' do
Sidekiq::Testing.inline! do
expect(web_hooks_table.where(confidential_note_events: nil).count).to eq 4
expect(web_hooks_table.where(confidential_note_events: true).count).to eq 1
migrate!
expect(web_hooks_table.where(confidential_note_events: nil).count).to eq 1
expect(web_hooks_table.where(confidential_note_events: true).count).to eq 4
end
end
end
......@@ -191,6 +191,21 @@ describe Note do
end
end
describe "confidential?" do
it "delegates to noteable" do
issue_note = build(:note, :on_issue)
confidential_note = build(:note, noteable: create(:issue, confidential: true))
expect(issue_note.confidential?).to be_falsy
expect(confidential_note.confidential?).to be_truthy
end
it "is falsey when noteable can't be confidential" do
commit_note = build(:note_on_commit)
expect(commit_note.confidential?).to be_falsy
end
end
describe "cross_reference_not_visible_for?" do
let(:private_user) { create(:user) }
let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_master(private_user) } }
......
......@@ -253,6 +253,21 @@ describe HipchatService do
"<b>#{title}</b>" \
"<pre>issue <strong>note</strong></pre>")
end
context 'with confidential issue' do
before do
issue.update!(confidential: true)
end
it 'calls Hipchat API with issue comment' do
data = Gitlab::DataBuilder::Note.build(issue_note, user)
hipchat.execute(data)
message = hipchat.send(:create_message, data)
expect(message).to include("<pre>issue <strong>note</strong></pre>")
end
end
end
context 'when snippet comment event triggered' do
......
......@@ -10,6 +10,22 @@ describe Service do
it { is_expected.to validate_presence_of(:type) }
end
describe 'Scopes' do
describe '.confidential_note_hooks' do
it 'includes services where confidential_note_events is true' do
create(:service, active: true, confidential_note_events: true)
expect(described_class.confidential_note_hooks.count).to eq 1
end
it 'excludes services where confidential_note_events is false' do
create(:service, active: true, confidential_note_events: false)
expect(described_class.confidential_note_hooks.count).to eq 0
end
end
end
describe "Test Button" do
describe '#can_test?' do
let(:service) { create(:service, project: project) }
......
......@@ -33,6 +33,7 @@ describe API::ProjectHooks, 'ProjectHooks' do
expect(json_response.first['merge_requests_events']).to eq(true)
expect(json_response.first['tag_push_events']).to eq(true)
expect(json_response.first['note_events']).to eq(true)
expect(json_response.first['confidential_note_events']).to eq(true)
expect(json_response.first['job_events']).to eq(true)