Commit b1dad8b2 authored by Nick Thomas's avatar Nick Thomas

Check permissions before showing a forked project's source

parent dbd50b6e
......@@ -110,17 +110,24 @@ module ProjectsHelper
{ project_full_name: project.full_name }
end
def remove_fork_project_message(project)
_("You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?") %
{ forked_from_project: fork_source_name(project) }
end
def remove_fork_project_description_message(project)
source = visible_fork_source(project)
def fork_source_name(project)
if @project.fork_source
@project.fork_source.full_name
if source
_('This will remove the fork relationship between this project and %{fork_source}.') %
{ fork_source: link_to(source.full_name, project_path(source)) }
else
@project.fork_network&.deleted_root_project_name
_('This will remove the fork relationship between this project and other projects in the fork network.')
end
end
def remove_fork_project_warning_message(project)
_("You are going to remove the fork relationship from %{project_full_name}. Are you ABSOLUTELY sure?") %
{ project_full_name: project.full_name }
end
def visible_fork_source(project)
project.fork_source if project.fork_source && can?(current_user, :read_project, project.fork_source)
end
def project_nav_tabs
......
......@@ -74,13 +74,12 @@
- if @project.forked?
%p
- if @project.fork_source
- source = visible_fork_source(@project)
- if source
#{ s_('ForkedFromProjectPath|Forked from') }
= link_to project_path(@project.fork_source) do
= fork_source_name(@project)
= link_to source.full_name, project_path(source)
- else
- deleted_message = s_('ForkedFromProjectPath|Forked from %{project_name} (deleted)')
= deleted_message % { project_name: fork_source_name(@project) }
= s_('ForkedFromProjectPath|Forked from an inaccessible project')
= render_if_exists "projects/home_mirror"
......
......@@ -126,17 +126,12 @@
- if @project.forked? && can?(current_user, :remove_fork_project, @project)
.sub-section
%h4.danger-title= _('Remove fork relationship')
%p
= _('This will remove the fork relationship to source project')
= succeed "." do
- if @project.fork_source
= link_to(fork_source_name(@project), project_path(@project.fork_source))
- else
= fork_source_name(@project)
%p= remove_fork_project_description_message(@project)
= form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_project_path(@project), method: :delete, remote: true, html: { class: 'transfer-project' }) do |f|
%p
%strong= _('Once removed, the fork relationship cannot be restored and you will no longer be able to send merge requests to the source.')
= button_to _('Remove fork relationship'), '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) }
= button_to _('Remove fork relationship'), '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_warning_message(@project) }
- if can?(current_user, :remove_project, @project)
.sub-section
......
---
title: Check permissions before showing a forked project's source
merge_request:
author:
type: security
......@@ -283,7 +283,9 @@ module API
expose :shared_runners_enabled
expose :lfs_enabled?, as: :lfs_enabled
expose :creator_id
expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? }
expose :forked_from_project, using: Entities::BasicProjectDetails, if: ->(project, options) do
project.forked? && Ability.allowed?(options[:current_user], :read_project, project.forked_from_project)
end
expose :import_status
expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } do |project|
......
......@@ -7686,7 +7686,7 @@ msgstr ""
msgid "ForkedFromProjectPath|Forked from"
msgstr ""
msgid "ForkedFromProjectPath|Forked from %{project_name} (deleted)"
msgid "ForkedFromProjectPath|Forked from an inaccessible project"
msgstr ""
msgid "Forking in progress"
......@@ -17890,7 +17890,10 @@ msgstr ""
msgid "This will redirect you to an external sign in page."
msgstr ""
msgid "This will remove the fork relationship to source project"
msgid "This will remove the fork relationship between this project and %{fork_source}."
msgstr ""
msgid "This will remove the fork relationship between this project and other projects in the fork network."
msgstr ""
msgid "Those emails automatically become issues (with the comments becoming the email conversation) listed here."
......@@ -19812,7 +19815,7 @@ msgstr ""
msgid "You are going to remove %{project_full_name}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?"
msgstr ""
msgid "You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?"
msgid "You are going to remove the fork relationship from %{project_full_name}. Are you ABSOLUTELY sure?"
msgstr ""
msgid "You are going to transfer %{project_full_name} to another owner. Are you ABSOLUTELY sure?"
......
......@@ -202,13 +202,13 @@ describe 'Project' do
expect(page).not_to have_content('Forked from')
end
it 'shows the name of the deleted project when the source was deleted', :sidekiq_might_not_need_inline do
it 'does not show the name of the deleted project when the source was deleted', :sidekiq_might_not_need_inline do
forked_project
Projects::DestroyService.new(base_project, base_project.owner).execute
visit project_path(forked_project)
expect(page).to have_content("Forked from #{base_project.full_name} (deleted)")
expect(page).to have_content('Forked from an inaccessible project')
end
context 'a fork of a fork' do
......
......@@ -49,6 +49,8 @@ shared_examples 'languages and percentages JSON response' do
end
describe API::Projects do
include ProjectForksHelper
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:user3) { create(:user) }
......@@ -1163,6 +1165,18 @@ describe API::Projects do
expect(json_response.keys).not_to include('permissions')
end
context 'the project is a public fork' do
it 'hides details of a public fork parent' do
public_project = create(:project, :repository, :public)
fork = fork_project(public_project)
get api("/projects/#{fork.id}")
expect(response).to have_gitlab_http_status(200)
expect(json_response['forked_from_project']).to be_nil
end
end
context 'and the project has a private repository' do
let(:project) { create(:project, :repository, :public, :repository_private) }
let(:protected_attributes) { %w(default_branch ci_config_path) }
......@@ -1479,6 +1493,28 @@ describe API::Projects do
end
end
context 'the project is a fork' do
it 'shows details of a visible fork parent' do
fork = fork_project(project, user)
get api("/projects/#{fork.id}", user)
expect(response).to have_gitlab_http_status(200)
expect(json_response['forked_from_project']).to include('id' => project.id)
end
it 'hides details of a hidden fork parent' do
fork = fork_project(project, user)
fork_user = create(:user)
fork.team.add_developer(fork_user)
get api("/projects/#{fork.id}", fork_user)
expect(response).to have_gitlab_http_status(200)
expect(json_response['forked_from_project']).to be_nil
end
end
describe 'permissions' do
context 'all projects' do
before do
......
......@@ -3,6 +3,8 @@
require 'spec_helper'
describe 'projects/_home_panel' do
include ProjectForksHelper
context 'notifications' do
let(:project) { create(:project) }
......@@ -144,4 +146,36 @@ describe 'projects/_home_panel' do
end
end
end
context 'forks' do
let(:source_project) { create(:project, :repository) }
let(:project) { fork_project(source_project) }
let(:user) { create(:user) }
before do
assign(:project, project)
allow(view).to receive(:current_user).and_return(user)
end
context 'user can read fork source' do
it 'shows the forked-from project' do
allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(true)
render
expect(rendered).to have_content("Forked from #{source_project.full_name}")
end
end
context 'user cannot read fork source' do
it 'does not show the forked-from project' do
allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(false)
render
expect(rendered).to have_content("Forked from an inaccessible project")
end
end
end
end
......@@ -4,6 +4,7 @@ require 'spec_helper'
describe 'projects/edit' do
include Devise::Test::ControllerHelpers
include ProjectForksHelper
let(:project) { create(:project) }
let(:user) { create(:admin) }
......@@ -26,4 +27,59 @@ describe 'projects/edit' do
expect(rendered).not_to have_content('Export project')
end
end
context 'forking' do
before do
assign(:project, project)
allow(view).to receive(:current_user).and_return(user)
end
context 'project is not a fork' do
it 'hides the remove fork relationship settings' do
render
expect(rendered).not_to have_content('Remove fork relationship')
end
end
context 'project is a fork' do
let(:source_project) { create(:project) }
let(:project) { fork_project(source_project) }
it 'shows the remove fork relationship settings to an authorized user' do
allow(view).to receive(:can?).with(user, :remove_fork_project, project).and_return(true)
render
expect(rendered).to have_content('Remove fork relationship')
end
it 'hides the fork relationship settings from an unauthorized user' do
allow(view).to receive(:can?).with(user, :remove_fork_project, project).and_return(false)
render
expect(rendered).not_to have_content('Remove fork relationship')
end
it 'hides the fork source from an unauthorized user' do
allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(false)
render
expect(rendered).to have_content('Remove fork relationship')
expect(rendered).not_to have_content(source_project.full_name)
end
it 'shows the fork source to an authorized user' do
allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(true)
render
expect(rendered).to have_content('Remove fork relationship')
expect(rendered).to have_content(source_project.full_name)
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