diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-07-26 19:50:06 +0300 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-07-26 19:50:06 +0300 |
commit | 1e6410cde2e7130a78e672da5f5230e4c57bd195 (patch) | |
tree | 92c06035f32bab18c9985a6b5fdc3aefa3944df6 | |
parent | cd66293dfc46e5cd08015485e18b4770e5c89607 (diff) | |
parent | 70676286e05c9a8bea36919535b20a087392f137 (diff) |
Merge branch 'security-mr-pipeline-permissions-12-1' into '12-1-stable'
MR pipeline permissions
See merge request gitlab/gitlabhq!3281
4 files changed, 102 insertions, 6 deletions
diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index dcc272aecff..006731c0e66 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -45,7 +45,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont def set_pipeline_variables @pipelines = - if can?(current_user, :read_pipeline, @project) + if can?(current_user, :read_pipeline, @merge_request.source_project) @merge_request.all_pipelines else Ci::Pipeline.none diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 2aa2508be16..f4d381244d9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -82,7 +82,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end def pipelines - @pipelines = @merge_request.all_pipelines.page(params[:page]).per(30) + set_pipeline_variables + @pipelines = @pipelines.page(params[:page]).per(30) Gitlab::PollingInterval.set_header(response, interval: 10_000) diff --git a/changelogs/unreleased/security-mr-pipeline-permissions.yml b/changelogs/unreleased/security-mr-pipeline-permissions.yml new file mode 100644 index 00000000000..a317c93228c --- /dev/null +++ b/changelogs/unreleased/security-mr-pipeline-permissions.yml @@ -0,0 +1,5 @@ +--- +title: Use source project as permissions reference for MergeRequestsController#pipelines +merge_request: +author: +type: security diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index f11880122b1..bdd7322290f 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -621,10 +621,100 @@ describe Projects::MergeRequestsController do format: :json end - it 'responds with serialized pipelines' do - expect(json_response['pipelines']).not_to be_empty - expect(json_response['count']['all']).to eq 1 - expect(response).to include_pagination_headers + context 'with "enabled" builds on a public project' do + let(:project) { create(:project, :repository, :public) } + + context 'for a project owner' do + it 'responds with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + + context 'for an unassociated user' do + let(:user) { create :user } + + it 'responds with no pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + end + + context 'with private builds on a public project' do + let(:project) { create(:project, :repository, :public, :builds_private) } + + context 'for a project owner' do + it 'responds with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + + context 'for an unassociated user' do + let(:user) { create :user } + + it 'responds with no pipelines' do + expect(json_response['pipelines']).to be_empty + expect(json_response['count']['all']).to eq(0) + expect(response).to include_pagination_headers + end + end + + context 'from a project fork' do + let(:fork_user) { create :user } + let(:forked_project) { fork_project(project, fork_user, repository: true) } # Forked project carries over :builds_private + let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: forked_project) } + + context 'with private builds' do + context 'for the target project member' do + it 'does not respond with serialized pipelines' do + expect(json_response['pipelines']).to be_empty + expect(json_response['count']['all']).to eq(0) + expect(response).to include_pagination_headers + end + end + + context 'for the source project member' do + let(:user) { fork_user } + + it 'responds with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + end + + context 'with public builds' do + let(:forked_project) do + fork_project(project, fork_user, repository: true).tap do |new_project| + new_project.project_feature.update(builds_access_level: ProjectFeature::ENABLED) + end + end + + context 'for the target project member' do + it 'does not respond with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + + context 'for the source project member' do + let(:user) { fork_user } + + it 'responds with serialized pipelines' do + expect(json_response['pipelines']).to be_present + expect(json_response['count']['all']).to eq(1) + expect(response).to include_pagination_headers + end + end + end + end end end |