Commit 6683298f authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'security-commit-private-related-mr' into 'master'

Don't allow non-members to see private related MRs

Closes #2787

See merge request gitlab/gitlabhq!2866
parents a43fd6ac 325527e6
...@@ -65,7 +65,11 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -65,7 +65,11 @@ class Projects::CommitController < Projects::ApplicationController
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def merge_requests def merge_requests
@merge_requests = @commit.merge_requests.map do |mr| @merge_requests = MergeRequestsFinder.new(
current_user,
project_id: @project.id,
commit_sha: @commit.sha
).execute.map do |mr|
{ iid: mr.iid, path: merge_request_path(mr), title: mr.title } { iid: mr.iid, path: merge_request_path(mr), title: mr.title }
end end
......
...@@ -37,13 +37,20 @@ class MergeRequestsFinder < IssuableFinder ...@@ -37,13 +37,20 @@ class MergeRequestsFinder < IssuableFinder
end end
def filter_items(_items) def filter_items(_items)
items = by_source_branch(super) items = by_commit(super)
items = by_source_branch(items)
items = by_wip(items) items = by_wip(items)
by_target_branch(items) by_target_branch(items)
end end
private private
def by_commit(items)
return items unless params[:commit_sha].presence
items.by_commit_sha(params[:commit_sha])
end
def source_branch def source_branch
@source_branch ||= params[:source_branch].presence @source_branch ||= params[:source_branch].presence
end end
......
---
title: Don't allow non-members to see private related MRs.
merge_request:
author:
type: security
...@@ -318,10 +318,18 @@ module API ...@@ -318,10 +318,18 @@ module API
use :pagination use :pagination
end end
get ':id/repository/commits/:sha/merge_requests', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do get ':id/repository/commits/:sha/merge_requests', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do
authorize! :read_merge_request, user_project
commit = user_project.commit(params[:sha]) commit = user_project.commit(params[:sha])
not_found! 'Commit' unless commit not_found! 'Commit' unless commit
present paginate(commit.merge_requests), with: Entities::MergeRequestBasic commit_merge_requests = MergeRequestsFinder.new(
current_user,
project_id: user_project.id,
commit_sha: commit.sha
).execute
present paginate(commit_merge_requests), with: Entities::MergeRequestBasic
end end
desc "Get a commit's GPG signature" do desc "Get a commit's GPG signature" do
......
...@@ -31,7 +31,7 @@ describe MergeRequestsFinder do ...@@ -31,7 +31,7 @@ describe MergeRequestsFinder do
p p
end end
end end
let(:project4) { create_project_without_n_plus_1(group: subgroup) } let(:project4) { create_project_without_n_plus_1(:repository, group: subgroup) }
let(:project5) { create_project_without_n_plus_1(group: subgroup) } let(:project5) { create_project_without_n_plus_1(group: subgroup) }
let(:project6) { create_project_without_n_plus_1(group: subgroup) } let(:project6) { create_project_without_n_plus_1(group: subgroup) }
...@@ -68,6 +68,15 @@ describe MergeRequestsFinder do ...@@ -68,6 +68,15 @@ describe MergeRequestsFinder do
expect(merge_requests.size).to eq(2) expect(merge_requests.size).to eq(2)
end end
it 'filters by commit sha' do
merge_requests = described_class.new(
user,
commit_sha: merge_request5.merge_request_diff.last_commit_sha
).execute
expect(merge_requests).to contain_exactly(merge_request5)
end
context 'filtering by group' do context 'filtering by group' do
it 'includes all merge requests when user has access' do it 'includes all merge requests when user has access' do
params = { group_id: group.id } params = { group_id: group.id }
...@@ -269,6 +278,21 @@ describe MergeRequestsFinder do ...@@ -269,6 +278,21 @@ describe MergeRequestsFinder do
expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request) expect(merge_requests).to contain_exactly(old_merge_request, new_merge_request)
end end
end end
context 'when project restricts merge requests' do
let(:non_member) { create(:user) }
let(:project) { create(:project, :repository, :public, :merge_requests_private) }
let!(:merge_request) { create(:merge_request, source_project: project) }
it "returns nothing to to non members" do
merge_requests = described_class.new(
non_member,
project_id: project.id
).execute
expect(merge_requests).to be_empty
end
end
end end
describe '#row_count', :request_store do describe '#row_count', :request_store do
......
...@@ -1430,8 +1430,8 @@ describe API::Commits do ...@@ -1430,8 +1430,8 @@ describe API::Commits do
end end
describe 'GET /projects/:id/repository/commits/:sha/merge_requests' do describe 'GET /projects/:id/repository/commits/:sha/merge_requests' do
let!(:project) { create(:project, :repository, :private) } let(:project) { create(:project, :repository, :private) }
let!(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') } let(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') }
let(:commit) { merged_mr.merge_request_diff.commits.last } let(:commit) { merged_mr.merge_request_diff.commits.last }
it 'returns the correct merge request' do it 'returns the correct merge request' do
...@@ -1456,6 +1456,17 @@ describe API::Commits do ...@@ -1456,6 +1456,17 @@ describe API::Commits do
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
context 'public project' do
let(:project) { create(:project, :repository, :public, :merge_requests_private) }
let(:non_member) { create(:user) }
it 'responds 403 when only members are allowed to read merge requests' do
get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", non_member)
expect(response).to have_gitlab_http_status(403)
end
end
end end
describe 'GET /projects/:id/repository/commits/:sha/signature' do describe 'GET /projects/:id/repository/commits/:sha/signature' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment