diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-04-03 09:57:41 +0000 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2018-04-03 14:53:19 +0100 |
commit | b7d6225a448b1191b0c982ca73343f4172dbd999 (patch) | |
tree | db0b8789af3efeeea269c003bd19c966694118bb /spec | |
parent | 3bac94ee97217252ca1bb8849628564a5932ab6f (diff) | |
download | gitlab-ce-b7d6225a448b1191b0c982ca73343f4172dbd999.tar.gz |
Merge branch 'jej/mattermost-notification-confidentiality-10-5' into 'security-10-5'
[10.5] Prevent notes on confidential issues from being sent to chat
See merge request gitlab/gitlabhq!2339
Diffstat (limited to 'spec')
14 files changed, 306 insertions, 9 deletions
diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb index 493b7bc021c..a448d565e4b 100644 --- a/spec/factories/project_hooks.rb +++ b/spec/factories/project_hooks.rb @@ -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 diff --git a/spec/lib/gitlab/background_migration/set_confidential_note_events_on_services_spec.rb b/spec/lib/gitlab/background_migration/set_confidential_note_events_on_services_spec.rb new file mode 100644 index 00000000000..6f3fb994f17 --- /dev/null +++ b/spec/lib/gitlab/background_migration/set_confidential_note_events_on_services_spec.rb @@ -0,0 +1,31 @@ +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 diff --git a/spec/lib/gitlab/background_migration/set_confidential_note_events_on_webhooks_spec.rb b/spec/lib/gitlab/background_migration/set_confidential_note_events_on_webhooks_spec.rb new file mode 100644 index 00000000000..82b484b7d5b --- /dev/null +++ b/spec/lib/gitlab/background_migration/set_confidential_note_events_on_webhooks_spec.rb @@ -0,0 +1,31 @@ +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 diff --git a/spec/lib/gitlab/data_builder/note_spec.rb b/spec/lib/gitlab/data_builder/note_spec.rb index aaa42566a4d..4f8412108ba 100644 --- a/spec/lib/gitlab/data_builder/note_spec.rb +++ b/spec/lib/gitlab/data_builder/note_spec.rb @@ -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 diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index feaab6673cd..8887b07ac0e 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -386,6 +386,7 @@ Service: - default - wiki_page_events - confidential_issues_events +- confidential_note_events ProjectHook: - id - url @@ -406,6 +407,7 @@ ProjectHook: - token - group_id - confidential_issues_events +- confidential_note_events - repository_update_events ProtectedBranch: - id diff --git a/spec/migrations/active_record/schedule_set_confidential_note_events_on_services_spec.rb b/spec/migrations/active_record/schedule_set_confidential_note_events_on_services_spec.rb new file mode 100644 index 00000000000..4395e2f8264 --- /dev/null +++ b/spec/migrations/active_record/schedule_set_confidential_note_events_on_services_spec.rb @@ -0,0 +1,44 @@ +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 diff --git a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb index c81ec887ded..df009cec25c 100644 --- a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb @@ -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 } diff --git a/spec/migrations/schedule_set_confidential_note_events_on_webhooks_spec.rb b/spec/migrations/schedule_set_confidential_note_events_on_webhooks_spec.rb new file mode 100644 index 00000000000..027f4a91c90 --- /dev/null +++ b/spec/migrations/schedule_set_confidential_note_events_on_webhooks_spec.rb @@ -0,0 +1,44 @@ +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 diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index c853f707e6d..86962cd8d61 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -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) } } diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 23db29cb541..8ff76a0b968 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -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 diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 79f25dc4360..a4e85d4e2c9 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -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) } diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 1fd082ecc38..85b4b1eddab 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -28,10 +28,12 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response.count).to eq(1) expect(json_response.first['url']).to eq("http://example.com") expect(json_response.first['issues_events']).to eq(true) + expect(json_response.first['confidential_issues_events']).to eq(true) expect(json_response.first['push_events']).to eq(true) 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) expect(json_response.first['pipeline_events']).to eq(true) expect(json_response.first['wiki_page_events']).to eq(true) @@ -56,10 +58,12 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(response).to have_gitlab_http_status(200) expect(json_response['url']).to eq(hook.url) expect(json_response['issues_events']).to eq(hook.issues_events) + expect(json_response['confidential_issues_events']).to eq(hook.issues_events) expect(json_response['push_events']).to eq(hook.push_events) expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) + expect(json_response['confidential_note_events']).to eq(hook.confidential_note_events) expect(json_response['job_events']).to eq(hook.job_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) @@ -97,10 +101,12 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(response).to have_gitlab_http_status(201) expect(json_response['url']).to eq('http://example.com') expect(json_response['issues_events']).to eq(true) + expect(json_response['confidential_issues_events']).to eq(false) expect(json_response['push_events']).to eq(true) expect(json_response['merge_requests_events']).to eq(false) expect(json_response['tag_push_events']).to eq(false) expect(json_response['note_events']).to eq(false) + expect(json_response['confidential_note_events']).to eq(nil) expect(json_response['job_events']).to eq(true) expect(json_response['pipeline_events']).to eq(false) expect(json_response['wiki_page_events']).to eq(true) @@ -144,10 +150,12 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(response).to have_gitlab_http_status(200) expect(json_response['url']).to eq('http://example.org') expect(json_response['issues_events']).to eq(hook.issues_events) + expect(json_response['confidential_issues_events']).to eq(hook.confidential_issues_events) expect(json_response['push_events']).to eq(false) expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) + expect(json_response['confidential_note_events']).to eq(hook.confidential_note_events) expect(json_response['job_events']).to eq(hook.job_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) diff --git a/spec/services/notes/post_process_service_spec.rb b/spec/services/notes/post_process_service_spec.rb index 6ef5e93cb20..4e2ab919f0f 100644 --- a/spec/services/notes/post_process_service_spec.rb +++ b/spec/services/notes/post_process_service_spec.rb @@ -23,5 +23,23 @@ describe Notes::PostProcessService do described_class.new(@note).execute end + + context 'with a confidential issue' do + let(:issue) { create(:issue, :confidential, project: project) } + + it "doesn't call note hooks/services" do + expect(project).not_to receive(:execute_hooks).with(anything, :note_hooks) + expect(project).not_to receive(:execute_services).with(anything, :note_hooks) + + described_class.new(@note).execute + end + + it "calls confidential-note hooks/services" do + expect(project).to receive(:execute_hooks).with(anything, :confidential_note_hooks) + expect(project).to receive(:execute_services).with(anything, :confidential_note_hooks) + + described_class.new(@note).execute + end + end end end diff --git a/spec/support/slack_mattermost_notifications_shared_examples.rb b/spec/support/slack_mattermost_notifications_shared_examples.rb index e827a8da0b7..328581bb668 100644 --- a/spec/support/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/slack_mattermost_notifications_shared_examples.rb @@ -4,6 +4,11 @@ RSpec.shared_examples 'slack or mattermost notifications' do let(:chat_service) { described_class.new } let(:webhook_url) { 'https://example.gitlab.com/' } + def execute_with_options(options) + receive(:new).with(webhook_url, options) + .and_return(double(:slack_service).as_null_object) + end + describe "Associations" do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } @@ -33,6 +38,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do let(:project) { create(:project, :repository) } let(:username) { 'slack_username' } let(:channel) { 'slack_channel' } + let(:issue_service_options) { { title: 'Awesome issue', description: 'please fix' } } let(:push_sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) @@ -48,12 +54,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do WebMock.stub_request(:post, webhook_url) - opts = { - title: 'Awesome issue', - description: 'please fix' - } - - issue_service = Issues::CreateService.new(project, user, opts) + issue_service = Issues::CreateService.new(project, user, issue_service_options) @issue = issue_service.execute @issues_sample_data = issue_service.hook_data(@issue, 'open') @@ -164,6 +165,26 @@ RSpec.shared_examples 'slack or mattermost notifications' do chat_service.execute(@issues_sample_data) end + context 'for confidential issues' do + let(:issue_service_options) { { title: 'Secret', confidential: true } } + + it "uses confidential issue channel" do + chat_service.update_attributes(confidential_issue_channel: 'confidential') + + expect(Slack::Notifier).to execute_with_options(channel: 'confidential') + + chat_service.execute(@issues_sample_data) + end + + it 'falls back to issue channel' do + chat_service.update_attributes(issue_channel: 'fallback_channel') + + expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel') + + chat_service.execute(@issues_sample_data) + end + end + it "uses the right channel for wiki event" do chat_service.update_attributes(wiki_page_channel: "random") @@ -194,6 +215,32 @@ RSpec.shared_examples 'slack or mattermost notifications' do chat_service.execute(note_data) end + + context 'for confidential notes' do + before do + issue_note.noteable.update!(confidential: true) + end + + it "uses confidential channel" do + chat_service.update_attributes(confidential_note_channel: "confidential") + + note_data = Gitlab::DataBuilder::Note.build(issue_note, user) + + expect(Slack::Notifier).to execute_with_options(channel: 'confidential') + + chat_service.execute(note_data) + end + + it 'falls back to note channel' do + chat_service.update_attributes(note_channel: "fallback_channel") + + note_data = Gitlab::DataBuilder::Note.build(issue_note, user) + + expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel') + + chat_service.execute(note_data) + end + end end end end @@ -248,8 +295,9 @@ RSpec.shared_examples 'slack or mattermost notifications' do create(:note_on_issue, project: project, note: "issue note") end + let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) } + it "calls Slack API for issue comment events" do - data = Gitlab::DataBuilder::Note.build(issue_note, user) chat_service.execute(data) expect(WebMock).to have_requested(:post, webhook_url).once |