Commit 29d8b4ee authored by Sean McGivern's avatar Sean McGivern Committed by DJ Mountney

Merge branch '29364-private-projects-mr-fix'

Don’t show source project name when user does not have access

See merge request !2081
parent 2e8aa209
...@@ -21,7 +21,9 @@ module MergeRequests ...@@ -21,7 +21,9 @@ module MergeRequests
delegate :target_branch, :source_branch, :source_project, :target_project, :compare_commits, :wip_title, :description, :errors, to: :merge_request delegate :target_branch, :source_branch, :source_project, :target_project, :compare_commits, :wip_title, :description, :errors, to: :merge_request
def find_source_project def find_source_project
source_project || project return source_project if source_project.present? && can?(current_user, :read_project, source_project)
project
end end
def find_target_project def find_target_project
......
---
title: Don’t show source project name when user does not have access
merge_request:
author:
...@@ -70,6 +70,18 @@ feature 'Create New Merge Request', feature: true, js: true do ...@@ -70,6 +70,18 @@ feature 'Create New Merge Request', feature: true, js: true do
visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { target_project_id: private_project.id }) visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { target_project_id: private_project.id })
expect(page).not_to have_content private_project.path_with_namespace expect(page).not_to have_content private_project.path_with_namespace
expect(page).to have_content project.path_with_namespace
end
end
context 'when source project cannot be viewed by the current user' do
it 'does not leak the private project name & namespace' do
private_project = create(:project, :private)
visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { source_project_id: private_project.id })
expect(page).not_to have_content private_project.path_with_namespace
expect(page).to have_content project.path_with_namespace
end end
end end
......
...@@ -4,6 +4,8 @@ describe MergeRequests::BuildService, services: true do ...@@ -4,6 +4,8 @@ describe MergeRequests::BuildService, services: true do
include RepoHelpers include RepoHelpers
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:source_project) { nil }
let(:target_project) { nil }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:issue_confidential) { false } let(:issue_confidential) { false }
let(:issue) { create(:issue, project: project, title: 'A bug', confidential: issue_confidential) } let(:issue) { create(:issue, project: project, title: 'A bug', confidential: issue_confidential) }
...@@ -20,7 +22,9 @@ describe MergeRequests::BuildService, services: true do ...@@ -20,7 +22,9 @@ describe MergeRequests::BuildService, services: true do
MergeRequests::BuildService.new(project, user, MergeRequests::BuildService.new(project, user,
description: description, description: description,
source_branch: source_branch, source_branch: source_branch,
target_branch: target_branch) target_branch: target_branch,
source_project: source_project,
target_project: target_project)
end end
before do before do
...@@ -256,5 +260,41 @@ describe MergeRequests::BuildService, services: true do ...@@ -256,5 +260,41 @@ describe MergeRequests::BuildService, services: true do
) )
end end
end end
context 'target_project is set and accessible by current_user' do
let(:target_project) { create(:project, :public, :repository)}
let(:commits) { Commit.decorate([commit_1], project) }
it 'sets target project correctly' do
expect(merge_request.target_project).to eq(target_project)
end
end
context 'target_project is set but not accessible by current_user' do
let(:target_project) { create(:project, :private, :repository)}
let(:commits) { Commit.decorate([commit_1], project) }
it 'sets target project correctly' do
expect(merge_request.target_project).to eq(project)
end
end
context 'source_project is set and accessible by current_user' do
let(:source_project) { create(:project, :public, :repository)}
let(:commits) { Commit.decorate([commit_1], project) }
it 'sets target project correctly' do
expect(merge_request.source_project).to eq(source_project)
end
end
context 'source_project is set but not accessible by current_user' do
let(:source_project) { create(:project, :private, :repository)}
let(:commits) { Commit.decorate([commit_1], project) }
it 'sets target project correctly' do
expect(merge_request.source_project).to eq(project)
end
end
end end
end end
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