Use MergeRequest#source_project as permissions reference for MergeRequest#all_pipelines
MergeRequest#all_pipelines fetches Ci::Pipeline records from the source project, so we should specifically check that project for permissions. This was already happening for intra-project merge requests, but in the event that the target and source projects both have private builds, we should ensure that the project permissions are respected.
This commit is contained in:
parent
2ad75a4f96
commit
019caa8de5
4 changed files with 102 additions and 6 deletions
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Use source project as permissions reference for MergeRequestsController#pipelines
|
||||
merge_request:
|
||||
author:
|
||||
type: security
|
|
@ -623,10 +623,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
|
||||
|
||||
|
|
Loading…
Reference in a new issue