Commit e08d1342 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch 'error-pipelines-for-blocked-users' into 'master'

Preventing blocked users and their PipelineSchdules from creating new Pipelines

Closes #47756

See merge request gitlab-org/gitlab-ce!27318
parents d87d965e 42d6d318
......@@ -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? }
......
# 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? }
......
......@@ -447,6 +447,10 @@ class ProjectPolicy < BasePolicy
prevent :owner_access
end
rule { blocked }.policy do
prevent :create_pipeline
end
private
def team_member?
......
---
title: preventing blocked users and their PipelineSchdules from creating new Pipelines
merge_request: 27318
author:
type: fixed
......@@ -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'
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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)
......
......@@ -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') }
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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) }
......
......@@ -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
......
......@@ -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) }
......
......@@ -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) }
......
......@@ -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) }
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment