Commit 8824deb0 authored by Fabio Pitino's avatar Fabio Pitino

Prevent non maintainers from using user-defined variables

- Introduce restrict_user_defined_variables project setting
  and allow it to be set via API.
- Define policy for user defined variables.
- Inject user-defined variables consistently throughout
  the codebase.
- Allow user-defined variables to be set only by maintainers
  if the project setting is enabled.
- Allow passing variables from parent to child pipeline
parent 6a0199bd
......@@ -210,7 +210,8 @@ module Ci
project: downstream_project,
source: :pipeline,
target_revision: {
ref: target_ref || downstream_project.default_branch
ref: target_ref || downstream_project.default_branch,
variables_attributes: downstream_variables
},
execute_params: {
ignore_skip_ci: true,
......@@ -230,7 +231,8 @@ module Ci
checkout_sha: parent_pipeline.sha,
before: parent_pipeline.before_sha,
source_sha: parent_pipeline.source_sha,
target_sha: parent_pipeline.target_sha
target_sha: parent_pipeline.target_sha,
variables_attributes: downstream_variables
},
execute_params: {
ignore_skip_ci: true,
......
......@@ -412,6 +412,8 @@ class Project < ApplicationRecord
delegate :default_git_depth, :default_git_depth=, to: :ci_cd_settings, prefix: :ci
delegate :forward_deployment_enabled, :forward_deployment_enabled=, :forward_deployment_enabled?, to: :ci_cd_settings, prefix: :ci
delegate :keep_latest_artifact, :keep_latest_artifact=, :keep_latest_artifact?, to: :ci_cd_settings, prefix: :ci
delegate :restrict_user_defined_variables, :restrict_user_defined_variables=, :restrict_user_defined_variables?,
to: :ci_cd_settings
delegate :actual_limits, :actual_plan_name, to: :namespace, allow_nil: true
delegate :allow_merge_on_skipped_pipeline, :allow_merge_on_skipped_pipeline?,
:allow_merge_on_skipped_pipeline=, :has_confluence?, :allow_editing_commit_messages?,
......
......@@ -135,6 +135,10 @@ class ProjectPolicy < BasePolicy
::Feature.enabled?(:build_service_proxy, @subject)
end
condition(:user_defined_variables_allowed) do
!@subject.restrict_user_defined_variables?
end
with_scope :subject
condition(:packages_disabled) { !@subject.packages_enabled }
......@@ -616,6 +620,10 @@ class ProjectPolicy < BasePolicy
enable :admin_resource_access_tokens
end
rule { user_defined_variables_allowed | can?(:maintainer_access) }.policy do
enable :set_pipeline_variables
end
private
def user_is_user?
......
......@@ -33,9 +33,7 @@ module Ci
pipeline_params.fetch(:target_revision))
downstream_pipeline = service.execute(
pipeline_params.fetch(:source), **pipeline_params[:execute_params]) do |pipeline|
pipeline.variables.build(@bridge.downstream_variables)
end
pipeline_params.fetch(:source), **pipeline_params[:execute_params])
downstream_pipeline.tap do |pipeline|
update_bridge_status!(@bridge, pipeline)
......
......@@ -21,10 +21,10 @@ module Ci
# this check is to not leak the presence of the project if user cannot read it
return unless trigger.project == project
pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: params[:ref])
pipeline = Ci::CreatePipelineService
.new(project, trigger.owner, ref: params[:ref], variables_attributes: variables)
.execute(:trigger, ignore_skip_ci: true) do |pipeline|
pipeline.trigger_requests.build(trigger: trigger)
pipeline.variables.build(variables)
end
if pipeline.persisted?
......@@ -44,7 +44,8 @@ module Ci
# this check is to not leak the presence of the project if user cannot read it
return unless can?(job.user, :read_project, project)
pipeline = Ci::CreatePipelineService.new(project, job.user, ref: params[:ref])
pipeline = Ci::CreatePipelineService
.new(project, job.user, ref: params[:ref], variables_attributes: variables)
.execute(:pipeline, ignore_skip_ci: true) do |pipeline|
source = job.sourced_pipelines.build(
source_pipeline: job.pipeline,
......@@ -53,7 +54,6 @@ module Ci
project: project)
pipeline.source_pipeline = source
pipeline.variables.build(variables)
end
if pipeline.persisted?
......
......@@ -5,6 +5,10 @@ module Ci
def execute(build, job_variables_attributes = nil)
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :play_job, build)
if job_variables_attributes.present? && !can?(current_user, :set_pipeline_variables, project)
raise Gitlab::Access::AccessDeniedError
end
# Try to enqueue the build, otherwise create a duplicate.
#
if build.enqueue
......
---
title: Prevent user-defined variables from being used by non-maintainers
merge_request: 51682
author:
type: security
# frozen_string_literal: true
class AddRestrictUserDefinedVariablesToProjectSettings < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
add_column :project_ci_cd_settings, :restrict_user_defined_variables, :boolean, default: false, null: false
end
end
def down
with_lock_retries do
remove_column :project_ci_cd_settings, :restrict_user_defined_variables
end
end
end
35acb5bbabfd12f97c988776aafa6ff380e2cbe2267e856b8f439f7102a6fbf2
\ No newline at end of file
......@@ -15572,7 +15572,8 @@ CREATE TABLE project_ci_cd_settings (
forward_deployment_enabled boolean,
merge_trains_enabled boolean DEFAULT false,
auto_rollback_enabled boolean DEFAULT false NOT NULL,
keep_latest_artifact boolean DEFAULT true NOT NULL
keep_latest_artifact boolean DEFAULT true NOT NULL,
restrict_user_defined_variables boolean DEFAULT false NOT NULL
);
CREATE SEQUENCE project_ci_cd_settings_id_seq
......
......@@ -160,6 +160,7 @@ When the user is authenticated and `simple` is not set this returns something li
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -253,6 +254,7 @@ When the user is authenticated and `simple` is not set this returns something li
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -418,6 +420,7 @@ GET /users/:user_id/projects
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -511,6 +514,7 @@ GET /users/:user_id/projects
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -640,6 +644,7 @@ Example response:
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -726,6 +731,7 @@ Example response:
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -881,6 +887,7 @@ GET /projects/:id
"repository_storage": "default",
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"printing_merge_requests_link_enabled": true,
......@@ -1234,6 +1241,7 @@ PUT /projects/:id
| `packages_enabled` | boolean | **{dotted-circle}** No | Enable or disable packages repository feature. |
| `pages_access_level` | string | **{dotted-circle}** No | One of `disabled`, `private`, `enabled`, or `public`. |
| `requirements_access_level` | string | **{dotted-circle}** No | One of `disabled`, `private`, `enabled` or `public` |
| `restrict_user_defined_variables` | boolean | **{dotted-circle}** No | Allow only maintainers to pass user-defined variables when triggering a pipeline. For example when the pipeline is triggered in the UI, with the API, or by a trigger token. |
| `path` | string | **{dotted-circle}** No | Custom repository name for the project. By default generated based on name. |
| `public_builds` | boolean | **{dotted-circle}** No | If `true`, jobs can be viewed by non-project members. |
| `remove_source_branch_after_merge` | boolean | **{dotted-circle}** No | Enable `Delete source branch` option by default for all new merge requests. |
......@@ -1356,6 +1364,7 @@ Example responses:
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -1449,6 +1458,7 @@ Example response:
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -1540,6 +1550,7 @@ Example response:
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -1725,6 +1736,7 @@ Example response:
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -1837,6 +1849,7 @@ Example response:
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": false,
"restrict_user_defined_variables": false,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": false,
"request_access_enabled": false,
......@@ -2397,6 +2410,7 @@ Example response:
"shared_with_groups": [],
"only_allow_merge_if_pipeline_succeeds": false,
"allow_merge_on_skipped_pipeline": null,
"restrict_user_defined_variables": false,
"request_access_enabled": true,
"only_allow_merge_if_all_discussions_are_resolved": false,
"remove_source_branch_after_merge": true,
......
......@@ -594,7 +594,35 @@ WARNING:
Variables with multi-line values are not supported due to
limitations with the Auto DevOps scripting environment.
### Override a variable by manually running a pipeline
### When you can override variables
You can override the value of a variable when:
1. [Manually running](#override-a-variable-by-manually-running-a-pipeline) pipelines in the UI.
1. Manually creating pipelines [via API](../../api/pipelines.md#create-a-new-pipeline).
1. Manually playing a job via the UI.
1. Using [push options](../../user/project/push_options.md#push-options-for-gitlab-cicd).
1. Manually triggering pipelines with [the API](../triggers/README.md#making-use-of-trigger-variables).
1. Passing variables to a [downstream pipeline](../multi_project_pipelines.md#passing-variables-to-a-downstream-pipeline).
These pipeline variables declared in these events take [priority over other variables](#priority-of-environment-variables).
#### Restrict who can override variables
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/295234) in GitLab 13.8.
To allow only users with Maintainer role to set these variables, you can use
[the API](../../api/projects.md#edit-project) to enable the project setting `restrict_user_defined_variables`.
When a user without Maintainer role tries to run a pipeline with overridden
variables, an `Insufficient permissions to set pipeline variables` error occurs.
The setting is `disabled` by default.
If you [store your CI configurations in a different repository](../../ci/pipelines/settings.md#custom-ci-configuration-path),
use this setting for strict control over all aspects of the environment
the pipeline runs in.
#### Override a variable by manually running a pipeline
> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/44059) in GitLab 10.8.
......
......@@ -100,6 +100,7 @@ module API
end
expose :only_allow_merge_if_pipeline_succeeds
expose :allow_merge_on_skipped_pipeline
expose :restrict_user_defined_variables
expose :request_access_enabled
expose :only_allow_merge_if_all_discussions_are_resolved
expose :remove_source_branch_after_merge
......
......@@ -87,6 +87,7 @@ module API
params :optional_update_params_ce do
optional :ci_forward_deployment_enabled, type: Boolean, desc: 'Skip older deployment jobs that are still pending'
optional :restrict_user_defined_variables, type: Boolean, desc: 'Restrict use of user-defined variables when triggering a pipeline'
end
params :optional_update_params_ee do
......@@ -141,6 +142,7 @@ module API
:repository_access_level,
:request_access_enabled,
:resolve_outdated_diff_discussions,
:restrict_user_defined_variables,
:shared_runners_enabled,
:snippets_access_level,
:tag_list,
......
......@@ -5,6 +5,9 @@ module Gitlab
module Pipeline
module Chain
class Build < Chain::Base
include Gitlab::Allowable
include Chain::Helpers
def perform!
@pipeline.assign_attributes(
source: @command.source,
......@@ -20,13 +23,34 @@ module Gitlab
pipeline_schedule: @command.schedule,
merge_request: @command.merge_request,
external_pull_request: @command.external_pull_request,
variables_attributes: Array(@command.variables_attributes),
locked: @command.project.latest_pipeline_locked
locked: @command.project.latest_pipeline_locked,
variables_attributes: variables_attributes
)
end
def break?
false
@pipeline.errors.any?
end
private
def variables_attributes
variables = Array(@command.variables_attributes)
# We allow parent pipelines to pass variables to child pipelines since
# these variables are coming from internal configurations. We will check
# permissions to :set_pipeline_variables when those are injected upstream,
# to the parent pipeline.
# In other scenarios (e.g. multi-project pipelines or run pipeline via UI)
# the variables are provided from the outside and those should be guarded.
return variables if @command.creates_child_pipeline?
if variables.present? && !can?(@command.current_user, :set_pipeline_variables, @command.project)
error("Insufficient permissions to set pipeline variables")
variables = []
end
variables
end
end
end
......
......@@ -79,6 +79,10 @@ module Gitlab
bridge&.parent_pipeline
end
def creates_child_pipeline?
bridge&.triggers_child_pipeline?
end
def metrics
@metrics ||= ::Gitlab::Ci::Pipeline::Metrics.new
end
......
......@@ -45,6 +45,7 @@ FactoryBot.define do
import_correlation_id { nil }
import_last_error { nil }
forward_deployment_enabled { nil }
restrict_user_defined_variables { nil }
end
before(:create) do |project, evaluator|
......@@ -84,6 +85,7 @@ FactoryBot.define do
project.merge_pipelines_enabled = evaluator.merge_pipelines_enabled unless evaluator.merge_pipelines_enabled.nil?
project.merge_trains_enabled = evaluator.merge_trains_enabled unless evaluator.merge_trains_enabled.nil?
project.ci_keep_latest_artifact = evaluator.ci_keep_latest_artifact unless evaluator.ci_keep_latest_artifact.nil?
project.restrict_user_defined_variables = evaluator.restrict_user_defined_variables unless evaluator.restrict_user_defined_variables.nil?
if evaluator.import_status
import_state = project.import_state || project.build_import_state
......
......@@ -3,7 +3,7 @@
require 'spec_helper'
RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
let_it_be(:project) { create(:project, :repository) }
let_it_be(:project, reload: true) { create(:project, :repository) }
let_it_be(:user) { create(:user, developer_projects: [project]) }
let(:pipeline) { Ci::Pipeline.new }
......@@ -29,29 +29,96 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
let(:step) { described_class.new(pipeline, command) }
before do
stub_ci_pipeline_yaml_file(gitlab_ci_yaml)
shared_examples 'builds pipeline' do
it 'builds a pipeline with the expected attributes' do
step.perform!
expect(pipeline.sha).not_to be_empty
expect(pipeline.sha).to eq project.commit.id
expect(pipeline.ref).to eq 'master'
expect(pipeline.tag).to be false
expect(pipeline.user).to eq user
expect(pipeline.project).to eq project
end
end
it 'never breaks the chain' do
step.perform!
shared_examples 'breaks the chain' do
it 'returns true' do
step.perform!
expect(step.break?).to be false
expect(step.break?).to be true
end
end
it 'fills pipeline object with data' do
shared_examples 'does not break the chain' do
it 'returns false' do
step.perform!
expect(step.break?).to be false
end
end
before do
stub_ci_pipeline_yaml_file(gitlab_ci_yaml)
end
it_behaves_like 'does not break the chain'
it_behaves_like 'builds pipeline'
it 'sets pipeline variables' do
step.perform!
expect(pipeline.sha).not_to be_empty
expect(pipeline.sha).to eq project.commit.id
expect(pipeline.ref).to eq 'master'
expect(pipeline.tag).to be false
expect(pipeline.user).to eq user
expect(pipeline.project).to eq project
expect(pipeline.variables.map { |var| var.slice(:key, :secret_value) })
.to eq variables_attributes.map(&:with_indifferent_access)
end
context 'when project setting restrict_user_defined_variables is enabled' do
before do
project.update!(restrict_user_defined_variables: true)
end
context 'when user is developer' do
it_behaves_like 'breaks the chain'
it_behaves_like 'builds pipeline'
it 'returns an error on variables_attributes', :aggregate_failures do
step.perform!
expect(pipeline.errors.full_messages).to eq(['Insufficient permissions to set pipeline variables'])
expect(pipeline.variables).to be_empty
end
context 'when variables_attributes is not specified' do
let(:variables_attributes) { nil }
it_behaves_like 'does not break the chain'
it_behaves_like 'builds pipeline'
it 'assigns empty variables' do
step.perform!
expect(pipeline.variables).to be_empty
end
end
end
context 'when user is maintainer' do
before do
project.add_maintainer(user)
end
it_behaves_like 'does not break the chain'
it_behaves_like 'builds pipeline'
it 'assigns variables_attributes' do
step.perform!
expect(pipeline.variables.map { |var| var.slice(:key, :secret_value) })
.to eq variables_attributes.map(&:with_indifferent_access)
end
end
end
it 'returns a valid pipeline' do
step.perform!
......
......@@ -295,4 +295,30 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Command do
it { is_expected.to eq(false) }
end
end
describe '#creates_child_pipeline?' do
let(:command) { described_class.new(bridge: bridge) }
subject { command.creates_child_pipeline? }
context 'when bridge is present' do
context 'when bridge triggers a child pipeline' do
let(:bridge) { double(:bridge, triggers_child_pipeline?: true) }
it { is_expected.to be_truthy }
end
context 'when bridge triggers a multi-project pipeline' do
let(:bridge) { double(:bridge, triggers_child_pipeline?: false) }
it { is_expected.to be_falsey }
end
end
context 'when bridge is not present' do
let(:bridge) { nil }
it { is_expected.to be_falsey }
end
end
end
......@@ -401,6 +401,48 @@ RSpec.describe ProjectPolicy do
end
end
describe 'set_pipeline_variables' do
context 'when user is developer' do
let(:current_user) { developer }
context 'when project allows user defined variables' do
before do
project.update!(restrict_user_defined_variables: false)
end
it { is_expected.to be_allowed(:set_pipeline_variables) }
end
context 'when project restricts use of user defined variables' do
before do
project.update!(restrict_user_defined_variables: true)
end
it { is_expected.not_to be_allowed(:set_pipeline_variables) }
end
end
context 'when user is maintainer' do
let(:current_user) { maintainer }
context 'when project allows user defined variables' do
before do
project.update!(restrict_user_defined_variables: false)
end
it { is_expected.to be_allowed(:set_pipeline_variables) }
end
context 'when project restricts use of user defined variables' do
before do
project.update!(restrict_user_defined_variables: true)
end
it { is_expected.to be_allowed(:set_pipeline_variables) }
end
end
end
context 'support bot' do
let(:current_user) { User.support_bot }
......
......@@ -1583,6 +1583,7 @@ RSpec.describe API::Projects do
expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access)
expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds)
expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline)
expect(json_response['restrict_user_defined_variables']).to eq(project.restrict_user_defined_variables?)
expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved)
expect(json_response['operations_access_level']).to be_present
end
......@@ -1654,6 +1655,7 @@ RSpec.describe API::Projects do
expect(json_response['shared_with_groups'][0]).to have_key('expires_at')
expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds)
expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline)
expect(json_response['restrict_user_defined_variables']).to eq(project.restrict_user_defined_variables?)
expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved)
expect(json_response['ci_default_git_depth']).to eq(project.ci_default_git_depth)
expect(json_response['ci_forward_deployment_enabled']).to eq(project.ci_forward_deployment_enabled)
......@@ -2597,6 +2599,18 @@ RSpec.describe API::Projects do
expect(response).to have_gitlab_http_status(:bad_request)
end
it 'updates restrict_user_defined_variables', :aggregate_failures do
project_param = { restrict_user_defined_variables: true }
put api("/projects/#{project3.id}", user), params: project_param
expect(response).to have_gitlab_http_status(:ok)
project_param.each_pair do |k, v|
expect(json_response[k.to_s]).to eq(v)
end
end
it 'updates avatar' do
project_param = {
avatar: fixture_file_upload('spec/fixtures/banana_sample.gif',
......
......@@ -371,6 +371,26 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
end
end
context 'when downstream project does not allow user-defined variables for child pipelines' do
before do
bridge.yaml_variables = [{ key: 'BRIDGE', value: '$PIPELINE_VARIABLE-var', public: true }]
upstream_pipeline.project.update!(restrict_user_defined_variables: true)
end
it 'creates a new pipeline allowing variables to be passed downstream' do
expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
end
it 'passes variables downstream from the bridge' do
pipeline = service.execute(bridge)
pipeline.variables.map(&:key).tap do |variables|
expect(variables).to include 'BRIDGE'
end
end
end
end
end
......@@ -460,6 +480,33 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
expect(variable.value).to eq 'my-value-var'
end
end
context 'when downstream project does not allow user-defined variables for multi-project pipelines' do
before do
downstream_project.update!(restrict_user_defined_variables: true)
end
it 'does not create a new pipeline' do
expect { service.execute(bridge) }
.not_to change { Ci::Pipeline.count }
end
it 'ignores variables passed downstream from the bridge' do
pipeline = service.execute(bridge)
pipeline.variables.map(&:key).tap do |variables|
expect(variables).not_to include 'BRIDGE'
end
end
it 'sets errors', :aggregate_failures do
service.execute(bridge)
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed')
expect(bridge.options[:downstream_errors]).to eq(['Insufficient permissions to set pipeline variables'])
end
end
end
end
......
......@@ -3,14 +3,16 @@
require 'spec_helper'
RSpec.describe Ci::PipelineTriggerService do
let(:project) { create(:project, :repository) }
include AfterNextHelpers
let_it_be(:project) { create(:project, :repository) }
before do
stub_ci_pipeline_to_return_yaml_file
end
describe '#execute' do
let(:user) { create(:user) }
let_it_be(:user) { create(:user) }
let(:result) { described_class.new(project, user, params).execute }
before do
......@@ -29,8 +31,8 @@ RSpec.describe Ci::PipelineTriggerService do
end
end
context 'when params have an existsed trigger token' do
context 'when params have an existsed ref' do
context 'when params have an existing trigger token' do
context 'when params have an existing ref' do
let(:params) { { token: trigger.token, ref: 'master', variables: nil } }
it 'triggers a pipeline' do
......@@ -45,9 +47,7 @@ RSpec.describe Ci::PipelineTriggerService do
context 'when commit message has [ci skip]' do
before do
allow_next_instance_of(Ci::Pipeline) do |instance|
allow(instance).to receive(:git_commit_message) { '[ci skip]' }
end
allow_next(Ci::Pipeline).to receive(:git_commit_message) { '[ci skip]' }
end
it 'ignores [ci skip] and create as general' do
......
......@@ -72,6 +72,31 @@ RSpec.describe Ci::PlayBuildService, '#execute' do
expect(build.reload.job_variables.map(&:key)).to contain_exactly('first', 'second')
end
context 'when user defined variables are restricted' do
before do
project.update!(restrict_user_defined_variables: true)
end
context 'when user is maintainer' do
before do
project.add_maintainer(user)
end
it 'assigns the variables to the build' do
service.execute(build, job_variables)
expect(build.reload.job_variables.map(&:key)).to contain_exactly('first', 'second')
end
end
context 'when user is developer' do
it 'raises an error' do
expect { service.execute(build, job_variables) }
.to raise_error Gitlab::Access::AccessDeniedError
end
end
end
end
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