diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 2232e231cf8..8b25332b73c 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -5,7 +5,7 @@ module Ci # If we can't read build we should also not have that # ability when looking at this in context of commit_status - %w(read create update admin).each do |rule| + %w[read create update admin].each do |rule| cannot! :"#{rule}_commit_status" unless can? :"#{rule}_build" end end diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb new file mode 100644 index 00000000000..3d2eef1c50c --- /dev/null +++ b/app/policies/ci/pipeline_policy.rb @@ -0,0 +1,4 @@ +module Ci + class PipelinePolicy < BuildPolicy + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 2cc9a9fd7bf..f48255b2e6c 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -489,9 +489,14 @@ class NotificationService end def reject_users_without_access(recipients, target) - return recipients unless target.is_a?(Issuable) + ability = case target + when Issuable + :"read_#{target.to_ability_name}" + when Ci::Pipeline + :read_build # We have build trace in pipeline emails + end - ability = :"read_#{target.to_ability_name}" + return recipients unless ability recipients.select do |user| user.can?(ability, target) diff --git a/spec/workers/pipeline_notification_worker_spec.rb b/spec/workers/pipeline_notification_worker_spec.rb index c334b2057a6..d487a719680 100644 --- a/spec/workers/pipeline_notification_worker_spec.rb +++ b/spec/workers/pipeline_notification_worker_spec.rb @@ -17,84 +17,114 @@ describe PipelineNotificationWorker do describe '#execute' do before do reset_delivered_emails! - pipeline.project.team << [watcher, Gitlab::Access::DEVELOPER] + pipeline.project.team << [pusher, Gitlab::Access::DEVELOPER] end - shared_examples 'sending emails' do - it 'sends emails' do + context 'when watcher has developer access' do + before do + pipeline.project.team << [watcher, Gitlab::Access::DEVELOPER] + end + + shared_examples 'sending emails' do + it 'sends emails' do + perform_enqueued_jobs do + subject.perform(pipeline.id) + end + + emails = ActionMailer::Base.deliveries + actual = emails.flat_map(&:bcc).sort + expected_receivers = receivers.map(&:email).uniq.sort + + expect(actual).to eq(expected_receivers) + expect(emails.size).to eq(1) + expect(emails.last.subject).to include(email_subject) + end + end + + context 'with success pipeline' do + let(:status) { 'success' } + let(:email_subject) { "Pipeline ##{pipeline.id} has succeeded" } + let(:receivers) { [pusher, watcher] } + + it_behaves_like 'sending emails' + + context 'with pipeline from someone else' do + let(:pusher) { create(:user) } + let(:watcher) { user } + + context 'with success pipeline notification on' do + before do + watcher.global_notification_setting. + update(level: 'custom', success_pipeline: true) + end + + it_behaves_like 'sending emails' + end + + context 'with success pipeline notification off' do + let(:receivers) { [pusher] } + + before do + watcher.global_notification_setting. + update(level: 'custom', success_pipeline: false) + end + + it_behaves_like 'sending emails' + end + end + + context 'with failed pipeline' do + let(:status) { 'failed' } + let(:email_subject) { "Pipeline ##{pipeline.id} has failed" } + + it_behaves_like 'sending emails' + + context 'with pipeline from someone else' do + let(:pusher) { create(:user) } + let(:watcher) { user } + + context 'with failed pipeline notification on' do + before do + watcher.global_notification_setting. + update(level: 'custom', failed_pipeline: true) + end + + it_behaves_like 'sending emails' + end + + context 'with failed pipeline notification off' do + let(:receivers) { [pusher] } + + before do + watcher.global_notification_setting. + update(level: 'custom', failed_pipeline: false) + end + + it_behaves_like 'sending emails' + end + end + end + end + end + + context 'when watcher has no read_build access' do + let(:status) { 'failed' } + let(:email_subject) { "Pipeline ##{pipeline.id} has failed" } + let(:watcher) { create(:user) } + + before do + pipeline.project.team << [watcher, Gitlab::Access::GUEST] + + watcher.global_notification_setting. + update(level: 'custom', failed_pipeline: true) + perform_enqueued_jobs do subject.perform(pipeline.id) end - - emails = ActionMailer::Base.deliveries - actual = emails.flat_map(&:bcc).sort - expected_receivers = [pusher, watcher].map(&:email).uniq.sort - - expect(actual).to eq(expected_receivers) - expect(emails.size).to eq(1) - expect(emails.last.subject).to include(email_subject) end - end - context 'with success pipeline' do - let(:status) { 'success' } - let(:email_subject) { "Pipeline ##{pipeline.id} has succeeded" } - - it_behaves_like 'sending emails' - - context 'with pipeline from someone else' do - let(:pusher) { create(:user) } - - context 'with success pipeline notification on' do - let(:watcher) { user } - - before do - watcher.global_notification_setting. - update(level: 'custom', success_pipeline: true) - end - - it_behaves_like 'sending emails' - end - - context 'with success pipeline notification off' do - before do - watcher.global_notification_setting. - update(level: 'custom', success_pipeline: false) - end - - it_behaves_like 'sending emails' - end - end - end - - context 'with failed pipeline' do - let(:status) { 'failed' } - let(:email_subject) { "Pipeline ##{pipeline.id} has failed" } - - it_behaves_like 'sending emails' - - context 'with pipeline from someone else' do - let(:pusher) { create(:user) } - - context 'with failed pipeline notification on' do - let(:watcher) { user } - - before do - watcher.global_notification_setting. - update(level: 'custom', failed_pipeline: true) - end - - it_behaves_like 'sending emails' - end - - context 'with failed pipeline notification off' do - before do - watcher.global_notification_setting. - update(level: 'custom', failed_pipeline: false) - end - - it_behaves_like 'sending emails' - end + it 'does not send emails' do + should_only_email(pusher, kind: :bcc) end end end