Permission fix for MergeRequestsController#pipeline_status

- Use set_pipeline_variables to filter for visible pipelines
- Mimic response of nonexistent pipeline if not found
- Provide set_pipeline_variables as a before_filter for other actions
This commit is contained in:
drew cimino 2019-07-22 14:16:15 -04:00
parent 9757636074
commit 1c7c91806d
3 changed files with 39 additions and 5 deletions

View File

@ -188,7 +188,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
def pipeline_status
render json: PipelineSerializer
.new(project: @project, current_user: @current_user)
.represent_status(@merge_request.head_pipeline)
.represent_status(head_pipeline)
end
def ci_environments_status
@ -238,6 +238,13 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
private
def head_pipeline
strong_memoize(:head_pipeline) do
pipeline = @merge_request.head_pipeline
pipeline if can?(current_user, :read_pipeline, pipeline)
end
end
def ci_environments_status_on_merge_result?
params[:environment_target] == 'merge_commit'
end

View File

@ -0,0 +1,5 @@
---
title: Check permissions before responding in MergeController#pipeline_status
merge_request:
author:
type: security

View File

@ -1052,17 +1052,39 @@ describe Projects::MergeRequestsController do
let(:status) { pipeline.detailed_status(double('user')) }
before do
it 'returns a detailed head_pipeline status in json' do
get_pipeline_status
end
it 'return a detailed head_pipeline status in json' do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['text']).to eq status.text
expect(json_response['label']).to eq status.label
expect(json_response['icon']).to eq status.icon
expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.png"
end
context 'with project member visibility on a public project' do
let(:user) { create(:user) }
let(:project) { create(:project, :repository, :public, :builds_private) }
it 'returns pipeline data to project members' do
project.add_developer(user)
get_pipeline_status
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['text']).to eq status.text
expect(json_response['label']).to eq status.label
expect(json_response['icon']).to eq status.icon
expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.png"
end
it 'returns blank OK response to non-project-members' do
get_pipeline_status
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_empty
end
end
end
context 'when head_pipeline does not exist' do
@ -1070,7 +1092,7 @@ describe Projects::MergeRequestsController do
get_pipeline_status
end
it 'return empty' do
it 'returns blank OK response' do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_empty
end