summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/helpers/projects_helper.rb23
-rw-r--r--app/views/projects/_home_panel.html.haml9
-rw-r--r--app/views/projects/edit.html.haml11
-rw-r--r--changelogs/unreleased/security-28802-respect-fork-parent-visibility-ee.yml5
-rw-r--r--lib/api/entities.rb4
-rw-r--r--locale/gitlab.pot9
-rw-r--r--spec/features/projects_spec.rb4
-rw-r--r--spec/requests/api/projects_spec.rb36
-rw-r--r--spec/views/projects/_home_panel.html.haml_spec.rb34
-rw-r--r--spec/views/projects/edit.html.haml_spec.rb56
10 files changed, 164 insertions, 27 deletions
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 64d61bba71a..466c782fc77 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -110,19 +110,26 @@ 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
@nav_tabs ||= get_project_nav_tabs(@project, current_user)
end
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index b7c4114d485..daedc52f298 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -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"
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index b5e24cbbffb..328fdd0be10 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -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
diff --git a/changelogs/unreleased/security-28802-respect-fork-parent-visibility-ee.yml b/changelogs/unreleased/security-28802-respect-fork-parent-visibility-ee.yml
new file mode 100644
index 00000000000..8872b73a0cc
--- /dev/null
+++ b/changelogs/unreleased/security-28802-respect-fork-parent-visibility-ee.yml
@@ -0,0 +1,5 @@
+---
+title: Check permissions before showing a forked project's source
+merge_request:
+author:
+type: security
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 21648b0f59a..cba4ec2c18f 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -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|
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a4f92ede7dc..120eec7a903 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -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?"
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index 90e48f3c230..47f32e0113c 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -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
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index f1447536e0f..cda2dd7d2f4 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -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
diff --git a/spec/views/projects/_home_panel.html.haml_spec.rb b/spec/views/projects/_home_panel.html.haml_spec.rb
index 4d5b369e88e..9956144b601 100644
--- a/spec/views/projects/_home_panel.html.haml_spec.rb
+++ b/spec/views/projects/_home_panel.html.haml_spec.rb
@@ -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
diff --git a/spec/views/projects/edit.html.haml_spec.rb b/spec/views/projects/edit.html.haml_spec.rb
index f576093ab45..40927a22dc4 100644
--- a/spec/views/projects/edit.html.haml_spec.rb
+++ b/spec/views/projects/edit.html.haml_spec.rb
@@ -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
View Lorry Controller Status