summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/policies/base_policy.rb4
-rw-r--r--app/policies/global_policy.rb4
-rw-r--r--app/policies/project_policy.rb4
-rw-r--r--changelogs/unreleased/error-pipelines-for-blocked-users.yml5
-rw-r--r--spec/factories/users.rb10
-rw-r--r--spec/features/commits_spec.rb6
-rw-r--r--spec/features/projects/pipelines/pipeline_spec.rb39
-rw-r--r--spec/features/projects/pipelines/pipelines_spec.rb2
-rw-r--r--spec/finders/pipelines_finder_spec.rb5
-rw-r--r--spec/javascripts/fixtures/pipelines.rb2
-rw-r--r--spec/lib/gitlab/ci/pipeline/chain/build_spec.rb2
-rw-r--r--spec/models/ci/pipeline_spec.rb2
-rw-r--r--spec/models/project_services/hipchat_service_spec.rb2
-rw-r--r--spec/services/ci/create_pipeline_service_spec.rb12
-rw-r--r--spec/services/ci/play_build_service_spec.rb37
-rw-r--r--spec/services/notification_service_spec.rb6
-rw-r--r--spec/views/notify/pipeline_failed_email.html.haml_spec.rb2
-rw-r--r--spec/views/notify/pipeline_failed_email.text.erb_spec.rb2
-rw-r--r--spec/views/notify/pipeline_success_email.html.haml_spec.rb2
-rw-r--r--spec/workers/auto_devops/disable_worker_spec.rb3
20 files changed, 106 insertions, 45 deletions
diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb
index 5dd2279ef99..82bf9bf8bf6 100644
--- a/app/policies/base_policy.rb
+++ b/app/policies/base_policy.rb
@@ -7,6 +7,10 @@ class BasePolicy < DeclarativePolicy::Base
with_options scope: :user, score: 0
condition(:admin) { @user&.admin? }
+ desc "User is blocked"
+ with_options scope: :user, score: 0
+ condition(:blocked) { @user&.blocked? }
+
desc "User has access to all private groups & projects"
with_options scope: :user, score: 0
condition(:full_private_access) { @user&.full_private_access? }
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
index e85397422e6..134de1c9ace 100644
--- a/app/policies/global_policy.rb
+++ b/app/policies/global_policy.rb
@@ -1,10 +1,6 @@
# frozen_string_literal: true
class GlobalPolicy < BasePolicy
- desc "User is blocked"
- with_options scope: :user, score: 0
- condition(:blocked) { @user&.blocked? }
-
desc "User is an internal user"
with_options scope: :user, score: 0
condition(:internal) { @user&.internal? }
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index a3632640ede..b3e29e775fc 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -447,6 +447,10 @@ class ProjectPolicy < BasePolicy
prevent :owner_access
end
+ rule { blocked }.policy do
+ prevent :create_pipeline
+ end
+
private
def team_member?
diff --git a/changelogs/unreleased/error-pipelines-for-blocked-users.yml b/changelogs/unreleased/error-pipelines-for-blocked-users.yml
new file mode 100644
index 00000000000..3ace28b6cfd
--- /dev/null
+++ b/changelogs/unreleased/error-pipelines-for-blocked-users.yml
@@ -0,0 +1,5 @@
+---
+title: preventing blocked users and their PipelineSchdules from creating new Pipelines
+merge_request: 27318
+author:
+type: fixed
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 1d2b724a5e5..4f3392cdcbf 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -66,6 +66,16 @@ FactoryBot.define do
end
end
+ transient do
+ developer_projects []
+ end
+
+ after(:create) do |user, evaluator|
+ evaluator.developer_projects.each do |project|
+ project.add_developer(user)
+ end
+ end
+
factory :omniauth_user do
transient do
extern_uid '123456'
diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb
index e6f44aa7d20..a2dd34e7f7c 100644
--- a/spec/features/commits_spec.rb
+++ b/spec/features/commits_spec.rb
@@ -10,8 +10,7 @@ describe 'Commits' do
stub_ci_pipeline_to_return_yaml_file
end
- let(:creator) { create(:user) }
-
+ let(:creator) { create(:user, developer_projects: [project]) }
let!(:pipeline) do
create(:ci_pipeline,
project: project,
@@ -77,10 +76,11 @@ describe 'Commits' do
describe 'Commit builds', :js do
before do
+ project.add_developer(user)
visit pipeline_path(pipeline)
end
- it 'shows pipeline`s data' do
+ it 'shows pipeline data' do
expect(page).to have_content pipeline.sha[0..7]
expect(page).to have_content pipeline.git_commit_message.gsub!(/\s+/, ' ')
expect(page).to have_content pipeline.user.name
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 77f0f237d0a..9759fd04ad2 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -5,6 +5,7 @@ require 'spec_helper'
describe 'Pipeline', :js do
include RoutesHelpers
include ProjectForksHelper
+ include ::ExclusiveLeaseHelpers
let(:project) { create(:project) }
let(:user) { create(:user) }
@@ -539,6 +540,44 @@ describe 'Pipeline', :js do
expect(page).to have_selector('.pipeline-visualization')
expect(page).to have_content('cross-build')
end
+
+ context 'when a scheduled pipeline is created by a blocked user' do
+ let(:project) { create(:project, :repository) }
+
+ let(:schedule) do
+ create(:ci_pipeline_schedule,
+ project: project,
+ owner: project.owner,
+ description: 'blocked user schedule'
+ ).tap do |schedule|
+ schedule.update_column(:next_run_at, 1.minute.ago)
+ end
+ end
+
+ before do
+ schedule.owner.block!
+
+ begin
+ PipelineScheduleWorker.new.perform
+ rescue Ci::CreatePipelineService::CreateError
+ # Do nothing, assert view code after the Pipeline failed to create.
+ end
+ end
+
+ it 'displays the PipelineSchedule in an active state' do
+ visit project_pipeline_schedules_path(project)
+ page.click_link('Active')
+
+ expect(page).to have_selector('table.ci-table > tbody > tr > td', text: 'blocked user schedule')
+ end
+
+ it 'does not create a new Pipeline' do
+ visit project_pipelines_path(project)
+
+ expect(page).not_to have_selector('.ci-table')
+ expect(schedule.last_pipeline).to be_nil
+ end
+ end
end
describe 'GET /:project/pipelines/:id/builds' do
diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb
index de780f13681..885d5f85989 100644
--- a/spec/features/projects/pipelines/pipelines_spec.rb
+++ b/spec/features/projects/pipelines/pipelines_spec.rb
@@ -469,7 +469,7 @@ describe 'Pipelines', :js do
visit_project_pipelines
end
- it 'has artifats' do
+ it 'has artifacts' do
expect(page).to have_selector('.build-artifacts')
end
diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb
index c2c304589c9..b23fd8ccdc6 100644
--- a/spec/finders/pipelines_finder_spec.rb
+++ b/spec/finders/pipelines_finder_spec.rb
@@ -170,8 +170,9 @@ describe PipelinesFinder do
context 'when order_by and sort are specified' do
context 'when order_by user_id' do
- let(:params) { { order_by: 'user_id', sort: 'asc' } }
- let!(:pipelines) { Array.new(2) { create(:ci_pipeline, project: project, user: create(:user)) } }
+ let(:params) { { order_by: 'user_id', sort: 'asc' } }
+ let(:users) { Array.new(2) { create(:user, developer_projects: [project]) } }
+ let!(:pipelines) { users.map { |user| create(:ci_pipeline, project: project, user: user) } }
it 'sorts as user_id: :asc' do
is_expected.to match_array(pipelines)
diff --git a/spec/javascripts/fixtures/pipelines.rb b/spec/javascripts/fixtures/pipelines.rb
index de6fcfe10f4..6b6b0eefab9 100644
--- a/spec/javascripts/fixtures/pipelines.rb
+++ b/spec/javascripts/fixtures/pipelines.rb
@@ -8,7 +8,7 @@ describe Projects::PipelinesController, '(JavaScript fixtures)', type: :controll
let(:project) { create(:project, :repository, namespace: namespace, path: 'pipelines-project') }
let(:commit) { create(:commit, project: project) }
let(:commit_without_author) { RepoHelpers.another_sample_commit }
- let!(:user) { create(:user, email: commit.author_email) }
+ let!(:user) { create(:user, developer_projects: [project], email: commit.author_email) }
let!(:pipeline) { create(:ci_pipeline, project: project, sha: commit.id, user: user) }
let!(:pipeline_without_author) { create(:ci_pipeline, project: project, sha: commit_without_author.id) }
let!(:pipeline_without_commit) { create(:ci_pipeline, project: project, sha: '0000') }
diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb
index 3debd42ac65..50cb45c39d1 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
describe Gitlab::Ci::Pipeline::Chain::Build do
set(:project) { create(:project, :repository) }
- set(:user) { create(:user) }
+ set(:user) { create(:user, developer_projects: [project]) }
let(:pipeline) { Ci::Pipeline.new }
let(:variables_attributes) do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index a8701f0efa4..c4e54be673f 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2736,7 +2736,7 @@ describe Ci::Pipeline, :mailer do
create(:ci_pipeline,
project: project,
sha: project.commit('master').sha,
- user: create(:user))
+ user: project.owner)
end
before do
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index a04b984c1f6..a1bd0855708 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/spec/models/project_services/hipchat_service_spec.rb
@@ -301,7 +301,7 @@ describe HipchatService do
end
context 'pipeline events' do
- let(:pipeline) { create(:ci_empty_pipeline, user: create(:user)) }
+ let(:pipeline) { create(:ci_empty_pipeline, user: project.owner) }
let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) }
context 'for failed' do
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 867692d4d64..d9b61dfe503 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -1132,5 +1132,17 @@ describe Ci::CreatePipelineService do
.with_message('Insufficient permissions to create a new pipeline')
end
end
+
+ context 'when a user with permissions has been blocked' do
+ before do
+ user.block!
+ end
+
+ it 'raises an error' do
+ expect { subject }
+ .to raise_error(described_class::CreateError)
+ .with_message('Insufficient permissions to create a new pipeline')
+ end
+ end
end
end
diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb
index 634f865e2d8..1e68b7956ea 100644
--- a/spec/services/ci/play_build_service_spec.rb
+++ b/spec/services/ci/play_build_service_spec.rb
@@ -3,7 +3,7 @@
require 'spec_helper'
describe Ci::PlayBuildService, '#execute' do
- let(:user) { create(:user) }
+ let(:user) { create(:user, developer_projects: [project]) }
let(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: project) }
let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
@@ -16,8 +16,6 @@ describe Ci::PlayBuildService, '#execute' do
let(:project) { create(:project) }
it 'allows user to play build if protected branch rules are met' do
- project.add_developer(user)
-
create(:protected_branch, :developers_can_merge,
name: build.ref, project: project)
@@ -27,8 +25,6 @@ describe Ci::PlayBuildService, '#execute' do
end
it 'does not allow user with developer role to play build' do
- project.add_developer(user)
-
expect { service.execute(build) }
.to raise_error Gitlab::Access::AccessDeniedError
end
@@ -38,23 +34,21 @@ describe Ci::PlayBuildService, '#execute' do
let(:project) { create(:project, :repository) }
it 'allows user with developer role to play a build' do
- project.add_developer(user)
-
service.execute(build)
expect(build.reload).to be_pending
end
+
+ it 'prevents a blocked developer from playing a build' do
+ user.block!
+
+ expect { service.execute(build) }.to raise_error(Gitlab::Access::AccessDeniedError)
+ end
end
context 'when build is a playable manual action' do
let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
-
- before do
- project.add_developer(user)
-
- create(:protected_branch, :developers_can_merge,
- name: build.ref, project: project)
- end
+ let!(:branch) { create(:protected_branch, :developers_can_merge, name: build.ref, project: project) }
it 'enqueues the build' do
expect(service.execute(build)).to eq build
@@ -70,13 +64,7 @@ describe Ci::PlayBuildService, '#execute' do
context 'when build is not a playable manual action' do
let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) }
-
- before do
- project.add_developer(user)
-
- create(:protected_branch, :developers_can_merge,
- name: build.ref, project: project)
- end
+ let!(:branch) { create(:protected_branch, :developers_can_merge, name: build.ref, project: project) }
it 'duplicates the build' do
duplicate = service.execute(build)
@@ -94,6 +82,7 @@ describe Ci::PlayBuildService, '#execute' do
end
context 'when build is not action' do
+ let(:user) { create(:user) }
let(:build) { create(:ci_build, :success, pipeline: pipeline) }
it 'raises an error' do
@@ -103,10 +92,8 @@ describe Ci::PlayBuildService, '#execute' do
end
context 'when user does not have ability to trigger action' do
- before do
- create(:protected_branch, :no_one_can_push,
- name: build.ref, project: project)
- end
+ let(:user) { create(:user) }
+ let!(:branch) { create(:protected_branch, :developers_can_merge, name: build.ref, project: project) }
it 'raises an error' do
expect { service.execute(build) }
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 4b40c86574f..f25e2fe5e2b 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -2217,10 +2217,12 @@ describe NotificationService, :mailer do
let(:pipeline) { create(:ci_pipeline, :failed, project: project, user: pipeline_user) }
it 'emails project owner and user that triggered the pipeline' do
+ project.add_developer(pipeline_user)
+
notification.autodevops_disabled(pipeline, [owner.email, pipeline_user.email])
- should_email(owner)
- should_email(pipeline_user)
+ should_email(owner, times: 1) # Once for the disable pipeline.
+ should_email(pipeline_user, times: 2) # Once for the new permission, once for the disable.
end
end
end
diff --git a/spec/views/notify/pipeline_failed_email.html.haml_spec.rb b/spec/views/notify/pipeline_failed_email.html.haml_spec.rb
index d04affc7df1..623237b111a 100644
--- a/spec/views/notify/pipeline_failed_email.html.haml_spec.rb
+++ b/spec/views/notify/pipeline_failed_email.html.haml_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
describe 'notify/pipeline_failed_email.html.haml' do
include Devise::Test::ControllerHelpers
- let(:user) { create(:user) }
+ let(:user) { create(:user, developer_projects: [project]) }
let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) }
diff --git a/spec/views/notify/pipeline_failed_email.text.erb_spec.rb b/spec/views/notify/pipeline_failed_email.text.erb_spec.rb
index 079fb865d7b..81245239eba 100644
--- a/spec/views/notify/pipeline_failed_email.text.erb_spec.rb
+++ b/spec/views/notify/pipeline_failed_email.text.erb_spec.rb
@@ -4,7 +4,7 @@ require 'spec_helper'
describe 'notify/pipeline_failed_email.text.erb' do
include Devise::Test::ControllerHelpers
- let(:user) { create(:user) }
+ let(:user) { create(:user, developer_projects: [project]) }
let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) }
diff --git a/spec/views/notify/pipeline_success_email.html.haml_spec.rb b/spec/views/notify/pipeline_success_email.html.haml_spec.rb
index 8ee7f954d70..a876bf13e11 100644
--- a/spec/views/notify/pipeline_success_email.html.haml_spec.rb
+++ b/spec/views/notify/pipeline_success_email.html.haml_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
describe 'notify/pipeline_success_email.html.haml' do
include Devise::Test::ControllerHelpers
- let(:user) { create(:user) }
+ let(:user) { create(:user, developer_projects: [project]) }
let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request, :simple, source_project: project) }
diff --git a/spec/workers/auto_devops/disable_worker_spec.rb b/spec/workers/auto_devops/disable_worker_spec.rb
index 800a07e41a5..53113f286ba 100644
--- a/spec/workers/auto_devops/disable_worker_spec.rb
+++ b/spec/workers/auto_devops/disable_worker_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
describe AutoDevops::DisableWorker, '#perform' do
- let(:user) { create(:user) }
+ let(:user) { create(:user, developer_projects: [project]) }
let(:project) { create(:project, :repository, :auto_devops) }
let(:auto_devops) { project.auto_devops }
let(:pipeline) { create(:ci_pipeline, :failed, :auto_devops_source, project: project, user: user) }
@@ -10,6 +10,7 @@ describe AutoDevops::DisableWorker, '#perform' do
subject { described_class.new }
before do
+ project.add_developer(user)
stub_application_setting(auto_devops_enabled: true)
auto_devops.update_attribute(:enabled, nil)
end