summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-08-26 07:41:52 +0000
committerGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-08-26 07:41:52 +0000
commitcb87b3156b090977efaf98689a2e090d5d34031b (patch)
treef3df0979ef26f9e89b21885c8f0f4ed7edc92b12
parent0ca9d12655399ee78cd58ac9523f77730d9af117 (diff)
parentacf29759100d1e0e83ac2d289231f342656dbe94 (diff)
downloadgitlab-ce-cb87b3156b090977efaf98689a2e090d5d34031b.tar.gz
Merge branch 'security-ci-metrics-permissions-12-2' into '12-2-stable'
Restrict MergeRequests#test_reports to authenticated users with read-access on Builds See merge request gitlab/gitlabhq!3354
-rw-r--r--app/controllers/projects/merge_requests_controller.rb6
-rw-r--r--changelogs/unreleased/security-ci-metrics-permissions.yml6
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb60
3 files changed, 64 insertions, 8 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index f4d381244d9..c9a1f28f87e 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -12,6 +12,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
skip_before_action :merge_request, only: [:index, :bulk_update]
before_action :whitelist_query_limiting, only: [:assign_related_issues, :update]
before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort]
+ before_action :authorize_test_reports!, only: [:test_reports]
before_action :set_issuables_index, only: [:index]
before_action :authenticate_user!, only: [:assign_related_issues]
before_action :check_user_can_push_to_source_branch!, only: [:rebase]
@@ -336,4 +337,9 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
render json: { status_reason: 'Unknown error' }, status: :internal_server_error
end
end
+
+ def authorize_test_reports!
+ # MergeRequest#actual_head_pipeline is the pipeline accessed in MergeRequest#compare_reports.
+ return render_404 unless can?(current_user, :read_build, merge_request.actual_head_pipeline)
+ end
end
diff --git a/changelogs/unreleased/security-ci-metrics-permissions.yml b/changelogs/unreleased/security-ci-metrics-permissions.yml
new file mode 100644
index 00000000000..51c6493442a
--- /dev/null
+++ b/changelogs/unreleased/security-ci-metrics-permissions.yml
@@ -0,0 +1,6 @@
+---
+title: Restrict MergeRequests#test_reports to authenticated users with read-access
+ on Builds
+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 b1dc6a65dd4..73a13447a3a 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -719,19 +719,63 @@ describe Projects::MergeRequestsController do
end
describe 'GET test_reports' do
+ let(:merge_request) do
+ create(:merge_request,
+ :with_diffs,
+ :with_merge_request_pipeline,
+ target_project: project,
+ source_project: project
+ )
+ end
+
subject do
- get :test_reports,
- params: {
- namespace_id: project.namespace.to_param,
- project_id: project,
- id: merge_request.iid
- },
- format: :json
+ get :test_reports, params: {
+ namespace_id: project.namespace.to_param,
+ project_id: project,
+ id: merge_request.iid
+ },
+ format: :json
end
before do
allow_any_instance_of(MergeRequest)
- .to receive(:compare_test_reports).and_return(comparison_status)
+ .to receive(:compare_test_reports)
+ .and_return(comparison_status)
+
+ allow_any_instance_of(MergeRequest)
+ .to receive(:actual_head_pipeline)
+ .and_return(merge_request.all_pipelines.take)
+ end
+
+ describe 'permissions on a public project with private CI/CD' do
+ let(:project) { create :project, :repository, :public, :builds_private }
+ let(:comparison_status) { { status: :parsed, data: { summary: 1 } } }
+
+ context 'while signed out' do
+ before do
+ sign_out(user)
+ end
+
+ it 'responds with a 404' do
+ subject
+
+ expect(response).to have_gitlab_http_status(404)
+ expect(response.body).to be_blank
+ end
+ end
+
+ context 'while signed in as an unrelated user' do
+ before do
+ sign_in(create(:user))
+ end
+
+ it 'responds with a 404' do
+ subject
+
+ expect(response).to have_gitlab_http_status(404)
+ expect(response.body).to be_blank
+ end
+ end
end
context 'when comparison is being processed' do