Merge branch '18028-respect-fork-project' into 'security'

Enforce the fork_project permission in Projects::CreateService

Projects::ForkService delegates to this service almost entirely, but needed one small change so it would propagate create errors correctly.

CreateService#execute needs significant refactoring; it is now right at the complexity limit set by Rubocop. I avoided doing so in this commit to keep the diff as small as possible.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/18028

See merge request !1996

2 files changed, 14 insertions, 0 deletions

diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index be749ba4a1c..696fe3efe8f 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -15,6 +15,11 @@ module Projects
         return @project
       end
 
+      unless allowed_fork?(forked_from_project_id)
+        @project.errors.add(:forked_from_project_id, 'is forbidden')
+        return @project
+      end
+
       # Set project name from path
       if @project.name.present? && @project.path.present?
         # if both name and path set - everything is ok
@@ -71,6 +76,13 @@ module Projects
       @project.errors.add(:namespace, "is not valid")
     end
 
+    def allowed_fork?(source_project_id)
+      return true if source_project_id.nil?
+
+      source_project = Project.find_by(id: source_project_id)
+      current_user.can?(:fork_project, source_project)
+    end
+
     def allowed_namespace?(user, namespace_id)
       namespace = Namespace.find_by(id: namespace_id)
       current_user.can?(:create_projects, namespace)
diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb
index a2de4dccece..a2b23ea6171 100644
--- a/app/services/projects/fork_service.rb
+++ b/app/services/projects/fork_service.rb
@@ -16,6 +16,8 @@ module Projects
       end
 
       new_project = CreateService.new(current_user, new_params).execute
+      return new_project unless new_project.persisted?
+
       builds_access_level = @project.project_feature.builds_access_level
       new_project.project_feature.update_attributes(builds_access_level: builds_access_level)