summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-08-26 07:42:14 +0000
committerGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-08-26 07:42:14 +0000
commit239a6dc18b34f5e54e8c2ae8167fdc7a3a440004 (patch)
tree6ab17c4acb655240335307e39fe907a65f9e6bb6
parent7d9fd99d3b1feb5366beae3f7b3f4d3d4ca9c5ee (diff)
parent211c3baec312eb588e2a3e8177a114d3c179d359 (diff)
downloadgitlab-ce-239a6dc18b34f5e54e8c2ae8167fdc7a3a440004.tar.gz
Merge branch 'security-mr-head-pipeline-leak-12-2' into '12-2-stable'
Permission fix for MergeRequestsController#pipeline_status See merge request gitlab/gitlabhq!3322
-rw-r--r--app/controllers/projects/merge_requests_controller.rb9
-rw-r--r--changelogs/unreleased/security-mr-head-pipeline-leak.yml5
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb30
3 files changed, 39 insertions, 5 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index c9a1f28f87e..a2e6f878f90 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -189,7 +189,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
@@ -239,6 +239,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
diff --git a/changelogs/unreleased/security-mr-head-pipeline-leak.yml b/changelogs/unreleased/security-mr-head-pipeline-leak.yml
new file mode 100644
index 00000000000..b15b353ff41
--- /dev/null
+++ b/changelogs/unreleased/security-mr-head-pipeline-leak.yml
@@ -0,0 +1,5 @@
+---
+title: Check permissions before responding in MergeController#pipeline_status
+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 73a13447a3a..89572cc4f3b 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -1096,17 +1096,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
@@ -1114,7 +1136,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